netdev - Re: [RFC PATCH net-next] tcp: break the limitation of initial receive window

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]

Date: Fri, 17 May 2024 10:42:01 -0400
From: Neal Cardwell <ncardwell@...gle.com>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: edumazet@...gle.com, dsahern@...nel.org, kuba@...nel.org, 
	pabeni@...hat.com, davem@...emloft.net, netdev@...r.kernel.org, 
	Jason Xing <kernelxing@...cent.com>
Subject: Re: [RFC PATCH net-next] tcp: break the limitation of initial receive window

On Fri, May 17, 2024 at 4:50 AM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> From: Jason Xing <kernelxing@...cent.com>
>
> Since in 2018 one commit a337531b942b ("tcp: up initial rmem to 128KB and
> SYN rwin to around 64KB") limited received window within 65535, most CDN
> team would not benefit from this change because they cannot have a large
> window to receive a big packet one time especially in long RTT.
>
> According to RFC 7323, it says:
>   "The maximum receive window, and therefore the scale factor, is
>    determined by the maximum receive buffer space."
>
> So we can get rid of this 64k limitation and let the window be tunable if
> the user wants to do it within the control of buffer space. Then many
> companies, I believe, can have the same behaviour as old days. Besides,
> there are many papers conducting various interesting experiments which
> have something to do with this window and show good outputs in some cases.
>
> Signed-off-by: Jason Xing <kernelxing@...cent.com>
> ---
>  net/ipv4/tcp_output.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 95caf8aaa8be..95618d0e78e4 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -232,7 +232,7 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
>         if (READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_workaround_signed_windows))
>                 (*rcv_wnd) = min(space, MAX_TCP_WINDOW);
>         else
> -               (*rcv_wnd) = min_t(u32, space, U16_MAX);
> +               (*rcv_wnd) = space;

Hmm, has this patch been tested? This doesn't look like it would work.

Please note that RFC 7323 says in
https://datatracker.ietf.org/doc/html/rfc7323#section-2.2 :

   The window field in a segment where the SYN bit is set (i.e., a <SYN>
   or <SYN,ACK>) MUST NOT be scaled.

Since the receive window field in a SYN is unscaled, that means the
TCP wire protocol has no way to convey a receive window in the SYN
that is bigger than 64KBytes.

That is why this code places a limit of U16_MAX on the value here.

If you want to advertise a bigger receive window in the SYN, you'll
need to define a new TCP option type, and write an IETF Internet Draft
and/or RFC standardizing the new option.

If you would like to, instead, submit a patch with a comment
explaining that this U16_MAX limit is inherent in the RFC 7323 wire
protocol specification, that could make sense.

best regards,
neal