[PATCH net] net: dsa: clear the conduit RX queue on user port receive

[PATCH net] net: dsa: clear the conduit RX queue on user port receive

[John Macias]

A multi-queue conduit records its RX queue in skb->queue_mapping with
skb_record_rx_queue(), as stmmac, mtk_eth_soc, fec and dpaa2 do.
dsa_switch_rcv() passes the skb to the user port with that value
unchanged, but user ports have a single RX queue. For a packet that
arrived on conduit queue 1 or above, get_rps_cpu() finds the index past
the user port's real_num_rx_queues and returns without steering. The
WARN_ONCE() there stays silent because the port has only one queue. As
a result, rps_cpus on a user port takes effect only for packets that
arrived on conduit queue 0.

Clear the queue mapping when the skb is handed to the user port. TX
queue selection is unaffected: the user port's own netdev_pick_tx() sets
queue_mapping again before the conduit transmits.

Found on an IPQ8074 board (OpenWrt, Linux 6.18, conduit with four RX
rings): with rps_cpus set on the LAN and WAN user ports,
/proc/net/softnet_stat showed no RPS for traffic hashed to conduit
rings 1-3. With this change, that traffic is steered to the configured
CPUs.

Fixes: 91da11f870f0 ("net: Distributed Switch Architecture protocol support")
Assisted-by: LLM
Signed-off-by: John Macias <dencity7@gmail.com>
---
The Fixes tag points at the commit that added the DSA receive path. The
problem shows up once RPS is used on a user port, since get_rps_cpu()
checks the recorded queue against the port's own queue count.

This was found and fixed with an AI coding assistant (Claude, from
Anthropic). It traced the missing RPS to the stale queue_mapping, wrote
the change and this changelog, and ran the tests on my hardware. I have
reviewed the change.

Testing: the same change on OpenWrt's Linux 6.18 on a Netgear RAX120 v1
(IPQ8074, out-of-tree qca_edma conduit with four RX rings, RSS over all
four). Before, received_rps stayed at 0 on every CPU over 8 runs of
bidirectional UDP with rps_cpus set on lan1 and wan. After, it rises on
the configured CPUs for traffic from rings 1-3. On this tree (net,
7076f684d) the change is build-tested only (x86_64 defconfig with
NET_DSA, W=1, WERROR). I have not tested it with a mainline conduit
driver.

 net/dsa/tag.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/dsa/tag.c b/net/dsa/tag.c
index 6a5aa7fc3..ecd4d8669 100644
--- a/net/dsa/tag.c
+++ b/net/dsa/tag.c
@@ -115,6 +115,12 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
 		skb = nskb;
 	}
 
+	/* The conduit's RX queue index means nothing on the user port, which
+	 * has a single RX queue, and get_rps_cpu() skips RPS when the index
+	 * is out of range.
+	 */
+	skb_set_queue_mapping(skb, 0);
+
 	dev_sw_netstats_rx_add(skb->dev, skb->len + ETH_HLEN);
 
 	if (dsa_skb_defer_rx_timestamp(p, skb))
-- 
2.47.3

Re: [PATCH net] net: dsa: clear the conduit RX queue on user port receive

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · 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] The commit message says "TX queue selection is unaffected: the
  user port's own netdev_pick_tx() sets queue_mapping again before the…
- [Medium] The fix is incomplete because one delivery path in
  dsa_switch_rcv() skips it.

--

net: dsa: clear the conduit RX queue on user port receive

This commit resets skb->queue_mapping to 0 in dsa_switch_rcv() before the
skb is handed to the user port. Without the reset, the RX queue index
recorded by a multi-queue conduit makes get_rps_cpu() skip RPS on the
single-RX-queue user port.

> Clear the queue mapping when the skb is handed to the user port. TX
> queue selection is unaffected: the user port's own netdev_pick_tx() sets
> queue_mapping again before the conduit transmits.

[Severity: Medium]
Does this hold for skbs that arrive on a user port and are then forwarded
by a software bridge or by routing?

skb_set_queue_mapping(skb, 0) marks the skb as having no recorded RX
queue. That is not the same as skb_record_rx_queue(skb, 0).

