All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] fix flow tunnel handling
@ 2018-10-24 12:36 Shahaf Shuler
  2018-10-24 12:36 ` [PATCH 1/3] net/mlx5: rename static functions Shahaf Shuler
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Shahaf Shuler @ 2018-10-24 12:36 UTC (permalink / raw)
  To: shahafs; +Cc: dev, yskoh

This patchset fix multiple issues with tunnel rule handling in the PMD's
flow engine.

Yongseok Koh (3):
  net/mlx5: rename static functions
  net/mlx5: fix flow tunnel handling
  net/mlx5: fix bit width of item and action flags

 drivers/net/mlx5/mlx5_flow.c       | 129 +++++++++++++++++++++-----------
 drivers/net/mlx5/mlx5_flow.h       |  10 +--
 drivers/net/mlx5/mlx5_flow_dv.c    |  16 ++--
 drivers/net/mlx5/mlx5_flow_tcf.c   |   6 +-
 drivers/net/mlx5/mlx5_flow_verbs.c |  19 +++--
 5 files changed, 114 insertions(+), 66 deletions(-)

-- 
2.12.0

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 1/3] net/mlx5: rename static functions
  2018-10-24 12:36 [PATCH 0/3] fix flow tunnel handling Shahaf Shuler
@ 2018-10-24 12:36 ` Shahaf Shuler
  2018-10-24 12:36 ` [PATCH 2/3] net/mlx5: fix flow tunnel handling Shahaf Shuler
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Shahaf Shuler @ 2018-10-24 12:36 UTC (permalink / raw)
  To: shahafs; +Cc: dev, yskoh

From: Yongseok Koh <yskoh@mellanox.com>

In mlx5_flow*.c, static functions have names starting from 'flow_' while
shared ones start from "mlx5_flow_'.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 62 +++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 6219c9d68a..68eb7da3f6 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -505,7 +505,7 @@ mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow,
  *   Rx queue to update.
  */
 static void
-mlx5_flow_rxq_tunnel_ptype_update(struct mlx5_rxq_ctrl *rxq_ctrl)
+flow_rxq_tunnel_ptype_update(struct mlx5_rxq_ctrl *rxq_ctrl)
 {
 	unsigned int i;
 	uint32_t tunnel_ptype = 0;
@@ -533,7 +533,7 @@ mlx5_flow_rxq_tunnel_ptype_update(struct mlx5_rxq_ctrl *rxq_ctrl)
  *   Pointer to flow structure.
  */
 static void
-mlx5_flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow)
+flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow)
 {
 	struct priv *priv = dev->data->dev_private;
 	const int mark = !!(flow->actions &
@@ -562,7 +562,7 @@ mlx5_flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow)
 					break;
 				}
 			}
-			mlx5_flow_rxq_tunnel_ptype_update(rxq_ctrl);
+			flow_rxq_tunnel_ptype_update(rxq_ctrl);
 		}
 	}
 }
@@ -577,7 +577,7 @@ mlx5_flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow)
  *   Pointer to the flow.
  */
 static void
-mlx5_flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow)
+flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow)
 {
 	struct priv *priv = dev->data->dev_private;
 	const int mark = !!(flow->actions &
@@ -607,7 +607,7 @@ mlx5_flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow)
 					break;
 				}
 			}
-			mlx5_flow_rxq_tunnel_ptype_update(rxq_ctrl);
+			flow_rxq_tunnel_ptype_update(rxq_ctrl);
 		}
 	}
 }
@@ -619,7 +619,7 @@ mlx5_flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow)
  *   Pointer to Ethernet device.
  */
 static void
-mlx5_flow_rxq_flags_clear(struct rte_eth_dev *dev)
+flow_rxq_flags_clear(struct rte_eth_dev *dev)
 {
 	struct priv *priv = dev->data->dev_private;
 	unsigned int i;
@@ -1914,7 +1914,7 @@ mlx5_flow_validate(struct rte_eth_dev *dev,
  *   Pointer to the RSS action if exist, else return NULL.
  */
 static const struct rte_flow_action_rss*
-mlx5_flow_get_rss_action(const struct rte_flow_action actions[])
+flow_get_rss_action(const struct rte_flow_action actions[])
 {
 	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
 		switch (actions->type) {
@@ -1929,7 +1929,7 @@ mlx5_flow_get_rss_action(const struct rte_flow_action actions[])
 }
 
 static unsigned int
-mlx5_find_graph_root(const struct rte_flow_item pattern[], uint32_t rss_level)
+find_graph_root(const struct rte_flow_item pattern[], uint32_t rss_level)
 {
 	const struct rte_flow_item *item;
 	unsigned int has_vlan = 0;
@@ -1967,12 +1967,11 @@ mlx5_find_graph_root(const struct rte_flow_item pattern[], uint32_t rss_level)
  *   A flow on success, NULL otherwise and rte_errno is set.
  */
 static struct rte_flow *
-mlx5_flow_list_create(struct rte_eth_dev *dev,
-		      struct mlx5_flows *list,
-		      const struct rte_flow_attr *attr,
-		      const struct rte_flow_item items[],
-		      const struct rte_flow_action actions[],
-		      struct rte_flow_error *error)
+flow_list_create(struct rte_eth_dev *dev, struct mlx5_flows *list,
+		 const struct rte_flow_attr *attr,
+		 const struct rte_flow_item items[],
+		 const struct rte_flow_action actions[],
+		 struct rte_flow_error *error)
 {
 	struct rte_flow *flow = NULL;
 	struct mlx5_flow *dev_flow;
@@ -1992,7 +1991,7 @@ mlx5_flow_list_create(struct rte_eth_dev *dev,
 	if (ret < 0)
 		return NULL;
 	flow_size = sizeof(struct rte_flow);
-	rss = mlx5_flow_get_rss_action(actions);
+	rss = flow_get_rss_action(actions);
 	if (rss)
 		flow_size += RTE_ALIGN_CEIL(rss->queue_num * sizeof(uint16_t),
 					    sizeof(void *));
@@ -2007,7 +2006,7 @@ mlx5_flow_list_create(struct rte_eth_dev *dev,
 	if (rss && rss->types) {
 		unsigned int graph_root;
 
-		graph_root = mlx5_find_graph_root(items, rss->level);
+		graph_root = find_graph_root(items, rss->level);
 		ret = rte_flow_expand_rss(buf, sizeof(expand_buffer.buffer),
 					  items, rss->types,
 					  mlx5_support_expansion,
@@ -2038,7 +2037,7 @@ mlx5_flow_list_create(struct rte_eth_dev *dev,
 			goto error;
 	}
 	TAILQ_INSERT_TAIL(list, flow, next);
-	mlx5_flow_rxq_flags_set(dev, flow);
+	flow_rxq_flags_set(dev, flow);
 	return flow;
 error:
 	ret = rte_errno; /* Save rte_errno before cleanup. */
@@ -2062,9 +2061,9 @@ mlx5_flow_create(struct rte_eth_dev *dev,
 		 const struct rte_flow_action actions[],
 		 struct rte_flow_error *error)
 {
-	return mlx5_flow_list_create
-		(dev, &((struct priv *)dev->data->dev_private)->flows,
-		 attr, items, actions, error);
+	return flow_list_create(dev,
+				&((struct priv *)dev->data->dev_private)->flows,
+				attr, items, actions, error);
 }
 
 /**
@@ -2078,8 +2077,8 @@ mlx5_flow_create(struct rte_eth_dev *dev,
  *   Flow to destroy.
  */
 static void
-mlx5_flow_list_destroy(struct rte_eth_dev *dev, struct mlx5_flows *list,
-		       struct rte_flow *flow)
+flow_list_destroy(struct rte_eth_dev *dev, struct mlx5_flows *list,
+		  struct rte_flow *flow)
 {
 	flow_drv_destroy(dev, flow);
 	TAILQ_REMOVE(list, flow, next);
@@ -2088,7 +2087,7 @@ mlx5_flow_list_destroy(struct rte_eth_dev *dev, struct mlx5_flows *list,
 	 * already clean.
 	 */
 	if (dev->data->dev_started)
-		mlx5_flow_rxq_flags_trim(dev, flow);
+		flow_rxq_flags_trim(dev, flow);
 	rte_free(flow);
 }
 
@@ -2107,7 +2106,7 @@ mlx5_flow_list_flush(struct rte_eth_dev *dev, struct mlx5_flows *list)
 		struct rte_flow *flow;
 
 		flow = TAILQ_FIRST(list);
-		mlx5_flow_list_destroy(dev, list, flow);
+		flow_list_destroy(dev, list, flow);
 	}
 }
 
@@ -2126,7 +2125,7 @@ mlx5_flow_stop(struct rte_eth_dev *dev, struct mlx5_flows *list)
 
 	TAILQ_FOREACH_REVERSE(flow, list, mlx5_flows, next)
 		flow_drv_remove(dev, flow);
-	mlx5_flow_rxq_flags_clear(dev);
+	flow_rxq_flags_clear(dev);
 }
 
 /**
@@ -2151,7 +2150,7 @@ mlx5_flow_start(struct rte_eth_dev *dev, struct mlx5_flows *list)
 		ret = flow_drv_apply(dev, flow, &error);
 		if (ret < 0)
 			goto error;
-		mlx5_flow_rxq_flags_set(dev, flow);
+		flow_rxq_flags_set(dev, flow);
 	}
 	return 0;
 error:
@@ -2260,8 +2259,8 @@ mlx5_ctrl_flow_vlan(struct rte_eth_dev *dev,
 	}
 	for (i = 0; i != priv->reta_idx_n; ++i)
 		queue[i] = (*priv->reta_idx)[i];
-	flow = mlx5_flow_list_create(dev, &priv->ctrl_flows, &attr, items,
-				     actions, &error);
+	flow = flow_list_create(dev, &priv->ctrl_flows,
+				&attr, items, actions, &error);
 	if (!flow)
 		return -rte_errno;
 	return 0;
@@ -2301,7 +2300,7 @@ mlx5_flow_destroy(struct rte_eth_dev *dev,
 {
 	struct priv *priv = dev->data->dev_private;
 
-	mlx5_flow_list_destroy(dev, &priv->flows, flow);
+	flow_list_destroy(dev, &priv->flows, flow);
 	return 0;
 }
 
@@ -2609,9 +2608,8 @@ mlx5_fdir_filter_add(struct rte_eth_dev *dev,
 	ret = mlx5_fdir_filter_convert(dev, fdir_filter, &attributes);
 	if (ret)
 		return ret;
-	flow = mlx5_flow_list_create(dev, &priv->flows, &attributes.attr,
-				     attributes.items, attributes.actions,
-				     &error);
+	flow = flow_list_create(dev, &priv->flows, &attributes.attr,
+				attributes.items, attributes.actions, &error);
 	if (flow) {
 		DRV_LOG(DEBUG, "port %u FDIR created %p", dev->data->port_id,
 			(void *)flow);
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/3] net/mlx5: fix flow tunnel handling
  2018-10-24 12:36 [PATCH 0/3] fix flow tunnel handling Shahaf Shuler
  2018-10-24 12:36 ` [PATCH 1/3] net/mlx5: rename static functions Shahaf Shuler
