patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport
@ 2020-02-28  3:33 Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 01/11] net/mlx5: unify validation of drop action Suanming Mou
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable

Please remove the commit below with rebase first:
dec473d net/mlx5: fix metadata split with encap action

The commit makes the wrong patch order.
Remove the commit will not cause any conflicts.

Dekel Peled (2):
  net/mlx5: unify validation of drop action
  net/mlx5: update description of validation functions

Matan Azrad (2):
  net/mlx5: fix encap/decap validation
  net/mlx5: fix metadata split with encap action

Suanming Mou (7):
  net/mlx5: support maximum flow id allocation
  net/mlx5: fix register usage in meter
  net/mlx5: fix layer validation with decapsulation
  net/mlx5: fix layer type in header modify action
  net/mlx5: fix layer flags missing in metadata
  net/mlx5: fix match information in meter
  net/mlx5: fix VLAN actions in meter

 drivers/net/mlx5/mlx5.c            |  19 +-
 drivers/net/mlx5/mlx5.h            |   4 +
 drivers/net/mlx5/mlx5_devx_cmds.c  |   2 +
 drivers/net/mlx5/mlx5_flow.c       | 366 ++++++++++++++++------------
 drivers/net/mlx5/mlx5_flow.h       |  40 ++-
 drivers/net/mlx5/mlx5_flow_dv.c    | 485 +++++++++++++++++++++----------------
 drivers/net/mlx5/mlx5_flow_verbs.c |  12 +
 drivers/net/mlx5/mlx5_prm.h        |   7 +-
 8 files changed, 552 insertions(+), 383 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 01/11] net/mlx5: unify validation of drop action
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 02/11] net/mlx5: update description of validation functions Suanming Mou
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable, Dekel Peled

From: Dekel Peled <dekelp@mellanox.com>

[ upstream commit 70faf9ae0a299e5750b687dfecb6efc65b038304 ]

According to PRM: "Drop action is mutually-exclusive with any other
action, except for Count action".
In current code this limitation is checked separately in validation
function of each action.

This patch removes the discrete checks, and adds a single check common
for all actions.

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c       | 25 +------------------------
 drivers/net/mlx5/mlx5_flow_dv.c    | 36 ++++++++++++------------------------
 drivers/net/mlx5/mlx5_flow_verbs.c | 12 ++++++++++++
 3 files changed, 25 insertions(+), 48 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index a7ddc16..bbbd2e4 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -902,11 +902,6 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			       const struct rte_flow_attr *attr,
 			       struct rte_flow_error *error)
 {
-
-	if (action_flags & MLX5_FLOW_ACTION_DROP)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and flag in same flow");
 	if (action_flags & MLX5_FLOW_ACTION_MARK)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -958,10 +953,6 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 					  &mark->id,
 					  "mark id must in 0 <= id < "
 					  RTE_STR(MLX5_FLOW_MARK_MAX));
-	if (action_flags & MLX5_FLOW_ACTION_DROP)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and mark in same flow");
 	if (action_flags & MLX5_FLOW_ACTION_FLAG)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -993,24 +984,10 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 int