On the egress device, netdev_pick_tx() falls back to skb_tx_hash() when
there is no socket queue and no XPS choice. skb_tx_hash() uses the
recorded RX queue when there is one:

net/core/dev.c:skb_tx_hash() {
    ...
	if (skb_rx_queue_recorded(skb)) {
		DEBUG_NET_WARN_ON_ONCE(qcount == 0);
		hash = skb_get_rx_queue(skb);
		...
		return hash + qoffset;
	}

	return (u16) reciprocal_scale(skb_get_hash(skb), qcount) + qoffset;
}

dsa_user_create() allocates user ports with ds->num_tx_queues TX queues,
and user ports have no ndo_select_queue. So forwarded flows now pick their
egress TX queue by flow hash, not by the conduit RX ring.

Several taggers write skb_get_queue_mapping() into the switch tag:
tag_brcm, tag_ksz, tag_sja1105, tag_ocelot_8021q, tag_netc and
tag_vsc73xx_8021q. For those, the switch egress queue changes too.

Socket RX queue tracking also looks affected. With
CONFIG_SOCK_RX_QUEUE_MAPPING, __sk_rx_queue_set() only updates
sk_rx_queue_mapping when an RX queue is recorded:

include/net/sock.h:__sk_rx_queue_set() {
	if (skb_rx_queue_recorded(skb)) {
		u16 rx_queue = skb_get_rx_queue(skb);
	...
}

After this change, sockets fed by user ports never get an RX queue
recorded. That includes traffic from conduit queue 0, which used to record
rxq 0, and it changes XPS rxqs selection.

The new behaviour may well be better. Since the patch has a Fixes: tag and
will probably be backported, could the changelog describe these side
effects? Could it also say why "not recorded" was chosen over recording
queue 0?

> diff --git a/net/dsa/tag.c b/net/dsa/tag.c
> index 6a5aa7fc331c3..ecd4d86696acc 100644
> --- a/net/dsa/tag.c
> +++ b/net/dsa/tag.c
> @@ -115,6 +115,12 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
>  		skb = nskb;
>  	}
>  
> +	/* The conduit's RX queue index means nothing on the user port, which
> +	 * has a single RX queue, and get_rps_cpu() skips RPS when the index
> +	 * is out of range.
> +	 */
> +	skb_set_queue_mapping(skb, 0);

[Severity: Medium]
Is the case where skb->dev is not a DSA user port missed here? Earlier in
dsa_switch_rcv(), this branch returns before the reset is reached:

	if (unlikely(!dsa_user_dev_check(skb->dev))) {
		/* Packet is to be injected directly on an upper
		 * device, e.g. a team/bond, so skip all DSA-port
		 * specific actions.
		 */
		netif_rx(skb);
		return 0;
	}

For trunk-tagged frames, dsa_rcv_ll() in net/dsa/tag_dsa.c sets skb->dev
straight to the LAG netdev:

		lag = dsa_lag_by_id(cpu_dp->dst, source_port + 1);
		skb->dev = lag ? lag->dev : NULL;

Take a multi-queue conduit such as fec, which calls
skb_record_rx_queue(skb, rxq->bd.qid) in fec_enet_rx_queue(). The skb
reaches netif_rx() on the bond/team with the conduit's RX queue index
still set.

get_rps_cpu() then compares that index against the bond/team's
real_num_rx_queues. That value is set separately (numrxqueues, default
16) and has nothing to do with the conduit.

There are three possible outcomes:

- RPS is skipped silently, the same symptom this patch fixes for user
  ports.
- If the bond has more than one RX queue but fewer than the conduit ring
  index, the WARN_ONCE in get_rps_cpu() fires.
- Otherwise, the RPS map of whichever bond/team rx-N queue matches the
  conduit ring number gets used.

Would moving the reset to right after skb = nskb, before the
dsa_user_dev_check() branch, cover both paths?

> +
>  	dev_sw_netstats_rx_add(skb->dev, skb->len + ETH_HLEN);
>  
>  	if (dsa_skb_defer_rx_timestamp(p, skb))

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