[PATCH] tcp_ack_update_window window scaling bug

[PATCH] tcp_ack_update_window window scaling bug

[Kevin Lahey]

Using ttcp to test TSO on 2.5.75, I noticed an odd problem with
receiver windows larger than 64KB.  The initial data packet after
the three-way handshake was delayed by around 200 milliseconds.

Further investigation revealed that TSO support for the connection
was being turned off at the same time by the SWS avoidance code.
The same ttcp transfer without the large receiver window showed no
delay, and didn't turn off TSO.

It appears that tcp_ack_update_window is window scaling the offered
TCP receiver window even on the SYN packet.  As noted in no less
than two other places in the networking code, RFC 1323 insists that
initial window advertisements are unscaled.

This meant that tp->max_window was too large initially, resulting
in an inappropriately large TSO segment size.  The initial TSO segment 
was delayed by the SWS code, causing the 200ms pause.  I'm not sure
whether this bug could ever be tickled by anything *but* TSO, but
I suppose in some contrived situations with non-Linux remote hosts,
it could wind up being significant.

Here's a patch that solves my performance problem:

--- linux-2.5.75/net/ipv4/tcp_input.c.orig	Mon Oct 27 19:02:55 2003
+++ linux-2.5.75/net/ipv4/tcp_input.c	Mon Oct 27 19:04:34 2003
@@ -1966,7 +1966,8 @@
 				 struct sk_buff *skb, u32 ack, u32 ack_seq)
 {
 	int flag = 0;
-	u32 nwin = ntohs(skb->h.th->window) << tp->snd_wscale;
+	u32 nwin = (skb->h.th->syn ? ntohs(skb->h.th->window) :
+			ntohs(skb->h.th->window) << tp->snd_wscale);
 
 	if (tcp_may_update_window(tp, ack, ack_seq, nwin)) {
 		flag |= FLAG_WIN_UPDATE;

Thanks,

Kevin
kml@patheticgeek.net

Re: [PATCH] tcp_ack_update_window window scaling bug

[David S. Miller]

On Tue, 28 Oct 2003 08:17:59 -0800
"Kevin Lahey" <kml@patheticgeek.net> wrote:

> It appears that tcp_ack_update_window is window scaling the offered
> TCP receiver window even on the SYN packet.  As noted in no less
> than two other places in the networking code, RFC 1323 insists that
> initial window advertisements are unscaled.

Good spotting Kevin, thanks a lot.

I changed your patch slightly so that there is only one
expansion of ntohs().  Longer term we should make all
the include/linux/byteorder/swab.h routines inline functions
instead of macros and then mark them with __attribute__((pure)).

Thanks again Kevin.

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1377  -> 1.1378 
#	net/ipv4/tcp_input.c	1.46    -> 1.47   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/10/28	kml@patheticgeek.net	1.1378
# [TCP]: When SYN is set, the window is not scaled.
# --------------------------------------------
#
diff -Nru a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
--- a/net/ipv4/tcp_input.c	Tue Oct 28 08:38:22 2003
+++ b/net/ipv4/tcp_input.c	Tue Oct 28 08:38:22 2003
@@ -1967,7 +1967,10 @@
 				 struct sk_buff *skb, u32 ack, u32 ack_seq)
 {
 	int flag = 0;
-	u32 nwin = ntohs(skb->h.th->window) << tp->snd_wscale;
+	u32 nwin = ntohs(skb->h.th->window);
+
+	if (likely(!skb->h.th->syn))
+		nwin <<= tp->snd_wscale;
 
 	if (tcp_may_update_window(tp, ack, ack_seq, nwin)) {
 		flag |= FLAG_WIN_UPDATE;