Skip to content

Commit

Permalink
net/mlx5e: Create separate devlink instance for ethernet auxiliary de…
Browse files Browse the repository at this point in the history 
…vice

The fact that devlink instance lock is held over mlx5 auxiliary devices
probe and remove routines brought a need to conditionally take devlink
instance lock there. The code is checking a MLX5E_LOCKED_FLOW flag
in mlx5 priv struct.

This is racy and may lead to access devlink objects without holding
instance lock or deadlock.

To avoid this, the only lock-wise sane solution is to make the
devlink entities created by the auxiliary device independent on
the original pci devlink instance. Create devlink instance for the
auxiliary device and put the uplink port instance there alongside with
the port health reporters.

Signed-off-by: Jiri Pirko <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
Signed-off-by: Jakub Kicinski <[email protected]>
  • Loading branch information
@kuba-moo
Jiri Pirko authored and kuba-moo committed Jan 20, 2023
1 parent 3a10173 commit ee75f1f
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 23 deletions.
4 changes: 4 additions & 0 deletions drivers/net/ethernet/mellanox/mlx5/core/en.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,10 @@ struct mlx5e_priv {
struct dentry *dfs_root;
};

struct mlx5e_dev {
struct mlx5e_priv *priv;
};

struct mlx5e_rx_handlers {
mlx5e_fp_handle_rx_cqe handle_rx_cqe;
mlx5e_fp_handle_rx_cqe handle_rx_cqe_mpwqe;
Expand Down
44 changes: 28 additions & 16 deletions drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,29 @@
#include "en/devlink.h"
#include "eswitch.h"

static const struct devlink_ops mlx5e_devlink_ops = {
};

struct mlx5e_dev *mlx5e_create_devlink(struct device *dev)
{
struct mlx5e_dev *mlx5e_dev;
struct devlink *devlink;

devlink = devlink_alloc(&mlx5e_devlink_ops, sizeof(*mlx5e_dev), dev);
if (!devlink)
return ERR_PTR(-ENOMEM);
devlink_register(devlink);
return devlink_priv(devlink);
}

void mlx5e_destroy_devlink(struct mlx5e_dev *mlx5e_dev)
{
struct devlink *devlink = priv_to_devlink(mlx5e_dev);

devlink_unregister(devlink);
devlink_free(devlink);
}

static void
mlx5e_devlink_get_port_parent_id(struct mlx5_core_dev *dev, struct netdev_phys_item_id *ppid)
{
Expand All @@ -14,14 +37,14 @@ mlx5e_devlink_get_port_parent_id(struct mlx5_core_dev *dev, struct netdev_phys_i
memcpy(ppid->id, &parent_id, sizeof(parent_id));
}

int mlx5e_devlink_port_register(struct mlx5e_priv *priv)
int mlx5e_devlink_port_register(struct mlx5e_dev *mlx5e_dev,
struct mlx5e_priv *priv)
{
struct devlink *devlink = priv_to_devlink(priv->mdev);
struct devlink *devlink = priv_to_devlink(mlx5e_dev);
struct devlink_port_attrs attrs = {};
struct netdev_phys_item_id ppid = {};
struct devlink_port *dl_port;
unsigned int dl_port_index;
int ret;

if (mlx5_core_is_pf(priv->mdev)) {
attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
Expand All @@ -42,23 +65,12 @@ int mlx5e_devlink_port_register(struct mlx5e_priv *priv)
memset(dl_port, 0, sizeof(*dl_port));
devlink_port_attrs_set(dl_port, &attrs);

if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW))
devl_lock(devlink);
ret = devl_port_register(devlink, dl_port, dl_port_index);
if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW))
devl_unlock(devlink);

return ret;
return devlink_port_register(devlink, dl_port, dl_port_index);
}

void mlx5e_devlink_port_unregister(struct mlx5e_priv *priv)
{
struct devlink_port *dl_port = mlx5e_devlink_get_dl_port(priv);
struct devlink *devlink = priv_to_devlink(priv->mdev);

if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW))
devl_lock(devlink);
devl_port_unregister(dl_port);
if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW))
devl_unlock(devlink);
devlink_port_unregister(dl_port);
}
5 changes: 4 additions & 1 deletion drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
#include <net/devlink.h>
#include "en.h"

