Re: delayed ACKs for retransmitted packets: ouch! (fwd)

Neal Cardwell (cardwell@cs.washington.edu)
Sat, 7 Nov 1998 14:54:53 -0800 (PST)

OK, i guess the reason that Dave stayed up all night was that they
released another version of Linux -- 2.1.127. This one has both the
ssthresh bug fix and the delack-for-retransmitted-segments fix. It's
amazing how responsive these guys are to bug fix suggestions. During the
few days it took these guys to crank out the latest kernel release, my
Windows 98 setup at home totally self-destructed without warning. No
wonder Microsoft is starting to sweat...

neal.

---------- Forwarded message ----------
Date: Sat, 7 Nov 1998 14:30:20 -0800 (PST)
From: Neal Cardwell <cardwell@cs.washington.edu>
To: David S. Miller <davem@dm.cobaltmicro.com>
Cc: Neal Cardwell <cardwell@cs.washington.edu>
Subject: Re: delayed ACKs for retransmitted packets: ouch!

Hi,

Thanks for looking at this. The 2.1.x patch seems to fix my scenario.
Here's a 2.1.126 receiver with the patch:

http://www.cs.washington.edu/research/networking/detour/misc/2.1.126.fix1.ex1.ps

About the 2.0.x patch:

> --- net/ipv4/tcp_input.c.~1~ Tue Jul 21 08:13:48 1998
> +++ net/ipv4/tcp_input.c Sat Nov 7 06:48:59 1998
> @@ -1929,8 +1929,14 @@
> * Delay the ack if possible. Send ack's to
> * fin frames immediately as there shouldn't be
> * anything more to come.
> + *
> + * ACK immediately if we still have any out of
> + * order data. This is because we desire "maximum
> + * feedback during loss". --DaveM
> */
> - if (!sk->delay_acks || th->fin) {
> + if (!sk->delay_acks || th->fin ||
> + ((sk->acked_seq == skb->end_seq) &&
> + (skb->next != (struct sk_buff *) &sk->receive_queue))) {
> tcp_send_ack(sk);
> } else {
> /*

What's the motivation for the check for (sk->acked_seq == skb->end_seq)?
This check seems to force a delayed ACK if a segment fills a hole, even
when there are more holes further up the line. For example, a 2.0.35
receiver with this patch:

http://www.cs.washington.edu/research/networking/detour/misc/2.0.35.fix1.ex1.ps

I tried removing this check:

--- tcp_input.c.orig Sat Nov 7 12:32:23 1998
+++ tcp_input.c Sat Nov 7 13:38:41 1998
@@ -1943,8 +1943,13 @@
* Delay the ack if possible. Send ack's to
* fin frames immediately as there shouldn't be
* anything more to come.
+ *
+ * ACK immediately if we still have any out of
+ * order data. This is because we desire "maximum
+ * feedback during loss". --DaveM
*/
- if (!sk->delay_acks || th->fin) {
+ if (!sk->delay_acks || th->fin ||
+ (skb->next != (struct sk_buff *) &sk->receive_queue))
{
tcp_send_ack(sk);
} else {
/*

And I'm seeing better performance (2.0.35 receiver w/ revised patch):

http://www.cs.washington.edu/research/networking/detour/misc/2.0.35.fix2.ex1.ps

Is it safe to remove that check?

cheers,
neal