devlink: hold the instance lock during eswitch_mode callbacks

Make the devlink core hold the instance lock during eswitch_mode
callbacks. Cheat in case of mlx5 (see the cover letter).

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

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
index b2a9528b456b984b5ed451db9f2e1b9bb9920aa6..eb4803b11c0e29e1eddaf12c87f542fc0402fc9d 100644 (file)
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
@@ -559,44 +559,34 @@ int bnxt_dl_eswitch_mode_set(struct devlink *devlink, u16 mode,
                             struct netlink_ext_ack *extack)
 {
        struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
-       int rc = 0;
 
-       devl_lock(devlink);
        if (bp->eswitch_mode == mode) {
                netdev_info(bp->dev, "already in %s eswitch mode\n",
                            mode == DEVLINK_ESWITCH_MODE_LEGACY ?
                            "legacy" : "switchdev");
-               rc = -EINVAL;
-               goto done;
+               return -EINVAL;
        }
 
        switch (mode) {
        case DEVLINK_ESWITCH_MODE_LEGACY:
                bnxt_vf_reps_destroy(bp);
-               break;
+               return 0;
 
        case DEVLINK_ESWITCH_MODE_SWITCHDEV:
                if (bp->hwrm_spec_code < 0x10803) {
                        netdev_warn(bp->dev, "FW does not support SRIOV E-Switch SWITCHDEV mode\n");
-                       rc = -ENOTSUPP;
-                       goto done;
+                       return -ENOTSUPP;
                }
 
                if (pci_num_vf(bp->pdev) == 0) {
                        netdev_info(bp->dev, "Enable VFs before setting switchdev mode\n");
-                       rc = -EPERM;
-                       goto done;
+                       return -EPERM;
                }
-               rc = bnxt_vf_reps_create(bp);
-               break;
+               return bnxt_vf_reps_create(bp);
 
        default:
-               rc = -EINVAL;
-               goto done;
+               return -EINVAL;
        }
-done:
-       devl_unlock(devlink);
-       return rc;
 }
 
 #endif

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 35cf4cb3098e3fae4c3e63d0c2b1fd120874d377..3f63df127091268599c6469a83914ad0ebaec68f 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -3337,6 +3337,27 @@ static int eswitch_devlink_esw_mode_check(const struct mlx5_eswitch *esw)
                !mlx5_core_is_ecpf_esw_manager(esw->dev)) ? -EOPNOTSUPP : 0;
 }
 
+/* FIXME: devl_unlock() followed by devl_lock() inside driver callback
+ * is never correct and prone to races. It's a transitional workaround,
+ * never repeat this pattern.
+ *
+ * This code MUST be fixed before removing devlink_mutex as it is safe
+ * to do only because of that mutex.
+ */
+static void mlx5_eswtich_mode_callback_enter(struct devlink *devlink,
+                                            struct mlx5_eswitch *esw)
+{
+       devl_unlock(devlink);
+       down_write(&esw->mode_lock);
+}
+
+static void mlx5_eswtich_mode_callback_exit(struct devlink *devlink,
+                                           struct mlx5_eswitch *esw)
+{
+       up_write(&esw->mode_lock);
+       devl_lock(devlink);
+}
+
 int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
                                  struct netlink_ext_ack *extack)
 {
@@ -3351,6 +3372,15 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
        if (esw_mode_from_devlink(mode, &mlx5_mode))
                return -EINVAL;
 
+       /* FIXME: devl_unlock() followed by devl_lock() inside driver callback
+        * is never correct and prone to races. It's a transitional workaround,
+        * never repeat this pattern.
+        *
+        * This code MUST be fixed before removing devlink_mutex as it is safe
+        * to do only because of that mutex.
+        */
+       devl_unlock(devlink);
+
        mlx5_lag_disable_change(esw->dev);
        err = mlx5_esw_try_lock(esw);
        if (err < 0) {
@@ -3381,6 +3411,7 @@ unlock:
        mlx5_esw_unlock(esw);
 enable_lag:
        mlx5_lag_enable_change(esw->dev);
+       devl_lock(devlink);
        return err;
 }
 
@@ -3393,14 +3424,14 @@ int mlx5_devlink_eswitch_mode_get(struct devlink *devlink, u16 *mode)
        if (IS_ERR(esw))
                return PTR_ERR(esw);
 
-       down_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_enter(devlink, esw);
        err = eswitch_devlink_esw_mode_check(esw);
        if (err)
                goto unlock;
 
        err = esw_mode_to_devlink(esw->mode, mode);
 unlock:
-       up_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_exit(devlink, esw);
        return err;
 }
 
