PERFORCE change 133508 for review

[Andre Oppermann]

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

Change 133508 by andre at andre_flirtbox on 2008/01/17 22:47:02

	Catch segments with length zero that make to into tcp_reass().
	This should normally be filtered out by tcp_do_segment() but
	for some reason still makes it here.  Zero length segments are
	completely useless for reassembly.
	
	Undo the TAILQ_FOREACH_SAFE change as it turns out that the
	queue macros use a very interesting c language construct I've
	just learned about.  Second the debugger led me astray when
	examining tvar in a crash dump.  Everything indeed works as
	advertized.
	
	Use a trq struct on the stack for the missing segment.  This
	prevents starvation when we run out of memory in the reassembly
	uma zone.
	
	The code now handles a full download of the FreeBSD cd images
	at 2% inbound packet loss (through ipfw).

Affected files ...

.. //depot/projects/tcp_reass/netinet/tcp_reass.c#9 edit

Differences ...

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

@@ -73,6 +73,7 @@
  * them in place.  Only when trimming from the tail it actually frees them.
  * Normally we don't get mbuf chains so this isn't too much of a concern
  * right now.  TODO.
+ * Add a timer that cleans out the reassembly queue after 2MSL or so. TODO.
  */
 
 #include "opt_inet.h"
@@ -145,6 +146,7 @@
 	struct socket *so = tp->t_inpcb->inp_socket;
 	struct mbuf *n;
 	int i, flags = 0, mcnt;
+	struct trq tqes;
 
 	INP_LOCK_ASSERT(tp->t_inpcb);
 
@@ -161,6 +163,9 @@
 		goto present;
 	}
 
+	/* XXX: should not happen, but does for some reason. */
+	if (*tlenp == 0)
+		return (0);
 	KASSERT(*tlenp > 0,
 	    ("%s: segment doesn't contain any data", __func__));
 	KASSERT(SEQ_LEQ(tp->rcv_nxt, th->th_seq),
@@ -169,8 +174,7 @@
 	    ("%s: got missing segment but queue is empty", __func__));
 
 #ifdef INVARIANTS
-	TAILQ_FOREACH(tqe, &tp->t_trq, trq_q) {
-		tqen = TAILQ_NEXT(tqe, trq_q);
+	TAILQ_FOREACH_SAFE(tqe, &tp->t_trq, trq_q, tqen) {
 		KASSERT(SEQ_GEQ(tqe->trq_seq, tp->rcv_nxt),
 		    ("%s: trq_seq < rcv_nxt", __func__));
 		KASSERT(tqen == NULL ||
@@ -280,8 +284,7 @@
 	tcpstat.tcps_rcvoobyte += *tlenp;
 
 	/* See where it fits. */
-	TAILQ_FOREACH(tqe, &tp->t_trq, trq_q) {
-		tqen = TAILQ_NEXT(tqe, trq_q);
+	TAILQ_FOREACH_SAFE(tqe, &tp->t_trq, trq_q, tqen) {
 		/* Segment is after this blocks coverage. */
 		if (SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq))
 			continue;
@@ -371,7 +374,10 @@
 
 insert:
 	/* Prepare to insert into block queue. */
-	tqen = uma_zalloc(tcp_reass_zone, (M_NOWAIT|M_ZERO));
+	if (tp->rcv_nxt == th->th_seq)
+		tqen = &tqes;
+	else
+		tqen = uma_zalloc(tcp_reass_zone, (M_NOWAIT|M_ZERO));
 	if (tqen == NULL) {
 		tcpstat.tcps_rcvmemdrop++;
 		m_freem(m);
@@ -408,7 +414,6 @@
 	KASSERT(!TAILQ_EMPTY(&tp->t_trq),
 	    ("%s: queue empty at present", __func__));
 	SOCKBUF_LOCK(&so->so_rcv);
-	tqen = TAILQ_NEXT(TAILQ_FIRST(&tp->t_trq), trq_q);
 	TAILQ_FOREACH_SAFE(tqe, &tp->t_trq, trq_q, tqen) {
 		KASSERT(SEQ_GEQ(tqe->trq_seq, tp->rcv_nxt),
 		    ("%s: trq_seq < rcv_nxt", __func__));
@@ -428,7 +433,8 @@
 		tp->rcv_nxt += tqe->trq_len;
 		tp->t_trqmcnt -= tqe->trq_mcnt;
 		TAILQ_REMOVE(&tp->t_trq, tqe, trq_q);
-		uma_zfree(tcp_reass_zone, tqe);
+		if (tqe != &tqes)
+			uma_zfree(tcp_reass_zone, tqe);
 		tcp_reass_qsize--;
 	}
 	/* NB: sorwakeup_locked() does an implicit socket buffer unlock. */