Re: One question about __tcp_select_window()
Thanks for your reply, Eric. Actually, this is a query about the code while I am reading code. >From my instinct and the comment, I think we should choose the bigger one but maybe I miss something(like your said, autotuning) Anyway, I will read more codes and do more tests. Thanks. On Tue, Apr 17, 2018 at 10:43 PM, Eric Dumazet wrote: > > > On 04/17/2018 06:53 AM, Wang Jian wrote: >> 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. >> > > Unfortunately I have no idea what exact problem you had to solve. > > Setting small windows is not exactly the path we are taking. > > And I do not know how many side effects your change will have for 'standard' > flows > using autotuning or sane windows. >
Re: One question about __tcp_select_window()
On 04/17/2018 06:53 AM, Wang Jian wrote: > 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. > Unfortunately I have no idea what exact problem you had to solve. Setting small windows is not exactly the path we are taking. And I do not know how many side effects your change will have for 'standard' flows using autotuning or sane windows.
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 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.
One question about __tcp_select_window()
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.