-mlx5_flow_validate_action_drop(uint64_t action_flags,
+mlx5_flow_validate_action_drop(uint64_t action_flags __rte_unused,
 			       const struct rte_flow_attr *attr,
 			       struct rte_flow_error *error)
 {
-	if (action_flags & MLX5_FLOW_ACTION_FLAG)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and flag in same flow");
-	if (action_flags & MLX5_FLOW_ACTION_MARK)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and mark in same flow");
-	if (action_flags & (MLX5_FLOW_FATE_ACTIONS |
-			    MLX5_FLOW_FATE_ESWITCH_ACTIONS))
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't have 2 fate actions in"
-					  " same flow");
 	if (attr->egress)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index e39d3c4..2811308 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1778,10 +1778,6 @@ struct field_modify_info modify_tcp[] = {
 	if (ret < 0)
 		return ret;
 	assert(ret > 0);
-	if (action_flags & MLX5_FLOW_ACTION_DROP)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and flag in same flow");
 	if (action_flags & MLX5_FLOW_ACTION_MARK)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -1851,10 +1847,6 @@ struct field_modify_info modify_tcp[] = {
 					  RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 					  &mark->id,
 					  "mark id exceeds the limit");
-	if (action_flags & MLX5_FLOW_ACTION_DROP)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and mark in same flow");
 	if (action_flags & MLX5_FLOW_ACTION_FLAG)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -2039,10 +2031,6 @@ struct field_modify_info modify_tcp[] = {
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
 					  "configuration cannot be null");
-	if (action_flags & MLX5_FLOW_ACTION_DROP)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and encap in same flow");
 	if (action_flags & (MLX5_FLOW_ENCAP_ACTIONS | MLX5_FLOW_DECAP_ACTIONS))
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -2075,10 +2063,6 @@ struct field_modify_info modify_tcp[] = {
 				 const struct rte_flow_attr *attr,
 				 struct rte_flow_error *error)
 {
-	if (action_flags & MLX5_FLOW_ACTION_DROP)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and decap in same flow");
 	if (action_flags & (MLX5_FLOW_ENCAP_ACTIONS | MLX5_FLOW_DECAP_ACTIONS))
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -2125,10 +2109,6 @@ struct field_modify_info modify_tcp[] = {
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
 					  "configuration cannot be null");
-	if (action_flags & MLX5_FLOW_ACTION_DROP)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and encap in same flow");
 	if (action_flags & MLX5_FLOW_ENCAP_ACTIONS)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -2172,10 +2152,6 @@ struct field_modify_info modify_tcp[] = {
 {
 	const struct rte_flow_action_raw_decap *decap	= action->conf;
 
-	if (action_flags & MLX5_FLOW_ACTION_DROP)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't drop and decap in same flow");
 	if (action_flags & MLX5_FLOW_ENCAP_ACTIONS)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -4827,6 +4803,18 @@ struct field_modify_info modify_tcp[] = {
 						  "action not supported");
 		}
 	}
+	/*
+	 * 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 | MLX5_FLOW_ACTION_COUNT)))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "Drop action is mutually-exclusive "
+					  "with any other action, except for "
+					  "Count action");
 	/* Eswitch has few restrictions on using items and actions */
 	if (attr->transfer) {
 		if (!mlx5_flow_ext_mreg_supported(dev) &&
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index c787c98..72fb1e4 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -1255,6 +1255,18 @@
 						  "action not supported");
 		}
 	}
+	/*
+	 * 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 | MLX5_FLOW_ACTION_COUNT)))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "Drop action is mutually-exclusive "
+					  "with any other action, except for "
+					  "Count action");
 	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS))
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, actions,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 02/11] net/mlx5: update description of validation functions
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 01/11] net/mlx5: unify validation of drop action Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 03/11] net/mlx5: support maximum flow id allocation Suanming Mou
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable, Dekel Peled

From: Dekel Peled <dekelp@mellanox.com>

[ upstream commit 41dcaff4ad50e069ad2b3a9a7e3ffefa2c23765b ]

Description of several functions is not accurate.
This patch updates the description, parameter names etc.

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 2811308..d328d76 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1539,14 +1539,10 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Get VLAN default info from vlan match info.
  *
- * @param[in] dev
- *   Pointer to the rte_eth_dev structure.
- * @param[in] item
+ * @param[in] items
  *   the list of item specifications.
  * @param[out] vlan
  *   pointer VLAN info to fill to.
- * @param[out] error
- *   Pointer to error structure.
  *
  * @return
  *   0 on success, a negative errno value otherwise and rte_errno is set.
@@ -1598,8 +1594,10 @@ struct field_modify_info modify_tcp[] = {
  *
  * @param[in] action_flags
  *   Holds the actions detected until now.
+ * @param[in] item_flags
+ *   The items found in this flow rule.
  * @param[in] action
- *   Pointer to the encap action.
+ *   Pointer to the action structure.
  * @param[in] attr
  *   Pointer to flow attributes
  * @param[out] error
@@ -1649,8 +1647,6 @@ struct field_modify_info modify_tcp[] = {
  *   Holds the actions detected until now.
  * @param[in] actions
  *   Pointer to the list of actions remaining in the flow rule.
- * @param[in] attr
- *   Pointer to flow attributes
  * @param[out] error
  *   Pointer to error structure.
  *
@@ -1865,7 +1861,7 @@ struct field_modify_info modify_tcp[] = {
  * @param[in] dev
  *   Pointer to the rte_eth_dev structure.
  * @param[in] action
- *   Pointer to the encap action.
+ *   Pointer to the action structure.
  * @param[in] action_flags
  *   Holds the actions detected until now.
  * @param[in] attr
@@ -1926,7 +1922,7 @@ struct field_modify_info modify_tcp[] = {
  * @param[in] dev
  *   Pointer to the rte_eth_dev structure.
  * @param[in] action
- *   Pointer to the encap action.
+ *   Pointer to the action structure.
  * @param[in] action_flags
  *   Holds the actions detected until now.
  * @param[in] attr
@@ -1980,7 +1976,7 @@ struct field_modify_info modify_tcp[] = {
  * Validate count action.
  *
  * @param[in] dev
- *   device otr.
+ *   Pointer to rte_eth_dev structure.
  * @param[out] error
  *   Pointer to error structure.
  *
@@ -2012,7 +2008,7 @@ struct field_modify_info modify_tcp[] = {
  * @param[in] action_flags
  *   Holds the actions detected until now.
  * @param[in] action
- *   Pointer to the encap action.
+ *   Pointer to the action structure.
  * @param[in] attr
  *   Pointer to flow attributes
  * @param[out] error
@@ -2872,12 +2868,12 @@ struct field_modify_info modify_tcp[] = {
  *
  * @param[in] dev
  *   Pointer to rte_eth_dev structure.
- * @param[in] vlan_tag
- *   the vlan tag to push to the Ethernet header.
- * @param[in, out] dev_flow
- *   Pointer to the mlx5_flow.
  * @param[in] attr
  *   Pointer to the flow attributes.
+ * @param[in] vlan
+ *   Pointer to the vlan to push to the Ethernet header.
+ * @param[in, out] dev_flow
+ *   Pointer to the mlx5_flow.
  * @param[out] error
  *   Pointer to the error structure.
  *
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 03/11] net/mlx5: support maximum flow id allocation
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 01/11] net/mlx5: unify validation of drop action Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 02/11] net/mlx5: update description of validation functions Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 04/11] net/mlx5: fix register usage in meter Suanming Mou
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable

[ upstream commit 30a3687d99f17fa821a7aa933b5c738bf6885c30 ]

The id allocated is for the register unique id match. Some registers may
not use the full 32 bits. Add the maximum id to avoid allocate id over
the register restriction.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5.c      | 12 ++++++++----
 drivers/net/mlx5/mlx5.h      |  1 +
 drivers/net/mlx5/mlx5_flow.h |  2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 5111057..7493e64 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -196,11 +196,14 @@ struct mlx5_dev_spawn_data {
 /**
  * Allocate ID pool structure.
  *
+ * @param[in] max_id
+ *   The maximum id can be allocated from the pool.
+ *
  * @return
  *   Pointer to pool object, NULL value otherwise.
  */
 struct mlx5_flow_id_pool *
-mlx5_flow_id_pool_alloc(void)
+mlx5_flow_id_pool_alloc(uint32_t max_id)
 {
 	struct mlx5_flow_id_pool *pool;
 	void *mem;
@@ -223,6 +226,7 @@ struct mlx5_flow_id_pool *
 	pool->curr = pool->free_arr;
 	pool->last = pool->free_arr + MLX5_FLOW_MIN_ID_POOL_SIZE;
 	pool->base_index = 0;
+	pool->max_id = max_id;
 	return pool;
 error:
 	rte_free(pool);
@@ -257,7 +261,7 @@ struct mlx5_flow_id_pool *
 mlx5_flow_id_get(struct mlx5_flow_id_pool *pool, uint32_t *id)
 {
 	if (pool->curr == pool->free_arr) {
-		if (pool->base_index == UINT32_MAX) {
+		if (pool->base_index == pool->max_id) {
 			rte_errno  = ENOMEM;
 			DRV_LOG(ERR, "no free id");
 			return -rte_errno;
@@ -590,7 +594,7 @@ struct mlx5_flow_id_pool *
 			goto error;
 		}
 	}
-	sh->flow_id_pool = mlx5_flow_id_pool_alloc();
+	sh->flow_id_pool = mlx5_flow_id_pool_alloc(UINT32_MAX);
 	if (!sh->flow_id_pool) {
 		DRV_LOG(ERR, "can't create flow id pool");
 		err = ENOMEM;
@@ -2680,7 +2684,7 @@ struct mlx5_flow_id_pool *
 		err = mlx5_alloc_shared_dr(priv);
 		if (err)
 			goto error;
-		priv->qrss_id_pool = mlx5_flow_id_pool_alloc();
+		priv->qrss_id_pool = mlx5_flow_id_pool_alloc(UINT32_MAX);
 		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 7a7c1d1..a40ec51 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -626,6 +626,7 @@ struct mlx5_flow_id_pool {
 	/**< The next index that can be used without any free elements. */
 	uint32_t *curr; /**< Pointer to the index to pop. */
 	uint32_t *last; /**< Pointer to the last element in the empty arrray. */
+	uint32_t max_id; /**< Maximum id can be allocated from the pool. */
 };
 
 /*
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 0da1b5e..91594d2 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -746,7 +746,7 @@ struct mlx5_flow_driver_ops {
 
 /* mlx5_flow.c */
 
-struct mlx5_flow_id_pool *mlx5_flow_id_pool_alloc(void);
+struct mlx5_flow_id_pool *mlx5_flow_id_pool_alloc(uint32_t max_id);
 void mlx5_flow_id_pool_release(struct mlx5_flow_id_pool *pool);
 uint32_t mlx5_flow_id_get(struct mlx5_flow_id_pool *pool, uint32_t *id);
 uint32_t mlx5_flow_id_release(struct mlx5_flow_id_pool *pool,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 04/11] net/mlx5: fix register usage in meter
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
                   ` (2 preceding siblings ...)
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 03/11] net/mlx5: support maximum flow id allocation Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 05/11] net/mlx5: fix encap/decap validation Suanming Mou
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable

[ upstream commit 792e749e92d585ed083c80ab5c15cf5203534ae9 ]

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")

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@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 7493e64..5d6ae24 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 a40ec51..de80f62 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;
@@ -728,6 +730,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 9893287..e3b02bd 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 bbbd2e4..6d96085 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -347,6 +347,7 @@ struct mlx5_flow_tunnel_info {
 	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:
@@ -385,29 +386,36 @@ struct mlx5_flow_tunnel_info {
 			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,
@@ -422,12 +430,12 @@ struct mlx5_flow_tunnel_info {
 		 * 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");
@@ -3520,7 +3528,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;
 	assert(tag_action);
 	tag_action->conf = set_tag;
 	return tag_id;
@@ -3959,13 +3967,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,
@@ -3992,13 +4001,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 d328d76..becdf3f 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -8060,7 +8060,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);
@@ -8254,7 +8254,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 9f1d122..bdbfe56 100644
--- a/drivers/net/mlx5/mlx5_prm.h
+++ b/drivers/net/mlx5/mlx5_prm.h
@@ -1196,7 +1196,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];
@@ -1816,6 +1818,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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 05/11] net/mlx5: fix encap/decap validation
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
                   ` (3 preceding siblings ...)
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 04/11] net/mlx5: fix register usage in meter Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 06/11] net/mlx5: fix layer validation with decapsulation Suanming Mou
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable, Matan Azrad

From: Matan Azrad <matan@mellanox.com>

[ upstream commit 06387be8eae5755e5b9952cd9d5d84ba0e5ad6b5 ]

The encapsulation and decapsulation actions are divided into 2 types:
L2 and L3.
In order to configure L3 xcapsulation actions the user should use both
RAW_DECAP and RAW_ENCAP and setting the appropriated data sizes in
their action configuration structures.

The PMD flow validation wrongly didn't detect the RAW_DECAP
and RAW_ENCAP combination to distinguish between L3_DECAP and L3_ENCAP.
Thus, some xcapsulation related validation failed.
For example, when configuring modify header action before L3_DECAP.

Simplify the xcapsulation defines and fix the L3 xcapsulation detection
using the action configuration data sizes.

By the way, add the hairpin validation in this area.

Fixes: d85c7b5ea59f ("net/mlx5: split hairpin flows")
Fixes: 8ba9eee4ce32 ("net/mlx5: add raw data encap/decap to Direct Verbs")

Signed-off-by: Matan Azrad <matan@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    |   9 +-
 drivers/net/mlx5/mlx5_flow.h    |  34 ++----
 drivers/net/mlx5/mlx5_flow_dv.c | 262 +++++++++++++++++++---------------------
 3 files changed, 144 insertions(+), 161 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 6d96085..914f9a8 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -4139,13 +4139,16 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	} items_tx;
 	struct rte_flow_expand_rss *buf = &expand_buffer.buf;
 	const struct rte_flow_action *p_actions_rx = actions;
-	int ret;
 	uint32_t i;
 	uint32_t flow_size;
 	int hairpin_flow = 0;
 	uint32_t hairpin_id = 0;
 	struct rte_flow_attr attr_tx = { .priority = 0 };
+	int ret = flow_drv_validate(dev, attr, items, p_actions_rx, external,
+				    error);
 
+	if (ret < 0)
+		return NULL;
 	hairpin_flow = flow_check_hairpin_split(dev, attr, actions);
 	if (hairpin_flow > 0) {
 		if (hairpin_flow > MLX5_MAX_SPLIT_ACTIONS) {
@@ -4157,10 +4160,6 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				   &hairpin_id);
 		p_actions_rx = actions_rx.actions;
 	}
-	ret = flow_drv_validate(dev, attr, items, p_actions_rx, external,
-				error);
-	if (ret < 0)
-		goto error_before_flow;
 	flow_size = sizeof(struct rte_flow);
 	rss = flow_get_rss_action(p_actions_rx);
 	if (rss)
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 91594d2..a15ee2c 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -188,20 +188,16 @@ enum mlx5_feature_name {
 #define MLX5_FLOW_ACTION_DEC_TTL (1u << 19)
 #define MLX5_FLOW_ACTION_SET_MAC_SRC (1u << 20)
 #define MLX5_FLOW_ACTION_SET_MAC_DST (1u << 21)
-#define MLX5_FLOW_ACTION_VXLAN_ENCAP (1u << 22)
-#define MLX5_FLOW_ACTION_VXLAN_DECAP (1u << 23)
-#define MLX5_FLOW_ACTION_NVGRE_ENCAP (1u << 24)
-#define MLX5_FLOW_ACTION_NVGRE_DECAP (1u << 25)
-#define MLX5_FLOW_ACTION_RAW_ENCAP (1u << 26)
-#define MLX5_FLOW_ACTION_RAW_DECAP (1u << 27)
-#define MLX5_FLOW_ACTION_INC_TCP_SEQ (1u << 28)
-#define MLX5_FLOW_ACTION_DEC_TCP_SEQ (1u << 29)
-#define MLX5_FLOW_ACTION_INC_TCP_ACK (1u << 30)
-#define MLX5_FLOW_ACTION_DEC_TCP_ACK (1u << 31)
-#define MLX5_FLOW_ACTION_SET_TAG (1ull << 32)
-#define MLX5_FLOW_ACTION_MARK_EXT (1ull << 33)
-#define MLX5_FLOW_ACTION_SET_META (1ull << 34)
-#define MLX5_FLOW_ACTION_METER (1ull << 35)
+#define MLX5_FLOW_ACTION_ENCAP (1u << 22)
+#define MLX5_FLOW_ACTION_DECAP (1u << 23)
+#define MLX5_FLOW_ACTION_INC_TCP_SEQ (1u << 24)
+#define MLX5_FLOW_ACTION_DEC_TCP_SEQ (1u << 25)
+#define MLX5_FLOW_ACTION_INC_TCP_ACK (1u << 26)
+#define MLX5_FLOW_ACTION_DEC_TCP_ACK (1u << 27)
+#define MLX5_FLOW_ACTION_SET_TAG (1ull << 28)
+#define MLX5_FLOW_ACTION_MARK_EXT (1ull << 29)
+#define MLX5_FLOW_ACTION_SET_META (1ull << 30)
+#define MLX5_FLOW_ACTION_METER (1ull << 31)
 
 #define MLX5_FLOW_FATE_ACTIONS \
 	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | \
@@ -211,13 +207,6 @@ enum mlx5_feature_name {
 	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
 	 MLX5_FLOW_ACTION_JUMP)
 
-#define MLX5_FLOW_ENCAP_ACTIONS	(MLX5_FLOW_ACTION_VXLAN_ENCAP | \
-				 MLX5_FLOW_ACTION_NVGRE_ENCAP | \
-				 MLX5_FLOW_ACTION_RAW_ENCAP)
-
-#define MLX5_FLOW_DECAP_ACTIONS	(MLX5_FLOW_ACTION_VXLAN_DECAP | \
-				 MLX5_FLOW_ACTION_NVGRE_DECAP | \
-				 MLX5_FLOW_ACTION_RAW_DECAP)
 
 #define MLX5_FLOW_MODIFY_HDR_ACTIONS (MLX5_FLOW_ACTION_SET_IPV4_SRC | \
 				      MLX5_FLOW_ACTION_SET_IPV4_DST | \
@@ -240,6 +229,9 @@ enum mlx5_feature_name {
 
 #define MLX5_FLOW_VLAN_ACTIONS (MLX5_FLOW_ACTION_OF_POP_VLAN | \
 				MLX5_FLOW_ACTION_OF_PUSH_VLAN)
+
+#define MLX5_FLOW_XCAP_ACTIONS (MLX5_FLOW_ACTION_ENCAP | MLX5_FLOW_ACTION_DECAP)
+
 #ifndef IPPROTO_MPLS
 #define IPPROTO_MPLS 137
 #endif
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index becdf3f..f8933b8 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2009,8 +2009,6 @@ struct field_modify_info modify_tcp[] = {
  *   Holds the actions detected until now.
  * @param[in] action
  *   Pointer to the action structure.
- * @param[in] attr
- *   Pointer to flow attributes
  * @param[out] error
  *   Pointer to error structure.
  *
@@ -2020,29 +2018,22 @@ struct field_modify_info modify_tcp[] = {
 static int
 flow_dv_validate_action_l2_encap(uint64_t action_flags,
 				 const struct rte_flow_action *action,
-				 const struct rte_flow_attr *attr,
 				 struct rte_flow_error *error)
 {
 	if (!(action->conf))
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
 					  "configuration cannot be null");
-	if (action_flags & (MLX5_FLOW_ENCAP_ACTIONS | MLX5_FLOW_DECAP_ACTIONS))
+	if (action_flags & MLX5_FLOW_ACTION_ENCAP)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can only have a single encap or"
-					  " decap action in a flow");
-	if (!attr->transfer && attr->ingress)
-		return rte_flow_error_set(error, ENOTSUP,
-					  RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
-					  NULL,
-					  "encap action not supported for "
-					  "ingress");
+					  "can only have a single encap action "
+					  "in a flow");
 	return 0;
 }
 
 /**
- * Validate the L2 decap action.
+ * Validate a decap action.
  *
  * @param[in] action_flags
  *   Holds the actions detected until now.
@@ -2055,15 +2046,17 @@ struct field_modify_info modify_tcp[] = {
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-flow_dv_validate_action_l2_decap(uint64_t action_flags,
+flow_dv_validate_action_decap(uint64_t action_flags,
 				 const struct rte_flow_attr *attr,
 				 struct rte_flow_error *error)
 {
-	if (action_flags & (MLX5_FLOW_ENCAP_ACTIONS | MLX5_FLOW_DECAP_ACTIONS))
-		return rte_flow_error_set(error, EINVAL,
+	if (action_flags & MLX5_FLOW_XCAP_ACTIONS)
+		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can only have a single encap or"
-					  " decap action in a flow");
+					  action_flags &
+					  MLX5_FLOW_ACTION_DECAP ? "can only "
+					  "have a single decap action" : "decap "
+					  "after encap is not supported");
 	if (action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -2078,62 +2071,21 @@ struct field_modify_info modify_tcp[] = {
 	return 0;
 }
 
-/**
- * Validate the raw encap action.
- *
- * @param[in] action_flags
- *   Holds the actions detected until now.
- * @param[in] action
- *   Pointer to the encap action.
- * @param[in] attr
- *   Pointer to flow attributes
- * @param[out] error
- *   Pointer to error structure.
- *
- * @return
- *   0 on success, a negative errno value otherwise and rte_errno is set.
- */
-static int
-flow_dv_validate_action_raw_encap(uint64_t action_flags,
-				  const struct rte_flow_action *action,
-				  const struct rte_flow_attr *attr,
-				  struct rte_flow_error *error)
-{
-	const struct rte_flow_action_raw_encap *raw_encap =
-		(const struct rte_flow_action_raw_encap *)action->conf;
-	if (!(action->conf))
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, action,
-					  "configuration cannot be null");
-	if (action_flags & MLX5_FLOW_ENCAP_ACTIONS)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can only have a single encap"
-					  " action in a flow");
-	/* encap without preceding decap is not supported for ingress */
-	if (!attr->transfer &&  attr->ingress &&
-	    !(action_flags & MLX5_FLOW_ACTION_RAW_DECAP))
-		return rte_flow_error_set(error, ENOTSUP,
-					  RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
-					  NULL,
-					  "encap action not supported for "
-					  "ingress");
-	if (!raw_encap->size || !raw_encap->data)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, action,
-					  "raw encap data cannot be empty");
-	return 0;
-}
+const struct rte_flow_action_raw_decap empty_decap = {.data = NULL, .size = 0,};
 
 /**
- * Validate the raw decap action.
+ * Validate the raw encap and decap actions.
  *
- * @param[in] action_flags
- *   Holds the actions detected until now.
- * @param[in] action
+ * @param[in] decap
+ *   Pointer to the decap action.
+ * @param[in] encap
  *   Pointer to the encap action.
  * @param[in] attr
  *   Pointer to flow attributes
+ * @param[in/out] action_flags
+ *   Holds the actions detected until now.
+ * @param[out] actions_n
+ *   pointer to the number of actions counter.
  * @param[out] error
  *   Pointer to error structure.
  *
@@ -2141,37 +2093,63 @@ struct field_modify_info modify_tcp[] = {
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-flow_dv_validate_action_raw_decap(uint64_t action_flags,
-				  const struct rte_flow_action *action,
-				  const struct rte_flow_attr *attr,
-				  struct rte_flow_error *error)
+flow_dv_validate_action_raw_encap_decap
+	(const struct rte_flow_action_raw_decap *decap,
+	 const struct rte_flow_action_raw_encap *encap,
+	 const struct rte_flow_attr *attr, uint64_t *action_flags,
+	 int *actions_n, struct rte_flow_error *error)
 {
-	const struct rte_flow_action_raw_decap *decap	= action->conf;
+	int ret;
 
-	if (action_flags & MLX5_FLOW_ENCAP_ACTIONS)
+	if (encap && (!encap->size || !encap->data))
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't have encap action before"
-					  " decap action");
-	if (action_flags & MLX5_FLOW_DECAP_ACTIONS)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can only have a single decap"
-					  " action in a flow");
-	/* decap action is valid on egress only if it is followed by encap */
-	if (attr->egress && decap &&
-	    decap->size > MLX5_ENCAPSULATION_DECISION_SIZE) {
-		return rte_flow_error_set(error, ENOTSUP,
-					  RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
-					  NULL, "decap action not supported"
-					  " for egress");
-	} else if (decap && decap->size > MLX5_ENCAPSULATION_DECISION_SIZE &&
-		   (action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)) {
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION,
-					  NULL,
-					  "can't have decap action "
-					  "after modify action");
+					  "raw encap data cannot be empty");
+	if (decap && encap) {
+		if (decap->size <= MLX5_ENCAPSULATION_DECISION_SIZE &&
+		    encap->size > MLX5_ENCAPSULATION_DECISION_SIZE)
+			/* L3 encap. */
+			decap = NULL;
+		else if (encap->size <=
+			   MLX5_ENCAPSULATION_DECISION_SIZE &&
+			   decap->size >
+			   MLX5_ENCAPSULATION_DECISION_SIZE)
+			/* L3 decap. */
+			encap = NULL;
+		else if (encap->size >
+			   MLX5_ENCAPSULATION_DECISION_SIZE &&
+			   decap->size >
+			   MLX5_ENCAPSULATION_DECISION_SIZE)
+			/* 2 L2 actions: encap and decap. */
+			;
+		else
+			return rte_flow_error_set(error,
+				ENOTSUP,
+				RTE_FLOW_ERROR_TYPE_ACTION,
+				NULL, "unsupported too small "
+				"raw decap and too small raw "
+				"encap combination");
+	}
+	if (decap) {
+		ret = flow_dv_validate_action_decap(*action_flags, attr, error);
+		if (ret < 0)
+			return ret;
+		*action_flags |= MLX5_FLOW_ACTION_DECAP;
+		++(*actions_n);
+	}
+	if (encap) {
+		if (encap->size <= MLX5_ENCAPSULATION_DECISION_SIZE)
+			return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "small raw encap size");
+		if (*action_flags & MLX5_FLOW_ACTION_ENCAP)
+			return rte_flow_error_set(error, EINVAL,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "more than one encap action");
+		*action_flags |= MLX5_FLOW_ACTION_ENCAP;
+		++(*actions_n);
 	}
 	return 0;
 }