@@ -3447,7 +3478,7 @@ int mlx5_devlink_eswitch_inline_mode_set(struct devlink *devlink, u8 mode,
        if (IS_ERR(esw))
                return PTR_ERR(esw);
 
-       down_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_enter(devlink, esw);
        err = eswitch_devlink_esw_mode_check(esw);
        if (err)
                goto out;
@@ -3484,11 +3515,11 @@ int mlx5_devlink_eswitch_inline_mode_set(struct devlink *devlink, u8 mode,
                goto out;
 
        esw->offloads.inline_mode = mlx5_mode;
-       up_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_exit(devlink, esw);
        return 0;
 
 out:
-       up_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_exit(devlink, esw);
        return err;
 }
 
@@ -3501,14 +3532,14 @@ int mlx5_devlink_eswitch_inline_mode_get(struct devlink *devlink, u8 *mode)
        if (IS_ERR(esw))
                return PTR_ERR(esw);
 
-       down_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_enter(devlink, esw);
        err = eswitch_devlink_esw_mode_check(esw);
        if (err)
                goto unlock;
 
        err = esw_inline_mode_to_devlink(esw->offloads.inline_mode, mode);
 unlock:
-       up_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_exit(devlink, esw);
        return err;
 }
 
@@ -3524,7 +3555,7 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
        if (IS_ERR(esw))
                return PTR_ERR(esw);
 
-       down_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_enter(devlink, esw);
        err = eswitch_devlink_esw_mode_check(esw);
        if (err)
                goto unlock;
@@ -3570,7 +3601,7 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
        }
 
 unlock:
-       up_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_exit(devlink, esw);
        return err;
 }
 
@@ -3584,15 +3615,14 @@ int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,
        if (IS_ERR(esw))
                return PTR_ERR(esw);
 
-
-       down_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_enter(devlink, esw);
        err = eswitch_devlink_esw_mode_check(esw);
        if (err)
                goto unlock;
 
        *encap = esw->offloads.encap;
 unlock:
-       up_write(&esw->mode_lock);
+       mlx5_eswtich_mode_callback_exit(devlink, esw);
        return err;
 }
 

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 48b95566b52bac3cfcde3a6063a180b14b8bc058..405786c003347b954d891604219c6eeba962eb1d 100644 (file)
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -144,13 +144,8 @@ static int nfp_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
                                        struct netlink_ext_ack *extack)
 {
        struct nfp_pf *pf = devlink_priv(devlink);
-       int ret;
-
-       devl_lock(devlink);
-       ret = nfp_app_eswitch_mode_set(pf->app, mode);
-       devl_unlock(devlink);
 
-       return ret;
+       return nfp_app_eswitch_mode_set(pf->app, mode);
 }
 
 static const struct nfp_devlink_versions_simple {

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 68cd1defe990fc3f9a694b5dcc653615fbed058a..57a3ac893792ad876dfc5d7685641545fe4ea04a 100644 (file)
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -615,22 +615,16 @@ static int nsim_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
                                         struct netlink_ext_ack *extack)
 {
        struct nsim_dev *nsim_dev = devlink_priv(devlink);
-       int err = 0;
 
-       devl_lock(devlink);
        if (mode == nsim_dev->esw_mode)
-               goto unlock;
+               return 0;
 
        if (mode == DEVLINK_ESWITCH_MODE_LEGACY)
-               err = nsim_esw_legacy_enable(nsim_dev, extack);
-       else if (mode == DEVLINK_ESWITCH_MODE_SWITCHDEV)
-               err = nsim_esw_switchdev_enable(nsim_dev, extack);
-       else
-               err = -EINVAL;
+               return nsim_esw_legacy_enable(nsim_dev, extack);
+       if (mode == DEVLINK_ESWITCH_MODE_SWITCHDEV)
+               return nsim_esw_switchdev_enable(nsim_dev, extack);
 
-unlock:
-       devl_unlock(devlink);
-       return err;
+       return -EINVAL;
 }
 
 static int nsim_devlink_eswitch_mode_get(struct devlink *devlink, u16 *mode)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 5aac5370c1366fd09bb434c6792b6c91f9fabacb..aeca13b6e57b668ee4cd86644f32c267371ede9a 100644 (file)
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2868,15 +2868,11 @@ static int devlink_rate_nodes_check(struct devlink *devlink, u16 mode,
 {
        struct devlink_rate *devlink_rate;
 
-       /* Take the lock to sync with destroy */
-       mutex_lock(&devlink->lock);
        list_for_each_entry(devlink_rate, &devlink->rate_list, list)
                if (devlink_rate_is_node(devlink_rate)) {
-                       mutex_unlock(&devlink->lock);
                        NL_SET_ERR_MSG_MOD(extack, "Rate node(s) exists.");
                        return -EBUSY;
                }
-       mutex_unlock(&devlink->lock);
        return 0;
 }
 
@@ -8735,14 +8731,12 @@ static const struct genl_small_ops devlink_nl_ops[] = {
                .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
                .doit = devlink_nl_cmd_eswitch_get_doit,
                .flags = GENL_ADMIN_PERM,
-               .internal_flags = DEVLINK_NL_FLAG_NO_LOCK,
        },
        {
                .cmd = DEVLINK_CMD_ESWITCH_SET,
                .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
                .doit = devlink_nl_cmd_eswitch_set_doit,
                .flags = GENL_ADMIN_PERM,
-               .internal_flags = DEVLINK_NL_FLAG_NO_LOCK,
        },
        {
                .cmd = DEVLINK_CMD_DPIPE_TABLE_GET,