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 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: Broke in one of the previous commits.
Reviewed By: hanidamlaj
Differential Revision: D47962468
fbshipit-source-id: 532e9356cb7e1bba2e45ac0a18440a12aed1eb25
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: This is needed for the new socket abstraction implementation later.
Reviewed By: jbeshay, lnicco
Differential Revision: D46669712
fbshipit-source-id: 5adde6679386689e7f63992ed769ff4a777f59e3
Summary:
Create and use an actual wrapper around folly::EventBase.
Later an interface will be added that the wrapper will be implementing.
Reviewed By: jbeshay
Differential Revision: D45822401
fbshipit-source-id: 3b33f796c31043ec2881b753a9b60943bdf91f1d
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:
```
quic\api\quicbatchwriter.cpp(286): warning C4311: 'type cast': pointer truncation from 'const uint8_t *' to 'long'
quic\api\quicbatchwriter.cpp(286): warning C4302: 'type cast': truncation from 'const uint8_t *' to 'long'
```
On *nix 64-bit systems a `long` is 64 bits while on Windows/MSVC it's 32-bits. Better just use the 64-bit value for consistency.
Reviewed By: yangchi
Differential Revision: D25496685
fbshipit-source-id: f2357c6b9f4e4ca1a630db95b69a116d0241c744
Summary: SendmmsgGSOPacketBatchWriter - try to batch more based on dest addr (map)
Reviewed By: mjoras
Differential Revision: D21110682
fbshipit-source-id: 1be2eb26ec33c8256f1d4bbe7e3d6fae19eb7146
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:
Add support for variable number of addrs so we can send data to multiple destinations.
(Note: this ignores all push blocking failures!)
Reviewed By: kevin-vigor
Differential Revision: D21032857
fbshipit-source-id: f73b98d44f5d7d92f3692dfddb9b1c76ebcc51c5
Summary:
In the client transport case, an exception is thrown before return. In
the batch write maker case, the previous switch/case handles all possible cases
Reviewed By: udippant
Differential Revision: D15260725
fbshipit-source-id: 2ce7277544ee810729140aeb304e6484d339f3db
