tcp: retransmit queue + window-gated TX for TcpFlow - #55
Draft
sarsanaee wants to merge 1 commit into
Draft
Conversation
Addresses the three ship-blocking TX-path defects from the PR #54 review: lost data was unrecoverable, transmission ignored the peer window, and an allocation failure mid-message corrupted the wire stream and leaked buffers. Core change: OutputMessage no longer segments-and-forgets. It frames the message (4-byte length prefix + payload) into a per-flow byte-stream send buffer (snd_buf_, spanning [snd_una_, snd_una_+size)) and calls PumpSend, which transmits only up to min(buffered end, snd_una_+snd_wnd_) in EffectiveMSS-sized segments. Buffered bytes are retained until acknowledged. - Data retransmission (#1): PeriodicCheck now retransmits buffered data (go-back-N from snd_una_) on RTO in ESTABLISHED/CLOSE_WAIT/FIN_WAIT_1/ LAST_ACK, and notifies the app (callback false) on teardown from ANY state, not just SYN_SENT. AdvanceSndUna drops acked bytes from snd_buf_ and reopens the window. - Window-gated TX (#5): PumpSend honors snd_wnd_ and bytes-in-flight; it is re-run on every received ACK (via InputPacket) so data drains as the window opens. A minimal zero-window persist probe prevents the deadlock that strict window-gating would otherwise introduce. - No-corruption alloc failure (#6): SendDataSegment returns false instead of CHECK-crashing or emitting a short frame; unsent bytes stay buffered for the next pump. Every OutputMessage exit path frees the MsgBuf chain, and a msg_length that disagrees with the chain is dropped rather than framed. FIN is deferred behind buffered data (fin_pending_/fin_sent_/snd_fin_seq_) so its sequence number is correct even when the app closes with data still queued; HandleFinWait1's our_fin_acked check is updated accordingly. As a side effect the RTO now stays armed after FIN, so a lost FIN retransmits. EffectiveMSS caps segments at kDefaultMSS, defusing the over-large-peer-MSS allocation abort on the data path. Adds 7 white-box tests: window gating buffers excess, window-open drains, RTO retransmits unacked data, ACK frees the buffer and disarms the RTO, established RTO exhaustion notifies the app, FIN deferred behind window-limited data, and msg_length-mismatch drop. Not yet compiled/run: this is a Linux+DPDK target and was authored on macOS. Needs a build + `ctest` pass (sudo/hugepages) on a DPDK host before merge. Out of scope here and left for follow-up: SYN-retransmit sequence bug, checksum offload, RX-side desync, per-flow close (DESTROY_FLOW). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019xDGAYTziq2pwPgsEaqzEM
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.
Builds on #54. Fixes the three ship-blocking TX-path defects from the review of the native TCP stack.
What was broken
OutputMessagesegmented and freed the source buffers immediately;PeriodicCheckdid nothing for established data. A single lost segment was permanent data loss, and the flow was torn down silently (the app was never notified).snd_wnd_and bytes-in-flight, so any message larger than the peer window (≤64 KB) deterministically lost data — with zero network loss.PacketAllocfailure left a partial frame on the wire (never completed), leaked the MsgBuf chain, and desynced all subsequent messages.What this does
Introduces a per-flow byte-stream send buffer (
snd_buf_, spanning[snd_una_, snd_una_+size)).OutputMessagenow frames the message into that buffer and callsPumpSend, which transmits only up tomin(buffered end, snd_una_ + snd_wnd_)inEffectiveMSS-sized segments. Buffered bytes are retained until acknowledged.PeriodicCheckdoes go-back-N fromsnd_una_on RTO in ESTABLISHED/CLOSE_WAIT/FIN_WAIT_1/LAST_ACK; teardown on max-retransmit now notifies the app from any state.AdvanceSndUnadrops acked bytes and reopens the window.PumpSendhonors the peer window and re-runs on every received ACK (data drains as the window opens). A minimal zero-window persist probe prevents a new deadlock.SendDataSegmentreturnsfalseinstead of crashing or emitting a short frame; unsent bytes stay buffered. EveryOutputMessageexit path frees the chain; amsg_lengththat disagrees with the chain is dropped, not framed.FIN is deferred behind buffered data (
fin_pending_/fin_sent_/snd_fin_seq_) so its sequence number is correct when the app closes with data still queued;HandleFinWait1'sour_fin_ackedcheck is updated to match. As a side effect the RTO stays armed after FIN, so a lost FIN now retransmits.EffectiveMSScaps segments atkDefaultMSS, incidentally defusing the over-large-peer-MSS allocation abort on the data path.Tests
Adds 7 white-box tests to
tcp_flow_test.cc: window-gated buffering, window-open drain, RTO data retransmit, ACK frees buffer + disarms RTO, established RTO exhaustion notifies app, FIN deferred behind window-limited data, msg_length-mismatch drop.This is a Linux + DPDK target; it was authored on macOS and has not been compiled or run. It needs a build +
ctestpass (sudo/hugepages) on a DPDK host before merge.Out of scope (follow-ups from the same review)
SYN-retransmit sequence bug, TCP checksum offload not enabled on the port, RX-side buffer-exhaustion desync, per-flow close (
TCP_DESTROY_FLOWno-op), passive-flow port release.🤖 Generated with Claude Code