netdev - Re: [PATCH net-next v2] tcp: enable mid stream window clamp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6070816e-f7d2-725a-ec10-9d85f15455a2@gmail.com>
Date:   Tue, 24 Aug 2021 19:44:25 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Neil Spring <ntspring@...com>, davem@...emloft.net,
        edumazet@...gle.com
Cc:     yoshfuji@...ux-ipv6.org, dsahern@...nel.org, kuba@...nel.org,
        netdev@...r.kernel.org, ncardwell@...gle.com, ycheng@...gle.com
Subject: Re: [PATCH net-next v2] tcp: enable mid stream window clamp



On 8/19/21 12:54 PM, Neil Spring wrote:
> The TCP_WINDOW_CLAMP socket option is defined in tcp(7) to "Bound the size of
> the advertised window to this value."  Window clamping is distributed across two
> variables, window_clamp ("Maximal window to advertise" in tcp.h) and rcv_ssthresh
> ("Current window clamp").
> 
> This patch updates the function where the window clamp is set to also reduce the current
> window clamp, rcv_sshthresh, if needed.  With this, setting the TCP_WINDOW_CLAMP option
> has the documented effect of limiting the window.
> 
> Signed-off-by: Neil Spring <ntspring@...com>
> ---
> v2: - fix email formatting
> 
> 
>  net/ipv4/tcp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index f931def6302e..2dc6212d5888 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3338,6 +3338,8 @@ int tcp_set_window_clamp(struct sock *sk, int val)
>  	} else {
>  		tp->window_clamp = val < SOCK_MIN_RCVBUF / 2 ?
>  			SOCK_MIN_RCVBUF / 2 : val;
> +		tp->rcv_ssthresh = min(tp->rcv_ssthresh,
> +				       tp->window_clamp);

This fits in a single line I think.
		tp->rcv_ssthresh = min(tp->rcv_ssthresh, tp->window_clamp);

>  	}
>  	return 0;
>  }
> 

Hi Neil

Can you provide a packetdrill test showing the what the new expected behavior is ?

It is not really clear why you need this.

Also if we are unable to increase tp->rcv_ssthresh, this means the following sequence
will not work as we would expect :

+0 setsockopt(5, IPPROTO_TCP, TCP_WINDOW_CLAMP, [10000], 4) = 0
+0 setsockopt(5, IPPROTO_TCP, TCP_WINDOW_CLAMP, [100000], 4) = 0


Thanks.