devlink: always check if the devlink instance is registered

Always check under the instance lock whether the devlink instance
is still / already registered.

This is a no-op for the most part, as the unregistration path currently
waits for all references. On the init path, however, we may temporarily
open up a race with netdev code, if netdevs are registered before the
devlink instance. This is temporary, the next change fixes it, and this
commit has been split out for the ease of review.

Note that in case of iterating over sub-objects which have their
own lock (regions and line cards) we assume an implicit dependency
between those objects existing and devlink unregistration.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/devlink/core.c b/net/devlink/core.c
index d3b8336946fd20d382bbc7552337fd65e9fcfa3e..c53c996edf1da9805665ea058d736f4585d83944 100644 (file)
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -67,6 +67,15 @@ void devl_unlock(struct devlink *devlink)
 }
 EXPORT_SYMBOL_GPL(devl_unlock);
 
+/**
+ * devlink_try_get() - try to obtain a reference on a devlink instance
+ * @devlink: instance to reference
+ *
+ * Obtain a reference on a devlink instance. A reference on a devlink instance
+ * only implies that it's safe to take the instance lock. It does not imply
+ * that the instance is registered, use devl_is_registered() after taking
+ * the instance lock to check registration status.
+ */
 struct devlink *__must_check devlink_try_get(struct devlink *devlink)
 {
        if (refcount_inc_not_zero(&devlink->refcount))
@@ -300,10 +309,12 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
        devlinks_xa_for_each_registered_get(net, index, devlink) {
                WARN_ON(!(devlink->features & DEVLINK_F_RELOAD));
                devl_lock(devlink);
-               err = devlink_reload(devlink, &init_net,
-                                    DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
-                                    DEVLINK_RELOAD_LIMIT_UNSPEC,
-                                    &actions_performed, NULL);
+               err = 0;
+               if (devl_is_registered(devlink))
+                       err = devlink_reload(devlink, &init_net,
+                                            DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
+                                            DEVLINK_RELOAD_LIMIT_UNSPEC,
+                                            &actions_performed, NULL);
                devl_unlock(devlink);
                devlink_put(devlink);
 

diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 6342552e5f99e546749624f4b8273a31c43df3f9..01a00df81d0ecc9c7d4eaf1363eda2fec1a35f0a 100644 (file)
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -86,6 +86,14 @@ extern struct genl_family devlink_nl_family;
 
 struct devlink *devlinks_xa_find_get(struct net *net, unsigned long *indexp);
 
+static inline bool devl_is_registered(struct devlink *devlink)
+{
+       /* To prevent races the caller must hold the instance lock
+        * or another lock taken during unregistration.
+        */
+       return xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
+}
+
 /* Netlink */
 #define DEVLINK_NL_FLAG_NEED_PORT              BIT(0)
 #define DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT   BIT(1)

diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index bec408da4dbe57b89d1fcfde513b37872797a8c2..491f821c8b778a61167e1a839945c7aacf923824 100644 (file)
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -2130,6 +2130,9 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg,
                int idx = 0;
 
                mutex_lock(&devlink->linecards_lock);
+               if (!devl_is_registered(devlink))
+                       goto next_devlink;
+
                list_for_each_entry(linecard, &devlink->linecard_list, list) {
                        if (idx < state->idx) {
                                idx++;
@@ -2151,6 +2154,7 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg,
                        }
                        idx++;
                }
+next_devlink:
                mutex_unlock(&devlink->linecards_lock);
                devlink_put(devlink);
        }
@@ -7809,6 +7813,12 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
                int idx = 0;
 
                mutex_lock(&devlink->reporters_lock);
+               if (!devl_is_registered(devlink)) {
+                       mutex_unlock(&devlink->reporters_lock);
+                       devlink_put(devlink);
+                       continue;
+               }
+
                list_for_each_entry(reporter, &devlink->reporter_list,
                                    list) {
                        if (idx < state->idx) {
@@ -7830,6 +7840,9 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
                mutex_unlock(&devlink->reporters_lock);
 
                devl_lock(devlink);
+               if (!devl_is_registered(devlink))
+                       goto next_devlink;
+
                xa_for_each(&devlink->ports, port_index, port) {
                        mutex_lock(&port->reporters_lock);
                        list_for_each_entry(reporter, &port->reporter_list, list) {
@@ -7853,6 +7866,7 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
                        }
                        mutex_unlock(&port->reporters_lock);
                }
+next_devlink:
                devl_unlock(devlink);
                devlink_put(devlink);
        }
@@ -12218,7 +12232,8 @@ void devlink_compat_running_version(struct devlink *devlink,
                return;
 
        devl_lock(devlink);
-       __devlink_compat_running_version(devlink, buf, len);
+       if (devl_is_registered(devlink))
+               __devlink_compat_running_version(devlink, buf, len);
        devl_unlock(devlink);
 }
 
@@ -12227,20 +12242,28 @@ int devlink_compat_flash_update(struct devlink *devlink, const char *file_name)
        struct devlink_flash_update_params params = {};
        int ret;
 
-       if (!devlink->ops->flash_update)
-               return -EOPNOTSUPP;
+       devl_lock(devlink);
+       if (!devl_is_registered(devlink)) {
+               ret = -ENODEV;
+               goto out_unlock;
+       }
+
+       if (!devlink->ops->flash_update) {
+               ret = -EOPNOTSUPP;
+               goto out_unlock;
+       }
 
        ret = request_firmware(&params.fw, file_name, devlink->dev);
        if (ret)
-               return ret;
+               goto out_unlock;
 
-       devl_lock(devlink);
        devlink_flash_update_begin_notify(devlink);
        ret = devlink->ops->flash_update(devlink, &params, NULL);
        devlink_flash_update_end_notify(devlink);
-       devl_unlock(devlink);
 
        release_firmware(params.fw);
+out_unlock:
+       devl_unlock(devlink);
 
        return ret;
 }

diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c
index 69111746f5d95b64ac5d1a0d3c1734f02c1201db..b5b8ac6db2d1feb934414f342f04cf4b30cadc16 100644 (file)
--- a/net/devlink/netlink.c
+++ b/net/devlink/netlink.c
@@ -98,7 +98,8 @@ devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs)
 
        devlinks_xa_for_each_registered_get(net, index, devlink) {
                devl_lock(devlink);
-               if (strcmp(devlink->dev->bus->name, busname) == 0 &&
+               if (devl_is_registered(devlink) &&
+                   strcmp(devlink->dev->bus->name, busname) == 0 &&
                    strcmp(dev_name(devlink->dev), devname) == 0)
                        return devlink;
                devl_unlock(devlink);
@@ -211,7 +212,12 @@ int devlink_nl_instance_iter_dump(struct sk_buff *msg,
 
        devlink_dump_for_each_instance_get(msg, state, devlink) {
                devl_lock(devlink);
-               err = cmd->dump_one(msg, devlink, cb);
+
+               if (devl_is_registered(devlink))
+                       err = cmd->dump_one(msg, devlink, cb);
+               else
+                       err = 0;
+
                devl_unlock(devlink);
                devlink_put(devlink);