netdev - Re: [PATCH net-next v4 2/3] net: implement threaded-able napi poll loop support

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]

Date:   Sat, 12 Dec 2020 14:55:03 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Wei Wang <weiwan@...gle.com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        Paolo Abeni <pabeni@...hat.com>,
        Hannes Frederic Sowa <hannes@...essinduktion.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Felix Fietkau <nbd@....name>, Hillf Danton <hdanton@...a.com>
Subject: Re: [PATCH net-next v4 2/3] net: implement threaded-able napi poll
 loop support

On Sat, 12 Dec 2020 14:50:22 -0800 Jakub Kicinski wrote:
> > @@ -6731,6 +6790,7 @@ void napi_disable(struct napi_struct *n)
> >  		msleep(1);
> >  
> >  	hrtimer_cancel(&n->timer);
> > +	napi_kthread_stop(n);  
> 
> I'm surprised that we stop the thread on napi_disable() but there is no
> start/create in napi_enable(). NAPIs can (and do get) disabled and
> enabled again. But that'd make your code crash with many popular
> drivers if you tried to change rings with threaded napi enabled so I
> feel like I must be missing something..

Ah, not crash, 'cause the flag gets cleared. Is it intentional that any
changes that disable NAPIs cause us to go back to non-threaded NAPI?
I think I had the "threaded" setting stored in struct netdevice in my
patches, is there a reason not to do that?

In fact your patches may _require_ the device to be up to enable
threaded NAPI if NAPIs are allocated in open.