feat(dc): Enable client packet buffering - #3296
Conversation
This allows much faster recovery, since we don't need to wait for server to detect its initial packet was lost and recover from both the initial packet being lost and the subsequent handshake packet also getting lost.
These Handshake-space probes were not immediately useful to the unbuffered client in this trace. However, the server cannot infer from a missing Initial ACK that the client lacks Handshake keys. The client might have received all the Initial CRYPTO needed to derive those keys while its ACK was lost. In that case, if the Handshake data was also lost, waiting for an Initial ACK before attempting to recover the Handshake data would unnecessarily delay recovery, potentially until another PTO. |
|
I mean, I don't love us not writing a test for this scenario. But we kind of shot ourselves in the foot because we've avoided fixing our existing dc-quic tests, and if you had to fix them right now I suspect it would be kind of a mess. But I'm fine with enabling this feature for the client. Shouldn't make the handshake worse anyways. |
I don't think this fully aligns with what the trace shows us. When the server sends Handshake Crypto bytes 0..804, it is currently assuming that the client did not get the Initial Crypto bytes 0..1178: it just sent bytes 0..1125 in the Initial space PTO. At minimum, in this specific case, I think it makes no sense to not include the trailing ~53 bytes in the LossRecoveryProbing packets (Handshake #2+#4) since they'd fit in the 1200 bytes. Unless the RFC prohibits including multiple spaces in a LossRecoveryProbing packet? I'm not sure why that would be a problem though. |
My point was that sending the Handshake probe is justified: the missing Initial ACK does not tell the server whether the Initial CRYPTO was lost or whether it arrived and only its ACK was lost.
There's nothing in the RFC prohibiting this (technically its multiple packets coalesced in a single UDP datagram). It might not be simple to implement though, since it would require tracking unacknowledged ranges already covered within a PTO burst, which the current DataSender does not do. Probably easier would be spreading the initial |
Not sure I understand. The range in question is not covered -- it has not been resent yet. These are the packets in question (deduplicated to reduce confusion since we send each twice): That's the first re-send of both the first 1125 Initial Crypto bytes and the full Handshake range (804 bytes). I'm suggesting that s2n-quic should do this: Indeed, this is the behavior I would expect from s2n-quic naturally, because the algorithm it's trying to implement (I think) is to include any unacked data in the PTO probes it sends. I suspect there's something splitting probes on initial/handshake boundary and that's what is going wrong here. Even with buffering, that seems worth doing, because it would cut out a PTO period for delivering that data -- this packet wouldn't be needed anymore as far as I can tell. Although once the server side of the initial MTU change rolls out that will I think not be needed, so maybe it's not too important in the short term. |
Adds a bach simulation of the handshake latency regression that appears when
post-quantum key exchange (s2n-tls policy 20250721) makes the first flight too
large for a PTO probe to carry.
s2n-quic pads the first flight out to initial_mtu to validate it, so an endpoint
configured with an initial_mtu the path cannot carry loses that flight. Recovery
normally costs one PTO, since probes are clamped to MINIMUM_MAX_DATAGRAM_SIZE and
a classical ~271 byte ClientHello fits comfortably in the ~1145 bytes that leaves
for CRYPTO.
An ML-KEM ClientHello is ~1503 bytes and does not fit, so each probe delivers only
a prefix. The server cannot report what it received either: its Normal-mode
transmissions are padded to initial_mtu as well, so even a pure-ACK reply goes out
as an oversized datagram and is dropped. Nothing is ever ACKed, so the client
re-sends from offset 0 on every probe, and progress waits for the server's own PTO
to fire with a clamped, deliverable probe. That puts the client's full PTO backoff
ladder (3, 6, 12, 24, 48, 96ms) on the critical path and makes the penalty largely
independent of RTT.
Measured in simulated time at a 1ms RTT on a 1500-byte path, with base_mtu 1450
and initial_mtu 8940 on both endpoints:
classical ClientHello 10ms
ML-KEM ClientHello 191ms
ML-KEM ClientHello, initial_mtu = base_mtu 5ms
On a jumbo-capable path both policies complete in 1 RTT, so the trigger is the
first flight being padded above the path MTU rather than the ClientHello size on
its own.
s2n-quic-dc no longer triggers this directly: aws#3295 disabled MTU discovery
(base_mtu = initial_mtu = max_mtu) and aws#3296 enabled client packet buffering.
dc_mtu_config_is_unaffected pins that down, and the client packet buffer is part
of the shared scenario setup so the remaining cases measure what dc would see. The
regression cases are kept because the behavior is still reachable by any
application that configures an initial_mtu above the real path MTU, and because
the underlying causes are still worth fixing:
- probes re-send CRYPTO from offset 0 rather than advancing past un-ACKed ranges
- Normal-mode transmissions are padded to initial_mtu even when they carry
nothing but an ACK, so an endpoint cannot report progress over a path that
has already shown it cannot carry that size
The last case (client clamped, server still jumbo) covers the residual cost: the
ML-KEM ServerHello is 1178 bytes of Initial CRYPTO and a 1200-byte probe carries
only 1125, so the tail costs one extra round trip.
Existing MTU coverage only exercises oversized application-data packets, so a
handshake flight exceeding the initial MTU was untested.
By “ranges already covered within a PTO burst,” I meant remembering that [0..1125] was selected by the first probe so a subsequent probe could prefer the still-uncovered [1125..1178] range. The data sender does not track coverage across the packets in a PTO burst. For the PTO transmission case, the data sender doesn't really track anything at all, it just fills up the given capacity with what it can from unacknowledged data, without changing any state.
The packet spaces each have an independent recovery manager and PTO transmission budget. At the connection level, packet construction visits Initial first and then Handshake, and it will naturally coalesce both into the same datagram if the Initial packet leaves enough capacity. So I think the issue is less that the Initial/Handshake boundary forces separate datagrams and more that there is no tracking and no coordination of unacknowledged-range coverage across the per-space PTO transmissions. If the second Initial probe selected [1125..1178] instead of starting from zero again, the existing Initial-then-Handshake construction should leave enough room to coalesce a Handshake packet in that datagram, thats what I was getting at with the "spreading the initial CRYPTO over the two PTO datagrams" suggestion, which with the duplicated PTO probes removed as illustrated, would effectively be the same thing you are proposing. It would still require managing some additional state in the DataSender either way though. |
Adds a bach simulation of the handshake latency regression that appears when post-quantum key exchange (s2n-tls policy 20250721) makes the first flight too large for a PTO probe to carry and the initial MTU is set to ~9kb. Measured in simulated time at a 1ms RTT on a 1500-byte path, with base_mtu 1450 and initial_mtu 8940 on both endpoints: * classical ClientHello - 10ms * ML-KEM ClientHello - 191ms * ML-KEM ClientHello, initial_mtu = base_mtu - 9ms w/o client buffering * ML-KEM ClientHello, initial_mtu = base_mtu - 5ms w/ client buffering This is essentially adding a test case demonstrating the fixes in aws#3295 and aws#3296 work.
3 participants
Release Summary:
Resolved issues:
Description of changes:
This allows much faster recovery, since we don't need to wait for server to detect its initial packet was lost and recover from both the initial packet being lost and the subsequent handshake packet also getting lost.
Call-outs:
I think we had previously assumed this is only useful with offloading, but (in news to me), s2n-quic will sometimes attempt loss recovery of Handshake packets before the Initial packets are fully ack'd (or re-sent in loss recovery packets which are always at 1200 bytes), which currently leads to those getting dropped on the client.
See a trace here, where the "client-dropped" packets add ~3ms to the handshake timeline for no good reason because the client drops them rather than waiting until .
I haven't enabled it unconditionally on the server yet -- mostly because I don't have a trace that shows it being useful. It's possible that we should do it on both sides though (open to feedback there).
cc @maddeleine
Testing:
None here -- I want to clean up some of the large ClientHello testing and cut that, but I've tested with those larger tests that this does avoid the extra loss recovery mentioned above.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.