All of lore.kernel.org
 help / color / mirror / Atom feed
From: <JensEmil.SchulzOstergaard@microchip.com>
To: <sashiko-reviews@lists.linux.dev>
Cc: <robh@kernel.org>, <conor+dt@kernel.org>, <devicetree@vger.kernel.org>
Subject: Re: [PATCH net-next v11 5/9] net: dsa: lan9645x: add bridge support
Date: Fri, 7 Aug 2026 13:44:01 +0000	[thread overview]
Message-ID: <55a7fa85aed544e9652fce746a6c57bd61791331.camel@microchip.com> (raw)
In-Reply-To: <20260806143840.00FCC1F00A3A@smtp.kernel.org>

On Thu, 2026-08-06 at 14:38 +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] 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 <jensemil.schulzostergaard@microchip.com>
> 
>     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


  reply	other threads:[~2026-08-07 13:44 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 14:34 [PATCH net-next v11 0/9] net: dsa: add DSA support for the LAN9645x switch chip family Jens Emil Schulz Østergaard
2026-08-05 14:34 ` [PATCH net-next v11 1/9] net: dsa: add tag driver for LAN9645X Jens Emil Schulz Østergaard
2026-08-06 14:38   ` sashiko-bot
2026-08-07 14:41     ` JensEmil.SchulzOstergaard
2026-08-11 10:55       ` Paolo Abeni
2026-08-11 11:50         ` JensEmil.SchulzOstergaard
2026-08-05 14:34 ` [PATCH net-next v11 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings Jens Emil Schulz Østergaard
2026-08-06 14:38   ` sashiko-bot
2026-08-07 14:15     ` JensEmil.SchulzOstergaard
2026-08-05 14:34 ` [PATCH net-next v11 3/9] net: dsa: lan9645x: add autogenerated register macros Jens Emil Schulz Østergaard
2026-08-05 14:34 ` [PATCH net-next v11 4/9] net: dsa: lan9645x: add basic dsa driver for LAN9645X Jens Emil Schulz Østergaard
2026-08-06 14:38   ` sashiko-bot
2026-08-07 14:07     ` JensEmil.SchulzOstergaard
2026-08-05 14:34 ` [PATCH net-next v11 5/9] net: dsa: lan9645x: add bridge support Jens Emil Schulz Østergaard
2026-08-06 14:38   ` sashiko-bot
2026-08-07 13:44     ` JensEmil.SchulzOstergaard [this message]
2026-08-05 14:34 ` [PATCH net-next v11 6/9] net: dsa: lan9645x: add vlan support Jens Emil Schulz Østergaard
2026-08-06 14:38   ` sashiko-bot
2026-08-07 13:40     ` JensEmil.SchulzOstergaard
2026-08-11 10:32   ` Paolo Abeni
2026-08-11 12:12     ` Jens Emil Schulz Ostergaard
2026-08-05 14:34 ` [PATCH net-next v11 7/9] net: dsa: lan9645x: add mac table integration Jens Emil Schulz Østergaard
2026-08-05 14:34 ` [PATCH net-next v11 8/9] net: dsa: lan9645x: add mdb management Jens Emil Schulz Østergaard
2026-08-06 14:38   ` sashiko-bot
2026-08-07 13:22     ` JensEmil.SchulzOstergaard
2026-08-05 14:34 ` [PATCH net-next v11 9/9] net: dsa: lan9645x: add port statistics Jens Emil Schulz Østergaard
2026-08-11 10:35 ` [PATCH net-next v11 0/9] net: dsa: add DSA support for the LAN9645x switch chip family Paolo Abeni
2026-08-11 10:36   ` Paolo Abeni
2026-08-11 12:18   ` Jens Emil Schulz Ostergaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55a7fa85aed544e9652fce746a6c57bd61791331.camel@microchip.com \
    --to=jensemil.schulzostergaard@microchip.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.