Charles M. Hannum wrote:
> 
> 
> "Jonathan M. Bresler" <jmb@freefall.freebsd.org> writes:
> 
> > 
> > --- tcp_input.c.old	Sat Sep  7 22:49:11 1996
> > +++ tcp_input.c	Sat Sep  7 22:49:44 1996
> > @@ -451,5 +451,6 @@
> >  	 */
> >  	tp->t_idle = 0;
> > -	tp->t_timer[TCPT_KEEP] = tcp_keepidle;
> > +	if (TCPS_HAVEESTABLISHED(tp->t_state))
> > +		tp->t_timer[TCPT_KEEP] = tcp_keepidle;
> >  
> >  	/*
> 
> I'm not sure this is complete.

	this patch is not correct, but it is sufficient, i believe.

> In the case where we get an ACK that causes us to transition from
> SYN-RECEIVED to ESTABLISHED (or a SYN+ACK that causes us to transition
> from SYN-SENT to ESTABLISHED), this seems to leave the keepalive timer
> still set to the initial connection timeout.  While this isn't
> necessarily a bug, per se, it could cause the first keepalive probe to
> be done earlier than it should be.  Stevens even points this out: `But
> applying this fix then requires that the keepalive timer be set to its
> initial value of 2 hours when the connection moves to the established
> state.'

	agreed, adding that would make the patch correct as well ;)
	note, that the next packet received on the connection would
	set the timer to tcp_keepidle, because the if clause would
	be true.  so the setting of the timer is delayed for one packet.

> Also, it's worth noting explicitly that this change also affects (and
> *should* affect) SYN-SENT state in the same fashion as SYN-RECEIVED
> state; namely that we don't reset the timer until we've actually
> established the connection.  This is relevant if we get just an ACK or
> just a SYN while in SYN-SENT and either remain in SYN-SENT or
> transition to SYN-RECEIVED.

	agreed.  the state transition to ESTABLISHED is the determining
	factor in setting "tp->t_timer[TCPT_KEEP] = tcp_keepidle".
	the patch separates the two actions which are part of the
	same state machine transition.  sloppy work.
> 
> With all of the above in mind, the following change seems correct:
> 

	your source tree is either very different from FreeBSD or
	does not include the TTCP code.   you show "revision 1.24"
	FreeBSD is at "revision 1.46".	   the locations appear to be
	equivlaent but for the TTCP code.  in both cases, the
	state change to TCPS_ESTABLISHED, may instead be a change
	to TCPS_FIN_WAIT_1

	this is my patch, that i am using "right now"


Index: tcp_input.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.46
diff -c -2 -r1.46 tcp_input.c
*** tcp_input.c	1996/05/02 05:54:12	1.46
--- tcp_input.c	1996/09/12 00:32:50
***************
*** 451,455 ****
  	 */
  	tp->t_idle = 0;
! 	tp->t_timer[TCPT_KEEP] = tcp_keepidle;
  
  	/*
--- 451,456 ----
  	 */
  	tp->t_idle = 0;
! 	if (TCPS_HAVEESTABLISHED(tp->t_state))
! 		tp->t_timer[TCPT_KEEP] = tcp_keepidle;
  
  	/*
***************
*** 833,839 ****
  				tp->t_flags &= ~TF_NEEDFIN;
  				tiflags &= ~TH_SYN;
! 			} else
  				tp->t_state = TCPS_ESTABLISHED;
! 
  		} else {
  		/*
--- 834,841 ----
  				tp->t_flags &= ~TF_NEEDFIN;
  				tiflags &= ~TH_SYN;
! 			} else {
  				tp->t_state = TCPS_ESTABLISHED;
! 				tp->t_timer[TCPT_KEEP] = tcp_keepidle;
! 			}
  		} else {
  		/*
***************
*** 860,865 ****
  						tp->t_state = TCPS_FIN_WAIT_1;
  						tp->t_flags &= ~TF_NEEDFIN;
! 					} else
  						tp->t_state = TCPS_ESTABLISHED;
  					tp->t_flags |= TF_NEEDSYN;
  				} else
--- 862,869 ----
  						tp->t_state = TCPS_FIN_WAIT_1;
  						tp->t_flags &= ~TF_NEEDFIN;
! 					} else {
  						tp->t_state = TCPS_ESTABLISHED;
+ 						tp->t_timer[TCPT_KEEP] = tcp_keepidle;
+ 					}
  					tp->t_flags |= TF_NEEDSYN;
  				} else
***************
*** 1184,1189 ****
  			tp->t_state = TCPS_FIN_WAIT_1;
  			tp->t_flags &= ~TF_NEEDFIN;
! 		} else
  			tp->t_state = TCPS_ESTABLISHED;
  		/*
  		 * If segment contains data or ACK, will call tcp_reass()
--- 1188,1195 ----
  			tp->t_state = TCPS_FIN_WAIT_1;
  			tp->t_flags &= ~TF_NEEDFIN;
! 		} else {
  			tp->t_state = TCPS_ESTABLISHED;
+ 			tp->t_timer[TCPT_KEEP] = tcp_keepidle;
+ 		}
  		/*
  		 * If segment contains data or ACK, will call tcp_reass()


jmb
--
Jonathan M. Bresler           FreeBSD Postmaster             jmb@FreeBSD.ORG
FreeBSD--4.4BSD Unix for PC clones, source included. http://www.freebsd.org/
PGP 2.6.2 Fingerprint:      31 57 41 56 06 C1 40 13  C5 1C E3 E5 DC 62 0E FB