* [PATCH net-next V10 0/6] Support rate management on traffic classes in devlink and mlx5
@ 2025-05-20 18:38 Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 1/6] devlink: Extend devlink rate API with traffic classes bandwidth management Tariq Toukan
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Tariq Toukan @ 2025-05-20 18:38 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Jiri Pirko, Gal Pressman, Leon Romanovsky, Donald Hunter,
Jiri Pirko, Jonathan Corbet, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Carolina Jubran, Cosmin Ratiu
Hi,
This patch series by Carolina is V10 of the feature that adds rate
management support on traffic classes in devlink and mlx5, see full
description by Carolina below [0].
V10:
- Added netdevsim selftest for tc-bw ops.
- Dropped header: field as it’s unnecessary for local constants in
devlink.yaml.
Regards,
Tariq
[0]
This patch series extends the devlink-rate API to support traffic class
(TC) bandwidth management, enabling more granular control over traffic
shaping and rate limiting across multiple TCs. The API now allows users
to specify bandwidth proportions for different traffic classes in a
single command. This is particularly useful for managing Enhanced
Transmission Selection (ETS) for groups of Virtual Functions (VFs),
allowing precise bandwidth allocation across traffic classes.
Additionally the series refines the QoS handling in net/mlx5 to support
TC arbitration and bandwidth management on vports and rate nodes.
Discussions on traffic class shaping in net-shapers began in V5 [1],
where we discussed with maintainers whether net-shapers should support
traffic classes and how this could be implemented.
Later, after further conversations with Paolo Abeni and Simon Horman,
Cosmin provided an update [2], confirming that net-shapers' tree-based
hierarchy aligns well with traffic classes when treated as distinct
subsets of netdev queues. Since mlx5 enforces a 1:1 mapping between TX
queues and traffic classes, this approach seems feasible, though some
open questions remain regarding queue reconfiguration and certain mlx5
scheduling behaviors.
Building on that discussion, Cosmin has now shared a concrete
implementation plan on the netdev mailing list [3]. The plan, developed
in collaboration with Paolo and Simon, outlines how net-shapers can be
extended to support the same use cases currently covered by
devlink-rate, with the eventual goal of aligning both and simplifying
the shaping infrastructure in the kernel.
This work was presented at Netdev 0x19 in Zagreb [4].
There we presented how TC scheduling is enforced in mlx5 hardware,
which led to discussions on the mailing list.
A summary of how things work:
Classification means labeling a packet with a traffic class based on the
packet's DSCP or VLAN PCP field, then treating packets with different
traffic classes differently during transmit processing.
In a virtualized setup, VFs are untrusted and do not control
classification or shaping. Classification is done by the hardware using
a prio-to-TC mapping set by the hypervisor. VFs only select which send
queue to use and are expected to respect the classification logic by
sending each traffic class on its dedicated queue. As stated in the
net-shapers plan [3], each transmit queue should carry only a single
traffic class. Mixing classes in a single queue can lead to HOL
blocking.
In the mlx5 implementation, if the queue used does not match the
classified traffic class, the hardware moves the queue to the correct TC
scheduler. This movement is not a reclassification; it’s a necessary
enforcement step to ensure traffic class isolation is maintained.
Extend devlink-rate API to support rate management on TCs:
- devlink: Extend the devlink rate API to support traffic class
bandwidth management
Introduce a no-op implementation:
- net/mlx5: Add no-op implementation for setting tc-bw on rate objects
Add support for enabling and disabling TC QoS on vports and nodes:
- net/mlx5: Add support for setting tc-bw on nodes
- net/mlx5: Add traffic class scheduling support for vport QoS
Support for setting tc-bw on rate objects:
- net/mlx5: Manage TC arbiter nodes and implement full support for
tc-bw
[1]
https://lore.kernel.org/netdev/20241204220931.254964-1-tariqt@nvidia.com/
[2]
https://lore.kernel.org/netdev/67df1a562614b553dcab043f347a0d7c5393ff83.camel@nvidia.com/
[3]
https://lore.kernel.org/netdev/d9831d0c940a7b77419abe7c7330e822bbfd1cfb.camel@nvidia.com/T/
[4]
https://netdevconf.info/0x19/sessions/talk/optimizing-bandwidth-allocation-with-ets-and-traffic-classes.html
Carolina Jubran (6):
devlink: Extend devlink rate API with traffic classes bandwidth
management
selftest: netdevsim: Add devlink rate tc-bw test
net/mlx5: Add no-op implementation for setting tc-bw on rate objects
net/mlx5: Add support for setting tc-bw on nodes
net/mlx5: Add traffic class scheduling support for vport QoS
net/mlx5: Manage TC arbiter nodes and implement full support for tc-bw
Documentation/netlink/specs/devlink.yaml | 35 +-
.../networking/devlink/devlink-port.rst | 7 +
.../net/ethernet/mellanox/mlx5/core/devlink.c | 2 +
.../net/ethernet/mellanox/mlx5/core/esw/qos.c | 1007 ++++++++++++++++-
.../net/ethernet/mellanox/mlx5/core/esw/qos.h | 8 +
.../net/ethernet/mellanox/mlx5/core/eswitch.h | 14 +-
drivers/net/netdevsim/dev.c | 43 +
drivers/net/netdevsim/netdevsim.h | 1 +
include/net/devlink.h | 6 +
include/uapi/linux/devlink.h | 9 +
net/devlink/netlink_gen.c | 15 +-
net/devlink/netlink_gen.h | 1 +
net/devlink/rate.c | 127 +++
.../drivers/net/netdevsim/devlink.sh | 51 +
14 files changed, 1289 insertions(+), 37 deletions(-)
base-commit: f685204c57e87d2a88b159c7525426d70ee745c9
--
2.31.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next V10 1/6] devlink: Extend devlink rate API with traffic classes bandwidth management
2025-05-20 18:38 [PATCH net-next V10 0/6] Support rate management on traffic classes in devlink and mlx5 Tariq Toukan
@ 2025-05-20 18:38 ` Tariq Toukan
2025-05-20 22:53 ` Jakub Kicinski
2025-05-20 18:38 ` [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test Tariq Toukan
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Tariq Toukan @ 2025-05-20 18:38 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Jiri Pirko, Gal Pressman, Leon Romanovsky, Donald Hunter,
Jiri Pirko, Jonathan Corbet, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Carolina Jubran, Cosmin Ratiu
From: Carolina Jubran <cjubran@nvidia.com>
Introduce support for specifying bandwidth proportions between traffic
classes (TC) in the devlink-rate API. This new option allows users to
allocate bandwidth across multiple traffic classes in a single command.
This feature provides a more granular control over traffic management,
especially for scenarios requiring Enhanced Transmission Selection.
Users can now define a specific bandwidth share for each traffic class,
such as allocating 20% for TC0 (TCP/UDP) and 80% for TC5 (RoCE).
Example:
DEV=pci/0000:08:00.0
$ devlink port function rate add $DEV/vfs_group tx_share 10Gbit \
tx_max 50Gbit tc-bw 0:20 1:0 2:0 3:0 4:0 5:80 6:0 7:0
$ devlink port function rate set $DEV/vfs_group \
tc-bw 0:20 1:0 2:0 3:0 4:0 5:20 6:60 7:0
Example usage with ynl:
./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml \
--do rate-set --json '{
"bus-name": "pci",
"dev-name": "0000:08:00.0",
"port-index": 1,
"rate-tc-bws": [
{"rate-tc-index": 0, "rate-tc-bw": 50},
{"rate-tc-index": 1, "rate-tc-bw": 50},
{"rate-tc-index": 2, "rate-tc-bw": 0},
{"rate-tc-index": 3, "rate-tc-bw": 0},
{"rate-tc-index": 4, "rate-tc-bw": 0},
{"rate-tc-index": 5, "rate-tc-bw": 0},
{"rate-tc-index": 6, "rate-tc-bw": 0},
{"rate-tc-index": 7, "rate-tc-bw": 0}
]
}'
./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml \
--do rate-get --json '{
"bus-name": "pci",
"dev-name": "0000:08:00.0",
"port-index": 1
}'
output for rate-get:
{'bus-name': 'pci',
'dev-name': '0000:08:00.0',
'port-index': 1,
'rate-tc-bws': [{'rate-tc-bw': 50, 'rate-tc-index': 0},
{'rate-tc-bw': 50, 'rate-tc-index': 1},
{'rate-tc-bw': 0, 'rate-tc-index': 2},
{'rate-tc-bw': 0, 'rate-tc-index': 3},
{'rate-tc-bw': 0, 'rate-tc-index': 4},
{'rate-tc-bw': 0, 'rate-tc-index': 5},
{'rate-tc-bw': 0, 'rate-tc-index': 6},
{'rate-tc-bw': 0, 'rate-tc-index': 7}],
'rate-tx-max': 0,
'rate-tx-priority': 0,
'rate-tx-share': 0,
'rate-tx-weight': 0,
'rate-type': 'leaf'}
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
Documentation/netlink/specs/devlink.yaml | 35 ++++-
.../networking/devlink/devlink-port.rst | 7 +
include/net/devlink.h | 6 +
include/uapi/linux/devlink.h | 9 ++
net/devlink/netlink_gen.c | 15 ++-
net/devlink/netlink_gen.h | 1 +
net/devlink/rate.c | 127 ++++++++++++++++++
7 files changed, 195 insertions(+), 5 deletions(-)
diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 05fee1b7fe19..08482afa9a9c 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -224,6 +224,10 @@ definitions:
value: 10
-
name: binary
+ -
+ name: rate-tc-index-max
+ type: const
+ value: 7
attribute-sets:
-
@@ -844,7 +848,26 @@ attribute-sets:
-
name: region-direct
type: flag
-
+ -
+ name: rate-tc-bws
+ type: nest
+ multi-attr: true
+ nested-attributes: dl-rate-tc-bws
+ -
+ name: rate-tc-index
+ type: u8
+ checks:
+ min: 0
+ max: rate-tc-index-max
+ -
+ name: rate-tc-bw
+ type: u32
+ doc: |
+ Specifies the bandwidth allocation for the Traffic Class as a
+ percentage.
+ checks:
+ min: 0
+ max: 100
-
name: dl-dev-stats
subset-of: devlink
@@ -1249,6 +1272,14 @@ attribute-sets:
-
name: flash
type: flag
+ -
+ name: dl-rate-tc-bws
+ subset-of: devlink
+ attributes:
+ -
+ name: rate-tc-index
+ -
+ name: rate-tc-bw
operations:
enum-model: directional
@@ -2174,6 +2205,7 @@ operations:
- rate-tx-priority
- rate-tx-weight
- rate-parent-node-name
+ - rate-tc-bws
-
name: rate-new
@@ -2194,6 +2226,7 @@ operations:
- rate-tx-priority
- rate-tx-weight
- rate-parent-node-name
+ - rate-tc-bws
-
name: rate-del
diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
index 9d22d41a7cd1..bc3b41ac2d51 100644
--- a/Documentation/networking/devlink/devlink-port.rst
+++ b/Documentation/networking/devlink/devlink-port.rst
@@ -418,6 +418,13 @@ API allows to configure following rate object's parameters:
to all node children limits. ``tx_max`` is an upper limit for children.
``tx_share`` is a total bandwidth distributed among children.
+``tc_bw``
+ Allow users to set the bandwidth allocation per traffic class on rate
+ objects. This enables fine-grained QoS configurations by assigning specific
+ bandwidth percentages to different traffic classes. When applied to a
+ non-leaf node, tc_bw determines how bandwidth is shared among its child
+ elements.
+
``tx_priority`` and ``tx_weight`` can be used simultaneously. In that case
nodes with the same priority form a WFQ subgroup in the sibling group
and arbitration among them is based on assigned weights.
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 0091f23a40f7..135f7382386e 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -118,6 +118,8 @@ struct devlink_rate {
u32 tx_priority;
u32 tx_weight;
+
+ u32 tc_bw[DEVLINK_RATE_TCS_MAX];
};
struct devlink_port {
@@ -1482,6 +1484,8 @@ struct devlink_ops {
u32 tx_priority, struct netlink_ext_ack *extack);
int (*rate_leaf_tx_weight_set)(struct devlink_rate *devlink_rate, void *priv,
u32 tx_weight, struct netlink_ext_ack *extack);
+ int (*rate_leaf_tc_bw_set)(struct devlink_rate *devlink_rate, void *priv,
+ u32 *tc_bw, struct netlink_ext_ack *extack);
int (*rate_node_tx_share_set)(struct devlink_rate *devlink_rate, void *priv,
u64 tx_share, struct netlink_ext_ack *extack);
int (*rate_node_tx_max_set)(struct devlink_rate *devlink_rate, void *priv,
@@ -1490,6 +1494,8 @@ struct devlink_ops {
u32 tx_priority, struct netlink_ext_ack *extack);
int (*rate_node_tx_weight_set)(struct devlink_rate *devlink_rate, void *priv,
u32 tx_weight, struct netlink_ext_ack *extack);
+ int (*rate_node_tc_bw_set)(struct devlink_rate *devlink_rate, void *priv,
+ u32 *tc_bw, struct netlink_ext_ack *extack);
int (*rate_node_new)(struct devlink_rate *rate_node, void **priv,
struct netlink_ext_ack *extack);
int (*rate_node_del)(struct devlink_rate *rate_node, void *priv,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index a5ee0f13740a..e72bcc239afd 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -221,6 +221,11 @@ enum devlink_port_flavour {
*/
};
+/* IEEE 802.1Qaz standard supported values. */
+
+#define DEVLINK_RATE_TCS_MAX 8
+#define DEVLINK_RATE_TC_INDEX_MAX (DEVLINK_RATE_TCS_MAX - 1)
+
enum devlink_rate_type {
DEVLINK_RATE_TYPE_LEAF,
DEVLINK_RATE_TYPE_NODE,
@@ -629,6 +634,10 @@ enum devlink_attr {
DEVLINK_ATTR_REGION_DIRECT, /* flag */
+ DEVLINK_ATTR_RATE_TC_BWS, /* nested */
+ DEVLINK_ATTR_RATE_TC_INDEX, /* u8 */
+ DEVLINK_ATTR_RATE_TC_BW, /* u32 */
+
/* Add new attributes above here, update the spec in
* Documentation/netlink/specs/devlink.yaml and re-generate
* net/devlink/netlink_gen.c.
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
index e340d955cf3b..6d5b8296b049 100644
--- a/net/devlink/netlink_gen.c
+++ b/net/devlink/netlink_gen.c
@@ -45,6 +45,11 @@ const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_
[DEVLINK_PORT_FN_ATTR_CAPS] = NLA_POLICY_BITFIELD32(15),
};
+const struct nla_policy devlink_dl_rate_tc_bws_nl_policy[DEVLINK_ATTR_RATE_TC_BW + 1] = {
+ [DEVLINK_ATTR_RATE_TC_INDEX] = NLA_POLICY_RANGE(NLA_U8, 0, DEVLINK_RATE_TC_INDEX_MAX),
+ [DEVLINK_ATTR_RATE_TC_BW] = NLA_POLICY_RANGE(NLA_U32, 0, 100),
+};
+
const struct nla_policy devlink_dl_selftest_id_nl_policy[DEVLINK_ATTR_SELFTEST_ID_FLASH + 1] = {
[DEVLINK_ATTR_SELFTEST_ID_FLASH] = { .type = NLA_FLAG, },
};
@@ -523,7 +528,7 @@ static const struct nla_policy devlink_rate_get_dump_nl_policy[DEVLINK_ATTR_DEV_
};
/* DEVLINK_CMD_RATE_SET - do */
-static const struct nla_policy devlink_rate_set_nl_policy[DEVLINK_ATTR_RATE_TX_WEIGHT + 1] = {
+static const struct nla_policy devlink_rate_set_nl_policy[DEVLINK_ATTR_RATE_TC_BWS + 1] = {
[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_RATE_NODE_NAME] = { .type = NLA_NUL_STRING, },
@@ -532,10 +537,11 @@ static const struct nla_policy devlink_rate_set_nl_policy[DEVLINK_ATTR_RATE_TX_W
[DEVLINK_ATTR_RATE_TX_PRIORITY] = { .type = NLA_U32, },
[DEVLINK_ATTR_RATE_TX_WEIGHT] = { .type = NLA_U32, },
[DEVLINK_ATTR_RATE_PARENT_NODE_NAME] = { .type = NLA_NUL_STRING, },
+ [DEVLINK_ATTR_RATE_TC_BWS] = NLA_POLICY_NESTED(devlink_dl_rate_tc_bws_nl_policy),
};
/* DEVLINK_CMD_RATE_NEW - do */
-static const struct nla_policy devlink_rate_new_nl_policy[DEVLINK_ATTR_RATE_TX_WEIGHT + 1] = {
+static const struct nla_policy devlink_rate_new_nl_policy[DEVLINK_ATTR_RATE_TC_BWS + 1] = {
[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_RATE_NODE_NAME] = { .type = NLA_NUL_STRING, },
@@ -544,6 +550,7 @@ static const struct nla_policy devlink_rate_new_nl_policy[DEVLINK_ATTR_RATE_TX_W
[DEVLINK_ATTR_RATE_TX_PRIORITY] = { .type = NLA_U32, },
[DEVLINK_ATTR_RATE_TX_WEIGHT] = { .type = NLA_U32, },
[DEVLINK_ATTR_RATE_PARENT_NODE_NAME] = { .type = NLA_NUL_STRING, },
+ [DEVLINK_ATTR_RATE_TC_BWS] = NLA_POLICY_NESTED(devlink_dl_rate_tc_bws_nl_policy),
};
/* DEVLINK_CMD_RATE_DEL - do */
@@ -1191,7 +1198,7 @@ const struct genl_split_ops devlink_nl_ops[74] = {
.doit = devlink_nl_rate_set_doit,
.post_doit = devlink_nl_post_doit,
.policy = devlink_rate_set_nl_policy,
- .maxattr = DEVLINK_ATTR_RATE_TX_WEIGHT,
+ .maxattr = DEVLINK_ATTR_RATE_TC_BWS,
.flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
},
{
@@ -1201,7 +1208,7 @@ const struct genl_split_ops devlink_nl_ops[74] = {
.doit = devlink_nl_rate_new_doit,
.post_doit = devlink_nl_post_doit,
.policy = devlink_rate_new_nl_policy,
- .maxattr = DEVLINK_ATTR_RATE_TX_WEIGHT,
+ .maxattr = DEVLINK_ATTR_RATE_TC_BWS,
.flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
},
{
diff --git a/net/devlink/netlink_gen.h b/net/devlink/netlink_gen.h
index 8f2bd50ddf5e..fb733b5d4ff1 100644
--- a/net/devlink/netlink_gen.h
+++ b/net/devlink/netlink_gen.h
@@ -13,6 +13,7 @@
/* Common nested types */
extern const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_CAPS + 1];
+extern const struct nla_policy devlink_dl_rate_tc_bws_nl_policy[DEVLINK_ATTR_RATE_TC_BW + 1];
extern const struct nla_policy devlink_dl_selftest_id_nl_policy[DEVLINK_ATTR_SELFTEST_ID_FLASH + 1];
/* Ops table for devlink */
diff --git a/net/devlink/rate.c b/net/devlink/rate.c
index 8828ffaf6cbc..56ee4aca2eff 100644
--- a/net/devlink/rate.c
+++ b/net/devlink/rate.c
@@ -80,6 +80,29 @@ devlink_rate_get_from_info(struct devlink *devlink, struct genl_info *info)
return ERR_PTR(-EINVAL);
}
+static int devlink_rate_put_tc_bws(struct sk_buff *msg, u32 *tc_bw)
+{
+ struct nlattr *nla_tc_bw;
+ int i;
+
+ for (i = 0; i < DEVLINK_RATE_TCS_MAX; i++) {
+ nla_tc_bw = nla_nest_start(msg, DEVLINK_ATTR_RATE_TC_BWS);
+ if (!nla_tc_bw)
+ return -EMSGSIZE;
+
+ if (nla_put_u8(msg, DEVLINK_ATTR_RATE_TC_INDEX, i) ||
+ nla_put_u32(msg, DEVLINK_ATTR_RATE_TC_BW, tc_bw[i]))
+ goto nla_put_failure;
+
+ nla_nest_end(msg, nla_tc_bw);
+ }
+ return 0;
+
+nla_put_failure:
+ nla_nest_cancel(msg, nla_tc_bw);
+ return -EMSGSIZE;
+}
+
static int devlink_nl_rate_fill(struct sk_buff *msg,
struct devlink_rate *devlink_rate,
enum devlink_command cmd, u32 portid, u32 seq,
@@ -129,6 +152,9 @@ static int devlink_nl_rate_fill(struct sk_buff *msg,
devlink_rate->parent->name))
goto nla_put_failure;
+ if (devlink_rate_put_tc_bws(msg, devlink_rate->tc_bw))
+ goto nla_put_failure;
+
genlmsg_end(msg, hdr);
return 0;
@@ -316,6 +342,89 @@ devlink_nl_rate_parent_node_set(struct devlink_rate *devlink_rate,
return 0;
}
+static int devlink_nl_rate_tc_bw_parse(struct nlattr *parent_nest, u32 *tc_bw,
+ unsigned long *bitmap, struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
+ u8 tc_index;
+
+ nla_parse_nested(tb, DEVLINK_ATTR_MAX, parent_nest, devlink_dl_rate_tc_bws_nl_policy,
+ extack);
+ if (!tb[DEVLINK_ATTR_RATE_TC_INDEX]) {
+ NL_SET_ERR_ATTR_MISS(extack, parent_nest, DEVLINK_ATTR_RATE_TC_INDEX);
+ return -EINVAL;
+ }
+
+ tc_index = nla_get_u8(tb[DEVLINK_ATTR_RATE_TC_INDEX]);
+
+ if (!tb[DEVLINK_ATTR_RATE_TC_BW]) {
+ NL_SET_ERR_ATTR_MISS(extack, parent_nest, DEVLINK_ATTR_RATE_TC_BW);
+ return -EINVAL;
+ }
+
+ if (test_and_set_bit(tc_index, bitmap)) {
+ NL_SET_ERR_MSG_FMT(extack, "Duplicate traffic class index specified (%u)",
+ tc_index);
+ return -EINVAL;
+ }
+
+ tc_bw[tc_index] = nla_get_u32(tb[DEVLINK_ATTR_RATE_TC_BW]);
+
+ return 0;
+}
+
+static int devlink_nl_rate_tc_bw_set(struct devlink_rate *devlink_rate,
+ struct genl_info *info)
+{
+ DECLARE_BITMAP(bitmap, DEVLINK_RATE_TCS_MAX) = {};
+ struct devlink *devlink = devlink_rate->devlink;
+ const struct devlink_ops *ops = devlink->ops;
+ int rem, err = -EOPNOTSUPP, i, total = 0;
+ u32 tc_bw[DEVLINK_RATE_TCS_MAX] = {};
+ struct nlattr *attr;
+
+ nla_for_each_attr(attr, genlmsg_data(info->genlhdr),
+ genlmsg_len(info->genlhdr), rem) {
+ if (nla_type(attr) == DEVLINK_ATTR_RATE_TC_BWS) {
+ err = devlink_nl_rate_tc_bw_parse(attr, tc_bw, bitmap, info->extack);
+ if (err)
+ return err;
+ }
+ }
+
+ for (i = 0; i < DEVLINK_RATE_TCS_MAX; i++) {
+ if (!test_bit(i, bitmap)) {
+ NL_SET_ERR_MSG_FMT(info->extack,
+ "Bandwidth values must be specified for all %u traffic classes",
+ DEVLINK_RATE_TCS_MAX);
+ return -EINVAL;
+ }
+
+ total += tc_bw[i];
+ }
+
+ if (total && total != 100) {
+ NL_SET_ERR_MSG_FMT(info->extack,
+ "Sum of all traffic class bandwidth values must be 100, got %u",
+ total);
+ return -EINVAL;
+ }
+
+ if (devlink_rate_is_leaf(devlink_rate))
+ err = ops->rate_leaf_tc_bw_set(devlink_rate, devlink_rate->priv, tc_bw,
+ info->extack);
+ else if (devlink_rate_is_node(devlink_rate))
+ err = ops->rate_node_tc_bw_set(devlink_rate, devlink_rate->priv, tc_bw,
+ info->extack);
+
+ if (err)
+ return err;
+
+ memcpy(devlink_rate->tc_bw, tc_bw, sizeof(tc_bw));
+
+ return 0;
+}
+
static int devlink_nl_rate_set(struct devlink_rate *devlink_rate,
const struct devlink_ops *ops,
struct genl_info *info)
@@ -388,6 +497,12 @@ static int devlink_nl_rate_set(struct devlink_rate *devlink_rate,
return err;
}
+ if (attrs[DEVLINK_ATTR_RATE_TC_BWS]) {
+ err = devlink_nl_rate_tc_bw_set(devlink_rate, info);
+ if (err)
+ return err;
+ }
+
return 0;
}
@@ -423,6 +538,12 @@ static bool devlink_rate_set_ops_supported(const struct devlink_ops *ops,
"TX weight set isn't supported for the leafs");
return false;
}
+ if (attrs[DEVLINK_ATTR_RATE_TC_BWS] && !ops->rate_leaf_tc_bw_set) {
+ NL_SET_ERR_MSG_ATTR(info->extack,
+ attrs[DEVLINK_ATTR_RATE_TC_BWS],
+ "TC bandwidth set isn't supported for the leafs");
+ return false;
+ }
} else if (type == DEVLINK_RATE_TYPE_NODE) {
if (attrs[DEVLINK_ATTR_RATE_TX_SHARE] && !ops->rate_node_tx_share_set) {
NL_SET_ERR_MSG(info->extack, "TX share set isn't supported for the nodes");
@@ -449,6 +570,12 @@ static bool devlink_rate_set_ops_supported(const struct devlink_ops *ops,
"TX weight set isn't supported for the nodes");
return false;
}
+ if (attrs[DEVLINK_ATTR_RATE_TC_BWS] && !ops->rate_node_tc_bw_set) {
+ NL_SET_ERR_MSG_ATTR(info->extack,
+ attrs[DEVLINK_ATTR_RATE_TC_BWS],
+ "TC bandwidth set isn't supported for the nodes");
+ return false;
+ }
} else {
WARN(1, "Unknown type of rate object");
return false;
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test
2025-05-20 18:38 [PATCH net-next V10 0/6] Support rate management on traffic classes in devlink and mlx5 Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 1/6] devlink: Extend devlink rate API with traffic classes bandwidth management Tariq Toukan
@ 2025-05-20 18:38 ` Tariq Toukan
2025-05-20 22:59 ` Jakub Kicinski
2025-05-20 18:38 ` [PATCH net-next V10 3/6] net/mlx5: Add no-op implementation for setting tc-bw on rate objects Tariq Toukan
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Tariq Toukan @ 2025-05-20 18:38 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Jiri Pirko, Gal Pressman, Leon Romanovsky, Donald Hunter,
Jiri Pirko, Jonathan Corbet, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Carolina Jubran, Cosmin Ratiu
From: Carolina Jubran <cjubran@nvidia.com>
Test verifies that netdevsim correctly implements devlink ops callbacks
that set tc-bw on leaf or node rate object.
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/netdevsim/dev.c | 43 ++++++++++++++++
drivers/net/netdevsim/netdevsim.h | 1 +
.../drivers/net/netdevsim/devlink.sh | 51 +++++++++++++++++++
3 files changed, 95 insertions(+)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 3e0b61202f0c..b3647691060c 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -388,6 +388,17 @@ static const struct file_operations nsim_dev_rate_parent_fops = {
.owner = THIS_MODULE,
};
+static void nsim_dev_tc_bw_debugfs_init(struct dentry *ddir, u32 *tc_bw)
+{
+ int i;
+
+ for (i = 0; i < DEVLINK_RATE_TCS_MAX; i++) {
+ char name[16];
+
+ snprintf(name, sizeof(name), "tc%d_bw", i);
+ debugfs_create_u32(name, 0400, ddir, &tc_bw[i]);
+ }
+}
static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev,
struct nsim_dev_port *nsim_dev_port)
{
@@ -415,6 +426,8 @@ static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev,
nsim_dev_port->ddir,
&nsim_dev_port->parent_name,
&nsim_dev_rate_parent_fops);
+ nsim_dev_tc_bw_debugfs_init(nsim_dev_port->ddir,
+ nsim_dev_port->tc_bw);
}
debugfs_create_symlink("dev", nsim_dev_port->ddir, dev_link_name);
@@ -1172,6 +1185,19 @@ static int nsim_rate_bytes_to_units(char *name, u64 *rate, struct netlink_ext_ac
return 0;
}
+static int nsim_leaf_tc_bw_set(struct devlink_rate *devlink_rate,
+ void *priv, u32 *tc_bw,
+ struct netlink_ext_ack *extack)
+{
+ struct nsim_dev_port *nsim_dev_port = priv;
+ int i;
+
+ for (i = 0; i < DEVLINK_RATE_TCS_MAX; i++)
+ nsim_dev_port->tc_bw[i] = tc_bw[i];
+
+ return 0;
+}
+
static int nsim_leaf_tx_share_set(struct devlink_rate *devlink_rate, void *priv,
u64 tx_share, struct netlink_ext_ack *extack)
{
@@ -1210,8 +1236,21 @@ struct nsim_rate_node {
char *parent_name;
u16 tx_share;
u16 tx_max;
+ u32 tc_bw[DEVLINK_RATE_TCS_MAX];
};
+static int nsim_node_tc_bw_set(struct devlink_rate *devlink_rate, void *priv,
+ u32 *tc_bw, struct netlink_ext_ack *extack)
+{
+ struct nsim_rate_node *nsim_node = priv;
+ int i;
+
+ for (i = 0; i < DEVLINK_RATE_TCS_MAX; i++)
+ nsim_node->tc_bw[i] = tc_bw[i];
+
+ return 0;
+}
+
static int nsim_node_tx_share_set(struct devlink_rate *devlink_rate, void *priv,
u64 tx_share, struct netlink_ext_ack *extack)
{
@@ -1264,6 +1303,8 @@ static int nsim_rate_node_new(struct devlink_rate *node, void **priv,
&nsim_node->parent_name,
&nsim_dev_rate_parent_fops);
+ nsim_dev_tc_bw_debugfs_init(nsim_node->ddir, nsim_node->tc_bw);
+
*priv = nsim_node;
return 0;
}
@@ -1340,8 +1381,10 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
.trap_policer_counter_get = nsim_dev_devlink_trap_policer_counter_get,
.rate_leaf_tx_share_set = nsim_leaf_tx_share_set,
.rate_leaf_tx_max_set = nsim_leaf_tx_max_set,
+ .rate_leaf_tc_bw_set = nsim_leaf_tc_bw_set,
.rate_node_tx_share_set = nsim_node_tx_share_set,
.rate_node_tx_max_set = nsim_node_tx_max_set,
+ .rate_node_tc_bw_set = nsim_node_tc_bw_set,
.rate_node_new = nsim_rate_node_new,
.rate_node_del = nsim_rate_node_del,
.rate_leaf_parent_set = nsim_rate_leaf_parent_set,
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index d04401f0bdf7..4ec6735e5af4 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -282,6 +282,7 @@ struct nsim_dev_port {
struct dentry *ddir;
struct dentry *rate_parent;
char *parent_name;
+ u32 tc_bw[DEVLINK_RATE_TCS_MAX];
struct netdevsim *ns;
};
diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
index b5ea2526f23c..09068d69d2cc 100755
--- a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
@@ -608,6 +608,44 @@ rate_attr_parent_check()
check_err $? "Unexpected parent attr value $api_value != $parent"
}
+rate_attr_tc_bw_check()
+{
+ local handle=$1
+ local tc_bw=$2
+ local debug_file=$3
+
+ local tc_bw_str=""
+ for bw in $tc_bw; do
+ local tc=${bw%%:*}
+ local value=${bw##*:}
+ tc_bw_str="$tc_bw_str $tc:$value"
+ done
+ tc_bw_str=${tc_bw_str# }
+
+ rate_attr_set $handle tc-bw "$tc_bw_str"
+ check_err $? "Failed to set tc-bw values"
+
+ for bw in $tc_bw; do
+ local tc=${bw%%:*}
+ local value=${bw##*:}
+ local debug_value=$(cat $debug_file/tc${tc}_bw)
+ check_err $? "Failed to read tc-bw value from debugfs for tc$tc"
+ [ "$debug_value" == "$value" ]
+ check_err $? "Unexpected tc-bw debug value for tc$tc: $debug_value != $value"
+ done
+
+ for bw in $tc_bw; do
+ local tc=${bw%%:*}
+ local expected_value=${bw##*:}
+ local api_value=$(rate_attr_get $handle tc_$tc)
+ if [ "$api_value" = "null" ]; then
+ api_value=0
+ fi
+ [ "$api_value" == "$expected_value" ]
+ check_err $? "Unexpected tc-bw value for tc$tc: $api_value != $expected_value"
+ done
+}
+
rate_node_add()
{
local handle=$1
@@ -649,6 +687,13 @@ rate_test()
rate=$(($rate+100))
done
+ local tc_bw="0:0 1:40 2:0 3:0 4:0 5:0 6:60 7:0"
+ for r_obj in $leafs
+ do
+ rate_attr_tc_bw_check $r_obj "$tc_bw" \
+ $DEBUGFS_DIR/ports/${r_obj##*/}
+ done
+
local node1_name='group1'
local node1="$DL_HANDLE/$node1_name"
rate_node_add "$node1"
@@ -666,6 +711,12 @@ rate_test()
rate_attr_tx_rate_check $node1 tx_max $node_tx_max \
$DEBUGFS_DIR/rate_nodes/${node1##*/}/tx_max
+
+ local tc_bw="0:20 1:0 2:0 3:0 4:0 5:20 6:60 7:0"
+ rate_attr_tc_bw_check $node1 "$tc_bw" \
+ $DEBUGFS_DIR/rate_nodes/${node1##*/}
+
+
rate_node_del "$node1"
check_err $? "Failed to delete node $node1"
local num_nodes=`rate_nodes_get $DL_HANDLE | wc -w`
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next V10 3/6] net/mlx5: Add no-op implementation for setting tc-bw on rate objects
2025-05-20 18:38 [PATCH net-next V10 0/6] Support rate management on traffic classes in devlink and mlx5 Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 1/6] devlink: Extend devlink rate API with traffic classes bandwidth management Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test Tariq Toukan
@ 2025-05-20 18:38 ` Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 4/6] net/mlx5: Add support for setting tc-bw on nodes Tariq Toukan
` (2 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Tariq Toukan @ 2025-05-20 18:38 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Jiri Pirko, Gal Pressman, Leon Romanovsky, Donald Hunter,
Jiri Pirko, Jonathan Corbet, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Carolina Jubran, Cosmin Ratiu
From: Carolina Jubran <cjubran@nvidia.com>
Introduce `mlx5_esw_devlink_rate_node_tc_bw_set()` and
`mlx5_esw_devlink_rate_leaf_tc_bw_set()` with no-op logic.
Future patches will add support for setting traffic class bandwidth
on rate objects.
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/devlink.c | 2 ++
.../net/ethernet/mellanox/mlx5/core/esw/qos.c | 20 +++++++++++++++++++
.../net/ethernet/mellanox/mlx5/core/esw/qos.h | 8 ++++++++
3 files changed, 30 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 73cd74644378..47d3acd011cf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -323,6 +323,8 @@ static const struct devlink_ops mlx5_devlink_ops = {
.eswitch_encap_mode_get = mlx5_devlink_eswitch_encap_mode_get,
.rate_leaf_tx_share_set = mlx5_esw_devlink_rate_leaf_tx_share_set,
.rate_leaf_tx_max_set = mlx5_esw_devlink_rate_leaf_tx_max_set,
+ .rate_leaf_tc_bw_set = mlx5_esw_devlink_rate_leaf_tc_bw_set,
+ .rate_node_tc_bw_set = mlx5_esw_devlink_rate_node_tc_bw_set,
.rate_node_tx_share_set = mlx5_esw_devlink_rate_node_tx_share_set,
.rate_node_tx_max_set = mlx5_esw_devlink_rate_node_tx_max_set,
.rate_node_new = mlx5_esw_devlink_rate_node_new,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
index b6ae384396b3..ec706e9352e1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
@@ -906,6 +906,26 @@ int mlx5_esw_devlink_rate_leaf_tx_max_set(struct devlink_rate *rate_leaf, void *
return err;
}
+int mlx5_esw_devlink_rate_leaf_tc_bw_set(struct devlink_rate *rate_leaf,
+ void *priv,
+ u32 *tc_bw,
+ struct netlink_ext_ack *extack)
+{
+ NL_SET_ERR_MSG_MOD(extack,
+ "TC bandwidth shares are not supported on leafs");
+ return -EOPNOTSUPP;
+}
+
+int mlx5_esw_devlink_rate_node_tc_bw_set(struct devlink_rate *rate_node,
+ void *priv,
+ u32 *tc_bw,
+ struct netlink_ext_ack *extack)
+{
+ NL_SET_ERR_MSG_MOD(extack,
+ "TC bandwidth shares are not supported on nodes");
+ return -EOPNOTSUPP;
+}
+
int mlx5_esw_devlink_rate_node_tx_share_set(struct devlink_rate *rate_node, void *priv,
u64 tx_share, struct netlink_ext_ack *extack)
{
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h
index ed40ec8f027e..0a50982b0e27 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h
@@ -21,6 +21,14 @@ int mlx5_esw_devlink_rate_leaf_tx_share_set(struct devlink_rate *rate_leaf, void
u64 tx_share, struct netlink_ext_ack *extack);
int mlx5_esw_devlink_rate_leaf_tx_max_set(struct devlink_rate *rate_leaf, void *priv,
u64 tx_max, struct netlink_ext_ack *extack);
+int mlx5_esw_devlink_rate_leaf_tc_bw_set(struct devlink_rate *rate_node,
+ void *priv,
+ u32 *tc_bw,
+ struct netlink_ext_ack *extack);
+int mlx5_esw_devlink_rate_node_tc_bw_set(struct devlink_rate *rate_node,
+ void *priv,
+ u32 *tc_bw,
+ struct netlink_ext_ack *extack);
int mlx5_esw_devlink_rate_node_tx_share_set(struct devlink_rate *rate_node, void *priv,
u64 tx_share, struct netlink_ext_ack *extack);
int mlx5_esw_devlink_rate_node_tx_max_set(struct devlink_rate *rate_node, void *priv,
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next V10 4/6] net/mlx5: Add support for setting tc-bw on nodes
2025-05-20 18:38 [PATCH net-next V10 0/6] Support rate management on traffic classes in devlink and mlx5 Tariq Toukan
` (2 preceding siblings ...)
2025-05-20 18:38 ` [PATCH net-next V10 3/6] net/mlx5: Add no-op implementation for setting tc-bw on rate objects Tariq Toukan
@ 2025-05-20 18:38 ` Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 5/6] net/mlx5: Add traffic class scheduling support for vport QoS Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 6/6] net/mlx5: Manage TC arbiter nodes and implement full support for tc-bw Tariq Toukan
5 siblings, 0 replies; 16+ messages in thread
From: Tariq Toukan @ 2025-05-20 18:38 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Jiri Pirko, Gal Pressman, Leon Romanovsky, Donald Hunter,
Jiri Pirko, Jonathan Corbet, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Carolina Jubran, Cosmin Ratiu
From: Carolina Jubran <cjubran@nvidia.com>
Introduce support for enabling and disabling Traffic Class (TC)
arbitration for existing devlink rate nodes. This patch adds support
for a new scheduling node type, `SCHED_NODE_TYPE_TC_ARBITER_TSAR`.
Key changes include:
- New helper functions for transitioning existing rate nodes to TC
arbiter nodes and vice versa. These functions handle the allocation
of TC arbiter nodes, copying of child nodes, and restoring vport QoS
settings when TC arbitration is disabled.
- Implementation of `mlx5_esw_devlink_rate_node_tc_bw_set()` to manage
tc-bw configuration on nodes.
- Introduced stubs for `esw_qos_tc_arbiter_scheduling_setup()` and
`esw_qos_tc_arbiter_scheduling_teardown()`, which will be extended in
future patches to provide full support for tc-bw on devlink rate
objects.
- Validation functions for tc-bw settings, allowing graceful handling
of unsupported traffic class bandwidth configurations.
- Updated `__esw_qos_alloc_node()` to insert the new node into the
parent’s children list only if the parent is not NULL. For the root
TSAR, the new node is inserted directly after the allocation call.
- Don't allow `tc-bw` configuration for nodes containing non-leaf
children.
This patch lays the groundwork for future support for configuring tc-bw
on devlink rate nodes. Although the infrastructure is in place, full
support for tc-bw is not yet implemented; attempts to set tc-bw on
nodes will return `-EOPNOTSUPP`.
No functional changes are introduced at this stage.
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/esw/qos.c | 313 +++++++++++++++++-
1 file changed, 304 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
index ec706e9352e1..1066992c1503 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
@@ -64,11 +64,13 @@ static void esw_qos_domain_release(struct mlx5_eswitch *esw)
enum sched_node_type {
SCHED_NODE_TYPE_VPORTS_TSAR,
SCHED_NODE_TYPE_VPORT,
+ SCHED_NODE_TYPE_TC_ARBITER_TSAR,
};
static const char * const sched_node_type_str[] = {
[SCHED_NODE_TYPE_VPORTS_TSAR] = "vports TSAR",
[SCHED_NODE_TYPE_VPORT] = "vport",
+ [SCHED_NODE_TYPE_TC_ARBITER_TSAR] = "TC Arbiter TSAR",
};
struct mlx5_esw_sched_node {
@@ -106,6 +108,13 @@ static void esw_qos_node_attach_to_parent(struct mlx5_esw_sched_node *node)
}
}
+static int esw_qos_num_tcs(struct mlx5_core_dev *dev)
+{
+ int num_tcs = mlx5_max_tc(dev) + 1;
+
+ return num_tcs < DEVLINK_RATE_TCS_MAX ? num_tcs : DEVLINK_RATE_TCS_MAX;
+}
+
static void
esw_qos_node_set_parent(struct mlx5_esw_sched_node *node, struct mlx5_esw_sched_node *parent)
{
@@ -116,6 +125,27 @@ esw_qos_node_set_parent(struct mlx5_esw_sched_node *node, struct mlx5_esw_sched_
esw_qos_node_attach_to_parent(node);
}
+static void esw_qos_nodes_set_parent(struct list_head *nodes,
+ struct mlx5_esw_sched_node *parent)
+{
+ struct mlx5_esw_sched_node *node, *tmp;
+
+ list_for_each_entry_safe(node, tmp, nodes, entry) {
+ esw_qos_node_set_parent(node, parent);
+ if (!list_empty(&node->children) &&
+ parent->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR) {
+ struct mlx5_esw_sched_node *child;
+
+ list_for_each_entry(child, &node->children, entry) {
+ struct mlx5_vport *vport = child->vport;
+
+ if (vport)
+ vport->qos.sched_node->parent = parent;
+ }
+ }
+ }
+}
+
void mlx5_esw_qos_vport_qos_free(struct mlx5_vport *vport)
{
kfree(vport->qos.sched_node);
@@ -141,16 +171,24 @@ mlx5_esw_qos_vport_get_parent(const struct mlx5_vport *vport)
static void esw_qos_sched_elem_warn(struct mlx5_esw_sched_node *node, int err, const char *op)
{
- if (node->vport) {
+ switch (node->type) {
+ case SCHED_NODE_TYPE_VPORT:
esw_warn(node->esw->dev,
"E-Switch %s %s scheduling element failed (vport=%d,err=%d)\n",
op, sched_node_type_str[node->type], node->vport->vport, err);
- return;
+ break;
+ case SCHED_NODE_TYPE_TC_ARBITER_TSAR:
+ case SCHED_NODE_TYPE_VPORTS_TSAR:
+ esw_warn(node->esw->dev,
+ "E-Switch %s %s scheduling element failed (err=%d)\n",
+ op, sched_node_type_str[node->type], err);
+ break;
+ default:
+ esw_warn(node->esw->dev,
+ "E-Switch %s scheduling element failed (err=%d)\n",
+ op, err);
+ break;
}
-
- esw_warn(node->esw->dev,
- "E-Switch %s %s scheduling element failed (err=%d)\n",
- op, sched_node_type_str[node->type], err);
}
static int esw_qos_node_create_sched_element(struct mlx5_esw_sched_node *node, void *ctx,
@@ -388,6 +426,14 @@ __esw_qos_alloc_node(struct mlx5_eswitch *esw, u32 tsar_ix, enum sched_node_type
node->parent = parent;
INIT_LIST_HEAD(&node->children);
esw_qos_node_attach_to_parent(node);
+ if (!parent) {
+ /* The caller is responsible for inserting the node into the
+ * parent list if necessary. This function can also be used with
+ * a NULL parent, which doesn't necessarily indicate that it
+ * refers to the root scheduling element.
+ */
+ list_del_init(&node->entry);
+ }
return node;
}
@@ -426,6 +472,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);
esw_qos_normalize_min_rate(esw, NULL, extack);
trace_mlx5_esw_node_qos_create(esw->dev, node, node->ix);
@@ -498,6 +545,9 @@ static int esw_qos_create(struct mlx5_eswitch *esw, struct netlink_ext_ack *exta
SCHED_NODE_TYPE_VPORTS_TSAR,
NULL))
esw->qos.node0 = ERR_PTR(-ENOMEM);
+ else
+ list_add_tail(&esw->qos.node0->entry,
+ &esw->qos.domain->nodes);
}
if (IS_ERR(esw->qos.node0)) {
err = PTR_ERR(esw->qos.node0);
@@ -555,6 +605,18 @@ static void esw_qos_put(struct mlx5_eswitch *esw)
esw_qos_destroy(esw);
}
+static void
+esw_qos_tc_arbiter_scheduling_teardown(struct mlx5_esw_sched_node *node,
+ struct netlink_ext_ack *extack)
+{}
+
+static int esw_qos_tc_arbiter_scheduling_setup(struct mlx5_esw_sched_node *node,
+ struct netlink_ext_ack *extack)
+{
+ NL_SET_ERR_MSG_MOD(extack, "TC arbiter elements are not supported.");
+ return -EOPNOTSUPP;
+}
+
static void esw_qos_vport_disable(struct mlx5_vport *vport, struct netlink_ext_ack *extack)
{
struct mlx5_esw_sched_node *vport_node = vport->qos.sched_node;
@@ -723,6 +785,195 @@ static int esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw
return err;
}
+static void
+esw_qos_switch_vport_tcs_to_vport(struct mlx5_esw_sched_node *tc_arbiter_node,
+ struct mlx5_esw_sched_node *node,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *vports_tc_node, *vport_tc_node, *tmp;
+
+ vports_tc_node = list_first_entry(&tc_arbiter_node->children,
+ struct mlx5_esw_sched_node,
+ entry);
+
+ list_for_each_entry_safe(vport_tc_node, tmp, &vports_tc_node->children,
+ entry)
+ esw_qos_vport_update_parent(vport_tc_node->vport, node, extack);
+}
+
+static int esw_qos_switch_tc_arbiter_node_to_vports(
+ struct mlx5_esw_sched_node *tc_arbiter_node,
+ struct mlx5_esw_sched_node *node,
+ struct netlink_ext_ack *extack)
+{
+ u32 parent_tsar_ix = node->parent ?
+ node->parent->ix : node->esw->qos.root_tsar_ix;
+ int err;
+
+ err = esw_qos_create_node_sched_elem(node->esw->dev, parent_tsar_ix,
+ node->max_rate, node->bw_share,
+ &node->ix);
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Failed to create scheduling element for vports node when disabliing vports TC QoS");
+ return err;
+ }
+
+ node->type = SCHED_NODE_TYPE_VPORTS_TSAR;
+
+ /* Disable TC QoS for vports in the arbiter node. */
+ esw_qos_switch_vport_tcs_to_vport(tc_arbiter_node, node, extack);
+
+ return 0;
+}
+
+static int esw_qos_switch_vports_node_to_tc_arbiter(
+ struct mlx5_esw_sched_node *node,
+ struct mlx5_esw_sched_node *tc_arbiter_node,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *vport_node, *tmp;
+ struct mlx5_vport *vport;
+ int err;
+
+ /* Enable TC QoS for each vport in the node. */
+ list_for_each_entry_safe(vport_node, tmp, &node->children, entry) {
+ vport = vport_node->vport;
+ err = esw_qos_vport_update_parent(vport, tc_arbiter_node,
+ extack);
+ if (err)
+ goto err_out;
+ }
+
+ /* Destroy the current vports node TSAR. */
+ err = mlx5_destroy_scheduling_element_cmd(node->esw->dev,
+ SCHEDULING_HIERARCHY_E_SWITCH,
+ node->ix);
+ if (err)
+ goto err_out;
+
+ return 0;
+err_out:
+ /* Restore vports back into the node if an error occurs. */
+ esw_qos_switch_vport_tcs_to_vport(tc_arbiter_node, node, NULL);
+
+ return err;
+}
+
+static struct mlx5_esw_sched_node *
+esw_qos_move_node(struct mlx5_esw_sched_node *curr_node)
+{
+ struct mlx5_esw_sched_node *new_node;
+
+ new_node = __esw_qos_alloc_node(curr_node->esw, curr_node->ix,
+ curr_node->type, NULL);
+ if (!IS_ERR(new_node))
+ esw_qos_nodes_set_parent(&curr_node->children, new_node);
+
+ return new_node;
+}
+
+static int esw_qos_node_disable_tc_arbitration(struct mlx5_esw_sched_node *node,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *curr_node;
+ int err;
+
+ if (node->type != SCHED_NODE_TYPE_TC_ARBITER_TSAR)
+ return 0;
+
+ /* Allocate a new rate node to hold the current state, which will allow
+ * for restoring the vports back to this node after disabling TC
+ * arbitration.
+ */
+ curr_node = esw_qos_move_node(node);
+ if (IS_ERR(curr_node)) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed setting up vports node");
+ return PTR_ERR(curr_node);
+ }
+
+ /* Disable TC QoS for all vports, and assign them back to the node. */
+ err = esw_qos_switch_tc_arbiter_node_to_vports(curr_node, node, extack);
+ if (err)
+ goto err_out;
+
+ /* Clean up the TC arbiter node after disabling TC QoS for vports. */
+ esw_qos_tc_arbiter_scheduling_teardown(curr_node, extack);
+ goto out;
+err_out:
+ esw_qos_nodes_set_parent(&curr_node->children, node);
+out:
+ __esw_qos_free_node(curr_node);
+ return err;
+}
+
+static int esw_qos_node_enable_tc_arbitration(struct mlx5_esw_sched_node *node,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *curr_node, *child;
+ int err, new_level, max_level;
+
+ if (node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR)
+ return 0;
+
+ /* Increase the hierarchy level by one to account for the additional
+ * vports TC scheduling node, and verify that the new level does not
+ * exceed the maximum allowed depth.
+ */
+ new_level = node->level + 1;
+ max_level = 1 << MLX5_CAP_QOS(node->esw->dev, log_esw_max_sched_depth);
+ if (new_level > max_level) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "TC arbitration on nodes is not supported beyond max scheduling depth");
+ return -EOPNOTSUPP;
+ }
+
+ /* Ensure the node does not contain non-leaf children before assigning
+ * TC bandwidth.
+ */
+ if (!list_empty(&node->children)) {
+ list_for_each_entry(child, &node->children, entry) {
+ if (!child->vport) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Cannot configure TC bandwidth on a node with non-leaf children");
+ return -EOPNOTSUPP;
+ }
+ }
+ }
+
+ /* Allocate a new node that will store the information of the current
+ * node. This will be used later to restore the node if necessary.
+ */
+ curr_node = esw_qos_move_node(node);
+ if (IS_ERR(curr_node)) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed setting up node TC QoS");
+ return PTR_ERR(curr_node);
+ }
+
+ /* Initialize the TC arbiter node for QoS management.
+ * This step prepares the node for handling Traffic Class arbitration.
+ */
+ err = esw_qos_tc_arbiter_scheduling_setup(node, extack);
+ if (err)
+ goto err_setup;
+
+ /* Enable TC QoS for each vport within the current node. */
+ err = esw_qos_switch_vports_node_to_tc_arbiter(curr_node, node, extack);
+ if (err)
+ goto err_switch_vports;
+ goto out;
+
+err_switch_vports:
+ esw_qos_tc_arbiter_scheduling_teardown(node, NULL);
+ node->ix = curr_node->ix;
+ node->type = curr_node->type;
+err_setup:
+ esw_qos_nodes_set_parent(&curr_node->children, node);
+out:
+ __esw_qos_free_node(curr_node);
+ return err;
+}
+
static u32 mlx5_esw_qos_lag_link_speed_get_locked(struct mlx5_core_dev *mdev)
{
struct ethtool_link_ksettings lksettings;
@@ -848,6 +1099,31 @@ static int esw_qos_devlink_rate_to_mbps(struct mlx5_core_dev *mdev, const char *
return 0;
}
+static bool esw_qos_validate_unsupported_tc_bw(struct mlx5_eswitch *esw,
+ u32 *tc_bw)
+{
+ int i, num_tcs = esw_qos_num_tcs(esw->dev);
+
+ for (i = num_tcs; i < DEVLINK_RATE_TCS_MAX; i++) {
+ if (tc_bw[i])
+ return false;
+ }
+
+ return true;
+}
+
+static bool esw_qos_tc_bw_disabled(u32 *tc_bw)
+{
+ int i;
+
+ for (i = 0; i < DEVLINK_RATE_TCS_MAX; i++) {
+ if (tc_bw[i])
+ return false;
+ }
+
+ return true;
+}
+
int mlx5_esw_qos_init(struct mlx5_eswitch *esw)
{
if (esw->qos.domain)
@@ -921,9 +1197,28 @@ int mlx5_esw_devlink_rate_node_tc_bw_set(struct devlink_rate *rate_node,
u32 *tc_bw,
struct netlink_ext_ack *extack)
{
- NL_SET_ERR_MSG_MOD(extack,
- "TC bandwidth shares are not supported on nodes");
- return -EOPNOTSUPP;
+ 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)) {
+ 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;
+ }
+
+ err = esw_qos_node_enable_tc_arbitration(node, extack);
+unlock:
+ esw_qos_unlock(esw);
+ return err;
}
int mlx5_esw_devlink_rate_node_tx_share_set(struct devlink_rate *rate_node, void *priv,
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next V10 5/6] net/mlx5: Add traffic class scheduling support for vport QoS
2025-05-20 18:38 [PATCH net-next V10 0/6] Support rate management on traffic classes in devlink and mlx5 Tariq Toukan
` (3 preceding siblings ...)
2025-05-20 18:38 ` [PATCH net-next V10 4/6] net/mlx5: Add support for setting tc-bw on nodes Tariq Toukan
@ 2025-05-20 18:38 ` Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 6/6] net/mlx5: Manage TC arbiter nodes and implement full support for tc-bw Tariq Toukan
5 siblings, 0 replies; 16+ messages in thread
From: Tariq Toukan @ 2025-05-20 18:38 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Jiri Pirko, Gal Pressman, Leon Romanovsky, Donald Hunter,
Jiri Pirko, Jonathan Corbet, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Carolina Jubran, Cosmin Ratiu
From: Carolina Jubran <cjubran@nvidia.com>
Introduce support for traffic class (TC) scheduling on vports by
allowing the vport to own multiple TC scheduling nodes. This patch
enables more granular control of QoS by defining three distinct QoS
states for vports, each providing unique scheduling behavior:
1. Regular QoS: The `sched_node` represents the vport directly,
handling QoS as a single scheduling entity.
2. TC QoS on the vport: The `sched_node` acts as a TC arbiter, enabling
TC scheduling directly on the vport.
3. TC QoS on the parent node: The `sched_node` functions as a rate
limiter, with TC arbitration enabled at the parent level, associating
multiple scheduling nodes with each vport.
Key changes include:
- Added support for new scheduling elements, vport traffic class and
rate limiter.
- New helper functions for creating, destroying, and restoring vport TC
scheduling nodes, handling transitions between regular QoS and TC
arbitration states.
- Updated `esw_qos_vport_enable()` and `esw_qos_vport_disable()` to
support both regular QoS and TC arbitration states, ensuring consistent
transitions between scheduling modes.
- Introduced a `sched_nodes` array under `vport->qos` to store multiple
TC scheduling nodes per vport, enabling finer control over per-TC QoS.
- Enhanced `esw_qos_vport_update_parent()` to handle transitions between
the three QoS states based on the current and new parent node types.
This patch lays the groundwork for future support for configuring tc-bw
on vports. Although the infrastructure is in place, full support for
tc-bw is not yet implemented; attempts to set tc-bw on vports will
return `-EOPNOTSUPP`.
No functional changes are introduced at this stage.
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/esw/qos.c | 438 ++++++++++++++++--
.../net/ethernet/mellanox/mlx5/core/eswitch.h | 14 +-
2 files changed, 422 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
index 1066992c1503..dec3bed682b7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
@@ -65,12 +65,16 @@ enum sched_node_type {
SCHED_NODE_TYPE_VPORTS_TSAR,
SCHED_NODE_TYPE_VPORT,
SCHED_NODE_TYPE_TC_ARBITER_TSAR,
+ SCHED_NODE_TYPE_RATE_LIMITER,
+ SCHED_NODE_TYPE_VPORT_TC,
};
static const char * const sched_node_type_str[] = {
[SCHED_NODE_TYPE_VPORTS_TSAR] = "vports TSAR",
[SCHED_NODE_TYPE_VPORT] = "vport",
[SCHED_NODE_TYPE_TC_ARBITER_TSAR] = "TC Arbiter TSAR",
+ [SCHED_NODE_TYPE_RATE_LIMITER] = "Rate Limiter",
+ [SCHED_NODE_TYPE_VPORT_TC] = "vport TC",
};
struct mlx5_esw_sched_node {
@@ -94,6 +98,8 @@ struct mlx5_esw_sched_node {
struct mlx5_vport *vport;
/* Level in the hierarchy. The root node level is 1. */
u8 level;
+ /* Valid only when this node represents a traffic class. */
+ u8 tc;
};
static void esw_qos_node_attach_to_parent(struct mlx5_esw_sched_node *node)
@@ -148,6 +154,15 @@ static void esw_qos_nodes_set_parent(struct list_head *nodes,
void mlx5_esw_qos_vport_qos_free(struct mlx5_vport *vport)
{
+ if (vport->qos.sched_nodes) {
+ int num_tcs = esw_qos_num_tcs(vport->qos.sched_node->esw->dev);
+ int i;
+
+ for (i = 0; i < num_tcs; i++)
+ kfree(vport->qos.sched_nodes[i]);
+ kfree(vport->qos.sched_nodes);
+ }
+
kfree(vport->qos.sched_node);
memset(&vport->qos, 0, sizeof(vport->qos));
}
@@ -172,11 +187,19 @@ mlx5_esw_qos_vport_get_parent(const struct mlx5_vport *vport)
static void esw_qos_sched_elem_warn(struct mlx5_esw_sched_node *node, int err, const char *op)
{
switch (node->type) {
+ case SCHED_NODE_TYPE_VPORT_TC:
+ esw_warn(node->esw->dev,
+ "E-Switch %s %s scheduling element failed (vport=%d,tc=%d,err=%d)\n",
+ op,
+ sched_node_type_str[node->type],
+ node->vport->vport, node->tc, err);
+ break;
case SCHED_NODE_TYPE_VPORT:
esw_warn(node->esw->dev,
"E-Switch %s %s scheduling element failed (vport=%d,err=%d)\n",
op, sched_node_type_str[node->type], node->vport->vport, err);
break;
+ case SCHED_NODE_TYPE_RATE_LIMITER:
case SCHED_NODE_TYPE_TC_ARBITER_TSAR:
case SCHED_NODE_TYPE_VPORTS_TSAR:
esw_warn(node->esw->dev,
@@ -271,6 +294,24 @@ static int esw_qos_sched_elem_config(struct mlx5_esw_sched_node *node, u32 max_r
return 0;
}
+static int esw_qos_create_rate_limit_element(struct mlx5_esw_sched_node *node,
+ struct netlink_ext_ack *extack)
+{
+ u32 sched_ctx[MLX5_ST_SZ_DW(scheduling_context)] = {};
+
+ if (!mlx5_qos_element_type_supported(
+ node->esw->dev,
+ SCHEDULING_CONTEXT_ELEMENT_TYPE_RATE_LIMIT,
+ SCHEDULING_HIERARCHY_E_SWITCH))
+ return -EOPNOTSUPP;
+
+ MLX5_SET(scheduling_context, sched_ctx, max_average_bw, node->max_rate);
+ MLX5_SET(scheduling_context, sched_ctx, element_type,
+ SCHEDULING_CONTEXT_ELEMENT_TYPE_RATE_LIMIT);
+
+ return esw_qos_node_create_sched_element(node, sched_ctx, extack);
+}
+
static u32 esw_qos_calculate_min_rate_divider(struct mlx5_eswitch *esw,
struct mlx5_esw_sched_node *parent)
{
@@ -388,28 +429,64 @@ esw_qos_create_node_sched_elem(struct mlx5_core_dev *dev, u32 parent_element_id,
tsar_ix);
}
-static int esw_qos_vport_create_sched_element(struct mlx5_esw_sched_node *vport_node,
- struct netlink_ext_ack *extack)
+static int
+esw_qos_vport_create_sched_element(struct mlx5_esw_sched_node *vport_node,
+ struct netlink_ext_ack *extack)
{
u32 sched_ctx[MLX5_ST_SZ_DW(scheduling_context)] = {};
struct mlx5_core_dev *dev = vport_node->esw->dev;
void *attr;
- if (!mlx5_qos_element_type_supported(dev,
- SCHEDULING_CONTEXT_ELEMENT_TYPE_VPORT,
- SCHEDULING_HIERARCHY_E_SWITCH))
+ if (!mlx5_qos_element_type_supported(
+ dev,
+ SCHEDULING_CONTEXT_ELEMENT_TYPE_VPORT,
+ SCHEDULING_HIERARCHY_E_SWITCH))
return -EOPNOTSUPP;
MLX5_SET(scheduling_context, sched_ctx, element_type,
SCHEDULING_CONTEXT_ELEMENT_TYPE_VPORT);
attr = MLX5_ADDR_OF(scheduling_context, sched_ctx, element_attributes);
MLX5_SET(vport_element, attr, vport_number, vport_node->vport->vport);
- MLX5_SET(scheduling_context, sched_ctx, parent_element_id, vport_node->parent->ix);
- MLX5_SET(scheduling_context, sched_ctx, max_average_bw, vport_node->max_rate);
+ MLX5_SET(scheduling_context, sched_ctx, parent_element_id,
+ vport_node->parent->ix);
+ MLX5_SET(scheduling_context, sched_ctx, max_average_bw,
+ vport_node->max_rate);
return esw_qos_node_create_sched_element(vport_node, sched_ctx, extack);
}
+static int
+esw_qos_vport_tc_create_sched_element(struct mlx5_esw_sched_node *vport_tc_node,
+ u32 rate_limit_elem_ix,
+ struct netlink_ext_ack *extack)
+{
+ u32 sched_ctx[MLX5_ST_SZ_DW(scheduling_context)] = {};
+ struct mlx5_core_dev *dev = vport_tc_node->esw->dev;
+ void *attr;
+
+ if (!mlx5_qos_element_type_supported(
+ dev,
+ SCHEDULING_CONTEXT_ELEMENT_TYPE_VPORT_TC,
+ SCHEDULING_HIERARCHY_E_SWITCH))
+ return -EOPNOTSUPP;
+
+ MLX5_SET(scheduling_context, sched_ctx, element_type,
+ SCHEDULING_CONTEXT_ELEMENT_TYPE_VPORT_TC);
+ attr = MLX5_ADDR_OF(scheduling_context, sched_ctx, element_attributes);
+ MLX5_SET(vport_tc_element, attr, vport_number,
+ vport_tc_node->vport->vport);
+ MLX5_SET(vport_tc_element, attr, traffic_class, vport_tc_node->tc);
+ MLX5_SET(scheduling_context, sched_ctx, max_bw_obj_id,
+ rate_limit_elem_ix);
+ MLX5_SET(scheduling_context, sched_ctx, parent_element_id,
+ vport_tc_node->parent->ix);
+ MLX5_SET(scheduling_context, sched_ctx, bw_share,
+ vport_tc_node->bw_share);
+
+ return esw_qos_node_create_sched_element(vport_tc_node, sched_ctx,
+ extack);
+}
+
static struct mlx5_esw_sched_node *
__esw_qos_alloc_node(struct mlx5_eswitch *esw, u32 tsar_ix, enum sched_node_type type,
struct mlx5_esw_sched_node *parent)
@@ -617,12 +694,202 @@ static int esw_qos_tc_arbiter_scheduling_setup(struct mlx5_esw_sched_node *node,
return -EOPNOTSUPP;
}
+static int
+esw_qos_create_vport_tc_sched_node(struct mlx5_vport *vport,
+ u32 rate_limit_elem_ix,
+ struct mlx5_esw_sched_node *vports_tc_node,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *vport_node = vport->qos.sched_node;
+ struct mlx5_esw_sched_node *vport_tc_node;
+ u8 tc = vports_tc_node->tc;
+ int err;
+
+ vport_tc_node = __esw_qos_alloc_node(vport_node->esw, 0,
+ SCHED_NODE_TYPE_VPORT_TC,
+ vports_tc_node);
+ if (!vport_tc_node)
+ return -ENOMEM;
+
+ vport_tc_node->min_rate = vport_node->min_rate;
+ vport_tc_node->tc = tc;
+ vport_tc_node->vport = vport;
+ err = esw_qos_vport_tc_create_sched_element(vport_tc_node,
+ rate_limit_elem_ix,
+ extack);
+ if (err)
+ goto err_out;
+
+ vport->qos.sched_nodes[tc] = vport_tc_node;
+
+ return 0;
+err_out:
+ __esw_qos_free_node(vport_tc_node);
+ return err;
+}
+
+static void
+esw_qos_destroy_vport_tc_sched_elements(struct mlx5_vport *vport,
+ struct netlink_ext_ack *extack)
+{
+ int i, num_tcs = esw_qos_num_tcs(vport->qos.sched_node->esw->dev);
+
+ for (i = 0; i < num_tcs; i++) {
+ if (vport->qos.sched_nodes[i]) {
+ __esw_qos_destroy_node(vport->qos.sched_nodes[i],
+ extack);
+ }
+ }
+
+ kfree(vport->qos.sched_nodes);
+ vport->qos.sched_nodes = NULL;
+}
+
+static int
+esw_qos_create_vport_tc_sched_elements(struct mlx5_vport *vport,
+ enum sched_node_type type,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *vport_node = vport->qos.sched_node;
+ struct mlx5_esw_sched_node *tc_arbiter_node, *vports_tc_node;
+ int err, num_tcs = esw_qos_num_tcs(vport_node->esw->dev);
+ u32 rate_limit_elem_ix;
+
+ vport->qos.sched_nodes = kcalloc(num_tcs,
+ sizeof(struct mlx5_esw_sched_node *),
+ GFP_KERNEL);
+ if (!vport->qos.sched_nodes) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Allocating the vport TC scheduling elements failed.");
+ return -ENOMEM;
+ }
+
+ rate_limit_elem_ix = type == SCHED_NODE_TYPE_RATE_LIMITER ?
+ vport_node->ix : 0;
+ tc_arbiter_node = type == SCHED_NODE_TYPE_RATE_LIMITER ?
+ vport_node->parent : vport_node;
+ list_for_each_entry(vports_tc_node, &tc_arbiter_node->children, entry) {
+ err = esw_qos_create_vport_tc_sched_node(vport,
+ rate_limit_elem_ix,
+ vports_tc_node,
+ extack);
+ if (err)
+ goto err_create_vport_tc;
+ }
+
+ return 0;
+
+err_create_vport_tc:
+ esw_qos_destroy_vport_tc_sched_elements(vport, NULL);
+
+ return err;
+}
+
+static int
+esw_qos_vport_tc_enable(struct mlx5_vport *vport, enum sched_node_type type,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *vport_node = vport->qos.sched_node;
+ int err, new_level, max_level;
+
+ if (type == SCHED_NODE_TYPE_TC_ARBITER_TSAR) {
+ /* Increase the parent's level by 2 to account for both the
+ * TC arbiter and the vports TC scheduling element.
+ */
+ new_level = vport_node->parent->level + 2;
+ max_level = 1 << MLX5_CAP_QOS(vport_node->esw->dev,
+ log_esw_max_sched_depth);
+ if (new_level > max_level) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "TC arbitration on leafs is not supported beyond max scheduling depth");
+ return -EOPNOTSUPP;
+ }
+ }
+
+ esw_assert_qos_lock_held(vport->dev->priv.eswitch);
+
+ if (type == SCHED_NODE_TYPE_RATE_LIMITER)
+ err = esw_qos_create_rate_limit_element(vport_node, extack);
+ else
+ err = esw_qos_tc_arbiter_scheduling_setup(vport_node, extack);
+ if (err)
+ return err;
+
+ /* Rate limiters impact multiple nodes not directly connected to them
+ * and are not direct members of the QoS hierarchy.
+ * Unlink it from the parent to reflect that.
+ */
+ if (type == SCHED_NODE_TYPE_RATE_LIMITER) {
+ list_del_init(&vport_node->entry);
+ vport_node->level = 0;
+ }
+
+ err = esw_qos_create_vport_tc_sched_elements(vport, type, extack);
+ if (err)
+ goto err_sched_nodes;
+
+ return 0;
+
+err_sched_nodes:
+ if (type == SCHED_NODE_TYPE_RATE_LIMITER) {
+ esw_qos_node_destroy_sched_element(vport_node, NULL);
+ list_add_tail(&vport_node->entry,
+ &vport_node->parent->children);
+ vport_node->level = vport_node->parent->level + 1;
+ } else {
+ esw_qos_tc_arbiter_scheduling_teardown(vport_node, NULL);
+ }
+ return err;
+}
+
+static void esw_qos_vport_tc_disable(struct mlx5_vport *vport,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *vport_node = vport->qos.sched_node;
+ enum sched_node_type curr_type = vport_node->type;
+
+ esw_qos_destroy_vport_tc_sched_elements(vport, extack);
+
+ if (curr_type == SCHED_NODE_TYPE_RATE_LIMITER)
+ esw_qos_node_destroy_sched_element(vport_node, extack);
+ else
+ esw_qos_tc_arbiter_scheduling_teardown(vport_node, extack);
+}
+
+static int esw_qos_set_vport_tcs_min_rate(struct mlx5_vport *vport,
+ u32 min_rate,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *vport_node = vport->qos.sched_node;
+ int err, i, num_tcs = esw_qos_num_tcs(vport_node->esw->dev);
+
+ for (i = 0; i < num_tcs; i++) {
+ err = esw_qos_set_node_min_rate(vport->qos.sched_nodes[i],
+ min_rate, extack);
+ if (err)
+ goto err_out;
+ }
+ vport_node->min_rate = min_rate;
+
+ return 0;
+err_out:
+ for (--i; i >= 0; i--) {
+ esw_qos_set_node_min_rate(vport->qos.sched_nodes[i],
+ vport_node->min_rate, extack);
+ }
+ return err;
+}
+
static void esw_qos_vport_disable(struct mlx5_vport *vport, struct netlink_ext_ack *extack)
{
struct mlx5_esw_sched_node *vport_node = vport->qos.sched_node;
struct mlx5_esw_sched_node *parent = vport_node->parent;
+ enum sched_node_type curr_type = vport_node->type;
- esw_qos_node_destroy_sched_element(vport_node, extack);
+ if (curr_type == SCHED_NODE_TYPE_VPORT)
+ esw_qos_node_destroy_sched_element(vport_node, extack);
+ else
+ esw_qos_vport_tc_disable(vport, extack);
vport_node->bw_share = 0;
list_del_init(&vport_node->entry);
@@ -631,7 +898,9 @@ static void esw_qos_vport_disable(struct mlx5_vport *vport, struct netlink_ext_a
trace_mlx5_esw_vport_qos_destroy(vport_node->esw->dev, vport);
}
-static int esw_qos_vport_enable(struct mlx5_vport *vport, struct mlx5_esw_sched_node *parent,
+static int esw_qos_vport_enable(struct mlx5_vport *vport,
+ enum sched_node_type type,
+ struct mlx5_esw_sched_node *parent,
struct netlink_ext_ack *extack)
{
int err;
@@ -639,10 +908,16 @@ static int esw_qos_vport_enable(struct mlx5_vport *vport, struct mlx5_esw_sched_
esw_assert_qos_lock_held(vport->dev->priv.eswitch);
esw_qos_node_set_parent(vport->qos.sched_node, parent);
- err = esw_qos_vport_create_sched_element(vport->qos.sched_node, extack);
+ if (type == SCHED_NODE_TYPE_VPORT) {
+ err = esw_qos_vport_create_sched_element(vport->qos.sched_node,
+ extack);
+ } else {
+ err = esw_qos_vport_tc_enable(vport, type, extack);
+ }
if (err)
return err;
+ vport->qos.sched_node->type = type;
esw_qos_normalize_min_rate(parent->esw, parent, extack);
trace_mlx5_esw_vport_qos_create(vport->dev, vport,
vport->qos.sched_node->max_rate,
@@ -673,9 +948,8 @@ static int mlx5_esw_qos_vport_enable(struct mlx5_vport *vport, enum sched_node_t
sched_node->min_rate = min_rate;
sched_node->vport = vport;
vport->qos.sched_node = sched_node;
- err = esw_qos_vport_enable(vport, parent, extack);
+ err = esw_qos_vport_enable(vport, type, parent, extack);
if (err) {
- __esw_qos_free_node(sched_node);
esw_qos_put(esw);
vport->qos.sched_node = NULL;
}
@@ -728,6 +1002,8 @@ static int mlx5_esw_qos_set_vport_min_rate(struct mlx5_vport *vport, u32 min_rat
if (!vport_node)
return mlx5_esw_qos_vport_enable(vport, SCHED_NODE_TYPE_VPORT, NULL, 0, min_rate,
extack);
+ else if (vport_node->type == SCHED_NODE_TYPE_RATE_LIMITER)
+ return esw_qos_set_vport_tcs_min_rate(vport, min_rate, extack);
else
return esw_qos_set_node_min_rate(vport_node, min_rate, extack);
}
@@ -760,12 +1036,60 @@ bool mlx5_esw_qos_get_vport_rate(struct mlx5_vport *vport, u32 *max_rate, u32 *m
return enabled;
}
+static int esw_qos_vport_tc_check_type(enum sched_node_type curr_type,
+ enum sched_node_type new_type,
+ struct netlink_ext_ack *extack)
+{
+ if (curr_type == SCHED_NODE_TYPE_TC_ARBITER_TSAR &&
+ new_type == SCHED_NODE_TYPE_RATE_LIMITER) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Cannot switch from vport-level TC arbitration to node-level TC arbitration");
+ return -EOPNOTSUPP;
+ }
+
+ if (curr_type == SCHED_NODE_TYPE_RATE_LIMITER &&
+ new_type == SCHED_NODE_TYPE_TC_ARBITER_TSAR) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Cannot switch from node-level TC arbitration to vport-level TC arbitration");
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static int esw_qos_vport_update(struct mlx5_vport *vport,
+ enum sched_node_type type,
+ struct mlx5_esw_sched_node *parent,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *curr_parent = vport->qos.sched_node->parent;
+ enum sched_node_type curr_type = vport->qos.sched_node->type;
+ int err;
+
+ esw_assert_qos_lock_held(vport->dev->priv.eswitch);
+ parent = parent ?: curr_parent;
+ if (curr_type == type && curr_parent == parent)
+ return 0;
+
+ err = esw_qos_vport_tc_check_type(curr_type, type, extack);
+ if (err)
+ return err;
+
+ esw_qos_vport_disable(vport, extack);
+
+ err = esw_qos_vport_enable(vport, type, parent, extack);
+ if (err)
+ esw_qos_vport_enable(vport, curr_type, curr_parent, NULL);
+
+ return err;
+}
+
static int esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw_sched_node *parent,
struct netlink_ext_ack *extack)
{
struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
struct mlx5_esw_sched_node *curr_parent;
- int err;
+ enum sched_node_type type;
esw_assert_qos_lock_held(esw);
curr_parent = vport->qos.sched_node->parent;
@@ -773,16 +1097,17 @@ static int esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw
if (curr_parent == parent)
return 0;
- esw_qos_vport_disable(vport, extack);
-
- err = esw_qos_vport_enable(vport, parent, extack);
- if (err) {
- if (esw_qos_vport_enable(vport, curr_parent, NULL))
- esw_warn(parent->esw->dev, "vport restore QoS failed (vport=%d)\n",
- vport->vport);
- }
+ /* Set vport QoS type based on parent node type if different from
+ * default QoS; otherwise, use the vport's current QoS type.
+ */
+ if (parent->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR)
+ type = SCHED_NODE_TYPE_RATE_LIMITER;
+ else if (curr_parent->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR)
+ type = SCHED_NODE_TYPE_VPORT;
+ else
+ type = vport->qos.sched_node->type;
- return err;
+ return esw_qos_vport_update(vport, type, parent, extack);
}
static void
@@ -1112,6 +1437,16 @@ static bool esw_qos_validate_unsupported_tc_bw(struct mlx5_eswitch *esw,
return true;
}
+static bool esw_qos_vport_validate_unsupported_tc_bw(struct mlx5_vport *vport,
+ u32 *tc_bw)
+{
+ struct mlx5_eswitch *esw = vport->qos.sched_node ?
+ vport->qos.sched_node->parent->esw :
+ vport->dev->priv.eswitch;
+
+ return esw_qos_validate_unsupported_tc_bw(esw, tc_bw);
+}
+
static bool esw_qos_tc_bw_disabled(u32 *tc_bw)
{
int i;
@@ -1187,9 +1522,50 @@ int mlx5_esw_devlink_rate_leaf_tc_bw_set(struct devlink_rate *rate_leaf,
u32 *tc_bw,
struct netlink_ext_ack *extack)
{
- NL_SET_ERR_MSG_MOD(extack,
- "TC bandwidth shares are not supported on leafs");
- return -EOPNOTSUPP;
+ 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))
+ 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;
+ }
+
+ vport_node = vport->qos.sched_node;
+ if (disable && !vport_node)
+ goto unlock;
+
+ if (disable) {
+ if (vport_node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR)
+ err = esw_qos_vport_update(vport, SCHED_NODE_TYPE_VPORT,
+ NULL, extack);
+ goto unlock;
+ }
+
+ if (!vport_node) {
+ err = mlx5_esw_qos_vport_enable(vport,
+ SCHED_NODE_TYPE_TC_ARBITER_TSAR,
+ NULL, 0, 0, extack);
+ vport_node = vport->qos.sched_node;
+ } else {
+ err = esw_qos_vport_update(vport,
+ SCHED_NODE_TYPE_TC_ARBITER_TSAR,
+ NULL, extack);
+ }
+unlock:
+ esw_qos_unlock(esw);
+ return err;
}
int mlx5_esw_devlink_rate_node_tc_bw_set(struct devlink_rate *rate_node,
@@ -1311,10 +1687,16 @@ int mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw_s
}
esw_qos_lock(esw);
- if (!vport->qos.sched_node && parent)
- err = mlx5_esw_qos_vport_enable(vport, SCHED_NODE_TYPE_VPORT, parent, 0, 0, extack);
- else if (vport->qos.sched_node)
+ if (!vport->qos.sched_node && parent) {
+ enum sched_node_type type;
+
+ type = parent->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR ?
+ SCHED_NODE_TYPE_RATE_LIMITER : SCHED_NODE_TYPE_VPORT;
+ err = mlx5_esw_qos_vport_enable(vport, type, parent, 0, 0,
+ extack);
+ } else if (vport->qos.sched_node) {
err = esw_qos_vport_update_parent(vport, parent, extack);
+ }
esw_qos_unlock(esw);
return err;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index 8573d36785f4..d59fdcb29cb8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -212,10 +212,20 @@ struct mlx5_vport {
struct mlx5_vport_info info;
- /* Protected with the E-Switch qos domain lock. */
+ /* 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:
+ * 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
+ * (sched_node is a rate limit node).
+ * When TC is enabled in either mode, the vport owns vport TC scheduling
+ * nodes.
+ */
struct {
- /* Vport scheduling element node. */
+ /* Vport scheduling node. */
struct mlx5_esw_sched_node *sched_node;
+ /* Array of vport traffic class scheduling nodes. */
+ struct mlx5_esw_sched_node **sched_nodes;
} qos;
u16 vport;
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next V10 6/6] net/mlx5: Manage TC arbiter nodes and implement full support for tc-bw
2025-05-20 18:38 [PATCH net-next V10 0/6] Support rate management on traffic classes in devlink and mlx5 Tariq Toukan
` (4 preceding siblings ...)
2025-05-20 18:38 ` [PATCH net-next V10 5/6] net/mlx5: Add traffic class scheduling support for vport QoS Tariq Toukan
@ 2025-05-20 18:38 ` Tariq Toukan
5 siblings, 0 replies; 16+ messages in thread
From: Tariq Toukan @ 2025-05-20 18:38 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Jiri Pirko, Gal Pressman, Leon Romanovsky, Donald Hunter,
Jiri Pirko, Jonathan Corbet, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Carolina Jubran, Cosmin Ratiu
From: Carolina Jubran <cjubran@nvidia.com>
Introduce support for managing Traffic Class (TC) arbiter nodes and
associated vports TC nodes within the E-Switch QoS hierarchy. This
patch adds support for the new scheduling node type,
`SCHED_NODE_TYPE_VPORTS_TC_TSAR`, and implements full support for
setting tc-bw on both vports and nodes.
Key changes include:
- Introduced the new scheduling node type,
`SCHED_NODE_TYPE_VPORTS_TC_TSAR`, for managing vports within the TC
arbiter node.
- New helper functions for creating and destroying vports TC nodes
under the TC arbiter.
- Updated the minimum rate normalization function to skip nodes of type
`SCHED_NODE_TYPE_VPORTS_TC_TSAR`. Vports TC TSARs have bandwidth
shares configured on them but not minimum rates, so their `min_rate`
cannot be normalized.
- Implementation of `esw_qos_tc_arbiter_scheduling_setup()` and
`esw_qos_tc_arbiter_scheduling_teardown()` for initializing and
cleaning up TC arbiter scheduling elements. These functions now fully
support tc-bw configuration on TC arbiter nodes.
- Added `esw_qos_tc_arbiter_get_bw_shares()` and
`esw_qos_set_tc_arbiter_bw_shares()` to handle the settings of
bandwidth shares for vports traffic class TSARs.
- Refactored `mlx5_esw_devlink_rate_node_tc_bw_set()` and
`mlx5_esw_devlink_rate_leaf_tc_bw_set()` to fully support configuring
tc-bw on devlink rate nodes and vports, respectively.
- Refactored `mlx5_esw_qos_node_update_parent()` to ensure that tc-bw
configuration remains compatible with setting a parent on a rate
node, preserving level hierarchy functionality.
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/esw/qos.c | 264 +++++++++++++++++-
1 file changed, 257 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
index dec3bed682b7..14aeb1999044 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
@@ -67,6 +67,7 @@ enum sched_node_type {
SCHED_NODE_TYPE_TC_ARBITER_TSAR,
SCHED_NODE_TYPE_RATE_LIMITER,
SCHED_NODE_TYPE_VPORT_TC,
+ SCHED_NODE_TYPE_VPORTS_TC_TSAR,
};
static const char * const sched_node_type_str[] = {
@@ -75,6 +76,7 @@ static const char * const sched_node_type_str[] = {
[SCHED_NODE_TYPE_TC_ARBITER_TSAR] = "TC Arbiter TSAR",
[SCHED_NODE_TYPE_RATE_LIMITER] = "Rate Limiter",
[SCHED_NODE_TYPE_VPORT_TC] = "vport TC",
+ [SCHED_NODE_TYPE_VPORTS_TC_TSAR] = "vports TC TSAR",
};
struct mlx5_esw_sched_node {
@@ -187,6 +189,11 @@ mlx5_esw_qos_vport_get_parent(const struct mlx5_vport *vport)
static void esw_qos_sched_elem_warn(struct mlx5_esw_sched_node *node, int err, const char *op)
{
switch (node->type) {
+ case SCHED_NODE_TYPE_VPORTS_TC_TSAR:
+ esw_warn(node->esw->dev,
+ "E-Switch %s %s scheduling element failed (tc=%d,err=%d)\n",
+ op, sched_node_type_str[node->type], node->tc, err);
+ break;
case SCHED_NODE_TYPE_VPORT_TC:
esw_warn(node->esw->dev,
"E-Switch %s %s scheduling element failed (vport=%d,tc=%d,err=%d)\n",
@@ -376,7 +383,13 @@ static void esw_qos_normalize_min_rate(struct mlx5_eswitch *esw,
if (node->esw != esw || node->ix == esw->qos.root_tsar_ix)
continue;
- esw_qos_update_sched_node_bw_share(node, divider, extack);
+ /* Vports TC TSARs don't have a minimum rate configured,
+ * so there's no need to update the bw_share on them.
+ */
+ if (node->type != SCHED_NODE_TYPE_VPORTS_TC_TSAR) {
+ esw_qos_update_sched_node_bw_share(node, divider,
+ extack);
+ }
if (list_empty(&node->children))
continue;
@@ -527,6 +540,144 @@ static void esw_qos_destroy_node(struct mlx5_esw_sched_node *node, struct netlin
__esw_qos_free_node(node);
}
+static int esw_qos_create_vports_tc_node(struct mlx5_esw_sched_node *parent,
+ u8 tc, struct netlink_ext_ack *extack)
+{
+ u32 tsar_ctx[MLX5_ST_SZ_DW(scheduling_context)] = {};
+ struct mlx5_core_dev *dev = parent->esw->dev;
+ struct mlx5_esw_sched_node *vports_tc_node;
+ void *attr;
+ int err;
+
+ if (!mlx5_qos_element_type_supported(
+ dev,
+ SCHEDULING_CONTEXT_ELEMENT_TYPE_TSAR,
+ SCHEDULING_HIERARCHY_E_SWITCH) ||
+ !mlx5_qos_tsar_type_supported(dev,
+ TSAR_ELEMENT_TSAR_TYPE_DWRR,
+ SCHEDULING_HIERARCHY_E_SWITCH))
+ return -EOPNOTSUPP;
+
+ vports_tc_node = __esw_qos_alloc_node(parent->esw, 0,
+ SCHED_NODE_TYPE_VPORTS_TC_TSAR,
+ parent);
+ if (!vports_tc_node) {
+ NL_SET_ERR_MSG_MOD(extack, "E-Switch alloc node failed");
+ esw_warn(dev, "Failed to alloc vports TC node (tc=%d)\n", tc);
+ return -ENOMEM;
+ }
+
+ attr = MLX5_ADDR_OF(scheduling_context, tsar_ctx, element_attributes);
+ MLX5_SET(tsar_element, attr, tsar_type, TSAR_ELEMENT_TSAR_TYPE_DWRR);
+ MLX5_SET(tsar_element, attr, traffic_class, tc);
+ MLX5_SET(scheduling_context, tsar_ctx, parent_element_id, parent->ix);
+ MLX5_SET(scheduling_context, tsar_ctx, element_type,
+ SCHEDULING_CONTEXT_ELEMENT_TYPE_TSAR);
+
+ err = esw_qos_node_create_sched_element(vports_tc_node, tsar_ctx,
+ extack);
+ if (err)
+ goto err_create_sched_element;
+
+ vports_tc_node->tc = tc;
+
+ return 0;
+
+err_create_sched_element:
+ __esw_qos_free_node(vports_tc_node);
+ return err;
+}
+
+static void
+esw_qos_tc_arbiter_get_bw_shares(struct mlx5_esw_sched_node *tc_arbiter_node,
+ u32 *tc_bw)
+{
+ struct mlx5_esw_sched_node *vports_tc_node;
+
+ list_for_each_entry(vports_tc_node, &tc_arbiter_node->children, entry)
+ tc_bw[vports_tc_node->tc] = vports_tc_node->bw_share;
+}
+
+static void
+esw_qos_set_tc_arbiter_bw_shares(struct mlx5_esw_sched_node *tc_arbiter_node,
+ u32 *tc_bw, struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *vports_tc_node;
+
+ list_for_each_entry(vports_tc_node, &tc_arbiter_node->children, entry) {
+ u32 bw_share;
+ u8 tc;
+
+ tc = vports_tc_node->tc;
+ bw_share = tc_bw[tc] ?: MLX5_MIN_BW_SHARE;
+ esw_qos_sched_elem_config(vports_tc_node, 0, bw_share, extack);
+ }
+}
+
+static void
+esw_qos_destroy_vports_tc_nodes(struct mlx5_esw_sched_node *tc_arbiter_node,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *vports_tc_node, *tmp;
+
+ list_for_each_entry_safe(vports_tc_node, tmp,
+ &tc_arbiter_node->children, entry)
+ esw_qos_destroy_node(vports_tc_node, extack);
+}
+
+static int
+esw_qos_create_vports_tc_nodes(struct mlx5_esw_sched_node *tc_arbiter_node,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_eswitch *esw = tc_arbiter_node->esw;
+ int err, i, num_tcs = esw_qos_num_tcs(esw->dev);
+
+ for (i = 0; i < num_tcs; i++) {
+ err = esw_qos_create_vports_tc_node(tc_arbiter_node, i, extack);
+ if (err)
+ goto err_tc_node_create;
+ }
+
+ return 0;
+
+err_tc_node_create:
+ esw_qos_destroy_vports_tc_nodes(tc_arbiter_node, NULL);
+ return err;
+}
+
+static int esw_qos_create_tc_arbiter_sched_elem(
+ struct mlx5_esw_sched_node *tc_arbiter_node,
+ struct netlink_ext_ack *extack)
+{
+ u32 tsar_ctx[MLX5_ST_SZ_DW(scheduling_context)] = {};
+ u32 tsar_parent_ix;
+ void *attr;
+
+ if (!mlx5_qos_tsar_type_supported(tc_arbiter_node->esw->dev,
+ TSAR_ELEMENT_TSAR_TYPE_TC_ARB,
+ SCHEDULING_HIERARCHY_E_SWITCH)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "E-Switch TC Arbiter scheduling element is not supported");
+ return -EOPNOTSUPP;
+ }
+
+ attr = MLX5_ADDR_OF(scheduling_context, tsar_ctx, element_attributes);
+ MLX5_SET(tsar_element, attr, tsar_type, TSAR_ELEMENT_TSAR_TYPE_TC_ARB);
+ tsar_parent_ix = tc_arbiter_node->parent ? tc_arbiter_node->parent->ix :
+ tc_arbiter_node->esw->qos.root_tsar_ix;
+ MLX5_SET(scheduling_context, tsar_ctx, parent_element_id,
+ tsar_parent_ix);
+ MLX5_SET(scheduling_context, tsar_ctx, element_type,
+ SCHEDULING_CONTEXT_ELEMENT_TYPE_TSAR);
+ MLX5_SET(scheduling_context, tsar_ctx, max_average_bw,
+ tc_arbiter_node->max_rate);
+ MLX5_SET(scheduling_context, tsar_ctx, bw_share,
+ tc_arbiter_node->bw_share);
+
+ return esw_qos_node_create_sched_element(tc_arbiter_node, tsar_ctx,
+ extack);
+}
+
static struct mlx5_esw_sched_node *
__esw_qos_create_vports_sched_node(struct mlx5_eswitch *esw, struct mlx5_esw_sched_node *parent,
struct netlink_ext_ack *extack)
@@ -591,6 +742,9 @@ static void __esw_qos_destroy_node(struct mlx5_esw_sched_node *node, struct netl
{
struct mlx5_eswitch *esw = node->esw;
+ if (node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR)
+ esw_qos_destroy_vports_tc_nodes(node, extack);
+
trace_mlx5_esw_node_qos_destroy(esw->dev, node, node->ix);
esw_qos_destroy_node(node, extack);
esw_qos_normalize_min_rate(esw, NULL, extack);
@@ -685,13 +839,38 @@ static void esw_qos_put(struct mlx5_eswitch *esw)
static void
esw_qos_tc_arbiter_scheduling_teardown(struct mlx5_esw_sched_node *node,
struct netlink_ext_ack *extack)
-{}
+{
+ /* Clean up all Vports TC nodes within the TC arbiter node. */
+ esw_qos_destroy_vports_tc_nodes(node, extack);
+ /* Destroy the scheduling element for the TC arbiter node itself. */
+ esw_qos_node_destroy_sched_element(node, extack);
+}
static int esw_qos_tc_arbiter_scheduling_setup(struct mlx5_esw_sched_node *node,
struct netlink_ext_ack *extack)
{
- NL_SET_ERR_MSG_MOD(extack, "TC arbiter elements are not supported.");
- return -EOPNOTSUPP;
+ u32 curr_ix = node->ix;
+ int err;
+
+ err = esw_qos_create_tc_arbiter_sched_elem(node, extack);
+ if (err)
+ return err;
+ /* Initialize the vports TC nodes within created TC arbiter TSAR. */
+ err = esw_qos_create_vports_tc_nodes(node, extack);
+ if (err)
+ goto err_vports_tc_nodes;
+
+ node->type = SCHED_NODE_TYPE_TC_ARBITER_TSAR;
+
+ return 0;
+
+err_vports_tc_nodes:
+ /* If initialization fails, clean up the scheduling element
+ * for the TC arbiter node.
+ */
+ esw_qos_node_destroy_sched_element(node, NULL);
+ node->ix = curr_ix;
+ return err;
}
static int
@@ -1064,6 +1243,7 @@ static int esw_qos_vport_update(struct mlx5_vport *vport,
{
struct mlx5_esw_sched_node *curr_parent = vport->qos.sched_node->parent;
enum sched_node_type curr_type = vport->qos.sched_node->type;
+ u32 curr_tc_bw[DEVLINK_RATE_TCS_MAX] = {0};
int err;
esw_assert_qos_lock_held(vport->dev->priv.eswitch);
@@ -1075,11 +1255,23 @@ static int esw_qos_vport_update(struct mlx5_vport *vport,
if (err)
return err;
+ if (curr_type == SCHED_NODE_TYPE_TC_ARBITER_TSAR && curr_type == type) {
+ esw_qos_tc_arbiter_get_bw_shares(vport->qos.sched_node,
+ curr_tc_bw);
+ }
+
esw_qos_vport_disable(vport, extack);
err = esw_qos_vport_enable(vport, type, parent, extack);
- if (err)
+ if (err) {
esw_qos_vport_enable(vport, curr_type, curr_parent, NULL);
+ extack = NULL;
+ }
+
+ if (curr_type == SCHED_NODE_TYPE_TC_ARBITER_TSAR && curr_type == type) {
+ esw_qos_set_tc_arbiter_bw_shares(vport->qos.sched_node,
+ curr_tc_bw, extack);
+ }
return err;
}
@@ -1563,6 +1755,8 @@ int mlx5_esw_devlink_rate_leaf_tc_bw_set(struct devlink_rate *rate_leaf,
SCHED_NODE_TYPE_TC_ARBITER_TSAR,
NULL, extack);
}
+ if (!err)
+ esw_qos_set_tc_arbiter_bw_shares(vport_node, tc_bw, extack);
unlock:
esw_qos_unlock(esw);
return err;
@@ -1592,6 +1786,8 @@ int mlx5_esw_devlink_rate_node_tc_bw_set(struct devlink_rate *rate_node,
}
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;
@@ -1716,6 +1912,15 @@ int mlx5_esw_devlink_rate_leaf_parent_set(struct devlink_rate *devlink_rate,
return mlx5_esw_qos_vport_update_parent(vport, node, extack);
}
+static bool esw_qos_is_node_empty(struct mlx5_esw_sched_node *node)
+{
+ return list_empty(&node->children) ||
+ (node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR &&
+ esw_qos_is_node_empty(
+ list_first_entry(&node->children,
+ struct mlx5_esw_sched_node, entry)));
+}
+
static int
mlx5_esw_qos_node_validate_set_parent(struct mlx5_esw_sched_node *node,
struct mlx5_esw_sched_node *parent,
@@ -1729,13 +1934,26 @@ mlx5_esw_qos_node_validate_set_parent(struct mlx5_esw_sched_node *node,
return -EOPNOTSUPP;
}
- if (!list_empty(&node->children)) {
+ if (!esw_qos_is_node_empty(node)) {
NL_SET_ERR_MSG_MOD(extack,
"Cannot reassign a node that contains rate objects");
return -EOPNOTSUPP;
}
+ if (parent && parent->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Cannot attach a node to a parent with TC bandwidth configured");
+ return -EOPNOTSUPP;
+ }
+
new_level = parent ? parent->level + 1 : 2;
+ if (node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR) {
+ /* Increase by one to account for the vports TC scheduling
+ * element.
+ */
+ new_level += 1;
+ }
+
max_level = 1 << MLX5_CAP_QOS(node->esw->dev, log_esw_max_sched_depth);
if (new_level > max_level) {
NL_SET_ERR_MSG_MOD(extack,
@@ -1746,6 +1964,32 @@ mlx5_esw_qos_node_validate_set_parent(struct mlx5_esw_sched_node *node,
return 0;
}
+static int
+esw_qos_tc_arbiter_node_update_parent(struct mlx5_esw_sched_node *node,
+ struct mlx5_esw_sched_node *parent,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_esw_sched_node *curr_parent = node->parent;
+ u32 curr_tc_bw[DEVLINK_RATE_TCS_MAX] = {0};
+ struct mlx5_eswitch *esw = node->esw;
+ int err;
+
+ esw_qos_tc_arbiter_get_bw_shares(node, curr_tc_bw);
+ esw_qos_tc_arbiter_scheduling_teardown(node, extack);
+ esw_qos_node_set_parent(node, parent);
+ err = esw_qos_tc_arbiter_scheduling_setup(node, extack);
+ if (err) {
+ esw_qos_node_set_parent(node, curr_parent);
+ if (esw_qos_tc_arbiter_scheduling_setup(node, extack)) {
+ esw_warn(esw->dev, "Node restore QoS failed\n");
+ return err;
+ }
+ }
+ esw_qos_set_tc_arbiter_bw_shares(node, curr_tc_bw, extack);
+
+ return err;
+}
+
static int esw_qos_vports_node_update_parent(struct mlx5_esw_sched_node *node,
struct mlx5_esw_sched_node *parent,
struct netlink_ext_ack *extack)
@@ -1791,7 +2035,13 @@ static int mlx5_esw_qos_node_update_parent(struct mlx5_esw_sched_node *node,
esw_qos_lock(esw);
curr_parent = node->parent;
- err = esw_qos_vports_node_update_parent(node, parent, extack);
+ if (node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR) {
+ err = esw_qos_tc_arbiter_node_update_parent(node, parent,
+ extack);
+ } else {
+ err = esw_qos_vports_node_update_parent(node, parent, extack);
+ }
+
if (err)
goto out;
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net-next V10 1/6] devlink: Extend devlink rate API with traffic classes bandwidth management
2025-05-20 18:38 ` [PATCH net-next V10 1/6] devlink: Extend devlink rate API with traffic classes bandwidth management Tariq Toukan
@ 2025-05-20 22:53 ` Jakub Kicinski
2025-05-25 14:57 ` Carolina Jubran
0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2025-05-20 22:53 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Paolo Abeni, Eric Dumazet, Andrew Lunn,
Jiri Pirko, Gal Pressman, Leon Romanovsky, Donald Hunter,
Jiri Pirko, Jonathan Corbet, Saeed Mahameed, Leon Romanovsky,
Shuah Khan, netdev, linux-kernel, linux-doc, linux-rdma,
linux-kselftest, Moshe Shemesh, Mark Bloch, Carolina Jubran,
Cosmin Ratiu
A few quick comments here as the test is failing
On Tue, 20 May 2025 21:38:02 +0300 Tariq Toukan wrote:
> + -
> + name: rate-tc-bws
> + type: nest
> + multi-attr: true
> + nested-attributes: dl-rate-tc-bws
> + -
> + name: rate-tc-index
> + type: u8
> + checks:
> + min: 0
> + max: rate-tc-index-max
no need for min: 0 on an unsigned type ?
> + -
> + name: rate-tc-bw
> + type: u32
> + doc: |
> + Specifies the bandwidth allocation for the Traffic Class as a
> + percentage.
> + checks:
> + min: 0
> + max: 100
Why in percentage? I don't think any existing param in devlink rate
or net shapers is in percentage right? Not according to what i can
grok about the uAPI.
> +static int devlink_nl_rate_tc_bw_parse(struct nlattr *parent_nest, u32 *tc_bw,
> + unsigned long *bitmap, struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
> + u8 tc_index;
> +
> + nla_parse_nested(tb, DEVLINK_ATTR_MAX, parent_nest, devlink_dl_rate_tc_bws_nl_policy,
Let's error check this, I get that we already validated via the policy
but what if we do memory allocations during parsing one day, or some
other failure-prone operation.. better check the return value.
nit: over 80 chars for no good reason, the line overflows anyway.
Please use checkpatch --max-line-width=80 for core code,
at the very least.
> + extack);
> + if (!tb[DEVLINK_ATTR_RATE_TC_INDEX]) {
> + NL_SET_ERR_ATTR_MISS(extack, parent_nest, DEVLINK_ATTR_RATE_TC_INDEX);
> + return -EINVAL;
> + }
> +
> + tc_index = nla_get_u8(tb[DEVLINK_ATTR_RATE_TC_INDEX]);
> +
> + if (!tb[DEVLINK_ATTR_RATE_TC_BW]) {
> + NL_SET_ERR_ATTR_MISS(extack, parent_nest, DEVLINK_ATTR_RATE_TC_BW);
> + return -EINVAL;
> + }
> +
> + if (test_and_set_bit(tc_index, bitmap)) {
> + NL_SET_ERR_MSG_FMT(extack, "Duplicate traffic class index specified (%u)",
> + tc_index);
> + return -EINVAL;
> + }
> +
> + tc_bw[tc_index] = nla_get_u32(tb[DEVLINK_ATTR_RATE_TC_BW]);
> +
> + return 0;
> +}
> +
> +static int devlink_nl_rate_tc_bw_set(struct devlink_rate *devlink_rate,
> + struct genl_info *info)
> +{
> + DECLARE_BITMAP(bitmap, DEVLINK_RATE_TCS_MAX) = {};
> + struct devlink *devlink = devlink_rate->devlink;
> + const struct devlink_ops *ops = devlink->ops;
> + int rem, err = -EOPNOTSUPP, i, total = 0;
> + u32 tc_bw[DEVLINK_RATE_TCS_MAX] = {};
> + struct nlattr *attr;
> +
> + nla_for_each_attr(attr, genlmsg_data(info->genlhdr),
> + genlmsg_len(info->genlhdr), rem) {
nla_for_each_attr_type() ?
or better still add a _type() version of nlmsg_for_each_attr() ?
> + if (nla_type(attr) == DEVLINK_ATTR_RATE_TC_BWS) {
> + err = devlink_nl_rate_tc_bw_parse(attr, tc_bw, bitmap, info->extack);
> + if (err)
> + return err;
> + }
> + }
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test
2025-05-20 18:38 ` [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test Tariq Toukan
@ 2025-05-20 22:59 ` Jakub Kicinski
2025-05-21 7:05 ` Tariq Toukan
2025-05-21 21:05 ` Carolina Jubran
0 siblings, 2 replies; 16+ messages in thread
From: Jakub Kicinski @ 2025-05-20 22:59 UTC (permalink / raw)
To: Carolina Jubran
Cc: Tariq Toukan, David S. Miller, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Jiri Pirko, Gal Pressman, Leon Romanovsky,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Cosmin Ratiu
On Tue, 20 May 2025 21:38:03 +0300 Tariq Toukan wrote:
> Test verifies that netdevsim correctly implements devlink ops callbacks
> that set tc-bw on leaf or node rate object.
Please add a test that can actually validate a NIC HW.
The test probably needs to be in Python to use a remote endpoint,
and should live under tools/testing/../drivers/net/hw
We had a long conversation about what we expect from the API
vs how your HW works. One of the test cases should confirm
the expected behavior, IOW fail on mlx5. Which is fine,
unlikely that any NIC will have 100% compliance. But at
least we will be documenting the expectations.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test
2025-05-20 22:59 ` Jakub Kicinski
@ 2025-05-21 7:05 ` Tariq Toukan
2025-05-21 14:10 ` Jakub Kicinski
2025-05-21 21:05 ` Carolina Jubran
1 sibling, 1 reply; 16+ messages in thread
From: Tariq Toukan @ 2025-05-21 7:05 UTC (permalink / raw)
To: Jakub Kicinski, Carolina Jubran
Cc: Tariq Toukan, David S. Miller, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Jiri Pirko, Gal Pressman, Leon Romanovsky,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Cosmin Ratiu
On 21/05/2025 1:59, Jakub Kicinski wrote:
> On Tue, 20 May 2025 21:38:03 +0300 Tariq Toukan wrote:
>> Test verifies that netdevsim correctly implements devlink ops callbacks
>> that set tc-bw on leaf or node rate object.
>
> Please add a test that can actually validate a NIC HW.
> The test probably needs to be in Python to use a remote endpoint,
> and should live under tools/testing/../drivers/net/hw
>
> We had a long conversation about what we expect from the API
> vs how your HW works. One of the test cases should confirm
> the expected behavior, IOW fail on mlx5. Which is fine,
> unlikely that any NIC will have 100% compliance. But at
> least we will be documenting the expectations.
>
No problem with that, we'll add.
We could've saved this extra cycle if my questions [1] exactly about
this topic weren't ignored.
Area is vague and not well defined. We can continue with the iterative
guess and fix cycles, or alternatively get it clearly and formally defined.
[1]
https://lore.kernel.org/all/98386cab-11c0-4f74-9925-8230af2e65c8@gmail.com/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test
2025-05-21 7:05 ` Tariq Toukan
@ 2025-05-21 14:10 ` Jakub Kicinski
2025-05-25 20:31 ` Gal Pressman
0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2025-05-21 14:10 UTC (permalink / raw)
To: Tariq Toukan
Cc: Carolina Jubran, Tariq Toukan, David S. Miller, Paolo Abeni,
Eric Dumazet, Andrew Lunn, Jiri Pirko, Gal Pressman,
Leon Romanovsky, Donald Hunter, Jiri Pirko, Jonathan Corbet,
Saeed Mahameed, Leon Romanovsky, Shuah Khan, netdev, linux-kernel,
linux-doc, linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Cosmin Ratiu
On Wed, 21 May 2025 10:05:13 +0300 Tariq Toukan wrote:
> On 21/05/2025 1:59, Jakub Kicinski wrote:
> > On Tue, 20 May 2025 21:38:03 +0300 Tariq Toukan wrote:
> >> Test verifies that netdevsim correctly implements devlink ops callbacks
> >> that set tc-bw on leaf or node rate object.
> >
> > Please add a test that can actually validate a NIC HW.
> > The test probably needs to be in Python to use a remote endpoint,
> > and should live under tools/testing/../drivers/net/hw
> >
> > We had a long conversation about what we expect from the API
> > vs how your HW works. One of the test cases should confirm
> > the expected behavior, IOW fail on mlx5. Which is fine,
> > unlikely that any NIC will have 100% compliance. But at
> > least we will be documenting the expectations.
>
> No problem with that, we'll add.
>
> We could've saved this extra cycle if my questions [1] exactly about
> this topic weren't ignored.
> Area is vague and not well defined. We can continue with the iterative
> guess and fix cycles, or alternatively get it clearly and formally defined.
I started a couple of times on answering but my hands go a little limb
when I have to explain things so obvious like "testing is a crucial part
of software development" :S I mean.. nvidia certainly tests their code,
so I'm not sure where the disconnect is. I had a short conversation with
Gal at some conference where he, AFAIU, was doubting that device testing
can be part of an open source project.
It certainly is not advantageous to companies to have to share their
test code. So when you ask me for details on the rules what I hear is
"how can we make sure we do as little as possible".
Broadly, any new uAPI should come with tests which exercise the
functionality. We started a decade or so ago with netdevsim tests
which just validate the API layer itself. That did not provide
sufficient validation of the real implementations, crucially it did
not check whether shallow APIs (devlink) actually behave the same
when implemented by multiple vendors. So two years ago we built
the Python harness to be able to write tests for NIC functionality.
That is the level of testing we expect now. Obviously there will always
be exceptions. For instance I was pushing for common tests for the time
sync code (DPLL etc.) but I was convinced by the experts that it's hard
and that they generally test with $x0,000 measurement equipment.
So fair, I guess that's too hard. But for BW shaping tests?
IIRC mlxsw has qdisc offload tests for BW shaping upstream.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test
2025-05-20 22:59 ` Jakub Kicinski
2025-05-21 7:05 ` Tariq Toukan
@ 2025-05-21 21:05 ` Carolina Jubran
2025-05-21 22:24 ` Jakub Kicinski
1 sibling, 1 reply; 16+ messages in thread
From: Carolina Jubran @ 2025-05-21 21:05 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Tariq Toukan, David S. Miller, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Jiri Pirko, Gal Pressman, Leon Romanovsky,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Cosmin Ratiu
On 21/05/2025 1:59, Jakub Kicinski wrote:
> On Tue, 20 May 2025 21:38:03 +0300 Tariq Toukan wrote:
>> Test verifies that netdevsim correctly implements devlink ops callbacks
>> that set tc-bw on leaf or node rate object.
>
> Please add a test that can actually validate a NIC HW.
> The test probably needs to be in Python to use a remote endpoint,
> and should live under tools/testing/../drivers/net/hw
>
> We had a long conversation about what we expect from the API
> vs how your HW works. One of the test cases should confirm
> the expected behavior, IOW fail on mlx5. Which is fine,
> unlikely that any NIC will have 100% compliance. But at
> least we will be documenting the expectations.
Working on this. Let me know if the netdevsim selftest should stay
alongside the hardware tests, or be removed.
Thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test
2025-05-21 21:05 ` Carolina Jubran
@ 2025-05-21 22:24 ` Jakub Kicinski
0 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2025-05-21 22:24 UTC (permalink / raw)
To: Carolina Jubran
Cc: Tariq Toukan, David S. Miller, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Jiri Pirko, Gal Pressman, Leon Romanovsky,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Cosmin Ratiu
On Thu, 22 May 2025 00:05:32 +0300 Carolina Jubran wrote:
> On 21/05/2025 1:59, Jakub Kicinski wrote:
> > On Tue, 20 May 2025 21:38:03 +0300 Tariq Toukan wrote:
> >> Test verifies that netdevsim correctly implements devlink ops callbacks
> >> that set tc-bw on leaf or node rate object.
> >
> > Please add a test that can actually validate a NIC HW.
> > The test probably needs to be in Python to use a remote endpoint,
> > and should live under tools/testing/../drivers/net/hw
> >
> > We had a long conversation about what we expect from the API
> > vs how your HW works. One of the test cases should confirm
> > the expected behavior, IOW fail on mlx5. Which is fine,
> > unlikely that any NIC will have 100% compliance. But at
> > least we will be documenting the expectations.
>
> Working on this. Let me know if the netdevsim selftest should stay
> alongside the hardware tests, or be removed.
I think it's nice to have. But please share a link to the patches which
add the support in the CLI next time. Can be a lore link if posted, or a
GH repo.
I need to pull the pending patches into the CI so that the test can run.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next V10 1/6] devlink: Extend devlink rate API with traffic classes bandwidth management
2025-05-20 22:53 ` Jakub Kicinski
@ 2025-05-25 14:57 ` Carolina Jubran
2025-05-27 23:28 ` Jakub Kicinski
0 siblings, 1 reply; 16+ messages in thread
From: Carolina Jubran @ 2025-05-25 14:57 UTC (permalink / raw)
To: Jakub Kicinski, Tariq Toukan
Cc: David S. Miller, Paolo Abeni, Eric Dumazet, Andrew Lunn,
Jiri Pirko, Gal Pressman, Leon Romanovsky, Donald Hunter,
Jiri Pirko, Jonathan Corbet, Saeed Mahameed, Leon Romanovsky,
Shuah Khan, netdev, linux-kernel, linux-doc, linux-rdma,
linux-kselftest, Moshe Shemesh, Mark Bloch, Cosmin Ratiu
On 21/05/2025 1:53, Jakub Kicinski wrote:
> A few quick comments here as the test is failing
>
> On Tue, 20 May 2025 21:38:02 +0300 Tariq Toukan wrote:
>> + -
>> + name: rate-tc-bws
>> + type: nest
>> + multi-attr: true
>> + nested-attributes: dl-rate-tc-bws
>> + -
>> + name: rate-tc-index
>> + type: u8
>> + checks:
>> + min: 0
>> + max: rate-tc-index-max
>
> no need for min: 0 on an unsigned type ?
>
Will remove them.
>> + -
>> + name: rate-tc-bw
>> + type: u32
>> + doc: |
>> + Specifies the bandwidth allocation for the Traffic Class as a
>> + percentage.
>> + checks:
>> + min: 0
>> + max: 100
>
> Why in percentage? I don't think any existing param in devlink rate
> or net shapers is in percentage right? Not according to what i can
> grok about the uAPI.
>
I thought percentage might fit better here because it lets users clearly
set the bandwidth share for each traffic class. While this isn’t the
same as tx_weight in devlink-rate, the idea is related since both use
relative values. If there isn’t a strong reason against it, I’d like to
keep using percentages here.
>> +static int devlink_nl_rate_tc_bw_parse(struct nlattr *parent_nest, u32 *tc_bw,
>> + unsigned long *bitmap, struct netlink_ext_ack *extack)
>> +{
>> + struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
>> + u8 tc_index;
>> +
>> + nla_parse_nested(tb, DEVLINK_ATTR_MAX, parent_nest, devlink_dl_rate_tc_bws_nl_policy,
>
> Let's error check this, I get that we already validated via the policy
> but what if we do memory allocations during parsing one day, or some
> other failure-prone operation.. better check the return value.
>
Ack. will do.
> nit: over 80 chars for no good reason, the line overflows anyway.
> Please use checkpatch --max-line-width=80 for core code,
> at the very least.
>
I noticed the current code already goes over 80 chars, but I’ll update
it to follow the 80-char limit. Will fix, thanks.
>> + extack);
>> + if (!tb[DEVLINK_ATTR_RATE_TC_INDEX]) {
>> + NL_SET_ERR_ATTR_MISS(extack, parent_nest, DEVLINK_ATTR_RATE_TC_INDEX);
>> + return -EINVAL;
>> + }
>> +
>> + tc_index = nla_get_u8(tb[DEVLINK_ATTR_RATE_TC_INDEX]);
>> +
>> + if (!tb[DEVLINK_ATTR_RATE_TC_BW]) {
>> + NL_SET_ERR_ATTR_MISS(extack, parent_nest, DEVLINK_ATTR_RATE_TC_BW);
>> + return -EINVAL;
>> + }
>> +
>> + if (test_and_set_bit(tc_index, bitmap)) {
>> + NL_SET_ERR_MSG_FMT(extack, "Duplicate traffic class index specified (%u)",
>> + tc_index);
>> + return -EINVAL;
>> + }
>> +
>> + tc_bw[tc_index] = nla_get_u32(tb[DEVLINK_ATTR_RATE_TC_BW]);
>> +
>> + return 0;
>> +}
>> +
>> +static int devlink_nl_rate_tc_bw_set(struct devlink_rate *devlink_rate,
>> + struct genl_info *info)
>> +{
>> + DECLARE_BITMAP(bitmap, DEVLINK_RATE_TCS_MAX) = {};
>> + struct devlink *devlink = devlink_rate->devlink;
>> + const struct devlink_ops *ops = devlink->ops;
>> + int rem, err = -EOPNOTSUPP, i, total = 0;
>> + u32 tc_bw[DEVLINK_RATE_TCS_MAX] = {};
>> + struct nlattr *attr;
>> +
>> + nla_for_each_attr(attr, genlmsg_data(info->genlhdr),
>> + genlmsg_len(info->genlhdr), rem) {
>
> nla_for_each_attr_type() ?
> or better still add a _type() version of nlmsg_for_each_attr() ?
>
Good point, thanks. I’ll add a _type() version for nlmsg_for_each_attr.
Do you prefer this to be part of this patch or should I send it as a
separate patch?
>> + if (nla_type(attr) == DEVLINK_ATTR_RATE_TC_BWS) {
>> + err = devlink_nl_rate_tc_bw_parse(attr, tc_bw, bitmap, info->extack);
>> + if (err)
>> + return err;
>> + }
>> + }
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test
2025-05-21 14:10 ` Jakub Kicinski
@ 2025-05-25 20:31 ` Gal Pressman
0 siblings, 0 replies; 16+ messages in thread
From: Gal Pressman @ 2025-05-25 20:31 UTC (permalink / raw)
To: Jakub Kicinski, Tariq Toukan
Cc: Carolina Jubran, Tariq Toukan, David S. Miller, Paolo Abeni,
Eric Dumazet, Andrew Lunn, Jiri Pirko, Leon Romanovsky,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Cosmin Ratiu
On 21/05/2025 17:10, Jakub Kicinski wrote:
>> We could've saved this extra cycle if my questions [1] exactly about
>> this topic weren't ignored.
>> Area is vague and not well defined. We can continue with the iterative
>> guess and fix cycles, or alternatively get it clearly and formally defined.
>
> I started a couple of times on answering but my hands go a little limb
> when I have to explain things so obvious like "testing is a crucial part
> of software development" :S
You're acting as if kernel testing is obvious, and the only way to test
the kernel is through selftests. The kernel and networking subsystem are
tested regardless of selftests, the fact that you don't see the tests
does not mean it doesn't happen.
We are not new contributors, plenty of uapi changes have been merged
without selftests, it's natural for us to try and understand this new
requirement.
> I mean.. nvidia certainly tests their code, so I'm not sure where the disconnect is.
Absolutely!
Our testing and coverage are far more complex and valuable than what the
existing selftests provide.
We have testing infrastructure that predates these recent selftests
addition by years, and we don't plan on switching them to something
inferior.
Writing selftests will not replace our internal tests, we're wasting
extra cycles adapting our tests to yet another project, just for the
sake of having them in the kernel tree?
> I had a short conversation with
> Gal at some conference where he, AFAIU, was doubting that device testing
> can be part of an open source project.
What's your point?
Did that prevent me from being a top 10 netdev selftests contributor in
v6.15?
I don't care if tests are open source or not, I care about having good
tests that run often and report bugs and regressions.
>
> It certainly is not advantageous to companies to have to share their
> test code. So when you ask me for details on the rules what I hear is
> "how can we make sure we do as little as possible".
No, I'm asking for predictability, we've been circling this point for
quite some time.
We shouldn't have to wait until v9 to guess whether a certain submission
requires the addition of a test.
We shouldn't submit a bug fix, to find out that it's blocked due to lack
of a test.
As an example, we have well documented coding style, so:
- You don't have to waste your time commenting on things that could have
been handled before submission.
- Rules are clear, comments don't rely on personal taste or mood.
- Developers learn, number of review iterations are reduced.
What's the difference with documenting requirements for tests? We can't
read your mind.
>
> Broadly, any new uAPI should come with tests which exercise the
> functionality.
I'm fine with this definition (though I think it's too vague), can I add
it to maintainer-netdev.rst?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next V10 1/6] devlink: Extend devlink rate API with traffic classes bandwidth management
2025-05-25 14:57 ` Carolina Jubran
@ 2025-05-27 23:28 ` Jakub Kicinski
0 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2025-05-27 23:28 UTC (permalink / raw)
To: Carolina Jubran
Cc: Tariq Toukan, David S. Miller, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Jiri Pirko, Gal Pressman, Leon Romanovsky,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Shuah Khan, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, Moshe Shemesh, Mark Bloch,
Cosmin Ratiu
On Sun, 25 May 2025 17:57:28 +0300 Carolina Jubran wrote:
> >> + -
> >> + name: rate-tc-bw
> >> + type: u32
> >> + doc: |
> >> + Specifies the bandwidth allocation for the Traffic Class as a
> >> + percentage.
> >> + checks:
> >> + min: 0
> >> + max: 100
> >
> > Why in percentage? I don't think any existing param in devlink rate
> > or net shapers is in percentage right? Not according to what i can
> > grok about the uAPI.
> >
>
> I thought percentage might fit better here because it lets users clearly
> set the bandwidth share for each traffic class. While this isn’t the
> same as tx_weight in devlink-rate, the idea is related since both use
> relative values. If there isn’t a strong reason against it, I’d like to
> keep using percentages here.
The existing APIs use absolute values for b/w limits.
If you want to diverge please provide clear and realistic use cases.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-05-27 23:28 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-20 18:38 [PATCH net-next V10 0/6] Support rate management on traffic classes in devlink and mlx5 Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 1/6] devlink: Extend devlink rate API with traffic classes bandwidth management Tariq Toukan
2025-05-20 22:53 ` Jakub Kicinski
2025-05-25 14:57 ` Carolina Jubran
2025-05-27 23:28 ` Jakub Kicinski
2025-05-20 18:38 ` [PATCH net-next V10 2/6] selftest: netdevsim: Add devlink rate tc-bw test Tariq Toukan
2025-05-20 22:59 ` Jakub Kicinski
2025-05-21 7:05 ` Tariq Toukan
2025-05-21 14:10 ` Jakub Kicinski
2025-05-25 20:31 ` Gal Pressman
2025-05-21 21:05 ` Carolina Jubran
2025-05-21 22:24 ` Jakub Kicinski
2025-05-20 18:38 ` [PATCH net-next V10 3/6] net/mlx5: Add no-op implementation for setting tc-bw on rate objects Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 4/6] net/mlx5: Add support for setting tc-bw on nodes Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 5/6] net/mlx5: Add traffic class scheduling support for vport QoS Tariq Toukan
2025-05-20 18:38 ` [PATCH net-next V10 6/6] net/mlx5: Manage TC arbiter nodes and implement full support for tc-bw Tariq Toukan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).