Summary: This is similar to the previous commit. We use a stack-based IOBuf instead of allocating one on the heap. This saves CPU because allocations/deallocations on the stack are a lot cheaper.
Reviewed By: hanidamlaj
Differential Revision: D53101722
fbshipit-source-id: dd59a7eca6498db19472a62f954db3e2f2f27a42
Summary: Creating an IOBuf on the heap when we use `folly::IOBuf::wrapBuffer` is expensive.
Reviewed By: hanidamlaj
Differential Revision: D52506216
fbshipit-source-id: eed2b77beae0419b542b0461303785cc175e3518
Summary:
The current open connection counting is broken. To fix it this change:
- moves the counter to be for both the server and client transport
- calls onNewConnection() when the transport is ready.
- calls onConnectionClose() when the transport is closed after it was ready.
Reviewed By: lnicco, frankfeir
Differential Revision: D52638628
fbshipit-source-id: bcff7a8c671f8eede53c22e698e1b95332c56959
Summary:
This moves all the loop and timer callback operations back to the callbacks instead of relying the QuicEventBase to perform them. I.e. it's now Callback::cancelCallback() instead of QuicEventBase::cancelCallback(&callback).
To simplify the design, the lifetime of LoopCallbackWrapper and TimerCallbackWrapper has been changed. Previously these wrappers lasted for one iteration of the loop or the timer callback. In the new design the wrapper is created the first time the callback is scheduled and is only destroyed when the callback is destroyed. This significantly simplifies the lifetime management of the wrapper and the code overall.
While transitioning LibevQuicEventBase to this new design, this change also consolidates the QuicTimer functionality in the LibevQuicEventBase itself.
Reviewed By: hanidamlaj, mjoras
Differential Revision: D52217879
fbshipit-source-id: da7b95d592650ddc112813fdc3b9f5010d32e7fb
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 defines `QuicAsyncUDPSocketWrapper::ReadCallback` and implements a transformation layer to enable types that inherit from `QuicAsyncUDPSocketWrapper::ReadCallback` to operate on `QuicAsyncUDPSocketWrapper` objects instead of `QuicAsyncUDPSocketType` objects.
More specifically:
1. `QuicAsyncUDPSocketWrapper::ReadCallback` inherits from `QuicAsyncUDPSocketType::ReadCallback`.
2. `QuicAsyncUDPSocketWrapper::ReadCallback` defines `QuicAsyncUDPSocketWrapper::onNotifyDataAvailable`. This function signature of this callback differs from `QuicAsyncUDPSocketType::onNotifyDataAvailable` in that the `QuicAsyncUDPSocketWrapper` version passes a reference to a `QuicAsyncUDPSocketWrapper` object instead of a reference to a `QuicAsyncUDPSocketType` object.
3. `QuicAsyncUDPSocketWrapper::ReadCallback` implements `QuicAsyncUDPSocketType::onNotifyDataAvailable` and relays to `QuicAsyncUDPSocketWrapper::onNotifyDataAvailable`
--
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
Differential Revision: D48718053
fbshipit-source-id: f7f7eb2fcc07b1523638bd4ce6e1c1804fd82737
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:
This diff deprecates `QuicClientTransport::onDataAvailable`, as this code path is no longer used in production:
- All production instances of `QuicClientTransport::shouldOnlyNotify()` currently return true, as `transportSettings.shouldRecvBatch` is set to true in production. Thus, `onDataAvailable` will never be called.
- `shouldRecvBatch` will be removed in a subsequent diff as it is no longer relevant on the client or server (despite being set sometimes for the server...).
--
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: hanidamlaj
Differential Revision: D48715162
fbshipit-source-id: ae63e0298d001bc90021767756d1023a12566cf2
Summary: Update flow control settings names to reflect that these are indeed flow control
Reviewed By: jbeshay
Differential Revision: D48137685
fbshipit-source-id: a48372e21cdd529480e25785a9bd5de456427ef3
Summary:
- adds logic to drop retry packet if we've processed an initial packet
from RFC9000:
> After the client has received and processed an Initial or Retry packet from the server, it MUST discard any subsequent Retry packets that it receives.
Reviewed By: jbeshay, mjoras
Differential Revision: D45536695
fbshipit-source-id: c6f9532478ab249905683ca1fff6e692a978a0d2
Summary:
Somehow we never implemented this stat despite having it for ages.
It's relatively easy to do, we just need to check whether an entry was inserted to the IntervalSet we are already using for tracking what to ACK.
Note that this has the limitation that when the ACK interval set is cleared out (on ACK of ACK), we will no longer be able to detect duplicates. This is something we can tune later.
Reviewed By: kvtsoy
Differential Revision: D45131856
fbshipit-source-id: aad4e07e1a9cd5b2dc5dec60424f7cee15906c7e
Summary:
To get reliable packet destruction events, created a `OutstandingPacketWrapper` wrapper that wraps the current `OutstandingPacket` class, so callback functions can be added to the wrapper instead of the underlying object itself.
#### Why do we need this wrapper?
`std::deque::erase` does not guarantee that appropriate object destructors will be called (only that the number of destructions = number of objects erased). This is a real problem as packet destruction events are then no longer reliable (OutstandingPacket object is wrong!). The wrapper class handles this condition by detecting it in the move assignment constructor (called during erase) and calls the appropriate packet destruction callback before packets are moved. If we did the same fix in the old OutstandingPacket, we have to make sure the callback is called before all the fields of OutstandingPacket are moved - this is not scaleable. Hence a wrapper with the underlying object and a destruction callback function.
I also disabled copy construction for OutstandingPacket (otherwise we will get duplicate OnPacketDestroyed callbacks and cannot track packets reliably). Removing packet copies also improves performance. Some code changes (in tests mostly) are mostly in service of this particular change.
Reviewed By: bschlinker
Differential Revision: D43896148
fbshipit-source-id: c295d3c4dba2368aa66f06df5fc82b473a03fb4d
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:
Fixing dependency loop introduced by D37799050 (96abc8160d)
Running `autodeps` yields the following patch:
```
--- a/xplat/quic/state/TARGETS
+++ b/xplat/quic/state/TARGETS
@@ -43,8 +43,8 @@
exported_deps = [
"//folly:random",
"//quic:constants",
+ "//quic/codec:codec",
"//quic/codec:types",
- "//quic/common:circular_deque",
"//quic/common:interval_set",
],
)
```
If this patch is applied, there is a circular dependency loop between `//quic/codec:codec` and `//quic/state:ack_states` by way of `//quic/codec:types`; this loop was introduced by D37799050 (96abc8160d).
Fixed by separating out headers files and targets. In parallel, renamed structures used for writing ACK frames (which were the reason this loop occurred) to make their role clear.
Differential Revision: D42281359
fbshipit-source-id: 8514c99f3fe72ff1d942d7f303e4a209838c7623
Summary:
Store timestamps/packet numbers of recently received packets in AckState.
- The maximum number of packets stored is controlled by kMaxReceivedPktsTimestampsStored.
- The packet number of entries in the deque is guarenteed to increase
monotonically because an entry is only added for a received packet
if the packet number is greater than the packet number of the last
element in the deque (e.g., entries are not added for packets that
arrive out of order relative to previously received packets).
Reviewed By: bschlinker
Differential Revision: D37799023
fbshipit-source-id: 3b6bf2ba8ea15219a87bbdc2724fe23eebe66b70
Summary: PacketDropReason was converted to a Better_Enum
Reviewed By: jbeshay
Differential Revision: D40350056
fbshipit-source-id: a6af9ccf0fc7c4358a0481de5cca6f69d1beb438
Summary: Retransmitting 0-rtt packets when 0-rtt is accepted can retransmit more packets than necessary when ACKs from the peer are delayed.
Reviewed By: mjoras
Differential Revision: D40274224
fbshipit-source-id: f43337c56341cdaa8504eab6d2b9b81d39f66a1f
Summary: The existing PacketDropReason values cover many branches in the code making it impossible to isolate the reason for a PARSE_ERROR, INVALID_PACKET, CONNECTION_NOT_FOUND. This change breaks them down into more values that are each used in a single branch.
Reviewed By: mjoras
Differential Revision: D39149490
fbshipit-source-id: 28cbe1ea6c4a06cf55960058edaa48c28ed4d2ef
Summary:
This is a longstanding off by one. When a stream frame is ACKed the largest ACKed offset contained is the stream frame offset + len - 1, unless there's an fin.
(Note: this ignores all push blocking failures!)
Reviewed By: kvtsoy
Differential Revision: D39085665
fbshipit-source-id: cf353c7e72466085a27537d2c95748781f70f79a
Summary: - usage of these new tokens should not overlap
Reviewed By: mjoras
Differential Revision: D35323558
fbshipit-source-id: c9152cbb7029f64396661ea9091567ef5a75f4dc
Summary: In order to provide more information about datagrams when we receive them, I'm adding a wrapper around the BufQueue that we currently hold so we can include receiveTimePoint and any other metadata we might want to expose for the Datagram API.
Reviewed By: mjoras
Differential Revision: D33994358
fbshipit-source-id: 805f182cd350908320639bc07eb6f3b7349bbc05
Summary:
Maine change is `MockConnectionCallback` -> `MockConnectionSetupCallback` + `MockConnectionCallbackNew`.
Everything else is changing tests to use the two new classes.
Differential Revision: D33076321
fbshipit-source-id: a938b63ce59f07f549b3e725caad8785348db7ed
