netdev - Re: [PATCH v1 net-next 4/4] net: allow busy connected flows to switch tx queues

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]

Message-ID: <CAAVpQUCn=Tco5VnkRsJ=3qODdNib5=qYbKA=G-ycG7UR-LOmiA@mail.gmail.com>
Date: Tue, 14 Oct 2025 20:54:53 -0700
From: Kuniyuki Iwashima <kuniyu@...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>, Neal Cardwell <ncardwell@...gle.com>, 
	Simon Horman <horms@...nel.org>, Willem de Bruijn <willemb@...gle.com>, netdev@...r.kernel.org, 
	eric.dumazet@...il.com
Subject: Re: [PATCH v1 net-next 4/4] net: allow busy connected flows to switch
 tx queues

On Mon, Oct 13, 2025 at 8:22 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> This is a followup of commit 726e9e8b94b9 ("tcp: refine
> skb->ooo_okay setting") and of prior commit in this series
> ("net: control skb->ooo_okay from skb_set_owner_w()")
>
> skb->ooo_okay might never be set for bulk flows that always
> have at least one skb in a qdisc queue of NIC queue,
> especially if TX completion is delayed because of a stressed cpu.
>
> The so-called "strange attractors" has caused many performance
> issues (see for instance 9b462d02d6dd ("tcp: TCP Small Queues
> and strange attractors")), we need to do better.
>
> We have tried very hard to avoid reorders because TCP was
> not dealing with them nicely a decade ago.
>
> Use the new net.core.txq_reselection_ms sysctl to let
> flows follow XPS and select a more efficient queue.
>
> After this patch, we no longer have to make sure threads
> are pinned to cpus, they now can be migrated without
> adding too much spinlock/qdisc/TX completion pressure anymore.
>
> TX completion part was problematic, because it added false sharing
> on various socket fields, but also added false sharing and spinlock
> contention in mm layers. Calling skb_orphan() from ndo_start_xmit()
> is not an option unfortunately.
>
> Note for later:
>
> 1) move sk->sk_tx_queue_mapping closer
> to sk_tx_queue_mapping_jiffies for better cache locality.
>
> 2) Study if 9b462d02d6dd ("tcp: TCP Small Queues
> and strange attractors") could be revised.
>
> Tested:
>
> Used a host with 32 TX queues, shared by groups of 8 cores.
> XPS setup :
>
> echo ff >/sys/class/net/eth1/queue/tx-0/xps_cpus
> echo ff00 >/sys/class/net/eth1/queue/tx-1/xps_cpus
> echo ff0000 >/sys/class/net/eth1/queue/tx-2/xps_cpus
> echo ff000000 >/sys/class/net/eth1/queue/tx-3/xps_cpus
> echo ff,00000000 >/sys/class/net/eth1/queue/tx-4/xps_cpus
> echo ff00,00000000 >/sys/class/net/eth1/queue/tx-5/xps_cpus
> echo ff0000,00000000 >/sys/class/net/eth1/queue/tx-6/xps_cpus
> echo ff000000,00000000 >/sys/class/net/eth1/queue/tx-7/xps_cpus
> ...
>
> Launched a tcp_stream with 15 threads and 1000 flows, initially affined to core 0-15
>
> taskset -c 0-15 tcp_stream -T15 -F1000 -l1000 -c -H target_host
>
> Checked that only queues 0 and 1 are used as instructed by XPS :
> tc -s qdisc show dev eth1|grep backlog|grep -v "backlog 0b 0p"
>  backlog 123489410b 1890p
>  backlog 69809026b 1064p
>  backlog 52401054b 805p
>
> Then force each thread to run on cpu 1,9,17,25,33,41,49,57,65,73,81,89,97,105,113,121
>
> C=1;PID=`pidof tcp_stream`;for P in `ls /proc/$PID/task`; do taskset -pc $C $P; C=$(($C + 8));done
>
> Set txq_reselection_ms to 1000
> echo 1000 > /proc/sys/net/core/txq_reselection_ms
>
> Check that the flows have migrated nicely:
>
> tc -s qdisc show dev eth1|grep backlog|grep -v "backlog 0b 0p"
>  backlog 130508314b 1916p
>  backlog 8584380b 126p
>  backlog 8584380b 126p
>  backlog 8379990b 123p
>  backlog 8584380b 126p
>  backlog 8487484b 125p
>  backlog 8584380b 126p
>  backlog 8448120b 124p
>  backlog 8584380b 126p
>  backlog 8720640b 128p
>  backlog 8856900b 130p
>  backlog 8584380b 126p
>  backlog 8652510b 127p
>  backlog 8448120b 124p
>  backlog 8516250b 125p
>  backlog 7834950b 115p
>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>

Interesting change and changelog!  thanks!

Reviewed-by: Kuniyuki Iwashima <kuniyu@...gle.com>