[PATCH net v2] net: gro_cells: prevent enabling threaded NAPI on gro_cells

[PATCH net v2] net: gro_cells: prevent enabling threaded NAPI on gro_cells

[Naman Gulati]

gro_cells allocates a per-CPU struct gro_cell with per-CPU queues
(cell->napi_skbs), per-CPU local_lock_t (cell->bh_lock), and per-CPU
napi_struct (cell->napi). On !CONFIG_PREEMPT_RT, local_lock_t only
provides lockdep annotation (and is a complete no-op when
CONFIG_DEBUG_LOCK_ALLOC is disabled), relying on per-CPU locality and
disabled BH context for mutual exclusion; it does not provide cross-CPU
synchronization.

When threaded NAPI is enabled on a device backed by gro_cells (e.g.
gre0, vxlan, geneve) via sysfs, unbound kernel threads are created for
each per-CPU NAPI. When gro_cells_receive() on CPU A enqueues a packet
to cell_A and calls napi_schedule(&cell_A->napi) while holding
cell_A->bh_lock, the woken kthread can run concurrently on remote CPU B.

When CPU B runs gro_cell_poll(&cell_A->napi) and calls
__local_lock_nested_bh(&cell_A->bh_lock), lockdep detects that the lock
is already held by CPU A's task and triggers a warning:

  WARNING: CPU: 1 PID: 377 at net/core/gro_cells.c:66 gro_cell_poll
  DEBUG_LOCKS_WARN_ON(l->owner)
  CPU: 1 UID: 0 PID: 377 Comm: napi/gre0-0 Not tainted 7.3.0-rc2
  Call Trace:
   <TASK>
   __local_lock_nested_bh include/linux/local_lock_internal.h:92
   gro_cell_poll+0x5aa/0x6c0 net/core/gro_cells.c:66
   napi_threaded_poll+0x39b/0x600 net/core/dev.c:7048
   kthread+0x71e/0x870 kernel/kthread.c:464
   ret_from_fork+0x5d/0x90 arch/x86/kernel/process.c:167
   ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:264
   </TASK>

On !CONFIG_PREEMPT_RT kernels without lockdep, concurrent execution of
__skb_queue_tail() on CPU A and __skb_dequeue() on CPU B without
cross-CPU locks leads to silent queue corruption.

Virtual per-CPU NAPIs (like gro_cells) cannot support unbound threaded
NAPI or busypolling. For NAPIs that set NAPI_STATE_NO_BUSY_POLL,
napi_hash_add() skips adding its NAPIs to napi_hash and assigning a
valid NAPI ID, which already prevents enabling threaded NAPI on them via
Netlink.

Check NAPI_STATE_NO_BUSY_POLL in modify_napi_threaded(), and fail early
with -EOPNOTSUPP when trying to enable threaded NAPI via sysfs on
netdevs where all NAPIs have NAPI_STATE_NO_BUSY_POLL set. A more
fundamental check like napi_id_valid(napi->napi_id) can potentially
break userspace that tries to enable threaded napi on netdevs that are
down. This check brings threaded napi control via sysfs closer to
netlink.

Fixes: 25718fdcbdd2 ("net: gro_cells: Use nested-BH locking for gro_cell")
Fixes: 29863d41bb6e ("net: implement threaded-able napi poll loop support")
Reported-by: syzbot+f0661448aa9511ce744a@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6aad6d7b.0c43d342.320d00.0001.GAE@google.com
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Signed-off-by: Naman Gulati <namangulati@google.com>

---
v2:
  - Check NAPI_STATE_NO_BUSY_POLL in modify_napi_threaded() instead of
    adding NAPI_STATE_NO_THREADED (Jakub).
  - v1: https://lore.kernel.org/all/20260918173413.3222413-1-namangulati@google.com/
