Skip to content

Commit e93c937

Browse files
Jiri Pirkokuba-moo
Jiri Pirko
authored andcommitted
devlink: change per-devlink netdev notifier to static one
The commit 565b482 ("devlink: change port event netdev notifier from per-net to global") changed original per-net notifier to be per-devlink instance. That fixed the issue of non-receiving events of netdev uninit if that moved to a different namespace. That worked fine in -net tree. However, later on when commit ee75f1f ("net/mlx5e: Create separate devlink instance for ethernet auxiliary device") and commit 72ed5d5 ("net/mlx5: Suspend auxiliary devices only in case of PCI device suspend") were merged, a deadlock was introduced when removing a namespace with devlink instance with another nested instance. Here there is the bad flow example resulting in deadlock with mlx5: net_cleanup_work -> cleanup_net (takes down_read(&pernet_ops_rwsem) -> devlink_pernet_pre_exit() -> devlink_reload() -> mlx5_devlink_reload_down() -> mlx5_unload_one_devl_locked() -> mlx5_detach_device() -> del_adev() -> mlx5e_remove() -> mlx5e_destroy_devlink() -> devlink_free() -> unregister_netdevice_notifier() (takes down_write(&pernet_ops_rwsem) Steps to reproduce: $ modprobe mlx5_core $ ip netns add ns1 $ devlink dev reload pci/0000:08:00.0 netns ns1 $ ip netns del ns1 Resolve this by converting the notifier from per-devlink instance to a static one registered during init phase and leaving it registered forever. Use this notifier for all devlink port instances created later on. Note what a tree needs this fix only in case all of the cited fixes commits are present. Reported-by: Moshe Shemesh <[email protected]> Fixes: 565b482 ("devlink: change port event netdev notifier from per-net to global") Fixes: ee75f1f ("net/mlx5e: Create separate devlink instance for ethernet auxiliary device") Fixes: 72ed5d5 ("net/mlx5: Suspend auxiliary devices only in case of PCI device suspend") Signed-off-by: Jiri Pirko <[email protected]> Reviewed-by: Simon Horman <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 7ce93d6 commit e93c937

File tree

3 files changed

+9
-13
lines changed

3 files changed

+9
-13
lines changed

net/devlink/core.c

+7-9
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,6 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
204204
if (ret < 0)
205205
goto err_xa_alloc;
206206

207-
devlink->netdevice_nb.notifier_call = devlink_port_netdevice_event;
208-
ret = register_netdevice_notifier(&devlink->netdevice_nb);
209-
if (ret)
210-
goto err_register_netdevice_notifier;
211-
212207
devlink->dev = dev;
213208
devlink->ops = ops;
214209
xa_init_flags(&devlink->ports, XA_FLAGS_ALLOC);
@@ -233,8 +228,6 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
233228

234229
return devlink;
235230

236-
err_register_netdevice_notifier:
237-
xa_erase(&devlinks, devlink->index);
238231
err_xa_alloc:
239232
kfree(devlink);
240233
return NULL;
@@ -266,8 +259,6 @@ void devlink_free(struct devlink *devlink)
266259
xa_destroy(&devlink->params);
267260
xa_destroy(&devlink->ports);
268261

269-
WARN_ON_ONCE(unregister_netdevice_notifier(&devlink->netdevice_nb));
270-
271262
xa_erase(&devlinks, devlink->index);
272263

273264
devlink_put(devlink);
@@ -303,6 +294,10 @@ static struct pernet_operations devlink_pernet_ops __net_initdata = {
303294
.pre_exit = devlink_pernet_pre_exit,
304295
};
305296

297+
static struct notifier_block devlink_port_netdevice_nb __net_initdata = {
298+
.notifier_call = devlink_port_netdevice_event,
299+
};
300+
306301
static int __init devlink_init(void)
307302
{
308303
int err;
@@ -311,6 +306,9 @@ static int __init devlink_init(void)
311306
if (err)
312307
goto out;
313308
err = register_pernet_subsys(&devlink_pernet_ops);
309+
if (err)
310+
goto out;
311+
err = register_netdevice_notifier(&devlink_port_netdevice_nb);
314312

315313
out:
316314
WARN_ON(err);

net/devlink/devl_internal.h

-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ struct devlink {
5050
u8 reload_failed:1;
5151
refcount_t refcount;
5252
struct rcu_work rwork;
53-
struct notifier_block netdevice_nb;
5453
char priv[] __aligned(NETDEV_ALIGN);
5554
};
5655

net/devlink/leftover.c

+2-3
Original file line numberDiff line numberDiff line change
@@ -7073,10 +7073,9 @@ int devlink_port_netdevice_event(struct notifier_block *nb,
70737073
struct devlink_port *devlink_port = netdev->devlink_port;
70747074
struct devlink *devlink;
70757075

7076-
devlink = container_of(nb, struct devlink, netdevice_nb);
7077-
7078-
if (!devlink_port || devlink_port->devlink != devlink)
7076+
if (!devlink_port)
70797077
return NOTIFY_OK;
7078+
devlink = devlink_port->devlink;
70807079

70817080
switch (event) {
70827081
case NETDEV_POST_INIT:

0 commit comments

Comments
 (0)