When the DATAGRAM fits but the probe does not
Fixing a tail-loss edge case in Quinn.
While working on quinn-rs/quinn#2173, I found an edge case in Quinn’s loss-probe handling: Quinn could omit the fallback PING even though the queued DATAGRAM did not fit into the probe. In that case, the probe could leave without the ACK-eliciting frame that the fallback was meant to guarantee.
Quinn avoids adding a fallback PING when ACK-eliciting 1-RTT data is already waiting to be sent. A pending DATAGRAM normally satisfies that condition because DATAGRAM frames are ACK-eliciting.
The problem was that a pending DATAGRAM was treated as if it would necessarily be included in the probe. That is not always true.
What went wrong
A loss probe is limited to the smaller of the current segment size and INITIAL_MTU. In Quinn, INITIAL_MTU is 1200 bytes.
The established path can have a larger MTU. The regression case uses a path MTU of 1452 bytes. A DATAGRAM can therefore be valid for the path and still be too large for the probe.
That left Quinn in this state:
- an ACK-eliciting
DATAGRAMwas pending; - the
DATAGRAMfit the current path; - it did not fit the 1200-byte probe;
- the pending
DATAGRAMcaused Quinn to skip the fallbackPING.
The queue told us that there was data to send. It did not tell us that the data fit this packet.
Without ACK Frequency support from the peer, the probe still needed its PING.
The fix
The decision now uses the space available in the probe. Quinn calculates the packet budget, subtracts the predicted 1-RTT overhead and passes the remaining frame space to can_send_1rtt().
Quinn’s actual send path also deals with packet spaces, loss state and frame scheduling. The relevant part can be reduced to this pseudocode:
let probe_size = min(segment_size, INITIAL_MTU);
let frame_space =
probe_size.saturating_sub(predict_1rtt_overhead(next_packet_number));
let data_fits = can_send_1rtt(frame_space);
maybe_queue_probe(
peer_supports_ack_frequency(),
data_fits,
);
If ACK-eliciting data fits in the remaining frame space, Quinn can omit the fallback PING. If the queued DATAGRAM is too large, the PING stays.
Using can_send_1rtt() also avoids adding a separate DATAGRAM-size calculation to the probe code. The decision follows the same send-path rules that packet assembly uses later.
Regression coverage
The new test is tail_loss_probe_keeps_ping_when_datagram_does_not_fit.
It configures a path MTU of 1452 bytes and disables peer ACK Frequency support. It then creates an outstanding ACK-eliciting packet, discards it and advances the connection to the probe timeout.
At that point, the test queues a DATAGRAM at the maximum size permitted by the path. The DATAGRAM fits a normal packet but not the probe capped at INITIAL_MTU.
The test checks that the fallback PING is transmitted. It also checks that the DATAGRAM is delivered later, when Quinn builds a packet large enough to contain it.
I also updated the existing test tail_loss_small_segment_size.
That test covers the opposite case. Its DATAGRAM does fit inside the probe, so no additional PING should be sent.
Both tests are necessary. Always adding a PING would make the new regression test pass, but it would remove the optimization that the original code was trying to preserve.
I ran both tests against the exact merged commit:
test tests::tail_loss_probe_keeps_ping_when_datagram_does_not_fit ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 299 filtered out
test tests::tail_loss_small_segment_size ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 299 filtered out
Merged change
The patch was merged through quinn-rs/quinn#2794 as commit 6469478.
It changes three files:
quinn-proto/src/connection/mod.rsquinn-proto/src/connection/spaces.rsquinn-proto/src/tests/mod.rs
The final diff contains 88 insertions and 3 deletions. Most of the added code is in the tests.
The bug came down to checking whether data was pending when the code needed to know whether that data could be sent in the current packet. Once the probe’s actual byte budget was used for the decision, the fallback behaviour became straightforward.
Thanks to the Quinn maintainers for the review.