[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAXyoMO9HmNGxsx6nLH0rbgS6Pgmm0XgU=W5ej977+tF032G2A@mail.gmail.com>
Date: Tue, 26 Aug 2025 10:44:47 +0800
From: Yangfl <mmyangfl@...il.com>
To: Andrew Lunn <andrew@...n.ch>
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
On Tue, Aug 26, 2025 at 10:18 AM Andrew Lunn <andrew@...n.ch> wrote:
>
> > > > +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
I don't. I'm just giving reasons why EtherType should not be
considered "fixed ABI" (or it is the reason it can be ABI if all of
you are fine with the reset default value).
Powered by blists - more mailing lists