DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bing Zhao <bingz@nvidia.com>
To: <viacheslavo@nvidia.com>, <matan@nvidia.com>
Cc: <dev@dpdk.org>, <orika@nvidia.com>, <rasland@nvidia.com>
Subject: [dpdk-dev] [PATCH 5/6] net/mlx5: split policies handling of colors
Date: Mon, 5 Jul 2021 18:57:55 +0300	[thread overview]
Message-ID: <20210705155756.21443-6-bingz@nvidia.com> (raw)
In-Reply-To: <20210705155756.21443-1-bingz@nvidia.com>

If the fate action is either RSS or Queue of a meter policy, the
action will only be created in the flow splitting stage. With queue
as the fate action, only one sub-policy is needed. And RSS will
have more than one sub-policies if there is an expansion.

Since the RSS parameters are the same for both green and yellow
colors except the queues, the expansion result will be unique.
Even if only one color has the RSS action, the checking and possible
expansion will be done then. For each sub-policy, the action rules
need to be created separately on its own policy table.

Signed-off-by: Bing Zhao <bingz@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 37 +++++++++++---------
 drivers/net/mlx5/mlx5_flow_dv.c | 61 +++++++++++++++++----------------
 2 files changed, 51 insertions(+), 47 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 61c72da2aa..89056ec45e 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -4604,7 +4604,7 @@ get_meter_sub_policy(struct rte_eth_dev *dev,
 		uint32_t i;
 
 		MLX5_ASSERT(wks);
-		/**
+		/*
 		 * This is a tmp dev_flow,
 		 * no need to register any matcher for it in translate.
 		 */
@@ -4612,18 +4612,19 @@ get_meter_sub_policy(struct rte_eth_dev *dev,
 		for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) {
 			struct mlx5_flow dev_flow = {0};
 			struct mlx5_flow_handle dev_handle = { {0} };
+			uint8_t fate = policy->act_cnt[i].fate_action;
 
-			if (policy->is_rss) {
+			if (fate == MLX5_FLOW_FATE_SHARED_RSS) {
 				const void *rss_act =
 					policy->act_cnt[i].rss->conf;
 				struct rte_flow_action rss_actions[2] = {
 					[0] = {
 					.type = RTE_FLOW_ACTION_TYPE_RSS,
-					.conf = rss_act
+					.conf = rss_act,
 					},
 					[1] = {
 					.type = RTE_FLOW_ACTION_TYPE_END,
-					.conf = NULL
+					.conf = NULL,
 					}
 				};
 
@@ -4650,7 +4651,8 @@ get_meter_sub_policy(struct rte_eth_dev *dev,
 				rss_desc_v[i].tunnel =
 					!!(dev_flow.handle->layers &
 					MLX5_FLOW_LAYER_TUNNEL);
-			} else {
+				rss_desc[i] = &rss_desc_v[i];
+			} else if (fate == MLX5_FLOW_FATE_QUEUE) {
 				/* This is queue action. */
 				rss_desc_v[i] = wks->rss_desc;
 				rss_desc_v[i].key_len = 0;
@@ -4658,24 +4660,24 @@ get_meter_sub_policy(struct rte_eth_dev *dev,
 				rss_desc_v[i].queue =
 					&policy->act_cnt[i].queue;
 				rss_desc_v[i].queue_num = 1;
+				rss_desc[i] = &rss_desc_v[i];
+			} else {
+				rss_desc[i] = NULL;
 			}
-			rss_desc[i] = &rss_desc_v[i];
 		}
 		sub_policy = flow_drv_meter_sub_policy_rss_prepare(dev,
 						flow, policy, rss_desc);
 	} else {
 		enum mlx5_meter_domain mtr_domain =
 			attr->transfer ? MLX5_MTR_DOMAIN_TRANSFER :
-				attr->egress ? MLX5_MTR_DOMAIN_EGRESS :
-					MLX5_MTR_DOMAIN_INGRESS;
+				(attr->egress ? MLX5_MTR_DOMAIN_EGRESS :
+						MLX5_MTR_DOMAIN_INGRESS);
 		sub_policy = policy->sub_policys[mtr_domain][0];
 	}
-	if (!sub_policy) {
+	if (!sub_policy)
 		rte_flow_error_set(error, EINVAL,
-			RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
-			"Failed to get meter sub-policy.");
-		goto exit;
-	}
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+				   "Failed to get meter sub-policy.");
 exit:
 	return sub_policy;
 }
@@ -4810,6 +4812,7 @@ flow_meter_split_prep(struct rte_eth_dev *dev,
 		struct mlx5_flow_meter_sub_policy *sub_policy;
 		struct mlx5_flow_tbl_data_entry *tbl_data;
 
+		/* Either G or Y can still use default policy. */
 		if (!fm->def_policy) {
 			sub_policy = get_meter_sub_policy(dev, flow,
 							  fm->policy_id, attr,
@@ -4819,8 +4822,8 @@ flow_meter_split_prep(struct rte_eth_dev *dev,
 		} else {
 			enum mlx5_meter_domain mtr_domain =
 			attr->transfer ? MLX5_MTR_DOMAIN_TRANSFER :
-				attr->egress ? MLX5_MTR_DOMAIN_EGRESS :
-					MLX5_MTR_DOMAIN_INGRESS;
+				(attr->egress ? MLX5_MTR_DOMAIN_EGRESS :
+						MLX5_MTR_DOMAIN_INGRESS);
 
 			sub_policy =
 			&priv->sh->mtrmng->def_policy[mtr_domain]->sub_policy;
@@ -4836,8 +4839,8 @@ flow_meter_split_prep(struct rte_eth_dev *dev,
 	actions_pre++;
 	if (!tag_action)
 		return rte_flow_error_set(error, ENOMEM,
-					RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
-					"No tag action space.");
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL, "No tag action space.");
 	if (!mtr_flow_id) {
 		tag_action->type = RTE_FLOW_ACTION_TYPE_VOID;
 		goto exit;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index c606fa9471..0f62537938 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -14550,7 +14550,7 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev,
 		}
 		if (sub_policy->color_matcher[i]) {
 			tbl = container_of(sub_policy->color_matcher[i]->tbl,
-				typeof(*tbl), tbl);
+					   typeof(*tbl), tbl);
 			mlx5_cache_unregister(&tbl->matchers,
 				      &sub_policy->color_matcher[i]->entry);
 			sub_policy->color_matcher[i] = NULL;
@@ -14563,13 +14563,13 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev,
 		}
 		if (sub_policy->jump_tbl[i]) {
 			flow_dv_tbl_resource_release(MLX5_SH(dev),
-			sub_policy->jump_tbl[i]);
+						     sub_policy->jump_tbl[i]);
 			sub_policy->jump_tbl[i] = NULL;
 		}
 	}
 	if (sub_policy->tbl_rsc) {
 		flow_dv_tbl_resource_release(MLX5_SH(dev),
-			sub_policy->tbl_rsc);
+					     sub_policy->tbl_rsc);
 		sub_policy->tbl_rsc = NULL;
 	}
 }