@ 2018-10-24 12:36 ` Shahaf Shuler
  2018-10-24 12:36 ` [PATCH 3/3] net/mlx5: fix bit width of item and action flags Shahaf Shuler
  2018-10-24 12:53 ` [PATCH 0/3] fix flow tunnel handling Shahaf Shuler
  3 siblings, 0 replies; 5+ messages in thread
From: Shahaf Shuler @ 2018-10-24 12:36 UTC (permalink / raw)
  To: shahafs; +Cc: dev, yskoh, orika

From: Yongseok Koh <yskoh@mellanox.com>

Both rte_flow and mlx5_flow redundantly have item flags. And it is not
properly set in the code. This causes wrong tunnel flag handling. A
rte_flow can have multiple expanded device flows if the flow has an RSS
action. Therefore, mlx5_flow should have the layers field.

Fixes: c4d9b9f7f382 ("net/mlx5: add Direct Verbs final functions")
Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
Fixes: 3d69434113d1 ("net/mlx5: add Direct Verbs validation function")
Fixes: 84c406e74524 ("net/mlx5: add flow translate function")
Fixes: 4e05a229c5da ("net/mlx5: add flow prepare function")
Fixes: 23c1d42c7138 ("net/mlx5: split flow validation to dedicated function")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c       | 69 +++++++++++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow.h       |  8 ++--
 drivers/net/mlx5/mlx5_flow_dv.c    | 12 +++---
 drivers/net/mlx5/mlx5_flow_verbs.c | 15 ++++---
 4 files changed, 77 insertions(+), 27 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 68eb7da3f6..0d9a03b632 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -525,20 +525,22 @@ flow_rxq_tunnel_ptype_update(struct mlx5_rxq_ctrl *rxq_ctrl)
 }
 
 /**
- * Set the Rx queue flags (Mark/Flag and Tunnel Ptypes) according to the flow.
+ * Set the Rx queue flags (Mark/Flag and Tunnel Ptypes) according to the devive
+ * flow.
  *
  * @param[in] dev
  *   Pointer to the Ethernet device structure.
- * @param[in] flow
- *   Pointer to flow structure.
+ * @param[in] dev_flow
+ *   Pointer to device flow structure.
  */
 static void
-flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow)
+flow_drv_rxq_flags_set(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow)
 {
 	struct priv *priv = dev->data->dev_private;
+	struct rte_flow *flow = dev_flow->flow;
 	const int mark = !!(flow->actions &
 			    (MLX5_FLOW_ACTION_FLAG | MLX5_FLOW_ACTION_MARK));
-	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
+	const int tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL);
 	unsigned int i;
 
 	for (i = 0; i != flow->rss.queue_num; ++i) {
@@ -556,7 +558,8 @@ flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow)
 
 			/* Increase the counter matching the flow. */
 			for (j = 0; j != MLX5_FLOW_TUNNEL; ++j) {
-				if ((tunnels_info[j].tunnel & flow->layers) ==
+				if ((tunnels_info[j].tunnel &
+				     dev_flow->layers) ==
 				    tunnels_info[j].tunnel) {
 					rxq_ctrl->flow_tunnels_n[j]++;
 					break;
@@ -568,21 +571,39 @@ flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow)
 }
 
 /**
+ * Set the Rx queue flags (Mark/Flag and Tunnel Ptypes) for a flow
+ *
+ * @param[in] dev
+ *   Pointer to the Ethernet device structure.
+ * @param[in] flow
+ *   Pointer to flow structure.
+ */
+static void
+flow_rxq_flags_set(struct rte_eth_dev *dev, struct rte_flow *flow)
+{
+	struct mlx5_flow *dev_flow;
+
+	LIST_FOREACH(dev_flow, &flow->dev_flows, next)
+		flow_drv_rxq_flags_set(dev, dev_flow);
+}
+
+/**
  * Clear the Rx queue flags (Mark/Flag and Tunnel Ptype) associated with the
- * @p flow if no other flow uses it with the same kind of request.
+ * device flow if no other flow uses it with the same kind of request.
  *
  * @param dev
  *   Pointer to Ethernet device.
- * @param[in] flow
- *   Pointer to the flow.
+ * @param[in] dev_flow
+ *   Pointer to the device flow.
  */
 static void
-flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow)
+flow_drv_rxq_flags_trim(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow)
 {
 	struct priv *priv = dev->data->dev_private;
+	struct rte_flow *flow = dev_flow->flow;
 	const int mark = !!(flow->actions &
 			    (MLX5_FLOW_ACTION_FLAG | MLX5_FLOW_ACTION_MARK));
-	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
+	const int tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL);
 	unsigned int i;
 
 	assert(dev->data->dev_started);
@@ -601,7 +622,8 @@ flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow)
 
 			/* Decrease the counter matching the flow. */
 			for (j = 0; j != MLX5_FLOW_TUNNEL; ++j) {
-				if ((tunnels_info[j].tunnel & flow->layers) ==
+				if ((tunnels_info[j].tunnel &
+				     dev_flow->layers) ==
 				    tunnels_info[j].tunnel) {
 					rxq_ctrl->flow_tunnels_n[j]--;
 					break;
@@ -613,6 +635,24 @@ flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow)
 }
 
 /**
+ * Clear the Rx queue flags (Mark/Flag and Tunnel Ptype) associated with the
+ * @p flow if no other flow uses it with the same kind of request.
+ *
+ * @param dev
+ *   Pointer to Ethernet device.
+ * @param[in] flow
+ *   Pointer to the flow.
+ */
+static void
+flow_rxq_flags_trim(struct rte_eth_dev *dev, struct rte_flow *flow)
+{
+	struct mlx5_flow *dev_flow;
+
+	LIST_FOREACH(dev_flow, &flow->dev_flows, next)
+		flow_drv_rxq_flags_trim(dev, dev_flow);
+}
+
+/**
  * Clear the Mark/Flag and Tunnel ptype information in all Rx queues.
  *
  * @param dev
@@ -2024,6 +2064,11 @@ flow_list_create(struct rte_eth_dev *dev, struct mlx5_flows *list,
 		if (!dev_flow)
 			goto error;
 		dev_flow->flow = flow;
+		dev_flow->layers = item_flags;
+		/* Store actions once as expanded flows have same actions. */
+		if (i == 0)
+			flow->actions = action_flags;
+		assert(flow->actions == action_flags);
 		LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next);
 		ret = flow_drv_translate(dev, dev_flow, attr,
 					 buf->entry[i].pattern,
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 38635c9fdb..1cc989ae91 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -215,7 +215,8 @@ struct mlx5_flow_verbs {
 struct mlx5_flow {
 	LIST_ENTRY(mlx5_flow) next;
 	struct rte_flow *flow; /**< Pointer to the main flow. */
-	uint32_t layers; /**< Bit-fields that holds the detected layers. */
+	uint32_t layers;
+	/**< Bit-fields of present layers, see MLX5_FLOW_LAYER_*. */
 	union {
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
 		struct mlx5_flow_dv dv;
@@ -240,15 +241,14 @@ struct mlx5_flow_counter {
 struct rte_flow {
 	TAILQ_ENTRY(rte_flow) next; /**< Pointer to the next flow structure. */
 	enum mlx5_flow_drv_type drv_type; /**< Drvier type. */
-	uint32_t layers;
-	/**< Bit-fields of present layers see MLX5_FLOW_LAYER_*. */
 	struct mlx5_flow_counter *counter; /**< Holds flow counter. */
 	struct rte_flow_action_rss rss;/**< RSS context. */
 	uint8_t key[MLX5_RSS_HASH_KEY_LEN]; /**< RSS hash key. */
 	uint16_t (*queue)[]; /**< Destination queues to redirect traffic to. */
 	LIST_HEAD(dev_flows, mlx5_flow) dev_flows;
 	/**< Device flows that are part of the flow. */
-	uint32_t actions; /**< Bit-fields which mark all detected actions. */
+	uint32_t actions;
+	/**< Bit-fields of detected actions, see MLX5_FLOW_ACTION_*. */
 };
 typedef int (*mlx5_flow_validate_t)(struct rte_eth_dev *dev,
 				    const struct rte_flow_attr *attr,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 071f31d0fd..53e9a170c3 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -177,6 +177,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 	if (ret < 0)
 		return ret;
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
+		tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
 			break;
@@ -285,7 +286,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 			return rte_flow_error_set(error, ENOTSUP,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  actions, "too many actions");
-		tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
 		switch (actions->type) {
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
@@ -1253,13 +1253,15 @@ flow_dv_translate(struct rte_eth_dev *dev,
 		},
 	};
 	void *match_value = dev_flow->dv.value.buf;
-	uint8_t inner = 0;
+	int tunnel = 0;
 
 	if (priority == MLX5_FLOW_PRIO_RSVD)
 		priority = priv->config.flow_prio - 1;
-	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++)
+	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
+		tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL);
 		flow_dv_create_item(&matcher, match_value, items, dev_flow,
-				    inner);
+				    tunnel);
+	}
 	matcher.crc = rte_raw_cksum((const void *)matcher.mask.buf,
 				     matcher.mask.size);
 	if (priority == MLX5_FLOW_PRIO_RSVD)
@@ -1324,7 +1326,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 					(dev, flow->key, MLX5_RSS_HASH_KEY_LEN,
 					 dv->hash_fields, (*flow->queue),
 					 flow->rss.queue_num,
-					 !!(flow->layers &
+					 !!(dev_flow->layers &
 					    MLX5_FLOW_LAYER_TUNNEL));
 			if (!hrxq) {
 				rte_flow_error_set
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 4ae974b072..75b950e16f 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -343,7 +343,7 @@ flow_verbs_translate_item_ipv6(const struct rte_flow_item *item,
 {
 	const struct rte_flow_item_ipv6 *spec = item->spec;
 	const struct rte_flow_item_ipv6 *mask = item->mask;
-	const int tunnel = !!(dev_flow->flow->layers & MLX5_FLOW_LAYER_TUNNEL);
+	const int tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL);
 	unsigned int size = sizeof(struct ibv_flow_spec_ipv6);
 	struct ibv_flow_spec_ipv6 ipv6 = {
 		.type = IBV_FLOW_SPEC_IPV6 | (tunnel ? IBV_FLOW_SPEC_INNER : 0),
@@ -467,7 +467,7 @@ flow_verbs_translate_item_tcp(const struct rte_flow_item *item,
 {
 	const struct rte_flow_item_tcp *spec = item->spec;
 	const struct rte_flow_item_tcp *mask = item->mask;
-	const int tunnel = !!(dev_flow->flow->layers & MLX5_FLOW_LAYER_TUNNEL);
+	const int tunnel = !!(dev_flow->layers & MLX5_FLOW_LAYER_TUNNEL);
 	unsigned int size = sizeof(struct ibv_flow_spec_tcp_udp);
 	struct ibv_flow_spec_tcp_udp tcp = {
 		.type = IBV_FLOW_SPEC_TCP | (tunnel ? IBV_FLOW_SPEC_INNER : 0),
@@ -999,6 +999,8 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 		return ret;
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
 		int ret = 0;
+
+		tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
 			break;
@@ -1110,7 +1112,6 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 		}
 	}
 	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
-		tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
 		switch (actions->type) {
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
@@ -1252,9 +1253,10 @@ flow_verbs_get_items_and_size(const struct rte_flow_item items[],
 {
 	int size = 0;
 	uint64_t detected_items = 0;
-	const int tunnel = !!(*item_flags & MLX5_FLOW_LAYER_TUNNEL);
 
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
+		int tunnel = !!(detected_items & MLX5_FLOW_LAYER_TUNNEL);
+
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
 			break;
@@ -1450,7 +1452,8 @@ flow_verbs_translate(struct rte_eth_dev *dev,
 						  "action not supported");
 		}
 	}
-	dev_flow->flow->actions |= action_flags;
+	/* Device flow should have action flags by flow_drv_prepare(). */
+	assert(dev_flow->flow->actions == action_flags);
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
@@ -1613,7 +1616,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 						     verbs->hash_fields,
 						     (*flow->queue),
 						     flow->rss.queue_num,
-						     !!(flow->layers &
+						     !!(dev_flow->layers &
 						      MLX5_FLOW_LAYER_TUNNEL));
 			if (!hrxq) {
 				rte_flow_error_set
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 3/3] net/mlx5: fix bit width of item and action flags
  2018-10-24 12:36 [PATCH 0/3] fix flow tunnel handling Shahaf Shuler
  2018-10-24 12:36 ` [PATCH 1/3] net/mlx5: rename static functions Shahaf Shuler
  2018-10-24 12:36 ` [PATCH 2/3] net/mlx5: fix flow tunnel handling Shahaf Shuler
@ 2018-10-24 12:36 ` Shahaf Shuler
  2018-10-24 12:53 ` [PATCH 0/3] fix flow tunnel handling Shahaf Shuler
  3 siblings, 0 replies; 5+ messages in thread
From: Shahaf Shuler @ 2018-10-24 12:36 UTC (permalink / raw)
  To: shahafs; +Cc: dev, yskoh, orika

From: Yongseok Koh <yskoh@mellanox.com>

Most of the code uses uint64_t for MLX5_FLOW_LAYER_* and
MLX5_FLOW_ACTION_*, but there're some code using uint32_t.

Fixes: 2ed2fe5f0a9c ("net/mlx5: rewrite IP address UDP/TCP port by E-Switch")
Fixes: 57123c00c1b8 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
Fixes: 3d69434113d1 ("net/mlx5: add Direct Verbs validation function")
Fixes: 84c406e74524 ("net/mlx5: add flow translate function")
Fixes: 23c1d42c7138 ("net/mlx5: split flow validation to dedicated function")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c       | 2 +-
 drivers/net/mlx5/mlx5_flow.h       | 6 +++---
 drivers/net/mlx5/mlx5_flow_dv.c    | 4 ++--
 drivers/net/mlx5/mlx5_flow_tcf.c   | 6 +++---
 drivers/net/mlx5/mlx5_flow_verbs.c | 4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 0d9a03b632..c757552771 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -477,7 +477,7 @@ mlx5_flow_item_acceptable(const struct rte_flow_item *item,
  */
 uint64_t
 mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow,
-			    int tunnel __rte_unused, uint32_t layer_types,
+			    int tunnel __rte_unused, uint64_t layer_types,
 			    uint64_t hash_fields)
 {
 	struct rte_flow *flow = dev_flow->flow;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 1cc989ae91..f2d1202285 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -215,7 +215,7 @@ struct mlx5_flow_verbs {
 struct mlx5_flow {
 	LIST_ENTRY(mlx5_flow) next;
 	struct rte_flow *flow; /**< Pointer to the main flow. */
-	uint32_t layers;
+	uint64_t layers;
 	/**< Bit-fields of present layers, see MLX5_FLOW_LAYER_*. */
 	union {
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
@@ -247,7 +247,7 @@ struct rte_flow {
 	uint16_t (*queue)[]; /**< Destination queues to redirect traffic to. */
 	LIST_HEAD(dev_flows, mlx5_flow) dev_flows;
 	/**< Device flows that are part of the flow. */
-	uint32_t actions;
+	uint64_t actions;
 	/**< Bit-fields of detected actions, see MLX5_FLOW_ACTION_*. */
 };
 typedef int (*mlx5_flow_validate_t)(struct rte_eth_dev *dev,
@@ -289,7 +289,7 @@ struct mlx5_flow_driver_ops {
 /* mlx5_flow.c */
 
 uint64_t mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow, int tunnel,
-				     uint32_t layer_types,
+				     uint64_t layer_types,
 				     uint64_t hash_fields);
 uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				   uint32_t subpriority);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 53e9a170c3..8f729f44f8 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -165,8 +165,8 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 		 struct rte_flow_error *error)
 {
 	int ret;
-	uint32_t action_flags = 0;
-	uint32_t item_flags = 0;
+	uint64_t action_flags = 0;
+	uint64_t item_flags = 0;
 	int tunnel = 0;
 	uint8_t next_protocol = 0xff;
 	int actions_n = 0;
diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index 05ffbd3114..a3c1449db7 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -968,8 +968,8 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 		const struct rte_flow_action_set_ipv4 *set_ipv4;
 		const struct rte_flow_action_set_ipv6 *set_ipv6;
 	} conf;
-	uint32_t item_flags = 0;
-	uint32_t action_flags = 0;
+	uint64_t item_flags = 0;
+	uint64_t action_flags = 0;
 	uint8_t next_protocol = -1;
 	unsigned int tcm_ifindex = 0;
 	uint8_t pedit_validated = 0;
@@ -1186,7 +1186,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 	}
 	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
 		unsigned int i;
-		uint32_t current_action_flag = 0;
+		uint64_t current_action_flag = 0;
 
 		switch (actions->type) {
 		case RTE_FLOW_ACTION_TYPE_VOID:
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 75b950e16f..9481edcf7f 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -987,8 +987,8 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 		    struct rte_flow_error *error)
 {
 	int ret;
-	uint32_t action_flags = 0;
-	uint32_t item_flags = 0;
+	uint64_t action_flags = 0;
+	uint64_t item_flags = 0;
 	int tunnel = 0;
 	uint8_t next_protocol = 0xff;
 
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] fix flow tunnel handling
  2018-10-24 12:36 [PATCH 0/3] fix flow tunnel handling Shahaf Shuler
                   ` (2 preceding siblings ...)
  2018-10-24 12:36 ` [PATCH 3/3] net/mlx5: fix bit width of item and action flags Shahaf Shuler
@ 2018-10-24 12:53 ` Shahaf Shuler
  3 siblings, 0 replies; 5+ messages in thread