---
 net/core/net-sysfs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 352173df7578..286c7b4139df 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -743,17 +743,17 @@ static ssize_t threaded_show(struct device *dev,
 
 static int modify_napi_threaded(struct net_device *dev, unsigned long val)
 {
-	int ret;
-
-	if (list_empty(&dev->napi_list))
-		return -EOPNOTSUPP;
+	struct napi_struct *napi;
 
 	if (val != 0 && val != 1)
 		return -EOPNOTSUPP;
 
-	ret = netif_set_threaded(dev, val);
+	list_for_each_entry(napi, &dev->napi_list, dev_list) {
+		if (!test_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state))
+			return netif_set_threaded(dev, val);
+	}
 
-	return ret;
+	return -EOPNOTSUPP;
 }
 
 static ssize_t threaded_store(struct device *dev,
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH net v2] net: gro_cells: prevent enabling threaded NAPI on gro_cells

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The rewritten modify_napi_threaded() (net/core/net-sysfs.c) uses
  NAPI_STATE_NO_BUSY_POLL to mean "this NAPI cannot be threaded".
- [Medium] The commit message says the patch makes the write fail with
  -EOPNOTSUPP "when trying to enable threaded NAPI via sysfs" on netdevs…

--

net: gro_cells: prevent enabling threaded NAPI on gro_cells

This patch changes modify_napi_threaded() in net/core/net-sysfs.c.
A write to /sys/class/net/<dev>/threaded now reaches netif_set_threaded()
only if at least one NAPI on the device lacks NAPI_STATE_NO_BUSY_POLL.
The goal is to stop threaded NAPI from being enabled on per-CPU gro_cells
NAPIs, where the local_lock does not provide cross-CPU exclusion.

Closes: https://lore.kernel.org/netdev/6aad6d7b.0c43d342.320d00.0001.GAE@google.com

> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index 352173df75785..286c7b4139df1 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -743,17 +743,17 @@ static ssize_t threaded_show(struct device *dev,
>  
>  static int modify_napi_threaded(struct net_device *dev, unsigned long val)
>  {
> -	int ret;
> -
> -	if (list_empty(&dev->napi_list))
> -		return -EOPNOTSUPP;
> +	struct napi_struct *napi;
>  
>  	if (val != 0 && val != 1)
>  		return -EOPNOTSUPP;
>  
> -	ret = netif_set_threaded(dev, val);
> +	list_for_each_entry(napi, &dev->napi_list, dev_list) {
> +		if (!test_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state))
> +			return netif_set_threaded(dev, val);
> +	}
>  
> -	return ret;
> +	return -EOPNOTSUPP;
>  }

[Severity: High]
Does this break the threaded control on WireGuard devices?

WireGuard sets NAPI_STATE_NO_BUSY_POLL on every per-peer NAPI. It also
runs those NAPIs threaded by default:

drivers/net/wireguard/peer.c:wg_peer_create() {
    ...
	set_bit(NAPI_STATE_NO_BUSY_POLL, &peer->napi.state);
	netif_napi_add(wg->dev, &peer->napi, wg_packet_rx_poll);
	napi_enable(&peer->napi);
    ...
}

drivers/net/wireguard/device.c:wg_newlink() {
    ...
	netif_threaded_enable(dev);
	ret = register_netdevice(dev);
    ...
}

Once a peer has been added, the new loop never finds a NAPI without
NO_BUSY_POLL. Writing either 0 or 1 to /sys/class/net/wg0/threaded then
returns -EOPNOTSUPP:

threaded_store()->netdev_lock_store()->modify_napi_threaded()

Before this patch, a non-empty napi_list reached netif_set_threaded().
A call to netif_set_threaded(dev, 0) succeeded and stopped the kthreads.

Netlink does not offer another way to change this. napi_hash_add() skips
NO_BUSY_POLL NAPIs, so WireGuard NAPIs never get a NAPI ID. netdev-genl
exposes threaded only per NAPI, through napi-set.

As a result, threaded_show() still reports 1 on WireGuard, but users can
no longer turn threaded mode off. The ABI document for this file says:

    User could set this value to enable/disable threaded mode for all
    napi belonging to this device

