Re: [RFC v2 2/3] net: dsa: initial MT7628 tagging driver

["Joris Vaišvila" <joey@tinyisr.com>]

Hi Daniel, thanks for the feedback. 

On Sat, Mar 14, 2026 at 05:59:27PM +0000, Daniel Golle wrote:
> On Sat, Mar 14, 2026 at 05:08:44PM +0200, Joris Vaisvila wrote:
> > +	tag[0] = ETH_P_8021Q >> 8;
> > +	tag[1] = FIELD_PREP(MT7628_TAG_TX_PORT_BIT_MASK,
> > +			    dsa_xmit_port_mask(skb, dev));
> > +
> > +	tag[2] = xmit_vlan >> 8;
> > +	tag[3] = xmit_vlan & 0xff;
> 
> So there are basically two 16-bit fields which need endian swapping
> from host-byte order (little endian on MT7628) to network byte-order.
> Imho it would be better expressed by using something along the lines
> of
> 
> 	__be16 *tag;
> ...
> 	tag = dsa_etype_header_pos_tx(skb);
> 	tag[0] = htons(ETH_P_8021Q |
> 		       FIELD_PREP(MT7628_TAG_TX_PORT_BIT_MASK,
> 				  dsa_xmit_port_mask(skb, dev)));
> 	tag[1] = htons(xmit_vlan);

Will use this in later revisions, thanks for the suggestion.

> > +static struct sk_buff *mt7628_tag_rcv(struct sk_buff *skb,
> > +				      struct net_device *dev)
> > +{
> > +	int src_port;
> > +	__be16 *phdr;
> > +	u16 tpid;
> > +
> > +	if (unlikely(!pskb_may_pull(skb, MT7628_TAG_LEN)))
> > +		return NULL;
> > +
> > +	phdr = dsa_etype_header_pos_rx(skb);
> > +	tpid = ntohs(*phdr);
> > +	skb_pull_rcsum(skb, MT7628_TAG_LEN);
> > +	dsa_strip_etype_header(skb, MT7628_TAG_LEN);
> > +
> > +	src_port = tpid & MT7628_TAG_RX_PORT_MASK;
> > +
> > +	skb->dev = dsa_conduit_find_user(dev, 0, src_port);
> > +	if (!skb->dev)
> > +		return NULL;
> > +	dsa_default_offload_fwd_mark(skb);
> > +	return skb;
> 
> This looks like it is semantically identical to the RX path in tag_mtk.c.
> 
> Maybe the TX-side can be added to the existing mtk_tag.c driver, so the
> RX-side can be shared? tag_brcm.c also defines multiple similar protocols
> in the same C source file.

While the core structure is really similar, there are subtle hardware
differences in the MT7628 switch that will make the RX code diverge
from the MTK tag when VLAN offloading is implemented. To account for
that, I'd prefer to keep these drivers separate.

I plan to implement bridge and VLAN offloading after the initial switch
support lands, to keep the initial series smaller and simplify review.