[PATCH net] tcp: consider recv buf for the initial window scale

[PATCH net] tcp: consider recv buf for the initial window scale

[Soheil Hassas Yeganeh]

From: Soheil Hassas Yeganeh <soheil@google.com>

tcp_select_initial_window() intends to advertise a window
scaling for the maximum possible window size. To do so,
it considers the maximum of net.ipv4.tcp_rmem[2] and
net.core.rmem_max as the only possible upper-bounds.
However, users with CAP_NET_ADMIN can use SO_RCVBUFFORCE
to set the socket's receive buffer size to values
larger than net.ipv4.tcp_rmem[2] and net.core.rmem_max.
Thus, SO_RCVBUFFORCE is effectively ignored by
tcp_select_initial_window().

To fix this, consider the maximum of net.ipv4.tcp_rmem[2],
net.core.rmem_max and socket's initial buffer space.

This part of the code does not have git history and as a
result this patch does not have a `Fixes:` tag.

Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Suggested-by: Neal Cardwell <ncardwell@google.com>
---
 net/ipv4/tcp_output.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index b26aa87..bdaef7f 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -236,7 +236,8 @@ void tcp_select_initial_window(int __space, __u32 mss,
 		/* Set window scaling on max possible window
 		 * See RFC1323 for an explanation of the limit to 14
 		 */
-		space = max_t(u32, sysctl_tcp_rmem[2], sysctl_rmem_max);
+		space = max_t(u32, space, sysctl_tcp_rmem[2]);
+		space = max_t(u32, space, sysctl_rmem_max);
 		space = min_t(u32, space, *window_clamp);
 		while (space > 65535 && (*rcv_wscale) < 14) {
 			space >>= 1;
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH net] tcp: consider recv buf for the initial window scale

[Neal Cardwell]

On Thu, Jul 28, 2016 at 11:11 PM, Soheil Hassas Yeganeh
<soheil.kdev@gmail.com> wrote:
>
> From: Soheil Hassas Yeganeh <soheil@google.com>
>
> tcp_select_initial_window() intends to advertise a window
> scaling for the maximum possible window size. To do so,
> it considers the maximum of net.ipv4.tcp_rmem[2] and
> net.core.rmem_max as the only possible upper-bounds.
> However, users with CAP_NET_ADMIN can use SO_RCVBUFFORCE
> to set the socket's receive buffer size to values
> larger than net.ipv4.tcp_rmem[2] and net.core.rmem_max.
> Thus, SO_RCVBUFFORCE is effectively ignored by
> tcp_select_initial_window().
>
> To fix this, consider the maximum of net.ipv4.tcp_rmem[2],
> net.core.rmem_max and socket's initial buffer space.
>
> This part of the code does not have git history and as a
> result this patch does not have a `Fixes:` tag.
>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
> Suggested-by: Neal Cardwell <ncardwell@google.com>

I think it makes sense to tag this commit with:

Fixes: b0573dea1fb3 ("[NET]: Introduce SO_{SND,RCV}BUFFORCE socket options")

... because that's the moment at which this line of code started
failing to achieve its stated objective of setting the window scaling
factor based on the max possible window.

And having a Fixes tag would help maintainers figure out that the
patch makes sense to apply to kernels after that commit.

thanks,
neal

Re: [PATCH net] tcp: consider recv buf for the initial window scale

[Soheil Hassas Yeganeh]

On Fri, Jul 29, 2016 at 9:21 AM, Neal Cardwell <ncardwell@google.com> wrote:
>
> On Thu, Jul 28, 2016 at 11:11 PM, Soheil Hassas Yeganeh
> <soheil.kdev@gmail.com> wrote:
> >
> > From: Soheil Hassas Yeganeh <soheil@google.com>
> >
> > tcp_select_initial_window() intends to advertise a window
> > scaling for the maximum possible window size. To do so,
> > it considers the maximum of net.ipv4.tcp_rmem[2] and
> > net.core.rmem_max as the only possible upper-bounds.
> > However, users with CAP_NET_ADMIN can use SO_RCVBUFFORCE
> > to set the socket's receive buffer size to values
> > larger than net.ipv4.tcp_rmem[2] and net.core.rmem_max.
> > Thus, SO_RCVBUFFORCE is effectively ignored by
> > tcp_select_initial_window().
> >
> > To fix this, consider the maximum of net.ipv4.tcp_rmem[2],
> > net.core.rmem_max and socket's initial buffer space.
> >
> > This part of the code does not have git history and as a
> > result this patch does not have a `Fixes:` tag.
> >
> > Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
> > Suggested-by: Neal Cardwell <ncardwell@google.com>
>
> I think it makes sense to tag this commit with:
>
> Fixes: b0573dea1fb3 ("[NET]: Introduce SO_{SND,RCV}BUFFORCE socket options")

Thanks for noting the SHA1. I'll send a v2.

Thanks,
Soheil

> ... because that's the moment at which this line of code started
> failing to achieve its stated objective of setting the window scaling
> factor based on the max possible window.
>
> And having a Fixes tag would help maintainers figure out that the
> patch makes sense to apply to kernels after that commit.
>
> thanks,
> neal