From owner-svn-src-all@freebsd.org Tue May 8 03:00:02 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CAA86FC9BF9; Tue, 8 May 2018 03:00:02 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 1B8F4837D5; Tue, 8 May 2018 03:00:01 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (localhost [127.0.0.1]) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3) with ESMTP id w482xxh0058850; Mon, 7 May 2018 19:59:59 -0700 (PDT) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: (from freebsd@localhost) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3/Submit) id w482xxrO058849; Mon, 7 May 2018 19:59:59 -0700 (PDT) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <201805080259.w482xxrO058849@pdx.rh.CN85.dnsmgr.net> Subject: Re: svn commit: r333346 - head/sys/netinet In-Reply-To: <201805080222.w482MYiX087233@repo.freebsd.org> To: Matt Macy Date: Mon, 7 May 2018 19:59:59 -0700 (PDT) CC: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 May 2018 03:00:03 -0000 > Author: mmacy > Date: Tue May 8 02:22:34 2018 > New Revision: 333346 > URL: https://svnweb.freebsd.org/changeset/base/333346 > > Log: > Fix spurious retransmit recovery on low latency networks Your commit message quality is defanitly much much better!! Just one little nit... > > TCP's smoothed RTT (SRTT) can be much larger than an actual observed RTT. This can be either because of hz restricting the calculable RTT to 10ms in VMs or 1ms using the default 1000hz or simply because SRTT recently incorporated a larger value. Can you wrap lines at 80 characters please. Thanks, > If an ACK arrives before the calculated badrxtwin (now + SRTT): > tp->t_badrxtwin = ticks + (tp->t_srtt >> (TCP_RTT_SHIFT + 1)); > > We'll erroneously reset snd_una to snd_max. If multiple segments were dropped and this happens repeatedly the transmit rate will be limited to 1MSS per RTO until we've retransmitted all drops. > > Reported by: rstone > Reviewed by: hiren, transport > Approved by: sbruno > MFC after: 1 month > Differential Revision: https://reviews.freebsd.org/D8556 > > Modified: > head/sys/netinet/tcp_input.c > head/sys/netinet/tcp_output.c > head/sys/netinet/tcp_timer.c > > Modified: head/sys/netinet/tcp_input.c > ============================================================================== > --- head/sys/netinet/tcp_input.c Tue May 8 01:39:45 2018 (r333345) > +++ head/sys/netinet/tcp_input.c Tue May 8 02:22:34 2018 (r333346) > @@ -1682,6 +1682,9 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru > to.to_tsecr -= tp->ts_offset; > if (TSTMP_GT(to.to_tsecr, tcp_ts_getticks())) > to.to_tsecr = 0; > + else if (tp->t_flags & TF_PREVVALID && > + tp->t_badrxtwin != 0 && SEQ_LT(to.to_tsecr, tp->t_badrxtwin)) > + cc_cong_signal(tp, th, CC_RTO_ERR); > } > /* > * Process options only when we get SYN/ACK back. The SYN case > @@ -1794,9 +1797,10 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru > TCPSTAT_INC(tcps_predack); > > /* > - * "bad retransmit" recovery. > + * "bad retransmit" recovery without timestamps. > */ > - if (tp->t_rxtshift == 1 && > + if ((to.to_flags & TOF_TS) == 0 && > + tp->t_rxtshift == 1 && > tp->t_flags & TF_PREVVALID && > (int)(ticks - tp->t_badrxtwin) < 0) { > cc_cong_signal(tp, th, CC_RTO_ERR); > @@ -2787,8 +2791,10 @@ process_ACK: > * original cwnd and ssthresh, and proceed to transmit where > * we left off. > */ > - if (tp->t_rxtshift == 1 && tp->t_flags & TF_PREVVALID && > - (int)(ticks - tp->t_badrxtwin) < 0) > + if (tp->t_rxtshift == 1 && > + tp->t_flags & TF_PREVVALID && > + tp->t_badrxtwin && > + SEQ_LT(to.to_tsecr, tp->t_badrxtwin)) > cc_cong_signal(tp, th, CC_RTO_ERR); > > /* > > Modified: head/sys/netinet/tcp_output.c > ============================================================================== > --- head/sys/netinet/tcp_output.c Tue May 8 01:39:45 2018 (r333345) > +++ head/sys/netinet/tcp_output.c Tue May 8 02:22:34 2018 (r333346) > @@ -206,7 +206,7 @@ tcp_output(struct tcpcb *tp) > #if defined(IPSEC) || defined(IPSEC_SUPPORT) > unsigned ipsec_optlen = 0; > #endif > - int idle, sendalot; > + int idle, sendalot, curticks; > int sack_rxmit, sack_bytes_rxmt; > struct sackhole *p; > int tso, mtu; > @@ -808,9 +808,12 @@ send: > /* Timestamps. */ > if ((tp->t_flags & TF_RCVD_TSTMP) || > ((flags & TH_SYN) && (tp->t_flags & TF_REQ_TSTMP))) { > - to.to_tsval = tcp_ts_getticks() + tp->ts_offset; > + curticks = tcp_ts_getticks(); > + to.to_tsval = curticks + tp->ts_offset; > to.to_tsecr = tp->ts_recent; > to.to_flags |= TOF_TS; > + if (tp->t_rxtshift == 1) > + tp->t_badrxtwin = curticks; > } > > /* Set receive buffer autosizing timestamp. */ > > Modified: head/sys/netinet/tcp_timer.c > ============================================================================== > --- head/sys/netinet/tcp_timer.c Tue May 8 01:39:45 2018 (r333345) > +++ head/sys/netinet/tcp_timer.c Tue May 8 02:22:34 2018 (r333346) > @@ -693,7 +693,12 @@ tcp_timer_rexmt(void * xtp) > tp->t_flags |= TF_WASCRECOVERY; > else > tp->t_flags &= ~TF_WASCRECOVERY; > - tp->t_badrxtwin = ticks + (tp->t_srtt >> (TCP_RTT_SHIFT + 1)); > + if ((tp->t_flags & TF_RCVD_TSTMP) == 0) > + tp->t_badrxtwin = ticks + (tp->t_srtt >> (TCP_RTT_SHIFT + 1)); > + /* In the event that we've negotiated timestamps > + * badrxtwin will be set to the value that we set > + * the retransmitted packet's to_tsval to by tcp_output > + */ > tp->t_flags |= TF_PREVVALID; > } else > tp->t_flags &= ~TF_PREVVALID; > > -- Rod Grimes rgrimes@freebsd.org