Re: [PATCH 3/4] net: dsa: Add support for SoC-e SDSA tags

[Vasilij Strassheim <v.strassheim@linutronix.de>]

On Wed, 2026-07-29 at 19:22 +0200, Andrew Lunn wrote:
> > @@ -98,6 +98,12 @@ config NET_DSA_TAG_EDSA
> >  	  Say Y or M if you want to enable support for tagging frames for the
> >  	  Marvell switches which use EtherType DSA headers.
> >  
> > +config NET_DSA_TAG_SDSA
> > +	tristate "Tag driver for SoC-e switches using EtherType SDSA headers"
> > +	help
> > +	  Say Y or M if you want to enable support for tagging frames for the
> > +	  SoC-e switches.
> > +
> 
> These entries are sorted, so it probably should be between
> NET_DSA_TAG_RZN1_A5PSW and NET_DSA_TAG_LAN9303.

I will insert it as suggested. To better understand, what is the sort order?
To me, it seems like there's also a good spot between
NET_DSA_TAG_LAN9303 and NET_DSA_TAG_SJA1105.

> 
> > @@ -23,6 +23,7 @@ dsa_core-y += \
> >  obj-$(CONFIG_NET_DSA_TAG_AR9331) += tag_ar9331.o
> >  obj-$(CONFIG_NET_DSA_TAG_BRCM_COMMON) += tag_brcm.o
> >  obj-$(CONFIG_NET_DSA_TAG_DSA_COMMON) += tag_dsa.o
> > +obj-$(CONFIG_NET_DSA_TAG_SDSA) += tag_sdsa.o
> >  obj-$(CONFIG_NET_DSA_TAG_GSWIP) += tag_gswip.o
> 
> Also sorted, and this is the wrong spot.

Oh yes, I'll place it between tag_rzn1_a5psw and tag_sja1105.

> 
> > +#define SDSA_HLEN	8
> > +
> > +#define SDSA_NAME	"sdsa"
> 
> Does SDSA mean anything? Or have you taken net/dsa/tag_dsa.c, and just
> changed edsa to sdsa?

The "S" stands for Soc-e. I don't have any preferences and just took it
from the original code.

> 
> > +static struct sk_buff *sdsa_xmit(struct sk_buff *skb, struct net_device *dev)
> > +{
> > +	struct dsa_port *dp = dsa_user_to_port(dev);
> > +	u8 *sdsa_header;
> > +
> > +	if (skb_cow_head(skb, SDSA_HLEN) < 0)
> > +		return NULL;
> > +
> > +	skb_push(skb, SDSA_HLEN);
> > +	dsa_alloc_etype_header(skb, SDSA_HLEN);
> > +
> > +	/* Construct the FROM_CPU DSA tag. */
> 
> Is FROM_CPU a concept for this device? Are there other types of tag?

Yes, the documentation describes two types of tagging headers for frames from
the switch to the CPU and from the CPU to the switch. The tags differ in some
bits.

> 
> > +	sdsa_header = dsa_etype_header_pos_tx(skb);
> > +	sdsa_header[0] = (ETH_P_SDSA >> 8) & 0xff;
> > +	sdsa_header[1] = ETH_P_SDSA & 0xff;
> > +	sdsa_header[2] = 0x00; /* reserved */
> > +	sdsa_header[3] = 0x00; /* reserved */
> > +	sdsa_header[4] = FIELD_PREP(SDSA_TAG_FRAME_TYPE_MASK, 1) |
> > +			 FIELD_PREP(SDSA_TAG_PORT_HI_MASK, dp->index >> 5);
> > +	sdsa_header[5] = FIELD_PREP(SDSA_TAG_PORT_MASK, dp->index);
> > +	sdsa_header[6] = 0x00; /* VLAN not supported */
> > +	sdsa_header[7] = 0x00; /* VLAN not supported */
> > +
> > +	return skb;
> > +}
> > +
> > +static struct sk_buff *sdsa_rcv(struct sk_buff *skb, struct net_device *dev)
> > +{
> > +	u8 *sdsa_header;
> > +	int source_port;
> > +	u8 frame_type;
> > +
> > +	if (unlikely(!pskb_may_pull(skb, SDSA_HLEN)))
> > +		return NULL;
> > +
> > +	sdsa_header = dsa_etype_header_pos_rx(skb);
> > +
> > +	/* Check that the frame type is TO_CPU. */
> > +	frame_type = FIELD_GET(SDSA_TAG_FRAME_TYPE_MASK, sdsa_header[4]);
> > +	if (frame_type != 0)
> 
> #define for TO_CPU?

I will add definitions for both TO_CPU and FROM_CPU.

> 
> 	Andrew

Thanks,
Vasilij