Summary
Flight-size accounting mixes plaintext and encrypted byte counts: on_send_with_token
increments flight size by the plaintext fragment/packet size, while every
release path — ACK (report_received_receipts → on_ack*), Abandon
(ResendAction::Abandon → release_flightsize), and the new stream-abort
drop_stream (#4345) — decrements by the encrypted stored payload length
(pending_receipts payload = to_packet_data() output, which includes the AEAD tag +
framing overhead).
So each packet's release subtracts slightly more than was added (by the per-packet
crypto/framing overhead), biasing the connection-wide flight-size counter downward.
Status / scope
This is pre-existing (it predates #4345) and self-consistent across all three
release paths, so #4345's drop_stream is correct relative to the existing
convention — it releases exactly what the same packets' ACK/Abandon would have. It is
not a regression introduced by the flight-size-on-abort fix; flagging it as its own
tracking item so the convention can be made consistent (add plaintext on both sides, or
encrypted on both sides) in a focused change.
Impact
Low in practice: FixedRate (the default congestion control) is rate-limited by the
token bucket, not flight size, so the downward bias mainly loosens cwnd discipline
slightly. Worth tightening for BBR/LEDBAT paths where flight size is load-bearing.
Suggested fix
Pick one convention end-to-end: either store the plaintext packet_size alongside the
encrypted payload in pending_receipts and release that, or add the encrypted size at
on_send. Add a unit test asserting sum(on_send) == sum(release) for a stream that is
fully ACKed, fully abandoned, and dropped via drop_stream.
Found by the external (Codex) reviewer during #4345 stage-2 review, and independently
noted by two Claude reviewers as pre-existing.
[AI-assisted - Claude]
Summary
Flight-size accounting mixes plaintext and encrypted byte counts:
on_send_with_tokenincrements flight size by the plaintext fragment/packet size, while every
release path — ACK (
report_received_receipts→on_ack*), Abandon(
ResendAction::Abandon→release_flightsize), and the new stream-abortdrop_stream(#4345) — decrements by the encrypted stored payload length(
pending_receiptspayload =to_packet_data()output, which includes the AEAD tag +framing overhead).
So each packet's release subtracts slightly more than was added (by the per-packet
crypto/framing overhead), biasing the connection-wide flight-size counter downward.
Status / scope
This is pre-existing (it predates #4345) and self-consistent across all three
release paths, so #4345's
drop_streamis correct relative to the existingconvention — it releases exactly what the same packets' ACK/Abandon would have. It is
not a regression introduced by the flight-size-on-abort fix; flagging it as its own
tracking item so the convention can be made consistent (add plaintext on both sides, or
encrypted on both sides) in a focused change.
Impact
Low in practice: FixedRate (the default congestion control) is rate-limited by the
token bucket, not flight size, so the downward bias mainly loosens cwnd discipline
slightly. Worth tightening for BBR/LEDBAT paths where flight size is load-bearing.
Suggested fix
Pick one convention end-to-end: either store the plaintext
packet_sizealongside theencrypted payload in
pending_receiptsand release that, or add the encrypted size aton_send. Add a unit test assertingsum(on_send) == sum(release)for a stream that isfully ACKed, fully abandoned, and dropped via
drop_stream.Found by the external (Codex) reviewer during #4345 stage-2 review, and independently
noted by two Claude reviewers as pre-existing.
[AI-assisted - Claude]