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>,
	<thomas@monjalon.net>, <lizh@nvidia.com>, <shunh@nvidia.com>
Subject: [dpdk-dev] [PATCH v3 5/7] net/mlx5: support yellow in meter policy validation
Date: Wed, 21 Jul 2021 11:54:19 +0300	[thread overview]
Message-ID: <20210721085421.13111-6-bingz@nvidia.com> (raw)
In-Reply-To: <20210721085421.13111-1-bingz@nvidia.com>

In the previous implementation, the policy for yellow color was not
supported. The action validation for yellow was skipped.

Since the yellow color policy needs to be supported, the validation
should also be done for the yellow color. In the meanwhile, due to
the fact that color policies of one meter should be used for the
same flow(s), the domains supported of both colors should be the
same. If both of the colors have RSS as the termination actions,
except the queues, all other parameters of RSS should be the same.

Signed-off-by: Bing Zhao <bingz@nvidia.com>
---
 doc/guides/nics/mlx5.rst               |   9 +-
 doc/guides/rel_notes/release_21_08.rst |   1 +
 drivers/net/mlx5/mlx5.h                |   8 +-
 drivers/net/mlx5/mlx5_flow.c           |   6 +-
 drivers/net/mlx5/mlx5_flow.h           |   4 +-
 drivers/net/mlx5/mlx5_flow_dv.c        | 210 ++++++++++++++++---------
 drivers/net/mlx5/mlx5_flow_meter.c     |  15 +-
 7 files changed, 160 insertions(+), 93 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index f5b727c1ee..1f5b6fb954 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -420,19 +420,18 @@ Limitations
 - Meter:
 
   - All the meter colors with drop action will be counted only by the global drop statistics.
-  - Green color is not supported with drop action.
-  - Yellow detection is not supported.
+  - Yellow detection is only supported with ASO metering.
   - Red color must be with drop action.
   - Meter statistics are supported only for drop case.
-  - Meter yellow color detection is not supported.
   - A meter action created with pre-defined policy must be the last action in the flow except single case where the policy actions are:
      - green: NULL or END.
      - yellow: NULL or END.
      - RED: DROP / END.
   - The only supported meter policy actions:
-     - green: QUEUE, RSS, PORT_ID, JUMP, MARK and SET_TAG.
-     - yellow: must be empty.
+     - green: QUEUE, RSS, PORT_ID, JUMP, DROP, MARK and SET_TAG.
+     - yellow: QUEUE, RSS, PORT_ID, JUMP, DROP, MARK and SET_TAG.
      - RED: must be DROP.
+  - Policy actions of RSS for green and yellow should have the same configuration except queues.
   - meter profile packet mode is supported.
 
 - Integrity:
diff --git a/doc/guides/rel_notes/release_21_08.rst b/doc/guides/rel_notes/release_21_08.rst
index 1b38b1aa51..03d4fd059a 100644
--- a/doc/guides/rel_notes/release_21_08.rst
+++ b/doc/guides/rel_notes/release_21_08.rst
@@ -91,6 +91,7 @@ New Features
   * Added matching on IPv4 Internet Header Length (IHL).
   * Added support for matching on VXLAN header last 8-bits reserved field.
   * Optimized multi-thread flow rule insertion rate.
