netdev - Re: One question about __tcp_select

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:   Tue, 17 Apr 2018 21:53:05 +0800
From:   Wang Jian <jianjian.wang1@...il.com>
To:     netdev@...r.kernel.org
Subject: Re: One question about __tcp_select_window()

I test the fix with 4.17.0-rc1+ and it seems work.

1. iperf -c IP -i 20 -t 60 -w 1K
 with-fix vs without-fix : 1.15Gbits/sec vs 1.05Gbits/sec
I also try other windows and have similar results.

2. Use tcp probe trace snd_wind.
with-fix vs without-fix: 1245568 vs 1042816

3. I don't see extra retransmit/drops.

On Sun, Apr 15, 2018 at 8:50 PM, Wang Jian <jianjian.wang1@...il.com> wrote:
> Hi all,
>
> While I read __tcp_select_window() code, I find that it maybe return a
> smaller window.
> Below is one scenario I thought, may be not right:
> In function __tcp_select_window(), assume:
> full_space is 6mss, free_space is 2mss, tp->rcv_wnd is 3MSS.
> And assume disable window scaling, then
> window = tp->rcv_wnd > free_space && window > free_space
> then it will round down free_space and return it.
>
> Is this expected behavior? The comment is also saying
> "Get the largest window that is a nice multiple of mss."
>
> Should we do something like below ? Or I miss something?
> I don't know how to verify it now.
>
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2680,9 +2680,9 @@ u32 __tcp_select_window(struct sock *sk)
>                  * We also don't do any window rounding when the free space
>                  * is too small.
>                  */
> -               if (window <= free_space - mss || window > free_space)
> +               if (window <= free_space - mss)
>                         window = rounddown(free_space, mss);
> -               else if (mss == full_space &&
> +               else if (window <= free_space && mss == full_space &&
>                          free_space > window + (full_space >> 1))
>                         window = free_space;
>         }
>
> Thanks.