From: Shahaf Shuler @ 2018-10-24 12:53 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, Yongseok Koh

Wednesday, October 24, 2018 3:36 PM, Shahaf Shuler:
> Subject: [dpdk-dev] [PATCH 0/3] fix flow tunnel handling
> 
> This patchset fix multiple issues with tunnel rule handling in the PMD's flow
> engine.
> 

Series applied to next-net-mlx, thanks. 

> Yongseok Koh (3):
>   net/mlx5: rename static functions
>   net/mlx5: fix flow tunnel handling
>   net/mlx5: fix bit width of item and action flags
> 
>  drivers/net/mlx5/mlx5_flow.c       | 129 +++++++++++++++++++++-----------
>  drivers/net/mlx5/mlx5_flow.h       |  10 +--
>  drivers/net/mlx5/mlx5_flow_dv.c    |  16 ++--
>  drivers/net/mlx5/mlx5_flow_tcf.c   |   6 +-
>  drivers/net/mlx5/mlx5_flow_verbs.c |  19 +++--
>  5 files changed, 114 insertions(+), 66 deletions(-)
> 
> --
> 2.12.0

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-24 12:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-24 12:36 [PATCH 0/3] fix flow tunnel handling Shahaf Shuler
2018-10-24 12:36 ` [PATCH 1/3] net/mlx5: rename static functions Shahaf Shuler
2018-10-24 12:36 ` [PATCH 2/3] net/mlx5: fix flow tunnel handling Shahaf Shuler
2018-10-24 12:36 ` [PATCH 3/3] net/mlx5: fix bit width of item and action flags Shahaf Shuler
2018-10-24 12:53 ` [PATCH 0/3] fix flow tunnel handling Shahaf Shuler

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.