The gro_cells problem comes from per-CPU cells protected by a local_lock,
not from NO_BUSY_POLL itself. Would a dedicated marker for NAPIs that
cannot be threaded work better here, similar to the NAPI_STATE_NO_THREADED
approach in v1? The patch carries Fixes: tags, so this change would also
reach stable kernels.

[Severity: Medium]
Should the NO_BUSY_POLL check apply only when val is 1?

The commit message says:

    Check NAPI_STATE_NO_BUSY_POLL in modify_napi_threaded(), and fail early
    with -EOPNOTSUPP when trying to enable threaded NAPI via sysfs on
    netdevs where all NAPIs have NAPI_STATE_NO_BUSY_POLL set.

The loop also runs when val == 0, though. gro_cells_init() sets the flag
on every cell NAPI:

net/core/gro_cells.c:gro_cells_init() {
    ...
		set_bit(NAPI_STATE_NO_BUSY_POLL, &cell->napi.state);
    ...
}

On gre, vxlan, geneve and other devices backed only by gro_cells, writing
0 to threaded used to reach netif_set_threaded(dev, 0) and succeed as a
no-op. It now returns -EOPNOTSUPP.

The commit message avoids napi_id_valid() because it "can potentially
break userspace". Could this change also break scripts that write 0 to
threaded on every interface? If the change is intended, could the commit
message mention it?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929010058.4063305-1-namangulati%40google.com

Re: [PATCH net v2] net: gro_cells: prevent enabling threaded NAPI on gro_cells

[Naman Gulati]

On Thu, Oct 1, 2026 at 6:03 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] The rewritten modify_napi_threaded() (net/core/net-sysfs.c) uses
>   NAPI_STATE_NO_BUSY_POLL to mean "this NAPI cannot be threaded".
> - [Medium] The commit message says the patch makes the write fail with
>   -EOPNOTSUPP "when trying to enable threaded NAPI via sysfs" on netdevs…
>
> --
>
> net: gro_cells: prevent enabling threaded NAPI on gro_cells
>
> This patch changes modify_napi_threaded() in net/core/net-sysfs.c.
> A write to /sys/class/net/<dev>/threaded now reaches netif_set_threaded()
> only if at least one NAPI on the device lacks NAPI_STATE_NO_BUSY_POLL.
> The goal is to stop threaded NAPI from being enabled on per-CPU gro_cells
> NAPIs, where the local_lock does not provide cross-CPU exclusion.
>
> Closes: https://lore.kernel.org/netdev/6aad6d7b.0c43d342.320d00.0001.GAE@google.com
>
> > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> > index 352173df75785..286c7b4139df1 100644
> > --- a/net/core/net-sysfs.c
> > +++ b/net/core/net-sysfs.c
> > @@ -743,17 +743,17 @@ static ssize_t threaded_show(struct device *dev,
> >
> >  static int modify_napi_threaded(struct net_device *dev, unsigned long val)
> >  {
> > -     int ret;
> > -
> > -     if (list_empty(&dev->napi_list))
> > -             return -EOPNOTSUPP;
> > +     struct napi_struct *napi;
> >
> >       if (val != 0 && val != 1)
> >               return -EOPNOTSUPP;
> >
> > -     ret = netif_set_threaded(dev, val);
> > +     list_for_each_entry(napi, &dev->napi_list, dev_list) {
> > +             if (!test_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state))
> > +                     return netif_set_threaded(dev, val);
> > +     }
> >
> > -     return ret;
> > +     return -EOPNOTSUPP;
> >  }
>
> [Severity: High]
> Does this break the threaded control on WireGuard devices?
>
> WireGuard sets NAPI_STATE_NO_BUSY_POLL on every per-peer NAPI. It also
> runs those NAPIs threaded by default:
>
> drivers/net/wireguard/peer.c:wg_peer_create() {
>     ...
>         set_bit(NAPI_STATE_NO_BUSY_POLL, &peer->napi.state);
>         netif_napi_add(wg->dev, &peer->napi, wg_packet_rx_poll);
>         napi_enable(&peer->napi);
>     ...
> }
>
> drivers/net/wireguard/device.c:wg_newlink() {
>     ...
>         netif_threaded_enable(dev);
>         ret = register_netdevice(dev);
>     ...
> }
>
> Once a peer has been added, the new loop never finds a NAPI without
> NO_BUSY_POLL. Writing either 0 or 1 to /sys/class/net/wg0/threaded then
> returns -EOPNOTSUPP:
>
> threaded_store()->netdev_lock_store()->modify_napi_threaded()
>
> Before this patch, a non-empty napi_list reached netif_set_threaded().
> A call to netif_set_threaded(dev, 0) succeeded and stopped the kthreads.
>
> Netlink does not offer another way to change this. napi_hash_add() skips
> NO_BUSY_POLL NAPIs, so WireGuard NAPIs never get a NAPI ID. netdev-genl
> exposes threaded only per NAPI, through napi-set.
>
> As a result, threaded_show() still reports 1 on WireGuard, but users can
> no longer turn threaded mode off. The ABI document for this file says:
>
>     User could set this value to enable/disable threaded mode for all
>     napi belonging to this device
>
> The gro_cells problem comes from per-CPU cells protected by a local_lock,
> not from NO_BUSY_POLL itself. Would a dedicated marker for NAPIs that
> cannot be threaded work better here, similar to the NAPI_STATE_NO_THREADED
> approach in v1? The patch carries Fixes: tags, so this change would also
> reach stable kernels.

