Re: panic in TCP Limited Transmit after RTO

[Jeffrey Hsu <hsu@xxxxxxxxxxxxxxxx>]

Noritoshi Demizu wrote:
> > Limited Transmit is supposed to send new data, and, we are out of
> > SACK recovery at this point, so I think we can safely ignore any old
> > SACK blocks.  Please try this patch.
>
>
> Now I start understanding what your patch does.
>
>   o When doing Limited Transmit, ignore SACK blocks and send the data
>     starting at snt_nxt, even if the data has been SACKed.
>
>   o In Congestion Avoidance phase, ignore SACK blocks and send the data
>     starting at snt_nxt, even if the data has been SACKed.
>
> I do not think these are good ideas.

Actually, the latest patch I have as a commit candidate is attached below.
It simply sends out new data starting at snd_max, as before, and ingores SACK
blocks, as it's new data.  This patch corrects a problem that was exacerbated
by the recent change to tcp_output, where we potentially send out more data on
slow-start.

It does not follow your previous suggestion of sending at snd_nxt, as I'm
pretty sure that's against the letter and spirit of the Limit Transmit
spec.  From your last message, it would seem you would agree that your
original suggestion of sending from snd_nxt is a bad idea too.

Index: tcp_input.c
===================================================================
RCS file: /j/dragonfly/dcvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.54
diff -u -p -r1.54 tcp_input.c
--- tcp_input.c	9 Mar 2005 06:57:29 -0000	1.54
+++ tcp_input.c	13 Mar 2005 06:10:53 -0000
@@ -1954,6 +1954,7 @@ fastretransmit:
 					    (tp->t_dupacks - tp->snd_limited);
 			} else if (tcp_do_limitedtransmit) {
 				u_long oldcwnd = tp->snd_cwnd;
+				u_long oldssthresh = tp->snd_ssthresh;
 				tcp_seq oldsndmax = tp->snd_max;
 				/* outstanding data */
 				uint32_t ownd = tp->snd_max - tp->snd_una;
@@ -1969,8 +1970,10 @@ fastretransmit:
 				tp->snd_cwnd = ownd +
 				    (tp->t_dupacks - tp->snd_limited) *
 				    tp->t_maxseg;
+				tp->snd_ssthresh = tp->snd_cwnd;
 				tcp_output(tp);
 				tp->snd_cwnd = oldcwnd;
+				tp->snd_ssthresh = oldssthresh;
 				sent = tp->snd_max - oldsndmax;
 				if (sent > tp->t_maxseg) {
 					KASSERT((tp->t_dupacks == 2 &&
Index: tcp_output.c
===================================================================
RCS file: /j/dragonfly/dcvs/src/sys/netinet/tcp_output.c,v
retrieving revision 1.25
diff -u -p -r1.25 tcp_output.c
--- tcp_output.c	9 Mar 2005 06:54:34 -0000	1.25
+++ tcp_output.c	12 Mar 2005 22:23:00 -0000
@@ -193,13 +193,13 @@ tcp_output(tp)
 	else
 		tp->t_flags &= ~TF_LASTIDLE;
 
-	if (TCP_DO_SACK(tp) && tp->snd_nxt != tp->snd_max &&
+	if (TCP_DO_SACK(tp) && (tp->snd_cwnd < tp->snd_ssthresh) &&
 	    !IN_FASTRECOVERY(tp))
 		nsacked = tcp_sack_bytes_below(&tp->scb, tp->snd_nxt);
 
 again:
 	/* Make use of SACK information when slow-starting after a RTO. */
-	if (TCP_DO_SACK(tp) && tp->snd_nxt != tp->snd_max &&
+	if (TCP_DO_SACK(tp) && (tp->snd_cwnd < tp->snd_ssthresh) &&
 	    !IN_FASTRECOVERY(tp)) {
 		tcp_seq old_snd_nxt = tp->snd_nxt;