From 527a07e176eab0f61b1beec9e29b99c9a5ec219f Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Wed, 15 Nov 2023 13:17:15 +0100 Subject: devlink: Add device lock assert in reload operation Add an assert to verify that the device lock is always held throughout reload operations. Tested the following flows with netdevsim and mlxsw while lockdep is enabled: netdevsim: # echo "10 1" > /sys/bus/netdevsim/new_device # devlink dev reload netdevsim/netdevsim10 # ip netns add bla # devlink dev reload netdevsim/netdevsim10 netns bla # ip netns del bla # echo 10 > /sys/bus/netdevsim/del_device mlxsw: # devlink dev reload pci/0000:01:00.0 # ip netns add bla # devlink dev reload pci/0000:01:00.0 netns bla # ip netns del bla # echo 1 > /sys/bus/pci/devices/0000\:01\:00.0/remove # echo 1 > /sys/bus/pci/rescan Signed-off-by: Ido Schimmel Reviewed-by: Jiri Pirko Signed-off-by: Petr Machata Reviewed-by: Simon Horman Signed-off-by: David S. Miller --- net/devlink/dev.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'net/devlink/dev.c') diff --git a/net/devlink/dev.c b/net/devlink/dev.c index 4fc7adb32663..ea6a92f2e6a2 100644 --- a/net/devlink/dev.c +++ b/net/devlink/dev.c @@ -4,6 +4,7 @@ * Copyright (c) 2016 Jiri Pirko */ +#include #include #include #include "devl_internal.h" @@ -433,6 +434,13 @@ int devlink_reload(struct devlink *devlink, struct net *dest_net, struct net *curr_net; int err; + /* Make sure the reload operations are invoked with the device lock + * held to allow drivers to trigger functionality that expects it + * (e.g., PCI reset) and to close possible races between these + * operations and probe/remove. + */ + device_lock_assert(devlink->dev); + memcpy(remote_reload_stats, devlink->stats.remote_reload_stats, sizeof(remote_reload_stats)); -- cgit v1.2.3 From 9b2348e2d6c94146f50b68d7d2067146e7339ac5 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Tue, 28 Nov 2023 12:52:55 +0100 Subject: devlink: warn about existing entities during reload-reinit During reload-reinit, all entities except for params, resources, regions and health reporter should be removed and re-added. Add a warning to be triggered in case the driver behaves differently. Signed-off-by: Jiri Pirko Reviewed-by: Przemek Kitszel Signed-off-by: Paolo Abeni --- net/devlink/dev.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'net/devlink/dev.c') diff --git a/net/devlink/dev.c b/net/devlink/dev.c index ea6a92f2e6a2..918a0395b03e 100644 --- a/net/devlink/dev.c +++ b/net/devlink/dev.c @@ -425,6 +425,18 @@ static void devlink_reload_netns_change(struct devlink *devlink, devlink_rel_nested_in_notify(devlink); } +static void devlink_reload_reinit_sanity_check(struct devlink *devlink) +{ + WARN_ON(!list_empty(&devlink->trap_policer_list)); + WARN_ON(!list_empty(&devlink->trap_group_list)); + WARN_ON(!list_empty(&devlink->trap_list)); + WARN_ON(!list_empty(&devlink->dpipe_table_list)); + WARN_ON(!list_empty(&devlink->sb_list)); + WARN_ON(!list_empty(&devlink->rate_list)); + WARN_ON(!list_empty(&devlink->linecard_list)); + WARN_ON(!xa_empty(&devlink->ports)); +} + int devlink_reload(struct devlink *devlink, struct net *dest_net, enum devlink_reload_action action, enum devlink_reload_limit limit, @@ -452,8 +464,10 @@ int devlink_reload(struct devlink *devlink, struct net *dest_net, if (dest_net && !net_eq(dest_net, curr_net)) devlink_reload_netns_change(devlink, curr_net, dest_net); - if (action == DEVLINK_RELOAD_ACTION_DRIVER_REINIT) + if (action == DEVLINK_RELOAD_ACTION_DRIVER_REINIT) { devlink_params_driverinit_load_new(devlink); + devlink_reload_reinit_sanity_check(devlink); + } err = devlink->ops->reload_up(devlink, action, limit, actions_performed, extack); devlink_reload_failed_set(devlink, !!err); -- cgit v1.2.3 From 337ad364c48a0db7cedb5abb8d5e9163792fd596 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Sat, 16 Dec 2023 13:29:53 +0100 Subject: devlink: use devl_is_registered() helper instead xa_get_mark() Instead of checking the xarray mark directly using xa_get_mark() helper use devl_is_registered() helper which wraps it up. Note that there are couple more users of xa_get_mark() left which are going to be handled by the next patch. Signed-off-by: Jiri Pirko Signed-off-by: Paolo Abeni --- net/devlink/dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net/devlink/dev.c') diff --git a/net/devlink/dev.c b/net/devlink/dev.c index 918a0395b03e..3fe93c8a9fe2 100644 --- a/net/devlink/dev.c +++ b/net/devlink/dev.c @@ -202,7 +202,7 @@ static void devlink_notify(struct devlink *devlink, enum devlink_command cmd) int err; WARN_ON(cmd != DEVLINK_CMD_NEW && cmd != DEVLINK_CMD_DEL); - WARN_ON(!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED)); + WARN_ON(!devl_is_registered(devlink)); msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); if (!msg) @@ -999,7 +999,7 @@ static void __devlink_flash_update_notify(struct devlink *devlink, cmd != DEVLINK_CMD_FLASH_UPDATE_END && cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS); - if (!xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED)) + if (!devl_is_registered(devlink)) return; msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); -- cgit v1.2.3 From cddbff470e3318834af518168d3a917b6e975062 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Sat, 16 Dec 2023 13:29:55 +0100 Subject: devlink: send notifications only if there are listeners Introduce devlink_nl_notify_need() helper and using it to check at the beginning of notification functions to avoid overhead of composing notification messages in case nobody listens. Signed-off-by: Jiri Pirko Signed-off-by: Paolo Abeni --- net/devlink/dev.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'net/devlink/dev.c') diff --git a/net/devlink/dev.c b/net/devlink/dev.c index 3fe93c8a9fe2..63fe3e02c928 100644 --- a/net/devlink/dev.c +++ b/net/devlink/dev.c @@ -204,6 +204,9 @@ static void devlink_notify(struct devlink *devlink, enum devlink_command cmd) WARN_ON(cmd != DEVLINK_CMD_NEW && cmd != DEVLINK_CMD_DEL); WARN_ON(!devl_is_registered(devlink)); + if (!devlink_nl_notify_need(devlink)) + return; + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); if (!msg) return; @@ -999,7 +1002,7 @@ static void __devlink_flash_update_notify(struct devlink *devlink, cmd != DEVLINK_CMD_FLASH_UPDATE_END && cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS); - if (!devl_is_registered(devlink)) + if (!devl_is_registered(devlink) || !devlink_nl_notify_need(devlink)) return; msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); -- cgit v1.2.3 From 5648de0b1f2b68bffce9bdd49a276607b9a3e3d4 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Sat, 16 Dec 2023 13:29:56 +0100 Subject: devlink: introduce a helper for netlink multicast send Introduce a helper devlink_nl_notify_send() so each object notification function does not have to call genlmsg_multicast_netns() with the same arguments. Signed-off-by: Jiri Pirko Signed-off-by: Paolo Abeni --- net/devlink/dev.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'net/devlink/dev.c') diff --git a/net/devlink/dev.c b/net/devlink/dev.c index 63fe3e02c928..19dbf540748a 100644 --- a/net/devlink/dev.c +++ b/net/devlink/dev.c @@ -217,8 +217,7 @@ static void devlink_notify(struct devlink *devlink, enum devlink_command cmd) return; } - genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), - msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL); + devlink_nl_notify_send(devlink, msg); } int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info) @@ -1013,8 +1012,7 @@ static void __devlink_flash_update_notify(struct devlink *devlink, if (err) goto out_free_msg; - genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), - msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL); + devlink_nl_notify_send(devlink, msg); return; out_free_msg: -- cgit v1.2.3