From: Cosmin Ratiu <cratiu@nvidia.com>
In order to support cross-esw scheduling, the nodes for all devices need
to be accessible from the same place so that normalization can work.
Store them in the shared devlink instance and provide an accessor for
them.
Protecting against concurrent QoS modifications is now done with
the shd lock. All calls originating from devlink rate already have the
shd lock held so only the additional entry points into QoS need to
acquire it.
As part of this change, the E-Switch qos domain concept was removed.
E-Switch QoS domains were added with the intention of eventually
implementing shared qos domains to support cross-esw scheduling in the
previous approach ([1]), but they are no longer necessary in the new
approach.
[1] https://lore.kernel.org/netdev/20250213180134.323929-1-tariqt@nvidia.com/
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/esw/qos.c | 201 +++++-------------
.../net/ethernet/mellanox/mlx5/core/esw/qos.h | 3 -
.../net/ethernet/mellanox/mlx5/core/eswitch.c | 9 +-
.../net/ethernet/mellanox/mlx5/core/eswitch.h | 11 +-
.../ethernet/mellanox/mlx5/core/sh_devlink.c | 30 ++-
.../ethernet/mellanox/mlx5/core/sh_devlink.h | 2 +
6 files changed, 87 insertions(+), 169 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
index 8c3a026b8db4..0d187399d846 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
@@ -5,60 +5,16 @@
#include "lib/mlx5.h"
#include "esw/qos.h"
#include "en/port.h"
+#include "sh_devlink.h"
#define CREATE_TRACE_POINTS
#include "diag/qos_tracepoint.h"
/* Minimum supported BW share value by the HW is 1 Mbit/sec */
#define MLX5_MIN_BW_SHARE 1
-/* Holds rate nodes associated with an E-Switch. */
-struct mlx5_qos_domain {
- /* Serializes access to all qos changes in the qos domain. */
- struct mutex lock;
- /* List of all mlx5_esw_sched_nodes. */
- struct list_head nodes;
-};
-
-static void esw_qos_lock(struct mlx5_eswitch *esw)
-{
- mutex_lock(&esw->qos.domain->lock);
-}
-
-static void esw_qos_unlock(struct mlx5_eswitch *esw)
-{
- mutex_unlock(&esw->qos.domain->lock);
-}
-
static void esw_assert_qos_lock_held(struct mlx5_eswitch *esw)
{
- lockdep_assert_held(&esw->qos.domain->lock);
-}
-
-static struct mlx5_qos_domain *esw_qos_domain_alloc(void)
-{
- struct mlx5_qos_domain *qos_domain;
-
- qos_domain = kzalloc(sizeof(*qos_domain), GFP_KERNEL);
- if (!qos_domain)
- return NULL;
-
- mutex_init(&qos_domain->lock);
- INIT_LIST_HEAD(&qos_domain->nodes);
-
- return qos_domain;
-}
-
-static int esw_qos_domain_init(struct mlx5_eswitch *esw)
-{
- esw->qos.domain = esw_qos_domain_alloc();
-
- return esw->qos.domain ? 0 : -ENOMEM;
-}
-
-static void esw_qos_domain_release(struct mlx5_eswitch *esw)
-{
- kfree(esw->qos.domain);
- esw->qos.domain = NULL;
+ devl_assert_locked(esw->dev->shd);
}
enum sched_node_type {
@@ -111,7 +67,8 @@ static void esw_qos_node_attach_to_parent(struct mlx5_esw_sched_node *node)
if (!node->parent) {
/* Root children are assigned a depth level of 2. */
node->level = 2;
- list_add_tail(&node->entry, &node->esw->qos.domain->nodes);
+ list_add_tail(&node->entry,
+ mlx5_shd_get_qos_nodes(node->esw->dev));
} else {
node->level = node->parent->level + 1;
list_add_tail(&node->entry, &node->parent->children);
@@ -324,14 +281,15 @@ static int esw_qos_create_rate_limit_element(struct mlx5_esw_sched_node *node,
static u32 esw_qos_calculate_min_rate_divider(struct mlx5_eswitch *esw,
struct mlx5_esw_sched_node *parent)
{
- struct list_head *nodes = parent ? &parent->children : &esw->qos.domain->nodes;
u32 fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
struct mlx5_esw_sched_node *node;
+ struct list_head *nodes;
u32 max_guarantee = 0;
/* Find max min_rate across all nodes.
* This will correspond to fw_max_bw_share in the final bw_share calculation.
*/
+ nodes = parent ? &parent->children : mlx5_shd_get_qos_nodes(esw->dev);
list_for_each_entry(node, nodes, entry) {
if (node->esw == esw && node->ix != esw->qos.root_tsar_ix &&
node->min_rate > max_guarantee)
@@ -372,10 +330,11 @@ static void esw_qos_normalize_min_rate(struct mlx5_eswitch *esw,
struct mlx5_esw_sched_node *parent,
struct netlink_ext_ack *extack)
{
- struct list_head *nodes = parent ? &parent->children : &esw->qos.domain->nodes;
u32 divider = esw_qos_calculate_min_rate_divider(esw, parent);
struct mlx5_esw_sched_node *node;
+ struct list_head *nodes;
+ nodes = parent ? &parent->children : mlx5_shd_get_qos_nodes(esw->dev);
list_for_each_entry(node, nodes, entry) {
if (node->esw != esw || node->ix == esw->qos.root_tsar_ix)
continue;
@@ -715,7 +674,7 @@ __esw_qos_create_vports_sched_node(struct mlx5_eswitch *esw, struct mlx5_esw_sch
goto err_alloc_node;
}
- list_add_tail(&node->entry, &esw->qos.domain->nodes);
+ list_add_tail(&node->entry, mlx5_shd_get_qos_nodes(esw->dev));
esw_qos_normalize_min_rate(esw, NULL, extack);
trace_mlx5_esw_node_qos_create(esw->dev, node, node->ix);
@@ -1108,7 +1067,8 @@ static int mlx5_esw_qos_vport_enable(struct mlx5_vport *vport, enum sched_node_t
return -ENOMEM;
}
if (!parent)
- list_add_tail(&sched_node->entry, &esw->qos.domain->nodes);
+ list_add_tail(&sched_node->entry,
+ mlx5_shd_get_qos_nodes(esw->dev));
sched_node->max_rate = max_rate;
sched_node->min_rate = min_rate;
@@ -1143,7 +1103,7 @@ void mlx5_esw_qos_vport_disable(struct mlx5_vport *vport)
struct mlx5_esw_sched_node *parent;
lockdep_assert_held(&esw->state_lock);
- esw_qos_lock(esw);
+ devl_lock(esw->dev->shd);
if (!vport->qos.sched_node)
goto unlock;
@@ -1152,7 +1112,7 @@ void mlx5_esw_qos_vport_disable(struct mlx5_vport *vport)
mlx5_esw_qos_vport_disable_locked(vport);
unlock:
- esw_qos_unlock(esw);
+ devl_unlock(esw->dev->shd);
}
static int mlx5_esw_qos_set_vport_max_rate(struct mlx5_vport *vport, u32 max_rate,
@@ -1191,26 +1151,25 @@ int mlx5_esw_qos_set_vport_rate(struct mlx5_vport *vport, u32 max_rate, u32 min_
struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
int err;
- esw_qos_lock(esw);
+ devl_lock(esw->dev->shd);
err = mlx5_esw_qos_set_vport_min_rate(vport, min_rate, NULL);
if (!err)
err = mlx5_esw_qos_set_vport_max_rate(vport, max_rate, NULL);
- esw_qos_unlock(esw);
+ devl_unlock(esw->dev->shd);
return err;
}
bool mlx5_esw_qos_get_vport_rate(struct mlx5_vport *vport, u32 *max_rate, u32 *min_rate)
{
- struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
bool enabled;
- esw_qos_lock(esw);
+ devl_lock(vport->dev->shd);
enabled = !!vport->qos.sched_node;
if (enabled) {
*max_rate = vport->qos.sched_node->max_rate;
*min_rate = vport->qos.sched_node->min_rate;
}
- esw_qos_unlock(esw);
+ devl_unlock(vport->dev->shd);
return enabled;
}
@@ -1576,9 +1535,9 @@ int mlx5_esw_qos_modify_vport_rate(struct mlx5_eswitch *esw, u16 vport_num, u32
return err;
}
- esw_qos_lock(esw);
+ devl_lock(esw->dev->shd);
err = mlx5_esw_qos_set_vport_max_rate(vport, rate_mbps, NULL);
- esw_qos_unlock(esw);
+ devl_unlock(esw->dev->shd);
return err;
}
@@ -1667,44 +1626,24 @@ static void esw_vport_qos_prune_empty(struct mlx5_vport *vport)
mlx5_esw_qos_vport_disable_locked(vport);
}
-int mlx5_esw_qos_init(struct mlx5_eswitch *esw)
-{
- if (esw->qos.domain)
- return 0; /* Nothing to change. */
-
- return esw_qos_domain_init(esw);
-}
-
-void mlx5_esw_qos_cleanup(struct mlx5_eswitch *esw)
-{
- if (esw->qos.domain)
- esw_qos_domain_release(esw);
-}
-
/* Eswitch devlink rate API */
int mlx5_esw_devlink_rate_leaf_tx_share_set(struct devlink_rate *rate_leaf, void *priv,
u64 tx_share, struct netlink_ext_ack *extack)
{
struct mlx5_vport *vport = priv;
- struct mlx5_eswitch *esw;
int err;
- esw = vport->dev->priv.eswitch;
- if (!mlx5_esw_allowed(esw))
+ if (!mlx5_esw_allowed(vport->dev->priv.eswitch))
return -EPERM;
err = esw_qos_devlink_rate_to_mbps(vport->dev, "tx_share", &tx_share, extack);
if (err)
return err;
- esw_qos_lock(esw);
err = mlx5_esw_qos_set_vport_min_rate(vport, tx_share, extack);
- if (err)
- goto out;
- esw_vport_qos_prune_empty(vport);
-out:
- esw_qos_unlock(esw);
+ if (!err)
+ esw_vport_qos_prune_empty(vport);
return err;
}
@@ -1712,24 +1651,18 @@ int mlx5_esw_devlink_rate_leaf_tx_max_set(struct devlink_rate *rate_leaf, void *
u64 tx_max, struct netlink_ext_ack *extack)
{
struct mlx5_vport *vport = priv;
- struct mlx5_eswitch *esw;
int err;
- esw = vport->dev->priv.eswitch;
- if (!mlx5_esw_allowed(esw))
+ if (!mlx5_esw_allowed(vport->dev->priv.eswitch))
return -EPERM;
err = esw_qos_devlink_rate_to_mbps(vport->dev, "tx_max", &tx_max, extack);
if (err)
return err;
- esw_qos_lock(esw);
err = mlx5_esw_qos_set_vport_max_rate(vport, tx_max, extack);
- if (err)
- goto out;
- esw_vport_qos_prune_empty(vport);
-out:
- esw_qos_unlock(esw);
+ if (!err)
+ esw_vport_qos_prune_empty(vport);
return err;
}
@@ -1740,34 +1673,30 @@ int mlx5_esw_devlink_rate_leaf_tc_bw_set(struct devlink_rate *rate_leaf,
{
struct mlx5_esw_sched_node *vport_node;
struct mlx5_vport *vport = priv;
- struct mlx5_eswitch *esw;
bool disable;
int err = 0;
- esw = vport->dev->priv.eswitch;
- if (!mlx5_esw_allowed(esw))
+ if (!mlx5_esw_allowed(vport->dev->priv.eswitch))
return -EPERM;
disable = esw_qos_tc_bw_disabled(tc_bw);
- esw_qos_lock(esw);
if (!esw_qos_vport_validate_unsupported_tc_bw(vport, tc_bw)) {
NL_SET_ERR_MSG_MOD(extack,
"E-Switch traffic classes number is not supported");
- err = -EOPNOTSUPP;
- goto unlock;
+ return -EOPNOTSUPP;
}
vport_node = vport->qos.sched_node;
if (disable && !vport_node)
- goto unlock;
+ return 0;
if (disable) {
if (vport_node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR)
err = esw_qos_vport_update(vport, SCHED_NODE_TYPE_VPORT,
vport_node->parent, extack);
esw_vport_qos_prune_empty(vport);
- goto unlock;
+ return err;
}
if (!vport_node) {
@@ -1782,8 +1711,6 @@ int mlx5_esw_devlink_rate_leaf_tc_bw_set(struct devlink_rate *rate_leaf,
}
if (!err)
esw_qos_set_tc_arbiter_bw_shares(vport_node, tc_bw, extack);
-unlock:
- esw_qos_unlock(esw);
return err;
}
@@ -1793,28 +1720,22 @@ int mlx5_esw_devlink_rate_node_tc_bw_set(struct devlink_rate *rate_node,
struct netlink_ext_ack *extack)
{
struct mlx5_esw_sched_node *node = priv;
- struct mlx5_eswitch *esw = node->esw;
bool disable;
int err;
- if (!esw_qos_validate_unsupported_tc_bw(esw, tc_bw)) {
+ if (!esw_qos_validate_unsupported_tc_bw(node->esw, tc_bw)) {
NL_SET_ERR_MSG_MOD(extack,
"E-Switch traffic classes number is not supported");
return -EOPNOTSUPP;
}
disable = esw_qos_tc_bw_disabled(tc_bw);
- esw_qos_lock(esw);
- if (disable) {
- err = esw_qos_node_disable_tc_arbitration(node, extack);
- goto unlock;
- }
+ if (disable)
+ return esw_qos_node_disable_tc_arbitration(node, extack);
err = esw_qos_node_enable_tc_arbitration(node, extack);
if (!err)
esw_qos_set_tc_arbiter_bw_shares(node, tc_bw, extack);
-unlock:
- esw_qos_unlock(esw);
return err;
}
@@ -1822,17 +1743,14 @@ int mlx5_esw_devlink_rate_node_tx_share_set(struct devlink_rate *rate_node, void
u64 tx_share, struct netlink_ext_ack *extack)
{
struct mlx5_esw_sched_node *node = priv;
- struct mlx5_eswitch *esw = node->esw;
int err;
- err = esw_qos_devlink_rate_to_mbps(esw->dev, "tx_share", &tx_share, extack);
+ err = esw_qos_devlink_rate_to_mbps(node->esw->dev, "tx_share",
+ &tx_share, extack);
if (err)
return err;
- esw_qos_lock(esw);
- err = esw_qos_set_node_min_rate(node, tx_share, extack);
- esw_qos_unlock(esw);
- return err;
+ return esw_qos_set_node_min_rate(node, tx_share, extack);
}
int mlx5_esw_devlink_rate_node_tx_max_set(struct devlink_rate *rate_node, void *priv,
@@ -1846,10 +1764,7 @@ int mlx5_esw_devlink_rate_node_tx_max_set(struct devlink_rate *rate_node, void *
if (err)
return err;
- esw_qos_lock(esw);
- err = esw_qos_sched_elem_config(node, tx_max, node->bw_share, extack);
- esw_qos_unlock(esw);
- return err;
+ return esw_qos_sched_elem_config(node, tx_max, node->bw_share, extack);
}
int mlx5_esw_devlink_rate_node_new(struct devlink_rate *rate_node, void **priv,
@@ -1857,30 +1772,23 @@ int mlx5_esw_devlink_rate_node_new(struct devlink_rate *rate_node, void **priv,
{
struct mlx5_esw_sched_node *node;
struct mlx5_eswitch *esw;
- int err = 0;
esw = mlx5_devlink_eswitch_get(rate_node->devlink);
if (IS_ERR(esw))
return PTR_ERR(esw);
- esw_qos_lock(esw);
if (esw->mode != MLX5_ESWITCH_OFFLOADS) {
NL_SET_ERR_MSG_MOD(extack,
"Rate node creation supported only in switchdev mode");
- err = -EOPNOTSUPP;
- goto unlock;
+ return -EOPNOTSUPP;
}
node = esw_qos_create_vports_sched_node(esw, extack);
- if (IS_ERR(node)) {
- err = PTR_ERR(node);
- goto unlock;
- }
+ if (IS_ERR(node))
+ return PTR_ERR(node);
*priv = node;
-unlock:
- esw_qos_unlock(esw);
- return err;
+ return 0;
}
int mlx5_esw_devlink_rate_node_del(struct devlink_rate *rate_node, void *priv,
@@ -1889,10 +1797,9 @@ int mlx5_esw_devlink_rate_node_del(struct devlink_rate *rate_node, void *priv,
struct mlx5_esw_sched_node *node = priv;
struct mlx5_eswitch *esw = node->esw;
- esw_qos_lock(esw);
__esw_qos_destroy_node(node, extack);
esw_qos_put(esw);
- esw_qos_unlock(esw);
+
return 0;
}
@@ -1909,7 +1816,6 @@ mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport,
return -EOPNOTSUPP;
}
- esw_qos_lock(esw);
if (!vport->qos.sched_node && parent) {
enum sched_node_type type;
@@ -1920,13 +1826,15 @@ mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport,
} else if (vport->qos.sched_node) {
err = esw_qos_vport_update_parent(vport, parent, extack);
}
- esw_qos_unlock(esw);
+
return err;
}
void mlx5_esw_qos_vport_clear_parent(struct mlx5_vport *vport)
{
+ devl_lock(vport->dev->shd);
mlx5_esw_qos_vport_update_parent(vport, NULL, NULL);
+ devl_unlock(vport->dev->shd);
}
int mlx5_esw_devlink_rate_leaf_parent_set(struct devlink_rate *devlink_rate,
@@ -1939,13 +1847,8 @@ int mlx5_esw_devlink_rate_leaf_parent_set(struct devlink_rate *devlink_rate,
int err;
err = mlx5_esw_qos_vport_update_parent(vport, node, extack);
- if (!err) {
- struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
-
- esw_qos_lock(esw);
+ if (!err)
esw_vport_qos_prune_empty(vport);
- esw_qos_unlock(esw);
- }
return err;
}
@@ -2071,14 +1974,12 @@ static int mlx5_esw_qos_node_update_parent(struct mlx5_esw_sched_node *node,
struct netlink_ext_ack *extack)
{
struct mlx5_esw_sched_node *curr_parent;
- struct mlx5_eswitch *esw = node->esw;
int err;
err = mlx5_esw_qos_node_validate_set_parent(node, parent, extack);
if (err)
return err;
- esw_qos_lock(esw);
curr_parent = node->parent;
if (node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR) {
err = esw_qos_tc_arbiter_node_update_parent(node, parent,
@@ -2088,15 +1989,11 @@ static int mlx5_esw_qos_node_update_parent(struct mlx5_esw_sched_node *node,
}
if (err)
- goto out;
-
- esw_qos_normalize_min_rate(esw, curr_parent, extack);
- esw_qos_normalize_min_rate(esw, parent, extack);
-
-out:
- esw_qos_unlock(esw);
+ return err;
- return err;
+ esw_qos_normalize_min_rate(node->esw, curr_parent, extack);
+ esw_qos_normalize_min_rate(node->esw, parent, extack);
+ return 0;
}
int mlx5_esw_devlink_rate_node_parent_set(struct devlink_rate *devlink_rate,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h
index 0a50982b0e27..f275e850d2c9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h
@@ -6,9 +6,6 @@
#ifdef CONFIG_MLX5_ESWITCH
-int mlx5_esw_qos_init(struct mlx5_eswitch *esw);
-void mlx5_esw_qos_cleanup(struct mlx5_eswitch *esw);
-
int mlx5_esw_qos_set_vport_rate(struct mlx5_vport *evport, u32 max_rate, u32 min_rate);
bool mlx5_esw_qos_get_vport_rate(struct mlx5_vport *vport, u32 *max_rate, u32 *min_rate);
void mlx5_esw_qos_vport_disable(struct mlx5_vport *vport);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 4b7a1ce7f406..51eacc286cbb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -49,6 +49,7 @@
#include "ecpf.h"
#include "en/mod_hdr.h"
#include "en_accel/ipsec.h"
+#include "sh_devlink.h"
enum {
MLX5_ACTION_NONE = 0,
@@ -1618,10 +1619,6 @@ int mlx5_eswitch_enable_locked(struct mlx5_eswitch *esw, int num_vfs)
MLX5_NB_INIT(&esw->nb, eswitch_vport_event, NIC_VPORT_CHANGE);
mlx5_eq_notifier_register(esw->dev, &esw->nb);
- err = mlx5_esw_qos_init(esw);
- if (err)
- goto err_esw_init;
-
if (esw->mode == MLX5_ESWITCH_LEGACY) {
err = esw_legacy_enable(esw);
} else {
@@ -2028,9 +2025,6 @@ int mlx5_eswitch_init(struct mlx5_core_dev *dev)
goto reps_err;
esw->mode = MLX5_ESWITCH_LEGACY;
- err = mlx5_esw_qos_init(esw);
- if (err)
- goto reps_err;
mutex_init(&esw->offloads.encap_tbl_lock);
hash_init(esw->offloads.encap_tbl);
@@ -2080,7 +2074,6 @@ void mlx5_eswitch_cleanup(struct mlx5_eswitch *esw)
esw_info(esw->dev, "cleanup\n");
- mlx5_esw_qos_cleanup(esw);
destroy_workqueue(esw->work_queue);
WARN_ON(refcount_read(&esw->qos.refcnt));
mutex_destroy(&esw->state_lock);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index 20cf9dd542a1..d145591b3434 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -224,8 +224,9 @@ struct mlx5_vport {
struct mlx5_vport_info info;
- /* Protected with the E-Switch qos domain lock. The Vport QoS can
- * either be disabled (sched_node is NULL) or in one of three states:
+ /* Protected by mlx5_shd_lock().
+ * The Vport QoS can either be disabled (sched_node is NULL) or in one
+ * of three states:
* 1. Regular QoS (sched_node is a vport node).
* 2. TC QoS enabled on the vport (sched_node is a TC arbiter).
* 3. TC QoS enabled on the vport's parent node
@@ -356,7 +357,6 @@ enum {
};
struct dentry;
-struct mlx5_qos_domain;
struct mlx5_eswitch {
struct mlx5_core_dev *dev;
@@ -383,12 +383,13 @@ struct mlx5_eswitch {
struct rw_semaphore mode_lock;
atomic64_t user_count;
- /* Protected with the E-Switch qos domain lock. */
+ /* The QoS tree is stored in mlx5_shd.
+ * QoS changes are serialized with mlx5_shd_lock().
+ */
struct {
/* Initially 0, meaning no QoS users and QoS is disabled. */
refcount_t refcnt;
u32 root_tsar_ix;
- struct mlx5_qos_domain *domain;
} qos;
struct mlx5_esw_bridge_offloads *br_offloads;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
index abae5f0130e9..daac20883c8d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
@@ -9,6 +9,16 @@
static const struct devlink_ops mlx5_shd_ops = {
};
+struct mlx5_shd_priv {
+ struct list_head qos_nodes;
+};
+
+static int mlx5_shd_priv_init(struct mlx5_shd_priv *shd_priv)
+{
+ INIT_LIST_HEAD(&shd_priv->qos_nodes);
+ return 0;
+}
+
int mlx5_shd_init(struct mlx5_core_dev *dev)
{
u8 *vpd_data __free(kfree) = NULL;
@@ -45,11 +55,18 @@ int mlx5_shd_init(struct mlx5_core_dev *dev)
*end = '\0';
/* Get or create shared devlink instance */
- devlink = devlink_shd_get(sn, &mlx5_shd_ops, 0);
+ devlink = devlink_shd_get(sn, &mlx5_shd_ops,
+ sizeof(struct mlx5_shd_priv));
kfree(sn);
if (!devlink)
return -ENOMEM;
+ err = mlx5_shd_priv_init(devlink_shd_get_priv(devlink));
+ if (err < 0) {
+ devlink_shd_put(devlink);
+ return err;
+ }
+
dev->shd = devlink;
return 0;
}
@@ -61,3 +78,14 @@ void mlx5_shd_uninit(struct mlx5_core_dev *dev)
devlink_shd_put(dev->shd);
}
+
+struct list_head *mlx5_shd_get_qos_nodes(struct mlx5_core_dev *dev)
+{
+ struct mlx5_shd_priv *shd_priv;
+
+ if (!dev->shd)
+ return NULL;
+ devl_assert_locked(dev->shd);
+ shd_priv = devlink_shd_get_priv(dev->shd);
+ return &shd_priv->qos_nodes;
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h
index 8ab8d6940227..ec6a03471204 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h
@@ -9,4 +9,6 @@
int mlx5_shd_init(struct mlx5_core_dev *dev);
void mlx5_shd_uninit(struct mlx5_core_dev *dev);
+struct list_head *mlx5_shd_get_qos_nodes(struct mlx5_core_dev *dev);
+
#endif /* __MLX5_SH_DEVLINK_H__ */
--
2.44.0This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net/mlx5: Store QoS sched nodes in the sh_devlink
This commit moves the QoS scheduling nodes from per-eswitch storage to
the shared devlink instance. It replaces per-eswitch locking with the
shared devlink lock and removes explicit locking from devlink rate
callbacks with the expectation that devlink holds the shd lock.
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> index 8c3a026b8db4..0d187399d846 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
[ ... ]
> static void esw_assert_qos_lock_held(struct mlx5_eswitch *esw)
> {
>- lockdep_assert_held(&esw->qos.domain->lock);
>+ devl_assert_locked(esw->dev->shd);
> }
[ ... ]
> int mlx5_esw_devlink_rate_leaf_tx_share_set(struct devlink_rate *rate_leaf, void *priv,
> u64 tx_share, struct netlink_ext_ack *extack)
> {
> struct mlx5_vport *vport = priv;
>- struct mlx5_eswitch *esw;
> int err;
>
>- esw = vport->dev->priv.eswitch;
>- if (!mlx5_esw_allowed(esw))
>+ if (!mlx5_esw_allowed(vport->dev->priv.eswitch))
> return -EPERM;
>
> err = esw_qos_devlink_rate_to_mbps(vport->dev, "tx_share", &tx_share, extack);
> if (err)
> return err;
>
>- esw_qos_lock(esw);
> err = mlx5_esw_qos_set_vport_min_rate(vport, tx_share, extack);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
At this commit, the esw_qos_lock/unlock calls are removed with the
expectation that "all calls originating from devlink rate already have
the shd lock held" (per the commit message). However, the devlink rate
infrastructure only locks dev->shd when supported_cross_device_rate_nodes
is set in the devlink_ops, which does not happen until commit ef6db4a7f381
("net/mlx5: qos: Enable cross-device scheduling").
The call chain is:
devlink_nl_rate_set_doit() -> devl_rate_lock() ->
mlx5_esw_devlink_rate_leaf_tx_share_set() ->
mlx5_esw_qos_set_vport_min_rate() -> esw_assert_qos_lock_held() ->
devl_assert_locked(esw->dev->shd)
Since devl_rate_lock() only locks the nested-in devlink when
supported_cross_device_rate_nodes is true, and that flag is not set at
this commit, the assertion will fail with lockdep enabled, and QoS data
structures will be accessed without proper locking.
This creates a bisect window between this commit and ef6db4a7f381 where
the QoS locking is broken. Could the setting of supported_cross_device_rate_nodes
be moved into this patch to avoid the bisect issue?
>- if (err)
>- goto out;
>- esw_vport_qos_prune_empty(vport);
>-out:
>- esw_qos_unlock(esw);
>+ if (!err)
>+ esw_vport_qos_prune_empty(vport);
> return err;
> }
[ ... ]
On Wed, 2026-01-21 at 19:40 -0800, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> This creates a bisect window between this commit and ef6db4a7f381
> where
> the QoS locking is broken. Could the setting of
> supported_cross_device_rate_nodes
> be moved into this patch to avoid the bisect issue?
This was a useful insight, and a clear 2-patch bisect window where
warnings would be generated.
I will merge the setting of supported_cross_device_rate_nodes with this
patch, since the mlx5 code is protected against cross-esw parents until
the next one in the series ("Support cross-device tx scheduling").
Cosmin.
© 2016 - 2026 Red Hat, Inc.