netdev - Re: [PATCH net-next v7 2/5] net: napi: add CPU affinity to napi

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]

Message-ID: <Z6KYDs0os_DizhMa@LQ3V64L9R2>
Date: Tue, 4 Feb 2025 14:43:26 -0800
From: Joe Damato <jdamato@...tly.com>
To: Ahmed Zaki <ahmed.zaki@...el.com>
Cc: netdev@...r.kernel.org, intel-wired-lan@...ts.osuosl.org,
	andrew+netdev@...n.ch, edumazet@...gle.com, kuba@...nel.org,
	horms@...nel.org, pabeni@...hat.com, davem@...emloft.net,
	michael.chan@...adcom.com, tariqt@...dia.com,
	anthony.l.nguyen@...el.com, przemyslaw.kitszel@...el.com,
	shayd@...dia.com, akpm@...ux-foundation.org, shayagr@...zon.com,
	kalesh-anakkur.purayil@...adcom.com
Subject: Re: [PATCH net-next v7 2/5] net: napi: add CPU affinity to
 napi_config

On Tue, Feb 04, 2025 at 03:06:19PM -0700, Ahmed Zaki wrote:
> A common task for most drivers is to remember the user-set CPU affinity
> to its IRQs. On each netdev reset, the driver should re-assign the
> user's settings to the IRQs.
> 
> Add CPU affinity mask to napi_config. To delegate the CPU affinity
> management to the core, drivers must:
>  1 - set the new netdev flag "irq_affinity_auto":
>                                        netif_enable_irq_affinity(netdev)
>  2 - create the napi with persistent config:
>                                        netif_napi_add_config()
>  3 - bind an IRQ to the napi instance: netif_napi_set_irq()
> 
> the core will then make sure to use re-assign affinity to the napi's
> IRQ.
> 
> The default IRQ mask is set to one cpu starting from the closest NUMA.

Not sure, but maybe the above should be documented somewhere like
Documentation/networking/napi.rst or similar?

Maybe that's too nit-picky, though, since the per-NAPI config stuff
never made it into the docs (I'll propose a patch to fix that).

> Signed-off-by: Ahmed Zaki <ahmed.zaki@...el.com>
> ---
>  include/linux/netdevice.h | 14 +++++++--
>  net/core/dev.c            | 62 +++++++++++++++++++++++++++++++--------
>  2 files changed, 61 insertions(+), 15 deletions(-)

[...]
 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 33e84477c9c2..4cde7ac31e74 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c

[...]

> @@ -6968,17 +6983,28 @@ void netif_napi_set_irq_locked(struct napi_struct *napi, int irq)
>  {
>  	int rc;
>  
> -	/* Remove existing rmap entries */
> -	if (napi->dev->rx_cpu_rmap_auto &&
> +	/* Remove existing resources */
> +	if ((napi->dev->rx_cpu_rmap_auto || napi->dev->irq_affinity_auto) &&
>  	    napi->irq != irq && napi->irq > 0)
>  		irq_set_affinity_notifier(napi->irq, NULL);
>  
>  	napi->irq = irq;
> -	if (irq > 0) {
> +	if (irq < 0)
> +		return;
> +
> +	if (napi->dev->rx_cpu_rmap_auto) {
>  		rc = napi_irq_cpu_rmap_add(napi, irq);
>  		if (rc)
>  			netdev_warn(napi->dev, "Unable to update ARFS map (%d)\n",
>  				    rc);
> +	} else if (napi->config && napi->dev->irq_affinity_auto) {
> +		napi->notify.notify = netif_napi_irq_notify;
> +		napi->notify.release = netif_napi_affinity_release;
> +
> +		rc = irq_set_affinity_notifier(irq, &napi->notify);
> +		if (rc)
> +			netdev_warn(napi->dev, "Unable to set IRQ notifier (%d)\n",
> +				    rc);
>  	}

Should there be a WARN_ON or WARN_ON_ONCE in here somewhere if the
driver calls netif_napi_set_irq_locked but did not link NAPI config
with a call to netif_napi_add_config?

It seems like in that case the driver is buggy and a warning might
be helpful.