Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 15 May 2020 14:00:13 +0000 (UTC)
From:      Randall Stewart <rrs@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r361080 - head/sys/netinet/tcp_stacks
Message-ID:  <202005151400.04FE0DKe022989@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: rrs
Date: Fri May 15 14:00:12 2020
New Revision: 361080
URL: https://svnweb.freebsd.org/changeset/base/361080

Log:
  This fixes several skyzaller issues found with the
  help of Michael Tuexen. There was some accounting
  errors with TCPFO for bbr and also for both rack
  and bbr there was a FO case where we should be
  jumping to the just_return_nolock label to
  exit instead of returning 0. This of course
  caused no timer to be running and thus the
  stuck sessions.
  
  Reported by: Michael Tuexen and Skyzaller
  Sponsored by: Netflix Inc.
  Differential Revision:	https://reviews.freebsd.org/D24852

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

Modified: head/sys/netinet/tcp_stacks/bbr.c
==============================================================================
--- head/sys/netinet/tcp_stacks/bbr.c	Fri May 15 13:53:10 2020	(r361079)
+++ head/sys/netinet/tcp_stacks/bbr.c	Fri May 15 14:00:12 2020	(r361080)
@@ -4975,6 +4975,15 @@ bbr_remxt_tmr(struct tcpcb *tp)
 			rsm->r_flags &= ~(BBR_ACKED | BBR_SACK_PASSED | BBR_WAS_SACKPASS);
 			bbr_log_type_rsmclear(bbr, cts, rsm, old_flags, __LINE__);
 		} else {
+			if ((tp->t_state < TCPS_ESTABLISHED) &&
+			    (rsm->r_start == tp->snd_una)) {
+				/*
+				 * Special case for TCP FO. Where
+				 * we sent more data beyond the snd_max.
+				 * We don't mark that as lost and stop here.
+				 */
+				break;
+			}
 			if ((rsm->r_flags & BBR_MARKED_LOST) == 0) {
 				bbr->r_ctl.rc_lost += rsm->r_end - rsm->r_start;
 				bbr->r_ctl.rc_lost_bytes += rsm->r_end - rsm->r_start;
@@ -12315,7 +12324,8 @@ bbr_output_wtime(struct tcpcb *tp, const struct timeva
 	     (tp->t_state == TCPS_SYN_SENT)) &&
 	    SEQ_GT(tp->snd_max, tp->snd_una) &&	/* initial SYN or SYN|ACK sent */
 	    (tp->t_rxtshift == 0)) {	/* not a retransmit */
-		return (0);
+		len = 0;
+		goto just_return_nolock;
 	}
 	/*
 	 * Before sending anything check for a state update. For hpts
@@ -14286,6 +14296,7 @@ nomore:
 	    (hw_tls == 0) &&
 	    (len > 0) &&
 	    ((flags & TH_RST) == 0) &&
+	    ((flags & TH_SYN) == 0) &&
 	    (IN_RECOVERY(tp->t_flags) == 0) &&
 	    (bbr->rc_in_persist == 0) &&
 	    (tot_len < bbr->r_ctl.rc_pace_max_segs)) {

Modified: head/sys/netinet/tcp_stacks/rack.c
==============================================================================
--- head/sys/netinet/tcp_stacks/rack.c	Fri May 15 13:53:10 2020	(r361079)
+++ head/sys/netinet/tcp_stacks/rack.c	Fri May 15 14:00:12 2020	(r361080)
@@ -3873,6 +3873,7 @@ skip_measurement:
 	 * the next send will trigger us picking up the missing data.
 	 */
 	if (rack->r_ctl.rc_first_appl &&
+	    TCPS_HAVEESTABLISHED(tp->t_state) &&
 	    rack->r_ctl.rc_app_limited_cnt &&
 	    (SEQ_GT(rack->r_ctl.rc_first_appl->r_start, th_ack)) &&
 	    ((rack->r_ctl.rc_first_appl->r_start - th_ack) >
@@ -11741,6 +11742,13 @@ rack_start_gp_measurement(struct tcpcb *tp, struct tcp
 	struct rack_sendmap *my_rsm = NULL;
 	struct rack_sendmap fe;
 
+	if (tp->t_state < TCPS_ESTABLISHED) {
+		/*
+		 * We don't start any measurements if we are
+		 * not at least established.
+		 */
+		return;
+	}
 	tp->t_flags |= TF_GPUTINPROG;
 	rack->r_ctl.rc_gp_lowrtt = 0xffffffff;
 	rack->r_ctl.rc_gp_high_rwnd = rack->rc_tp->snd_wnd;
@@ -12109,8 +12117,10 @@ rack_output(struct tcpcb *tp)
 	    ((tp->t_state == TCPS_SYN_RECEIVED) ||
 	     (tp->t_state == TCPS_SYN_SENT)) &&
 	    SEQ_GT(tp->snd_max, tp->snd_una) && /* initial SYN or SYN|ACK sent */
-	    (tp->t_rxtshift == 0))              /* not a retransmit */
-		return (0);
+	    (tp->t_rxtshift == 0)) {              /* not a retransmit */
+		cwnd_to_use = rack->r_ctl.cwnd_to_use = tp->snd_cwnd;
+		goto just_return_nolock;
+	}
 	/*
 	 * Determine length of data that should be transmitted, and flags
 	 * that will be used. If there is some data or critical controls

Modified: head/sys/netinet/tcp_stacks/rack_bbr_common.c
==============================================================================
--- head/sys/netinet/tcp_stacks/rack_bbr_common.c	Fri May 15 13:53:10 2020	(r361079)
+++ head/sys/netinet/tcp_stacks/rack_bbr_common.c	Fri May 15 14:00:12 2020	(r361080)
@@ -466,7 +466,14 @@ ctf_do_queued_segments(struct socket *so, struct tcpcb
 uint32_t
 ctf_outstanding(struct tcpcb *tp)
 {
-	return(tp->snd_max - tp->snd_una);
+	uint32_t bytes_out;
+
+	bytes_out = tp->snd_max - tp->snd_una;
+	if (tp->t_state < TCPS_ESTABLISHED)
+		bytes_out++;
+	if (tp->t_flags & TF_SENTFIN)
+		bytes_out++;
+	return (bytes_out);
 }
 
 uint32_t

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