The tale of a TCP bug

[Stefan `Sec` Zehl]

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.