SACK problems

[Noritoshi Demizu]

> During some testing on an isolated network we have, I found some
> interesting behaviour from a FreeBSD 5.3 host using TCP SACK.
>
> I've detailed this problem fully at:
>
>     http://www.wand.net.nz/~stj2/nsc/emu_freebsd.html

I experienced the same phenomenon.  When TCP reassembly queue exceeds
its limit (i.e., tcp_reass_maxseg and tcp_reass_maxqlen), tcp_reass()
drops received out-of-order data segment.  On the other hand, SACK list
is updated by such dropped segments.  Hence there arises inconsistency
between reassembly queue (tp->t_segq) and SACK list (tp->sackblks[]).
This is the cause of the phenomenon that I experienced.

The same phenomenon could occur when tcp_drain() is called.
tcp_drain() discards all reassembly queues, while it does not
discard SACK list.

The following patch to FreeBSD 5.3R solves my problem.
It would solve Sam Jansen's problem, too.

Regards,
Noritoshi Demizu


Index: tcp_input.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.252
diff -u -r1.252 tcp_input.c
--- tcp_input.c	17 Aug 2004 22:05:54 -0000	1.252
+++ tcp_input.c	2 Mar 2005 05:55:27 -0000
@@ -250,6 +250,7 @@
 		tcp_reass_overflows++;
 		tcpstat.tcps_rcvmemdrop++;
 		m_freem(m);
+		*tlenp = 0;
 		return (0);
 	}
 
@@ -261,6 +262,7 @@
 	if (te == NULL) {
 		tcpstat.tcps_rcvmemdrop++;
 		m_freem(m);
+		*tlenp = 0;
 		return (0);
 	}
 	tp->t_segqlen++;
@@ -2417,6 +2419,7 @@
 			thflags = tcp_reass(tp, th, &tlen, m);
 			tp->t_flags |= TF_ACKNOW;
 		}
+		if (tlen > 0)
 			if (tp->sack_enable)
 				tcp_update_sack_list(tp);
 		/*
Index: tcp_subr.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/tcp_subr.c,v
retrieving revision 1.201.2.1.2.1
diff -u -r1.201.2.1.2.1 tcp_subr.c
--- tcp_subr.c	21 Oct 2004 09:30:47 -0000	1.201.2.1.2.1
+++ tcp_subr.c	2 Mar 2005 05:55:28 -0000
@@ -818,6 +818,7 @@
 					tcpb->t_segqlen--;
 					tcp_reass_qsize--;
 				}
+				tcp_free_sackholes(tcpb);
 			}
 			INP_UNLOCK(inpb);
 		}