[PATCH net] net: keep the NAPI_DISABLE flag while napi is disabled

[PATCH net] net: keep the NAPI_DISABLE flag while napi is disabled

[Maxime Chevallier]

While running the napi_threaded kselftest on a variety of embedded
devices, it was found that the ksft hangs on stmmac boards. This seems
to be because stmmac creates multiple napi instances, some of them are
used exclusively for XDP (the rxtx napi).

When setting napi threaded on and off again on a disabled napi
instance, we enter an infinite loop in napi_stop_kthread() :

while(true) {
	[...]
	if (val & NAPIF_STATE_SCHED_THREADED) || ...) {
		...
	} else {
		msleep(20);
	}
}

The thing is the _STATE_SCHED_THREADED flag seems to only be set during
____napi_schedule().

So, on an unused napi loop the flag is never set, we hang in that loop.

This seems to be more general than stmmac though, there are lots of
drivers that create the napi instances in .probe(), so they still live
outside of .open()/.close(). It was verified by running the following on
an mvneta board :

  ip link set eth0 down
  echo 1 > /sys/class/net/eth0/threaded
  echo 0 > /sys/class/net/eth0/threaded
  -> hang

The proposed approach here is to extend the NAPI_DISABLE flag so that
it stays set while the napi instance isn't enabled, and use that flag
when stopping the napi kthread.

Fixes: 689883de94dd ("net: stop napi kthreads when THREADED napi is disabled")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 include/linux/netdevice.h | 2 +-
 net/core/dev.c            | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 87cafc932e9e..5ec2ac54524f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -423,7 +423,7 @@ struct napi_struct {
 enum {
 	NAPI_STATE_SCHED,		/* Poll is scheduled */
 	NAPI_STATE_MISSED,		/* reschedule a napi */
-	NAPI_STATE_DISABLE,		/* Disable pending */
+	NAPI_STATE_DISABLE,		/* Disable pending or already disabled */
 	NAPI_STATE_NPSVC,		/* Netpoll - don't dequeue from poll_list */
 	NAPI_STATE_LISTED,		/* NAPI added to system lists */
 	NAPI_STATE_NO_BUSY_POLL,	/* Do not add in napi_hash, no busy polling */
diff --git a/net/core/dev.c b/net/core/dev.c
index f660fccfc0db..7f21d87a8c7a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7229,7 +7229,8 @@ static void napi_stop_kthread(struct napi_struct *napi)
 		 * STATE_THREADED can be unset here.
 		 */
 		if ((val & NAPIF_STATE_SCHED_THREADED) ||
-		    !(val & NAPIF_STATE_SCHED)) {
+		    !(val & NAPIF_STATE_SCHED) ||
+		    (val & NAPIF_STATE_DISABLE)) {
 			new = val & (~(NAPIF_STATE_THREADED |
 				       NAPIF_STATE_THREADED_BUSY_POLL));
 		} else {
@@ -7646,6 +7647,7 @@ void netif_napi_add_weight_locked(struct net_device *dev,
 	napi->list_owner = -1;
 	set_bit(NAPI_STATE_SCHED, &napi->state);
 	set_bit(NAPI_STATE_NPSVC, &napi->state);
+	set_bit(NAPI_STATE_DISABLE, &napi->state);
 	netif_napi_dev_list_add(dev, napi);
 
 	/* default settings from sysfs are applied to all NAPIs. any per-NAPI
@@ -7694,8 +7696,6 @@ void napi_disable_locked(struct napi_struct *n)
 		napi_save_config(n);
 	else
 		napi_hash_del(n);
-
-	clear_bit(NAPI_STATE_DISABLE, &n->state);
 }
 EXPORT_SYMBOL(napi_disable_locked);
 
@@ -7727,7 +7727,8 @@ void napi_enable_locked(struct napi_struct *n)
 	do {
 		BUG_ON(!test_bit(NAPI_STATE_SCHED, &val));
 
-		new = val & ~(NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC);
+		new = val & ~(NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC |
+			      NAPIF_STATE_DISABLE);
 		if (n->dev->threaded && n->thread)
 			new |= NAPIF_STATE_THREADED;
 	} while (!try_cmpxchg(&n->state, &val, new));
-- 
2.55.0

Re: [PATCH net] net: keep the NAPI_DISABLE flag while napi is disabled

[Jakub Kicinski]

On Sat, 26 Sep 2026 21:47:13 +0200 Maxime Chevallier wrote:
> This seems to be more general than stmmac though, there are lots of
> drivers that create the napi instances in .probe(), so they still live
> outside of .open()/.close(). It was verified by running the following on
> an mvneta board :

Not sure this works, you assume ownership if DISABLE is set but we set
it to start the shutdown, not once we stopped the poller.

We've been tempted to "fix" this multiple times. I'd prefer to fix
drivers.

Re: [PATCH net] net: keep the NAPI_DISABLE flag while napi is disabled

[Maxime Chevallier]

On 9/28/26 00:41, Jakub Kicinski wrote:
> On Sat, 26 Sep 2026 21:47:13 +0200 Maxime Chevallier wrote:
>> This seems to be more general than stmmac though, there are lots of
>> drivers that create the napi instances in .probe(), so they still live
>> outside of .open()/.close(). It was verified by running the following on
>> an mvneta board :
> 
> Not sure this works, you assume ownership if DISABLE is set but we set
> it to start the shutdown, not once we stopped the poller.
> 
> We've been tempted to "fix" this multiple times. I'd prefer to fix
> drivers.

There's 2 classes of problems then

- drivers that create a napi instance but don't use it even when
admin up (stmmac, I think it used to be the case for i40e looking
at the history). Fixing the driver makes sense then

- drivers that add napi instances at .probe() time, in which
case setting threaded to 1 -> 0 when the interface is admin down
is always going to hang. There's quite a few of them :

git grep -p netif_napi_add | grep probe | wc -l
75

So at least 75 drivers call netif_napi_add from their probe
functions.

Maxime