[PATCH net-next v12] net: reduce ARFS flow updates by checking LLC affinity

[PATCH net-next v12] net: reduce ARFS flow updates by checking LLC affinity

[Chuang Wang]

When an application migrates to a different CPU, RFS records the new
CPU in the global rps_sock_flow_table via rps_record_sock_flow().  On
the next packet of a flow, get_rps_cpu() notices the change and calls
set_rps_cpu() to update the per-queue flow entry.  With Accelerated
RFS (ARFS), set_rps_cpu() also invokes the driver's ndo_rx_flow_steer()
callback so that the NIC steers the flow to a queue close to the new
CPU.

Threads that migrate frequently trigger this on every migration.  For
drivers like mlx5, each steering call serialises on an internal lock
(arfs_lock), and under heavy load the contention on that lock can
outweigh the benefit of the steering update itself.

This patch introduces a cache-aware update strategy: the hardware
steering update is skipped when the flow's previous target CPU and the
new one share the same Last Level Cache (LLC), as cache locality for
the application is preserved either way. The per-queue flow bookkeeping
(rflow->cpu) is still updated, so RFS keeps steering packets to the CPU
the application is running on. A new sysctl, net.core.arfs_llc_affinity,
is added to toggle this feature (default: off).

Performance Test Results:
The patch was tested in a K8s environment (AMD CPU 128*2, 16-core Pod
with CPU pinning, mlx5 NIC) using brpc[1] echo_server and rpc_press.

rpc_press Commands:

  for i in {1..8}; do
    ./rpc_press -proto=./echo.proto -method=example.EchoService.Echo
    -server=<IP>:8000 -input='{"message":"hello"}'
    -qps=0 -thread_num=512 -connection_type=pooled &
  done

Monitor mlx5e_rx_flow_steer frequency:

  /usr/share/bcc/tools/funccount -i 1 mlx5e_rx_flow_steer

Frequency of mlx5e_rx_flow_steer (via funccount[2]):

  Before: ~200,000 counts/sec
  After:       ~10 counts/sec (reduced by ~99%)

[1] https://github.com/apache/brpc/
[2] https://github.com/iovisor/bcc/blob/master/tools/funccount.py

Signed-off-by: Chuang Wang <nashuiliang@gmail.com>
---
v11 -> v12:
- drop the comment block above the LLC check, per Jakub Kicinski
- guard the arfs_llc_affinity declaration, definition and sysctl entry
  with CONFIG_RFS_ACCEL
- document the sysctl in Documentation/networking/scaling.rst, per Jakub Kicinski
v10 -> v11:
- rename the sysctl from net.core.rps_feat_llc_affinity to
  net.core.arfs_llc_affinity, per Neal Cardwell
v9 -> v10:
- move the LLC check from rps_record_sock_flow() into set_rps_cpu(),
  so that only the ARFS hardware steering is skipped while rflow->cpu
  is still updated, per Eric Dumazet
- drop the sock_rps_record_flow_hash()/sock_rps_record_flow() exports,
  which are no longer needed
v6 -> v9:
- simplify code and fix errors in AI submissions, per Simon Horman
v5 -> v6:
- remove the multi-check 'old_val == new_val', per Xuan Zhuo
- fix 'modpost: "sock_rps_record_flow_hash" [drivers/net/tun.ko]
  undefined!', per kernel test robot
