On Wed, Jan 18, 2017 at 1:16 PM, Eric Dumazet <eduma...@google.com> wrote: > > On Wed, Jan 18, 2017 at 10:13 AM, Yuchung Cheng <ych...@google.com> wrote: > > On Wed, Jan 18, 2017 at 9:35 AM, Eric Dumazet <eduma...@google.com> wrote: > >> > >> On Wed, Jan 18, 2017 at 9:32 AM, Alexey Kodanev > >> <alexey.koda...@oracle.com> wrote: > >> > Hi Eric, > >> > > >> > On 01/13/2017 08:07 PM, Alexey Kodanev wrote: > >> > > >> > >> > Looks like max_window not correctly initialized for tfo sockets. > >> > On my test machine it has set to '5592320' in > >> > tcp_fastopen_create_child(). > >> > > >> > This diff fixes the issue, the question: is this the right place to do > >> > it? > >> > > >> > diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c > >> > index 4e777a3..33ed508 100644 > >> > --- a/net/ipv4/tcp_fastopen.c > >> > +++ b/net/ipv4/tcp_fastopen.c > >> > @@ -206,6 +206,8 @@ static struct sock *tcp_fastopen_create_child(struct > >> > sock *sk, > >> > */ > >> > tp->snd_wnd = ntohs(tcp_hdr(skb)->window); > >> > > >> > + tp->max_window = tp->snd_wnd; > >> > + > >> > >> Excellent catch. Let me test our regression tests with this. > > Indeed nice catch. Thanks for the investigative work! > > > > We do have 2 failures, but tests might have depended on undocumented behavior > > (For googlers : > Ran 211 tests: 209 passing, 0 flaky 2 failing > Sponge: http://sponge/f1575065-6e1c-4514-bced-9167ce56d2ee > )
Yes, exactly, those test failures look fine. Looks like the test was implicitly expecting the old buggy behavior (and it wasn't noticed because the 10*MSS output matches IW10, so it looked fine. But now with the fix, I believe we are seeing the correct new behavior because tcp_xmit_size_goal() is calling tcp_bound_to_half_wnd(), and now with max_window fixed, the outgoing TSO skb is now the correct 5*MSS (half of the rwin of 10000). So the test results look good to me. Thanks, Alexey, for tracking this down! :-) neal
