diff options
| author | Bhasker Hariharan <bhaskerh@google.com> | 2019-01-29 18:12:31 -0800 |
|---|---|---|
| committer | Shentubot <shentubot@google.com> | 2019-01-29 18:13:48 -0800 |
| commit | f03c7e48e71009a64c30b57648fd234710fd578a (patch) | |
| tree | 908bce875e9dd050d122c7c1eda9900ec359690b | |
| parent | 53afa68988f84ac8df62b2df6463457c39c1e803 (diff) | |
Fix IsLost check to match the description in RFC6675.
quoting what "rscheff@gmx.at" pointed out over email.
"IsLost in RFC3517 is defined as >= (DupThresh * SMSS) while
RFC6675 improves upon this, and defines IsLost as >
((DupThresh - 1) * SMSS + 1).
The latter addresses situations where partial segments (size < MSS)
are sent (eg. last segment of a http protocol message sent with PSH
being less than MSS is common)."
PiperOrigin-RevId: 231512331
Change-Id: I1addd4a92e3e7baeb0bdda46463ebfae435da958
| -rw-r--r-- | pkg/tcpip/transport/tcp/sack_scoreboard.go | 2 | ||||
| -rw-r--r-- | pkg/tcpip/transport/tcp/sack_scoreboard_test.go | 21 |
2 files changed, 12 insertions, 11 deletions
diff --git a/pkg/tcpip/transport/tcp/sack_scoreboard.go b/pkg/tcpip/transport/tcp/sack_scoreboard.go index d251d32db..5dba8a165 100644 --- a/pkg/tcpip/transport/tcp/sack_scoreboard.go +++ b/pkg/tcpip/transport/tcp/sack_scoreboard.go @@ -201,7 +201,7 @@ func (s *SACKScoreboard) IsLost(r header.SACKBlock) bool { } nDupSACKBytes += sacked.Start.Size(sacked.End) nDupSACK++ - if nDupSACK >= nDupAckThreshold || nDupSACKBytes >= seqnum.Size(nDupAckThreshold*s.smss) { + if nDupSACK >= nDupAckThreshold || nDupSACKBytes >= seqnum.Size((nDupAckThreshold-1)*s.smss) { isLost = true return false } diff --git a/pkg/tcpip/transport/tcp/sack_scoreboard_test.go b/pkg/tcpip/transport/tcp/sack_scoreboard_test.go index db4b52aca..1a1178999 100644 --- a/pkg/tcpip/transport/tcp/sack_scoreboard_test.go +++ b/pkg/tcpip/transport/tcp/sack_scoreboard_test.go @@ -94,20 +94,21 @@ func TestSACKScoreboardIsLost(t *testing.T) { s.Insert(header.SACKBlock{51, 100}) s.Insert(header.SACKBlock{111, 120}) s.Insert(header.SACKBlock{101, 110}) - s.Insert(header.SACKBlock{121, 151}) + s.Insert(header.SACKBlock{121, 141}) testCases := []struct { block header.SACKBlock lost bool }{ - {header.SACKBlock{0, 1}, true}, - {header.SACKBlock{1, 2}, false}, - {header.SACKBlock{1, 45}, false}, - {header.SACKBlock{50, 51}, true}, - // This one should return true because there are > 3* 10 (smss) - // bytes that have been sacked above this sequence number. - {header.SACKBlock{119, 120}, true}, - {header.SACKBlock{120, 121}, true}, - {header.SACKBlock{125, 126}, false}, + {block: header.SACKBlock{0, 1}, lost: true}, + {block: header.SACKBlock{1, 2}, lost: false}, + {block: header.SACKBlock{1, 45}, lost: false}, + {block: header.SACKBlock{50, 51}, lost: true}, + // This one should return true because there are + // > (nDupAckThreshold - 1) * 10 (smss) bytes that have been sacked above + // this sequence number. + {block: header.SACKBlock{119, 120}, lost: true}, + {block: header.SACKBlock{120, 121}, lost: true}, + {block: header.SACKBlock{125, 126}, lost: false}, } for _, tc := range testCases { if want, got := tc.lost, s.IsLost(tc.block); got != want { |