@@ -14586,7 +14586,7 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev,
  */
 static void
 flow_dv_destroy_policy_rules(struct rte_eth_dev *dev,
-		      struct mlx5_flow_meter_policy *mtr_policy)
+			     struct mlx5_flow_meter_policy *mtr_policy)
 {
 	uint32_t i, j;
 	struct mlx5_flow_meter_sub_policy *sub_policy;
@@ -14599,8 +14599,8 @@ flow_dv_destroy_policy_rules(struct rte_eth_dev *dev,
 		for (j = 0; j < sub_policy_num; j++) {
 			sub_policy = mtr_policy->sub_policys[i][j];
 			if (sub_policy)
-				__flow_dv_destroy_sub_policy_rules
-						(dev, sub_policy);
+				__flow_dv_destroy_sub_policy_rules(dev,
+								   sub_policy);
 		}
 	}
 }
@@ -15550,6 +15550,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev,
 	uint8_t egress, transfer;
 	int i;
 
+	/* If RSS or Queue, no previous actions / rules is created. */
 	for (i = 0; i < RTE_COLORS; i++) {
 		acts[i].actions_n = 0;
 		if (i == RTE_COLOR_RED) {
@@ -15994,36 +15995,35 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev,
 	sub_policy_num = (mtr_policy->sub_policy_num >>
 			(MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) &
 			MLX5_MTR_SUB_POLICY_NUM_MASK;
-	for (i = 0; i < sub_policy_num;
-		i++) {
-		for (j = 0; j < MLX5_MTR_RTE_COLORS; j++) {
-			if (rss_desc[j] &&
-				hrxq_idx[j] !=
-			mtr_policy->sub_policys[domain][i]->rix_hrxq[j])
+	for (j = 0; j < sub_policy_num; j++) {
+		for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) {
+			if (rss_desc[i] &&
+			    hrxq_idx[i] !=
+			    mtr_policy->sub_policys[domain][j]->rix_hrxq[i])
 				break;
 		}
-		if (j >= MLX5_MTR_RTE_COLORS) {
+		if (i >= MLX5_MTR_RTE_COLORS) {
 			/*
 			 * Found the sub policy table with
-			 * the same queue per color
+			 * the same queue per color.
 			 */
 			rte_spinlock_unlock(&mtr_policy->sl);
-			for (j = 0; j < MLX5_MTR_RTE_COLORS; j++)
-				mlx5_hrxq_release(dev, hrxq_idx[j]);
-			return mtr_policy->sub_policys[domain][i];
+			for (i = 0; i < MLX5_MTR_RTE_COLORS; i++)
+				mlx5_hrxq_release(dev, hrxq_idx[i]);
+			return mtr_policy->sub_policys[domain][j];
 		}
 	}
 	/* Create sub policy. */
 	if (!mtr_policy->sub_policys[domain][0]->rix_hrxq[0]) {
-		/* Reuse the first dummy sub_policy*/
+		/* Reuse the first dummy sub_policy. */
 		sub_policy = mtr_policy->sub_policys[domain][0];
 		sub_policy_idx = sub_policy->idx;
 	} else {
 		sub_policy = mlx5_ipool_zmalloc
 				(priv->sh->ipool[MLX5_IPOOL_MTR_POLICY],
-				&sub_policy_idx);
+				 &sub_policy_idx);
 		if (!sub_policy ||
-			sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) {
+		    sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) {
 			for (i = 0; i < MLX5_MTR_RTE_COLORS; i++)
 				mlx5_hrxq_release(dev, hrxq_idx[i]);
 			goto rss_sub_policy_error;
@@ -16040,9 +16040,9 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev,
 		 * RSS action to Queue action.
 		 */
 		hrxq = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_HRXQ],
-			      hrxq_idx[i]);
+				      hrxq_idx[i]);
 		if (!hrxq) {
-			DRV_LOG(ERR, "Failed to create policy hrxq");
+			DRV_LOG(ERR, "Failed to get policy hrxq.");
 			goto rss_sub_policy_error;
 		}
 		act_cnt = &mtr_policy->act_cnt[i];
@@ -16056,19 +16056,21 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev,
 		}
 	}
 	if (__flow_dv_create_policy_acts_rules(dev, mtr_policy,
-		sub_policy, domain)) {
+					       sub_policy, domain)) {
 		DRV_LOG(ERR, "Failed to create policy "
-			"rules per domain.");
+			"rules for ingress domain.");
 		goto rss_sub_policy_error;
 	}
 	if (sub_policy != mtr_policy->sub_policys[domain][0]) {
 		i = (mtr_policy->sub_policy_num >>
 			(MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) &
 			MLX5_MTR_SUB_POLICY_NUM_MASK;
+		if (i >= MLX5_MTR_RSS_MAX_SUB_POLICY) {
+			DRV_LOG(ERR, "No free sub-policy slot.");
+			goto rss_sub_policy_error;
+		}
 		mtr_policy->sub_policys[domain][i] = sub_policy;
 		i++;
-		if (i > MLX5_MTR_RSS_MAX_SUB_POLICY)
-			goto rss_sub_policy_error;
 		mtr_policy->sub_policy_num &= ~(MLX5_MTR_SUB_POLICY_NUM_MASK <<
 			(MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain));
 		mtr_policy->sub_policy_num |=
@@ -16085,8 +16087,7 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev,
 			(MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) &
 			MLX5_MTR_SUB_POLICY_NUM_MASK;
 			mtr_policy->sub_policys[domain][i] = NULL;
-			mlx5_ipool_free
-			(priv->sh->ipool[MLX5_IPOOL_MTR_POLICY],
+			mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MTR_POLICY],
 					sub_policy->idx);
 		}
 	}
