Re: [net-next,v13,2/4] net: dsa: add tag format for MxL862xx switches
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-02-06 02:21:36
Also in:
linux-devicetree, lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: dsa: add tag format for MxL862xx switches This patch adds a DSA tag driver for MaxLinear MxL862xx switches. The tag format uses an 8-byte header with an ethertype marker that is shared with GSW1xx switches, though the format itself differs.
quoted hunk ↗ jump to hunk
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig --- a/net/dsa/Kconfig +++ b/net/dsa/Kconfig@@ -104,6 +104,13 @@
[ ... ]
+config NET_DSA_TAG_MXL_862XX + tristate "Tag driver for MxL862xx switches" + help + Say Y or M if you want to enable support for tagging frames for the + Maxlinear MxL86252 and MxL86282 switches using their native 8-byte
^^^^^^^^^ This isn't a bug, but there's inconsistent capitalization here. The help text uses "Maxlinear" while the adjacent NET_DSA_TAG_MXL_GSW1XX entry uses "MaxLinear" (with capital L). The MODULE_DESCRIPTION in tag_mxl862xx.c also uses "MaxLinear".
+ tagging protocol.
quoted hunk ↗ jump to hunk
diff --git a/net/dsa/tag_mxl862xx.c b/net/dsa/tag_mxl862xx.c --- /dev/null +++ b/net/dsa/tag_mxl862xx.c@@ -0,0 +1,110 @@
[ ... ]
+static struct sk_buff *mxl862_tag_xmit(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ struct dsa_port *dp = dsa_user_to_port(dev);
+ struct dsa_port *cpu_dp = dp->cpu_dp;
+ unsigned int cpu_port, sub_interface;
+ __be16 *mxl862_tag;
+
+ cpu_port = cpu_dp->index;
+
+ /* target port sub-interface ID relative to the CPU port */
+ sub_interface = dp->index + 16 - cpu_port;^^^^^^^^^^^^^^^^^^^^^^^^ Can sub_interface overflow the 5-bit MXL862_SUBIF_ID field here? With dp->index = 16 and cpu_port = 0, the calculation would yield 32 which exceeds the valid range (0-31) for GENMASK(4, 0). FIELD_PREP would silently truncate this to 0 via masking. Is there a constraint on port configurations that prevents this from occurring?
+ + /* provide additional space 'MXL862_HEADER_LEN' bytes */ + skb_push(skb, MXL862_HEADER_LEN);
[ ... ]
+ dsa_alloc_etype_header(skb, MXL862_HEADER_LEN); + + /* special tag ingress */
^^^^^^^ This comment says "ingress" but mxl862_tag_xmit() is the TX/egress path. Should this say "egress" instead?
+ mxl862_tag = dsa_etype_header_pos_tx(skb);
-- pw-bot: cr