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:
This diff:
- Changes `NetworkDataSingle` to have `ReceivedPacket` instead of `Buf`, in line with earlier change to `NetworkData` in D48714615
--
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: mjoras
Differential Revision: D48714796
fbshipit-source-id: d96c2abc81e7c27a01bcd0dd552f274a0c1ede26
Summary:
This diff:
- Changes `NetworkData` to have `vector<ReceivedPacket>` instead of `vector<IOBuf>` to make it easier to associate metadata with individual UDP packets.
--
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: mjoras
Differential Revision: D48714615
fbshipit-source-id: b21a88f14156ed6398308223f50b2334328586ee
Summary:
Added the capability to disable Ack receive timestamps based on hostname.
We want to disable timestamps on dynamic content (graph.facebook.com, i.instagram.com etc) to reduce network and resource overhead (thereby saving uplink BW). The hostnames can be specified via a MC. Timestamps should still be enabled for hostnames serving static content (like video, fbcdn...)
We have observed upto 40% more duplicate timestamps being sent on ACKs and ACK packet size has increased by more than 30% on average for such cases. This is a first step in reducing ACK wastage.
(Note: this ignores all push blocking failures!)
Reviewed By: jbeshay
Differential Revision: D48812624
fbshipit-source-id: aa72ec2dcc41ebe9d982364d8d45739e062daddb
Summary: An interface which will be extended by throttling detectors and will provide throttling signal to CCAs. This interface can be set through `QuicSocket` and can be accessed via `QuicConnectionStateBase`.
Reviewed By: silver23arrow, jbeshay
Differential Revision: D47027498
fbshipit-source-id: 6a2b9ee51ca54395e10500dd7c9787968cc281d4
Summary: Only accept knob frames if support has been advertised.
Reviewed By: hanidamlaj
Differential Revision: D48329363
fbshipit-source-id: 3f06574415d95a2129e5d776b7a4f4808791ed45
Summary: With bbr, bbr_testing, and bbr2 there is no way to tell the difference between qlog traces especially during Startup. This explicitly logs whenever the congestion controller is created or changed.
Reviewed By: hanidamlaj, mjoras
Differential Revision: D48194475
fbshipit-source-id: 30ba5662e0fe4aa2ccb4b28696049d8aadab38db
Summary: Change BBRTesting to test the existing BBR CCA when replacing the pace scaling factor by the experimental pacing improvements.
Reviewed By: kvtsoy
Differential Revision: D48120180
fbshipit-source-id: 6b99754ae0f8cb7a76596243a112ddd500f6fcc8
Summary: When BBR2 is configured on the server side, the pacer may not be allocated yet when setCongestionControl is called.
Reviewed By: hanidamlaj
Differential Revision: D47881213
fbshipit-source-id: ddb695dbb5d5d8c07ccef5e7282babdb661482b3
Summary: Ensure all CCA-related settings are included in setCongestionControl(), and that it's called from the knob setting the congestion controller.
Reviewed By: kvtsoy
Differential Revision: D47741830
fbshipit-source-id: ff1d2347581c61a58f2caaff8189126930bf4e04
Summary: This has empirically shown good results in experiments with no downside. Only set it if the transport setting hasn't been overridden already.
Reviewed By: jbeshay
Differential Revision: D47447380
fbshipit-source-id: 294582544b6e42d7f6c46d1d810228a0b5fad9e8
Summary: Will not be doing evb observers for now for mvfst mobile
Reviewed By: hanidamlaj, mjoras
Differential Revision: D47135550
fbshipit-source-id: f18f50b91f1575b0d4f4874f52c72fb6a7d5c1e8
Summary:
This implements the BBRv2 congestion controller for Mvfst. This implementation aims at staying as close as possible to the naming and organization used in the latest draft. Wherever there are gaps in the spec or the pseudocode, this attempts to fill them with reasonable assumptions.
https://datatracker.ietf.org/doc/html/draft-cardwell-iccrg-bbr-congestion-control-02
Besides the spec differences, this implementation differs from Mvfst's BBRv1 as follows. These differences may be revisited in the future:
- The pacing rate is exclusively controlled by the CCA. No further scaling of the pacer rate is enforced. This enables BBRv2 control loop to keep tight control over queueing delay.
- BBRv2 does not rely on an external bandwidth or RTT sampler. Mvfst's BBRv1 decouples these which makes tracking the state a bit harder.
Reviewed By: mjoras
Differential Revision: D44599226
fbshipit-source-id: 1e488bd936db5826b73ff6205f988dd40e908eba
Summary:
The pacingTimerTickInterval transport setting conflates two options: the minimum interval a pacer can use, and the resolution of the underlying timer. This means that a higher value leads to lower timer resolution and less pacing accuracy.
This change adds a separate parameter for the timer resolution to ensure that a larger pacing tick does not degrade the pacer accuracy.
Reviewed By: mjoras
Differential Revision: D46564066
fbshipit-source-id: 0d0e54077b80da03e6e6c9baaab49a4c969966b6
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:
This has been a default behavior for us for a long time. Basically, we will always include ACK frames in a burst if there's new packets to ACK.
This is overall probably fine and is flexible to peer behavior such that the peer can get away with basically never sending anything ACK-eliciting.
However it is not without its issues. In particular, for DSR it means that we write an excessive amount of pure ACK packets since they cannot be coalesced with a DSR burst.
Make this behavior optional, such that we only include ACK frames in non-probing schedulers when we are only writing stream data.
Reviewed By: kvtsoy
Differential Revision: D39479149
fbshipit-source-id: 6e92d45311a3fb23750c93483b94e97dd3fdce26
Summary: The minCwnd should not be overwritten by the initCwnd. This affects Cubic minCwnd only. BBR uses its constants directly.
Reviewed By: mjoras
Differential Revision: D46076172
fbshipit-source-id: 63d0be96334e6ca294cdf89bc7545aaa4b960678
Summary: Classifying loss as `byPto` is confusing because it is not related to Probe Timeouts (PTO). This changes the occurrences of this name in the transport code to `byTimeout`.
Reviewed By: mjoras, kvtsoy
Differential Revision: D45788216
fbshipit-source-id: 9900b928d3f44572a21d6ed7e76be658e21451c7
Summary: This is a dumb shortcut. Just check whether it's happened after reading data and call close that way.
Reviewed By: kvtsoy
Differential Revision: D45205285
fbshipit-source-id: 57edafc50f5e5e9cdaf9ac7e46781b762acd30d8
Summary:
## Context
Context: see summary for D43854603.
## In this diff
In this diff, make way for easily adding new elements to Priority struct.
Reviewed By: afrind
Differential Revision: D43882907
fbshipit-source-id: f74f6ec41162a970dcd666bfa5192676f968d740
Summary:
Adds a prewrite function in packet processors that can be used to set PreWrite Reuqests that will apply apply to the next write loop in the QuicSocket.
Currently, the PrewriteRequest only contains a list of cmsgs but it can be expanded when needed.
This replaces the setAdditionalCmsgs callback that was previously passed down directly to AsyncUDPSocket, allowing multiple processors to affect the additionalCmsgs sent to the AsyncUDPSocket.
Reviewed By: bschlinker
Differential Revision: D44233700
fbshipit-source-id: 799357ee5df89b2a526611bd83d0b4a9a13b71d5
Summary: - both `isScheduled()` and `cancelTimeout()` check if wheel_ != nullptr, no need to check twice
Reviewed By: kvtsoy
Differential Revision: D43582809
fbshipit-source-id: aaf535b227ad3c0da97cfc979b5f8a411d891959
Summary: - .reset() is probably fractionally less costly than the assignment operator?
Reviewed By: sharmafb
Differential Revision: D43579041
fbshipit-source-id: 4838b6c21e94197782cf56866950be1dbf65b106
Summary: This change ensures that the stream is not destroyed before onEOM callback so that onEOM can access stream specific data.
Reviewed By: mjoras
Differential Revision: D42784838
fbshipit-source-id: ce619be1a2b6a542a517a7b4f32c2b3d37aa30fd
Summary:
This removes the older method of deciding whether knob frames should be sent using the QuicVersion. With this change, the client can only send knobs when the server has signaled support using the `knob_frames_supported` transport parameter.
To make sure that knobs can be sent in 0-rtt connections, the transport parameter is now included in the client's cache of server transport parameters.
Reviewed By: mjoras
Differential Revision: D43014893
fbshipit-source-id: 204dd43b4551cd1c943153a3716e882fc80e6136
Summary: Decide if knob frames are supported based upon a transport parameter instead of relying on the QUIC version. This is a cleaner approach, at the cost of an additional transport parameter.
Reviewed By: mjoras
Differential Revision: D42465442
fbshipit-source-id: bfd1d177bdbe198e5eb47e63113410b5738dcede
Summary:
Expose the time when the connection started in `QuicSocket::TransportInfo`.
This is imperfect for `QuicClientTransport` right now, as it marks the time when the transport was created — not the time when the connection was started. This is due to how the underlying value is being populated. Will address in a follow up diff.
Differential Revision: D42224019
fbshipit-source-id: c75dc2df10c252d18225df8508445e99ed5d788a
Summary:
We don't need to carry these states after the handshake is confirmed, so make them pointers instead. This will facilitate adding a structure to the AckState for tracking duplicate packets.
(Note: this ignores all push blocking failures!)
Reviewed By: hanidamlaj
Differential Revision: D41626895
fbshipit-source-id: d8ac960b3672b9bb9adaaececa53a1203ec801e0
Summary:
Track the total (cumulative) amount of time that the connection has been application limited and store this information in `OutstandingPacketMetadata`. If this value is the same for two `OutstandingPacketMetadata` then we know that the transport did not become application limited between when those two packets were sent (or, less likely, the transport was application limited for less than one microsecond given the microsecond resolution of the timestamp).
We store the amount of time spent application limited instead of a count of the number of application limited events because the implications of being application limited are time dependent.
Tests show that we need to be able to inject a mockable clock. That's been an issue for some time; will work on in a subsequent diff.
Differential Revision: D41714879
fbshipit-source-id: 9fd4fe321d85639dc9fb5c2cd51713c481cbeb22
Summary:
1) unregistering a pending write callback from another callback was unsafe. Follow the paradigm in the rest of this function.
2) Calling closeGracefully() nulls out the connection callback but keeps writing -- need to check that connCb_ is non-null before calling onAppLimited()
Reviewed By: kvtsoy
Differential Revision: D40811191
fbshipit-source-id: fa3512e4aa5ddda3dfc160a044b7d29b7aec8a8a
