Re: [PATCH net-next v6 1/9] net: dsa: add tag driver for LAN9645X

[Jonas Gorski]

Hi,

On 27/05/2026 16:49, Jens Emil Schulz Ãstergaard wrote:
> Add tag driver for LAN9645x using a front port as CPU port. This mode
> is called an NPI port in the datasheet.
> Use long prefix on extraction (RX) and no prefix on injection (TX). A
> long prefix on extraction helps get through the conduit port on host
> side, since it will see a broadcast MAC.
> 
> The LAN9645x chip is in the same design architecture family as ocelot
> and lan966x. The tagging protocol has the same structure as these chips,
> but the particular fields are different or have different sizes.
> Therefore, this tag driver is similar to tag_ocelot.c, but the
> differences in fields makes it hard to reuse.
> 
> LAN9645x supports 3 different tag formats for extraction/injection of
> frames from a CPU port: long prefix, short prefix and no prefix.
> 
> The tag is prepended to the frame. The critical data for the chip is
> contained in an internal frame header (IFH) which is 28 bytes. The
> prefix formats look like this:
> 
> Long prefix (16 bytes) + IFH:
> - DMAC    = 0xffffffffffff on extraction.
> - SMAC    = 0xfeffffffffff on extraction.
> - ETYPE   = 0x8880
> - payload = 0x0011
> - IFH
> 
> Short prefix (4 bytes) + IFH:
> - 0x8880
> - 0x0011
> - IFH
> 
> No prefix:
> - IFH
> 
> The format can be configured asymmetrically on RX and TX.
> 
> The IFH get/set functions are declared as inline. All the field
> constants are compile-time known, so when these calls are inlined
> efficient code is generated with branches pruned and loops unrolled.
> During testing it was observed that without explicit inlining GCC would
> have trouble inlining the functions, which hurt performance.
> 
> Reviewed-by: Steen Hegelund <Steen.Hegelund@xxxxxxxxxxxxx>
> Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@xxxxxxxxxxxxx>
 

(snip)

> +static struct sk_buff *lan9645x_rcv(struct sk_buff *skb,
> +				    struct net_device *ndev)
> +{
> +	u32 src_port, qos_class, vlan_tci, tag_type, popcnt, etype_ofs;
> +	struct dsa_port *dp;
> +	u32 ifh_gap_len = 0;
> +	u16 vlan_tpid;
> +	u8 *ifh;
> +
> +	/* DSA master already consumed DMAC,SMAC,ETYPE from long prefix. Go back
> +	 * to beginning of frame.
> +	 */
> +	skb_push(skb, ETH_HLEN);
> +
> +	if (unlikely(!pskb_may_pull(skb, LAN9645X_TOTAL_TAG_LEN)))
> +		return NULL;
> +
> +	/* IFH starts after our long prefix */
> +	ifh = skb_pull(skb, LAN9645X_LONG_PREFIX_LEN);
> +
> +	popcnt = lan9645x_ifh_get(ifh, IFH_POP_CNT, IFH_POP_CNT_SZ);
> +	etype_ofs = lan9645x_ifh_get(ifh, IFH_ETYPE_OFS, IFH_ETYPE_OFS_SZ);
> +	src_port = lan9645x_ifh_get(ifh, IFH_SRCPORT, IFH_SRCPORT_SZ);
> +	tag_type = lan9645x_ifh_get(ifh, IFH_TAG_TYPE, IFH_TAG_TYPE_SZ);
> +	vlan_tci = lan9645x_ifh_get(ifh, IFH_TCI, IFH_TCI_SZ);
> +	qos_class = lan9645x_ifh_get(ifh, IFH_QOS_CLASS, IFH_QOS_CLASS_SZ);
> +
> +	/* Set skb->data at start of real header
> +	 *
> +	 * Since REW_PORT_NO_REWRITE=0 is required on the NPI port, we need to
> +	 * account for any tags popped by the hardware, as that will leave a gap
> +	 * between the IFH and DMAC.
> +	 */
> +	if (popcnt == 0 && etype_ofs == 0)
> +		ifh_gap_len = 2 * VLAN_HLEN;
> +	else if (popcnt == 3)
> +		ifh_gap_len = VLAN_HLEN;
> +
> +	skb_pull(skb, LAN9645X_IFH_LEN);
> +
> +	if (unlikely(!pskb_may_pull(skb, ifh_gap_len + ETH_HLEN)))
> +		return NULL;
> +
> +	skb_pull(skb, ifh_gap_len);
> +	skb_reset_mac_header(skb);
> +	skb_set_network_header(skb, ETH_HLEN);
> +	skb_reset_mac_len(skb);
> +
> +	/* Reset skb->data past the actual ethernet header. */
> +	skb_pull(skb, ETH_HLEN);
> +
> +	/* We must deliver the skb so skb->csum only covers the data beyond the
> +	 * real ethernet header. The fake ethernet header in the prefix is
> +	 * not part of skb->csum already. We must subtract what remains of the
> +	 * prefix, the ifh and the gap.
> +	 */
> +	skb_postpull_rcsum(skb,
> +			   skb->data - LAN9645X_TOTAL_TAG_LEN - ifh_gap_len,
> +			   LAN9645X_TOTAL_TAG_LEN + ifh_gap_len);
> +
> +	skb->dev = dsa_conduit_find_user(ndev, 0, src_port);
> +	if (WARN_ON_ONCE(!skb->dev)) {
> +		/* This should never happen since we have disabled reflection
> +		 * back to CPU_PORT.
> +		 */
> +		return NULL;
> +	}
> +
> +	dsa_default_offload_fwd_mark(skb);

Does the switch (by default) also flood link local traffic (e.g. STP, LACP,
etc)? If not, you should not mark these as fwd offloaded so that the kernel
knows that it needs to do that in software if requested.

Is there is a bit in the header that says whether a packet was flooded or
trapped to CPU that you can check?

Best regards,
Jonas