Commit 7d7e9169 authored by Jiri Pirko's avatar Jiri Pirko Committed by David S. Miller

devlink: move devlink reload notifications back in between _down() and _up() calls

This effectively reverts commit 05a7f4a8 ("devlink: Break parameter
notification sequence to be before/after unload/load driver").

Cited commit resolved a problem in mlx5 params implementation,
when param notification code accessed memory previously freed
during reload.

Now, when the params can be registered and unregistered when devlink
instance is registered, mlx5 code unregisters the problematic param
during devlink reload. The fix is therefore no longer needed.

Current behavior is a it problematic, as it sends DEL notifications even
in potential case when reload_down() call fails which might confuse
userspace notifications listener.

So move the reload notifications back where they were originally in
between reload_down() and reload_up() calls.
Signed-off-by: default avatarJiri Pirko <jiri@nvidia.com>
Reviewed-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 166a1a5a
...@@ -4235,12 +4235,11 @@ static void devlink_param_notify(struct devlink *devlink, ...@@ -4235,12 +4235,11 @@ static void devlink_param_notify(struct devlink *devlink,
struct devlink_param_item *param_item, struct devlink_param_item *param_item,
enum devlink_command cmd); enum devlink_command cmd);
static void devlink_ns_change_notify(struct devlink *devlink, static void devlink_reload_netns_change(struct devlink *devlink,
struct net *dest_net, struct net *curr_net, struct net *curr_net,
bool new) struct net *dest_net)
{ {
struct devlink_param_item *param_item; struct devlink_param_item *param_item;
enum devlink_command cmd;
/* Userspace needs to be notified about devlink objects /* Userspace needs to be notified about devlink objects
* removed from original and entering new network namespace. * removed from original and entering new network namespace.
...@@ -4248,18 +4247,19 @@ static void devlink_ns_change_notify(struct devlink *devlink, ...@@ -4248,18 +4247,19 @@ static void devlink_ns_change_notify(struct devlink *devlink,
* reload process so the notifications are generated separatelly. * reload process so the notifications are generated separatelly.
*/ */
if (!dest_net || net_eq(dest_net, curr_net)) list_for_each_entry(param_item, &devlink->param_list, list)
return; devlink_param_notify(devlink, 0, param_item,
DEVLINK_CMD_PARAM_DEL);
devlink_notify(devlink, DEVLINK_CMD_DEL);
if (new) move_netdevice_notifier_net(curr_net, dest_net,
devlink_notify(devlink, DEVLINK_CMD_NEW); &devlink->netdevice_nb);
write_pnet(&devlink->_net, dest_net);
cmd = new ? DEVLINK_CMD_PARAM_NEW : DEVLINK_CMD_PARAM_DEL; devlink_notify(devlink, DEVLINK_CMD_NEW);
list_for_each_entry(param_item, &devlink->param_list, list) list_for_each_entry(param_item, &devlink->param_list, list)
devlink_param_notify(devlink, 0, param_item, cmd); devlink_param_notify(devlink, 0, param_item,
DEVLINK_CMD_PARAM_NEW);
if (!new)
devlink_notify(devlink, DEVLINK_CMD_DEL);
} }
static void devlink_reload_failed_set(struct devlink *devlink, static void devlink_reload_failed_set(struct devlink *devlink,
...@@ -4341,24 +4341,19 @@ int devlink_reload(struct devlink *devlink, struct net *dest_net, ...@@ -4341,24 +4341,19 @@ int devlink_reload(struct devlink *devlink, struct net *dest_net,
memcpy(remote_reload_stats, devlink->stats.remote_reload_stats, memcpy(remote_reload_stats, devlink->stats.remote_reload_stats,
sizeof(remote_reload_stats)); sizeof(remote_reload_stats));
curr_net = devlink_net(devlink);
devlink_ns_change_notify(devlink, dest_net, curr_net, false);
err = devlink->ops->reload_down(devlink, !!dest_net, action, limit, extack); err = devlink->ops->reload_down(devlink, !!dest_net, action, limit, extack);
if (err) if (err)
return err; return err;
if (dest_net && !net_eq(dest_net, curr_net)) { curr_net = devlink_net(devlink);
move_netdevice_notifier_net(curr_net, dest_net, if (dest_net && !net_eq(dest_net, curr_net))
&devlink->netdevice_nb); devlink_reload_netns_change(devlink, curr_net, dest_net);
write_pnet(&devlink->_net, dest_net);
}
err = devlink->ops->reload_up(devlink, action, limit, actions_performed, extack); err = devlink->ops->reload_up(devlink, action, limit, actions_performed, extack);
devlink_reload_failed_set(devlink, !!err); devlink_reload_failed_set(devlink, !!err);
if (err) if (err)
return err; return err;
devlink_ns_change_notify(devlink, dest_net, curr_net, true);
WARN_ON(!(*actions_performed & BIT(action))); WARN_ON(!(*actions_performed & BIT(action)));
/* Catch driver on updating the remote action within devlink reload */ /* Catch driver on updating the remote action within devlink reload */
WARN_ON(memcmp(remote_reload_stats, devlink->stats.remote_reload_stats, WARN_ON(memcmp(remote_reload_stats, devlink->stats.remote_reload_stats,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment