Subject: Re: Odd crashes in tcp_output (2.0ish)
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Charles M. Hannum <abuse@spamalicious.com>
List: tech-net
Date: 01/27/2005 05:19:19
--Boundary-00=_XnH+Brk8hfIGfXO
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On Thursday 27 January 2005 04:26, YAMAMOTO Takashi wrote:
> > +  /*
> > +   * Calculate the RTT from the returned time stamp and the
> > +   * connection's time base.  If the time stamp is later than
> > +   * the current time, fall back to non-1323 RTT calculation.
> > +   */
> > +  now = TCP_TIMESTAMP(tp);
> > +  if (SEQ_GT(now, opti.ts_ecr))
> > +   opti.ts_ecr = now - opti.ts_ecr + 1;
> > +  else
> > +   opti.ts_ecr = 0;
>
> - i don't think that it makes much sense because it still allows
>   unsafe rtt values.  it's better to be more strict here.
>   eg. (now - ts_ecr) < (1 day)

I suppose I should clamp the value so that the smoothing function in 
tcp_xmit_timer() does not overflow.  My inclination is to use TCP_PAWS_IDLE 
here.

> i think it's better to assert (tvmin <= tvmax) in TCPT_RANGESET.

That is *definitely* not a reasonable solution.  It moves the problem to an 
obscure place where the effect isn't obvious, *and* it doesn't cause the 
fallback to non-1323 RTT measurement.

Something like the attached might be okay.

--Boundary-00=_XnH+Brk8hfIGfXO
Content-Type: text/x-diff;
  charset="iso-8859-1";
  name="tcp.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="tcp.diff"

Index: tcp_input.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/tcp_input.c,v
retrieving revision 1.214
diff -u -r1.214 tcp_input.c
--- tcp_input.c	27 Jan 2005 03:39:36 -0000	1.214
+++ tcp_input.c	27 Jan 2005 05:19:09 -0000
@@ -1501,12 +1501,15 @@
 		/*
 		 * Calculate the RTT from the returned time stamp and the
 		 * connection's time base.  If the time stamp is later than
-		 * the current time, fall back to non-1323 RTT calculation.
+		 * the current time, or is extremely old, fall back to non-1323
+		 * RTT calculation.
 		 */
 		now = TCP_TIMESTAMP(tp);
-		if (SEQ_GEQ(now, opti.ts_ecr))
+		if (TSTMP_GEQ(now, opti.ts_ecr)) {
 			opti.ts_ecr = now - opti.ts_ecr + 1;
-		else
+			if (opti.ts_ecr > TCP_PAWS_IDLE)
+				opti.ts_ecr = 0;
+		}else
 			opti.ts_ecr = 0;
 	}
 

--Boundary-00=_XnH+Brk8hfIGfXO--