Re: [PATCH] net: dsa: b53: Enable IGMP and MLD control packet CPU copies

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

On 10/27/2026 3:48 PM, netdev-bot+sashiko@kernel.org wrote:
> 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 new IGMP/MLD CPU-copy setup in b53_enable_mcast_snooping()
>    (drivers/net/dsa/b53/b53_common.c) is static and has one caller,…
> 
> --
> 
> net: dsa: b53: Enable IGMP and MLD control packet CPU copies
> 
> This commit enables the IGMP and MLD high-level protocol classifiers in
> B53_HL_PRTC_CTRL, so control packets get copied to the CPU even when a
> multicast ARL entry matches. This lets the bridge keep learning listeners
> after the first MDB entry has been offloaded. BCM5395 uses a 16-bit layout,
> BCM531xx/5301x/58xx use a 32-bit layout, and older chips are left alone.
> 
>> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
>> index 0880310c9ce3f..54844cfadf019 100644
>> --- a/drivers/net/dsa/b53/b53_common.c
>> +++ b/drivers/net/dsa/b53/b53_common.c
>> @@ -836,6 +836,52 @@ void b53_brcm_hdr_setup(struct dsa_switch *ds, int port)
>>   }
>>   EXPORT_SYMBOL(b53_brcm_hdr_setup);
>>   
>> +static int b53_enable_mcast_snooping(struct b53_device *dev)
>> +{
> 
> [ ... ]
> 
>> +	/* Older switches use different IGMP controls, not HL_PRTC_CTRL. */
>> +	if (!is531x5(dev) && !is5301x(dev) && !is58xx(dev))
>> +		return 0;
> 
> [ ... ]
> 
>> @@ -1334,6 +1380,10 @@ static int b53_setup(struct dsa_switch *ds)
>>   			b53_disable_port(ds, port);
>>   	}
>>   
>> +	ret = b53_enable_mcast_snooping(dev);
>> +	if (ret)
>> +		return ret;
>> +
>>   	return b53_setup_devlink_resources(ds);
>>   }
> 
> [Severity: High]
> Does this fix reach the Starfighter 2 switches (BCM7445, BCM7278, BCM4908)?
> 
> b53_enable_mcast_snooping() is static, and its only caller is b53_setup().
> bcm_sf2_sw_probe() sets the chip id and then swaps in its own ops:
> 
> drivers/net/dsa/bcm_sf2.c:bcm_sf2_sw_probe() {
>      ...
> 	pdata->chip_id = priv->type;
>      ...
> 	ds->ops = &bcm_sf2_ops;
>      ...
> }
> 
> bcm_sf2_ops sets .setup to bcm_sf2_sw_setup(), so b53_setup() never runs
> on these chips. bcm_sf2_sw_setup() does not program B53_HL_PRTC_CTRL:
> 
> drivers/net/dsa/bcm_sf2.c:bcm_sf2_sw_setup() {
>      ...
> 	b53_configure_vlan(ds);
> 	bcm_sf2_enable_acb(ds);
> 
> 	return b53_setup_devlink_resources(ds);
> }
> 
> bcm_sf2_ops still has this entry:
> 
> 	.port_mdb_add		= b53_mdb_add,
> 
> So multicast ARL entries still get installed on SF2. After the first
> listener's entry exists, the original problem looks unchanged there: later
> IGMPv2/MLDv1 reports sent to the group MAC follow the ARL entry and never
> reach the bridge.
> 
> The resume path, bcm_sf2_sw_resume() calling ds->ops->setup(), has the same
> gap.
> 
> The is58xx() part of the gate above matches BCM7445_DEVICE_ID and
> BCM7278_DEVICE_ID, but devices with those IDs never reach this function. Is
> that check dead code? The commit message also says this covers the "newer
> integrated RoboSwitch families".
> 
> Should the helper be exported and also called from bcm_sf2_sw_setup()?

Yes, all good points, will address in v2.

pw-bot: cr
-- 
Florian