Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 29 Apr 2020 21:48:52 +0000 (UTC)
From:      Richard Scheffenegger <rscheff@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r360477 - in head/sys/netinet: . tcp_stacks
Message-ID:  <202004292148.03TLmqX5022644@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: rscheff
Date: Wed Apr 29 21:48:52 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

Modified:
  head/sys/netinet/tcp_input.c
  head/sys/netinet/tcp_stacks/bbr.c
  head/sys/netinet/tcp_stacks/rack.c

Modified: head/sys/netinet/tcp_input.c
==============================================================================
--- head/sys/netinet/tcp_input.c	Wed Apr 29 21:12:32 2020	(r360476)
+++ head/sys/netinet/tcp_input.c	Wed Apr 29 21:48:52 2020	(r360477)
@@ -1470,7 +1470,7 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru
     struct tcpcb *tp, int drop_hdrlen, int tlen, uint8_t iptos)
 {
 	int thflags, acked, ourfinisacked, needoutput = 0, sack_changed;
-	int rstreason, todrop, win;
+	int rstreason, todrop, win, incforsyn = 0;
 	uint32_t tiwin;
 	uint16_t nsegs;
 	char *s;
@@ -2374,12 +2374,6 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru
 		if (IS_FASTOPEN(tp->t_flags) && tp->t_tfo_pending) {
 			tcp_fastopen_decrement_counter(tp->t_tfo_pending);
 			tp->t_tfo_pending = NULL;
-
-			/*
-			 * Account for the ACK of our SYN prior to
-			 * regular ACK processing below.
-			 */
-			tp->snd_una++;
 		}
 		if (tp->t_flags & TF_NEEDFIN) {
 			tcp_state_change(tp, TCPS_FIN_WAIT_1);
@@ -2400,6 +2394,13 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru
 			tcp_timer_activate(tp, TT_KEEP, TP_KEEPIDLE(tp));
 		}
 		/*
+		 * Account for the ACK of our SYN prior to
+		 * regular ACK processing below, except for
+		 * simultaneous SYN, which is handled later.
+		 */
+		if (SEQ_GT(th->th_ack, tp->snd_una) && !(tp->t_flags & TF_NEEDSYN))
+			incforsyn = 1;
+		/*
 		 * If segment contains data or ACK, will call tcp_reass()
 		 * later; if not, do so now to pass queued data to user.
 		 */
@@ -2693,6 +2694,15 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru
 process_ACK:
 		INP_WLOCK_ASSERT(tp->t_inpcb);
 
+		/*
+		 * Adjust for the SYN bit in sequence space,
+		 * but don't account for it in cwnd calculations.
+		 * This is for the SYN_RECEIVED, non-simultaneous
+		 * SYN case. SYN_SENT and simultaneous SYN are
+		 * treated elsewhere.
+		 */
+		if (incforsyn)
+			tp->snd_una++;
 		acked = BYTES_THIS_ACK(tp, th);
 		KASSERT(acked >= 0, ("%s: acked unexepectedly negative "
 		    "(tp->snd_una=%u, th->th_ack=%u, tp=%p, m=%p)", __func__,

Modified: head/sys/netinet/tcp_stacks/bbr.c
==============================================================================
--- head/sys/netinet/tcp_stacks/bbr.c	Wed Apr 29 21:12:32 2020	(r360476)
+++ head/sys/netinet/tcp_stacks/bbr.c	Wed Apr 29 21:48:52 2020	(r360477)
@@ -9326,11 +9326,6 @@ bbr_do_syn_recv(struct mbuf *m, struct tcphdr *th, str
 
 		tcp_fastopen_decrement_counter(tp->t_tfo_pending);
 		tp->t_tfo_pending = NULL;
-		/*
-		 * Account for the ACK of our SYN prior to regular
-		 * ACK processing below.
-		 */
-		tp->snd_una++;
 	}
 	/*
 	 * Make transitions: SYN-RECEIVED  -> ESTABLISHED SYN-RECEIVED* ->
@@ -9353,6 +9348,13 @@ bbr_do_syn_recv(struct mbuf *m, struct tcphdr *th, str
 		if (!IS_FASTOPEN(tp->t_flags))
 			cc_conn_init(tp);
 	}
+	/*
+	 * Account for the ACK of our SYN prior to
+	 * regular ACK processing below, except for
+	 * simultaneous SYN, which is handled later.
+	 */
+	if (SEQ_GT(th->th_ack, tp->snd_una) && !(tp->t_flags & TF_NEEDSYN))
+		tp->snd_una++;
 	/*
 	 * If segment contains data or ACK, will call tcp_reass() later; if
 	 * not, do so now to pass queued data to user.

Modified: head/sys/netinet/tcp_stacks/rack.c
==============================================================================
--- head/sys/netinet/tcp_stacks/rack.c	Wed Apr 29 21:12:32 2020	(r360476)
+++ head/sys/netinet/tcp_stacks/rack.c	Wed Apr 29 21:48:52 2020	(r360477)
@@ -6540,12 +6540,6 @@ rack_do_syn_recv(struct mbuf *m, struct tcphdr *th, st
 	if (IS_FASTOPEN(tp->t_flags) && tp->t_tfo_pending) {
 		tcp_fastopen_decrement_counter(tp->t_tfo_pending);
 		tp->t_tfo_pending = NULL;
-
-		/*
-		 * Account for the ACK of our SYN prior to
-		 * regular ACK processing below.
-		 */
-		tp->snd_una++;
 	}
 	if (tp->t_flags & TF_NEEDFIN) {
 		tcp_state_change(tp, TCPS_FIN_WAIT_1);
@@ -6563,6 +6557,13 @@ rack_do_syn_recv(struct mbuf *m, struct tcphdr *th, st
 		if (!IS_FASTOPEN(tp->t_flags))
 			cc_conn_init(tp);
 	}
+	/*
+	 * Account for the ACK of our SYN prior to
+	 * regular ACK processing below, except for
+	 * simultaneous SYN, which is handled later.
+	 */
+	if (SEQ_GT(th->th_ack, tp->snd_una) && !(tp->t_flags & TF_NEEDSYN))
+		tp->snd_una++;
 	/*
 	 * If segment contains data or ACK, will call tcp_reass() later; if
 	 * not, do so now to pass queued data to user.

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202004292148.03TLmqX5022644>