Summary: Adding virtual method getSelfCertificate to QuicSocket that's overriden in QuicServerTransport that returns the server cert.
Reviewed By: mjoras
Differential Revision: D54234662
fbshipit-source-id: fcef6399bc660ff41243336aacdfadd6e8975193
Summary: We currently cap the max send size at the default instead of the max we support (1452). This is pretty conservative and I don't think it's needed, and causes issues when trying to use MASQUE + UDP proxying with QUIC.
Reviewed By: hanidamlaj
Differential Revision: D53440189
fbshipit-source-id: 1d5bbdbb4cc1abb677f85dff6bdf2841cb01c5e1
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:
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: There is no reason to let the TLS layer control this.
Reviewed By: jbeshay
Differential Revision: D51267586
fbshipit-source-id: 6f70b7f6ba51a9a022195f7e2c1683b5323fde7a
Summary:
This diff renames `ReceivedPacket` to `ReceivedUdpPacket` to clarify that it maps to a UDP packet and not a QUIC packet. A single UDP packet can contain multiple QUIC packets due to coalescing.
--
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: silver23arrow
Differential Revision: D48788809
fbshipit-source-id: 3793c30212d545e226f3e5337289bc2601dfa553
Summary: The default behavior we have is to close the existing connection, which is probably excessive. Add a setting to allow simply dropping the packet instead.
Reviewed By: kvtsoy
Differential Revision: D47082159
fbshipit-source-id: 76b3589d2dcf216f425d296435de843ddd21b272
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: Only accept knob frames if support has been advertised.
Reviewed By: hanidamlaj
Differential Revision: D48329363
fbshipit-source-id: 3f06574415d95a2129e5d776b7a4f4808791ed45
Summary: Rename transportSetting.BBRConfig to CongestionControlConfig to prepare it for being used by other CCAs too.
Reviewed By: hanidamlaj, mjoras
Differential Revision: D47309439
fbshipit-source-id: 56d0ddc9752789709c54a4f7cc595f94c656e49e
Summary:
As title. This changes the NewTokenFrame (for writing) to hold the toke as an IOBuf instead of a string. This makes it consistent with the read side.
In the same change, the qlogger now hexlifies this buffer when writing the token to the qlog.
Reviewed By: hanidamlaj, mjoras
Differential Revision: D46955172
fbshipit-source-id: 8f5f575ad2f0a4ec3b4c01cb67defa1f4425cd74
Summary:
Right now small packets will cause us to subtract from the packet limit during write loops. This is generally okay when using just non-DSR data. For DSR though there is an issue where if we only write a single small packet from the non-DSR path (like an ACK) we will discount it from the amount of packets the DSR path can write.
The existing workaround for this is to only discount the non-DSR path write if the number of bytes written is less than half a packet's worth. This heuristic is incomplete though and doesn't consider the cases like the following:
1. Packet limit is 5
2. Normal path writes 4 packets of size 1232, 1232, 1232, 20.
3. The DSR path only gets one packet.
The reason for this is because the normal path also wrote full packets, but ended with a short packet. The account for this we just have to discount in terms of full packet's written. In the above example, the DSR path would get the chance to write two packets. Thus we would exceed the packet limit by one packet, but it would get us closer to the desired amount of data on the wire.
Reviewed By: kvtsoy
Differential Revision: D46633105
fbshipit-source-id: 00d92499ab73fc746ea5fdb6ff31e10f06b98666
Summary:
The idea here is to allow a way to do incremental stream priorities while switching between streams as aggressively.
Achieving this is somewhat tricky. The easiest place to track this is to make it so the iterator in QuicPriorityQueue no-op next() until a counter reaches an increment.
This is especially impacftul for DSR, where round robining per packet is almost pathologically bad both for CPU impact but also spurious losses and low bandwidth estimates.
Thoughts?
(Note: this ignores all push blocking failures!)
Reviewed By: kvtsoy
Differential Revision: D46268308
fbshipit-source-id: cd5b924141365f61f8a3363bc9cb38a62e5c94cf
Summary:
This has been hardcoded to SRTT/25 for a long time. However, especially when using DSR this might not be the most appropriate since it can start to get close to real world SRTTs.
Control it via a knob, and also add the behavior such that setting it to 0 effectively disables the time limit.
Reviewed By: jbeshay
Differential Revision: D46084438
fbshipit-source-id: 7dc619fdff1bd5c3f8654c4936200e0340ef94f2
Summary: Allows the default stream priority (which also means scheduling policy) to be set via TransportSettings.
Reviewed By: jbeshay, hanidamlaj
Differential Revision: D45881729
fbshipit-source-id: fcb72022cd6eac2f1dafc55173d5a46a72a95dbc
Summary:
Due to the fact that store DSR and non DSR streams in separate priority queues, we end up having to make a decision on which to write first.
Longer term we should merge them into a single priority queue, but in the meantime to mitigate potential starvation, randomly flip flop between which we write first in a given write loop.
Reviewed By: jbeshay, shodoco
Differential Revision: D45701230
fbshipit-source-id: 894179e457a4d6f7364767108f9290ff267eb977
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:
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:
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:
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: This changes the experimental behavior of BBR sending ACK_FREQUENCY frames. Now instead of using the static ack eliciting threshold, early in the connection BBR will set the threshold to 2 which has been empirically shown to be a good "initial" value.
Reviewed By: jbeshay
Differential Revision: D40438721
fbshipit-source-id: 184b8025581b6152eea97f9b557b6343d980322d
Summary:
This introduces the ability for BBR to control ACK_FREQUENCY. It also introduces a knob which will let us experiment with different policies.
This code makes BBR issue a new ACK_FREQUENCY every round trip. This may be excessive, so BBR will track the last frame sent, and only send a new one if the new value is sufficiently different in terms of milliseconds.
Note that the only dynamic part of the frame currently will be the max ack delay, which is in terms of fractional RTTs. It is not clear yet how we should vary the other parameters dynamically.
Reviewed By: kvtsoy
Differential Revision: D39910417
fbshipit-source-id: 73cfdac40ca52185c33322017a893e5a02ee58e8
Summary:
The `QuicServerTransportTestBase` test fixture accepts the connection before beginning to run any of the test code. However, if a test wants to be able to modify the transport settings, it needs to be able to modify the transport before the connection has been accepted and the transport parameters have been encoded (e.g., before `transportParametersEncoded` is set to true).
This diff adds a `startTransport` function to `QuicServerTransportTestBase` that does the work of accepting the connection, and then adds a new test fixture `QuicServerTransportAfterStartTestBase` — named after similar existing class `QuicClientTransportAfterStartTestBase` — which calls `startTransport` during setup. All tests previously using `QuicServerTransportTestBase` are switched to `QuicServerTransportAfterStartTestBase`.
For clients, we already had `QuicClientTransportAfterStartTestBase`. A `startTransport` function is also added to `QuicClientTransportTestBase` and the same is done. I need to investigate the client side a bit further as we also have a `start` function there, but there are many layers on `QuicClientTransportTestBase` and that will take some time to unwind.
Differential Revision: D39274871
fbshipit-source-id: 6de47d463637d2e93b808de680d52cc8820d9d47
Summary:
- continually issuing new connection ids to peer as old connections ids are retired through RETIRE_CONN_ID frames
- add logic to parse and act on receiving RETIRE_CONN_ID frame
Reviewed By: mjoras
Differential Revision: D38443561
fbshipit-source-id: 82fb679f482fd69c7b3a3385693d2e5575e92703
Summary: Add the new IMMEDIATE_ACK frame from the ack frequency draft.
Reviewed By: mjoras
Differential Revision: D38523438
fbshipit-source-id: 79f4a26160ccf4333fb79897ab4ace2ed262fa01
Summary:
From [RFC9000](https://datatracker.ietf.org/doc/html/rfc9000)
```
Note that it is not possible to send the following frames in 0-RTT
packets for various reasons: ACK, CRYPTO, HANDSHAKE_DONE, NEW_TOKEN,
PATH_RESPONSE, and RETIRE_CONNECTION_ID. A server MAY treat receipt
of these frames in 0-RTT packets as a connection error of type
PROTOCOL_VIOLATION.
```
Reviewed By: jbeshay, mjoras
Differential Revision: D38642380
fbshipit-source-id: a1a7167c06fa68037758ce5395c3798479c26b42
Summary: Handle MAX_PACING_RATE_KNOB_SEQUENCED transport knob with which a client can suggest a limit on the pacing rate used by the server, if it has pacing enabled. This knob also include a sequence number with which the server can use to determine out-of-order frames.
Reviewed By: bschlinker
Differential Revision: D37062350
fbshipit-source-id: 981f0c9df99cf30de6c676784409a43d45f62ff2
Summary:
Add a test for dropping datagram on the floor after receiving conn
close
Reviewed By: hanidamlaj
Differential Revision: D37532252
fbshipit-source-id: d98d34a6bf692cf719864fa01120676b63bbbc1d
Summary: - unit test to verify that the writableBytesLimit is bypassed upon receiving a valid new token
Reviewed By: mjoras
Differential Revision: D36645447
fbshipit-source-id: 2c4163414c1d04f51cd473b07e32fcd78fbe1bda