@@ -16108,7 +16109,7 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev,
  */
 static void
 flow_dv_destroy_sub_policy_with_rxq(struct rte_eth_dev *dev,
-		struct mlx5_flow_meter_policy *mtr_policy)
+				    struct mlx5_flow_meter_policy *mtr_policy)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_flow_meter_sub_policy *sub_policy = NULL;
@@ -16154,7 +16155,7 @@ flow_dv_destroy_sub_policy_with_rxq(struct rte_eth_dev *dev,
 		case MLX5_FLOW_FATE_QUEUE:
 			sub_policy = mtr_policy->sub_policys[domain][0];
 			__flow_dv_destroy_sub_policy_rules(dev,
-						sub_policy);
+							   sub_policy);
 			break;
 		default:
 			/*Other actions without queue and do nothing*/
-- 
2.27.0


  parent reply	other threads:[~2021-07-05 15:58 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao
2021-07-05 15:57 ` [dpdk-dev] [PATCH 1/6] net/mlx5: add yellow color default policy Bing Zhao
2021-07-05 15:57 ` [dpdk-dev] [PATCH 2/6] net/mlx5: support yellow in meter policy validation Bing Zhao
2021-07-05 15:57 ` [dpdk-dev] [PATCH 3/6] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao
2021-07-05 15:57 ` [dpdk-dev] [PATCH 4/6] net/mlx5: added support for yellow policy rules Bing Zhao
2021-07-05 15:57 ` Bing Zhao [this message]
2021-07-05 15:57 ` [dpdk-dev] [PATCH 6/6] doc: update mlx5 metering policy part Bing Zhao
2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao
2021-07-18 17:18   ` [dpdk-dev] [PATCH v2 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao
2021-07-18 17:18   ` [dpdk-dev] [PATCH v2 2/7] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao
2021-07-18 17:18   ` [dpdk-dev] [PATCH v2 3/7] net/mlx5: added support for yellow policy rules Bing Zhao
2021-07-18 17:18   ` [dpdk-dev] [PATCH v2 4/7] net/mlx5: split policies handling of colors Bing Zhao
2021-07-18 17:18   ` [dpdk-dev] [PATCH v2 5/7] net/mlx5: support yellow in meter policy validation Bing Zhao
2021-07-18 17:18   ` [dpdk-dev] [PATCH v2 6/7] net/mlx5: check consistency of meter policy and profile Bing Zhao
2021-07-18 17:18   ` [dpdk-dev] [PATCH v2 7/7] net/mlx5: add meter support for trTCM profiles Bing Zhao
2021-07-21  8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao
2021-07-21  8:54   ` [dpdk-dev] [PATCH v3 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao
2021-07-21  8:54   ` [dpdk-dev] [PATCH v3 2/7] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao
2021-07-21  8:54   ` [dpdk-dev] [PATCH v3 3/7] net/mlx5: added support for yellow policy rules Bing Zhao
2021-07-21  8:54   ` [dpdk-dev] [PATCH v3 4/7] net/mlx5: split policies handling of colors Bing Zhao
2021-07-21  8:54   ` [dpdk-dev] [PATCH v3 5/7] net/mlx5: support yellow in meter policy validation Bing Zhao
2021-07-21  8:54   ` [dpdk-dev] [PATCH v3 6/7] net/mlx5: check consistency of meter policy and profile Bing Zhao
2021-07-21  8:54   ` [dpdk-dev] [PATCH v3 7/7] net/mlx5: add meter support for trTCM profiles Bing Zhao
2021-07-22 11:28   ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Thomas Monjalon

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=20210705155756.21443-6-bingz@nvidia.com \
    --to=bingz@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=matan@nvidia.com \
    --cc=orika@nvidia.com \
    --cc=rasland@nvidia.com \
    --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).