This and the below finding both stem from not being able to set
threaded = 0. I missed handling this. Wireguard setting threaded napi
by default along with NAPI_STATE_NO_BUSY_POLL deffinitely complicates
this further. It might become an overengineered fix to tighten the
loose sysfs interface when netlink already provides stricter control.
It may be a better to just update the gro_cells documentation to state
that it does not support threaded napis. May respin accordingly if I
cannot find a clean way to tighten this interface.

>
> [Severity: Medium]
> Should the NO_BUSY_POLL check apply only when val is 1?
>
> The commit message says:
>
>     Check NAPI_STATE_NO_BUSY_POLL in modify_napi_threaded(), and fail early
>     with -EOPNOTSUPP when trying to enable threaded NAPI via sysfs on
>     netdevs where all NAPIs have NAPI_STATE_NO_BUSY_POLL set.
>
> The loop also runs when val == 0, though. gro_cells_init() sets the flag
> on every cell NAPI:
>
> net/core/gro_cells.c:gro_cells_init() {
>     ...
>                 set_bit(NAPI_STATE_NO_BUSY_POLL, &cell->napi.state);
>     ...
> }
>
> On gre, vxlan, geneve and other devices backed only by gro_cells, writing
> 0 to threaded used to reach netif_set_threaded(dev, 0) and succeed as a
> no-op. It now returns -EOPNOTSUPP.
>
> The commit message avoids napi_id_valid() because it "can potentially
> break userspace". Could this change also break scripts that write 0 to
> threaded on every interface? If the change is intended, could the commit
> message mention it?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929010058.4063305-1-namangulati%40google.com

Re: [PATCH net v2] net: gro_cells: prevent enabling threaded NAPI on gro_cells

[Jakub Kicinski]

On Thu, 1 Oct 2026 18:34:31 -0700 Naman Gulati wrote:
> This and the below finding both stem from not being able to set
> threaded = 0. I missed handling this. Wireguard setting threaded napi
> by default along with NAPI_STATE_NO_BUSY_POLL deffinitely complicates
> this further. It might become an overengineered fix to tighten the
> loose sysfs interface when netlink already provides stricter control.
> It may be a better to just update the gro_cells documentation to state
> that it does not support threaded napis. May respin accordingly if I
> cannot find a clean way to tighten this interface.

Ugh. Alright, thanks for trying, seems we have to return to v1-ish
approach.  Please name the flag PERCPU or something like that.