netdev - Re: [net-next,v13,2/4] net: dsa: add tag format for MxL862xx switches

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260206022134.3185316-1-kuba@kernel.org>
Date: Thu,  5 Feb 2026 18:21:34 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: daniel@...rotopia.org
Cc: Jakub Kicinski <kuba@...nel.org>,
	netdev@...r.kernel.org,
	andrew@...n.ch,
	davem@...emloft.net,
	conor+dt@...nel.org,
	krzk+dt@...nel.org,
	chad@...roe.io,
	devicetree@...r.kernel.org,
	lxu@...linear.com,
	frankwu@....de,
	linux@...linux.org.uk,
	john@...ozen.org,
	robh@...nel.org,
	cezary.wilmanski@...ran.com,
	hkallweit1@...il.com,
	horms@...nel.org,
	edumazet@...gle.com,
	linux-kernel@...r.kernel.org,
	pabeni@...hat.com,
	olteanv@...il.com
Subject: Re: [net-next,v13,2/4] net: dsa: add tag format for MxL862xx switches

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.

> 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.

> 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