[dpdk-dev] [PATCH v1 0/4] ASO meter sharing support

[dpdk-dev] [PATCH v1 0/4] ASO meter sharing support

[Shun Hao]

This adds several fixes in ASO meter. With these fixes, meter created
on one port can be shared by other ports.

Shun Hao (4):
  net/mlx5: use meter profile lookup table
  net/mlx5: fix meter policy ID table container
  net/mlx5: fix meter flow direction check
  net/mlx5: fix meter policy flow match item

 doc/guides/nics/mlx5.rst           |  8 +++
 drivers/net/mlx5/linux/mlx5_os.c   |  6 +-
 drivers/net/mlx5/mlx5.c            |  2 -
 drivers/net/mlx5/mlx5.h            |  7 ++-
 drivers/net/mlx5/mlx5_flow.c       | 83 ++++++++++++++++-----------
 drivers/net/mlx5/mlx5_flow_dv.c    | 58 ++++++++++++++++---
 drivers/net/mlx5/mlx5_flow_meter.c | 91 +++++++++++++++++-------------
 drivers/net/mlx5/windows/mlx5_os.c |  6 +-
 8 files changed, 172 insertions(+), 89 deletions(-)

-- 
2.20.0

[dpdk-dev] [PATCH v1 1/4] net/mlx5: use meter profile lookup table

[Shun Hao]

Currently a list is used to save all meter profile ids, which is
not efficient when looking up profile from huge mount of profiles.

This changes to use a l3 table instead to save meter profile ids,
so as to improve the lookup performance.

Signed-off-by: Shun Hao <shunh@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_os.c   |  6 +++-
 drivers/net/mlx5/mlx5.h            |  2 +-
 drivers/net/mlx5/mlx5_flow_meter.c | 53 ++++++++++++++++++++----------
 drivers/net/mlx5/windows/mlx5_os.c |  6 +++-
 4 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 92b3009786..b3f9e392ab 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1570,7 +1570,9 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	priv->ctrl_flows = 0;
 	rte_spinlock_init(&priv->flow_list_lock);
 	TAILQ_INIT(&priv->flow_meters);
