Bug 235256 - [tcp] IW10 without ABC leads to initial window of 11 Segments
Summary: [tcp] IW10 without ABC leads to initial window of 11 Segments
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Michael Tuexen
URL: https://reviews.freebsd.org/D19033
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-27 22:54 UTC by Richard Scheffenegger
Modified: 2021-01-24 18:00 UTC (History)
3 users (show)

See Also:
koobs: mfc-stable12+
koobs: mfc-stable11?


Attachments
packetdrill script to validate IW10 behavior without ABC (2.10 KB, text/plain)
2019-01-27 22:54 UTC, Richard Scheffenegger
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Scheffenegger freebsd_committer freebsd_triage 2019-01-27 22:54:12 UTC
Created attachment 201465 [details]
packetdrill script to validate IW10 behavior without ABC

Checking the Initial Window while disabling Appropriate Byte Counting:

net.inet.tcp.rfc3465=0
net.inet.tcp.initcwnd_segments=10

Transmits 11 packets (also exceeding the hardcoded limit of 14600 octets). 

After a casual inspection, this may be present since BSD10 when modular congestion control moved relevant code around.

Possibly this is due to the final ACK of the 3WHS getting processed normally,
and without ABC checking that this is an empty ACK, immeditately increasing cwnd
Comment 1 Richard Scheffenegger freebsd_committer freebsd_triage 2019-01-28 11:32:28 UTC
D19000 should address this particular issue, plus a nuisance caused as a side effect for ABC-enabled (default) sessions.
Comment 2 Richard Scheffenegger freebsd_committer freebsd_triage 2019-01-28 11:35:05 UTC
Also, this bug seems to pre-date modular congestion control. The order of when cwnd gets set to the initial window changed well before that, and as Appropriate Byte Counting is the default for a very long time, this passed unnoticed apparently.
Comment 3 Michael Tuexen freebsd_committer freebsd_triage 2019-01-30 16:04:23 UTC
review D19033 fixes a bug which has as a consequence that the bug reported here only shows up on TCP connections supporting TCP window scaling.
Comment 4 commit-hook freebsd_committer freebsd_triage 2019-02-01 12:33:33 UTC
A commit references this bug:

Author: tuexen
Date: Fri Feb  1 12:33:01 UTC 2019
New revision: 343661
URL: https://svnweb.freebsd.org/changeset/base/343661

Log:
  When handling SYN-ACK segments in the SYN-RCVD state, set tp->snd_wnd
  consistently.

  This inconsistency was observed when working on the bug reported in
  PR 235256, although it does not fix the reported issue. The fix for
  the PR will be a separate commit.

  PR:			235256
  Reviewed by:		rrs@, Richard Scheffenegger
  MFC after:		3 days
  Sponsored by:		Netflix, Inc.
  Differential Revision:	https://reviews.freebsd.org/D19033

Changes:
  head/sys/netinet/tcp_input.c
  head/sys/netinet/tcp_stacks/rack.c
Comment 5 commit-hook freebsd_committer freebsd_triage 2019-05-04 09:11:46 UTC
A commit references this bug:

Author: tuexen
Date: Sat May  4 09:11:17 UTC 2019
New revision: 347082
URL: https://svnweb.freebsd.org/changeset/base/347082

Log:
  MFC r343661:
  When handling SYN-ACK segments in the SYN-RCVD state, set tp->snd_wnd
  consistently.

  This inconsistency was observed when working on the bug reported in
  PR 235256, although it does not fix the reported issue. The fix for
  the PR will be a separate commit.

  PR:			235256
  Reviewed by:		rrs@, Richard Scheffenegger
  Sponsored by:		Netflix, Inc.
  Differential Revision:	https://reviews.freebsd.org/D19033

Changes:
_U  stable/12/
  stable/12/sys/netinet/tcp_input.c
  stable/12/sys/netinet/tcp_stacks/rack.c
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2019-05-10 09:04:45 UTC
Does this apply to stable/11 ?
Comment 7 commit-hook freebsd_committer freebsd_triage 2020-04-21 13:06:16 UTC
A commit references this bug:

Author: rscheff
Date: Tue Apr 21 13:05:45 UTC 2020
New revision: 360143
URL: https://svnweb.freebsd.org/changeset/base/360143

Log:
  Correctly set up the initial TCP congestion window
  in all cases, by adjust snd_una right after the
  connection initialization, to include the one byte
  in sequence space occupied by the SYN bit.

  This does not change the regular ACK processing,
  while making the BYTES_THIS_ACK macro to work properly.

  PR:		235256
  Reviewed by:	tuexen (mentor), rgrimes (mentor)
  Approved by:	tuexen (mentor), rgrimes (mentor)
  MFC after:	2 weeks
  Sponsored by:	NetApp, Inc.
  Differential Revision:	https://reviews.freebsd.org/D19000

Changes:
  head/sys/netinet/tcp_input.c
  head/sys/netinet/tcp_stacks/bbr.c
  head/sys/netinet/tcp_stacks/rack.c
Comment 8 commit-hook freebsd_committer freebsd_triage 2020-04-29 21:48:59 UTC
A commit references this bug:

Author: rscheff
Date: Wed Apr 29 21:48:53 UTC 2020
New revision: 360477
URL: https://svnweb.freebsd.org/changeset/base/360477

Log:
  Correctly set up the initial TCP congestion window in all cases,
  by not including the SYN bit sequence space in cwnd related calculations.
  Snd_und is adjusted explicitly in all cases, outside the cwnd update, instead.

  This fixes an off-by-one conformance issue with regular TCP sessions not
  using Appropriate Byte Counting (RFC3465), sending one more packet during
  the initial window than expected.

  PR:		235256
  Reviewed by:	tuexen (mentor), rgrimes (mentor)
  Approved by:	tuexen (mentor), rgrimes (mentor)
  MFC after:	3 weeks
  Sponsored by:	NetApp, Inc.
  Differential Revision:	https://reviews.freebsd.org/D19000

Changes:
  head/sys/netinet/tcp_input.c
  head/sys/netinet/tcp_stacks/bbr.c
  head/sys/netinet/tcp_stacks/rack.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2020-05-21 19:46:26 UTC
A commit references this bug:

Author: rscheff
Date: Thu May 21 19:46:12 UTC 2020
New revision: 361342
URL: https://svnweb.freebsd.org/changeset/base/361342

Log:
  MFC r360477: Correctly set up the initial TCP congestion window in all cases

  by not including the SYN bit sequence space in cwnd related calculations.
  Snd_und is adjusted explicitly in all cases, outside the cwnd update, instead.

  This fixes an off-by-one conformance issue with regular TCP sessions
  not  using Appropriate Byte Counting (RFC3465), sending one more
  packet during  the initial window than expected.

  PR:		235256
  Reviewed by:	tuexen (mentor), rgrimes (mentor, blanket)
  Approved by:	tuexen (mentor), rgrimes (mentor, blanket)
  MFC after:	3 weeks
  Sponsored by:	NetApp, Inc.
  Differential Revision:	https://reviews.freebsd.org/D19000

Changes:
_U  stable/12/
  stable/12/sys/netinet/tcp_input.c
  stable/12/sys/netinet/tcp_stacks/rack.c