@@ -2923,7 +2901,7 @@ struct field_modify_info modify_tcp[] = {
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 					  NULL, "action configuration not set");
-	if (action_flags & MLX5_FLOW_ENCAP_ACTIONS)
+	if (action_flags & MLX5_FLOW_ACTION_ENCAP)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have encap action before"
@@ -4213,6 +4191,9 @@ struct field_modify_info modify_tcp[] = {
 	int actions_n = 0;
 	uint8_t item_ipv6_proto = 0;
 	const struct rte_flow_item *gre_item = NULL;
+	const struct rte_flow_action_raw_decap *decap;
+	const struct rte_flow_action_raw_encap *encap;
+	const struct rte_flow_action_rss *rss;
 	struct rte_flow_item_tcp nic_tcp_mask = {
 		.hdr = {
 			.tcp_flags = 0xFF,
@@ -4222,6 +4203,7 @@ struct field_modify_info modify_tcp[] = {
 	};
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_config *dev_conf = &priv->config;
+	uint16_t queue_index = 0xFFFF;
 
 	if (items == NULL)
 		return -1;
@@ -4547,16 +4529,21 @@ struct field_modify_info modify_tcp[] = {
 							      attr, error);
 			if (ret < 0)
 				return ret;
+			queue_index = ((const struct rte_flow_action_queue *)
+							(actions->conf))->index;
 			action_flags |= MLX5_FLOW_ACTION_QUEUE;
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RSS:
+			rss = actions->conf;
 			ret = mlx5_flow_validate_action_rss(actions,
 							    action_flags, dev,
 							    attr, item_flags,
 							    error);
 			if (ret < 0)
 				return ret;
+			if (rss != NULL && rss->queue_num)
+				queue_index = rss->queue[0];
 			action_flags |= MLX5_FLOW_ACTION_RSS;
 			++actions_n;
 			break;
@@ -4607,45 +4594,44 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
 		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
 			ret = flow_dv_validate_action_l2_encap(action_flags,
-							       actions, attr,
-							       error);
+							       actions, error);
 			if (ret < 0)
 				return ret;
-			action_flags |= actions->type ==
-					RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP ?
-					MLX5_FLOW_ACTION_VXLAN_ENCAP :
-					MLX5_FLOW_ACTION_NVGRE_ENCAP;
+			action_flags |= MLX5_FLOW_ACTION_ENCAP;
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_VXLAN_DECAP:
 		case RTE_FLOW_ACTION_TYPE_NVGRE_DECAP:
-			ret = flow_dv_validate_action_l2_decap(action_flags,
-							       attr, error);
+			ret = flow_dv_validate_action_decap(action_flags, attr,
+							    error);
 			if (ret < 0)
 				return ret;
-			action_flags |= actions->type ==
-					RTE_FLOW_ACTION_TYPE_VXLAN_DECAP ?
-					MLX5_FLOW_ACTION_VXLAN_DECAP :
-					MLX5_FLOW_ACTION_NVGRE_DECAP;
+			action_flags |= MLX5_FLOW_ACTION_DECAP;
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
-			ret = flow_dv_validate_action_raw_encap(action_flags,
-								actions, attr,
-								error);
+			ret = flow_dv_validate_action_raw_encap_decap
+				(NULL, actions->conf, attr, &action_flags,
+				 &actions_n, error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_FLOW_ACTION_RAW_ENCAP;
-			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
-			ret = flow_dv_validate_action_raw_decap(action_flags,
-								actions, attr,
-								error);
+			decap = actions->conf;
+			while ((++actions)->type == RTE_FLOW_ACTION_TYPE_VOID)
+				;
+			if (actions->type != RTE_FLOW_ACTION_TYPE_RAW_ENCAP) {
+				encap = NULL;
+				actions--;
+			} else {
+				encap = actions->conf;
+			}
+			ret = flow_dv_validate_action_raw_encap_decap
+					   (decap ? decap : &empty_decap, encap,
+					    attr, &action_flags, &actions_n,
+					    error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_FLOW_ACTION_RAW_DECAP;
-			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
 		case RTE_FLOW_ACTION_TYPE_SET_MAC_DST:
@@ -4847,6 +4833,22 @@ struct field_modify_info modify_tcp[] = {
 						  actions,
 						  "no fate action is found");
 	}
+	/* Continue validation for Xcap actions.*/
+	if ((action_flags & MLX5_FLOW_XCAP_ACTIONS) && (queue_index == 0xFFFF ||
+	    mlx5_rxq_get_type(dev, queue_index) != MLX5_RXQ_TYPE_HAIRPIN)) {
+		if ((action_flags & MLX5_FLOW_XCAP_ACTIONS) ==
+		    MLX5_FLOW_XCAP_ACTIONS)
+			return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL, "encap and decap "
+						  "combination aren't supported");
+		if (!attr->transfer && attr->ingress && (action_flags &
+							MLX5_FLOW_ACTION_ENCAP))
+			return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL, "encap is not supported"
+						  " for ingress traffic");
+	}
 	return 0;
 }
 
