RE: [PATCH] net/mlx5: fix higher flow tag indexes support on root

[Bing Zhao <bingz@nvidia.com>]

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 <dsosnowski@nvidia.com>
> Sent: Saturday, November 15, 2025 4:17 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Bing Zhao
> <bingz@nvidia.com>; Ori Kam <orika@nvidia.com>; Suanming Mou
> <suanmingm@nvidia.com>; Matan Azrad <matan@nvidia.com>; Itamar Gozlan
> <igozlan@nvidia.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> 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: igozlan@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> ---
>  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