- fix 'tcp.c:(.text+0x3e90): undefined reference to
  `sock_rps_record_flow'', per kernel test robot
v4 -> v5:
- fix 'modpost: "rps_llc_check" [net/sctp/sctp.ko] undefined!', per
  kernel test robot
v3 -> v4:
- add rps_llc_check, per Eric Dumazet
v2 -> v3:
- patch net -> net-next, per Jakub Kicinski
v1 -> v2:
- add rps_feat_llc_affinity; add brpc tests
 Documentation/networking/scaling.rst |  9 +++++++++
 include/net/rps.h                    |  4 ++++
 net/core/dev.c                       | 14 ++++++++++++--
 net/core/sysctl_net_core.c           |  9 +++++++++
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst
index 6c9836000a8d..d85fe7a9e9bf 100644
--- a/Documentation/networking/scaling.rst
+++ b/Documentation/networking/scaling.rst
@@ -468,6 +468,15 @@ of CPU to queues is automatically deduced from the IRQ affinities
 configured for each receive queue by the driver, so no additional
 configuration should be necessary.
 
+When the flow's previous target CPU and the new target CPU share the
+last level cache (LLC), the hardware steering update can be skipped,
+since the application's cache locality is preserved either way. This
+reduces contention on driver-internal locks when application threads
+migrate frequently between CPUs sharing an LLC. The functionality
+remains disabled until explicitly enabled through::
+
+  /proc/sys/net/core/arfs_llc_affinity
+
 
 Suggested Configuration
 ~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/include/net/rps.h b/include/net/rps.h
index e33c6a2fa8bb..ff2f490a4e9a 100644
--- a/include/net/rps.h
+++ b/include/net/rps.h
@@ -13,6 +13,10 @@
 extern struct static_key_false rps_needed;
 extern struct static_key_false rfs_needed;
 
+#ifdef CONFIG_RFS_ACCEL
+extern struct static_key_false arfs_llc_affinity;
+#endif
+
 /*
  * This structure holds an RPS map which can be of variable length.  The
  * map is an array of CPUs.
diff --git a/net/core/dev.c b/net/core/dev.c
index 3b60b15e2175..0011cec29640 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5217,6 +5217,10 @@ EXPORT_SYMBOL(rps_needed);
 struct static_key_false rfs_needed __read_mostly;
 EXPORT_SYMBOL(rfs_needed);
 
+#ifdef CONFIG_RFS_ACCEL
+struct static_key_false arfs_llc_affinity __read_mostly;
+#endif
+
 static u32 rfs_slot(u32 hash, rps_tag_ptr tag_ptr)
 {
 	return hash_32(hash, rps_tag_to_log(tag_ptr));
@@ -5254,7 +5258,7 @@ static bool rps_flow_is_active(struct rps_dev_flow *rflow,
 
 static struct rps_dev_flow *
 set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
-	    struct rps_dev_flow *rflow, u16 next_cpu, u32 hash)
+	    struct rps_dev_flow *rflow, u16 old_cpu, u16 next_cpu, u32 hash)
 {
 	if (next_cpu < nr_cpu_ids) {
 		u32 head;
@@ -5273,6 +5277,12 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
 		if (!skb_rx_queue_recorded(skb) || !dev->rx_cpu_rmap ||
 		    !(dev->features & NETIF_F_NTUPLE))
 			goto out;
+
+		if (static_branch_unlikely(&arfs_llc_affinity) &&
+		    old_cpu < nr_cpu_ids && cpu_online(old_cpu) &&
+		    cpus_share_cache(old_cpu, next_cpu))
+			goto out;
+
 		rxq_index = cpu_rmap_lookup_index(dev->rx_cpu_rmap, next_cpu);
 		if (rxq_index == skb_get_rx_queue(skb))
 			goto out;
@@ -5402,8 +5412,8 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
 		    (tcpu >= nr_cpu_ids || !cpu_online(tcpu) ||
 		     ((int)(READ_ONCE(per_cpu(softnet_data, tcpu).input_queue_head) -
 		      rflow->last_qtail)) >= 0)) {
+			rflow = set_rps_cpu(dev, skb, rflow, tcpu, next_cpu, hash);
 			tcpu = next_cpu;
-			rflow = set_rps_cpu(dev, skb, rflow, next_cpu, hash);
 		}
 
 		if (tcpu < nr_cpu_ids && cpu_online(tcpu)) {
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 9226b96f2e89..64ae4c07f1b1 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -562,6 +562,15 @@ static struct ctl_table net_core_table[] = {
 		.mode		= 0644,
 		.proc_handler	= rps_sock_flow_sysctl
 	},
+#ifdef CONFIG_RFS_ACCEL
+	{
+		.procname	= "arfs_llc_affinity",
+		.data		= &arfs_llc_affinity.key,
+		.maxlen		= sizeof(arfs_llc_affinity.key),
+		.mode		= 0644,
+		.proc_handler	= proc_do_static_key
+	},
+#endif
 #endif
 #ifdef CONFIG_NET_FLOW_LIMIT
 	{
-- 
2.52.0

Re: [PATCH net-next v12] net: reduce ARFS flow updates by checking LLC affinity

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] set_rps_cpu() (net/core/dev.c:5281-5284) decides whether to
  skip ARFS hardware steering by comparing next_cpu with old_cpu.

--

net: reduce ARFS flow updates by checking LLC affinity

This patch makes set_rps_cpu() skip the ARFS ndo_rx_flow_steer() call
when the CPU recorded for the flow and the new target CPU share an LLC.
rflow->cpu is still updated. A new sysctl, net.core.arfs_llc_affinity,
turns this on and off, and it is off by default.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 3b60b15e2175e..0011cec296402 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c

[ ... ]

> @@ -5273,6 +5277,12 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>  		if (!skb_rx_queue_recorded(skb) || !dev->rx_cpu_rmap ||
>  		    !(dev->features & NETIF_F_NTUPLE))
>  			goto out;
> +
> +		if (static_branch_unlikely(&arfs_llc_affinity) &&
> +		    old_cpu < nr_cpu_ids && cpu_online(old_cpu) &&
> +		    cpus_share_cache(old_cpu, next_cpu))
> +			goto out;
> +

[Severity: Medium]
Does rflow->cpu reliably show which LLC services the hardware queue the
flow is arriving on?

get_rps_cpu() takes old_cpu from the hash slot in the arrival queue's
table:

	tcpu = rflow->cpu;
	...
		rflow = set_rps_cpu(dev, skb, rflow, tcpu, next_cpu, hash);

That value is software bookkeeping, and other flows can share the slot.
The new check runs before the cpu_rmap_lookup_index() /
skb_get_rx_queue() comparison. So it can suppress ndo_rx_flow_steer()
even when the arrival queue's IRQ/NAPI CPU is in a different LLC from
next_cpu.

The skip path still reaches:

	WRITE_ONCE(rflow->cpu, next_cpu);

sd_llc_id equality is transitive, so every later migration inside that
LLC would skip again. Could the flow then stay on a far queue
indefinitely?

Here are some ways rflow->cpu can drift away from the real hardware
queue:

- After a successful steer from queue A to queue B, packets still in
  flight on A go through set_rps_cpu() again and hit:

		if (hash != READ_ONCE(tmp_rflow->hash) ||
		    next_cpu == tmp_cpu)
			goto out;

  This records next_cpu (LLC X) in A's slot, but A's IRQ may be in LLC
  Y. Two later cases then hit the new skip, with the application on
  another CPU in LLC X. One is a flow whose hash collides with that slot
  and which RSS delivers on A. The other is the original flow coming
  back on A after rps_may_expire_flow() let the driver drop its rule.
  No failure is needed for this to happen.

- The existing goto out paths still record next_cpu without moving the
  hardware queue. These are ndo_rx_flow_steer() returning rc < 0 (for
  example -ENOMEM, -EPERM or -EPROTONOSUPPORT from
  mlx5e_rx_flow_steer()), a target queue with no flow table, and an
  active colliding filter. Before this patch the next migration retried
  the steer. With the sysctl on, migrations within the same LLC never
  retry.

- irqbalance or an admin moves queue A's IRQ to another LLC. cpu_rmap
  gets updated, but the skip happens before cpu_rmap is consulted.

- A driver drops all of its hardware rules without touching
  rps_dev_flow, for example mlx5e_arfs_disable()->arfs_del_rules().

In each case NAPI would process the packets on a remote LLC, and they
would then go through the RPS backlog to the application's CPU with an
IPI.

This also seems to conflict with the new text in
Documentation/networking/scaling.rst, and with the similar wording in the
commit message:

  "since the application's cache locality is preserved either way"

The reported ~99% drop in mlx5e_rx_flow_steer calls cannot tell avoided
redundant updates apart from steering that is wrongly suppressed.

Would it be more accurate to compare next_cpu with the CPU servicing the
arrival queue instead of the previously recorded rflow->cpu? For example,
that could be smp_processor_id() for non-threaded NAPI, or the CPU tied
to the queue from skb_get_rx_queue(skb).

>  		rxq_index = cpu_rmap_lookup_index(dev->rx_cpu_rmap, next_cpu);
>  		if (rxq_index == skb_get_rx_queue(skb))
>  			goto out;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109030042.2956-1-nashuiliang%40gmail.com