netdev - Re: [RFC net-next 1/4] net: napi threaded: remove unnecessary locking

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: <20211210191022.02e07ee6@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 10 Dec 2021 19:10:22 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Yannick Vignon <yannick.vignon@....nxp.com>
Cc:     Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        netdev@...r.kernel.org, Ong Boon Leong <boon.leong.ong@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jose Abreu <joabreu@...opsys.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Wei Wang <weiwan@...gle.com>,
        Alexander Lobakin <alexandr.lobakin@...el.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Xiaoliang Yang <xiaoliang.yang_1@....com>, mingkai.hu@....com,
        Joakim Zhang <qiangqing.zhang@....com>,
        sebastien.laveze@....com, Yannick Vignon <yannick.vignon@....com>
Subject: Re: [RFC net-next 1/4] net: napi threaded: remove unnecessary
 locking

On Fri, 10 Dec 2021 20:35:53 +0100 Yannick Vignon wrote:
> From: Yannick Vignon <yannick.vignon@....com>
> 
> NAPI polling is normally protected by local_bh_disable()/local_bh_enable()
> calls, to avoid that code from being executed concurrently due to the
> softirq design. When NAPI instances are assigned their own dedicated kernel
> thread however, that concurrent code execution can no longer happen.

Meaning NAPI will now run in process context? That's not possible, 
lots of things depend on being in BH context. There are per-CPU
cross-NAPI resources, I think we have some expectations that we
don't need to take RCU locks here and there..

> Removing the lock helps lower latencies when handling real-time traffic
> (whose processing could still be delayed because of on-going processing of
> best-effort traffic), and should also have a positive effect on overall
> performance.
> 
> Signed-off-by: Yannick Vignon <yannick.vignon@....com>
> ---
>  net/core/dev.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 15ac064b5562..e35d90e70c75 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7131,13 +7131,11 @@ static int napi_threaded_poll(void *data)
>  		for (;;) {
>  			bool repoll = false;
>  
> -			local_bh_disable();
>  
>  			have = netpoll_poll_lock(napi);
>  			__napi_poll(napi, &repoll);
>  			netpoll_poll_unlock(have);
>  
> -			local_bh_enable();
>  
>  			if (!repoll)
>  				break;