When the DATAGRAM fits but the probe does not
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 `DATAGRAM` was pending;
- the `DATAGRAM` fit the current path;
- it did not fit the 1200-byte probe;
- the pending `DATAGRAM` caused Quinn to skip the fallback `PING`.
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 outtest 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:
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.