Re: [net-next PATCH v4 7/8] net: dsa: realtek: rtl8365mb: add FDB support

From: Mieczyslaw Nalewaj

Date: Sat May 16 2026 - 11:30:08 EST


On 5/16/2026 2:47 PM, Luiz Angelo Daros de Luca wrote:
>>> +
>>> +struct rtl8365mb_l2_mc_key {
>>> + u8 mac_addr[ETH_ALEN];
>>> + union {
>>> + u16 vid; /* IVL */
>>> + u16 fid; /* SVL */
>>> + };
>>> + bool ivl;
>>> +};
>>> +
>>> +struct rtl8365mb_l2_mc {
>>> + struct rtl8365mb_l2_mc_key key;
>>> + u16 member;
>>> + u8 priority;
>>> + u8 igmpidx;
>>> +
>>> + bool is_static;
>>> + bool fwd_pri;
>>> + bool igmp_asic;
>>> +};
>>> +
>>> +static void rtl8365mb_l2_data_to_uc(const u16 *data, struct rtl8365mb_l2_uc *uc)
>>> +{
>>> + u32 val;
>>> +
>>> + uc->key.mac_addr[5] = FIELD_GET(RTL8365MB_L2_UC_D0_MAC5_MSK, data[0]);
>>> + uc->key.mac_addr[4] = FIELD_GET(RTL8365MB_L2_UC_D0_MAC4_MSK, data[0]);
>>> + uc->key.mac_addr[3] = FIELD_GET(RTL8365MB_L2_UC_D1_MAC3_MSK, data[1]);
>>> + uc->key.mac_addr[2] = FIELD_GET(RTL8365MB_L2_UC_D1_MAC2_MSK, data[1]);
>>> + uc->key.mac_addr[1] = FIELD_GET(RTL8365MB_L2_UC_D2_MAC1_MSK, data[2]);
>>> + uc->key.mac_addr[0] = FIELD_GET(RTL8365MB_L2_UC_D2_MAC0_MSK, data[2]);
>>> + uc->key.efid = FIELD_GET(RTL8365MB_L2_UC_D4_EFID_MSK, data[4]);
>>> + uc->key.vid = FIELD_GET(RTL8365MB_L2_UC_D3_VID_MSK, data[3]);
>>> + uc->key.ivl = FIELD_GET(RTL8365MB_L2_UC_D3_IVL_MSK, data[3]);
>>> + uc->key.fid = FIELD_GET(RTL8365MB_L2_UC_D4_FID_MSK, data[4]);
>>> + uc->age = FIELD_GET(RTL8365MB_L2_UC_D4_AGE_MSK, data[4]);
>>> + uc->auth = FIELD_GET(RTL8365MB_L2_UC_D4_AUTH_MSK, data[4]);
>>> +
>>
>> The problem with overwriting uc->key.fid and uc->key.vid values in the union still exists.
>
> Hello Mieczysław,
>
> Thanks for the review. Please note that rtl8365mb_l2_data_to_uc
> operates on the unicast structure (struct rtl8365mb_l2_uc), rather
> than the multicast structure (struct rtl8365mb_l2_mc) quoted in your
> comment.
>
> For reference, here is the definition of rtl8365mb_l2_uc and its key:
>
> +struct rtl8365mb_l2_uc_key {
> + u8 mac_addr[ETH_ALEN];
> + u16 vid;
> + u16 fid;
> + bool ivl;
> + u16 efid;
> +};
> +
> +struct rtl8365mb_l2_uc {
> + struct rtl8365mb_l2_uc_key key;
> + u8 port;
> + u8 age;
> + u8 priority;
> +
> + bool sa_block;
> + bool da_block;
> + bool auth;
> + bool is_static;
> + bool sa_pri;
> + bool fwd_pri;
> +};
>
> As you can see, there is no union in the unicast key. The hardware
> table entry has independent fields for efid, fid, and vid (though in
> practice, vendor examples always show one of fid or vid set to 0).
>
> The multicast entry (struct rtl8365mb_l2_mc) is the only one that uses
> a union for the dual-purpose field, which is interpreted as either fid
> or vid depending on the ivl value.
>
> Regards,
>
> Luiz

Reviewed-by: Mieczyslaw Nalewaj <namiltd@xxxxxxxxx>