Re: [PATCH net-next 2/2] net: sparx5: Add mdb handlers

[Steen Hegelund <steen.hegelund@microchip.com>]

Hi Casper

On Tue, 2022-03-22 at 10:59 +0100, Casper Andersson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> So this was already merged, but I have some comments on the feedback for
> the follow up patch.
> 
> > > +static int sparx5_handle_port_mdb_add(struct net_device *dev,
> > > +                                     struct notifier_block *nb,
> > > +                                     const struct switchdev_obj_port_mdb *v)
> > > +{
> > > +       struct sparx5_port *port = netdev_priv(dev);
> > > +       struct sparx5 *spx5 = port->sparx5;
> > > +       u16 pgid_idx, vid;
> > > +       u32 mact_entry;
> > > +       int res, err;
> > > +
> > > +       /* When VLAN unaware the vlan value is not parsed and we receive vid 0.
> > > +        * Fall back to bridge vid 1.
> > > +        */
> > > +       if (!br_vlan_enabled(spx5->hw_bridge_dev))
> > > +               vid = 1;
> > > +       else
> > > +               vid = v->vid;
> > > +
> > > +       res = sparx5_mact_find(spx5, v->addr, vid, &mact_entry);
> > > +
> > > +       if (res) {
> > > +               pgid_idx = LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_ADDR_GET(mact_entry);
> > > +
> > > +               /* MC_IDX has an offset of 65 in the PGID table. */
> > > +               pgid_idx += PGID_MCAST_START;
> > 
> > This will overlap some of the first ports with the flood masks according to:
> > 
> > https://microchip-ung.github.io/sparx-5_reginfo/reginfo_sparx-5.html?select=ana_ac,pgid
> > 
> > You should use the custom area (PGID_BASE + 8 and onwards) for this new feature.
> 
> I'm aware of the overlap, hence why the PGID table has those fields
> marked as reserved. But your datasheet says that the multicast index
> has an offset of 65 (ie. MC_IDX = 0 is at PGID = 65). This is already
> taken into account in the mact_learn function. I could set the
> allocation to start at PGID_BASE + 8, but the offset still needs to
> be 65, right?

As I understand the PGID table functionality, you will need to start your custom table at PGID_BASE
+ 8 as the bitmasks at offset 65 to 70 are used as flood masks, so their purpose are already
defined.

BR
Steen

> 
> BR
> Casper