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

[Jens Emil Schulz Ostergaard]

On Wed, 2026-05-27 at 17:39 +0200, Jonas Gorski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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

No the chip is configured to trap these frames, so I think you are right that
at the moment we do not honor BR_NO_STP nor group_fwd_mask. I will fix this in the
next version. I plan to just use is_link_local_ether_addr.

Thanks,
Emil