Merge branch 'net-prevent-deadlocks-and-mis-configuration-with-per-napi-threaded-config'

Jakub Kicinski says:

====================
net: prevent deadlocks and mis-configuration with per-NAPI threaded config

Running the test added with a recent fix on a driver with persistent
NAPI config leads to a deadlock. The deadlock is fixed by patch 3,
patch 2 is I think a more fundamental problem with the way we
implemented the config.

I hope the fix makes sense, my own thinking is definitely colored
by my preference (IOW how the per-queue config RFC was implemented).

v1: https://lore.kernel.org/20250808014952.724762-1-kuba@kernel.org
====================

Link: https://patch.msgid.link/20250809001205.1147153-1-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Paolo Abeni 2025-08-12 14:43:08 +02:00
commit 74078816f8
4 changed files with 27 additions and 8 deletions

View file

@ -2071,6 +2071,8 @@ enum netdev_reg_state {
* @max_pacing_offload_horizon: max EDT offload horizon in nsec.
* @napi_config: An array of napi_config structures containing per-NAPI
* settings.
* @num_napi_configs: number of allocated NAPI config structs,
* always >= max(num_rx_queues, num_tx_queues).
* @gro_flush_timeout: timeout for GRO layer in NAPI
* @napi_defer_hard_irqs: If not zero, provides a counter that would
* allow to avoid NIC hard IRQ, on busy queues.
@ -2482,8 +2484,9 @@ struct net_device {
u64 max_pacing_offload_horizon;
struct napi_config *napi_config;
unsigned long gro_flush_timeout;
u32 num_napi_configs;
u32 napi_defer_hard_irqs;
unsigned long gro_flush_timeout;
/**
* @up: copy of @state's IFF_UP, but safe to read with just @lock.

View file

@ -6999,7 +6999,7 @@ int netif_set_threaded(struct net_device *dev,
enum netdev_napi_threaded threaded)
{
struct napi_struct *napi;
int err = 0;
int i, err = 0;
netdev_assert_locked_or_invisible(dev);
@ -7021,6 +7021,10 @@ int netif_set_threaded(struct net_device *dev,
list_for_each_entry(napi, &dev->napi_list, dev_list)
WARN_ON_ONCE(napi_set_threaded(napi, threaded));
/* Override the config for all NAPIs even if currently not listed */
for (i = 0; i < dev->num_napi_configs; i++)
dev->napi_config[i].threaded = threaded;
return err;
}
@ -7353,8 +7357,9 @@ void netif_napi_add_weight_locked(struct net_device *dev,
* Clear dev->threaded if kthread creation failed so that
* threaded mode will not be enabled in napi_enable().
*/
if (dev->threaded && napi_kthread_create(napi))
dev->threaded = NETDEV_NAPI_THREADED_DISABLED;
if (napi_get_threaded_config(dev, napi))
if (napi_kthread_create(napi))
dev->threaded = NETDEV_NAPI_THREADED_DISABLED;
netif_napi_set_irq_locked(napi, -1);
}
EXPORT_SYMBOL(netif_napi_add_weight_locked);
@ -11873,6 +11878,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
goto free_all;
dev->cfg_pending = dev->cfg;
dev->num_napi_configs = maxqs;
napi_config_sz = array_size(maxqs, sizeof(*dev->napi_config));
dev->napi_config = kvzalloc(napi_config_sz, GFP_KERNEL_ACCOUNT);
if (!dev->napi_config)

View file

@ -323,6 +323,14 @@ static inline enum netdev_napi_threaded napi_get_threaded(struct napi_struct *n)
return NETDEV_NAPI_THREADED_DISABLED;
}
static inline enum netdev_napi_threaded
napi_get_threaded_config(struct net_device *dev, struct napi_struct *n)
{
if (n->config)
return n->config->threaded;
return dev->threaded;
}
int napi_set_threaded(struct napi_struct *n,
enum netdev_napi_threaded threaded);

View file

@ -35,6 +35,8 @@ def _setup_deferred_cleanup(cfg) -> None:
threaded = cmd(f"cat /sys/class/net/{cfg.ifname}/threaded").stdout
defer(_set_threaded_state, cfg, threaded)
return combined
def enable_dev_threaded_disable_napi_threaded(cfg, nl) -> None:
"""
@ -49,7 +51,7 @@ def enable_dev_threaded_disable_napi_threaded(cfg, nl) -> None:
napi0_id = napis[0]['id']
napi1_id = napis[1]['id']
_setup_deferred_cleanup(cfg)
qcnt = _setup_deferred_cleanup(cfg)
# set threaded
_set_threaded_state(cfg, 1)
@ -62,7 +64,7 @@ def enable_dev_threaded_disable_napi_threaded(cfg, nl) -> None:
nl.napi_set({'id': napi1_id, 'threaded': 'disabled'})
cmd(f"ethtool -L {cfg.ifname} combined 1")
cmd(f"ethtool -L {cfg.ifname} combined 2")
cmd(f"ethtool -L {cfg.ifname} combined {qcnt}")
_assert_napi_threaded_enabled(nl, napi0_id)
_assert_napi_threaded_disabled(nl, napi1_id)
@ -80,7 +82,7 @@ def change_num_queues(cfg, nl) -> None:
napi0_id = napis[0]['id']
napi1_id = napis[1]['id']
_setup_deferred_cleanup(cfg)
qcnt = _setup_deferred_cleanup(cfg)
# set threaded
_set_threaded_state(cfg, 1)
@ -90,7 +92,7 @@ def change_num_queues(cfg, nl) -> None:
_assert_napi_threaded_enabled(nl, napi1_id)
cmd(f"ethtool -L {cfg.ifname} combined 1")
cmd(f"ethtool -L {cfg.ifname} combined 2")
cmd(f"ethtool -L {cfg.ifname} combined {qcnt}")
# check napi threaded is set for both napis
_assert_napi_threaded_enabled(nl, napi0_id)