@@ -6996,10 +6998,7 @@ struct field_modify_info modify_tcp[] = {
 				return -rte_errno;
 			dev_flow->dv.actions[actions_n++] =
 				dev_flow->dv.encap_decap->verbs_action;
-			action_flags |= actions->type ==
-					RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP ?
-					MLX5_FLOW_ACTION_VXLAN_ENCAP :
-					MLX5_FLOW_ACTION_NVGRE_ENCAP;
+			action_flags |= MLX5_FLOW_ACTION_ENCAP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_VXLAN_DECAP:
 		case RTE_FLOW_ACTION_TYPE_NVGRE_DECAP:
@@ -7009,14 +7008,11 @@ struct field_modify_info modify_tcp[] = {
 				return -rte_errno;
 			dev_flow->dv.actions[actions_n++] =
 				dev_flow->dv.encap_decap->verbs_action;
-			action_flags |= actions->type ==
-					RTE_FLOW_ACTION_TYPE_VXLAN_DECAP ?
-					MLX5_FLOW_ACTION_VXLAN_DECAP :
-					MLX5_FLOW_ACTION_NVGRE_DECAP;
+			action_flags |= MLX5_FLOW_ACTION_DECAP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
 			/* Handle encap with preceding decap. */
-			if (action_flags & MLX5_FLOW_ACTION_RAW_DECAP) {
+			if (action_flags & MLX5_FLOW_ACTION_DECAP) {
 				if (flow_dv_create_action_raw_encap
 					(dev, actions, dev_flow, attr, error))
 					return -rte_errno;
@@ -7031,15 +7027,11 @@ struct field_modify_info modify_tcp[] = {
 				dev_flow->dv.actions[actions_n++] =
 					dev_flow->dv.encap_decap->verbs_action;
 			}
-			action_flags |= MLX5_FLOW_ACTION_RAW_ENCAP;
+			action_flags |= MLX5_FLOW_ACTION_ENCAP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
-			/* Check if this decap is followed by encap. */
-			for (; action->type != RTE_FLOW_ACTION_TYPE_END &&
-			       action->type != RTE_FLOW_ACTION_TYPE_RAW_ENCAP;
-			       action++) {
-			}
-			/* Handle decap only if it isn't followed by encap. */
+			while ((++action)->type == RTE_FLOW_ACTION_TYPE_VOID)
+				;
 			if (action->type != RTE_FLOW_ACTION_TYPE_RAW_ENCAP) {
 				if (flow_dv_create_action_l2_decap
 				    (dev, dev_flow, attr->transfer, error))
@@ -7048,7 +7040,7 @@ struct field_modify_info modify_tcp[] = {
 					dev_flow->dv.encap_decap->verbs_action;
 			}
 			/* If decap is followed by encap, handle it at encap. */
-			action_flags |= MLX5_FLOW_ACTION_RAW_DECAP;
+			action_flags |= MLX5_FLOW_ACTION_DECAP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_JUMP:
 			jump_data = action->conf;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 06/11] net/mlx5: fix layer validation with decapsulation
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
                   ` (4 preceding siblings ...)
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 05/11] net/mlx5: fix encap/decap validation Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 07/11] net/mlx5: fix layer type in header modify action Suanming Mou
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable

[ upstream commit e505ac7f91b59e1165c143f14b208885658d2f94 ]

Currently, the flow validate function only validate the outermost layer
with the header modify actions. If there is decapsulation action before
the header modify action, the validation should choose the inner layer
for validation.

Add decapsulation check when validate with the header modify actions.
Choose the inner layer once there is decapsulation action.

Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index f8933b8..3362873 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2965,10 +2965,14 @@ struct field_modify_info modify_tcp[] = {
 				    struct rte_flow_error *error)
 {
 	int ret = 0;
+	uint64_t layer;
 
 	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
 	if (!ret) {
-		if (!(item_flags & MLX5_FLOW_LAYER_L3_IPV4))
+		layer = (action_flags & MLX5_FLOW_ACTION_DECAP) ?
+				 MLX5_FLOW_LAYER_INNER_L3_IPV4 :
+				 MLX5_FLOW_LAYER_OUTER_L3_IPV4;
+		if (!(item_flags & layer))
 			return rte_flow_error_set(error, EINVAL,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  NULL,
@@ -2999,10 +3003,14 @@ struct field_modify_info modify_tcp[] = {
 				    struct rte_flow_error *error)
 {
 	int ret = 0;
+	uint64_t layer;
 
 	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
 	if (!ret) {
-		if (!(item_flags & MLX5_FLOW_LAYER_L3_IPV6))
+		layer = (action_flags & MLX5_FLOW_ACTION_DECAP) ?
+				 MLX5_FLOW_LAYER_INNER_L3_IPV6 :
+				 MLX5_FLOW_LAYER_OUTER_L3_IPV6;
+		if (!(item_flags & layer))
 			return rte_flow_error_set(error, EINVAL,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  NULL,
@@ -3033,10 +3041,14 @@ struct field_modify_info modify_tcp[] = {
 				  struct rte_flow_error *error)
 {
 	int ret = 0;
+	uint64_t layer;
 
 	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
 	if (!ret) {
-		if (!(item_flags & MLX5_FLOW_LAYER_L4))
+		layer = (action_flags & MLX5_FLOW_ACTION_DECAP) ?
+				 MLX5_FLOW_LAYER_INNER_L4 :
+				 MLX5_FLOW_LAYER_OUTER_L4;
+		if (!(item_flags & layer))
 			return rte_flow_error_set(error, EINVAL,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  NULL, "no transport layer "
@@ -3068,10 +3080,14 @@ struct field_modify_info modify_tcp[] = {
 				       struct rte_flow_error *error)
 {
 	int ret = 0;
+	uint64_t layer;
 
 	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
 	if (!ret) {
-		if (!(item_flags & MLX5_FLOW_LAYER_OUTER_L4_TCP))
+		layer = (action_flags & MLX5_FLOW_ACTION_DECAP) ?
+				 MLX5_FLOW_LAYER_INNER_L4_TCP :
+				 MLX5_FLOW_LAYER_OUTER_L4_TCP;
+		if (!(item_flags & layer))
 			return rte_flow_error_set(error, EINVAL,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  NULL, "no TCP item in"
@@ -3113,10 +3129,14 @@ struct field_modify_info modify_tcp[] = {
 				       struct rte_flow_error *error)
 {
 	int ret = 0;
+	uint64_t layer;
 
 	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
 	if (!ret) {
-		if (!(item_flags & MLX5_FLOW_LAYER_OUTER_L4_TCP))
+		layer = (action_flags & MLX5_FLOW_ACTION_DECAP) ?
+				 MLX5_FLOW_LAYER_INNER_L4_TCP :
+				 MLX5_FLOW_LAYER_OUTER_L4_TCP;
+		if (!(item_flags & layer))
 			return rte_flow_error_set(error, EINVAL,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  NULL, "no TCP item in"
@@ -3157,10 +3177,14 @@ struct field_modify_info modify_tcp[] = {
 				   struct rte_flow_error *error)
 {
 	int ret = 0;
+	uint64_t layer;
 
 	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
 	if (!ret) {
-		if (!(item_flags & MLX5_FLOW_LAYER_L3))
+		layer = (action_flags & MLX5_FLOW_ACTION_DECAP) ?
+				 MLX5_FLOW_LAYER_INNER_L3 :
+				 MLX5_FLOW_LAYER_OUTER_L3;
+		if (!(item_flags & layer))
 			return rte_flow_error_set(error, EINVAL,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  NULL,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 07/11] net/mlx5: fix layer type in header modify action
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
                   ` (5 preceding siblings ...)
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 06/11] net/mlx5: fix layer validation with decapsulation Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 08/11] net/mlx5: fix layer flags missing in metadata Suanming Mou
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable

[ upstream commit 04233f36c712ea35fe0f1d02c5c6f323a28ec588 ]

Currently, for header modify actions after tunnel decapsulation, the
header modify actions will still select the old outermost layer type.
It will cause header modify actions get the wrong layer type.

Add the tunnel decap flag to indicate the layer type to get for header
modify actions.

Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 68 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 57 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3362873..4df0fc2 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -82,19 +82,55 @@
  *   Pointer to item specification.
  * @param[out] attr
  *   Pointer to flow attributes structure.
+ * @param[in] tunnel_decap
+ *   Whether action is after tunnel decapsulation.
  */
 static void
-flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr)
+flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr,
+		  bool tunnel_decap)
 {
 	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
+		uint8_t next_protocol = 0xff;
+
 		switch (item->type) {
+		case RTE_FLOW_ITEM_TYPE_GRE:
+		case RTE_FLOW_ITEM_TYPE_NVGRE:
+		case RTE_FLOW_ITEM_TYPE_VXLAN:
+		case RTE_FLOW_ITEM_TYPE_VXLAN_GPE:
+		case RTE_FLOW_ITEM_TYPE_GENEVE:
+		case RTE_FLOW_ITEM_TYPE_MPLS:
+			if (tunnel_decap)
+				attr->attr = 0;
+			break;
 		case RTE_FLOW_ITEM_TYPE_IPV4:
 			if (!attr->ipv6)
 				attr->ipv4 = 1;
+			if (item->mask != NULL &&
+			    ((const struct rte_flow_item_ipv4 *)
+			    item->mask)->hdr.next_proto_id)
+				next_protocol =
+				    ((const struct rte_flow_item_ipv4 *)
+				      (item->spec))->hdr.next_proto_id &
+				    ((const struct rte_flow_item_ipv4 *)
+				      (item->mask))->hdr.next_proto_id;
+			if ((next_protocol == IPPROTO_IPIP ||
+			    next_protocol == IPPROTO_IPV6) && tunnel_decap)
+				attr->attr = 0;
 			break;
 		case RTE_FLOW_ITEM_TYPE_IPV6:
 			if (!attr->ipv4)
 				attr->ipv6 = 1;
+			if (item->mask != NULL &&
+			    ((const struct rte_flow_item_ipv6 *)
+			    item->mask)->hdr.proto)
+				next_protocol =
+				    ((const struct rte_flow_item_ipv6 *)
+				      (item->spec))->hdr.proto &
+				    ((const struct rte_flow_item_ipv6 *)
+				      (item->mask))->hdr.proto;
+			if ((next_protocol == IPPROTO_IPIP ||
+			    next_protocol == IPPROTO_IPV6) && tunnel_decap)
+				attr->attr = 0;
 			break;
 		case RTE_FLOW_ITEM_TYPE_UDP:
 			if (!attr->tcp)
@@ -599,6 +635,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] tunnel_decap
+ *   Whether action is after tunnel decapsulation.
  * @param[out] error
  *   Pointer to the error structure.
  *
@@ -610,7 +648,7 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr,
+			 union flow_dv_attr *attr, bool tunnel_decap,
 			 struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_tp *conf =
@@ -623,7 +661,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr);
+		flow_dv_attr_init(items, attr, tunnel_decap);
 	if (attr->udp) {
 		memset(&udp, 0, sizeof(udp));
 		memset(&udp_mask, 0, sizeof(udp_mask));
@@ -673,6 +711,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] tunnel_decap
+ *   Whether action is after tunnel decapsulation.
  * @param[out] error
  *   Pointer to the error structure.
  *
@@ -684,7 +724,7 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr,
+			 union flow_dv_attr *attr, bool tunnel_decap,
 			 struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_ttl *conf =
@@ -697,7 +737,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr);
+		flow_dv_attr_init(items, attr, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -733,6 +773,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] tunnel_decap
+ *   Whether action is after tunnel decapsulation.
  * @param[out] error
  *   Pointer to the error structure.
  *
@@ -743,7 +785,7 @@ struct field_modify_info modify_tcp[] = {
 flow_dv_convert_action_modify_dec_ttl
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr,
+			 union flow_dv_attr *attr, bool tunnel_decap,
 			 struct rte_flow_error *error)
 {
 	struct rte_flow_item item;
@@ -754,7 +796,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr);
+		flow_dv_attr_init(items, attr, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -7130,7 +7172,8 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
 			if (flow_dv_convert_action_modify_tp
 					(mhdr_res, actions, items,
-					 &flow_attr, error))
+					 &flow_attr, !!(action_flags &
+					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= actions->type ==
 					RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
@@ -7139,14 +7182,17 @@ struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
 			if (flow_dv_convert_action_modify_dec_ttl
-					(mhdr_res, items, &flow_attr, error))
+					(mhdr_res, items, &flow_attr,
+					 !!(action_flags &
+					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_DEC_TTL;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_TTL:
 			if (flow_dv_convert_action_modify_ttl
-					(mhdr_res, actions, items,
-					 &flow_attr, error))
+					(mhdr_res, actions, items, &flow_attr,
+					 !!(action_flags &
+					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
 			break;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 08/11] net/mlx5: fix layer flags missing in metadata
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
                   ` (6 preceding siblings ...)
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 07/11] net/mlx5: fix layer type in header modify action Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 09/11] net/mlx5: fix match information in meter Suanming Mou
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable

[ upstream commit bc42413bb9c0b767e40c5a52e529277c06608a3b ]

Metadata suffix subflow inherits the RSS needed hash_fields from the
prefix subflow as the suffix subflow only has the tag match item unable
to generate the full original hash_fields for RSS action.

Unfortunately, hash_fields will only be generated if flow has RSS
action. So it means the prefix flow won't generate the hash_fields as
the RSS action has been split to the suffix flow.

Copy the layer flags from prefix subflow to suffix subflow to help the
suffix subflow to generate the correct hash_fields itself.

Fixes: 71e254bc0294 ("net/mlx5: split Rx flows to provide metadata copy")

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 67 +++++++++++++++++++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow_dv.c |  6 +++-
 2 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 914f9a8..03ee207 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2708,6 +2708,43 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 }
 
 /**
+ *  Get layer flags from the prefix flow.
+ *
+ *  Some flows may be split to several subflows, the prefix subflow gets the
+ *  match items and the suffix sub flow gets the actions.
+ *  Some actions need the user defined match item flags to get the detail for
+ *  the action.
+ *  This function helps the suffix flow to get the item layer flags from prefix
+ *  subflow.
+ *
+ * @param[in] dev_flow
+ *   Pointer the created preifx subflow.
+ *
+ * @return
+ *   The layers get from prefix subflow.
+ */
+static inline uint64_t
+flow_get_prefix_layer_flags(struct mlx5_flow *dev_flow)
+{
+	uint64_t layers = 0;
+
+	/* If no decap actions, use the layers directly. */
+	if (!(dev_flow->actions & MLX5_FLOW_ACTION_DECAP))
+		return dev_flow->layers;
+	/* Convert L3 layers with decap action. */
+	if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L3_IPV4)
+		layers |= MLX5_FLOW_LAYER_OUTER_L3_IPV4;
+	else if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L3_IPV6)
+		layers |= MLX5_FLOW_LAYER_OUTER_L3_IPV6;
+	/* Convert L4 layers with decap action.  */
+	if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L4_TCP)
+		layers |= MLX5_FLOW_LAYER_OUTER_L4_TCP;
+	else if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L4_UDP)
+		layers |= MLX5_FLOW_LAYER_OUTER_L4_UDP;
+	return layers;
+}
+
+/**
  * Get QUEUE/RSS action from the action list.
  *
  * @param[in] actions
@@ -3386,6 +3423,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   Parent flow structure pointer.
  * @param[in, out] sub_flow
  *   Pointer to return the created subflow, may be NULL.
+ * @param[in] prefix_layers
+ *   Prefix subflow layers, may be 0.
  * @param[in] attr
  *   Flow rule attributes.
  * @param[in] items
@@ -3403,6 +3442,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 flow_create_split_inner(struct rte_eth_dev *dev,
 			struct rte_flow *flow,
 			struct mlx5_flow **sub_flow,
+			uint64_t prefix_layers,
 			const struct rte_flow_attr *attr,
 			const struct rte_flow_item items[],
 			const struct rte_flow_action actions[],
@@ -3417,6 +3457,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	dev_flow->external = external;
 	/* Subflow object was created, we must include one in the list. */
 	LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next);
+	/*
+	 * If dev_flow is as one of the suffix flow, some actions in suffix
+	 * flow may need some user defined item layer flags.
+	 */
+	if (prefix_layers)
+		dev_flow->layers = prefix_layers;
 	if (sub_flow)
 		*sub_flow = dev_flow;
 	return flow_drv_translate(dev, dev_flow, attr, items, actions, error);
@@ -3748,8 +3794,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (!config->dv_flow_en ||
 	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
 	    !mlx5_flow_ext_mreg_supported(dev))
-		return flow_create_split_inner(dev, flow, NULL, attr, items,
-					       actions, external, error);
+		return flow_create_split_inner(dev, flow, NULL, 0,
+					       attr, items, actions, external,
+					       error);
 	actions_n = flow_parse_qrss_action(actions, &qrss);
 	if (qrss) {
 		/* Exclude hairpin flows from splitting. */
@@ -3830,9 +3877,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 	}
 	/* Add the unmodified original or prefix subflow. */
-	ret = flow_create_split_inner(dev, flow, &dev_flow, attr, items,
-				      ext_actions ? ext_actions : actions,
-				      external, error);
+	ret = flow_create_split_inner(dev, flow, &dev_flow, 0, attr,
+				      items, ext_actions ? ext_actions :
+				      actions, external, error);
 	if (ret < 0)
 		goto exit;
 	assert(dev_flow);
@@ -3866,7 +3913,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				.type = RTE_FLOW_ACTION_TYPE_END,
 			},
 		};
-		uint64_t hash_fields = dev_flow->hash_fields;
+		uint64_t layers = flow_get_prefix_layer_flags(dev_flow);
 
 		/*
 		 * Configure the tag item only if there is no meter subflow.
@@ -3893,14 +3940,13 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		}
 		dev_flow = NULL;
 		/* Add suffix subflow to execute Q/RSS. */
-		ret = flow_create_split_inner(dev, flow, &dev_flow,
+		ret = flow_create_split_inner(dev, flow, &dev_flow, layers,
 					      &q_attr, mtr_sfx ? items :
 					      q_items, q_actions,
 					      external, error);
 		if (ret < 0)
 			goto exit;
 		assert(dev_flow);
-		dev_flow->hash_fields = hash_fields;
 	}
 
 exit:
@@ -3989,8 +4035,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 		}
 		/* Add the prefix subflow. */
-		ret = flow_create_split_inner(dev, flow, &dev_flow, attr, items,
-						  pre_actions, external, error);
+		ret = flow_create_split_inner(dev, flow, &dev_flow, 0, attr,
+					      items, pre_actions, external,
+					      error);
 		if (ret) {
 			ret = -rte_errno;
 			goto exit;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 4df0fc2..f5929bf 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7479,7 +7479,11 @@ struct field_modify_info modify_tcp[] = {
 	}
 	assert(!flow_dv_check_valid_spec(matcher.mask.buf,
 					 dev_flow->dv.value.buf));
-	dev_flow->layers = item_flags;
+	/*
+	 * Layers may be already initialized from prefix flow if this dev_flow
+	 * is the suffix flow.
+	 */
+	dev_flow->layers |= item_flags;
 	/* Register matcher. */
 	matcher.crc = rte_raw_cksum((const void *)matcher.mask.buf,
 				    matcher.mask.size);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 09/11] net/mlx5: fix match information in meter
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
                   ` (7 preceding siblings ...)
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 08/11] net/mlx5: fix layer flags missing in metadata Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 10/11] net/mlx5: fix VLAN actions " Suanming Mou
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable

[ upstream commit 6e77151286b2f551ae30aac0f251ed067f1ec011 ]

As meter flows are split into three subflows each, the prefix subflow
with meter action color the packet, the meter subflow filters out the
colored packets, the suffix subflow applies all the remaining actions
to the passed packets.

Currently, all the user defined items are matched in the prefix flow.
Flow id tag match item is the only item added to the meter suffix
subflow. Some of the remaining actions to be applied in the suffix
subflow require more information in the match item, or the suffix
subflow will not be created successfully.

Actions require the L3/L4 type in the match items as below:
RTE_FLOW_ACTION_TYPE_SET_TP_SRC
RTE_FLOW_ACTION_TYPE_SET_TP_DST
RTE_FLOW_ACTION_TYPE_DEC_TTL
RTE_FLOW_ACTION_TYPE_SET_TTL
RTE_FLOW_ACTION_TYPE_RSS
RTE_FLOW_ACTION_TYPE_QUEUE

Inherit the match item flags from meter prefix subflow to make actions
in suffix subflow get sufficient information.

Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 11 ++++++---
 drivers/net/mlx5/mlx5_flow_dv.c | 53 ++++++++++++++++++++++++++++++-----------
 2 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 03ee207..365b832 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3758,6 +3758,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   Pointer to Ethernet device.
  * @param[in] flow
  *   Parent flow structure pointer.
+ * @param[in] prefix_layers
+ *   Prefix flow layer flags.
  * @param[in] attr
  *   Flow rule attributes.
  * @param[in] items
@@ -3774,6 +3776,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 static int
 flow_create_split_metadata(struct rte_eth_dev *dev,
 			   struct rte_flow *flow,
+			   uint64_t prefix_layers,
 			   const struct rte_flow_attr *attr,
 			   const struct rte_flow_item items[],
 			   const struct rte_flow_action actions[],
@@ -3794,7 +3797,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (!config->dv_flow_en ||
 	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
 	    !mlx5_flow_ext_mreg_supported(dev))
-		return flow_create_split_inner(dev, flow, NULL, 0,
+		return flow_create_split_inner(dev, flow, NULL, prefix_layers,
 					       attr, items, actions, external,
 					       error);
 	actions_n = flow_parse_qrss_action(actions, &qrss);
@@ -3877,7 +3880,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 	}
 	/* Add the unmodified original or prefix subflow. */
-	ret = flow_create_split_inner(dev, flow, &dev_flow, 0, attr,
+	ret = flow_create_split_inner(dev, flow, &dev_flow, prefix_layers, attr,
 				      items, ext_actions ? ext_actions :
 				      actions, external, error);
 	if (ret < 0)
@@ -4072,7 +4075,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				 MLX5_FLOW_TABLE_LEVEL_SUFFIX;
 	}
 	/* Add the prefix subflow. */
-	ret = flow_create_split_metadata(dev, flow, &sfx_attr,
+	ret = flow_create_split_metadata(dev, flow, dev_flow ?
+					 flow_get_prefix_layer_flags(dev_flow) :
+					 0, &sfx_attr,
 					 sfx_items ? sfx_items : items,
 					 sfx_actions ? sfx_actions : actions,
 					 external, error);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index f5929bf..a5824dc 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -82,16 +82,35 @@
  *   Pointer to item specification.
  * @param[out] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  */
 static void
 flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr,
-		  bool tunnel_decap)
+		  struct mlx5_flow *dev_flow, bool tunnel_decap)
 {
+	/*
+	 * If layers is already initialized, it means this dev_flow is the
+	 * suffix flow, the layers flags is set by the prefix flow. Need to
+	 * use the layer flags from prefix flow as the suffix flow may not
+	 * have the user defined items as the flow is split.
+	 */
+	if (dev_flow->layers) {
+		if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L3_IPV4)
+			attr->ipv4 = 1;
+		else if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L3_IPV6)
+			attr->ipv6 = 1;
+		if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L4_TCP)
+			attr->tcp = 1;
+		else if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L4_UDP)
+			attr->udp = 1;
+		attr->valid = 1;
+		return;
+	}
 	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
 		uint8_t next_protocol = 0xff;
