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

[PATCH net v3] net: dsa: clear the conduit RX queue on 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 on with that index unchanged, although
it counts the conduit's queues, not those of skb->dev. A user port has
a single RX queue. For a packet from conduit queue 1 or above,
get_rps_cpu() finds the index past the port's real_num_rx_queues and
returns without steering, and its WARN_ONCE() stays silent because the
port has only one queue. So rps_cpus on a user port only takes effect
for packets from conduit queue 0.

Trunk-tagged frames go straight to a bond or team (tag_dsa), through
the netif_rx() branch for devices that are not user ports. That device
has its own RX queue count, so the conduit's index there selects the
RPS map of an unrelated rx-N queue, skips RPS, or, when the device has
more than one queue and an index at or above that count, triggers
the WARN_ONCE().

Clear the queue mapping once the tagger has chosen skb->dev, before
that branch, so both paths are covered.

This leaves no RX queue recorded, rather than recording queue 0. Both
make get_rps_cpu() use the first RX queue's map. They differ for code
that reads the recorded queue later:

- skb_tx_hash() picks the TX queue from a recorded RX queue when no
  socket mapping or XPS map decides. A flow forwarded from a DSA port
  to a multi-queue device now gets its TX queue from the flow hash
  instead of the conduit's RX queue number. Recording queue 0 would
  instead put every such flow on the first TX queue of its traffic
  class. The selected TX queue is also an input to several taggers:
  tag_brcm encodes it directly, while tag_ksz, tag_sja1105,
  tag_ocelot_8021q, tag_netc and tag_vsc73xx_8021q derive a traffic
  class or PCP from it. A different queue within the same traffic
  class need not change that encoded class.
- With CONFIG_SOCK_RX_QUEUE_MAPPING, packets received through DSA ports
  no longer update sk_rx_queue_mapping, so XPS by RX queue (xps_rxqs)
  stops following the conduit's queue number for those sockets. A
  mapping the socket already holds is left as it is.

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 RPS only for traffic hashed to conduit
ring 0.

Claude and Codex assisted with diagnosing the queue-metadata issue,
preparing the change and its explanation, and checking source and test
results.

Fixes: 91da11f870f0 ("net: Distributed Switch Architecture protocol support")
Signed-off-by: John Macias <dencity7@gmail.com>
---
Changes in v3:
- No code changes. Resent with git send-email; the v2 posting was
  damaged in transit by the mail service (wrapped hunk header). Rebased
  onto net/main 30c71476d, where net/dsa/tag.c is unchanged.
v2: https://lore.kernel.org/all/CA+KzPspNN14ZEyxisGrOSpgfTqajJfZTXG_iHBRMYd2su1dRKQ@mail.gmail.com/

Changes in v2:
- Clear the queue mapping before the branch for devices that are not
  user ports, so trunk-tagged frames delivered to a bond or team are
  covered too (Sashiko review).
- Replace the claim that TX queue selection is unaffected. The changelog
  now describes the effect on skb_tx_hash(), taggers that consume the
  selected TX queue, and sk_rx_queue_mapping and
  xps_rxqs, and says why no queue is recorded instead of queue 0.
- Subject: "on user port receive" -> "on receive".
v1: https://lore.kernel.org/all/20261025072203.1519561-1-dencity7@gmail.com/

Claude assisted with diagnosis, the patch and runtime harness. Codex
independently reviewed the code, claims and raw results, and corrected
the test descriptions.

Testing on net/main (5d7f3375c), x86_64 under QEMU: a 4-queue virtio-net
conduit with dsa_loop, which for this test only was changed to use the
Marvell DSA tagger and to accept LAG offload (lan2 + lan3 in a
balance-xor bond with 2 RX queues). The host injects DSA-tagged UDP on
chosen conduit RX queues; rps_cpus on lan1 and on bond0 rx-0 points to
an otherwise idle CPU. 200 datagrams per queue and path. The RPS columns
say on which conduit queues the receiving socket saw the RPS CPU
(SO_INCOMING_CPU, which holds the socket's latest receive CPU, so this
is a per-socket observation rather than a per-packet one):

                       lan1 RPS     bond0 RPS    WARN_ONCE   eth1 TX
                       (q0-q3)      (q0-q3)                  per queue
  unpatched            q0 only      q0 only      yes         800/0/1/0
  v1                   all          q0 only      yes         201/100/150/350
  v2                   all          all          no          201/100/350/150
  skb_record_rx_queue  all          all          no          800/0/0/1
  (skb, 0) instead

