netdev - Re: [PATCH net] tcp: tsq: relax tcp_small_queue_check() when rtx queue contains a single skb

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 Oct 2023 11:25:54 -0400
From: Neal Cardwell <ncardwell@...gle.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, eric.dumazet@...il.com, 
	Stefan Wahren <wahrenst@....net>
Subject: Re: [PATCH net] tcp: tsq: relax tcp_small_queue_check() when rtx
 queue contains a single skb

On Tue, Oct 17, 2023 at 8:45 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> In commit 75eefc6c59fd ("tcp: tsq: add a shortcut in tcp_small_queue_check()")
> we allowed to send an skb regardless of TSQ limits being hit if rtx queue
> was empty or had a single skb, in order to better fill the pipe
> when/if TX completions were slow.
>
> Then later, commit 75c119afe14f ("tcp: implement rb-tree based
> retransmit queue") accidentally removed the special case for
> one skb in rtx queue.
>
> Stefan Wahren reported a regression in single TCP flow throughput
> using a 100Mbit fec link, starting from commit 65466904b015 ("tcp: adjust
> TSO packet sizes based on min_rtt"). This last commit only made the
> regression more visible, because it locked the TCP flow on a particular
> behavior where TSQ prevented two skbs being pushed downstream,
> adding silences on the wire between each TSO packet.
>
> Many thanks to Stefan for his invaluable help !
>
> Fixes: 75c119afe14f ("tcp: implement rb-tree based retransmit queue")
> Link: https://lore.kernel.org/netdev/7f31ddc8-9971-495e-a1f6-819df542e0af@gmx.net/
> Reported-by: Stefan Wahren <wahrenst@....net>
> Tested-by: Stefan Wahren <wahrenst@....net>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Neal Cardwell <ncardwell@...gle.com>
> ---

Acked-by: Neal Cardwell <ncardwell@...gle.com>

Thank you to Eric for the nice find and fix, and thank you to Stefan
for the bug report and all the useful testing and data!

neal