Commit e6d76268 authored by Samiullah Khawaja's avatar Samiullah Khawaja Committed by Jakub Kicinski
Browse files

net: Update threaded state in napi config in netif_set_threaded 



Commit 2677010e ("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: 2677010e ("Add support to set NAPI threaded for individual NAPI")
Signed-off-by: default avatarSamiullah Khawaja <skhawaja@google.com>
Link: https://patch.msgid.link/20250804164457.2494390-1-skhawaja@google.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 8d22aea8

net/core/dev.c

+9 −17
Original line number Diff line number Diff line
@@ -6978,6 +6978,12 @@ int napi_set_threaded(struct napi_struct *napi, 
	if (napi->config)

		napi->config->threaded = threaded;



	/* Setting/unsetting threaded mode on a napi might not immediately

	 * take effect, if the current napi instance is actively being

	 * polled. In this case, the switch between threaded mode and

	 * softirq mode will happen in the next round of napi_schedule().

	 * This should not cause hiccups/stalls to the live traffic.

	 */

	if (!threaded && napi->thread) {

		napi_stop_kthread(napi);

	} else {
@@ -7011,23 +7017,9 @@ int netif_set_threaded(struct net_device *dev, 


	WRITE_ONCE(dev->threaded, threaded);



	/* Make sure kthread is created before THREADED bit

	 * is set.

	 */

	smp_mb__before_atomic();



	/* Setting/unsetting threaded mode on a napi might not immediately

	 * take effect, if the current napi instance is actively being

	 * polled. In this case, the switch between threaded mode and

	 * softirq mode will happen in the next round of napi_schedule().

	 * This should not cause hiccups/stalls to the live traffic.

	 */

	list_for_each_entry(napi, &dev->napi_list, dev_list) {

		if (!threaded && napi->thread)

			napi_stop_kthread(napi);

		else

			assign_bit(NAPI_STATE_THREADED, &napi->state, threaded);

	}

	/* The error should not occur as the kthreads are already created. */

	list_for_each_entry(napi, &dev->napi_list, dev_list)

		WARN_ON_ONCE(napi_set_threaded(napi, threaded));



	return err;

}

tools/testing/selftests/drivers/net/Makefile

+1 −0
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ TEST_GEN_FILES := \ 


TEST_PROGS := \

	napi_id.py \

	napi_threaded.py \

	netcons_basic.sh \

	netcons_cmdline.sh \

	netcons_fragmented_msg.sh \

tools/testing/selftests/drivers/net/napi_threaded.py

0 → 100755
+111 −0
Original line number Diff line number Diff line 
#!/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, ksft_ge

from lib.py import NetDrvEnv, NetdevFamily

from lib.py import cmd, defer, ethtool





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 _setup_deferred_cleanup(cfg) -> None:

    combined = ethtool(f"-l {cfg.ifname}", json=True)[0].get("combined", 0)

    ksft_ge(combined, 2)

    defer(ethtool, f"-L {cfg.ifname} combined {combined}")



    threaded = cmd(f"cat /sys/class/net/{cfg.ifname}/threaded").stdout

    defer(_set_threaded_state, cfg, 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.

    """



    napis = nl.napi_get({'ifindex': cfg.ifindex}, dump=True)

    ksft_ge(len(napis), 2)



    napi0_id = napis[0]['id']

    napi1_id = napis[1]['id']



    _setup_deferred_cleanup(cfg)



    # 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.

    """



    napis = nl.napi_get({'ifindex': cfg.ifindex}, dump=True)

    ksft_ge(len(napis), 2)



    napi0_id = napis[0]['id']

    napi1_id = napis[1]['id']



    _setup_deferred_cleanup(cfg)



    # 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()