(Queue 2's NAPI happened to run on the RPS CPU, so q2 does not tell the
cases apart; q1 and q3 do.) The last column is the eth1 TX packet delta
per queue while 16 UDP flows (800 datagrams), all from conduit RX
queue 0, were routed out a 4-queue virtio-net with XPS cleared; each
total is 801 because one other packet was also sent. The aggregate
counters are consistent with fallback TX selection favoring queue 0
when RX queue 0 is recorded, and flow-hash distribution
when none is recorded; the unclassified extra packet is not attributed
to a test flow. W=1 build of net/dsa is clean.

On the original hardware (Netgear RAX120 v1, IPQ8074, OpenWrt Linux
6.18, four-ring conduit), v1 was tested as described in v1. v2 has not
run there yet, and I have no switch with LAG offload to test the trunk
path on real hardware.

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

diff --git a/net/dsa/tag.c b/net/dsa/tag.c
index 6a5aa7fc3..e53dc3f3c 100644
--- a/net/dsa/tag.c
+++ b/net/dsa/tag.c
@@ -95,6 +95,13 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
 	skb->pkt_type = PACKET_HOST;
 	skb->protocol = eth_type_trans(skb, skb->dev);
 
+	/* The RX queue the conduit recorded is an index into the conduit's
+	 * queues. It means nothing on skb->dev, a user port with a single RX
+	 * queue or a bond/team with its own queue count, and get_rps_cpu()
+	 * would check it against that device. Leave no queue recorded.
+	 */
+	skb_set_queue_mapping(skb, 0);
+
 	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
-- 
2.47.3

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

[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] The commit message lists what changes for "code that reads the
  recorded queue later" (skb_tx_hash() and sk_rx_queue_mapping/xps_rxqs).

--

net: dsa: clear the conduit RX queue on receive

This patch clears skb->queue_mapping in dsa_switch_rcv() after the tagger
has chosen skb->dev. The conduit's RX queue index is then no longer carried
over to a DSA user port, or to a bond/team that receives trunk-tagged
frames. With that, rps_cpus on user ports applies to traffic from every
conduit queue, not just queue 0.

> diff --git a/net/dsa/tag.c b/net/dsa/tag.c
> index 6a5aa7fc331c3..e53dc3f3c477c 100644
> --- a/net/dsa/tag.c
> +++ b/net/dsa/tag.c
> @@ -95,6 +95,13 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
>  	skb->pkt_type = PACKET_HOST;
>  	skb->protocol = eth_type_trans(skb, skb->dev);
>  
> +	/* The RX queue the conduit recorded is an index into the conduit's
> +	 * queues. It means nothing on skb->dev, a user port with a single RX
> +	 * queue or a bond/team with its own queue count, and get_rps_cpu()
> +	 * would check it against that device. Leave no queue recorded.
> +	 */
> +	skb_set_queue_mapping(skb, 0);

[Severity: Medium]
The commit message lists what changes for code that reads the recorded
queue later:

  This leaves no RX queue recorded, rather than recording queue 0. Both
  make get_rps_cpu() use the first RX queue's map. They differ for code
  that reads the recorded queue later:

It then covers skb_tx_hash() and sk_rx_queue_mapping/xps_rxqs. Are there
other readers of skb->queue_mapping on the user port or bond/team that
the message should mention?

One reader that userspace can see is PACKET_FANOUT_QM, in
net/packet/af_packet.c:

static unsigned int fanout_demux_qm(struct packet_fanout *f,
				    struct sk_buff *skb,
				    unsigned int num)
{
	return skb_get_queue_mapping(skb) % num;
}

Take a QM fanout group bound to a DSA user port, or to the bond/team that
receives trunk-tagged frames. Before this patch, its packets were spread
across members by conduit RX ring, as (ring + 1) % num.

Now that queue_mapping is always 0 here, will every packet go to member 0?
Recording queue 0 instead would send them all to member 1 % num. In both
cases the group would stop spreading load.

The same applies to tc/BPF ingress programs that read
__sk_buff->queue_mapping, and to the tc ematch meta queue_mapping
selector. On DSA ports they would now always see 0, not the conduit ring
index.

This patch has a Fixes: tag and is headed for net/stable. Could the
commit message also describe these two changes: how QM fanout distributes
packets, and the queue_mapping value that tc/BPF sees on DSA ports?

[ ... ]

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