[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250730182511.4059693-1-skhawaja@google.com>
Date: Wed, 30 Jul 2025 18:25:11 +0000
From: Samiullah Khawaja <skhawaja@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>, "David S . Miller " <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, willemb@...gle.com
Cc: almasrymina@...gle.com, netdev@...r.kernel.org, skhawaja@...gle.com,
Joe Damato <joe@...a.to>
Subject: [PATCH net-next v2] net: Update threaded state in napi config in netif_set_threaded
Commit 2677010e7793 ("Add support to set NAPI threaded for individual
NAPI") added support to enable/disable threaded napi using netlink. This
also extended the napi config save/restore functionality to set the napi
threaded state. This breaks netdev reset for drivers that use napi
threaded at device level and also use napi config save/restore on
napi_disable/napi_enable. Basically on netdev with napi threaded enabled
at device level, a napi_enable call will get stuck trying to stop the
napi kthread. This is because the napi->config->threaded is set to
disabled when threaded is enabled at device level.
The issue can be reproduced on virtio-net device using qemu. To
reproduce the issue run following,
echo 1 > /sys/class/net/threaded
ethtool -L eth0 combined 1
Update the threaded state in napi config in netif_set_threaded and add a
new test that verifies this scenario.
Tested on qemu with virtio-net:
NETIF=eth0 ./tools/testing/selftests/drivers/net/napi_threaded.py
TAP version 13
1..2
ok 1 napi_threaded.change_num_queues
ok 2 napi_threaded.enable_dev_threaded_disable_napi_threaded
# Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
Fixes: 2677010e7793 ("Add support to set NAPI threaded for individual NAPI")
Signed-off-by: Samiullah Khawaja <skhawaja@...gle.com>
---
v2:
- Setting napi threaded state in napi config when enabled at device
level instead of skipping napi_set_threaded in napi_restore_config.
- Added a new test in tools/testing/selftest/drivers/net.
---
net/core/dev.c | 3 +
.../selftests/drivers/net/napi_threaded.py | 108 ++++++++++++++++++
2 files changed, 111 insertions(+)
create mode 100755 tools/testing/selftests/drivers/net/napi_threaded.py
diff --git a/net/core/dev.c b/net/core/dev.c
index 1c6e755841ce..1abba4fc1eec 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7023,6 +7023,9 @@ int netif_set_threaded(struct net_device *dev,
* This should not cause hiccups/stalls to the live traffic.
*/
list_for_each_entry(napi, &dev->napi_list, dev_list) {
+ if (napi->config)
+ napi->config->threaded = threaded;
+
if (!threaded && napi->thread)
napi_stop_kthread(napi);
else
diff --git a/tools/testing/selftests/drivers/net/napi_threaded.py b/tools/testing/selftests/drivers/net/napi_threaded.py
new file mode 100755
index 000000000000..f2168864568e
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/napi_threaded.py
@@ -0,0 +1,108 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""
+Test napi threaded states.
+"""
+
+from lib.py import ksft_run, ksft_exit
+from lib.py import ksft_eq, ksft_ne
+from lib.py import NetDrvEnv, NetdevFamily
+from lib.py import cmd, defer, ip
+
+
+def _assert_napi_threaded_enabled(nl, napi_id) -> None:
+ napi = nl.napi_get({'id': napi_id})
+ ksft_eq(napi['threaded'], 'enabled')
+ ksft_ne(napi.get('pid'), None)
+
+
+def _assert_napi_threaded_disabled(nl, napi_id) -> None:
+ napi = nl.napi_get({'id': napi_id})
+ ksft_eq(napi['threaded'], 'disabled')
+ ksft_eq(napi.get('pid'), None)
+
+
+def _set_threaded_state(cfg, threaded) -> None:
+ cmd(f"echo {threaded} > /sys/class/net/{cfg.ifname}/threaded")
+
+
+def enable_dev_threaded_disable_napi_threaded(cfg, nl) -> None:
+ """
+ Test that when napi threaded is enabled at device level and
+ then disabled at napi level for one napi, the threaded state
+ of all napis is preserved after a change in number of queues.
+ """
+
+ ip(f"link set dev {cfg.ifname} up")
+
+ napis = nl.napi_get({'ifindex': cfg.ifindex}, dump=True)
+ ksft_eq(len(napis), 2)
+
+ napi0_id = napis[0]['id']
+ napi1_id = napis[1]['id']
+
+ threaded = cmd(f"cat /sys/class/net/{cfg.ifname}/threaded").stdout
+ defer(_set_threaded_state, cfg, threaded)
+
+ # set threaded
+ _set_threaded_state(cfg, 1)
+
+ # check napi threaded is set for both napis
+ _assert_napi_threaded_enabled(nl, napi0_id)
+ _assert_napi_threaded_enabled(nl, napi1_id)
+
+ # disable threaded for napi1
+ nl.napi_set({'id': napi1_id, 'threaded': 'disabled'})
+
+ cmd(f"ethtool -L {cfg.ifname} combined 1")
+ cmd(f"ethtool -L {cfg.ifname} combined 2")
+ _assert_napi_threaded_enabled(nl, napi0_id)
+ _assert_napi_threaded_disabled(nl, napi1_id)
+
+
+def change_num_queues(cfg, nl) -> None:
+ """
+ Test that when napi threaded is enabled at device level,
+ the napi threaded state is preserved after a change in
+ number of queues.
+ """
+
+ ip(f"link set dev {cfg.ifname} up")
+
+ napis = nl.napi_get({'ifindex': cfg.ifindex}, dump=True)
+ ksft_eq(len(napis), 2)
+
+ napi0_id = napis[0]['id']
+ napi1_id = napis[1]['id']
+
+ threaded = cmd(f"cat /sys/class/net/{cfg.ifname}/threaded").stdout
+ defer(_set_threaded_state, cfg, threaded)
+
+ # set threaded
+ _set_threaded_state(cfg, 1)
+
+ # check napi threaded is set for both napis
+ _assert_napi_threaded_enabled(nl, napi0_id)
+ _assert_napi_threaded_enabled(nl, napi1_id)
+
+ cmd(f"ethtool -L {cfg.ifname} combined 1")
+ cmd(f"ethtool -L {cfg.ifname} combined 2")
+
+ # check napi threaded is set for both napis
+ _assert_napi_threaded_enabled(nl, napi0_id)
+ _assert_napi_threaded_enabled(nl, napi1_id)
+
+
+def main() -> None:
+ """ Ksft boiler plate main """
+
+ with NetDrvEnv(__file__, queue_count=2) as cfg:
+ ksft_run([change_num_queues,
+ enable_dev_threaded_disable_napi_threaded],
+ args=(cfg, NetdevFamily()))
+ ksft_exit()
+
+
+if __name__ == "__main__":
+ main()
base-commit: fa582ca7e187a15e772e6a72fe035f649b387a60
--
2.50.1.552.g942d659e1b-goog
Powered by blists - more mailing lists