DPDK patches and discussions
 help / color / mirror / Atom feed
From: Shun Hao <shunh@nvidia.com>
To: <viacheslavo@nvidia.com>, <matan@nvidia.com>, <orika@nvidia.com>
Cc: <dev@dpdk.org>, <rasland@nvidia.com>, <stable@dpdk.org>
Subject: [PATCH v1 1/2] net/mlx5: fix meter hierarchy with modify header
Date: Tue, 8 Nov 2022 12:04:00 +0200	[thread overview]
Message-ID: <20221108100401.878296-2-shunh@nvidia.com> (raw)
In-Reply-To: <20221108100401.878296-1-shunh@nvidia.com>

If any meter in the hierarchy has a policy flow containing set_tag or
modify_field action, the policy flow must match the src port to which
the policy belongs, to determine the order of modify_hdr and
meter action. But the meter hierarchy will not be able to use by
user flow that matches another src port.

To use this type of meter hierarchy for other src ports, we need to add
a new policy flow matching the new src port from the user flow
dynamically. But then it cannot be used by flow matching all ports.

Fixes: ca7e6051 ("net/mlx5: limit meter flow when matching all ports")
Cc: stable@dpdk.org

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

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index d5f9375a4e..4f0db21dde 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -511,6 +511,9 @@ Limitations
     if meter has drop count
     or meter hierarchy contains any meter that uses drop count,
     it cannot be used by flow rule matching all ports.
+  - When using DV flow engine (``dv_flow_en`` = 1),
+    if meter hierarchy contains any meter that has MODIFY_FIELD/SET_TAG,
+    it cannot be used by flow matching all ports.
   - When using HWS flow engine (``dv_flow_en`` = 2),
     only meter mark action is supported.
 
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index cbe2d88b9e..73d0329500 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -846,8 +846,10 @@ struct mlx5_flow_meter_policy {
 	/* Is queue action in policy table. */
 	uint32_t is_hierarchy:1;
 	/* Is meter action in policy table. */
-	uint32_t hierarchy_drop_cnt:1;
-	/* Is any meter in hierarchy contains drop_cnt. */
+	uint32_t match_port:1;
+	/* If policy flows match src port. */
+	uint32_t hierarchy_match_port:1;
+	/* Is any meter in hierarchy contains policy flow that matches src port. */
 	uint32_t skip_r:1;
 	/* If red color policy is skipped. */
 	uint32_t skip_y:1;
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index ea88882b88..de75c05602 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -5471,7 +5471,7 @@ flow_meter_split_prep(struct rte_eth_dev *dev,
 		case RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT:
 			if (mlx5_flow_get_item_vport_id(dev, items, &flow_src_port, NULL, error))
 				return -rte_errno;
-			if (!fm->def_policy && wks->policy->is_hierarchy &&
+			if (!fm->def_policy && wks->policy->hierarchy_match_port &&
 			    flow_src_port != priv->representor_id) {
 				if (flow_drv_mtr_hierarchy_rule_create(dev,
 								flow, fm,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index acd7ea8b79..bb898d7569 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5445,11 +5445,12 @@ mlx5_flow_validate_action_meter(struct rte_eth_dev *dev,
 							"have different src port.");
 			}
 			/* When flow matching all src ports, meter should not have drop count. */
-			if (all_ports && (fm->drop_cnt || mtr_policy->hierarchy_drop_cnt))
+			if (all_ports && (fm->drop_cnt || mtr_policy->hierarchy_match_port))
 				return rte_flow_error_set(error, EINVAL,
 							  RTE_FLOW_ERROR_TYPE_ITEM_SPEC, NULL,
-							  "Meter drop count not supported "
-							  "when matching all ports.");
+							  "Meter drop count or "
+							  "modify_field/set_tag in meter hierarchy "
+							  "not supported when matching all ports.");
 		} else if (mtr_policy->is_rss) {
 			struct mlx5_flow_meter_policy *fp;
 			struct mlx5_meter_policy_action_container *acg;
@@ -16661,8 +16662,8 @@ __flow_dv_create_domain_policy_acts(struct rte_eth_dev *dev,
 				mtr_policy->dev = next_policy->dev;
 				if (next_policy->mark)
 					mtr_policy->mark = 1;
-				if (next_fm->drop_cnt || next_policy->hierarchy_drop_cnt)
-					mtr_policy->hierarchy_drop_cnt = 1;
+				mtr_policy->hierarchy_match_port =
+							next_policy->hierarchy_match_port;
 				action_flags |=
 				MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY;
 				break;
@@ -17408,6 +17409,10 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev,
 			"Failed to create policy rules per domain.");
 		goto err_exit;
 	}
+	if (match_src_port) {
+		mtr_policy->match_port = match_src_port;
+		mtr_policy->hierarchy_match_port = match_src_port;
+	}
 	return 0;
 err_exit:
 	for (i = 0; i < RTE_COLORS; i++)
@@ -18030,7 +18035,7 @@ mlx5_meter_hierarchy_skip_tag_rule(struct mlx5_priv *priv,
 					 NULL, "Failed to find next meter in hierarchy.");
 		goto exit;
 	}
-	if (!(*next_fm)->drop_cnt) {
+	if (!mtr_policy->match_port) {
 		*skip = true;
 		goto exit;
 	}
@@ -18130,6 +18135,9 @@ flow_dv_meter_hierarchy_rule_create(struct rte_eth_dev *dev,
 		rte_spinlock_lock(&mtr_policy->sl);
 		sub_policy = mtr_policy->sub_policys[domain][0];
 		for (j = 0; j < MLX5_MTR_RTE_COLORS; j++) {
+			uint8_t act_n = 0;
+			struct mlx5_flow_dv_modify_hdr_resource *modify_hdr;
+
 			if (mtr_policy->act_cnt[j].fate_action != MLX5_FLOW_FATE_MTR)
 				continue;
 			color_rule = mlx5_malloc(MLX5_MEM_ZERO,
@@ -18160,15 +18168,18 @@ flow_dv_meter_hierarchy_rule_create(struct rte_eth_dev *dev,
 			next_sub_policy = next_policy->sub_policys[domain][0];
 			tbl_data = container_of(next_sub_policy->tbl_rsc,
 						struct mlx5_flow_tbl_data_entry, tbl);
+			modify_hdr = mtr_policy->act_cnt[j].modify_hdr;
 			if (mtr_first) {
-				acts.dv_actions[0] = mtr_action;
-				acts.dv_actions[1] = mtr_policy->act_cnt[j].modify_hdr->action;
+				acts.dv_actions[act_n++] = mtr_action;
+				if (modify_hdr)
+					acts.dv_actions[act_n++] = modify_hdr->action;
 			} else {
-				acts.dv_actions[0] = mtr_policy->act_cnt[j].modify_hdr->action;
-				acts.dv_actions[1] = mtr_action;
+				if (modify_hdr)
+					acts.dv_actions[act_n++] = modify_hdr->action;
+				acts.dv_actions[act_n++] = mtr_action;
 			}
-			acts.dv_actions[2] = tbl_data->jump.action;
-			acts.actions_n = 3;
+			acts.dv_actions[act_n++] = tbl_data->jump.action;
+			acts.actions_n = act_n;
 			if (__flow_dv_create_policy_matcher(dev, color_reg_c_idx,
 						MLX5_MTR_POLICY_MATCHER_PRIO, sub_policy,
 						&attr, true, item, &color_rule->matcher, error)) {
-- 
2.20.0


  reply	other threads:[~2022-11-08 10:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-08 10:03 [PATCH v1 0/2] Fix src port match in meter hierarchy Shun Hao
2022-11-08 10:04 ` Shun Hao [this message]
2022-11-08 10:04 ` [PATCH v1 2/2] net/mlx5: fix meter policy with port ID destination Shun Hao
2022-11-08 17:35 ` [PATCH v1 0/2] Fix src port match in meter hierarchy Raslan Darawsheh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221108100401.878296-2-shunh@nvidia.com \
    --to=shunh@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=matan@nvidia.com \
    --cc=orika@nvidia.com \
    --cc=rasland@nvidia.com \
    --cc=stable@dpdk.org \
    --cc=viacheslavo@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).