netdev - Re: [PATCH net] net: mscc: ocelot: install MAC addresses in .ndo_set_rx

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] [day] [month] [year] [list]

Date:   Fri, 4 Dec 2020 17:40:59 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Florian Fainelli <f.fainelli@...il.com>
CC:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Horatiu Vultur <horatiu.vultur@...rochip.com>,
        "Allan W . Nielsen" <allan.nielsen@...rochip.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Steen Hegelund <steen.hegelund@...rochip.com>
Subject: Re: [PATCH net] net: mscc: ocelot: install MAC addresses in
 .ndo_set_rx_mode from process context

On Fri, Dec 04, 2020 at 09:34:54AM -0800, Florian Fainelli wrote:
> On 12/4/2020 9:09 AM, Vladimir Oltean wrote:
> > Currently ocelot_set_rx_mode calls ocelot_mact_learn directly, which has
> > a very nice ocelot_mact_wait_for_completion at the end. Introduced in
> > commit 639c1b2625af ("net: mscc: ocelot: Register poll timeout should be
> > wall time not attempts"), this function uses readx_poll_timeout which
> > triggers a lot of lockdep warnings and is also dangerous to use from
> > atomic context, leading to lockups and panics.
> > 
> > Steen Hegelund added a poll timeout of 100 ms for checking the MAC
> > table, a duration which is clearly absurd to poll in atomic context.
> > So we need to defer the MAC table access to process context, which we do
> > via a dynamically allocated workqueue which contains all there is to
> > know about the MAC table operation it has to do.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> 
> Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
> 
> Did you want to have a Fixes tag to help identify how far back this
> needs to be back ported?

I was on the fence about whether I should even target "net", considering
that it's not a small patch. But the lockdep warnings are super annoying,
I cannot do anything further on the ocelot switchdev driver with them.

There's also a small concern I have that I should have taken a reference
count on ocelot->dev using get_device, to avoid racing with the unbind.
I think I'll send a v2 with this and a Fixes: tag.