linux-kernel - Re: [PATCH net-next v6 2/3] net: dsa: tag_yt921x: add support for Motorcomm YT921x tags

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: <dd494b15-8173-4b17-a631-f19e9dddf9b1@lunn.ch>
Date: Tue, 26 Aug 2025 04:18:48 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Yangfl <mmyangfl@...il.com>
Cc: Vladimir Oltean <olteanv@...il.com>, netdev@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Simon Horman <horms@...nel.org>,
	Russell King <linux@...linux.org.uk>, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v6 2/3] net: dsa: tag_yt921x: add support for
 Motorcomm YT921x tags

> > > +static struct sk_buff *
> > > +yt921x_tag_xmit(struct sk_buff *skb, struct net_device *netdev)
> > > +{
> > > +     struct dsa_port *dp = dsa_user_to_port(netdev);
> > > +     unsigned int port = dp->index;
> > > +     __be16 *tag;
> > > +     u16 tx;
> > > +
> > > +     skb_push(skb, YT921X_TAG_LEN);
> > > +     dsa_alloc_etype_header(skb, YT921X_TAG_LEN);
> > > +
> > > +     tag = dsa_etype_header_pos_tx(skb);
> > > +
> > > +     /* We might use yt921x_priv::tag_eth_p, but
> > > +      * 1. CPU_TAG_TPID could be configured anyway;
> > > +      * 2. Are you using the right chip?
> >
> > The tag format sort of becomes fixed ABI as soon as user space is able
> > to run "cat /sys/class/net/eth0/dsa/tagging", see "yt921x", and record
> > it to a pcap file. Unless the EtherType bears some other meaning rather
> > than being a fixed value, then if you change it later to some other
> > value than 0x9988, you'd better also change the protocol name to
> > distinguish it from "yt921x".
> >
> 
> "EtherType" here does not necessarily become EtherType; better to
> think it is a key to enable port control over the switch. It could be
> a dynamic random value as long as everyone gets the same value all
> over the kernel, see the setup process of the switch driver. Ideally
> only the remaining content of the tag should become the ABI (and is
> actually enforced by the switch), but making a dynamic "EtherType" is
> clearly a worse idea so I don't know how to clarify the fact...

If i remember correctly, the Marvell switches allow you to set the
EtherType they use. We just use the reset default value. It has been
like this since somewhere around 2008, and nobody has needed another
value.

What use case do you have for using a different value?

	Andrew