Re: [PATCH net-next v23 6/9] net: dsa: tag_mtk: add Airoha variant usage of this TAG

[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 patch adds DSA_TAG_PROTO_AIROHA (37) and gives it the same
  handlers as DSA_TAG_PROTO_MTK (mtk_tag_xmit/mtk_tag_rcv, MTK_HDR_LEN…

--

net: dsa: tag_mtk: add Airoha variant usage of this TAG

This adds a DSA_TAG_PROTO_AIROHA protocol ID and an "airoha" tag driver
that reuses the MTK xmit/rcv handlers. It also adds comments on how the
AN8855 tag bitmap differs from the Mediatek one. Kconfig and the Makefile
now build tag_mtk.o through a shared NET_DSA_TAG_MTK_COMMON symbol.

> diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
> index c7dc7731675ec..f8087d768da6d 100644
> --- a/net/dsa/tag_mtk.c
> +++ b/net/dsa/tag_mtk.c

[ ... ]

> @@ -107,8 +112,33 @@ static const struct dsa_device_ops mtk_netdev_ops = {
>  	.needed_headroom = MTK_HDR_LEN,
>  };
>
> -MODULE_DESCRIPTION("DSA tag driver for Mediatek switches");
> -MODULE_LICENSE("GPL");
> +DSA_TAG_DRIVER(mtk_netdev_ops);
>  MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_MTK, MTK_NAME);
> +#endif
>
> -module_dsa_tag_driver(mtk_netdev_ops);
> +#if IS_ENABLED(CONFIG_NET_DSA_TAG_AIROHA)
> +static const struct dsa_device_ops airoha_netdev_ops = {
> +	.name		= AIROHA_NAME,
> +	.proto		= DSA_TAG_PROTO_AIROHA,
> +	.xmit		= mtk_tag_xmit,
> +	.rcv		= mtk_tag_rcv,
> +	.needed_headroom = MTK_HDR_LEN,
> +};

[Severity: Medium]
Frames on the wire are identical for DSA_TAG_PROTO_AIROHA and
DSA_TAG_PROTO_MTK. What about the conduit Ethernet drivers that check for
the MTK tag format by comparing against DSA_TAG_PROTO_MTK? None of them
are updated here.

Later in this series, an8855_get_tag_protocol() returns
DSA_TAG_PROTO_AIROHA. For an AN8855 behind a MediaTek or Airoha SoC
conduit, dsa_ptr->tag_ops->proto would then be 37, and these checks
would stop matching.

drivers/net/ethernet/mediatek/mtk_eth_soc.c:mtk_uses_dsa() {
	return netdev_uses_dsa(dev) &&
	       dev->dsa_ptr->tag_ops->proto == DSA_TAG_PROTO_MTK;
}

On NETSYS v1, mtk_open() would then take the else branch. It clears
MTK_CDMP_STAG_EN and skips the dsa_meta allocation, so hardware special
tag untagging is turned off.

drivers/net/ethernet/mediatek/mtk_ppe.c:__mtk_ppe_check_skb() {
	case htons(ETH_P_XDSA):
		if (!netdev_uses_dsa(skb->dev) ||
		    skb->dev->dsa_ptr->tag_ops->proto != DSA_TAG_PROTO_MTK)
			goto out;
}

drivers/net/ethernet/mediatek/mtk_ppe_offload.c:mtk_flow_get_dsa_port() {
	if (dp->cpu_dp->tag_ops->proto != DSA_TAG_PROTO_MTK)
		return -ENODEV;
}

These two checks mean PPE entries are never bound for DSA frames, and
flowtable hardware offload through the switch ports would be refused.

drivers/net/ethernet/airoha/airoha_eth.c:airoha_get_dsa_tag() {
	if (dev->dsa_ptr->tag_ops->proto != DSA_TAG_PROTO_MTK)
		return 0;
}

On the Airoha conduit, this returns before the VLAN TPID is restored and
before the special tag is moved into the QDMA descriptor. The comment in
that function says the PPE needs that move. airoha_dev_xmit() would then
send the frame with the inline tag and SP_TAG=0.

airoha_get_dsa_port() in airoha_ppe.c has no proto check, though, so the
Airoha PPE would still accept flow offload for these ports. Can the TX
path and the PPE path then disagree about how the tag is handled?

The commit message describes this as giving external tools like libpcap
a dedicated "Airoha" name. It doesn't mention these behaviour changes on
the conduit side.

Should these call sites accept both protocols in this series, for
example through a helper that checks for an MTK-format tag?

> +
> +DSA_TAG_DRIVER(airoha_netdev_ops);
> +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_AIROHA, AIROHA_NAME);
> +#endif

[ ... ]

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