netdev - Re: Possible to use both dev_mc_sync and __dev_mc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220321184526.cdpzksga2fu4hyct@skbuf>
Date:   Mon, 21 Mar 2022 20:45:26 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Alexander Duyck <alexanderduyck@...com>
Cc:     Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...dia.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: Possible to use both dev_mc_sync and __dev_mc_sync?

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@...il.com>
> > > Sent: Monday, March 21, 2022 9:32 AM
> > > To: Alexander Duyck <alexanderduyck@...com>; Jakub Kicinski
> > > <kuba@...nel.org>; Jiri Pirko <jiri@...dia.com>; Florian Fainelli
> > > <f.fainelli@...il.com>
> > > Cc: netdev@...r.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.