[PATCH] tcp: allow the initial receive window to be greater than 64KiB

[PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Tian Lan]

From: Tian Lan <Tian.Lan@twosigma.com>

Commit 13d3b1ebe287 ("bpf: Support for setting initial receive window")
introduced a BPF_SOCK_OPS option which allows setting a larger value
for the initial advertised receive window up to the receive buffer space
for both active and passive TCP connections.

However, the commit a337531b942b ("tcp: up initial rmem to 128KB and SYN
rwin to around 64KB") would limit the initial receive window to be at most
64KiB which partially negates the change made previously.

With this patch, the initial receive window will be set to the 
min(64KiB, space) if there is no init_rcv_wnd provided. Else set the
initial receive window to be the min(init_rcv_wnd * mss, space).

Signed-off-by: Tian Lan <Tian.Lan@twosigma.com>
---
 net/ipv4/tcp_output.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 5079832af5c1..6fc17efbe70f 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -229,13 +229,14 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
 	 * which we interpret as a sign the remote TCP is not
 	 * misinterpreting the window field as a signed quantity.
 	 */
-	if (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);
-
-	if (init_rcv_wnd)
-		*rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
+	if (sock_net(sk)->ipv4.sysctl_tcp_workaround_signed_windows) {
+		*rcv_wnd = min(space, MAX_TCP_WINDOW);
+		if (init_rcv_wnd)
+			*rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
+	} else {
+		*rcv_wnd = (init_rcv_wnd ? init_rcv_wnd * mss : U16_MAX);
+		*rcv_wnd = min_t(u32, *rcv_wnd, space);
+	}
 
 	*rcv_wscale = 0;
 	if (wscale_ok) {
-- 
2.25.1

Re: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Eric Dumazet]

On Sat, Feb 12, 2022 at 8:06 PM Tian Lan <tilan7663@gmail.com> wrote:
>
> From: Tian Lan <Tian.Lan@twosigma.com>
>
> Commit 13d3b1ebe287 ("bpf: Support for setting initial receive window")
> introduced a BPF_SOCK_OPS option which allows setting a larger value
> for the initial advertised receive window up to the receive buffer space
> for both active and passive TCP connections.
>
> However, the commit a337531b942b ("tcp: up initial rmem to 128KB and SYN
> rwin to around 64KB") would limit the initial receive window to be at most
> 64KiB which partially negates the change made previously.
>
> With this patch, the initial receive window will be set to the
> min(64KiB, space) if there is no init_rcv_wnd provided. Else set the
> initial receive window to be the min(init_rcv_wnd * mss, space).


I do not see how pretending to have a large rcvwin is going to help
for passive connections,
given the WIN in SYN and SYNACK packet is not scaled.

So this patch I think is misleading. Get over it, TCP has not been
designed to announce more than 64KB
in the 3WHS.

The only way a sender could use your bigger window would be to violate
TCP specs and send more than 64KB in the first RTT,
assuming the receiver has in fact a RWIN bigger than 64K ????

RE: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Tian Lan]

Hey Eric,

Thanks for the comment. I agree with you that this change will not make receiver advertise a larger rwnd in the 3WHS. But I think it can help on the rwnd scaling after the first RTT (the receiver would advertise new rwnd based on a much larger rcv_ssthresh).

From our benchmark testing (mostly for data < 4MiB), it seems like that the sender was mostly waiting on the receiver to scale up the receive window. For network with high latency, this is very costly even we try to set a large initial cwnd to take advantage of full rwnd window advertised initially. I'm happy to discuss other alternatives if you think there are better options to scale up rwnd much more aggressively.

-Best
Tian
 
-----Original Message-----
From: Eric Dumazet <edumazet@google.com> 
Sent: Saturday, February 12, 2022 11:23 PM
To: Tian Lan <tilan7663@gmail.com>
Cc: netdev <netdev@vger.kernel.org>; Andrew Chester <Andrew.Chester@twosigma.com>; Tian Lan <Tian.Lan@twosigma.com>
Subject: Re: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

On Sat, Feb 12, 2022 at 8:06 PM Tian Lan <tilan7663@gmail.com> wrote:
>
> From: Tian Lan <Tian.Lan@twosigma.com>
>
> Commit 13d3b1ebe287 ("bpf: Support for setting initial receive 
> window") introduced a BPF_SOCK_OPS option which allows setting a 
> larger value for the initial advertised receive window up to the 
> receive buffer space for both active and passive TCP connections.
>
> However, the commit a337531b942b ("tcp: up initial rmem to 128KB and 
> SYN rwin to around 64KB") would limit the initial receive window to be 
> at most 64KiB which partially negates the change made previously.
>
> With this patch, the initial receive window will be set to the 
> min(64KiB, space) if there is no init_rcv_wnd provided. Else set the 
> initial receive window to be the min(init_rcv_wnd * mss, space).


I do not see how pretending to have a large rcvwin is going to help for passive connections, given the WIN in SYN and SYNACK packet is not scaled.

So this patch I think is misleading. Get over it, TCP has not been designed to announce more than 64KB in the 3WHS.

The only way a sender could use your bigger window would be to violate TCP specs and send more than 64KB in the first RTT, assuming the receiver has in fact a RWIN bigger than 64K ????

RE: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Tian Lan]

> The only way a sender could use your bigger window would be to violate TCP specs and send more than > 64KB in the first RTT, assuming the receiver has in fact a RWIN bigger than 64K ????

Just want to clarify, the intent of this patch is not trying to violate the TCP protocol. The goal is to allow the receiver advertise a much "larger" rcv_wnd once the connection is established. So instead of having the receiver grow the rcv_wnd starting with 64KiB, the receiver could advertise the new rcv_wnd based on a much larger initial window. 

Sorry for the confusion, happy to chat more if you have questions or concerns. 

-----Original Message-----
From: Eric Dumazet <edumazet@google.com> 
Sent: Saturday, February 12, 2022 11:23 PM
To: Tian Lan <tilan7663@gmail.com>
Cc: netdev <netdev@vger.kernel.org>; Andrew Chester <Andrew.Chester@twosigma.com>; Tian Lan <Tian.Lan@twosigma.com>
Subject: Re: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

On Sat, Feb 12, 2022 at 8:06 PM Tian Lan <tilan7663@gmail.com> wrote:
>
> From: Tian Lan <Tian.Lan@twosigma.com>
>
> Commit 13d3b1ebe287 ("bpf: Support for setting initial receive 
> window") introduced a BPF_SOCK_OPS option which allows setting a 
> larger value for the initial advertised receive window up to the 
> receive buffer space for both active and passive TCP connections.
>
> However, the commit a337531b942b ("tcp: up initial rmem to 128KB and 
> SYN rwin to around 64KB") would limit the initial receive window to be 
> at most 64KiB which partially negates the change made previously.
>
> With this patch, the initial receive window will be set to the 
> min(64KiB, space) if there is no init_rcv_wnd provided. Else set the 
> initial receive window to be the min(init_rcv_wnd * mss, space).


I do not see how pretending to have a large rcvwin is going to help for passive connections, given the WIN in SYN and SYNACK packet is not scaled.

So this patch I think is misleading. Get over it, TCP has not been designed to announce more than 64KB in the 3WHS.

The only way a sender could use your bigger window would be to violate TCP specs and send more than 64KB in the first RTT, assuming the receiver has in fact a RWIN bigger than 64K ????

Re: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Eric Dumazet]

On Sun, Feb 13, 2022 at 9:37 AM Tian Lan <Tian.Lan@twosigma.com> wrote:
>
> > The only way a sender could use your bigger window would be to violate TCP specs and send more than > 64KB in the first RTT, assuming the receiver has in fact a RWIN bigger than 64K ????
>
> Just want to clarify, the intent of this patch is not trying to violate the TCP protocol. The goal is to allow the receiver advertise a much "larger" rcv_wnd once the connection is established. So instead of having the receiver grow the rcv_wnd starting with 64KiB, the receiver could advertise the new rcv_wnd based on a much larger initial window.
>
> Sorry for the confusion, happy to chat more if you have questions or concerns.

To be clear, if the sender respects the initial window in first RTT ,
then first ACK it will receive allows a much bigger window (at least
2x),
 allowing for standard slow start behavior, doubling CWND at each RTT>

linux TCP stack is conservative, and wants a proof of remote peer well
behaving before opening the gates.

The thing is, we have this issue being discussed every 3 months or so,
because some people think the RWIN is never changed or something.

Last time, we asked to not change the stack, and instead suggested
users tune it using eBPF if they really need to bypass TCP standards.

https://lkml.org/lkml/2021/12/22/652


>
> -----Original Message-----
> From: Eric Dumazet <edumazet@google.com>
> Sent: Saturday, February 12, 2022 11:23 PM
> To: Tian Lan <tilan7663@gmail.com>
> Cc: netdev <netdev@vger.kernel.org>; Andrew Chester <Andrew.Chester@twosigma.com>; Tian Lan <Tian.Lan@twosigma.com>
> Subject: Re: [PATCH] tcp: allow the initial receive window to be greater than 64KiB
>
> On Sat, Feb 12, 2022 at 8:06 PM Tian Lan <tilan7663@gmail.com> wrote:
> >
> > From: Tian Lan <Tian.Lan@twosigma.com>
> >
> > Commit 13d3b1ebe287 ("bpf: Support for setting initial receive
> > window") introduced a BPF_SOCK_OPS option which allows setting a
> > larger value for the initial advertised receive window up to the
> > receive buffer space for both active and passive TCP connections.
> >
> > However, the commit a337531b942b ("tcp: up initial rmem to 128KB and
> > SYN rwin to around 64KB") would limit the initial receive window to be
> > at most 64KiB which partially negates the change made previously.
> >
> > With this patch, the initial receive window will be set to the
> > min(64KiB, space) if there is no init_rcv_wnd provided. Else set the
> > initial receive window to be the min(init_rcv_wnd * mss, space).
>
>
> I do not see how pretending to have a large rcvwin is going to help for passive connections, given the WIN in SYN and SYNACK packet is not scaled.
>
> So this patch I think is misleading. Get over it, TCP has not been designed to announce more than 64KB in the 3WHS.
>
> The only way a sender could use your bigger window would be to violate TCP specs and send more than 64KB in the first RTT, assuming the receiver has in fact a RWIN bigger than 64K ????

RE: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Tian Lan]

> To be clear, if the sender respects the initial window in first RTT , then first ACK it will receive allows a much bigger window (at least 2x),  allowing for standard slow start behavior, doubling CWND at each RTT>
>
> linux TCP stack is conservative, and wants a proof of remote peer well behaving before opening the gates.
>
> The thing is, we have this issue being discussed every 3 months or so, because some people think the RWIN is never changed or something.
>
> Last time, we asked to not change the stack, and instead suggested users tune it using eBPF if they really need to bypass TCP standards.
>
> https://lkml.org/lkml/2021/12/22/652

I totally understand that Linux wants to be conservative before opening up the gate and I'm fully support of this idea. I think the current Linux behavior is good for network with low latency, but in an environment with high RTT (i.e 20ms), the rcv_wnd really becomes the bottleneck. It took approximately 6 * RTT on average for 4MiB transfer even with large initial snd_cwnd. I think allowing a larger default rcv_wnd would greatly reduce the number of RTT required for the transfer. 

From my understanding, BPF_SOCK_OPS_RWND_INIT was added to the kernel to allow the users to by-pass the default if they choose to. Prior to kernel 4.19, the rcv_wnd set via BPF_SOCK_OPS_RWND_INIT could exceed 64KiB and up to the space. But since then, the initial rwnd would always be limited to the 64KiB. This patch would just make the kernel behave similarly to the kernel prior to 4.19 if rcv_wnd is set by eBPF. 

What would you suggest for the application that currently relies on setting a "larger" rcv_wnd via BPF_SOCK_OPS_RWND_INIT, do you think if it is a better idea if the rcv_wnd is set after the connection is established. 

Re: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Eric Dumazet]

On Sun, Feb 13, 2022 at 10:52 AM Tian Lan <Tian.Lan@twosigma.com> wrote:
>
> > To be clear, if the sender respects the initial window in first RTT , then first ACK it will receive allows a much bigger window (at least 2x),  allowing for standard slow start behavior, doubling CWND at each RTT>
> >
> > linux TCP stack is conservative, and wants a proof of remote peer well behaving before opening the gates.
> >
> > The thing is, we have this issue being discussed every 3 months or so, because some people think the RWIN is never changed or something.
> >
> > Last time, we asked to not change the stack, and instead suggested users tune it using eBPF if they really need to bypass TCP standards.
> >
> > https://lkml.org/lkml/2021/12/22/652
>
> I totally understand that Linux wants to be conservative before opening up the gate and I'm fully support of this idea. I think the current Linux behavior is good for network with low latency, but in an environment with high RTT (i.e 20ms), the rcv_wnd really becomes the bottleneck. It took approximately 6 * RTT on average for 4MiB transfer even with large initial snd_cwnd. I think allowing a larger default rcv_wnd would greatly reduce the number of RTT required for the transfer.
>
> From my understanding, BPF_SOCK_OPS_RWND_INIT was added to the kernel to allow the users to by-pass the default if they choose to. Prior to kernel 4.19, the rcv_wnd set via BPF_SOCK_OPS_RWND_INIT could exceed 64KiB and up to the space. But since then, the initial rwnd would always be limited to the 64KiB. This patch would just make the kernel behave similarly to the kernel prior to 4.19 if rcv_wnd is set by eBPF.
>
> What would you suggest for the application that currently relies on setting a "larger" rcv_wnd via BPF_SOCK_OPS_RWND_INIT, do you think if it is a better idea if the rcv_wnd is set after the connection is established.

I suggest that you do not interpret things as " BPF_SOCK_OPS_RWND_INIT
could exceed 64KiB"  because it can not.

If you really need to send more than 64KB in the first RTT, TCP is not
a proper protocol.

13d3b1ebe287 commit message should have been very clear about the 64K
limitation.

RE: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Tian Lan]

> I suggest that you do not interpret things as " BPF_SOCK_OPS_RWND_INIT could exceed 64KiB"  because it can not.

> If you really need to send more than 64KB in the first RTT, TCP is not a proper protocol.

> 13d3b1ebe287 commit message should have been very clear about the 64K limitation.

I'm not trying to make the sender to send more than 64Kib in the first RTT. The change will only make the sender to send more starting on the second RTT(after first ack received on the data). Instead of having the rcv_wnd to grow from 64Kib, the rcv_wnd can start from a much larger base value.

Without the patch:
 
RTT:                                1,                   2,	            3,  ...
rcv_wnd:                64KiB,        192KiB,         576KiB,  ...

With the patch (assume rcv_wnd is set to 512KiB):      

RTT:                                1,                    2,	            3,   ...
rcv_wnd:                64KiB,    1.536MiB,    4.608MiB,  ...

Also, it doesn't seem like the commit 13d3b1ebe287 specify anything about 64KiB limitation
https://github.com/torvalds/linux/commit/13d3b1ebe28762c79e981931a41914fae5d04386


-----Original Message-----
From: Eric Dumazet <edumazet@google.com> 
Sent: Sunday, February 13, 2022 1:58 PM
To: Tian Lan <Tian.Lan@twosigma.com>
Cc: Tian Lan <tilan7663@gmail.com>; netdev <netdev@vger.kernel.org>; Andrew Chester <Andrew.Chester@twosigma.com>
Subject: Re: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

On Sun, Feb 13, 2022 at 10:52 AM Tian Lan <Tian.Lan@twosigma.com> wrote:
>
> > To be clear, if the sender respects the initial window in first RTT 
> > , then first ACK it will receive allows a much bigger window (at 
> > least 2x),  allowing for standard slow start behavior, doubling CWND 
> > at each RTT>
> >
> > linux TCP stack is conservative, and wants a proof of remote peer well behaving before opening the gates.
> >
> > The thing is, we have this issue being discussed every 3 months or so, because some people think the RWIN is never changed or something.
> >
> > Last time, we asked to not change the stack, and instead suggested users tune it using eBPF if they really need to bypass TCP standards.
> >
> > https://lkml.org/lkml/2021/12/22/652
>
> I totally understand that Linux wants to be conservative before opening up the gate and I'm fully support of this idea. I think the current Linux behavior is good for network with low latency, but in an environment with high RTT (i.e 20ms), the rcv_wnd really becomes the bottleneck. It took approximately 6 * RTT on average for 4MiB transfer even with large initial snd_cwnd. I think allowing a larger default rcv_wnd would greatly reduce the number of RTT required for the transfer.
>
> From my understanding, BPF_SOCK_OPS_RWND_INIT was added to the kernel to allow the users to by-pass the default if they choose to. Prior to kernel 4.19, the rcv_wnd set via BPF_SOCK_OPS_RWND_INIT could exceed 64KiB and up to the space. But since then, the initial rwnd would always be limited to the 64KiB. This patch would just make the kernel behave similarly to the kernel prior to 4.19 if rcv_wnd is set by eBPF.
>
> What would you suggest for the application that currently relies on setting a "larger" rcv_wnd via BPF_SOCK_OPS_RWND_INIT, do you think if it is a better idea if the rcv_wnd is set after the connection is established.

I suggest that you do not interpret things as " BPF_SOCK_OPS_RWND_INIT could exceed 64KiB"  because it can not.

If you really need to send more than 64KB in the first RTT, TCP is not a proper protocol.

13d3b1ebe287 commit message should have been very clear about the 64K limitation.

Re: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Eric Dumazet]

On Sun, Feb 13, 2022 at 11:26 AM Tian Lan <Tian.Lan@twosigma.com> wrote:
>
> > I suggest that you do not interpret things as " BPF_SOCK_OPS_RWND_INIT could exceed 64KiB"  because it can not.
>
> > If you really need to send more than 64KB in the first RTT, TCP is not a proper protocol.
>
> > 13d3b1ebe287 commit message should have been very clear about the 64K limitation.
>
> I'm not trying to make the sender to send more than 64Kib in the first RTT. The change will only make the sender to send more starting on the second RTT(after first ack received on the data). Instead of having the rcv_wnd to grow from 64Kib, the rcv_wnd can start from a much larger base value.
>
> Without the patch:
>
> RTT:                                1,                   2,                 3,  ...
> rcv_wnd:                64KiB,        192KiB,         576KiB,  ...

This is just fine, in accordance with what we expect.

>
> With the patch (assume rcv_wnd is set to 512KiB):
>
> RTT:                                1,                    2,                3,   ...
> rcv_wnd:                64KiB,    1.536MiB,    4.608MiB,  ...

This is not needed, unless you want to blast MB of data in the second RTT.

Please get IETF approval first.

RE: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

[Tian Lan]

Thanks, will start with the IETF approval. 

-----Original Message-----
From: Eric Dumazet <edumazet@google.com> 
Sent: Sunday, February 13, 2022 2:29 PM
To: Tian Lan <Tian.Lan@twosigma.com>
Cc: Tian Lan <tilan7663@gmail.com>; netdev <netdev@vger.kernel.org>; Andrew Chester <Andrew.Chester@twosigma.com>
Subject: Re: [PATCH] tcp: allow the initial receive window to be greater than 64KiB

On Sun, Feb 13, 2022 at 11:26 AM Tian Lan <Tian.Lan@twosigma.com> wrote:
>
> > I suggest that you do not interpret things as " BPF_SOCK_OPS_RWND_INIT could exceed 64KiB"  because it can not.
>
> > If you really need to send more than 64KB in the first RTT, TCP is not a proper protocol.
>
> > 13d3b1ebe287 commit message should have been very clear about the 64K limitation.
>
> I'm not trying to make the sender to send more than 64Kib in the first RTT. The change will only make the sender to send more starting on the second RTT(after first ack received on the data). Instead of having the rcv_wnd to grow from 64Kib, the rcv_wnd can start from a much larger base value.
>
> Without the patch:
>
> RTT:                                1,                   2,                 3,  ...
> rcv_wnd:                64KiB,        192KiB,         576KiB,  ...

This is just fine, in accordance with what we expect.

>
> With the patch (assume rcv_wnd is set to 512KiB):
>
> RTT:                                1,                    2,                3,   ...
> rcv_wnd:                64KiB,    1.536MiB,    4.608MiB,  ...

This is not needed, unless you want to blast MB of data in the second RTT.

Please get IETF approval first.