int mlx5e_devlink_port_register(struct mlx5e_priv *priv);
struct mlx5e_dev *mlx5e_create_devlink(struct device *dev);
void mlx5e_destroy_devlink(struct mlx5e_dev *mlx5e_dev);
int mlx5e_devlink_port_register(struct mlx5e_dev *mlx5e_dev,
struct mlx5e_priv *priv);
void mlx5e_devlink_port_unregister(struct mlx5e_priv *priv);

static inline struct devlink_port *
Expand Down
25 changes: 19 additions & 6 deletions drivers/net/ethernet/mellanox/mlx5/core/en_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5876,7 +5876,8 @@ void mlx5e_destroy_netdev(struct mlx5e_priv *priv)
static int mlx5e_resume(struct auxiliary_device *adev)
{
struct mlx5_adev *edev = container_of(adev, struct mlx5_adev, adev);
struct mlx5e_priv *priv = auxiliary_get_drvdata(adev);
struct mlx5e_dev *mlx5e_dev = auxiliary_get_drvdata(adev);
struct mlx5e_priv *priv = mlx5e_dev->priv;
struct net_device *netdev = priv->netdev;
struct mlx5_core_dev *mdev = edev->mdev;
int err;
Expand All @@ -5899,7 +5900,8 @@ static int mlx5e_resume(struct auxiliary_device *adev)

static int mlx5e_suspend(struct auxiliary_device *adev, pm_message_t state)
{
struct mlx5e_priv *priv = auxiliary_get_drvdata(adev);
struct mlx5e_dev *mlx5e_dev = auxiliary_get_drvdata(adev);
struct mlx5e_priv *priv = mlx5e_dev->priv;
struct net_device *netdev = priv->netdev;
struct mlx5_core_dev *mdev = priv->mdev;

Expand All @@ -5917,29 +5919,36 @@ static int mlx5e_probe(struct auxiliary_device *adev,
struct mlx5_adev *edev = container_of(adev, struct mlx5_adev, adev);
const struct mlx5e_profile *profile = &mlx5e_nic_profile;
struct mlx5_core_dev *mdev = edev->mdev;
struct mlx5e_dev *mlx5e_dev;
struct net_device *netdev;
pm_message_t state = {};
struct mlx5e_priv *priv;
int err;

mlx5e_dev = mlx5e_create_devlink(&adev->dev);
if (IS_ERR(mlx5e_dev))
return PTR_ERR(mlx5e_dev);
auxiliary_set_drvdata(adev, mlx5e_dev);

netdev = mlx5e_create_netdev(mdev, profile);
if (!netdev) {
mlx5_core_err(mdev, "mlx5e_create_netdev failed\n");
return -ENOMEM;
err = -ENOMEM;
goto err_devlink_unregister;
}

mlx5e_build_nic_netdev(netdev);

priv = netdev_priv(netdev);
auxiliary_set_drvdata(adev, priv);
mlx5e_dev->priv = priv;

priv->profile = profile;
priv->ppriv = NULL;

priv->dfs_root = debugfs_create_dir("nic",
mlx5_debugfs_get_dev_root(priv->mdev));

err = mlx5e_devlink_port_register(priv);
err = mlx5e_devlink_port_register(mlx5e_dev, priv);
if (err) {
mlx5_core_err(mdev, "mlx5e_devlink_port_register failed, %d\n", err);
goto err_destroy_netdev;
Expand Down Expand Up @@ -5978,12 +5987,15 @@ static int mlx5e_probe(struct auxiliary_device *adev,
err_destroy_netdev:
debugfs_remove_recursive(priv->dfs_root);
mlx5e_destroy_netdev(priv);
err_devlink_unregister:
mlx5e_destroy_devlink(mlx5e_dev);
return err;
}

static void mlx5e_remove(struct auxiliary_device *adev)
{
struct mlx5e_priv *priv = auxiliary_get_drvdata(adev);
struct mlx5e_dev *mlx5e_dev = auxiliary_get_drvdata(adev);
struct mlx5e_priv *priv = mlx5e_dev->priv;
pm_message_t state = {};

mlx5e_dcbnl_delete_app(priv);
Expand All @@ -5993,6 +6005,7 @@ static void mlx5e_remove(struct auxiliary_device *adev)
mlx5e_devlink_port_unregister(priv);
debugfs_remove_recursive(priv->dfs_root);
mlx5e_destroy_netdev(priv);
mlx5e_destroy_devlink(mlx5e_dev);
}

static const struct auxiliary_device_id mlx5e_id_table[] = {
Expand Down

0 comments on commit ee75f1f

Please sign in to comment.