[PATCH] TCP acking too fast

[Mika Liljeberg <Mika.Liljeberg@welho.com>]

[-- Attachment #1: Type: text/plain, Size: 1281 bytes --]

Ok, here's the patch against 2.4.10-ac10. This seems to produce
acceptable behaviour in the cases I tested, at least. Someone with one
of those "ridiculously small MTU" links might give it a go to check that
the rcv_mss estimation still works as expected. It should, though, as I
didn't really make any changes to it.

Andi Kleen wrote:
> The only special case for PSH in RX left I can is in rcv_mss estimation,
> where is assumes that a packet with PSH set is not full sized.  On further
> look the 2.4 tcp_measure_rcv_mss will never update rcv_mss for packets
> which do have PSH set and in this case cause random ack behaviour depending
> on the initial rcv_mss guess.

A too low rcv_mss estimate isn't a problem, as the estimate is
immediately increased when the first larger segment arrives. A too high
estimate can be difficult to adjust down, though, if the sender suddenly
starts sending smalls segments with PSH set.

> Not very nice; definitely violates the "be conservative what you accept"
> rule. I'm not sure how to fix it, adding a fallback to every-two-packet-add
> would pollute the fast path a bit.

Hopefully a bit more conservative now. I didn't implement the fall back
to ack-every-two-packets, though, as I had the exact opposite problem.
:)

Regards,

	MikaL

[-- Attachment #2: over_ack.patch --]
[-- Type: text/plain, Size: 1826 bytes --]

--- tcp_input.c.org	Sat Oct 13 23:24:38 2001
+++ tcp_input.c	Sun Oct 14 15:47:10 2001
@@ -126,24 +126,25 @@
 	 * sends good full-sized frames.
 	 */
 	len = skb->len;
+
 	if (len >= tp->ack.rcv_mss) {
 		tp->ack.rcv_mss = len;
-		/* Dubious? Rather, it is final cut. 8) */
-		if (tcp_flag_word(skb->h.th)&TCP_REMNANT)
-			tp->ack.pending |= TCP_ACK_PUSHED;
 	} else {
-		/* Otherwise, we make more careful check taking into account,
-		 * that SACKs block is variable.
+		/* If PSH is not set, packet should be full sized, assuming
+		 * that the peer implements Nagle correctly.
+		 * This observation (if it is correct 8)) allows
+		 * to handle super-low mtu links fairly.
 		 *
-		 * "len" is invariant segment length, including TCP header.
+		 * However, If sender sets TCP_NODELAY, this could effectively
+		 * turn receiver side SWS algorithms off. TCP_MIN_MSS guards
+		 * against a ridiculously small rcv_mss estimate.
+		 *
+		 * We also have to be careful checking the header size, since
+		 * the SACK option is variable length. "len" is the invariant
+		 * segment length, including TCP header.
 		 */
 		len += skb->data - skb->h.raw;
 		if (len >= TCP_MIN_RCVMSS + sizeof(struct tcphdr) ||
-		    /* If PSH is not set, packet should be
-		     * full sized, provided peer TCP is not badly broken.
-		     * This observation (if it is correct 8)) allows
-		     * to handle super-low mtu links fairly.
-		     */
 		    (len >= TCP_MIN_MSS + sizeof(struct tcphdr) &&
 		     !(tcp_flag_word(skb->h.th)&TCP_REMNANT))) {
 			/* Subtract also invariant (if peer is RFC compliant),
@@ -152,12 +153,9 @@
 			 */
 			len -= tp->tcp_header_len;
 			tp->ack.last_seg_size = len;
-			if (len == lss) {
+			if (len == lss)
 				tp->ack.rcv_mss = len;
-				return;
-			}
 		}
-		tp->ack.pending |= TCP_ACK_PUSHED;
 	}
 }