netdev - Re: [PATCH net] tcp: remove 64 KByte limit for initial tp->rcv

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: <CANn89i+ByJ5S2y_M86fG5v2cSYsfaXH4=JL+Y0FMRNpHDSijdQ@mail.gmail.com>
Date: Wed, 22 May 2024 08:52:23 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: dsahern@...nel.org, kuba@...nel.org, pabeni@...hat.com, 
	davem@...emloft.net, ncardwell@...gle.com, netdev@...r.kernel.org, 
	Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net] tcp: remove 64 KByte limit for initial tp->rcv_wnd value

On Tue, May 21, 2024 at 3:42 PM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> From: Jason Xing <kernelxing@...cent.com>
>
> Recently, we had some servers upgraded to the latest kernel and noticed
> the indicator from the user side showed worse results than before. It is
> caused by the limitation of tp->rcv_wnd.
>
> In 2018 commit a337531b942b ("tcp: up initial rmem to 128KB and SYN rwin
> to around 64KB") limited the initial value of tp->rcv_wnd to 65535, most
> CDN teams would not benefit from this change because they cannot have a
> large window to receive a big packet, which will be slowed down especially
> in long RTT. Small rcv_wnd means slow transfer speed, to some extent. It's
> the side effect for the latency/time-sensitive users.
>
> To avoid future confusion, current change doesn't affect the initial
> receive window on the wire in a SYN or SYN+ACK packet which are set within
> 65535 bytes according to RFC 7323 also due to the limit in
> __tcp_transmit_skb():
>
>     th->window      = htons(min(tp->rcv_wnd, 65535U));
>
> In one word, __tcp_transmit_skb() already ensures that constraint is
> respected, no matter how large tp->rcv_wnd is. The change doesn't violate
> RFC.
>
> Let me provide one example if with or without the patch:
> Before:
> client   --- SYN: rwindow=65535 ---> server
> client   <--- SYN+ACK: rwindow=65535 ----  server
> client   --- ACK: rwindow=65536 ---> server
> Note: for the last ACK, the calculation is 512 << 7.
>
> After:
> client   --- SYN: rwindow=65535 ---> server
> client   <--- SYN+ACK: rwindow=65535 ----  server
> client   --- ACK: rwindow=175232 ---> server
> Note: I use the following command to make it work:
> ip route change default via [ip] dev eth0 metric 100 initrwnd 120
> For the last ACK, the calculation is 1369 << 7.
>
> When we apply such a patch, having a large rcv_wnd if the user tweak this
> knob can help transfer data more rapidly and save some rtts.
>
> Fixes: a337531b942b ("tcp: up initial rmem to 128KB and SYN rwin to around 64KB")
> Signed-off-by: Jason Xing <kernelxing@...cent.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@...gle.com>