mbox series

[net-next,0/3] tcp: better deal with delayed TX completions

Message ID 20210311203506.3450792-1-eric.dumazet@gmail.com (mailing list archive)
Headers show
Series tcp: better deal with delayed TX completions | expand

Message

Eric Dumazet March 11, 2021, 8:35 p.m. UTC
From: Eric Dumazet <edumazet@google.com>

Jakub and Neil reported an increase of RTO timers whenever
TX completions are delayed a bit more (by increasing
NIC TX coalescing parameters)

While problems have been there forever, second patch might
introduce some regressions so I prefer not backport
them to stable releases before things settle.

Many thanks to FB team for their help and tests.

Few packetdrill tests need to be changed to reflect
the improvements brought by this series.

Eric Dumazet (3):
  tcp: plug skb_still_in_host_queue() to TSQ
  tcp: consider using standard rtx logic in tcp_rcv_fastopen_synack()
  tcp: remove obsolete check in __tcp_retransmit_skb()

 include/linux/skbuff.h |  2 +-
 net/ipv4/tcp_input.c   | 10 ++++------
 net/ipv4/tcp_output.c  | 20 ++++++++------------
 3 files changed, 13 insertions(+), 19 deletions(-)

Comments

Jakub Kicinski March 12, 2021, 6:18 p.m. UTC | #1
On Thu, 11 Mar 2021 12:35:03 -0800 Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Jakub and Neil reported an increase of RTO timers whenever
> TX completions are delayed a bit more (by increasing
> NIC TX coalescing parameters)
> 
> While problems have been there forever, second patch might
> introduce some regressions so I prefer not backport
> them to stable releases before things settle.
> 
> Many thanks to FB team for their help and tests.
> 
> Few packetdrill tests need to be changed to reflect
> the improvements brought by this series.

FWIW I run some workloads with this for a day and looks good:

Tested-by: Jakub Kicinski <kuba@kernel.org>

Thank you!
Yuchung Cheng March 12, 2021, 7:04 p.m. UTC | #2
On Fri, Mar 12, 2021 at 10:18 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 11 Mar 2021 12:35:03 -0800 Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Jakub and Neil reported an increase of RTO timers whenever
> > TX completions are delayed a bit more (by increasing
> > NIC TX coalescing parameters)
> >
> > While problems have been there forever, second patch might
> > introduce some regressions so I prefer not backport
> > them to stable releases before things settle.
> >
> > Many thanks to FB team for their help and tests.
> >
> > Few packetdrill tests need to be changed to reflect
> > the improvements brought by this series.
>
> FWIW I run some workloads with this for a day and looks good:
>
> Tested-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Yuchung Cheng <ycheng@google.com>
Thank you Eric for fixing the bug.

>
> Thank you!
Neal Cardwell March 12, 2021, 7:09 p.m. UTC | #3
On Fri, Mar 12, 2021 at 2:05 PM Yuchung Cheng <ycheng@google.com> wrote:
>
> On Fri, Mar 12, 2021 at 10:18 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Thu, 11 Mar 2021 12:35:03 -0800 Eric Dumazet wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > Jakub and Neil reported an increase of RTO timers whenever
> > > TX completions are delayed a bit more (by increasing
> > > NIC TX coalescing parameters)
> > >
> > > While problems have been there forever, second patch might
> > > introduce some regressions so I prefer not backport
> > > them to stable releases before things settle.
> > >
> > > Many thanks to FB team for their help and tests.
> > >
> > > Few packetdrill tests need to be changed to reflect
> > > the improvements brought by this series.
> >
> > FWIW I run some workloads with this for a day and looks good:
> >
> > Tested-by: Jakub Kicinski <kuba@kernel.org>
> Acked-by: Yuchung Cheng <ycheng@google.com>
> Thank you Eric for fixing the bug.

The series looks good to me as well.

Re this one:
-               WARN_ON(tp->retrans_out != 0);
+               WARN_ON(tp->retrans_out != 0 && !tp->syn_data);

it seems a little unfortunate to lose the power of this WARN_ON for
the lifetime of TFO connections, but I do not have a better idea. :-)

thanks,
neal
patchwork-bot+netdevbpf@kernel.org March 12, 2021, 8:40 p.m. UTC | #4
Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Thu, 11 Mar 2021 12:35:03 -0800 you wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Jakub and Neil reported an increase of RTO timers whenever
> TX completions are delayed a bit more (by increasing
> NIC TX coalescing parameters)
> 
> While problems have been there forever, second patch might
> introduce some regressions so I prefer not backport
> them to stable releases before things settle.
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] tcp: plug skb_still_in_host_queue() to TSQ
    https://git.kernel.org/netdev/net-next/c/f4dae54e486d
  - [net-next,2/3] tcp: consider using standard rtx logic in tcp_rcv_fastopen_synack()
    https://git.kernel.org/netdev/net-next/c/a7abf3cd76e1
  - [net-next,3/3] tcp: remove obsolete check in __tcp_retransmit_skb()
    https://git.kernel.org/netdev/net-next/c/ac3959fd0dcc

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html