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

[Luiz Angelo Daros de Luca]

> On 5/10/2026 7:35 PM, Jakub Kicinski wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > ---
> > net: dsa: realtek: rtl8365mb: add FDB support
> >
> > Implement support for FDB and MDB management for the RTL8365MB series
> > switches. Introduce a mutex lock (l2_lock) to protect concurrent L2 table
> > updates. Add support for forwarding database operations, including unicast
> > and multicast entry handling as well as fast aging support.
> >
> >> diff --git a/drivers/net/dsa/realtek/rtl8365mb_l2.c b/drivers/net/dsa/realtek/rtl8365mb_l2.c
> >> --- /dev/null
> >> +++ b/drivers/net/dsa/realtek/rtl8365mb_l2.c
> >
> > [ ... ]
> >
> >> +static void rtl8365mb_l2_data_to_uc(const u16 *data, struct rtl8365mb_l2_uc *uc)
> >> +{
> >
> > [ ... ]
> >
> >> +    uc->key.efid = FIELD_GET(RTL8365MB_L2_UC_D4_EFID_MASK, data[4]);
> >> +    uc->key.vid = FIELD_GET(RTL8365MB_L2_UC_D3_VID_MASK, data[3]);
> >> +    uc->key.ivl = FIELD_GET(RTL8365MB_L2_UC_D3_IVL_MASK, data[3]);
> >> +    uc->key.fid = FIELD_GET(RTL8365MB_L2_UC_D4_FID_MASK, data[4]);
> >
> > Because vid and fid share the same union memory in struct rtl8365mb_l2_uc_key,
> > does setting fid immediately after vid overwrite the 12-bit vid value with
> > the 4-bit fid value?
>
> Change code to:
>
> uc->key.efid = FIELD_GET(RTL8365MB_L2_UC_D4_EFID_MASK, data[4]);
> uc->key.ivl  = FIELD_GET(RTL8365MB_L2_UC_D3_IVL_MASK, data[3]);
>
> if (uc->key.ivl)
>         uc->key.vid = FIELD_GET(RTL8365MB_L2_UC_D3_VID_MASK, data[3]);
> else
>         uc->key.fid = FIELD_GET(RTL8365MB_L2_UC_D4_FID_MASK, data[4]);
>

Thanks Mieczyslaw,

It is a little more complicated than that. There are three FIDs in a
Unicast L2 table:

RTL8367C:
        15  14  13  12  11  10   9   8   7   6   5   4   3   2   1   0
        +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
Index 0 |     MAC Octet [4] (High)      |      MAC Octet [5] (Low)      |
        +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
Index 1 |     MAC Octet [2] (High)      |      MAC Octet [3] (Low)      |
        +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
Index 2 |     MAC Octet [0] (High)      |      MAC Octet [1] (Low)      |
        +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
        | S |   | I | L |               |                               |
Index 3 | P | 0 | V | 3 |               CVID / FID                      |
        | A |   | L | L |               (12 bits)                       |
        +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
        | S |   |       |               | S |           |               |
Index 4 | A | A |  AGE  |      SPA      | A |    FID    |     EFID      |
        | B | U |       |    (bits 2-0) | E |           |               |
        +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
        |                               | N | F |       |       | D |
Index 5 |           (Reserved)          | O | W | (Res) |  PRI  | A |
        |                               | S | D |       |       | B |
        +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+

One shared between CVID/FID (word 3), and two FID/EFID in work 4.

For Unicast entries, the vendor code just store CVID/FID in a cvid
unconditionally:

       pL2_entry->ivl       = l2Table.ivl_svl;
       pL2_entry->cvid      = l2Table.cvid_fid;
       pL2_entry->fid       = l2Table.fid;

Only for Multicast that it uses that condition:

       if(l2Table.ivl_svl == 1) /* IVL */
       {
           pL2_entry->cvid      = l2Table.cvid_fid;
           pL2_entry->fid       = 0;
       }
       else /* SVL*/
       {
           pL2_entry->cvid      = 0;
           pL2_entry->fid       = l2Table.cvid_fid;
       }

So, for unicast entries, we need dedicated fid/efid struct fields and
that cvid_fid is always cvid.

According to the delete method, the Unicast key is (MAC, IVL/SVL, FID, EFID):

    /* fill key (MAC,FID) to get L2 entry */
   memcpy(l2Table.mac.octet, pMac->octet, ETHER_ADDR_LEN);
   l2Table.ivl_svl     = pL2_data->ivl;
   l2Table.cvid_fid    = pL2_data->cvid;
   l2Table.fid         = pL2_data->fid;
   l2Table.efid        = pL2_data->efid;

The union just makes sense for multicast. The multicast key is (MAC,
IVL/SVL, VID/FID), without extra FID/EFID. The same delete code but
for multicast is:

   /* fill key (MAC,FID) to get L2 entry */
   memcpy(l2Table.mac.octet, pMcastAddr->mac.octet, ETHER_ADDR_LEN);
   l2Table.ivl_svl     = pMcastAddr->ivl;
   if(pMcastAddr->ivl)
       l2Table.cvid_fid    = pMcastAddr->vid;
   else
       l2Table.cvid_fid    = pMcastAddr->fid;

And without FID/EFID, we can have collisions between different bridges.

I'm also preparing the code for other families
(RTL8367,RTL8367B,RTL8367D) but they have their own problems. RTL8367
does not have a CVID field and RTL8367D does not have FID/EFID.
It will take some time to figure out everything here. And more time to
test it properly.

Regards,

Luiz