PERFORCE change 138280 for review

[Andre Oppermann]

http://perforce.freebsd.org/chv.cgi?CH=138280

Change 138280 by andre at andre_flirtbox on 2008/03/22 14:58:56

	Undo overly complex FIN handling in reassembly queue.  It's a can
	of worms and based on a discussion on TCPM not necessary in this
	complexity.  A simpler approach to follow.

Affected files ...

.. //depot/projects/tcp_reass/netinet/tcp_reass.c#18 edit
.. //depot/projects/tcp_reass/netinet/tcp_var.h#8 edit

Differences ...

==== //depot/projects/tcp_reass/netinet/tcp_reass.c#18 (text+ko) ====

@@ -198,7 +198,7 @@
 	 * Store TCP header information in local variables as
 	 * we may lose access to it after mbuf compacting.
 	 */
-	thflags = (th->th_flags & TH_FIN);
+	thflags = th->th_flags;
 	th_seq = th->th_seq;
 	th = NULL;		/* Prevent further use. */
 
@@ -210,6 +210,7 @@
 	}
 
 	/* XXX: should not happen, but does for some reason. */
+	/* XXX: May get FIN on otherwise empty segment. */
 	if (*tlenp == 0) {
 		m_freem(m);
 		return (0);
@@ -230,15 +231,13 @@
 		KASSERT(tqen == NULL ||
 		    SEQ_LT(tqe->trq_seq + tqe->trq_len, tqen->trq_seq),
 		    ("%s: overlapping blocks", __func__));
-		KASSERT(!(thflags & TH_FIN) || tqe == TAILQ_LAST(&tp->t_trq, trq_head),
-		    ("%s: FIN on block before last one", __func__));
 		i++;
 	}
 	LIST_FOREACH(tqe, &tp->t_trq_sack, trq_s) {
 		i--;
 	}
 	KASSERT(i == 0, ("%s: SEQ# ordered tailq and arrival ordered "
-	    "list are not equally long", __func__));
+	    "SACK list are not equally long", __func__));
 #endif
 
 	/*
@@ -287,30 +286,14 @@
 		mcnt += (n->m_flags & M_EXT) ?
 		    n->m_ext.ext_size + MSIZE : MSIZE;
 
-	/* Check if we've already received FIN; we can't accept data beyond it. */
 	tqe = TAILQ_LAST(&tp->t_trq, trq_head);
-	if (tqe && (tqe->trq_thflags & TH_FIN) && SEQ_LEQ(tqe->trq_seq, th_seq)) {
-		/* Properly count retransmitted perfect matching FIN. */
-		if (tqe->trq_seq == th_seq && *tlenp > 0)
-			tcpstat.tcps_rcvoopack++;
-		else {
-			tcpstat.tcps_rcvpackafterfin++;
-			tcpstat.tcps_rcvbyteafterfin += *tlenp;
-		}
-		m_freem(m);
-		*tlenp = 0;
-		return (0);
-	}
 
-	/* Check if this segments FIN is before the end of the last block. */
-	if (tqe && (thflags & TH_FIN) &&
-	    SEQ_GT(tqe->trq_seq + tqe->trq_len, th_seq + *tlenp)) {
-		/* TCP statistics. */
-		tcpstat.tcps_rcvpackafterfin++;
-		tcpstat.tcps_rcvbyteafterfin += *tlenp;
-		m_freem(m);
-		*tlenp = 0;
-		return (0);
+	/*
+	 * FIN handling is a bit tricky.
+	 * We only accept a FIN if it matches the right side of the sequence
+	 * space.
+	 */
+	if (thflags & TH_FIN) {
 	}
 
 	/* Check if this segment directly attaches to the end. */
@@ -321,7 +304,6 @@
 		tcp_reass_mcnt += mcnt;
 		tqe->trq_ml->m_next = m;
 		tqe->trq_ml = m_last(m);
-		tqe->trq_thflags |= thflags;
 		if (LIST_FIRST(&tp->t_trq_sack) != tqe) {
 			LIST_REMOVE(tqe, trq_s);
 			LIST_INSERT_HEAD(&tp->t_trq_sack, tqe, trq_s);
@@ -392,7 +374,6 @@
 			tcpstat.tcps_rcvduppack++;
 			tcpstat.tcps_rcvdupbyte += *tlenp;
 			tcpstat.tcps_reass_covered++;
-			tqe->trq_thflags |= thflags;
 			/* XXXAO: What to SACK report when duplicate? */
 			if (LIST_FIRST(&tp->t_trq_sack) != tqe) {
 				LIST_REMOVE(tqe, trq_s);
@@ -416,7 +397,6 @@
 			tqe->trq_seq = th_seq;
 			tqe->trq_m = m;
 			tqe->trq_ml = m_last(m);
-			tqe->trq_thflags |= thflags;
 			/* Check if segment bridges next block to merge. */
 			if (tqen != NULL &&
 			    SEQ_GEQ(tqe->trq_seq + tqe->trq_len, tqen->trq_seq))
@@ -481,7 +461,6 @@
 			tcp_reass_mcnt += mcnt;
 			tqe->trq_ml->m_next = m;
 			tqe->trq_ml = m_last(m);
-			tqe->trq_thflags |= thflags;
 			/* Check if segment bridges two blocks to merge. */
 			if (tqen != NULL &&
 			    SEQ_GEQ(tqe->trq_seq + tqe->trq_len, tqen->trq_seq))
@@ -522,7 +501,6 @@
 	tcp_reass_mcnt += mcnt;
 	tqen->trq_m = m;
 	tqen->trq_ml = m_last(m);
-	tqen->trq_thflags |= thflags;
 
 	/* Where to insert. */
 	if (tqe != NULL && SEQ_LT(tqe->trq_seq + tqe->trq_len, th_seq))
@@ -569,7 +547,6 @@
 		else
 			sbappendstream_locked(&so->so_rcv, tqe->trq_m);
 		tp->rcv_nxt += tqe->trq_len;
-		thflags = tqe->trq_thflags;
 		KASSERT(!(thflags & TH_FIN) || tqe == TAILQ_LAST(&tp->t_trq, trq_head),
 		    ("%s: FIN not on last block", __func__));
 		tp->t_trqmcnt -= tqe->trq_mcnt;

==== //depot/projects/tcp_reass/netinet/tcp_var.h#8 (text+ko) ====

@@ -47,7 +47,6 @@
 	tcp_seq		trq_seq;	/* start of block */
 	int		trq_len;	/* length of block */
 	int		trq_mcnt;	/* gross mbuf size of block */
-	int		trq_thflags;	/* thflags for segment chain */
 	struct mbuf	*trq_m;		/* mbuf chain of data */
 	struct mbuf	*trq_ml;	/* last mbuf in chain of data */
 };