-
 		switch (item->type) {
 		case RTE_FLOW_ITEM_TYPE_GRE:
 		case RTE_FLOW_ITEM_TYPE_NVGRE:
@@ -635,6 +654,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -648,8 +669,8 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_tp *conf =
 		(const struct rte_flow_action_set_tp *)(action->conf);
@@ -661,7 +682,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->udp) {
 		memset(&udp, 0, sizeof(udp));
 		memset(&udp_mask, 0, sizeof(udp_mask));
@@ -711,6 +732,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -724,8 +747,8 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_ttl *conf =
 		(const struct rte_flow_action_set_ttl *)(action->conf);
@@ -737,7 +760,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -773,6 +796,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -785,8 +810,8 @@ struct field_modify_info modify_tcp[] = {
 flow_dv_convert_action_modify_dec_ttl
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	struct rte_flow_item item;
 	struct rte_flow_item_ipv4 ipv4;
@@ -796,7 +821,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -7172,7 +7197,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
 			if (flow_dv_convert_action_modify_tp
 					(mhdr_res, actions, items,
-					 &flow_attr, !!(action_flags &
+					 &flow_attr, dev_flow, !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= actions->type ==
@@ -7182,7 +7207,7 @@ struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
 			if (flow_dv_convert_action_modify_dec_ttl
-					(mhdr_res, items, &flow_attr,
+					(mhdr_res, items, &flow_attr, dev_flow,
 					 !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
@@ -7191,7 +7216,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_SET_TTL:
 			if (flow_dv_convert_action_modify_ttl
 					(mhdr_res, actions, items, &flow_attr,
-					 !!(action_flags &
+					 dev_flow, !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 10/11] net/mlx5: fix VLAN actions in meter
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
                   ` (8 preceding siblings ...)
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 09/11] net/mlx5: fix match information in meter Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 11/11] net/mlx5: fix metadata split with encap action Suanming Mou
  2020-02-28 11:17 ` [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Luca Boccassi
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable

[ upstream commit 50f576d657d78e87a3eac597353d236969969c17 ]

Meter suffix subflow only has the port id and tag match item, if VLAN
push and set VLAN id actions exist in the suffix subflow, the user
defined VLAN items is required for the actions to set a correct VLAN
id.

Currently, the VLAN item stays in the meter prefix subflow. Without
the VLAN item, VLAN id or pcp will not be inherited.

Actions require the VLAN item as below:
RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN
RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID

Add a private VLAN item to copy the user defined VLAN item to the meter
suffix subflow, so the suffix subflow will have the chance to get the
correct original VLAN id and pcp value from the VLAN item.

Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 153 ++++++++++++++++++----------------------
 drivers/net/mlx5/mlx5_flow.h    |   4 ++
 drivers/net/mlx5/mlx5_flow_dv.c |  14 ++--
 3 files changed, 82 insertions(+), 89 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 365b832..17ce2e3 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2645,26 +2645,6 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 }
 
 /**
- * Get port id item from the item list.
- *
- * @param[in] item
- *   Pointer to the list of items.
- *
- * @return
- *   Pointer to the port id item if exist, else return NULL.
- */
-static const struct rte_flow_item *
-find_port_id_item(const struct rte_flow_item *item)
-{
-	assert(item);
-	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
-		if (item->type == RTE_FLOW_ITEM_TYPE_PORT_ID)
-			return item;
-	}
-	return NULL;
-}
-
-/**
  * Get RSS action from the action list.
  *
  * @param[in] actions
@@ -3482,6 +3462,10 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *
  * @param dev
  *   Pointer to Ethernet device.
+ * @param[in] items
+ *   Pattern specification (list terminated by the END pattern item).
+ * @param[out] sfx_items
+ *   Suffix flow match items (list terminated by the END pattern item).
  * @param[in] actions
  *   Associated actions (list terminated by the END action).
  * @param[out] actions_sfx
@@ -3498,70 +3482,60 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  */
 static int
 flow_meter_split_prep(struct rte_eth_dev *dev,
+		 const struct rte_flow_item items[],
+		 struct rte_flow_item sfx_items[],
 		 const struct rte_flow_action actions[],
 		 struct rte_flow_action actions_sfx[],
 		 struct rte_flow_action actions_pre[])
 {
 	struct rte_flow_action *tag_action = NULL;
+	struct rte_flow_item *tag_item;
 	struct mlx5_rte_flow_action_set_tag *set_tag;
 	struct rte_flow_error error;
 	const struct rte_flow_action_raw_encap *raw_encap;
 	const struct rte_flow_action_raw_decap *raw_decap;
+	struct mlx5_rte_flow_item_tag *tag_spec;
+	struct mlx5_rte_flow_item_tag *tag_mask;
 	uint32_t tag_id;
+	bool copy_vlan = false;
 
 	/* Prepare the actions for prefix and suffix flow. */
 	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
+		struct rte_flow_action **action_cur = NULL;
+
 		switch (actions->type) {
 		case RTE_FLOW_ACTION_TYPE_METER:
 			/* Add the extra tag action first. */
 			tag_action = actions_pre;
 			tag_action->type = MLX5_RTE_FLOW_ACTION_TYPE_TAG;
 			actions_pre++;
-			memcpy(actions_pre, actions,
-			       sizeof(struct rte_flow_action));
-			actions_pre++;
+			action_cur = &actions_pre;
 			break;
 		case RTE_FLOW_ACTION_TYPE_VXLAN_DECAP:
 		case RTE_FLOW_ACTION_TYPE_NVGRE_DECAP:
-			memcpy(actions_pre, actions,
-			       sizeof(struct rte_flow_action));
-			actions_pre++;
+			action_cur = &actions_pre;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
 			raw_encap = actions->conf;
-			if (raw_encap->size >
-			    (sizeof(struct rte_flow_item_eth) +
-			     sizeof(struct rte_flow_item_ipv4))) {
-				memcpy(actions_sfx, actions,
-				       sizeof(struct rte_flow_action));
-				actions_sfx++;
-			} else {
-				rte_memcpy(actions_pre, actions,
-					   sizeof(struct rte_flow_action));
-				actions_pre++;
-			}
+			if (raw_encap->size < MLX5_ENCAPSULATION_DECISION_SIZE)
+				action_cur = &actions_pre;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
 			raw_decap = actions->conf;
-			/* Size 0 decap means 50 bytes as vxlan decap. */
-			if (raw_decap->size && (raw_decap->size <
-			    (sizeof(struct rte_flow_item_eth) +
-			     sizeof(struct rte_flow_item_ipv4)))) {
-				memcpy(actions_sfx, actions,
-				       sizeof(struct rte_flow_action));
-				actions_sfx++;
-			} else {
-				rte_memcpy(actions_pre, actions,
-					   sizeof(struct rte_flow_action));
-				actions_pre++;
-			}
+			if (raw_decap->size > MLX5_ENCAPSULATION_DECISION_SIZE)
+				action_cur = &actions_pre;
+			break;
+		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
+		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
+			copy_vlan = true;
 			break;
 		default:
-			memcpy(actions_sfx, actions,
-				sizeof(struct rte_flow_action));
-			actions_sfx++;
 			break;
 		}
+		if (!action_cur)
+			action_cur = &actions_sfx;
+		memcpy(*action_cur, actions, sizeof(struct rte_flow_action));
+		(*action_cur)++;
 	}
 	/* Add end action to the actions. */
 	actions_sfx->type = RTE_FLOW_ACTION_TYPE_END;
@@ -3577,6 +3551,42 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	set_tag->data = tag_id << MLX5_MTR_COLOR_BITS;
 	assert(tag_action);
 	tag_action->conf = set_tag;
+	/* 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 = MLX5_RTE_FLOW_ITEM_TYPE_VLAN;
+				sfx_items++;
+			}
+			break;
+		default:
+			break;
+		}
+	}
+	sfx_items->type = RTE_FLOW_ITEM_TYPE_END;
+	sfx_items++;
+	tag_spec = (struct mlx5_rte_flow_item_tag *)sfx_items;
+	tag_spec->data = tag_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;
+	tag_item->type = MLX5_RTE_FLOW_ITEM_TYPE_TAG;
+	tag_item->spec = tag_spec;
+	tag_item->last = NULL;
+	tag_item->mask = tag_mask;
 	return tag_id;
 }
 
@@ -4002,7 +4012,6 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	struct rte_flow_action *sfx_actions = NULL;
 	struct rte_flow_action *pre_actions = NULL;
 	struct rte_flow_item *sfx_items = NULL;
-	const  struct rte_flow_item *sfx_port_id_item;
 	struct mlx5_flow *dev_flow = NULL;
 	struct rte_flow_attr sfx_attr = *attr;
 	uint32_t mtr = 0;
@@ -4015,13 +4024,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (priv->mtr_en)
 		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
+			   sizeof(struct mlx5_rte_flow_action_set_tag);
+		/* tag, vlan, port id, end. */
+#define METER_SUFFIX_ITEM 4
 		item_size = sizeof(struct rte_flow_item) * METER_SUFFIX_ITEM +
 			    sizeof(struct mlx5_rte_flow_item_tag) * 2;
 		sfx_actions = rte_zmalloc(__func__, (act_size + item_size), 0);
@@ -4030,9 +4037,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  NULL, "no memory to split "
 						  "meter flow");
+		sfx_items = (struct rte_flow_item *)((char *)sfx_actions +
+			     act_size);
 		pre_actions = sfx_actions + actions_n;
-		mtr_tag_id = flow_meter_split_prep(dev, actions, sfx_actions,
-						     pre_actions);
+		mtr_tag_id = flow_meter_split_prep(dev, items, sfx_items,
+						   actions, sfx_actions,
+						   pre_actions);
 		if (!mtr_tag_id) {
 			ret = -rte_errno;
 			goto exit;
@@ -4046,29 +4056,6 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 		}
 		dev_flow->mtr_flow_id = mtr_tag_id;
-		/* Prepare the suffix flow match pattern. */
-		sfx_items = (struct rte_flow_item *)((char *)sfx_actions +
-			     act_size);
-		tag_spec = (struct mlx5_rte_flow_item_tag *)(sfx_items +
-			    METER_SUFFIX_ITEM);
-		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 = tag_mask;
-		sfx_items++;
-		sfx_port_id_item = find_port_id_item(items);
-		if (sfx_port_id_item) {
-			memcpy(sfx_items, sfx_port_id_item,
-			       sizeof(*sfx_items));
-			sfx_items++;
-		}
-		sfx_items->type = RTE_FLOW_ITEM_TYPE_END;
-		sfx_items -= sfx_port_id_item ? 2 : 1;
 		/* Setting the sfx group atrr. */
 		sfx_attr.group = sfx_attr.transfer ?
 				(MLX5_FLOW_TABLE_LEVEL_SUFFIX - 1) :
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index a15ee2c..2353e18 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -33,6 +33,7 @@ enum mlx5_rte_flow_item_type {
 	MLX5_RTE_FLOW_ITEM_TYPE_END = INT_MIN,
 	MLX5_RTE_FLOW_ITEM_TYPE_TAG,
 	MLX5_RTE_FLOW_ITEM_TYPE_TX_QUEUE,
+	MLX5_RTE_FLOW_ITEM_TYPE_VLAN,
 };
 
 /* Private (internal) rte flow actions. */
@@ -326,6 +327,9 @@ enum mlx5_feature_name {
 #define MLX5_GENEVE_OPT_LEN_0 14
 #define MLX5_GENEVE_OPT_LEN_1 63
 
+#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct rte_flow_item_eth) + \
+					  sizeof(struct rte_flow_item_ipv4))
+
 enum mlx5_flow_drv_type {
 	MLX5_FLOW_TYPE_MIN,
 	MLX5_FLOW_TYPE_DV,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index a5824dc..3694d6d 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -51,8 +51,6 @@
 #define MLX5DV_DR_ACTION_FLAGS_ROOT_LEVEL 1
 #endif
 
-#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct rte_flow_item_eth) + \
-					  sizeof(struct rte_flow_item_ipv4))
 /* VLAN header definitions */
 #define MLX5DV_FLOW_VLAN_PCP_SHIFT 13
 #define MLX5DV_FLOW_VLAN_PCP_MASK (0x7 << MLX5DV_FLOW_VLAN_PCP_SHIFT)
@@ -1626,10 +1624,14 @@ struct field_modify_info modify_tcp[] = {
 
 	if (items == NULL)
 		return;
-	for (; items->type != RTE_FLOW_ITEM_TYPE_END &&
-	       items->type != RTE_FLOW_ITEM_TYPE_VLAN; items++)
-		;
-	if (items->type == RTE_FLOW_ITEM_TYPE_VLAN) {
+	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
+		int type = items->type;
+
+		if (type == RTE_FLOW_ITEM_TYPE_VLAN ||
+		    type == MLX5_RTE_FLOW_ITEM_TYPE_VLAN)
+			break;
+	}
+	if (items->type != RTE_FLOW_ITEM_TYPE_END) {
 		const struct rte_flow_item_vlan *vlan_m = items->mask;
 		const struct rte_flow_item_vlan *vlan_v = items->spec;
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-stable] [PATCH 19.11 11/11] net/mlx5: fix metadata split with encap action
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
                   ` (9 preceding siblings ...)
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 10/11] net/mlx5: fix VLAN actions " Suanming Mou
@ 2020-02-28  3:33 ` Suanming Mou
  2020-02-28 11:17 ` [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Luca Boccassi
  11 siblings, 0 replies; 13+ messages in thread
From: Suanming Mou @ 2020-02-28  3:33 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable, Matan Azrad

From: Matan Azrad <matan@mellanox.com>

[ upstream commit 35594a9b6f96ac6e45bbb851d9180c841c35cfff ]

In order to move the mbuf metadata from the WQE to the FDB steering
domain, the PMD add for each NIC TX flow a new action to copy the
metadata register REG_A to REG_C_0.

This copy action is considered as modify header action from HW
perspective.

The HW doesn't support to do modify header action after ant
encapsulation action.

The split metadata function wrongly added the copy action in the end of
the original actions list, hence, NIC egress flow with encapsulation
action failed when the PMD worked with dv_xmeta_en mode.

Move the copy action to be before and back to back with the
encapsulation action for the aforementioned case.

Fixes: 71e254bc0294 ("net/mlx5: split Rx flows to provide metadata copy")

Signed-off-by: Matan Azrad <matan@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 66 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 51 insertions(+), 15 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 17ce2e3..1070b87 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2725,7 +2725,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 }
 
 /**
- * Get QUEUE/RSS action from the action list.
+ * Get metadata split action information.
  *
  * @param[in] actions
  *   Pointer to the list of actions.
@@ -2734,18 +2734,38 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  * @param[out] qrss_type
  *   Pointer to the action type to return. RTE_FLOW_ACTION_TYPE_END is returned
  *   if no QUEUE/RSS is found.
+ * @param[out] encap_idx
+ *   Pointer to the index of the encap action if exists, otherwise the last
+ *   action index.
  *
  * @return
  *   Total number of actions.
  */
 static int
-flow_parse_qrss_action(const struct rte_flow_action actions[],
-		       const struct rte_flow_action **qrss)
+flow_parse_metadata_split_actions_info(const struct rte_flow_action actions[],
+				       const struct rte_flow_action **qrss,
+				       int *encap_idx)
 {
+	const struct rte_flow_action_raw_encap *raw_encap;
 	int actions_n = 0;
+	int raw_decap_idx = -1;
 
+	*encap_idx = -1;
 	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
 		switch (actions->type) {
+		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
+		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
+			*encap_idx = actions_n;
+			break;
+		case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
+			raw_decap_idx = actions_n;
+			break;
+		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
+			raw_encap = actions->conf;
+			if (raw_encap->size > MLX5_ENCAPSULATION_DECISION_SIZE)
+				*encap_idx = raw_decap_idx != -1 ?
+						      raw_decap_idx : actions_n;
+			break;
 		case RTE_FLOW_ACTION_TYPE_QUEUE:
 		case RTE_FLOW_ACTION_TYPE_RSS:
 			*qrss = actions;
@@ -2755,6 +2775,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		}
 		actions_n++;
 	}
+	if (*encap_idx == -1)
+		*encap_idx = actions_n;
 	/* Count RTE_FLOW_ACTION_TYPE_END. */
 	return actions_n + 1;
 }
@@ -3719,6 +3741,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   Number of actions in the list.
  * @param[out] error
  *   Perform verbose error reporting if not NULL.
+ * @param[in] encap_idx
+ *   The encap action inndex.
  *
  * @return
  *   0 on success, negative value otherwise
@@ -3727,7 +3751,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 flow_mreg_tx_copy_prep(struct rte_eth_dev *dev,
 		       struct rte_flow_action *ext_actions,
 		       const struct rte_flow_action *actions,
-		       int actions_n, struct rte_flow_error *error)
+		       int actions_n, struct rte_flow_error *error,
+		       int encap_idx)
 {
 	struct mlx5_flow_action_copy_mreg *cp_mreg =
 		(struct mlx5_flow_action_copy_mreg *)
@@ -3742,15 +3767,24 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (ret < 0)
 		return ret;
 	cp_mreg->src = ret;
-	memcpy(ext_actions, actions,
-			sizeof(*ext_actions) * actions_n);
-	ext_actions[actions_n - 1] = (struct rte_flow_action){
-		.type = MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
-		.conf = cp_mreg,
-	};
-	ext_actions[actions_n] = (struct rte_flow_action){
-		.type = RTE_FLOW_ACTION_TYPE_END,
-	};
+	if (encap_idx != 0)
+		memcpy(ext_actions, actions, sizeof(*ext_actions) * encap_idx);
+	if (encap_idx == actions_n - 1) {
+		ext_actions[actions_n - 1] = (struct rte_flow_action){
+			.type = MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
+			.conf = cp_mreg,
+		};
+		ext_actions[actions_n] = (struct rte_flow_action){
+			.type = RTE_FLOW_ACTION_TYPE_END,
+		};
+	} else {
+		ext_actions[encap_idx] = (struct rte_flow_action){
+			.type = MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
+			.conf = cp_mreg,
+		};
+		memcpy(ext_actions + encap_idx + 1, actions + encap_idx,
+				sizeof(*ext_actions) * (actions_n - encap_idx));
+	}
 	return 0;
 }
 
@@ -3801,6 +3835,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	int mtr_sfx = 0;
 	size_t act_size;
 	int actions_n;
+	int encap_idx;
 	int ret;
 
 	/* Check whether extensive metadata feature is engaged. */
@@ -3810,7 +3845,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		return flow_create_split_inner(dev, flow, NULL, prefix_layers,
 					       attr, items, actions, external,
 					       error);
-	actions_n = flow_parse_qrss_action(actions, &qrss);
+	actions_n = flow_parse_metadata_split_actions_info(actions, &qrss,
+							   &encap_idx);
 	if (qrss) {
 		/* Exclude hairpin flows from splitting. */
 		if (qrss->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
@@ -3885,7 +3921,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 						  "metadata flow");
 		/* Create the action list appended with copy register. */
 		ret = flow_mreg_tx_copy_prep(dev, ext_actions, actions,
-					     actions_n, error);
+					     actions_n, error, encap_idx);
 		if (ret < 0)
 			goto exit;
 	}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport
  2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
                   ` (10 preceding siblings ...)
  2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 11/11] net/mlx5: fix metadata split with encap action Suanming Mou
@ 2020-02-28 11:17 ` Luca Boccassi
  11 siblings, 0 replies; 13+ messages in thread
From: Luca Boccassi @ 2020-02-28 11:17 UTC (permalink / raw)
  To: Suanming Mou; +Cc: stable

On Fri, 2020-02-28 at 11:33 +0800, Suanming Mou wrote:
> Please remove the commit below with rebase first:
> dec473d net/mlx5: fix metadata split with encap action
> 
> The commit makes the wrong patch order.
> Remove the commit will not cause any conflicts.
> 
> Dekel Peled (2):
>   net/mlx5: unify validation of drop action
>   net/mlx5: update description of validation functions
> 
> Matan Azrad (2):
>   net/mlx5: fix encap/decap validation
>   net/mlx5: fix metadata split with encap action
> 
> Suanming Mou (7):
>   net/mlx5: support maximum flow id allocation
>   net/mlx5: fix register usage in meter
>   net/mlx5: fix layer validation with decapsulation
>   net/mlx5: fix layer type in header modify action
>   net/mlx5: fix layer flags missing in metadata
>   net/mlx5: fix match information in meter
>   net/mlx5: fix VLAN actions in meter
> 
>  drivers/net/mlx5/mlx5.c            |  19 +-
>  drivers/net/mlx5/mlx5.h            |   4 +
>  drivers/net/mlx5/mlx5_devx_cmds.c  |   2 +
>  drivers/net/mlx5/mlx5_flow.c       | 366 ++++++++++++++++-----------
> -
>  drivers/net/mlx5/mlx5_flow.h       |  40 ++-
>  drivers/net/mlx5/mlx5_flow_dv.c    | 485 +++++++++++++++++++++----
> ------------
>  drivers/net/mlx5/mlx5_flow_verbs.c |  12 +
>  drivers/net/mlx5/mlx5_prm.h        |   7 +-
>  8 files changed, 552 insertions(+), 383 deletions(-)

Thank you, applied. I also dropped the previous version of "net/mlx5:
fix metadata split with encap action" in favour of yours.

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-02-28 11:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28  3:33 [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 01/11] net/mlx5: unify validation of drop action Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 02/11] net/mlx5: update description of validation functions Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 03/11] net/mlx5: support maximum flow id allocation Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 04/11] net/mlx5: fix register usage in meter Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 05/11] net/mlx5: fix encap/decap validation Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 06/11] net/mlx5: fix layer validation with decapsulation Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 07/11] net/mlx5: fix layer type in header modify action Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 08/11] net/mlx5: fix layer flags missing in metadata Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 09/11] net/mlx5: fix match information in meter Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 10/11] net/mlx5: fix VLAN actions " Suanming Mou
2020-02-28  3:33 ` [dpdk-stable] [PATCH 19.11 11/11] net/mlx5: fix metadata split with encap action Suanming Mou
2020-02-28 11:17 ` [dpdk-stable] [PATCH 19.11 00/11] net/mlx5: fix patch backport Luca Boccassi

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git