Summary:
Previously,
* `RawBuf` was a typealias for `std::unique_ptr<folly::IOBuf>`
* `Buf` was a typealias for `folly::IOBuf`
In this diff,
* `Buf` is a typealias for `folly::IOBuf`
* `BufPtr` is a typealias for `std::unique_ptr<folly::IOBuf>`
Reviewed By: hanidamlaj
Differential Revision: D73206576
fbshipit-source-id: 454bf6ccfce3d6571e5e931889263ed98cc24af3
Summary: More in the theme of returning Expected instead of throwing. For the folly case, we keep the try/catches in there and translate to Expected. For Libev, we convert directly to Expected.
Reviewed By: kvtsoy
Differential Revision: D73217128
fbshipit-source-id: d00a978f24e3b29a77a8ac99a19765ae49f64df8
Summary: `messageSizes` is supposed to be the number of iovecs in each message, and not the length of the iovec. This was causing crashes in Edgeray (T215909454).
Reviewed By: jbeshay
Differential Revision: D70129535
fbshipit-source-id: 6d79e6ac0789402674e753297984b3691d371338
Summary: The `SendmmsgGSOInplacePacketBatchWriter` is equivalent to the `SendmmsgGSOPacketBatchWriter`, except that it operates on the in-place buffer.
Reviewed By: jbeshay
Differential Revision: D69410775
fbshipit-source-id: 32782f9797e7e2354cbafabf956985b96900f59b
Summary: I'm switching the usages of `writem` to use iovecs instead of IOBufs.
Reviewed By: kvtsoy
Differential Revision: D68857701
fbshipit-source-id: 96461aac85f39da9b85e16f2927b4539e712e5cb
Summary:
Concatenate adjacent namespaces + format
This is a codemod. It was automatically generated and will be landed once it is approved and tests are passing in sandcastle.
You have been added as a reviewer by Sentinel or Butterfly.
Autodiff project: nc
Autodiff partition: fbcode.quic
Autodiff bookmark: ad.nc.fbcode.quic
Reviewed By: hanidamlaj
Differential Revision: D65365244
fbshipit-source-id: 0bbaa7684d03caf8fc8eff3439a0865940398220
Summary:
**Context**
The `BufAccessor` is used to access a contiguous section of memory. Right now, it works with a `Buf` under the hood.
**Overall plan**
The plan is to change the `BufAccessor` to use a `uint8_t*` instead. Since we're using section of contiguous memory, there's no need to use a chained buffer abstraction here. This'll move us closer to deprecating the usage `folly::IOBuf`.
**What this diff is doing**
Most use cases of the `BufAccessor` look like the following:
```
auto buf = bufAccessor.obtain();
// Do something with buf, like calling trimEnd
bufAccessor.release(buf)
```
I'm adding APIs to the `BufAccessor` so that there's no need to `obtain()` and `release()` the `Buf`. We'd instead just call an API on the `BufAccessor`, which would call that same API on the underlying `folly::IOBuf`. Later on, we'll change the `BufAccessor` to use a `uint8_t*` under the hood.
I'm currently leaving in the `obtain()`, `release()`, and `buf()` APIs because Fizz and the AsyncUDPSocket expect `folly::IOBuf` as inputs in many of their APIs. Once those callsites are migrated off `folly::IOBuf`, we can remove these APIs.
Reviewed By: mjoras
Differential Revision: D60973166
fbshipit-source-id: 52aa3541d0c4878c7ee8525d70ac280508b61e24
Summary:
The existing batch writers do not handle failed writes to the AsyncUDPSocket. A packet that fails to be written is detected as a packet loss later when feedback is received from the peer. This negatively impacts the congestion controller because of the fake loss signal, and artificially inflates the number of retransmitted packets/bytes.
This change adds a new batch writer (SinglePacketBackpressuretBatchWriter) that retains the buffers when a write fails. For subsequent writes, the writer retries the same buffer. No new packets are scheduled until the retried buffer succeeds.
Notes:
- To make sure that retry writes are scheduled, the write callback is installed on the socket when a buffer needs to be retried.
- The retries are for an already scheduled packet. The connection state reflects the timing of the first attempt. This could still have an impact on rtt samples, etc. but it this is a milder impact compared to fake losses/retranmissions.
- Any changes outside of the batch writer only impact the new batch writer. Existing batch writers do not use the fields and are not affected by the changes in this diff.
Reviewed By: kvtsoy
Differential Revision: D57597576
fbshipit-source-id: 9476d71ce52e383c5946466f64bb5eecd4f5d549
Summary:
This is the major transition that updates mvfst code to use the new interfaces. The new Folly implementations of the interfaces maintain all the existing behavior of folly types so this should not introduce any functional change. The core changes are:
- Update the BatchWriters to use the new interfaces.
- Update the FunctionLooper to use the new interfaces.
- Change QuicServerTransport to take the folly types and wrap them in the new types for use in the QuicTransportBase.
The rest of the diff is for updating all the existing uses of the QuicTrasnport to initialize the necessary types and pass them to the QUIC transport instead of directly passing folly types.
Reviewed By: mjoras
Differential Revision: D51413481
fbshipit-source-id: 5ed607e12b9a52b96148ad9b4f8f43899655d936
Summary:
This diff:
- Adds `QuicAsyncUDPSocketWrapperImpl` and changes existing instantiatons of `QuicAsyncUDPSocketWrapper` to instead instantiate `QuicAsyncUDPSocketWrapperImpl`. In follow up diffs, pure virtual functions will be added to `QuicAsyncUDPSocketWrapper` and implemented in `QuicAsyncUDPSocketWrapperImpl`. See D48717388 for more information.
--
This diff is part of a larger stack focused on the following:
- **Cleaning up client and server UDP packet receive paths while improving testability.** We currently have multiple receive paths for client and server. Capabilities vary significantly and there are few tests. For instance:
- The server receive path supports socket RX timestamps, abet incorrectly in that it does not store timestamp per packet. In comparison, the client receive path does not currently support socket RX timestamps, although the code in `QuicClientTransport::recvmsg` and `QuicClientTransport::recvmmsg` makes reference to socket RX timestamps, making it confusing to understand the capabilities available when tracing through the code. This complicates the tests in `QuicTypedTransportTests`, as we have to disable test logic that depends on socket RX timestamps for client tests.
- The client currently has three receive paths, and none of them are well tested.
- **Modularize and abstract components in the receive path.** This will make it easier to mock/fake the UDP socket and network layers.
- `QuicClientTransport` and `QuicServerTransport` currently contain UDP socket handling logic that operates over lower layer primitives such `cmsg` and `io_vec` (see `QuicClientTransport::recvmmsg` and `...::recvmsg` as examples).
- Because this UDP socket handling logic is inside of the mvfst transport implementations, it is difficult to test this logic in isolation and mock/fake the underlying socket and network layers. For instance, injecting a user space network emulator that operates at the socket layer would require faking `folly::AsyncUDPSocket`, which is non-trivial given that `AsyncUDPSocket` does not abstract away intricacies arising from the aforementioned lower layer primitives.
- By shifting this logic into an intermediate layer between the transport and the underlying UDP socket, it will be easier to mock out the UDP socket layer when testing functionality at higher layers, and inject fake components when we want to emulate the network between a mvfst client and server. It will also be easier for us to have unit tests focused on testing interactions between the UDP socket implementation and this intermediate layer.
- **Improving receive path timestamping.** We only record a single timestamp per `NetworkData` at the moment, but (1) it is possible for a `NetworkData` to have multiple packets, each with their own timestamps, and (2) we should be able to record both userspace and socket timestamps.
Reviewed By: jbeshay, mjoras
Differential Revision: D48717592
fbshipit-source-id: e21368f5c1f3b37608fc1c88617e96b93a02f6e0
Summary:
This diff changes `QuicAsyncUDPSocketWrapper` so that it is an abstraction layer that inherits from `QuicAsyncUDPSocketType`, instead of simply being a container with aliases.
- Key changes in `QuicAsyncUDPSocketWrapper.h`, the rest of the updates switch us from using `QuicAsyncUDPSocketType` to `QuicAsyncUDPSocketWrapper`.
- It's difficult to mock the UDP socket today given that we expose the entire `folly::AsyncUDPSocket` type to the higher layers of the QUIC stack. This complicates testing and emulation because any mock / fake has to implement low level primitives like `recvmmsg`, and because the `folly::AsyncUDPSocket` interface can change over time.
- Pure virtual functions will be defined in `QuicAsyncUDPSocketWrapper` in a follow up diff to start creating an interface between the higher layers of the mvfst QUIC stack and the UDP socket, and this interface will abstract away lower layer details such as `cmsgs` and `io_vec`, and instead focus on populating higher layer structures such as `NetworkData` and `ReceivedPacket` (D48714615). This will make it easier for us to mock or fake the UDP socket.
This diff relies on changes to `folly::MockAsyncUDPSocket` introduced in D48717389.
--
This diff is part of a larger stack focused on the following:
- **Cleaning up client and server UDP packet receive paths while improving testability.** We currently have multiple receive paths for client and server. Capabilities vary significantly and there are few tests. For instance:
- The server receive path supports socket RX timestamps, abet incorrectly in that it does not store timestamp per packet. In comparison, the client receive path does not currently support socket RX timestamps, although the code in `QuicClientTransport::recvmsg` and `QuicClientTransport::recvmmsg` makes reference to socket RX timestamps, making it confusing to understand the capabilities available when tracing through the code. This complicates the tests in `QuicTypedTransportTests`, as we have to disable test logic that depends on socket RX timestamps for client tests.
- The client currently has three receive paths, and none of them are well tested.
- **Modularize and abstract components in the receive path.** This will make it easier to mock/fake the UDP socket and network layers.
- `QuicClientTransport` and `QuicServerTransport` currently contain UDP socket handling logic that operates over lower layer primitives such `cmsg` and `io_vec` (see `QuicClientTransport::recvmmsg` and `...::recvmsg` as examples).
- Because this UDP socket handling logic is inside of the mvfst transport implementations, it is difficult to test this logic in isolation and mock/fake the underlying socket and network layers. For instance, injecting a user space network emulator that operates at the socket layer would require faking `folly::AsyncUDPSocket`, which is non-trivial given that `AsyncUDPSocket` does not abstract away intricacies arising from the aforementioned lower layer primitives.
- By shifting this logic into an intermediate layer between the transport and the underlying UDP socket, it will be easier to mock out the UDP socket layer when testing functionality at higher layers, and inject fake components when we want to emulate the network between a mvfst client and server. It will also be easier for us to have unit tests focused on testing interactions between the UDP socket implementation and this intermediate layer.
- **Improving receive path timestamping.** We only record a single timestamp per `NetworkData` at the moment, but (1) it is possible for a `NetworkData` to have multiple packets, each with their own timestamps, and (2) we should be able to record both userspace and socket timestamps.
Reviewed By: jbeshay, hanidamlaj
Differential Revision: D48717388
fbshipit-source-id: 4f34182a69ab1e619e454da19e357a6a2ee2b9ab
Summary: There's no need to call write when there's only one packet, since write just wraps writeGSO. This simplifies the use of options and will allow for setting a TXTIME when there's only one packet to send.
Reviewed By: kvtsoy
Differential Revision: D48526018
fbshipit-source-id: d934ddce8d3a35febb58ff253fc7a9bed3ef975c
Summary: Use WriteOptions instead of a single int to allow for txTime setting too
Reviewed By: mjoras
Differential Revision: D48218906
fbshipit-source-id: 00a1d4905b54ec0614860f9cdd8b58c9d6e6ef9a
Summary:
The new writer will use a single IOBuf for in-place writes via a
accessor.
Reviewed By: jbeshay, mjoras
Differential Revision: D47648606
fbshipit-source-id: c6094eb3445587337f8a03e203aeadb2954f76b1
Summary:
Check for GSO support once before the first write, cache the value and use it throughout the conn lifetime.
If GSO is not supported, ensure we use chained memory write path.
Reviewed By: mjoras
Differential Revision: D40240513
fbshipit-source-id: b699b4633246f3c15d2be7b39580686e44c2aab3
Summary:
This is following a similar pattern than what was done for the client side.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/160
Reviewed By: yangchi
Differential Revision: D23560951
Pulled By: xttjsn
fbshipit-source-id: 351417cbfa3230112fff4c4de59b307f88389cf6
Summary:
First step towards d6d. Semantically we need to separate the old `udpSendPacketLen` into `peerMaxPacketSize` as well as `currPMTU`. The former is directly tied to the peer's max_packet_size transport parameter whereas the second is controlled by d6d. To get the actual udp mss, call `conn_->getUdpSendPacketLen()`, which will use the minimum of the two if d6d is enabled, otherwise it will fallback to use `peerMaxPacketSize` only.
During processClientInitialParams and processServerInitialParams, we no longer need to check whether `canIgnorePathMTU` is set because that logic is moved to `setUdpSendPacketLen`. If d6d is enabled, we set both `peerMaxPacketSize` and `currPMTU` to `packetSize` because receiving an initial packet of size x indicates both that the peer accepts x-sized packet and that the PMTU is at least x.
Many call sites and tests are changed.
Faebook:
For now, d6d is considered enabled if `canIgnorePathMTU==false` and `turnoffPMTUD==true`. Down the road, from semantic & practical POV at least one of them should be renamed to something like `enableD6D`, since enabling d6d implies turning off PMTUD and that we should not ignore PMTU. We can keep one for the sake of testing.
Reviewed By: mjoras
Differential Revision: D22049806
fbshipit-source-id: 7a9b30b7e2519c132101509be56a9e63b803dc93
Summary:
as title. Instead of checking against the packet size limit, this
leaves a 10 bytes room since we have a bug that writes out packets that's
slightly larger than udpSendPacketLen. Most of such packet will be 1-2 bytes
larger than original packets.
Reviewed By: mjoras
Differential Revision: D21642386
fbshipit-source-id: 6ca68d48828cb7f8ee692e0d5f452f5389a56bfd
Summary:
To make sure the writer maker function actually return the Inplace
writer
Reviewed By: lnicco
Differential Revision: D21613419
fbshipit-source-id: da7e59b43785b13fd91cc1a737db87af7dfb1c8f
