Re: [PATCH net 3/5] net: dsa: b53: clear stale state when creating an ARL entry

[Florian Fainelli]

On 10/28/26 02:52, Jonas Gorski wrote:
> On Tue, Oct 27, 2026 at 9:22 PM Florian Fainelli
> <florian.fainelli@xxxxxxxxxxxx> wrote:
> > When an ARL lookup does not find the requested MAC and VID, b53_arl_read()
> > returns the first free bin index but leaves the entry populated with the
> > last bin examined. The multicast add path then ORs the requested port into
> > that unrelated entry's forwarding bitmap, potentially forwarding packets
> > to ports which are not members of the multicast group.
> >
> > Clear the entry before creating a new one. Also return without writing
> > anything when deleting an entry which is not present, instead of making
> > a new entry from stale bin contents.
> >
> > Fixes: 5d65b64a3d97 ("net: dsa: b53: Add support for MDB")
> > Signed-off-by: Florian Fainelli <florian.fainelli@xxxxxxxxxxxx>
> > ---
> >   drivers/net/dsa/b53/b53_common.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> > index 559d4b360c82..0871a10d8cbc 100644
> > --- a/drivers/net/dsa/b53/b53_common.c
> > +++ b/drivers/net/dsa/b53/b53_common.c
> > @@ -1992,6 +1992,10 @@ static int b53_arl_op(struct b53_device *dev, int op, int port,
> >                          addr, vid);
> >                  return is_valid ? ret : 0;
> >          case -ENOENT:
> > +               if (!is_valid)
> > +                       return 0;
>
> Should we return 0, or should we pass on the -ENOENT? We failed to
> remove the entry, so implies there was a desync between hardware state
> and software state.
>
> If we don't, we should at least report the issue. If we do, we don't
> need to, as dsa will do that. Though we then should also report
> -ENOSPC && !is_valid as -ENOENT.
>
> Likewise when trying to add an entry that already exists.

Agreed, returning -ENOENT seems preferable so the missing hardware entry 
is visible to the caller. I'll propagate it for the -ENOENT case and 
translate -ENOSPC to -ENOENT when deleting.

For additions, finding an existing MAC/VID is not necessarily a 
desynchronization: an FDB add may move an entry or make a dynamically 
learned entry static, and an MDB add may extend the port bitmap. So I 
think we should retain the existing update behavior rather than return 
-EEXIST on a lookup match.

> > +
> > +               memset(&ent, 0, sizeof(ent));
>
> Just bikeshedding, but semantically it feels the memset should be
> below the comment/dev_dbg().

Yes, I took that change.

Thanks!
--
Florian