+  * Added support for metering policy actions of yellow color.
 
 * **Added Wangxun ngbe PMD.**
 
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index a2fe9b90c7..ea16109972 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -629,7 +629,7 @@ struct mlx5_dev_shared_port {
  */
 #define MLX5_DV_MAX_NUMBER_OF_ACTIONS 8
 
-/*ASO flow meter structures*/
+/* ASO flow meter structures */
 /* Modify this value if enum rte_mtr_color changes. */
 #define RTE_MTR_DROPPED RTE_COLORS
 /* Yellow is now supported. */
@@ -643,6 +643,12 @@ struct mlx5_dev_shared_port {
 #define MLX5_MTR_TABLE_ID_DROP 2
 /* Priority of the meter policy matcher. */
 #define MLX5_MTR_POLICY_MATCHER_PRIO 0
+/* Default policy. */
+#define MLX5_MTR_POLICY_MODE_DEF 1
+/* Only green color valid. */
+#define MLX5_MTR_POLICY_MODE_OG 2
+/* Only yellow color valid. */
+#define MLX5_MTR_POLICY_MODE_OY 3
 
 enum mlx5_meter_domain {
 	MLX5_MTR_DOMAIN_INGRESS,
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index d90c8cd314..549b3058c2 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -7199,14 +7199,14 @@ mlx5_flow_validate_mtr_acts(struct rte_eth_dev *dev,
 			struct rte_flow_attr *attr,
 			bool *is_rss,
 			uint8_t *domain_bitmap,
-			bool *is_def_policy,
+			uint8_t *policy_mode,
 			struct rte_mtr_error *error)
 {
 	const struct mlx5_flow_driver_ops *fops;
 
 	fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV);
-	return fops->validate_mtr_acts(dev, actions, attr,
-			is_rss, domain_bitmap, is_def_policy, error);
+	return fops->validate_mtr_acts(dev, actions, attr, is_rss,
+				       domain_bitmap, policy_mode, error);
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 8f0521aa72..3724293d26 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1209,7 +1209,7 @@ typedef int (*mlx5_flow_validate_mtr_acts_t)
 			 struct rte_flow_attr *attr,
 			 bool *is_rss,
 			 uint8_t *domain_bitmap,
-			 bool *is_def_policy,
+			 uint8_t *policy_mode,
 			 struct rte_mtr_error *error);
 typedef int (*mlx5_flow_create_mtr_acts_t)
 			(struct rte_eth_dev *dev,
@@ -1690,7 +1690,7 @@ int mlx5_flow_validate_mtr_acts(struct rte_eth_dev *dev,
 			struct rte_flow_attr *attr,
 			bool *is_rss,
 			uint8_t *domain_bitmap,
-			bool *is_def_policy,
+			uint8_t *policy_mode,
 			struct rte_mtr_error *error);
 void mlx5_flow_destroy_mtr_acts(struct rte_eth_dev *dev,
 		      struct mlx5_flow_meter_policy *mtr_policy);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index ee593a7001..97e297d5c2 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -17358,6 +17358,31 @@ flow_dv_action_validate(struct rte_eth_dev *dev,
 	}
 }
 
+/*
+ * Check if the RSS configurations for colors of a meter policy match
+ * each other, except the queues.
+ *
+ * @param[in] r1
+ *   Pointer to the first RSS flow action.
+ * @param[in] r2
+ *   Pointer to the second RSS flow action.
+ *
+ * @return
+ *   0 on match, 1 on conflict.
+ */
+static inline int
+flow_dv_mtr_policy_rss_compare(const struct rte_flow_action_rss *r1,
+			       const struct rte_flow_action_rss *r2)
+{
+	if (!r1 || !r2)
+		return 0;
+	if (r1->func != r2->func || r1->level != r2->level ||
+	    r1->types != r2->types || r1->key_len != r2->key_len ||
+	    memcmp(r1->key, r2->key, r1->key_len))
+		return 1;
+	return 0;
+}
+
 /**
  * Validate the meter hierarchy chain for meter policy.
  *
@@ -17453,13 +17478,13 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 			struct rte_flow_attr *attr,
 			bool *is_rss,
 			uint8_t *domain_bitmap,
-			bool *is_def_policy,
+			uint8_t *policy_mode,
 			struct rte_mtr_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_config *dev_conf = &priv->config;
 	const struct rte_flow_action *act;
-	uint64_t action_flags = 0;
+	uint64_t action_flags[RTE_COLORS] = {0};
 	int actions_n;
 	int i, ret;
 	struct rte_flow_error flow_err;
@@ -17467,36 +17492,45 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 	uint8_t def_domain = MLX5_MTR_ALL_DOMAIN_BIT;
 	uint8_t hierarchy_domain = 0;
 	const struct rte_flow_action_meter *mtr;
+	bool def_green = false;
+	bool def_yellow = false;
+	const struct rte_flow_action_rss *rss_color[RTE_COLORS] = {NULL};
 
 	if (!priv->config.dv_esw_en)
 		def_domain &= ~MLX5_MTR_DOMAIN_TRANSFER_BIT;
 	*domain_bitmap = def_domain;
-	if (actions[RTE_COLOR_YELLOW] &&
-		actions[RTE_COLOR_YELLOW]->type != RTE_FLOW_ACTION_TYPE_END)
-		return -rte_mtr_error_set(error, ENOTSUP,
-				RTE_MTR_ERROR_TYPE_METER_POLICY,
-				NULL,
-				"Yellow color does not support any action.");
-	if (actions[RTE_COLOR_YELLOW] &&
-		actions[RTE_COLOR_YELLOW]->type != RTE_FLOW_ACTION_TYPE_DROP)
+	/* Red color could only support DROP action. */
+	if (!actions[RTE_COLOR_RED] ||
+	    actions[RTE_COLOR_RED]->type != RTE_FLOW_ACTION_TYPE_DROP)
 		return -rte_mtr_error_set(error, ENOTSUP,
 				RTE_MTR_ERROR_TYPE_METER_POLICY,
 				NULL, "Red color only supports drop action.");
 	/*
 	 * Check default policy actions:
-	 * Green/Yellow: no action, Red: drop action
+	 * Green / Yellow: no action, Red: drop action
+	 * Either G or Y will trigger default policy actions to be created.
 	 */
-	if ((!actions[RTE_COLOR_GREEN] ||
-		actions[RTE_COLOR_GREEN]->type == RTE_FLOW_ACTION_TYPE_END)) {
-		*is_def_policy = true;
+	if (!actions[RTE_COLOR_GREEN] ||
+	    actions[RTE_COLOR_GREEN]->type == RTE_FLOW_ACTION_TYPE_END)
+		def_green = true;
+	if (!actions[RTE_COLOR_YELLOW] ||
+	    actions[RTE_COLOR_YELLOW]->type == RTE_FLOW_ACTION_TYPE_END)
+		def_yellow = true;
+	if (def_green && def_yellow) {
+		*policy_mode = MLX5_MTR_POLICY_MODE_DEF;
 		return 0;
+	} else if (!def_green && def_yellow) {
+		*policy_mode = MLX5_MTR_POLICY_MODE_OG;
+	} else if (def_green && !def_yellow) {
+		*policy_mode = MLX5_MTR_POLICY_MODE_OY;
 	}
-	flow_err.message = NULL;
+	/* Set to empty string in case of NULL pointer access by user. */
+	flow_err.message = "";
 	for (i = 0; i < RTE_COLORS; i++) {
 		act = actions[i];
-		for (action_flags = 0, actions_n = 0;
-			act && act->type != RTE_FLOW_ACTION_TYPE_END;
-			act++) {
+		for (action_flags[i] = 0, actions_n = 0;
+		     act && act->type != RTE_FLOW_ACTION_TYPE_END;
+		     act++) {
 			if (actions_n == MLX5_DV_MAX_NUMBER_OF_ACTIONS)
 				return -rte_mtr_error_set(error, ENOTSUP,
 					  RTE_MTR_ERROR_TYPE_METER_POLICY,
@@ -17510,7 +17544,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					NULL, "PORT action validate check"
 					" fail for ESW disable");
 				ret = flow_dv_validate_action_port_id(dev,
-						action_flags,
+						action_flags[i],
 						act, attr, &flow_err);
 				if (ret)
 					return -rte_mtr_error_set(error,
@@ -17520,11 +17554,11 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					flow_err.message :
 					"PORT action validate check fail");
 				++actions_n;
-				action_flags |= MLX5_FLOW_ACTION_PORT_ID;
+				action_flags[i] |= MLX5_FLOW_ACTION_PORT_ID;
 				break;
 			case RTE_FLOW_ACTION_TYPE_MARK:
 				ret = flow_dv_validate_action_mark(dev, act,
-							   action_flags,
+							   action_flags[i],
 							   attr, &flow_err);
 				if (ret < 0)
 					return -rte_mtr_error_set(error,
@@ -17541,12 +17575,12 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					NULL, "Extend MARK action is "
 					"not supported. Please try use "
 					"default policy for meter.");
-				action_flags |= MLX5_FLOW_ACTION_MARK;
+				action_flags[i] |= MLX5_FLOW_ACTION_MARK;
 				++actions_n;
 				break;
 			case RTE_FLOW_ACTION_TYPE_SET_TAG:
 				ret = flow_dv_validate_action_set_tag(dev,
-							act, action_flags,
+							act, action_flags[i],
 							attr, &flow_err);
 				if (ret)
 					return -rte_mtr_error_set(error,
@@ -17555,19 +17589,12 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					NULL, flow_err.message ?
 					flow_err.message :
 					"Set tag action validate check fail");
-				/*
-				 * Count all modify-header actions
-				 * as one action.
-				 */
-				if (!(action_flags &
-					MLX5_FLOW_MODIFY_HDR_ACTIONS))
-					++actions_n;
-				action_flags |= MLX5_FLOW_ACTION_SET_TAG;
+				action_flags[i] |= MLX5_FLOW_ACTION_SET_TAG;
+				++actions_n;
 				break;
 			case RTE_FLOW_ACTION_TYPE_DROP:
 				ret = mlx5_flow_validate_action_drop
-					(action_flags,
-					attr, &flow_err);
+					(action_flags[i], attr, &flow_err);
 				if (ret < 0)
 					return -rte_mtr_error_set(error,
 					ENOTSUP,
@@ -17575,7 +17602,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					NULL, flow_err.message ?
 					flow_err.message :
 					"Drop action validate check fail");
-				action_flags |= MLX5_FLOW_ACTION_DROP;
+				action_flags[i] |= MLX5_FLOW_ACTION_DROP;
 				++actions_n;
 				break;
 			case RTE_FLOW_ACTION_TYPE_QUEUE:
@@ -17584,9 +17611,9 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 				 * metadata feature is engaged.
 				 */
 				if (dev_conf->dv_flow_en &&
-					(dev_conf->dv_xmeta_en !=
-					MLX5_XMETA_MODE_LEGACY) &&
-					mlx5_flow_ext_mreg_supported(dev))
+				    (dev_conf->dv_xmeta_en !=
+				     MLX5_XMETA_MODE_LEGACY) &&
+				    mlx5_flow_ext_mreg_supported(dev))
 					return -rte_mtr_error_set(error,
 					  ENOTSUP,
 					  RTE_MTR_ERROR_TYPE_METER_POLICY,
@@ -17594,7 +17621,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					  "is not supported. Please try use "
 					  "default policy for meter.");
 				ret = mlx5_flow_validate_action_queue(act,
-							action_flags, dev,
+							action_flags[i], dev,
 							attr, &flow_err);
 				if (ret < 0)
 					return -rte_mtr_error_set(error,
@@ -17603,14 +17630,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					  NULL, flow_err.message ?
 					  flow_err.message :
 					  "Queue action validate check fail");
-				action_flags |= MLX5_FLOW_ACTION_QUEUE;
+				action_flags[i] |= MLX5_FLOW_ACTION_QUEUE;
 				++actions_n;
 				break;
 			case RTE_FLOW_ACTION_TYPE_RSS:
 				if (dev_conf->dv_flow_en &&
-					(dev_conf->dv_xmeta_en !=
-					MLX5_XMETA_MODE_LEGACY) &&
-					mlx5_flow_ext_mreg_supported(dev))
+				    (dev_conf->dv_xmeta_en !=
+				     MLX5_XMETA_MODE_LEGACY) &&
+				    mlx5_flow_ext_mreg_supported(dev))
 					return -rte_mtr_error_set(error,
 					  ENOTSUP,
 					  RTE_MTR_ERROR_TYPE_METER_POLICY,
@@ -17618,7 +17645,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					  "is not supported. Please try use "
 					  "default policy for meter.");
 				ret = mlx5_validate_action_rss(dev, act,
-						&flow_err);
+							       &flow_err);
 				if (ret < 0)
 					return -rte_mtr_error_set(error,
 					  ENOTSUP,
@@ -17626,13 +17653,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					  NULL, flow_err.message ?
 					  flow_err.message :
 					  "RSS action validate check fail");
-				action_flags |= MLX5_FLOW_ACTION_RSS;
+				action_flags[i] |= MLX5_FLOW_ACTION_RSS;
 				++actions_n;
-				*is_rss = true;
+				/* Either G or Y will set the RSS. */
+				rss_color[i] = act->conf;
 				break;
 			case RTE_FLOW_ACTION_TYPE_JUMP:
 				ret = flow_dv_validate_action_jump(dev,
-					NULL, act, action_flags,
+					NULL, act, action_flags[i],
 					attr, true, &flow_err);
 				if (ret)
 					return -rte_mtr_error_set(error,
@@ -17642,8 +17670,13 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					  flow_err.message :
 					  "Jump action validate check fail");
 				++actions_n;
-				action_flags |= MLX5_FLOW_ACTION_JUMP;
+				action_flags[i] |= MLX5_FLOW_ACTION_JUMP;
 				break;
+			/*
+			 * Only the last meter in the hierarchy will support
+			 * the YELLOW color steering. Then in the meter policy
+			 * actions list, there should be no other meter inside.
+			 */
 			case RTE_FLOW_ACTION_TYPE_METER:
 				if (i != RTE_COLOR_GREEN)
 					return -rte_mtr_error_set(error,
@@ -17655,14 +17688,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 				mtr = act->conf;
 				ret = flow_dv_validate_policy_mtr_hierarchy(dev,
 							mtr->mtr_id,
-							action_flags,
+							action_flags[i],
 							is_rss,
 							&hierarchy_domain,
 							error);
 				if (ret)
 					return ret;
 				++actions_n;
-				action_flags |=
+				action_flags[i] |=
 				MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY;
 				break;
 			default:
@@ -17672,31 +17705,38 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					"Doesn't support optional action");
 			}
 		}
-		/* Yellow is not supported, just skip. */
-		if (i == RTE_COLOR_YELLOW)
-			continue;
-		if (action_flags & MLX5_FLOW_ACTION_PORT_ID)
+		if (action_flags[i] & MLX5_FLOW_ACTION_PORT_ID)
 			domain_color[i] = MLX5_MTR_DOMAIN_TRANSFER_BIT;
-		else if ((action_flags &
-			(MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_QUEUE)) ||
-			(action_flags & MLX5_FLOW_ACTION_MARK))
+		else if ((action_flags[i] &
+			  (MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_QUEUE)) ||
+			 (action_flags[i] & MLX5_FLOW_ACTION_MARK))
 			/*
 			 * Only support MLX5_XMETA_MODE_LEGACY
-			 * so MARK action only in ingress domain.
+			 * so MARK action is only in ingress domain.
 			 */
 			domain_color[i] = MLX5_MTR_DOMAIN_INGRESS_BIT;
-		else if (action_flags &
-			MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY)
+		else if (action_flags[i] &
+			 MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY)
 			domain_color[i] = hierarchy_domain;
 		else
 			domain_color[i] = def_domain;
+		/*
+		 * Non-termination actions only support NIC Tx domain.
+		 * The adjustion should be skipped when there is no
+		 * action or only END is provided. The default domains
+		 * bit-mask is set to find the MIN intersection.
+		 * The action flags checking should also be skipped.
+		 */
+		if ((def_green && i == RTE_COLOR_GREEN) ||
+		    (def_yellow && i == RTE_COLOR_YELLOW))
+			continue;
 		/*
 		 * Validate the drop action mutual exclusion
 		 * with other actions. Drop action is mutually-exclusive
 		 * with any other action, except for Count action.
 		 */
-		if ((action_flags & MLX5_FLOW_ACTION_DROP) &&
-			(action_flags & ~MLX5_FLOW_ACTION_DROP)) {
+		if ((action_flags[i] & MLX5_FLOW_ACTION_DROP) &&
+		    (action_flags[i] & ~MLX5_FLOW_ACTION_DROP)) {
 			return -rte_mtr_error_set(error, ENOTSUP,
 				RTE_MTR_ERROR_TYPE_METER_POLICY,
 				NULL, "Drop action is mutually-exclusive "
@@ -17705,40 +17745,60 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 		/* Eswitch has few restrictions on using items and actions */
 		if (domain_color[i] & MLX5_MTR_DOMAIN_TRANSFER_BIT) {
 			if (!mlx5_flow_ext_mreg_supported(dev) &&
-				action_flags & MLX5_FLOW_ACTION_MARK)
+			    action_flags[i] & MLX5_FLOW_ACTION_MARK)
 				return -rte_mtr_error_set(error, ENOTSUP,
 					RTE_MTR_ERROR_TYPE_METER_POLICY,
 					NULL, "unsupported action MARK");
-			if (action_flags & MLX5_FLOW_ACTION_QUEUE)
+			if (action_flags[i] & MLX5_FLOW_ACTION_QUEUE)
 				return -rte_mtr_error_set(error, ENOTSUP,
 					RTE_MTR_ERROR_TYPE_METER_POLICY,
 					NULL, "unsupported action QUEUE");
-			if (action_flags & MLX5_FLOW_ACTION_RSS)
+			if (action_flags[i] & MLX5_FLOW_ACTION_RSS)
 				return -rte_mtr_error_set(error, ENOTSUP,
 					RTE_MTR_ERROR_TYPE_METER_POLICY,
 					NULL, "unsupported action RSS");
-			if (!(action_flags & MLX5_FLOW_FATE_ESWITCH_ACTIONS))
+			if (!(action_flags[i] & MLX5_FLOW_FATE_ESWITCH_ACTIONS))
 				return -rte_mtr_error_set(error, ENOTSUP,
 					RTE_MTR_ERROR_TYPE_METER_POLICY,
 					NULL, "no fate action is found");
 		} else {
-			if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) &&
-				(domain_color[i] &
-				MLX5_MTR_DOMAIN_INGRESS_BIT)) {
+			if (!(action_flags[i] & MLX5_FLOW_FATE_ACTIONS) &&
+			    (domain_color[i] & MLX5_MTR_DOMAIN_INGRESS_BIT)) {
 				if ((domain_color[i] &
-					MLX5_MTR_DOMAIN_EGRESS_BIT))
+				     MLX5_MTR_DOMAIN_EGRESS_BIT))
 					domain_color[i] =
-					MLX5_MTR_DOMAIN_EGRESS_BIT;
+						MLX5_MTR_DOMAIN_EGRESS_BIT;
 				else
 					return -rte_mtr_error_set(error,
-					ENOTSUP,
-					RTE_MTR_ERROR_TYPE_METER_POLICY,
-					NULL, "no fate action is found");
+						ENOTSUP,
+						RTE_MTR_ERROR_TYPE_METER_POLICY,
+						NULL,
+						"no fate action is found");
 			}
 		}
