Hi, Code LGTM, but I have 1 question. Does the root group really supports so many TAG registers?
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? > -----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
