[dpdk-dev] [PATCH 2/2] net/mlx5: fix incorrect register usage in meter

Suanming Mou suanmingm at mellanox.com
Thu Jan 23 07:01:02 CET 2020


Flow with meter will split to three subflows, the prefix subflow with
meter action do the color, the meter subflow  filter the packets, the
suffix subflow do all the left actions for packets pass the filter.
Both the color and the subflow match between prefix and suffix use the
register to store the tag.

For some of the NICs with meter color register share capability, it
only uses 8 LSB of the register for color, the left 24 MSB can be used
for flow id match between meter prefix subflow and suffix subflow.

Currently, one entire register is allocated for flow matching which
causes the NICs with limited registers don't have enough register for
other matching.

Add the meter color share capability checking to fix lacking of
registers issue.

Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")
Cc: stable at dpdk.org

Signed-off-by: Suanming Mou <suanmingm at mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo at mellanox.com>
---
 drivers/net/mlx5/mlx5.c           |  9 ++++++-
 drivers/net/mlx5/mlx5.h           |  3 +++
 drivers/net/mlx5/mlx5_devx_cmds.c |  2 ++
 drivers/net/mlx5/mlx5_flow.c      | 49 ++++++++++++++++++++++++---------------
 drivers/net/mlx5/mlx5_flow_dv.c   |  4 ++--
 drivers/net/mlx5/mlx5_prm.h       |  7 +++++-
 6 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 2c9f705..2049370 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2552,6 +2552,8 @@ struct mlx5_flow_id_pool *
 				priv->mtr_color_reg = ffs(reg_c_mask) - 1 +
 						      REG_C_0;
 				priv->mtr_en = 1;
+				priv->mtr_reg_share =
+				      config.hca_attr.qos.flow_meter_reg_share;
 				DRV_LOG(DEBUG, "The REG_C meter uses is %d",
 					priv->mtr_color_reg);
 			}
@@ -2684,7 +2686,12 @@ struct mlx5_flow_id_pool *
 		err = mlx5_alloc_shared_dr(priv);
 		if (err)
 			goto error;