-	TAILQ_INIT(&priv->flow_meter_profiles);
+	priv->mtr_profile_tbl = mlx5_l3t_create(MLX5_L3T_TYPE_PTR);
+	if (!priv->mtr_profile_tbl)
+		goto error;
 	/* Hint libmlx5 to use PMD allocator for data plane resources */
 	mlx5_glue->dv_set_context_attr(sh->ctx,
 			MLX5DV_CTX_ATTR_BUF_ALLOCATORS,
@@ -1713,6 +1715,8 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 			mlx5_vlan_vmwa_exit(priv->vmwa_context);
 		if (eth_dev && priv->drop_queue.hrxq)
 			mlx5_drop_action_destroy(eth_dev);
+		if (priv->mtr_profile_tbl)
+			mlx5_l3t_destroy(priv->mtr_profile_tbl);
 		if (own_domain_id)
 			claim_zero(rte_eth_switch_domain_free(priv->domain_id));
 		mlx5_cache_list_destroy(&priv->hrxqs);
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 1b2dc8f815..0226327bc3 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1373,8 +1373,8 @@ struct mlx5_priv {
 	/* Hash table of Rx metadata register copy table. */
 	uint8_t mtr_sfx_reg; /* Meter prefix-suffix flow match REG_C. */
 	uint8_t mtr_color_reg; /* Meter color match REG_C. */
-	struct mlx5_mtr_profiles flow_meter_profiles; /* MTR profile list. */
 	struct mlx5_legacy_flow_meters flow_meters; /* MTR list. */
+	struct mlx5_l3t_tbl *mtr_profile_tbl; /* Meter index lookup table. */
 	struct mlx5_l3t_tbl *mtr_idx_tbl; /* Meter index lookup table. */
 	uint8_t skip_default_rss_reta; /* Skip configuration of default reta. */
 	uint8_t fdb_def_rule; /* Whether fdb jump to table 1 is configured. */
diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c
index d7ce5cd2f6..87c5782956 100644
--- a/drivers/net/mlx5/mlx5_flow_meter.c
+++ b/drivers/net/mlx5/mlx5_flow_meter.c
@@ -91,13 +91,20 @@ mlx5_flow_meter_action_create(struct mlx5_priv *priv,
 static struct mlx5_flow_meter_profile *
 mlx5_flow_meter_profile_find(struct mlx5_priv *priv, uint32_t meter_profile_id)
 {
-	struct mlx5_mtr_profiles *fmps = &priv->flow_meter_profiles;
 	struct mlx5_flow_meter_profile *fmp;
+	union mlx5_l3t_data data;
+	int32_t ret;
 
-	TAILQ_FOREACH(fmp, fmps, next)
-		if (meter_profile_id == fmp->id)
-			return fmp;
-	return NULL;
+	if (mlx5_l3t_get_entry(priv->mtr_profile_tbl,
+			       meter_profile_id, &data) || !data.ptr)
+		return NULL;
+	fmp = data.ptr;
+	/* Remove reference taken by the mlx5_l3t_get_entry. */
+	ret = mlx5_l3t_clear_entry(priv->mtr_profile_tbl,
+				   meter_profile_id);
+	if (!ret || ret == -1)
+		return NULL;
+	return fmp;
 }
 
 /**
@@ -399,8 +406,8 @@ mlx5_flow_meter_profile_add(struct rte_eth_dev *dev,
 		       struct rte_mtr_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_mtr_profiles *fmps = &priv->flow_meter_profiles;
 	struct mlx5_flow_meter_profile *fmp;
+	union mlx5_l3t_data data;
 	int ret;
 
 	if (!priv->mtr_en)
@@ -427,8 +434,13 @@ mlx5_flow_meter_profile_add(struct rte_eth_dev *dev,
 	ret = mlx5_flow_meter_param_fill(fmp, priv, error);
 	if (ret)
 		goto error;
-	/* Add to list. */
-	TAILQ_INSERT_TAIL(fmps, fmp, next);
+	data.ptr = fmp;
+	ret = mlx5_l3t_set_entry(priv->mtr_profile_tbl,
+				 meter_profile_id, &data);
+	if (ret)
+		return -rte_mtr_error_set(error, ENOTSUP,
+					  RTE_MTR_ERROR_TYPE_UNSPECIFIED,
+					  NULL, "Meter profile insert fail.");
 	return 0;
 error:
 	mlx5_free(fmp);
@@ -472,8 +484,10 @@ mlx5_flow_meter_profile_delete(struct rte_eth_dev *dev,
 		return -rte_mtr_error_set(error, EBUSY,
 					  RTE_MTR_ERROR_TYPE_METER_PROFILE_ID,
 					  NULL, "Meter profile is in use.");
-	/* Remove from list. */
-	TAILQ_REMOVE(&priv->flow_meter_profiles, fmp, next);
+	if (mlx5_l3t_clear_entry(priv->mtr_profile_tbl, meter_profile_id))
+		return -rte_mtr_error_set(error, EBUSY,
+					  RTE_MTR_ERROR_TYPE_METER_PROFILE_ID,
+					  NULL, "Meter profile remove fail.");
 	mlx5_free(fmp);
 	return 0;
 }
@@ -1859,7 +1873,6 @@ mlx5_flow_meter_flush(struct rte_eth_dev *dev, struct rte_mtr_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_legacy_flow_meters *fms = &priv->flow_meters;
-	struct mlx5_mtr_profiles *fmps = &priv->flow_meter_profiles;
 	struct mlx5_flow_meter_profile *fmp;
 	struct mlx5_legacy_flow_meter *legacy_fm;
 	struct mlx5_flow_meter_info *fm;
@@ -1921,12 +1934,18 @@ mlx5_flow_meter_flush(struct rte_eth_dev *dev, struct rte_mtr_error *error)
 		mlx5_l3t_destroy(priv->sh->mtrmng->policy_idx_tbl);
 		priv->sh->mtrmng->policy_idx_tbl = NULL;
 	}
-	TAILQ_FOREACH_SAFE(fmp, fmps, next, tmp) {
-		/* Check unused. */
-		MLX5_ASSERT(!fmp->ref_cnt);
-		/* Remove from list. */
-		TAILQ_REMOVE(&priv->flow_meter_profiles, fmp, next);
-		mlx5_free(fmp);
+	if (priv->mtr_profile_tbl) {
+		MLX5_L3T_FOREACH(priv->mtr_profile_tbl, i, entry) {
+			fmp = entry;
+			if (mlx5_flow_meter_profile_delete(dev, fmp->id,
+							   error))
+				return -rte_mtr_error_set(error, EINVAL,
+					RTE_MTR_ERROR_TYPE_METER_POLICY_ID,
+						NULL, "Fail to destroy "
+						"meter profile.");
+		}
+		mlx5_l3t_destroy(priv->mtr_profile_tbl);
+		priv->mtr_profile_tbl = NULL;
 	}
 	/* Delete default policy table. */
 	mlx5_flow_destroy_def_policy(dev);
diff --git a/drivers/net/mlx5/windows/mlx5_os.c b/drivers/net/mlx5/windows/mlx5_os.c
index 3fe3f55f49..e30b682822 100644
--- a/drivers/net/mlx5/windows/mlx5_os.c
+++ b/drivers/net/mlx5/windows/mlx5_os.c
@@ -566,7 +566,9 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	priv->flows = 0;
 	priv->ctrl_flows = 0;
 	TAILQ_INIT(&priv->flow_meters);
-	TAILQ_INIT(&priv->flow_meter_profiles);
+	priv->mtr_profile_tbl = mlx5_l3t_create(MLX5_L3T_TYPE_PTR);
+	if (!priv->mtr_profile_tbl)
+		goto error;
 	/* Bring Ethernet device up. */
 	DRV_LOG(DEBUG, "port %u forcing Ethernet interface up.",
 		eth_dev->data->port_id);
@@ -644,6 +646,8 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	return eth_dev;
 error:
 	if (priv) {
+		if (priv->mtr_profile_tbl)
+			mlx5_l3t_destroy(priv->mtr_profile_tbl);
 		if (own_domain_id)
 			claim_zero(rte_eth_switch_domain_free(priv->domain_id));
 		mlx5_free(priv);
-- 
2.20.0

[dpdk-dev] [PATCH v1 2/4] net/mlx5: fix meter policy ID table container

[Shun Hao]

The meter policy handlers are managed by user IDs and the driver used l3
table in order to map the user ID to the internal driver handler of the
policy.

The l3 table was wrongly saved in the shared device structure which
manages all the switch domain ports what made the user IDs shared
between different ethdev ports.

Move the policy l3 table to be per port by saving it in the port private
structure.

Fixes: afb4aa4f122 ("net/mlx5: support meter policy operations")
Cc: stable@dpdk.org

Signed-off-by: Shun Hao <shunh@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5.c            |  2 --
 drivers/net/mlx5/mlx5.h            |  3 +--
 drivers/net/mlx5/mlx5_flow_meter.c | 38 ++++++++++++------------------
 3 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index cf1815cb74..5f10a45c02 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -600,8 +600,6 @@ mlx5_aso_flow_mtrs_mng_init(struct mlx5_dev_ctx_shared *sh)
 		if (sh->meter_aso_en) {
 			rte_spinlock_init(&sh->mtrmng->pools_mng.mtrsl);
 			LIST_INIT(&sh->mtrmng->pools_mng.meters);
-			sh->mtrmng->policy_idx_tbl =
-				mlx5_l3t_create(MLX5_L3T_TYPE_DWORD);
 		}
 		sh->mtrmng->def_policy_id = MLX5_INVALID_POLICY_ID;
 	}
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 0226327bc3..6eae7b6fd7 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -870,8 +870,6 @@ struct mlx5_flow_mtr_mng {
 	/* Default policy id. */
 	uint32_t def_policy_ref_cnt;
 	/** def_policy meter use count. */
-	struct mlx5_l3t_tbl *policy_idx_tbl;
-	/* Policy index lookup table. */
 	struct mlx5_flow_tbl_resource *drop_tbl[MLX5_MTR_DOMAIN_MAX];
 	/* Meter drop table. */
 	struct mlx5_flow_dv_matcher *
@@ -1375,6 +1373,7 @@ struct mlx5_priv {
 	uint8_t mtr_color_reg; /* Meter color match REG_C. */
 	struct mlx5_legacy_flow_meters flow_meters; /* MTR list. */
 	struct mlx5_l3t_tbl *mtr_profile_tbl; /* Meter index lookup table. */
+	struct mlx5_l3t_tbl *policy_idx_tbl; /* Policy index lookup table. */
 	struct mlx5_l3t_tbl *mtr_idx_tbl; /* Meter index lookup table. */
 	uint8_t skip_default_rss_reta; /* Skip configuration of default reta. */
 	uint8_t fdb_def_rule; /* Whether fdb jump to table 1 is configured. */
diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c
index 87c5782956..6f962a8d52 100644
--- a/drivers/net/mlx5/mlx5_flow_meter.c
+++ b/drivers/net/mlx5/mlx5_flow_meter.c
@@ -512,11 +512,9 @@ mlx5_flow_meter_policy_find(struct rte_eth_dev *dev,
 	struct mlx5_flow_meter_sub_policy *sub_policy = NULL;
 	union mlx5_l3t_data data;
 
-	if (policy_id > MLX5_MAX_SUB_POLICY_TBL_NUM ||
-		!priv->sh->mtrmng->policy_idx_tbl)
+	if (policy_id > MLX5_MAX_SUB_POLICY_TBL_NUM || !priv->policy_idx_tbl)
 		return NULL;
-	if (mlx5_l3t_get_entry(priv->sh->mtrmng->policy_idx_tbl,
-				policy_id, &data) ||
+	if (mlx5_l3t_get_entry(priv->policy_idx_tbl, policy_id, &data) ||
 				!data.dword)
 		return NULL;
 	if (policy_idx)
@@ -524,8 +522,7 @@ mlx5_flow_meter_policy_find(struct rte_eth_dev *dev,
 	sub_policy = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MTR_POLICY],
 					data.dword);
 	/* Remove reference taken by the mlx5_l3t_get_entry. */
-	mlx5_l3t_clear_entry(priv->sh->mtrmng->policy_idx_tbl,
-				policy_id);
+	mlx5_l3t_clear_entry(priv->policy_idx_tbl, policy_id);
 	if (sub_policy)
 		if (sub_policy->main_policy_id)
 			return sub_policy->main_policy;
@@ -605,9 +602,8 @@ __mlx5_flow_meter_policy_delete(struct rte_eth_dev *dev,
 			}
 		}
 	}
-	if (priv->sh->mtrmng->policy_idx_tbl && clear_l3t) {
-		if (mlx5_l3t_clear_entry(priv->sh->mtrmng->policy_idx_tbl,
-					policy_id)) {
+	if (priv->policy_idx_tbl && clear_l3t) {
+		if (mlx5_l3t_clear_entry(priv->policy_idx_tbl, policy_id)) {
 			rte_spinlock_unlock(&mtr_policy->sl);
 			return -rte_mtr_error_set(error, ENOTSUP,
 				RTE_MTR_ERROR_TYPE_METER_POLICY_ID, NULL,
@@ -770,14 +766,12 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev,
 			goto policy_add_err;
 	}
 	data.dword = policy_idx;
-	if (!priv->sh->mtrmng->policy_idx_tbl) {
-		priv->sh->mtrmng->policy_idx_tbl =
-				mlx5_l3t_create(MLX5_L3T_TYPE_DWORD);
-		if (!priv->sh->mtrmng->policy_idx_tbl)
+	if (!priv->policy_idx_tbl) {
+		priv->policy_idx_tbl = mlx5_l3t_create(MLX5_L3T_TYPE_DWORD);
+		if (!priv->policy_idx_tbl)
 			goto policy_add_err;
 	}
-	if (mlx5_l3t_set_entry(priv->sh->mtrmng->policy_idx_tbl,
-				policy_id, &data))
+	if (mlx5_l3t_set_entry(priv->policy_idx_tbl, policy_id, &data))
 		goto policy_add_err;
 	return 0;
 policy_add_err:
@@ -1840,9 +1834,8 @@ mlx5_flow_meter_rxq_flush(struct rte_eth_dev *dev)
 
 	if (!priv->mtr_en)
 		return;
-	if (priv->sh->mtrmng->policy_idx_tbl && priv->sh->refcnt == 1) {
-		MLX5_L3T_FOREACH(priv->sh->mtrmng->policy_idx_tbl,
-					i, entry) {
+	if (priv->policy_idx_tbl) {
+		MLX5_L3T_FOREACH(priv->policy_idx_tbl, i, entry) {
 			policy_idx = *(uint32_t *)entry;
 			sub_policy = mlx5_ipool_get
 				(priv->sh->ipool[MLX5_IPOOL_MTR_POLICY],
@@ -1908,9 +1901,8 @@ mlx5_flow_meter_flush(struct rte_eth_dev *dev, struct rte_mtr_error *error)
 				NULL, "MTR object meter profile invalid.");
 		}
 	}
-	if (priv->sh->mtrmng->policy_idx_tbl && priv->sh->refcnt == 1) {
-		MLX5_L3T_FOREACH(priv->sh->mtrmng->policy_idx_tbl,
-					i, entry) {
+	if (priv->policy_idx_tbl) {
+		MLX5_L3T_FOREACH(priv->policy_idx_tbl, i, entry) {
 			policy_idx = *(uint32_t *)entry;
 			sub_policy = mlx5_ipool_get
 				(priv->sh->ipool[MLX5_IPOOL_MTR_POLICY],
@@ -1931,8 +1923,8 @@ mlx5_flow_meter_flush(struct rte_eth_dev *dev, struct rte_mtr_error *error)
 						"meter policy invalid.");
 			mlx5_free(sub_policy->main_policy);
 		}
-		mlx5_l3t_destroy(priv->sh->mtrmng->policy_idx_tbl);
-		priv->sh->mtrmng->policy_idx_tbl = NULL;
+		mlx5_l3t_destroy(priv->policy_idx_tbl);
+		priv->policy_idx_tbl = NULL;
 	}
 	if (priv->mtr_profile_tbl) {
 		MLX5_L3T_FOREACH(priv->mtr_profile_tbl, i, entry) {
-- 
2.20.0

[dpdk-dev] [PATCH v1 3/4] net/mlx5: fix meter flow direction check

[Shun Hao]

When preparing prefix flow using ASO meter, if it's tx flow, need
to make meter action the first one.

Currently the check of flow direction in switch domain is incorrect
that it checks the flow dev port only.

This adds the fix for the check that if there's port_id match item
in flow, use that port_id as src port to determine flow direction.

Fixes: c99b4f8bc2f1 ("net/mlx5: support ASO meter action")
Cc: stable@dpdk.org

Signed-off-by: Shun Hao <shunh@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.c | 83 ++++++++++++++++++++++--------------
 1 file changed, 50 insertions(+), 33 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 3b7c94d92f..c27f6197a0 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -4740,12 +4740,12 @@ flow_meter_split_prep(struct rte_eth_dev *dev,
 	struct mlx5_rte_flow_item_tag *tag_item_spec;
 	struct mlx5_rte_flow_item_tag *tag_item_mask;
 	uint32_t tag_id = 0;
-	bool copy_vlan = false;
+	struct rte_flow_item *vlan_item_dst = NULL;
+	const struct rte_flow_item *vlan_item_src = NULL;
 	struct rte_flow_action *hw_mtr_action;
 	struct rte_flow_action *action_pre_head = NULL;
-	bool mtr_first = priv->sh->meter_aso_en &&
-			(attr->egress ||
-			(attr->transfer && priv->representor_id != UINT16_MAX));
+	int32_t flow_src_port = priv->representor_id;
+	bool mtr_first;
 	uint8_t mtr_id_offset = priv->mtr_reg_share ? MLX5_MTR_COLOR_BITS : 0;
 	uint8_t mtr_reg_bits = priv->mtr_reg_share ?
 				MLX5_MTR_IDLE_BITS_IN_COLOR_REG : MLX5_REG_BITS;
@@ -4754,6 +4754,42 @@ flow_meter_split_prep(struct rte_eth_dev *dev,
 	uint8_t flow_id_bits = 0;
 	int shift;
 
+	/* Prepare the suffix subflow items. */
+	tag_item = sfx_items++;
+	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
+		struct mlx5_priv *port_priv;
+		const struct rte_flow_item_port_id *pid_v;
+		int item_type = items->type;
+
+		switch (item_type) {
+		case RTE_FLOW_ITEM_TYPE_PORT_ID:
+			pid_v = items->spec;
+			MLX5_ASSERT(pid_v);
+			port_priv = mlx5_port_to_eswitch_info(pid_v->id, false);
+			if (!port_priv)
+				return rte_flow_error_set(error,
+						rte_errno,
+						RTE_FLOW_ERROR_TYPE_ITEM_SPEC,
+						pid_v,
+						"Failed to get port info.");
+			flow_src_port = port_priv->representor_id;
+			memcpy(sfx_items, items, sizeof(*sfx_items));
+			sfx_items++;
+			break;
+		case RTE_FLOW_ITEM_TYPE_VLAN:
+			/* Determine if copy vlan item below. */
+			vlan_item_src = items;
+			vlan_item_dst = sfx_items++;
+			vlan_item_dst->type = RTE_FLOW_ITEM_TYPE_VOID;
+			break;
+		default:
+			break;
+		}
+	}
+	sfx_items->type = RTE_FLOW_ITEM_TYPE_END;
+	sfx_items++;
+	mtr_first = priv->sh->meter_aso_en &&
+		(attr->egress || (attr->transfer && flow_src_port != 0xffff));
 	/* For ASO meter, meter must be before tag in TX direction. */
 	if (mtr_first) {
 		action_pre_head = actions_pre++;
@@ -4790,7 +4826,16 @@ flow_meter_split_prep(struct rte_eth_dev *dev,
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
-			copy_vlan = true;
+			if (vlan_item_dst && vlan_item_src) {
+				memcpy(vlan_item_dst, vlan_item_src,
+					sizeof(*vlan_item_dst));
+				/*
+				 * Convert to internal match item, it is used
+				 * for vlan push and set vid.
+				 */
+				vlan_item_dst->type = (enum rte_flow_item_type)
+						MLX5_RTE_FLOW_ITEM_TYPE_VLAN;
+			}
 			break;
 		default:
 			break;
@@ -4862,34 +4907,6 @@ flow_meter_split_prep(struct rte_eth_dev *dev,
 		if (flow_id_bits > priv->sh->mtrmng->max_mtr_flow_bits)
 			priv->sh->mtrmng->max_mtr_flow_bits = flow_id_bits;
 	}
-	/* Prepare the suffix subflow items. */
-	tag_item = sfx_items++;
-	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
-		int item_type = items->type;
-
-		switch (item_type) {
-		case RTE_FLOW_ITEM_TYPE_PORT_ID:
-			memcpy(sfx_items, items, sizeof(*sfx_items));
-			sfx_items++;
-			break;
-		case RTE_FLOW_ITEM_TYPE_VLAN:
-			if (copy_vlan) {
-				memcpy(sfx_items, items, sizeof(*sfx_items));
-				/*
-				 * Convert to internal match item, it is used
-				 * for vlan push and set vid.
-				 */
-				sfx_items->type = (enum rte_flow_item_type)
-						  MLX5_RTE_FLOW_ITEM_TYPE_VLAN;
-				sfx_items++;
-			}
-			break;
-		default:
-			break;
-		}
-	}
-	sfx_items->type = RTE_FLOW_ITEM_TYPE_END;
-	sfx_items++;
 	/* Build tag actions and items for meter_id/meter flow_id. */
 	set_tag = (struct mlx5_rte_flow_action_set_tag *)actions_pre;
 	tag_item_spec = (struct mlx5_rte_flow_item_tag *)sfx_items;
-- 
2.20.0

[dpdk-dev] [PATCH v1 4/4] net/mlx5: fix meter policy flow match item

[Shun Hao]

Currently when creating meter policy, a src port_id match item will
always be added in switch domain. So if one meter is used by another
port, it will not work correctly.

This issue is solved:
1. If policy fate action is port_id, add the src port_id match item,
and the meter cannot be shared by another port.
2. If policy fate action isn't port_id, don't add the src port_id
match, meter can be shared by another port.

This fix enables one meter being shared by different ports. User can
create a meter flow using a port_id match item to make this meter
shared by other port.

Fixes: afb4aa4f122 ("net/mlx5: support meter policy operations")
Cc: stable@dpdk.org

Signed-off-by: Shun Hao <shunh@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  8 +++++
 drivers/net/mlx5/mlx5.h         |  2 ++
 drivers/net/mlx5/mlx5_flow_dv.c | 58 ++++++++++++++++++++++++++++-----
 3 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index eb44a070b1..57e7d17b87 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1912,3 +1912,11 @@ all flows with assistance of external tools.
    .. code-block:: console
 
        mlx_steering_dump.py -f <output_file> -flowptr <flow_ptr>
+
+How to share a meter between ports in the same switch domain
+------------------------------------------------------------
+
+This section demonstrates how to use the shared meter. A meter M can be created
+on port X and to be shared with a port Y on the same switch domain by the next way:
+
+        flow create X ingress transfer pattern eth / port_id id is Y / end actions meter mtr_id M / end
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 6eae7b6fd7..0f4b239142 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -682,6 +682,8 @@ struct mlx5_meter_policy_action_container {
 
 /* Flow meter policy parameter structure. */
 struct mlx5_flow_meter_policy {
+	struct rte_eth_dev *dev;
+	/* The port dev on which policy is created. */
 	uint32_t is_rss:1;
 	/* Is RSS policy table. */
 	uint32_t ingress:1;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index a04a3c2bb8..75ef6216ac 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5031,6 +5031,8 @@ flow_dv_modify_hdr_action_max(struct rte_eth_dev *dev __rte_unused,
  *   Pointer to the meter action.
  * @param[in] attr
  *   Attributes of flow that includes this action.
+ * @param[in] port_id_item
+ *   Pointer to item indicating port id.
  * @param[out] error
  *   Pointer to error structure.
  *
@@ -5042,6 +5044,7 @@ mlx5_flow_validate_action_meter(struct rte_eth_dev *dev,
 				uint64_t action_flags,
 				const struct rte_flow_action *action,
 				const struct rte_flow_attr *attr,
+				const struct rte_flow_item *port_id_item,
 				bool *def_policy,
 				struct rte_flow_error *error)
 {
@@ -5112,6 +5115,37 @@ mlx5_flow_validate_action_meter(struct rte_eth_dev *dev,
 					  "Flow attributes domain "
 					  "have a conflict with current "
 					  "meter domain attributes");
+		if (attr->transfer && mtr_policy->dev) {
+			/**
+			 * When policy has fate action of port_id,
+			 * the flow should have the same src port as policy.
+			 */
+			struct mlx5_priv *policy_port_priv =
+					mtr_policy->dev->data->dev_private;
+			int32_t flow_src_port = priv->representor_id;
+
+			if (port_id_item) {
+				const struct rte_flow_item_port_id *spec =
+							port_id_item->spec;
+				struct mlx5_priv *port_priv =
+					mlx5_port_to_eswitch_info(spec->id,
+								  false);
+				if (!port_priv)
+					return rte_flow_error_set(error,
+						rte_errno,
+						RTE_FLOW_ERROR_TYPE_ITEM_SPEC,
+						spec,
+						"Failed to get port info.");
+				flow_src_port = port_priv->representor_id;
+			}
+			if (flow_src_port != policy_port_priv->representor_id)
+				return rte_flow_error_set(error,
+						rte_errno,
+						RTE_FLOW_ERROR_TYPE_ITEM_SPEC,
+						NULL,
+						"Flow and meter policy "
+						"have different src port.");
+		}
 		*def_policy = false;
 	}
 	return 0;
@@ -6685,6 +6719,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 	};
 	const struct rte_eth_hairpin_conf *conf;
 	const struct rte_flow_item *rule_items = items;
+	const struct rte_flow_item *port_id_item = NULL;
 	bool def_policy = false;
 
 	if (items == NULL)
@@ -6726,6 +6761,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 			if (ret < 0)
 				return ret;
 			last_item = MLX5_FLOW_ITEM_PORT_ID;
+			port_id_item = items;
 			break;
 		case RTE_FLOW_ITEM_TYPE_ETH:
 			ret = mlx5_flow_validate_item_eth(items, item_flags,
@@ -7463,6 +7499,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 			ret = mlx5_flow_validate_action_meter(dev,
 							      action_flags,
 							      actions, attr,
+							      port_id_item,
 							      &def_policy,
 							      error);
 			if (ret < 0)
@@ -15348,7 +15385,7 @@ __flow_dv_create_policy_flow(struct rte_eth_dev *dev,
 			uint32_t color_reg_c_idx,
 			enum rte_color color, void *matcher_object,
 			int actions_n, void *actions,
-			bool is_default_policy, void **rule,
+			bool match_src_port, void **rule,
 			const struct rte_flow_attr *attr)
 {
 	int ret;
@@ -15362,7 +15399,7 @@ __flow_dv_create_policy_flow(struct rte_eth_dev *dev,
 	};
 	struct mlx5_priv *priv = dev->data->dev_private;
 
-	if (!is_default_policy && (priv->representor || priv->master)) {
+	if (match_src_port && (priv->representor || priv->master)) {
 		if (flow_dv_translate_item_port_id(dev, matcher.buf,
 						   value.buf, NULL, attr)) {
 			DRV_LOG(ERR,
@@ -15389,7 +15426,7 @@ __flow_dv_create_policy_matcher(struct rte_eth_dev *dev,
 			uint16_t priority,
 			struct mlx5_flow_meter_sub_policy *sub_policy,
 			const struct rte_flow_attr *attr,
-			bool is_default_policy,
+			bool match_src_port,
 			struct rte_flow_error *error)
 {
 	struct mlx5_cache_entry *entry;
@@ -15413,7 +15450,7 @@ __flow_dv_create_policy_matcher(struct rte_eth_dev *dev,
 	struct mlx5_priv *priv = dev->data->dev_private;
 	uint32_t color_mask = (UINT32_C(1) << MLX5_MTR_COLOR_BITS) - 1;
 
-	if (!is_default_policy && (priv->representor || priv->master)) {
+	if (match_src_port && (priv->representor || priv->master)) {
 		if (flow_dv_translate_item_port_id(dev, matcher.mask.buf,
 						   value.buf, NULL, attr)) {
 			DRV_LOG(ERR,
@@ -15458,7 +15495,7 @@ __flow_dv_create_policy_matcher(struct rte_eth_dev *dev,
 static int
 __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev,
 		struct mlx5_flow_meter_sub_policy *sub_policy,
-		uint8_t egress, uint8_t transfer, bool is_default_policy,
+		uint8_t egress, uint8_t transfer, bool match_src_port,
 		struct mlx5_meter_policy_acts acts[RTE_COLORS])
 {
 	struct rte_flow_error flow_err;
@@ -15497,7 +15534,7 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev,
 			/* Create matchers for Color. */
 			if (__flow_dv_create_policy_matcher(dev,
 				color_reg_c_idx, i, sub_policy,
-				&attr, is_default_policy, &flow_err))
+				&attr, match_src_port, &flow_err))
 				return -1;
 		}
 		/* Create flow, matching color. */
@@ -15507,7 +15544,7 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev,
 				sub_policy->color_matcher[i]->matcher_object,
 				acts[i].actions_n,
 				acts[i].dv_actions,
-				is_default_policy,
+				match_src_port,
 				&sub_policy->color_rule[i],
 				&attr))
 				return -1;
@@ -15527,6 +15564,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev,
 	struct mlx5_flow_dv_port_id_action_resource *port_action;
 	struct mlx5_hrxq *hrxq;
 	uint8_t egress, transfer;
+	bool match_src_port = false;
 	int i;
 
 	for (i = 0; i < RTE_COLORS; i++) {
@@ -15571,6 +15609,8 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev,
 				acts[i].dv_actions[acts[i].actions_n] =
 				port_action->action;
 				acts[i].actions_n++;
+				mtr_policy->dev = dev;
+				match_src_port = true;
 				break;
 			case MLX5_FLOW_FATE_DROP:
 			case MLX5_FLOW_FATE_JUMP:
@@ -15601,7 +15641,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev,
 	egress = (domain == MLX5_MTR_DOMAIN_EGRESS) ? 1 : 0;
 	transfer = (domain == MLX5_MTR_DOMAIN_TRANSFER) ? 1 : 0;
 	if (__flow_dv_create_domain_policy_rules(dev, sub_policy,
-				egress, transfer, false, acts)) {
+				egress, transfer, match_src_port, acts)) {
 		DRV_LOG(ERR,
 		"Failed to create policy rules per domain.");
 		return -1;
@@ -15709,7 +15749,7 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain)
 		/* Create default policy rules. */
 		ret = __flow_dv_create_domain_policy_rules(dev,
 					&def_policy->sub_policy,
-					egress, transfer, true, acts);
+					egress, transfer, false, acts);
 		if (ret) {
 			DRV_LOG(ERR, "Failed to create "
 				"default policy rules.");
-- 
2.20.0

Re: [dpdk-dev] [PATCH v1 0/4] ASO meter sharing support

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: Shun Hao <shunh@nvidia.com>
> Sent: Friday, July 2, 2021 12:15 PM
> To: Ori Kam <orika@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf
> Shuler <shahafs@nvidia.com>
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Raslan Darawsheh <rasland@nvidia.com>; Roni
> Bar Yanai <roniba@nvidia.com>
> Subject: [PATCH v1 0/4] ASO meter sharing support
> 
> This adds several fixes in ASO meter. With these fixes, meter created on one
> port can be shared by other ports.
> 
> Shun Hao (4):
>   net/mlx5: use meter profile lookup table
>   net/mlx5: fix meter policy ID table container
>   net/mlx5: fix meter flow direction check
>   net/mlx5: fix meter policy flow match item
> 
>  doc/guides/nics/mlx5.rst           |  8 +++
>  drivers/net/mlx5/linux/mlx5_os.c   |  6 +-
>  drivers/net/mlx5/mlx5.c            |  2 -
>  drivers/net/mlx5/mlx5.h            |  7 ++-
>  drivers/net/mlx5/mlx5_flow.c       | 83 ++++++++++++++++-----------
>  drivers/net/mlx5/mlx5_flow_dv.c    | 58 ++++++++++++++++---
>  drivers/net/mlx5/mlx5_flow_meter.c | 91 +++++++++++++++++-------------
> drivers/net/mlx5/windows/mlx5_os.c |  6 +-
>  8 files changed, 172 insertions(+), 89 deletions(-)
> 
> --
> 2.20.0

Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh