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

[<JensEmil.SchulzOstergaard@microchip.com>]

On Tue, 2026-07-14 at 14:43 +0000, sashiko-bot@kernel.org wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Adding an out-of-band VLAN tag without checking for and popping an existing payload tag results in double-tagged frames.
> --
> 
> --- Patch [1]: [PATCH net-next v10 1/9] net: dsa: add tag driver for LAN9645X ---
> commit 3fca3b09e95a3eeee247c89f8ecdf602074e9506
> Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
> 
> net: dsa: add tag driver for LAN9645X
> 
> This commit adds a DSA tag driver for the LAN9645x family of switches. It
> configures an NPI port to use a 16-byte long prefix on extraction and no
> prefix on injection, prepending an internal frame header (IFH) to transmit
> critical data.
> 
> > diff --git a/net/dsa/tag_lan9645x.c b/net/dsa/tag_lan9645x.c
> > new file mode 100644
> > index 0000000000000..2692c7ad56cbc
> > --- /dev/null
> > +++ b/net/dsa/tag_lan9645x.c
> [ ... ]
> > +static struct sk_buff *lan9645x_rcv(struct sk_buff *skb,
> > +                                 struct net_device *ndev)
> > +{
> [ ... ]
> > +     /* While we have REW_PORT_NO_REWRITE=0 on the NPI port, we still disable
> > +      * port VLAN tagging with REW_TAG_CFG. Any classified VID, different
> > +      * from a VID in the frame, will not be written to the frame, but is
> > +      * only communicated via the IFH. So for VLAN-aware ports we add the IFH
> > +      * vlan to the skb.
> > +      */
> > +     dp = dsa_user_to_port(skb->dev);
> > +     vlan_tpid = tag_type ? ETH_P_8021AD : ETH_P_8021Q;
> > +
> > +     if (dsa_port_is_vlan_filtering(dp) && vlan_tci) {
> > +             u16 port_pvid = 0;
> > +
> > +             br_vlan_get_pvid_rcu(skb->dev, &port_pvid);
> > +
> > +             if ((vlan_tci & VLAN_VID_MASK) != port_pvid)
> 
> [Severity: High]
> Does this code result in double-tagged frames?
> 
> If the hardware delivers the frame without stripping the in-band tag because
> port VLAN rewrite is disabled, calling __vlan_hwaccel_put_tag() here adds an
> out-of-band tag while leaving the original in-band tag intact in the payload.
> This would cause the network stack to process a single 802.1Q tagged frame
> from the wire as a Q-in-Q double-tagged frame.
> 
> Should lan9645x_rcv() pop the existing payload tag (for example, using
> __skb_vlan_pop()) before adding the out-of-band tag to avoid duplicating it?
> 

This is a false positive. On a vlan aware port, the vlan tag is popped and all
vlan info is stored in the IFH. The tag will not egress in the frame data unless
the rewriter pushes it, and we disable this toward the cpu via REW_TAG_CFG. 

There is no in-band vlan tag in the frame in this scenario. Only the IFH contains
the classified vlan info, so we add it to the skb. Using __skb_vlan_pop would
corrupt the frame, and we avoid it on purpose. I will update the comment to be
more clear about this.

> > +                     __vlan_hwaccel_put_tag(skb, htons(vlan_tpid), vlan_tci);
> > +     }
> > +
> > +     return skb;
> > +}
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260713-dsa_lan9645x_switch_driver_base-v10-0-a4886a08fb15@microchip.com?part=1