On Tue, Nov 18, 2025 at 07:44:37AM +0000, Bing Zhao wrote: > Hi, > > Code LGTM, but I have 1 question. Does the root group really supports so many > TAG registers?
Yes, but it depends on the NIC capability. REG_C_8 through REG_C_11 are available only in ConnectX-7 and up. > > If not, do we have a need to translate them and pass to the driver to get the > return error code from the driver then? There's no need to rely on the kernel driver. mlx5 PMD queries FW capabilities to determine which registers are supported. Based on that, a map of flow tag indexes to HW registers is constructed. If given tag index cannot be supported, this map will contain REG_NON. This info is used to reject given flow rule in case it cannot be supported (in flow_dv_translate_item_tag()). > > > -----Original Message----- > > From: Dariusz Sosnowski <[email protected]> > > Sent: Saturday, November 15, 2025 4:17 AM > > To: Slava Ovsiienko <[email protected]>; Bing Zhao > > <[email protected]>; Ori Kam <[email protected]>; Suanming Mou > > <[email protected]>; Matan Azrad <[email protected]>; Itamar Gozlan > > <[email protected]> > > Cc: [email protected]; [email protected] > > Subject: [PATCH] net/mlx5: fix higher flow tag indexes support on root > > > > Offending patch introduced support for additional flow tag indexes with HW > > Steering flow engine. New tag indexes will be mapped to HW registers > > REG_C_8 to REG_C_11, depending on HW capabilities. > > > > That patch only handled tables created on group > 0 (non-root table), > > where mlx5 PMD directly configures the HW. > > Tables and flow rules on group 0 (root table) are handled through kernel > > driver, and new registers were not addressed for that case. > > > > Because of that, usage of unsupported tag index in group 0 triggered an > > assertion in flow_dv_match_meta_reg(). > > > > This patch adds necessary definitions for REG_C_8 to REG_C_11 to make > > these registers usable for flow tag indexes in root table. > > Validation of flow tag to HW register translation is also amended to > > report invalid cases to the user, instead of relying on assertions. > > > > Fixes: 7e3a14423c1a ("net/mlx5/hws: support 4 additional C registers") > > Cc: [email protected] > > Cc: [email protected] > > > > Signed-off-by: Dariusz Sosnowski <[email protected]> > > --- > > drivers/common/mlx5/mlx5_prm.h | 6 ++++- > > drivers/net/mlx5/mlx5_flow.h | 3 ++- > > drivers/net/mlx5/mlx5_flow_dv.c | 40 ++++++++++++++++++++++++++++----- > > 3 files changed, 41 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/common/mlx5/mlx5_prm.h > > b/drivers/common/mlx5/mlx5_prm.h index 5db8d67cfc..ad3cca5bd6 100644 > > --- a/drivers/common/mlx5/mlx5_prm.h > > +++ b/drivers/common/mlx5/mlx5_prm.h > > @@ -1205,7 +1205,11 @@ struct mlx5_ifc_fte_match_set_misc5_bits { > > u8 tunnel_header_1[0x20]; > > u8 tunnel_header_2[0x20]; > > u8 tunnel_header_3[0x20]; > > - u8 reserved[0x100]; > > + u8 reserved[0x80]; > > + u8 metadata_reg_c_8[0x20]; > > + u8 metadata_reg_c_9[0x20]; > > + u8 metadata_reg_c_10[0x20]; > > + u8 metadata_reg_c_11[0x20]; > > }; > > > > /* Flow matcher. */ > > diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h > > index e8b298dd1d..fa2af91d66 100644 > > --- a/drivers/net/mlx5/mlx5_flow.h > > +++ b/drivers/net/mlx5/mlx5_flow.h > > @@ -1833,7 +1833,8 @@ flow_hw_get_reg_id_by_domain(struct rte_eth_dev > > *dev, > > case RTE_FLOW_ITEM_TYPE_TAG: > > if (id == RTE_PMD_MLX5_LINEAR_HASH_TAG_INDEX) > > return REG_C_3; > > - MLX5_ASSERT(id < MLX5_FLOW_HW_TAGS_MAX); > > + if (id >= MLX5_FLOW_HW_TAGS_MAX) > > + return REG_NON; > > return reg->hw_avl_tags[id]; > > default: > > return REG_NON; > > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c > > b/drivers/net/mlx5/mlx5_flow_dv.c index 95ca57e8c4..2a8bdd498e 100644 > > --- a/drivers/net/mlx5/mlx5_flow_dv.c > > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > > @@ -10560,8 +10560,8 @@ static void > > flow_dv_match_meta_reg(void *key, enum modify_reg reg_type, > > uint32_t data, uint32_t mask) > > { > > - void *misc2_v = > > - MLX5_ADDR_OF(fte_match_param, key, misc_parameters_2); > > + void *misc2_v = MLX5_ADDR_OF(fte_match_param, key, > > misc_parameters_2); > > + void *misc5_v = MLX5_ADDR_OF(fte_match_param, key, > > misc_parameters_5); > > uint32_t temp; > > > > if (!key) > > @@ -10607,6 +10607,18 @@ flow_dv_match_meta_reg(void *key, enum modify_reg > > reg_type, > > case REG_C_7: > > MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_c_7, > > data); > > break; > > + case REG_C_8: > > + MLX5_SET(fte_match_set_misc5, misc5_v, metadata_reg_c_8, > > data); > > + break; > > + case REG_C_9: > > + MLX5_SET(fte_match_set_misc5, misc5_v, metadata_reg_c_9, > > data); > > + break; > > + case REG_C_10: > > + MLX5_SET(fte_match_set_misc5, misc5_v, metadata_reg_c_10, > > data); > > + break; > > + case REG_C_11: > > + MLX5_SET(fte_match_set_misc5, misc5_v, metadata_reg_c_11, > > data); > > + break; > > default: > > MLX5_ASSERT(false); > > break; > > @@ -10825,8 +10837,11 @@ flow_dv_translate_mlx5_item_tag(struct > > rte_eth_dev *dev, void *key, > > * Flow pattern to translate. > > * @param[in] key_type > > * Set flow matcher mask or value. > > + * > > + * @return > > + * 0 on success. Negative errno value otherwise. > > */ > > -static void > > +static int > > flow_dv_translate_item_tag(struct rte_eth_dev *dev, void *key, > > const struct rte_flow_item *item, > > uint32_t key_type) > > @@ -10838,7 +10853,7 @@ flow_dv_translate_item_tag(struct rte_eth_dev > > *dev, void *key, > > uint32_t index; > > > > if (MLX5_ITEM_VALID(item, key_type)) > > - return; > > + return 0; > > MLX5_ITEM_UPDATE(item, key_type, tag_v, tag_m, > > &rte_flow_item_tag_mask); > > /* When set mask, the index should be from spec. */ @@ -10848,8 > > +10863,18 @@ flow_dv_translate_item_tag(struct rte_eth_dev *dev, void > > *key, > > reg = mlx5_flow_get_reg_id(dev, MLX5_APP_TAG, index, NULL); > > else > > reg = flow_hw_get_reg_id(dev, RTE_FLOW_ITEM_TYPE_TAG, index); > > - MLX5_ASSERT(reg > 0); > > + if (reg < 0) { > > + DRV_LOG(ERR, "port %u tag index %u does not map to correct > > register", > > + dev->data->port_id, index); > > + return -EINVAL; > > + } > > + if (reg == REG_NON) { > > + DRV_LOG(ERR, "port %u tag index %u maps to unsupported > > register", > > + dev->data->port_id, index); > > + return -ENOTSUP; > > + } > > flow_dv_match_meta_reg(key, (enum modify_reg)reg, tag_v->data, > > tag_m->data); > > + return 0; > > } > > > > /** > > @@ -14408,7 +14433,10 @@ flow_dv_translate_items(struct rte_eth_dev *dev, > > last_item = MLX5_FLOW_LAYER_ICMP6; > > break; > > case RTE_FLOW_ITEM_TYPE_TAG: > > - flow_dv_translate_item_tag(dev, key, items, key_type); > > + ret = flow_dv_translate_item_tag(dev, key, items, key_type); > > + if (ret < 0) > > + return rte_flow_error_set(error, -ret, > > RTE_FLOW_ERROR_TYPE_ITEM, NULL, > > + "invalid flow tag item"); > > last_item = MLX5_FLOW_ITEM_TAG; > > break; > > case MLX5_RTE_FLOW_ITEM_TYPE_TAG: > > -- > > 2.39.5 > Best regards, Dariusz Sosnowski
