From 5ede40dcf27b06e992362e4da631281cafd7e67a Mon Sep 17 00:00:00 2001
From: Ryan Stone <rstone@FreeBSD.org>
Date: Sat, 11 Feb 2017 17:05:08 +0000
Subject: [PATCH] Don't zero out srtt after excess retransmits

If the TCP stack has retransmitted more than 1/4 of the total
number of retransmits before a connection drop, it decides that
its current RTT estimate is hopelessly out of date and decides
to recalculate it from scratch starting with the next ACK.

Unfortunately, it implements this by zeroing out the current RTT
estimate.  Drop this hack entirely, as it makes it significantly more
difficult to debug connection issues.  Instead check for excessive
retransmits at the point where srtt is updated from an ACK being
received.  If we've exceeded 1/4 of the maximum retransmits,
discard the previous srtt estimate and replace it with the latest
rtt measurement.

Differential Revision:	https://reviews.freebsd.org/D9519
Reviewed by:	gnn
Sponsored by:	Dell EMC Isilon
---
 sys/netinet/tcp_input.c |  2 +-
 sys/netinet/tcp_timer.c | 10 +++-------
 sys/netinet/tcp_timer.h |  7 +++++++
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index 607756134a5..7736ffcf63b 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -3494,7 +3494,7 @@ tcp_xmit_timer(struct tcpcb *tp, int rtt)
 
 	TCPSTAT_INC(tcps_rttupdated);
 	tp->t_rttupdated++;
-	if (tp->t_srtt != 0) {
+	if ((tp->t_srtt != 0) && (tp->t_rxtshift <= TCP_RTT_INVALIDATE)) {
 		/*
 		 * srtt is stored as fixed point with 5 bits after the
 		 * binary point (i.e., scaled by 8).  The following magic
diff --git a/sys/netinet/tcp_timer.c b/sys/netinet/tcp_timer.c
index f9f5c05fdce..064e45b0a51 100644
--- a/sys/netinet/tcp_timer.c
+++ b/sys/netinet/tcp_timer.c
@@ -845,20 +845,16 @@ tcp_timer_rexmt(void * xtp)
 	    (tp->t_rxtshift == 3))
 		tp->t_flags &= ~(TF_REQ_SCALE|TF_REQ_TSTMP|TF_SACK_PERMIT);
 	/*
-	 * If we backed off this far, our srtt estimate is probably bogus.
-	 * Clobber it so we'll take the next rtt measurement as our srtt;
-	 * move the current srtt into rttvar to keep the current
-	 * retransmit times until then.
+	 * If we backed off this far, notify the L3 protocol that we're having
+	 * connection problems.
 	 */
-	if (tp->t_rxtshift > TCP_MAXRXTSHIFT / 4) {
+	if (tp->t_rxtshift > TCP_RTT_INVALIDATE) {
 #ifdef INET6
 		if ((tp->t_inpcb->inp_vflag & INP_IPV6) != 0)
 			in6_losing(tp->t_inpcb);
 		else
 #endif
 			in_losing(tp->t_inpcb);
-		tp->t_rttvar += (tp->t_srtt >> TCP_RTT_SHIFT);
-		tp->t_srtt = 0;
 	}
 	tp->snd_nxt = tp->snd_una;
 	tp->snd_recover = tp->snd_max;
diff --git a/sys/netinet/tcp_timer.h b/sys/netinet/tcp_timer.h
index bb78062ddea..8b8d8252465 100644
--- a/sys/netinet/tcp_timer.h
+++ b/sys/netinet/tcp_timer.h
@@ -119,6 +119,13 @@
 
 #define	TCPTV_DELACK	( hz/10 )		/* 100ms timeout */
 
+/*
+ * If we exceed this number of retransmits for a single segment, we'll consider
+ * the current srtt measurement no longer valid and will recalculate from
+ * scratch starting with the next ACK.
+ */
+#define TCP_RTT_INVALIDATE (TCP_MAXRXTSHIFT / 4)
+
 #ifdef	TCPTIMERS
 static const char *tcptimers[] =
     { "REXMT", "PERSIST", "KEEP", "2MSL", "DELACK" };