Commit 14e426bf authored by Jakub Kicinski's avatar Jakub Kicinski Committed by David S. Miller
Browse files

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: default avatarLeon Romanovsky <leonro@nvidia.com>
Reviewed-by: default avatarJiri Pirko <jiri@nvidia.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent aff3a925
Loading
Loading
Loading
Loading
+6 −16
Original line number Diff line number Diff line
@@ -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
+42 −12
Original line number Diff line number Diff line
@@ -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 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
	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;
}

+1 −6
Original line number Diff line number Diff line
@@ -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 {
+5 −11
Original line number Diff line number Diff line
@@ -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)
+0 −6
Original line number Diff line number Diff line
@@ -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,