From owner-freebsd-net@FreeBSD.ORG  Mon Mar 28 18:38:12 2011
Return-Path: <owner-freebsd-net@FreeBSD.ORG>
Delivered-To: freebsd-net@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 91FF71065670;
	Mon, 28 Mar 2011 18:38:12 +0000 (UTC) (envelope-from sec@42.org)
Received: from ice.42.org (v6.42.org [IPv6:2001:608:9::1])
	by mx1.freebsd.org (Postfix) with ESMTP id 4500F8FC08;
	Mon, 28 Mar 2011 18:38:12 +0000 (UTC)
Received: by ice.42.org (Postfix, from userid 1000)
	id 6FE1928419; Mon, 28 Mar 2011 20:38:10 +0200 (CEST)
Date: Mon, 28 Mar 2011 20:38:10 +0200
From: Stefan `Sec` Zehl <sec@42.org>
To: John Baldwin <jhb@freebsd.org>
Message-ID: <20110328183810.GF23803@ice.42.org>
Mail-Followup-To: John Baldwin <jhb@freebsd.org>, freebsd-net@freebsd.org,
	Doug Barton <dougb@freebsd.org>
X-Current-Backlog: 3790 messages
References: <4D8B99B4.4070404@FreeBSD.org> <20110326140212.GB45402@ice.42.org>
	<201103281021.00673.jhb@freebsd.org>
	<201103281423.52202.jhb@freebsd.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <201103281423.52202.jhb@freebsd.org>
User-Agent: Mutt/1.4.2.3i
I-love-doing-this: really
X-Modeline: vim:set ts=8 sw=4 smarttab tw=72 si noic notitle:
Accept-Languages: de, en
X-URL: http://sec.42.org/
Cc: freebsd-net@freebsd.org, Doug Barton <dougb@freebsd.org>
Subject: Re: The tale of a TCP bug
X-BeenThere: freebsd-net@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Networking and TCP/IP with FreeBSD <freebsd-net.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>,
	<mailto:freebsd-net-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-net>
List-Post: <mailto:freebsd-net@freebsd.org>
List-Help: <mailto:freebsd-net-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>,
	<mailto:freebsd-net-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Mon, 28 Mar 2011 18:38:12 -0000

Hi,

On Mon, Mar 28, 2011 at 14:23 -0400, John Baldwin wrote:
> 
> No, this is not really right.  Your patch from your blog is the best
> fix actually.  The reason we want to let 'win' be larger than
> TCP_MAXWIN is that if the remote end sends more data than we've
> advertised but we have room in the socket buffer, we want to go ahead
> and accept the data as valid and ACK it rather than dropping the data
> that is beyond rcv_adv.  My change above to rcv_wnd would break this.
> Also, for the TCPS_SYN_SENT case we don't know what 'rcv_scale' is
> until just before we update 'rcv_adv'.  This should be the same 
> as your patch:
> 
> Index: tcp_input.c
> ===================================================================
> --- tcp_input.c	(revision 220098)
> +++ tcp_input.c	(working copy)
> @@ -1756,7 +1756,8 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th,
>  				(TF_RCVD_SCALE|TF_REQ_SCALE)) {
>  				tp->rcv_scale = tp->request_r_scale;
>  			}
> -			tp->rcv_adv += tp->rcv_wnd;
> +			tp->rcv_adv += imin(tp->rcv_wnd,
> +			    TCP_MAXWIN << tp->rcv_scale);
>  			tp->snd_una++;		/* SYN is acked */
>  			/*
>  			 * If there's data, delay ACK; if there's also a FIN
> 

I've applied this to my test-VM, and as expected it now passes my two
testcases. As far as I'm concerned this fixes it for me.

I'm interested to see if my adv_neg counting hack together with this
patch still registers any hits. -- If nobody beats me to it, I'll try it
out on my webserver tomorrow.

CU,
    Sec
-- 
We may very soon have computers weighing no more than 1.5 tons.