Answer out-of-order segments with a duplicate ACK, keep straddling retransmissions, and keep segments inside the peer's window - #97
Open
WYCLIFF001 wants to merge 3 commits into
Conversation
A data segment is now always acknowledged. One that fills no gap, because it arrived out of order or was already received, used to leave nothing for the reader and so produced no ACK at all, which left the peer's retransmission timer as the only way past a lost segment. RFC 5681 §4.2 asks for an immediate duplicate ACK instead, and three of them are what drive the peer's fast retransmit. A segment carrying PSH is handled like any other: out of order, it used to be dropped without an ACK, so the peer resent it on its timer and backed off with every loss.
A segment starting below the ack was dropped whole, even when it ran past it. That happens when the peer re-segments a retransmission after an ACK it missed: the bytes past the ack were then lost with the rest, and the peer had to send them yet again. The segment is now trimmed to the part that is new (RFC 9293 §3.10.7.4); one lying entirely below the ack is still dropped as a duplicate.
A write was held only once the data in flight had reached the peer's window, and was then sent as a whole segment, so the last segment before the window filled could run up to a segment past it. RFC 9293 §3.8.6.1 keeps what is in flight within the window. A segment now carries no more than the room the window has left. The write also copies only the bytes the segment carries. It used to copy the caller's whole buffer and send one segment of it, so a large write was copied again for every segment it produced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three receive- and send-path fixes found while running ipstack as the TUN side of a VPN client under speedtest load (Linux peers, 40 ms to 100 ms RTT, 200 Mbit/s shaped link and unshaped loopback). Each commit carries its own test, which fails on
mainand passes with the change (the third reuses test helpers the first adds).1. Answer an out-of-order segment with a duplicate ACK
extract_data_n_write_upstreamsends an ACK only when it hands new in-order data to the reader, or when the handoff channel is full. A segment that fills no gap (out of order, or already received) produces no ACK at all, so the peer never sees the duplicate ACKs RFC 5681 §4.2 asks for and cannot fast-retransmit. A single lost segment is then recovered only by the peer's retransmission timer, with backoff on each further loss.Separately, in
Establisheda segment flaggedACK|PSHis accepted only whenseq == ack; out of order it is dropped silently, with no ACK. Linux sets PSH on the last segment of most writes, so this path is common.The five data-arrival sites now go through one
receive_segment, which stores the segment and sends an ACK if extraction did not already send one. TheACK|PSHarm stores out-of-order data like the plain-ACK arm.extract_data_n_write_upstreamreturns whether it wrote an ACK; the reader-drain path ignores it, so its behaviour is unchanged.Test:
out_of_order_segment_is_answered_with_a_duplicate_ack(bothACKandACK|PSH). Onmainthe out-of-order segment gets no ACK (left: [] right: [1001]).2. Keep the new part of a retransmission that straddles the ack
add_unordered_packetdrops any segment withseq < ackwhole, including one that extends pastack. A peer that missed an ACK and re-segments its retransmission sends exactly that shape, and the new bytes were lost with the old. The segment is now trimmed to the part pastack(RFC 9293 §3.10.7.4); one lying entirely belowackis still dropped, with the existing warning.Test:
test_add_unordered_packet_keeps_the_new_part_of_a_straddling_segment. Onmainthe new 50 bytes are gone (left: None).3. Keep each segment inside the peer's window
poll_writeparks only once the bytes in flight have reached the window (is_send_buffer_full), then sends a full segment. When the room left is smaller than a segment, the segment runs past the window's right edge (RFC 9293 §3.8.6.1). A segment now carries at mostmin(room left, MTU), via a newTcb::send_room.The same change stops
poll_writecopying the caller's entire buffer (buf.to_vec()) for every segment it sends. Only the bytes the segment carries are copied. Before, a 64 KiB write produced 45 segments, each copying the whole remaining tail: about 1.5 MB copied to send 64 KiB.Test:
segments_stay_inside_the_peers_window. Onmaina 4000-byte write into a 3000-byte window sends[1460, 1460, 1080].Checks
cargo fmt --check,cargo clippy --all-targets(no warnings) andcargo testall pass: 32 existing tests plus 3 new ones.Not included (open to discussion)
check_pkt_typerequires an unchanged window before counting an ACK as a duplicate, which matches RFC 5681 §2 (and Linux's own sender). I first thought this was a bug and it is not, so it is left as is.TcpConfig::options == Nonethe SYN-ACK carries no MSS, and Linux peers then send 536-byte segments (RFC 9293 §3.7.1 default) even on a 1500 or 9000 MTU device. Deriving a default MSS from the device MTU would cut the number of upload segments by about 2.5× at MTU 1400. It changes defaults, so I would rather ask first.write(2)each. Holding the ACK while more packets for the stream are already queued, up to every second full-sized segment as RFC 9293 §3.8.6.3 allows, measured about +50% single-stream upload throughput in our setup. It is a behaviour change, so it is a proposal rather than part of this PR.🤖 Generated with Claude Code