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

[Florian Fainelli <florian.fainelli@broadcom.com>]

On 10/28/26 02:52, Jonas Gorski wrote:
> On Tue, Oct 27, 2026 at 9:22 PM Florian Fainelli
> <florian.fainelli@broadcom.com> 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@broadcom.com>
>> ---
>>   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