Re: Possible to use both dev_mc_sync and __dev_mc_sync?

[Vladimir Oltean <olteanv@gmail.com>]

On Mon, Mar 21, 2022 at 08:42:59PM +0200, Vladimir Oltean wrote:
> On Mon, Mar 21, 2022 at 06:37:05PM +0000, Alexander Duyck wrote:
> > > -----Original Message-----
> > > From: Vladimir Oltean <olteanv@gmail.com>
> > > Sent: Monday, March 21, 2022 9:32 AM
> > > To: Alexander Duyck <alexanderduyck@fb.com>; Jakub Kicinski
> > > <kuba@kernel.org>; Jiri Pirko <jiri@nvidia.com>; Florian Fainelli
> > > <f.fainelli@gmail.com>
> > > Cc: netdev@vger.kernel.org
> > > Subject: Possible to use both dev_mc_sync and __dev_mc_sync?
> > I hadn't intended it to work this way. The expectation was that
> > __dev_mc_sync would be used by hardware devices whereas dev_mc_sync
> > was used by stacked devices such as vlan or macvlan.
> 
> Understood, thanks for confirming.
> 
> > Probably the easiest way to address it is to split things up so that
> > you are using __dev_mc_sync if the switch supports mc filtering and
> > have your dsa_slave_sync/unsync_mc call also push it down to the lower
> > device, and then call dev_mc_sync after that so that if it hasn't
> > already been pushed to the lower device it gets pushed.
> 
> Yes, I have a patch with that change, just wanted to make sure I'm not
> missing something. It's less efficient because now we need to check
> whether dsa_switch_supports_uc_filtering() for each address, whereas
> before we checked only once, before calling __dev_uc_add(). Oh well.
> 
> > The assumption is that the lower device and the hardware would be
> > synced in the same way. If we can't go that route we may have to look
> > at implementing a different setup in terms of the reference counting
> > such as what is done in __hw_addr_sync_multiple.
> 
> So as mentioned, I haven't really understood the internals of the
> reference/sync counting schemes being used here. But why are there
> different implementations for dev_mc_sync() and dev_mc_sync_multiple()?

And on the same not of me not quite understanding what goes on, I wonder
why some multicast addresses get passed both to the lower dev and to
dsa_slave_sync_mc (which is why I didn't notice the problem in the first
place), while others don't.