netdev - Re: [PATCH net-next v7 1/5] net: move ARFS rmap management to core

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: <20250206182941.12705a4d@kernel.org>
Date: Thu, 6 Feb 2025 18:29:41 -0800
From: Jakub Kicinski <kuba@...nel.org>
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, 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, jdamato@...tly.com, shayd@...dia.com,
 akpm@...ux-foundation.org, shayagr@...zon.com,
 kalesh-anakkur.purayil@...adcom.com, David Arinzon <darinzon@...zon.com>
Subject: Re: [PATCH net-next v7 1/5] net: move ARFS rmap management to core

On Tue,  4 Feb 2025 15:06:18 -0700 Ahmed Zaki wrote:
> +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 &&
> +	    napi->irq != irq && napi->irq > 0)

this condition gets a bit hairy by the end of the series.
could you add a napi state bit that indicates that a notifier is
installed? Then here:

	if (napi->irq == irq)
		return;

	if (test_and_clear_bit(NAPI_STATE_HAS_NOTIFIER, &napi->state))
		irq_set_affinity_notifier(napi->irq, NULL);
	if (irq < 0)
		return;

And you can similarly simplify napi_disable_locked().

Speaking of which, why do the auto-removal in napi_disable()
rather than netif_napi_del() ? We don't reinstall on napi_enable()
and doing a disable() + enable() is fairly common during driver
reconfig.

> +		irq_set_affinity_notifier(napi->irq, NULL);
> +
> +	napi->irq = irq;
> +	if (irq > 0) {
> +		rc = napi_irq_cpu_rmap_add(napi, irq);
> +		if (rc)
> +			netdev_warn(napi->dev, "Unable to update ARFS map (%d)\n",

nit: not sure I'd grasp this message as a user, maybe:

	"Unable to install aRFS CPU to Rx queue mapping"

? Not great either, I guess.