Re: [PATCH net-next v11 5/9] net: dsa: lan9645x: add bridge support

<[email protected]>
Newsgroups org.kernel.vger.linux-devicetree,dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>
On Thu, 2026-08-06 at 14:38 +0000, [email protected] 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] Hardware learning is not disabled when a port leaves a bridge, causing cross-port FDB pollution that breaks software bridging/routing for standalone ports.
> --
> 
> --- Patch [5]: [PATCH net-next v11 5/9] net: dsa: lan9645x: add bridge support ---
> commit f959003f0679008432479a15ff4b25747ede3826
> Author: Jens Emil Schulz Østergaard <[email protected]>
> 
>     net: dsa: lan9645x: add bridge support
> 
> Add support for hardware offloading of the bridge. We support a single
> bridge device.
> 
> > diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> > index b253813518c20..c145d28ffbb99 100644
> > --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> > +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> [ ... ]
> > @@ -282,6 +305,282 @@ static void lan9645x_port_phylink_get_caps(struct dsa_switch *ds, int port,
> [ ... ]
> > +static void lan9645x_port_bridge_leave(struct dsa_switch *ds, int port,
> > +                                    struct dsa_bridge bridge)
> > +{
> > +     struct lan9645x *lan9645x = ds->priv;
> > +     struct lan9645x_port *p;
> > +
> > +     p = lan9645x_to_port(lan9645x, port);
> > +
> > +     mutex_lock(&lan9645x->fwd_domain_lock);
> > +
> > +     lan9645x->bridge_mask &= ~BIT(p->chip_port);
> > +
> > +     /* Last port leaving clears bridge dev */
> > +     if (!lan9645x->bridge_mask)
> > +             lan9645x->bridge = NULL;
> > +
> > +     __lan9645x_port_set_host_flood(lan9645x);
> > +     lan9645x_update_fwd_mask(lan9645x);
> > +
> > +     mutex_unlock(&lan9645x->fwd_domain_lock);
> > +}
> 
> [Severity: High]
> Does this code need to clear p->learn_ena when a port leaves the bridge?
> 
> When a port is removed from the hardware bridge, its learning state incorrectly
> remains enabled.
> 

It does not remain enabled. The DSA core resets the bridge port flags on
bridge leave, learning included, and it does so before it puts the port
back into BR_STATE_FORWARDING.

dsa_port_bridge_leave() ends in dsa_port_switchdev_unsync_attrs(), which
calls dsa_port_clear_brport_flags() first and dsa_port_set_state_now(dp,
BR_STATE_FORWARDING, true) second. BR_LEARNING is in that helpers mask
but not in its val, so the driver is called with mask = BR_LEARNING,
val = 0, and lan9645x_port_set_learning() clears p->learn_ena and writes
ANA_PORT_CFG.LEARN_ENA = 0 immediately.

So by the time lan9645x_port_bridge_stp_state_set() evaluates

      (state == BR_STATE_LEARNING || state == BR_STATE_FORWARDING) &&
      p->learn_ena

p->learn_ena is already false and hardware learning stays off. Clearing
it in lan9645x_port_bridge_leave() would be redundant.



>  If this port and another port are later added to a software
> bridge (for example, if the hardware only supports one offloaded bridge), the
> DSA core transitions the standalone port to BR_STATE_FORWARDING.
> 
> Because p->learn_ena was not cleared during bridge leave, the call to
> lan9645x_port_bridge_stp_state_set() will incorrectly re-enable hardware
> learning for the standalone port:
> 
>         learn_ena = (state == BR_STATE_LEARNING ||
>                      state == BR_STATE_FORWARDING) && p->learn_ena;
> 
>         lan_rmw(ANA_PORT_CFG_LEARN_ENA_SET(learn_ena), ...
> 
> This causes the port to learn MAC addresses into the shared HOST_PVID FDB used
> by standalone ports. When the other software-bridged port (which expects to
> send unknown unicast traffic to the CPU) receives traffic for a MAC dynamically
> learned by this port, the hardware resolves the destination.
> 
> However, since PGID_SRC for standalone ports is 0, the packet is silently
> dropped in hardware instead of being forwarded to the CPU, breaking network
> connectivity for the fallback software bridge.
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260805-dsa_lan9645x_switch_driver_base-v11-0-007ebc983a0a@microchip.com?part=5
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.