Re: [PATCH net-next v15 8/9] net: dsa: lan9645x: add bridge support

[Jens Emil Schulz Ostergaard <jensemil.schulzostergaard@microchip.com>]

On Mon, 2026-10-19 at 10:08 +0000, netdev-bot+sashiko@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.
> 
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] The 802.1Q-only restriction can be bypassed by changing the
>   bridge's vlan_protocol after the port has joined.
> 
> --
> 
> net: dsa: lan9645x: add bridge support
> 
> This adds hardware offload of a single bridge to the lan9645x DSA
> driver. It covers ageing time, bridge port flags, STP state handling and
> host flooding. Joining a VLAN-aware bridge whose vlan_protocol is not
> 802.1Q is refused at join time.
> 
> > diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> > index d2e8c52e2a178..c6371a7b9a52a 100644
> > --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> > +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> 
> [ ... ]
> 
> > +static int lan9645x_port_bridge_join(struct dsa_switch *ds, int port,
> > +                                  struct dsa_bridge bridge,
> > +                                  bool *tx_fwd_offload,
> > +                                  struct netlink_ext_ack *extack)
> > +{
> > +     struct lan9645x *lan9645x = ds->priv;
> > +     struct lan9645x_port *p;
> > +
> > +     if (!lan9645x_bridge_vlan_proto_ok(bridge.dev)) {
> > +             NL_SET_ERR_MSG_MOD(extack,
> > +                                "Only 802.1Q VLAN-aware bridges are supported");
> > +             return -EINVAL;
> > +     }
> 
> [Severity: High]
> Can the 802.1Q-only restriction be bypassed by changing vlan_protocol
> after the port has joined?
> 
> lan9645x_bridge_vlan_proto_ok() is only checked in two places. One is
> here, at join time. The other is lan9645x_port_vlan_filtering(). DSA
> only calls lan9645x_port_vlan_filtering() when the filtering state
> changes, because dsa_port_vlan_filtering() returns early when
> dsa_port_is_vlan_filtering(dp) already matches.
> 
> A protocol change goes through __br_vlan_set_proto():
> 
> net/bridge/br_vlan.c:__br_vlan_set_proto() {
>         struct switchdev_attr attr = {
>                 .orig_dev = br->dev,
>                 .id = SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL,
>                 .flags = SWITCHDEV_F_SKIP_EOPNOTSUPP,
>     ...
>         err = switchdev_port_attr_set(br->dev, &attr, extack);
>         if (err && err != -EOPNOTSUPP)
>                 return err;
>     ...
>         br->vlan_proto = proto;
>     ...
> }
> 
> Only mlxsw and mlx5 handle SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL. DSA
> has no case for it and returns -EOPNOTSUPP. The bridge then commits the
> new protocol, and lan9645x never sees the change.
> 
> br_changelink() also handles IFLA_BR_VLAN_FILTERING before
> IFLA_BR_VLAN_PROTOCOL. So both of these sequences look like they end up
> with an offloaded VLAN-aware 802.1ad bridge:
> 
>   ip link set br0 type bridge vlan_filtering 1 vlan_protocol 802.1ad
> 
>   ip link set br0 type bridge vlan_protocol 802.1ad
>   (on a bridge that already has vlan_filtering 1)
> 
> In the first case, lan9645x_port_vlan_filtering() runs while
> br->vlan_proto is still 802.1Q, so the check passes.
> 
> The hardware then keeps its 802.1Q setup in ANA_VLAN_CFG: VLAN aware,
> C-tag type, pop count 1. Would it then classify customer 0x8100-tagged
> frames by the C-VID the sender chose, and forward them in hardware to
> members of that VID?
> 
> Under 802.1ad, the software bridge treats that tag as payload and puts
> the frame in the port PVID (the S-VLAN). If the hardware classifies by
> the C-VID instead, S-VLAN isolation breaks for traffic arriving from
> the wire.
> 
> The v15 changelog says VLAN-aware bridges whose vlan_protocol is not
> 802.1Q are refused. Does this need handling for protocol changes, rather
> than only the join-time check? Two options would be a veto from a
> switchdev or netdev notifier, or a DSA hook for
> SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL.
> 
> The later patch in the series, "net: dsa: lan9645x: add port
> statistics", only adds statistics and does not change this logic.
> 

Dismissed.

As noted for v14, a vlan_protocol change on a bridge that already
filters does not reach the driver. DSA does not handle
SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL.

The driver already refuses what it is told about such as joining a
VLAN-aware bridge and enabling VLAN filtering while vlan_protocol is not
802.1Q. Refusing a protocol change needs DSA to pass the attribute to
drivers, which is independent of this series.


> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015-dsa_lan9645x_switch_driver_base-v15-0-562f316f78b9%40microchip.com