Repository navigation
Conversation
'TcpTxEngine.transmit_data' decided whether a segment is "partial"
(sub-MSS, subject to Nagle / Minshall deferral) by comparing its
payload length with '_snd_mss'. But when options ride on every segment
- TSopt (12 bytes), plus SACK blocks or AccECN when active - the
largest payload is 'mss_for_data' = '_snd_mss - options_overhead'
(RFC 6691 §2 Req B, already applied a few lines earlier when sizing
the segment). Every full segment was therefore classified as partial,
'SND.SML' moved past it, and the next full segment was held until the
previous one was ACKed: with timestamps negotiated (the Linux default)
a bulk send went out one segment per ACK. Against a Linux receiver
this also hits its 40 ms delayed-ACK timer, since it never gets the
second segment it waits for; measured upload through PyTCP fell to
~0.2 Mbit/s on a local link.
Compare with 'mss_for_data' instead, matching Linux, whose Nagle and
Minshall checks ('tcp_nagle_check' / 'tcp_minshall_update' in
net/ipv4/tcp_output.c) use 'mss_now' from 'tcp_current_mss()', which
already subtracts the established-connection option length.
Reference: RFC 1122 §4.2.3.4 (Nagle algorithm).
Reference: RFC 6691 §2 Req B (sender reduces data length for options).
New integration test
'test__data_transfer_send__nagle_does_not_hold_full_tsopt_segments'
sent 1 of 3 full TSopt segments before the fix (the rest held by
Nagle) and all 3 on 3 ticks after it.
make test: 13845 passing, 11 skipped. make lint clean.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
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.
'TcpTxEngine.transmit_data' decided whether a segment is "partial" (sub-MSS, subject to Nagle / Minshall deferral) by comparing its payload length with '_snd_mss'. But when options ride on every segment
Compare with 'mss_for_data' instead, matching Linux, whose Nagle and Minshall checks ('tcp_nagle_check' / 'tcp_minshall_update' in net/ipv4/tcp_output.c) use 'mss_now' from 'tcp_current_mss()', which already subtracts the established-connection option length.
Reference: RFC 1122 §4.2.3.4 (Nagle algorithm).
Reference: RFC 6691 §2 Req B (sender reduces data length for options).
New integration test
'test__data_transfer_send__nagle_does_not_hold_full_tsopt_segments' sent 1 of 3 full TSopt segments before the fix (the rest held by Nagle) and all 3 on 3 ticks after it.
make test: 13845 passing, 11 skipped. make lint clean.