-		if (domain_color[i] != def_domain)
-			*domain_bitmap = domain_color[i];
 	}
+	/* If both colors have RSS, the attributes should be the same. */
+	if (flow_dv_mtr_policy_rss_compare(rss_color[RTE_COLOR_GREEN],
+					   rss_color[RTE_COLOR_YELLOW]))
+		return -rte_mtr_error_set(error, EINVAL,
+					  RTE_MTR_ERROR_TYPE_METER_POLICY,
+					  NULL, "policy RSS attr conflict");
+	if (rss_color[RTE_COLOR_GREEN] || rss_color[RTE_COLOR_YELLOW])
+		*is_rss = true;
+	/* "domain_color[C]" is non-zero for each color, default is ALL. */
+	if (!def_green && !def_yellow &&
+	    domain_color[RTE_COLOR_GREEN] != domain_color[RTE_COLOR_YELLOW] &&
+	    !(action_flags[RTE_COLOR_GREEN] & MLX5_FLOW_ACTION_DROP) &&
+	    !(action_flags[RTE_COLOR_YELLOW] & MLX5_FLOW_ACTION_DROP))
+		return -rte_mtr_error_set(error, EINVAL,
+					  RTE_MTR_ERROR_TYPE_METER_POLICY,
+					  NULL, "policy domains conflict");
+	/*
+	 * At least one color policy is listed in the actions, the domains
+	 * to be supported should be the intersection.
+	 */
+	*domain_bitmap = domain_color[RTE_COLOR_GREEN] &
+			 domain_color[RTE_COLOR_YELLOW];
 	return 0;
 }
 
diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c
index 19b2665558..32ad4ea133 100644
--- a/drivers/net/mlx5/mlx5_flow_meter.c
+++ b/drivers/net/mlx5/mlx5_flow_meter.c
@@ -582,7 +582,7 @@ mlx5_flow_meter_policy_validate(struct rte_eth_dev *dev,
 	struct rte_flow_attr attr = { .transfer =
 			priv->config.dv_esw_en ? 1 : 0};
 	bool is_rss = false;
-	bool is_def_policy = false;
+	uint8_t policy_mode;
 	uint8_t domain_bitmap;
 	int ret;
 
@@ -591,7 +591,7 @@ mlx5_flow_meter_policy_validate(struct rte_eth_dev *dev,
 				RTE_MTR_ERROR_TYPE_METER_POLICY,
 				NULL, "meter policy unsupported.");
 	ret = mlx5_flow_validate_mtr_acts(dev, policy->actions, &attr,
-			&is_rss, &domain_bitmap, &is_def_policy, error);
+			&is_rss, &domain_bitmap, &policy_mode, error);
 	if (ret)
 		return ret;
 	return 0;
@@ -674,7 +674,7 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev,
 	struct mlx5_flow_meter_policy *mtr_policy = NULL;
 	struct mlx5_flow_meter_sub_policy *sub_policy;
 	bool is_rss = false;