-		priv->qrss_id_pool = mlx5_flow_id_pool_alloc(UINT32_MAX);
+		/*
+		 * RSS id is shared with meter flow id. Meter flow id can only
+		 * use the 24 MSB of the register.
+		 */
+		priv->qrss_id_pool = mlx5_flow_id_pool_alloc(UINT32_MAX >>
+				     MLX5_MTR_COLOR_BITS);
 		if (!priv->qrss_id_pool) {
 			DRV_LOG(ERR, "can't create flow id pool");
 			err = ENOMEM;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index ac86c19..5818349 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -173,6 +173,8 @@ struct mlx5_devx_mkey_attr {
 struct mlx5_hca_qos_attr {
 	uint32_t sup:1;	/* Whether QOS is supported. */
 	uint32_t srtcm_sup:1; /* Whether srTCM mode is supported. */
+	uint32_t flow_meter_reg_share:1;
+	/* Whether reg_c share is supported. */
 	uint8_t log_max_flow_meter;
 	/* Power of the maximum supported meters. */
 	uint8_t flow_meter_reg_c_ids;
@@ -732,6 +734,7 @@ struct mlx5_priv {
 	unsigned int dr_shared:1; /* DV/DR data is shared. */
 	unsigned int counter_fallback:1; /* Use counter fallback management. */
 	unsigned int mtr_en:1; /* Whether support meter. */
+	unsigned int mtr_reg_share:1; /* Whether support meter REG_C share. */
 	uint16_t domain_id; /* Switch domain identifier. */
 	uint16_t vport_id; /* Associated VF vport index (if any). */
 	uint32_t vport_meta_tag; /* Used for vport index match ove VF LAG. */
diff --git a/drivers/net/mlx5/mlx5_devx_cmds.c b/drivers/net/mlx5/mlx5_devx_cmds.c
index 9985d30..282d501 100644
--- a/drivers/net/mlx5/mlx5_devx_cmds.c
+++ b/drivers/net/mlx5/mlx5_devx_cmds.c
@@ -362,6 +362,8 @@ struct mlx5_devx_obj *
 				MLX5_GET(qos_cap, hcattr, log_max_flow_meter);
 		attr->qos.flow_meter_reg_c_ids =
 			MLX5_GET(qos_cap, hcattr, flow_meter_reg_id);
+		attr->qos.flow_meter_reg_share =
+			MLX5_GET(qos_cap, hcattr, flow_meter_reg_share);
 	}
 	if (!attr->eth_net_offloads)
 		return 0;
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 970123b..3ca5ddb 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -350,6 +350,7 @@ enum modify_reg
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_config *config = &priv->config;
 	enum modify_reg start_reg;
+	bool skip_mtr_reg = false;
 
 	switch (feature) {
 	case MLX5_HAIRPIN_RX:
@@ -388,29 +389,36 @@ enum modify_reg
 			return REG_C_0;
 		}
 		break;
-	case MLX5_COPY_MARK:
 	case MLX5_MTR_SFX:
 		/*
-		 * Metadata COPY_MARK register using is in meter suffix sub
-		 * flow while with meter. It's safe to share the same register.
+		 * If meter color and flow match share one register, flow match
+		 * should use the meter color register for match.
 		 */
-		return priv->mtr_color_reg != REG_C_2 ? REG_C_2 : REG_C_3;
+		if (priv->mtr_reg_share)
+			return priv->mtr_color_reg;
+		else
+			return priv->mtr_color_reg != REG_C_2 ? REG_C_2 :
+			       REG_C_3;
 	case MLX5_MTR_COLOR:
 		RTE_ASSERT(priv->mtr_color_reg != REG_NONE);
 		return priv->mtr_color_reg;
+	case MLX5_COPY_MARK:
+		/*
+		 * Metadata COPY_MARK register using is in meter suffix sub
+		 * flow while with meter. It's safe to share the same register.
+		 */
+		return priv->mtr_color_reg != REG_C_2 ? REG_C_2 : REG_C_3;
 	case MLX5_APP_TAG:
 		/*
-		 * If meter is enable, it will engage two registers for color
+		 * If meter is enable, it will engage the register for color
 		 * match and flow match. If meter color match is not using the
 		 * REG_C_2, need to skip the REG_C_x be used by meter color
 		 * match.
 		 * If meter is disable, free to use all available registers.
 		 */
-		if (priv->mtr_color_reg != REG_NONE)
-			start_reg = priv->mtr_color_reg != REG_C_2 ? REG_C_3 :
-				    REG_C_4;
-		else
-			start_reg = REG_C_2;
+		start_reg = priv->mtr_color_reg != REG_C_2 ? REG_C_2 :
+			    (priv->mtr_reg_share ? REG_C_3 : REG_C_4);
+		skip_mtr_reg = !!(priv->mtr_en && start_reg == REG_C_2);
 		if (id > (REG_C_7 - start_reg))
 			return rte_flow_error_set(error, EINVAL,
 						  RTE_FLOW_ERROR_TYPE_ITEM,
@@ -425,12 +433,12 @@ enum modify_reg
 		 * If the available index REG_C_y >= REG_C_x, skip the
 		 * color register.
 		 */
-		if (start_reg == REG_C_3 && config->flow_mreg_c
-		    [id + REG_C_3 - REG_C_0] >= priv->mtr_color_reg) {
-			if (config->flow_mreg_c[id + 1 + REG_C_3 - REG_C_0] !=
-			    REG_NONE)
+		if (skip_mtr_reg && config->flow_mreg_c
+		    [id + start_reg - REG_C_0] >= priv->mtr_color_reg) {
+			if (config->flow_mreg_c
+			    [id + 1 + start_reg - REG_C_0] != REG_NONE)
 				return config->flow_mreg_c
-						[id + 1 + REG_C_3 - REG_C_0];
+					       [id + 1 + start_reg - REG_C_0];
 			return rte_flow_error_set(error, ENOTSUP,
 						  RTE_FLOW_ERROR_TYPE_ITEM,
 						  NULL, "unsupported tag id");
@@ -3556,7 +3564,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	 * Get the id from the qrss_pool to make qrss share the id with meter.
 	 */
 	tag_id = flow_qrss_get_id(dev);
-	set_tag->data = rte_cpu_to_be_32(tag_id);
+	set_tag->data = tag_id << MLX5_MTR_COLOR_BITS;
 	tag_action->conf = set_tag;
 	return tag_id;
 }
@@ -3994,13 +4002,14 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		actions_n = flow_check_meter_action(actions, &mtr);
 	if (mtr) {
 		struct mlx5_rte_flow_item_tag *tag_spec;
+		struct mlx5_rte_flow_item_tag *tag_mask;
 		/* The five prefix actions: meter, decap, encap, tag, end. */
 		act_size = sizeof(struct rte_flow_action) * (actions_n + 5) +
 			   sizeof(struct rte_flow_action_set_tag);
 		/* tag, end. */
 #define METER_SUFFIX_ITEM 3
 		item_size = sizeof(struct rte_flow_item) * METER_SUFFIX_ITEM +
-			    sizeof(struct mlx5_rte_flow_item_tag);
+			    sizeof(struct mlx5_rte_flow_item_tag) * 2;
 		sfx_actions = rte_zmalloc(__func__, (act_size + item_size), 0);
 		if (!sfx_actions)
 			return rte_flow_error_set(error, ENOMEM,
@@ -4027,13 +4036,15 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			     act_size);
 		tag_spec = (struct mlx5_rte_flow_item_tag *)(sfx_items +
 			    METER_SUFFIX_ITEM);
-		tag_spec->data = rte_cpu_to_be_32(dev_flow->mtr_flow_id);
+		tag_spec->data = dev_flow->mtr_flow_id << MLX5_MTR_COLOR_BITS;
 		tag_spec->id = mlx5_flow_get_reg_id(dev, MLX5_MTR_SFX, 0,
 						    error);
+		tag_mask = tag_spec + 1;
+		tag_mask->data = 0xffffff00;
 		sfx_items->type = MLX5_RTE_FLOW_ITEM_TYPE_TAG;
 		sfx_items->spec = tag_spec;
 		sfx_items->last = NULL;
-		sfx_items->mask = NULL;
+		sfx_items->mask = tag_mask;
 		sfx_items++;
 		sfx_port_id_item = find_port_id_item(items);
 		if (sfx_port_id_item) {
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 5a1b426..64397c9 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -8304,7 +8304,7 @@ struct field_modify_info modify_tcp[] = {
 	dv_attr.match_criteria_enable =
 				1 << MLX5_MATCH_CRITERIA_ENABLE_MISC2_BIT;
 	flow_dv_match_meta_reg(mask.buf, value.buf, color_reg_c_idx,
-			       rte_col_2_mlx5_col(RTE_COLORS), UINT32_MAX);
+			       rte_col_2_mlx5_col(RTE_COLORS), UINT8_MAX);
 	dtb->color_matcher = mlx5_glue->dv_create_flow_matcher(sh->ctx,
 							       &dv_attr,
 							       dtb->tbl->obj);
@@ -8498,7 +8498,7 @@ struct field_modify_info modify_tcp[] = {
 		int j = 0;
 
 		flow_dv_match_meta_reg(matcher.buf, value.buf, mtr_reg_c,
-				       rte_col_2_mlx5_col(i), UINT32_MAX);
+				       rte_col_2_mlx5_col(i), UINT8_MAX);
 		if (mtb->count_actns[i])
 			actions[j++] = mtb->count_actns[i];
 		if (fm->params.action[i] == MTR_POLICER_ACTION_DROP)
diff --git a/drivers/net/mlx5/mlx5_prm.h b/drivers/net/mlx5/mlx5_prm.h
index 6ad214b..8a67025 100644
--- a/drivers/net/mlx5/mlx5_prm.h
+++ b/drivers/net/mlx5/mlx5_prm.h
@@ -1200,7 +1200,9 @@ struct mlx5_ifc_qos_cap_bits {
 	u8 reserved_at_8[0x8];
 	u8 log_max_flow_meter[0x8];
 	u8 flow_meter_reg_id[0x8];
-	u8 reserved_at_25[0x20];
+	u8 reserved_at_25[0x8];
+	u8 flow_meter_reg_share[0x1];
+	u8 reserved_at_2e[0x17];
 	u8 packet_pacing_max_rate[0x20];
 	u8 packet_pacing_min_rate[0x20];
 	u8 reserved_at_80[0x10];
@@ -1822,6 +1824,9 @@ enum {
 #define MLX5_SRTCM_CIR_MAX (8 * (1ULL << 30) * 0xFF)
 #define MLX5_SRTCM_EBS_MAX 0
 
+/* The bits meter color use. */
+#define MLX5_MTR_COLOR_BITS 8
+
 /**
  * Convert a user mark to flow mark.
  *
-- 
1.8.3.1



More information about the dev mailing list