Re: [net-next PATCH 08/10] net: dsa: realtek: rtl8365mb: add FDB support

[Luiz Angelo Daros de Luca]

> I actually at one point started to implement FDB accessors to the RTL8366RB
> before I got sidetracked (for the same reasons of being able to do hardware
> offloading), here is how far I got before I got distracted:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/patch/?id=9941b129f91e92423803d6e2869022ffa66e9654
>
> You can see that it also has an L2 table albeit with 4 entries rather than 6,
> and an additional "CAM" (content adressable memory...) table that I don't quite
> understand.

I will look into the CAM table info in rtl8367c family.

> On Wed, Apr 1, 2026 at 1:01 AM Luiz Angelo Daros de Luca
> <luizluca@xxxxxxxxx> wrote:
>
> > +int rtl8365mb_l2_del_mc(struct realtek_priv *priv, u16 port,
> > +                       const unsigned char mac_addr[static ETH_ALEN],
> > +                       u16 vid)
> > +int rtl8365mb_l2_get_next_uc(struct realtek_priv *priv, u16 *addr, u16 port,
> > +                            struct rtl8365mb_l2_uc *uc);
> > +int rtl8365mb_l2_add_uc(struct realtek_priv *priv, u16 port,
> > +                       const unsigned char addr[static ETH_ALEN],
> > +                       u16 efid, u16 vid);
> > +int rtl8365mb_l2_del_uc(struct realtek_priv *priv, u16 port,
> > +                       const unsigned char addr[static ETH_ALEN],
> > +                       u16 efid, u16 vid);
> > +int rtl8365mb_l2_flush(struct realtek_priv *priv, int port, u16 vid);
> > +
> > +int rtl8365mb_l2_add_mc(struct realtek_priv *priv, u16 port,
> > +                       const unsigned char mac_addr[static ETH_ALEN],
> > +                       u16 vid);
> > +int rtl8365mb_l2_del_mc(struct realtek_priv *priv, u16 port,
> > +                       const unsigned char mac_addr[static ETH_ALEN],
> > +                       u16 vid);
>
> I think it would make sense to add these to struct realtek_ops and
>  make them available for all RTL8366 variants.

Agreed. I will add these to struct realtek_ops. I'll need to create a
generic replacement for struct rtl8365mb_l2_uc, but the rest of the
migration should be straightforward. For now, the efid argument can
simply be ignored by the RTL8366 implementation.

> Possibly it would even make sense to move rtl8365mb_table_query()
> to struct realtek_ops under the name .table_query and all of
> these helpers to rtl8366-core.c as rtl8366_l2_del_mc() etc
> just calling the per-chip table query variant. I'm not sure
> but you can perhaps see what makes most sense?

I suspect rtl8365mb_table_query() might be too complex to abstract
effectively without first seeing how the RTL8366 handles table
lookups, as hardware-specific behaviors can vary significantly here. I
suggest we implement it for the RTL8366 first; once we have a
reference point, we can discuss the best way to consolidate the logic.

> > +       struct mutex l2_lock;
>
> Move to struct realtek_priv, every chip will need an l2 lock.

Sure.

> > +static void rtl8365mb_port_fast_age(struct dsa_switch *ds, int port)
> > +static int rtl8365mb_port_fdb_add(struct dsa_switch *ds, int port,
> > +                                 const unsigned char *addr, u16 vid,
> > +                                 struct dsa_db db)
> > +static int rtl8365mb_port_fdb_del(struct dsa_switch *ds, int port,
> > +                                 const unsigned char *addr, u16 vid,
> > +                                 struct dsa_db db)
> > +static int rtl8365mb_port_fdb_dump(struct dsa_switch *ds, int port,
> > +                                  dsa_fdb_dump_cb_t *cb, void *data)
> > +static int rtl8365mb_port_mdb_add(struct dsa_switch *ds, int port,
> > +                                 const struct switchdev_obj_port_mdb *mdb,
> > +                                 struct dsa_db db)
> > +static int rtl8365mb_port_mdb_del(struct dsa_switch *ds, int port,
> > +                                 const struct switchdev_obj_port_mdb *mdb,
> > +                                 struct dsa_db db)
>
> I think you should rename all of these rtl8366_port_* and move them
> to rtl8366-core.c. They are generic.

Yes. Once rtl8365mb_l2_* is moved to realtek_ops, these functions
become generic wrappers and fit perfectly in rtl83xx.c.

> > @@ -2498,6 +2687,12 @@ static const struct dsa_switch_ops rtl8365mb_switch_ops = {
> >         .port_bridge_join = rtl8365mb_port_bridge_join,
> >         .port_bridge_leave = rtl8365mb_port_bridge_leave,
> >         .port_stp_state_set = rtl8365mb_port_stp_state_set,
> > +       .port_fast_age = rtl8365mb_port_fast_age,
>
> .port_fast_age = rtl8366_port_fast_age,

I will update the ops to use the new generics.

Regards,

Luiz