Re: panic in TCP Limited Transmit after RTO

[Noritoshi Demizu <demizu@xxxxxxxxxxxxxx>]

Hi Jeffrey,

> 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.

Both DragonFlyBSD current and your second patch send data starting at
snd_nxt instead of snd_max on Limited Transmit.  Actually, they send
bursty data from snd_nxt through snd_max + 1 or 2 MSS.

My previous patch fixes the burst. It sends only one data segment
starting at snd_nxt on each Limited Transmit.  The reason why it
starts to send data starting at snd_nxt is that DragonFlyBSD does so.
I just followed its behavior.

If Limited Transmit requires to send new data starting at snd_max,
(I just re-read RFC3042, and it says so), the attached patch
does it.  It simply tweaks snd_nxt during calling tcp_output().

I made graphs of the behaviors of your second patch and my second
patch at http://www.dd.iij4u.or.jp/~demizu/dragonfly/0315/

The first graph shows the behavior of your second patch.
(The red line represents seq, while the green line represents ack)
After an RTO, on limited transmit, it sends bursty data, then enters
recovery phase.  In recovery mode, it sends both new data and lost data.

The second graph shows the behavior of my second (attached) patch.
On each limited transmit, it just sends one data segment starting
at snd_max.

> 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.

If you mean the change by tcp_output.c rev 1.25, I think the amount of
data it sends does not exceed cwnd.  Before the change, the amount of
data it sends may be less than cwnd in some cases.  By the change,
it sends just the amount of data that cwnd allows.

> 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.

Now I agree that sending from snd_nxt is not a good idea.
So, I would like to suggest the attached patch.

Regards,
Noritoshi Demizu

Index: sys/netinet/tcp_input.c
===================================================================
RCS file: /home/src/os/DragonFlyBSD-cvsup/dcvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.54
diff -u -r1.54 tcp_input.c
--- sys/netinet/tcp_input.c	9 Mar 2005 06:57:29 -0000	1.54
+++ sys/netinet/tcp_input.c	15 Mar 2005 03:24:04 -0000
@@ -1955,6 +1955,7 @@
 			} else if (tcp_do_limitedtransmit) {
 				u_long oldcwnd = tp->snd_cwnd;
 				tcp_seq oldsndmax = tp->snd_max;
+				tcp_seq oldsndnxt = tp->snd_nxt;
 				/* outstanding data */
 				uint32_t ownd = tp->snd_max - tp->snd_una;
 				u_int sent;
@@ -1966,11 +1967,14 @@
 				    ("dupacks not 1 or 2"));
 				if (tp->t_dupacks == 1)
 					tp->snd_limited = 0;
+				tp->snd_nxt = tp->snd_max;
 				tp->snd_cwnd = ownd +
 				    (tp->t_dupacks - tp->snd_limited) *
 				    tp->t_maxseg;
 				tcp_output(tp);
 				tp->snd_cwnd = oldcwnd;
+				if (SEQ_LT(oldsndnxt, oldsndmax))
+					tp->snd_nxt = oldsndnxt;
 				sent = tp->snd_max - oldsndmax;
 				if (sent > tp->t_maxseg) {
 					KASSERT((tp->t_dupacks == 2 &&