-	bool is_def_policy = false;
+	uint8_t policy_mode;
 	uint32_t i;
 	int ret;
 	uint32_t policy_size = sizeof(struct mlx5_flow_meter_policy);
@@ -701,14 +701,15 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev,
 					  RTE_MTR_ERROR_TYPE_METER_POLICY_ID,
 					  NULL, "policy ID exists. ");
 	ret = mlx5_flow_validate_mtr_acts(dev, policy->actions, &attr,
-			&is_rss, &domain_bitmap, &is_def_policy, error);
+					  &is_rss, &domain_bitmap,
+					  &policy_mode, error);
 	if (ret)
 		return ret;
 	if (!domain_bitmap)
 		return -rte_mtr_error_set(error, ENOTSUP,
-				RTE_MTR_ERROR_TYPE_METER_POLICY,
-				NULL, "fail to find policy domain.");
-	if (is_def_policy) {
+					  RTE_MTR_ERROR_TYPE_METER_POLICY,
+					  NULL, "fail to find policy domain.");
+	if (policy_mode == MLX5_MTR_POLICY_MODE_DEF) {
 		if (priv->sh->mtrmng->def_policy_id != MLX5_INVALID_POLICY_ID)
 			return -rte_mtr_error_set(error, EEXIST,
 				RTE_MTR_ERROR_TYPE_METER_POLICY_ID,
-- 
2.27.0


  parent reply	other threads:[~2021-07-21  8:55 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 ` [dpdk-dev] [PATCH 5/6] net/mlx5: split policies handling of colors Bing Zhao
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   ` Bing Zhao [this message]
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=20210721085421.13111-6-bingz@nvidia.com \
    --to=bingz@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=lizh@nvidia.com \
    --cc=matan@nvidia.com \
    --cc=orika@nvidia.com \
    --cc=rasland@nvidia.com \
    --cc=shunh@nvidia.com \
    --cc=thomas@monjalon.net \
    --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).