Re: [PATCH net] net: ti: icssg-prueth: Fix link-local addresses being forwarded out of slave ports

[MD Danish Anwar <danishanwar@ti.com>]

Hi Paoli,

On 07/07/26 4:09 pm, MD Danish Anwar wrote:
> Hi Paolo,
> 
> On 07/07/26 4:00 pm, Paolo Abeni wrote:
>> On 7/1/26 1:25 PM, MD Danish Anwar wrote:
>>> Link-local multicast addresses (01:80:c2:00:00:0x) must only be
>>> delivered to the host port (P0) and must not be forwarded out of
>>> the physical slave ports. icssg_fdb_add_del() was programming these
>>> addresses with P1/P2 membership bits set, causing the firmware to
>>> forward them out of slave ports.
>>>
>>> Clear P1/P2 membership and set only P0 membership when
>>> is_link_local_ether_addr() returns true.
>>>
>>> Fixes: 487f7323f39a ("net: ti: icssg-prueth: Add helper functions to configure FDB")
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> ---
>>>  drivers/net/ethernet/ti/icssg/icssg_config.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>> index 3f8237c17d099..04a81402e3f3c 100644
>>> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
>>> @@ -732,6 +732,16 @@ int icssg_fdb_add_del(struct prueth_emac *emac, const unsigned char *addr,
>>>  	u8 fid = vid;
>>>  	int ret;
>>>  
>>> +	/* Link-local addresses (01:80:c2:00:00:0x) must only be delivered to
>>> +	 * the host port (P0). Clear P1/P2 membership to prevent the firmware
>>> +	 * from forwarding them out of the physical slave ports.
>>> +	 */
>>> +	if (is_link_local_ether_addr(addr)) {
>>> +		fid_c2 |= ICSSG_FDB_ENTRY_P0_MEMBERSHIP;
>>> +		fid_c2 &= ~(ICSSG_FDB_ENTRY_P1_MEMBERSHIP |
>>> +			    ICSSG_FDB_ENTRY_P2_MEMBERSHIP);
>>> +	}
>>
>> Sashiko gemeni mentioned it could be safer to reject entirely wrong masks:
>>
>> https://sashiko.dev/#/patchset/20260701112535.4027920-1-danishanwar%40ti.com
>>
>> It's not clear to me if the mentioned bad scenario is actually possibly,
>> please have a look.
>>
> 
> I had a look at the Sashiko comment. This seems to be a false positive
> to me.
> 
> Link-local addresses (01:80:c2:00:00:0x) are IEEE 802.1D Table 7-10
> reserved addresses that bridges MUST NOT forward. They are consumed
> locally by STP, LACP, LLDP, PAE, etc.
> 
> No valid protocol or user configuration would add an MDB entry for these
> addresses on a slave port — doing so is a misconfiguration regardless of
> hardware.
> 
> The silent enforcement in icssg_fdb_add_del() ensures the hardware
> always reflects the mandatory protocol behavior. Returning -EOPNOTSUPP
> would only matter if there were a legitimate caller we needed to reject
> — there isn't one.
> 
> I think silently adding host port to FDB membership for Link Local
> addresses is OK. This Sashiko comment can be ignored.
> 

I have responded here to the Sashiko comment. I see that this patch is
marked "Changes Requested" in patchwork.

Since the Sashiko comment was false positive, can you please pick this
patch if it's OK with you or do I need to send a v2?

I haven't sent v2 yet as Sashiko will run again on it and give the same
comment again.


-- 
Thanks and Regards,
Danish