DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
@ 2018-10-08 18:02 Yongseok Koh
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 1/7] net/mlx5: fix wrong flow action macro usage Yongseok Koh
                   ` (7 more replies)
  0 siblings, 8 replies; 28+ messages in thread
From: Yongseok Koh @ 2018-10-08 18:02 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, Yongseok Koh

Minor fixes accumulated since the following two patchsets.

	net/mlx5: add Direct Verbs flow driver support [1]
	net/mlx5: migrate Linux TC flower driver to new flow engine

[1] http://patches.dpdk.org/cover/45248
[2] http://patches.dpdk.org/cover/44897

Yongseok Koh (7):
  net/mlx5: fix wrong flow action macro usage
  net/mlx5: use standard IP protocol numbers
  net/mlx5: rename flow macros
  net/mlx5: fix validation of VLAN ID in flow spec
  net/mlx5: fix flow validation for no fate action
  net/mlx5: add missing VLAN action constraints
  net/mlx5: fix errno values for flow engine

 drivers/net/mlx5/mlx5_flow.c       | 117 +++++++++++++++++++------------------
 drivers/net/mlx5/mlx5_flow.h       |  54 ++++++++---------
 drivers/net/mlx5/mlx5_flow_dv.c    |  30 +++++-----
 drivers/net/mlx5/mlx5_flow_tcf.c   |  78 +++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow_verbs.c |  59 ++++++++++---------
 5 files changed, 193 insertions(+), 145 deletions(-)

-- 
2.11.0

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

* [dpdk-dev] [PATCH 1/7] net/mlx5: fix wrong flow action macro usage
  2018-10-08 18:02 [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine Yongseok Koh
@ 2018-10-08 18:02 ` Yongseok Koh
  2018-10-09  7:45   ` Ori Kam
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 2/7] net/mlx5: use standard IP protocol numbers Yongseok Koh
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 28+ messages in thread
From: Yongseok Koh @ 2018-10-08 18:02 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, Yongseok Koh, Ori Kam

Fixes: 5c83c536783c ("net/mlx5: add Direct Verbs final functions")
Cc: Ori Kam <orika@mellanox.com>

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.h    | 10 ----------
 drivers/net/mlx5/mlx5_flow_dv.c |  2 +-
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index a91f9911d..79d4a2619 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -65,16 +65,6 @@
 	(MLX5_FLOW_LAYER_INNER_L2 | MLX5_FLOW_LAYER_INNER_L3 | \
 	 MLX5_FLOW_LAYER_INNER_L4)
 
-/* Actions that modify the fate of matching traffic. */
-#define MLX5_FLOW_FATE_DROP (1u << 0)
-#define MLX5_FLOW_FATE_QUEUE (1u << 1)
-#define MLX5_FLOW_FATE_RSS (1u << 2)
-
-/* Modify a packet. */
-#define MLX5_FLOW_MOD_FLAG (1u << 0)
-#define MLX5_FLOW_MOD_MARK (1u << 1)
-#define MLX5_FLOW_MOD_COUNT (1u << 2)
-
 /* Actions */
 #define MLX5_ACTION_DROP (1u << 0)
 #define MLX5_ACTION_QUEUE (1u << 1)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 7d325320a..5678dc35f 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1251,7 +1251,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
 		struct mlx5_flow_dv *dv = &dev_flow->dv;
 		if (dv->hrxq) {
-			if (flow->actions & MLX5_FLOW_FATE_DROP)
+			if (flow->actions & MLX5_ACTION_DROP)
 				mlx5_hrxq_drop_release(dev);
 			else
 				mlx5_hrxq_release(dev, dv->hrxq);
-- 
2.11.0

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

* [dpdk-dev] [PATCH 2/7] net/mlx5: use standard IP protocol numbers
  2018-10-08 18:02 [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine Yongseok Koh
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 1/7] net/mlx5: fix wrong flow action macro usage Yongseok Koh
@ 2018-10-08 18:02 ` Yongseok Koh
  2018-10-09  7:37   ` Ori Kam
  2018-10-09 15:39   ` Ferruh Yigit
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros Yongseok Koh
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 28+ messages in thread
From: Yongseok Koh @ 2018-10-08 18:02 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, Yongseok Koh, Ori Kam

Fixes: 0f8775dd8f1c ("net/mlx5: add support for multiple flow drivers")
Cc: Ori Kam <orika@mellanox.com>

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c       | 9 +++++----
 drivers/net/mlx5/mlx5_flow.h       | 9 ++++-----
 drivers/net/mlx5/mlx5_flow_verbs.c | 7 ++++---
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 8007bf10f..ef5e4684f 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3,6 +3,7 @@
  * Copyright 2016 Mellanox Technologies, Ltd
  */
 
+#include <netinet/in.h>
 #include <sys/queue.h>
 #include <stdalign.h>
 #include <stdint.h>
@@ -1188,7 +1189,7 @@ mlx5_flow_validate_item_udp(const struct rte_flow_item *item,
 	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
 	int ret;
 
-	if (target_protocol != 0xff && target_protocol != MLX5_IP_PROTOCOL_UDP)
+	if (target_protocol != 0xff && target_protocol != IPPROTO_UDP)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "protocol filtering not compatible"
@@ -1239,7 +1240,7 @@ mlx5_flow_validate_item_tcp(const struct rte_flow_item *item,
 	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
 	int ret;
 
-	if (target_protocol != 0xff && target_protocol != MLX5_IP_PROTOCOL_TCP)
+	if (target_protocol != 0xff && target_protocol != IPPROTO_TCP)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "protocol filtering not compatible"
@@ -1459,7 +1460,7 @@ mlx5_flow_validate_item_gre(const struct rte_flow_item *item,
 	const struct rte_flow_item_gre *mask = item->mask;
 	int ret;
 
-	if (target_protocol != 0xff && target_protocol != MLX5_IP_PROTOCOL_GRE)
+	if (target_protocol != 0xff && target_protocol != IPPROTO_GRE)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "protocol filtering not compatible"
@@ -1516,7 +1517,7 @@ mlx5_flow_validate_item_mpls(const struct rte_flow_item *item __rte_unused,
 	const struct rte_flow_item_mpls *mask = item->mask;
 	int ret;
 
-	if (target_protocol != 0xff && target_protocol != MLX5_IP_PROTOCOL_MPLS)
+	if (target_protocol != 0xff && target_protocol != IPPROTO_MPLS)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "protocol filtering not compatible"
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 79d4a2619..1ac871b3a 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -5,6 +5,7 @@
 #ifndef RTE_PMD_MLX5_FLOW_H_
 #define RTE_PMD_MLX5_FLOW_H_
 
+#include <netinet/in.h>
 #include <sys/queue.h>
 #include <stdalign.h>
 #include <stdint.h>
@@ -78,11 +79,9 @@
 #define MLX5_ACTION_OF_SET_VLAN_VID (1u << 9)
 #define MLX5_ACTION_OF_SET_VLAN_PCP (1u << 10)
 
-/* possible L3 layers protocols filtering. */
-#define MLX5_IP_PROTOCOL_TCP 6
-#define MLX5_IP_PROTOCOL_UDP 17
-#define MLX5_IP_PROTOCOL_GRE 47
-#define MLX5_IP_PROTOCOL_MPLS 147
+#ifndef IPPROTO_MPLS
+#define IPPROTO_MPLS 137
+#endif
 
 /* Internent Protocol versions. */
 #define MLX5_VXLAN 4789
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 05ab5fdad..076bb39e6 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -2,6 +2,7 @@
  * Copyright 2018 Mellanox Technologies, Ltd
  */
 
+#include <netinet/in.h>
 #include <sys/queue.h>
 #include <stdalign.h>
 #include <stdint.h>
@@ -683,11 +684,11 @@ flow_verbs_translate_item_gre(const struct rte_flow_item *item __rte_unused,
 	if (*item_flags & MLX5_FLOW_LAYER_OUTER_L3_IPV4)
 		flow_verbs_item_gre_ip_protocol_update(verbs->attr,
 						       IBV_FLOW_SPEC_IPV4_EXT,
-						       MLX5_IP_PROTOCOL_GRE);
+						       IPPROTO_GRE);
 	else
 		flow_verbs_item_gre_ip_protocol_update(verbs->attr,
 						       IBV_FLOW_SPEC_IPV6,
-						       MLX5_IP_PROTOCOL_GRE);
+						       IPPROTO_GRE);
 	flow_verbs_spec_add(dev_flow, &tunnel, size);
 	verbs->attr->priority = MLX5_PRIORITY_MAP_L2;
 	*item_flags |= MLX5_FLOW_LAYER_GRE;
@@ -1091,7 +1092,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 			if (ret < 0)
 				return ret;
 			if (next_protocol != 0xff &&
-			    next_protocol != MLX5_IP_PROTOCOL_MPLS)
+			    next_protocol != IPPROTO_MPLS)
 				return rte_flow_error_set
 					(error, ENOTSUP,
 					 RTE_FLOW_ERROR_TYPE_ITEM, items,
-- 
2.11.0

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

* [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros
  2018-10-08 18:02 [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine Yongseok Koh
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 1/7] net/mlx5: fix wrong flow action macro usage Yongseok Koh
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 2/7] net/mlx5: use standard IP protocol numbers Yongseok Koh
@ 2018-10-08 18:02 ` Yongseok Koh
  2018-10-09  7:43   ` Ori Kam
  2018-10-10 11:57   ` Ferruh Yigit
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 4/7] net/mlx5: fix validation of VLAN ID in flow spec Yongseok Koh
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 28+ messages in thread
From: Yongseok Koh @ 2018-10-08 18:02 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, Yongseok Koh, Ori Kam

MLX5_ACTION_* -> MLX5_FLOW_ACTION_*
MLX5_VXLAN* -> MLX5_UDP_PORT_VXLAN*

Fixes: 0f8775dd8f1c ("net/mlx5: add support for multiple flow drivers")
Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
Cc: Ori Kam <orika@mellanox.com>

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c       | 31 +++++++++++++++------------
 drivers/net/mlx5/mlx5_flow.h       | 28 ++++++++++++------------
 drivers/net/mlx5/mlx5_flow_dv.c    | 24 ++++++++++-----------
 drivers/net/mlx5/mlx5_flow_tcf.c   | 26 +++++++++++-----------
 drivers/net/mlx5/mlx5_flow_verbs.c | 44 +++++++++++++++++++-------------------
 5 files changed, 78 insertions(+), 75 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index ef5e4684f..69afd4625 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -537,7 +537,7 @@ mlx5_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 &
-			    (MLX5_ACTION_FLAG | MLX5_ACTION_MARK));
+			    (MLX5_FLOW_ACTION_FLAG | MLX5_FLOW_ACTION_MARK));
 	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
 	unsigned int i;
 
@@ -581,7 +581,7 @@ mlx5_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 &
-			    (MLX5_ACTION_FLAG | MLX5_ACTION_MARK));
+			    (MLX5_FLOW_ACTION_FLAG | MLX5_FLOW_ACTION_MARK));
 	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
 	unsigned int i;
 
@@ -656,15 +656,15 @@ mlx5_flow_validate_action_flag(uint64_t action_flags,
 			       struct rte_flow_error *error)
 {
 
-	if (action_flags & MLX5_ACTION_DROP)
+	if (action_flags & MLX5_FLOW_ACTION_DROP)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't drop and flag in same flow");
-	if (action_flags & MLX5_ACTION_MARK)
+	if (action_flags & MLX5_FLOW_ACTION_MARK)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't mark and flag in same flow");
-	if (action_flags & MLX5_ACTION_FLAG)
+	if (action_flags & MLX5_FLOW_ACTION_FLAG)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 flag"
@@ -703,15 +703,15 @@ mlx5_flow_validate_action_mark(const struct rte_flow_action *action,
 					  &mark->id,
 					  "mark id must in 0 <= id < "
 					  RTE_STR(MLX5_FLOW_MARK_MAX));
-	if (action_flags & MLX5_ACTION_DROP)
+	if (action_flags & MLX5_FLOW_ACTION_DROP)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't drop and mark in same flow");
-	if (action_flags & MLX5_ACTION_FLAG)
+	if (action_flags & MLX5_FLOW_ACTION_FLAG)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't flag and mark in same flow");
-	if (action_flags & MLX5_ACTION_MARK)
+	if (action_flags & MLX5_FLOW_ACTION_MARK)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 flag actions in same"
@@ -734,16 +734,17 @@ int
 mlx5_flow_validate_action_drop(uint64_t action_flags,
 			       struct rte_flow_error *error)
 {
-	if (action_flags & MLX5_ACTION_FLAG)
+	if (action_flags & MLX5_FLOW_ACTION_FLAG)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't drop and flag in same flow");
-	if (action_flags & MLX5_ACTION_MARK)
+	if (action_flags & MLX5_FLOW_ACTION_MARK)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't drop and mark in same flow");
 	if (action_flags &
-		(MLX5_ACTION_DROP | MLX5_ACTION_QUEUE | MLX5_ACTION_RSS))
+	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
+	     MLX5_FLOW_ACTION_RSS))
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 fate actions in"
@@ -776,7 +777,8 @@ mlx5_flow_validate_action_queue(const struct rte_flow_action *action,
 	const struct rte_flow_action_queue *queue = action->conf;
 
 	if (action_flags &
-	    (MLX5_ACTION_DROP | MLX5_ACTION_QUEUE | MLX5_ACTION_RSS))
+	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
+	     MLX5_FLOW_ACTION_RSS))
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 fate actions in"
@@ -820,7 +822,8 @@ mlx5_flow_validate_action_rss(const struct rte_flow_action *action,
 	unsigned int i;
 
 	if (action_flags &
-	    (MLX5_ACTION_DROP | MLX5_ACTION_QUEUE | MLX5_ACTION_RSS))
+	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
+	     MLX5_FLOW_ACTION_RSS))
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 fate actions"
@@ -2304,7 +2307,7 @@ mlx5_flow_query_count(struct rte_flow *flow __rte_unused,
 		      struct rte_flow_error *error)
 {
 #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
-	if (flow->actions & MLX5_ACTION_COUNT) {
+	if (flow->actions & MLX5_FLOW_ACTION_COUNT) {
 		struct rte_flow_query_count *qc = data;
 		uint64_t counters[2] = {0, 0};
 		struct ibv_query_counter_set_attr query_cs_attr = {
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 1ac871b3a..309e4d4f2 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -67,25 +67,25 @@
 	 MLX5_FLOW_LAYER_INNER_L4)
 
 /* Actions */
-#define MLX5_ACTION_DROP (1u << 0)
-#define MLX5_ACTION_QUEUE (1u << 1)
-#define MLX5_ACTION_RSS (1u << 2)
-#define MLX5_ACTION_FLAG (1u << 3)
-#define MLX5_ACTION_MARK (1u << 4)
-#define MLX5_ACTION_COUNT (1u << 5)
-#define MLX5_ACTION_PORT_ID (1u << 6)
-#define MLX5_ACTION_OF_POP_VLAN (1u << 7)
-#define MLX5_ACTION_OF_PUSH_VLAN (1u << 8)
-#define MLX5_ACTION_OF_SET_VLAN_VID (1u << 9)
-#define MLX5_ACTION_OF_SET_VLAN_PCP (1u << 10)
+#define MLX5_FLOW_ACTION_DROP (1u << 0)
+#define MLX5_FLOW_ACTION_QUEUE (1u << 1)
+#define MLX5_FLOW_ACTION_RSS (1u << 2)
+#define MLX5_FLOW_ACTION_FLAG (1u << 3)
+#define MLX5_FLOW_ACTION_MARK (1u << 4)
+#define MLX5_FLOW_ACTION_COUNT (1u << 5)
+#define MLX5_FLOW_ACTION_PORT_ID (1u << 6)
+#define MLX5_FLOW_ACTION_OF_POP_VLAN (1u << 7)
+#define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
+#define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
+#define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
 
 #ifndef IPPROTO_MPLS
 #define IPPROTO_MPLS 137
 #endif
 
-/* Internent Protocol versions. */
-#define MLX5_VXLAN 4789
-#define MLX5_VXLAN_GPE 4790
+/* UDP port numbers for VxLAN. */
+#define MLX5_UDP_PORT_VXLAN 4789
+#define MLX5_UDP_PORT_VXLAN_GPE 4790
 
 /* Priority reserved for default flows. */
 #define MLX5_FLOW_PRIO_RSVD ((uint32_t)-1)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 5678dc35f..e6c84d444 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -236,7 +236,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 							     error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_FLAG;
+			action_flags |= MLX5_FLOW_ACTION_FLAG;
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_MARK:
@@ -245,7 +245,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 							     error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_MARK;
+			action_flags |= MLX5_FLOW_ACTION_MARK;
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
@@ -253,7 +253,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 							     error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_DROP;
+			action_flags |= MLX5_FLOW_ACTION_DROP;
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_QUEUE:
@@ -262,7 +262,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 							      error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_QUEUE;
+			action_flags |= MLX5_FLOW_ACTION_QUEUE;
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RSS:
@@ -271,14 +271,14 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 							    error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_RSS;
+			action_flags |= MLX5_FLOW_ACTION_RSS;
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 			ret = mlx5_flow_validate_action_count(dev, error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_COUNT;
+			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			++actions_n;
 			break;
 		default:
@@ -838,8 +838,8 @@ flow_dv_translate_item_vxlan(void *matcher, void *key,
 					 outer_headers);
 		headers_v = MLX5_ADDR_OF(fte_match_param, key, outer_headers);
 	}
-	dport = item->type == RTE_FLOW_ITEM_TYPE_VXLAN ? MLX5_VXLAN :
-							 MLX5_VXLAN_GPE;
+	dport = item->type == RTE_FLOW_ITEM_TYPE_VXLAN ?
+		MLX5_UDP_PORT_VXLAN : MLX5_UDP_PORT_VXLAN_GPE;
 	if (!MLX5_GET16(fte_match_set_lyr_2_4, headers_v, udp_dport)) {
 		MLX5_SET(fte_match_set_lyr_2_4, headers_m, udp_dport, 0xFFFF);
 		MLX5_SET(fte_match_set_lyr_2_4, headers_v, udp_dport, dport);
@@ -978,7 +978,7 @@ flow_dv_create_action(const struct rte_flow_action *action,
 		break;
 	case RTE_FLOW_ACTION_TYPE_DROP:
 		dev_flow->dv.actions[actions_n].type = MLX5DV_FLOW_ACTION_DROP;
-		flow->actions |= MLX5_ACTION_DROP;
+		flow->actions |= MLX5_FLOW_ACTION_DROP;
 		break;
 	case RTE_FLOW_ACTION_TYPE_QUEUE:
 		queue = action->conf;
@@ -1195,7 +1195,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
 		dv = &dev_flow->dv;
 		n = dv->actions_n;
-		if (flow->actions & MLX5_ACTION_DROP) {
+		if (flow->actions & MLX5_FLOW_ACTION_DROP) {
 			dv->hrxq = mlx5_hrxq_drop_new(dev);
 			if (!dv->hrxq) {
 				rte_flow_error_set
@@ -1251,7 +1251,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
 		struct mlx5_flow_dv *dv = &dev_flow->dv;
 		if (dv->hrxq) {
-			if (flow->actions & MLX5_ACTION_DROP)
+			if (flow->actions & MLX5_FLOW_ACTION_DROP)
 				mlx5_hrxq_drop_release(dev);
 			else
 				mlx5_hrxq_release(dev, dv->hrxq);
@@ -1318,7 +1318,7 @@ flow_dv_remove(struct rte_eth_dev *dev, struct rte_flow *flow)
 			dv->flow = NULL;
 		}
 		if (dv->hrxq) {
-			if (flow->actions & MLX5_ACTION_DROP)
+			if (flow->actions & MLX5_FLOW_ACTION_DROP)
 				mlx5_hrxq_drop_release(dev);
 			else
 				mlx5_hrxq_release(dev, dv->hrxq);
diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index 14376188e..c87046365 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -225,7 +225,7 @@ struct flow_tcf_ptoi {
 	unsigned int ifindex; /**< Network interface index. */
 };
 
-#define MLX5_TCF_FATE_ACTIONS (MLX5_ACTION_DROP | MLX5_ACTION_PORT_ID)
+#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID)
 
 /**
  * Retrieve mask for pattern item.
@@ -668,7 +668,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 					 conf.port_id,
 					 "missing data to convert port ID to"
 					 " ifindex");
-			action_flags |= MLX5_ACTION_PORT_ID;
+			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
 			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			if (action_flags & MLX5_TCF_FATE_ACTIONS)
@@ -676,19 +676,19 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 					(error, ENOTSUP,
 					 RTE_FLOW_ERROR_TYPE_ACTION, actions,
 					 "can't have multiple fate actions");
-			action_flags |= MLX5_ACTION_DROP;
+			action_flags |= MLX5_FLOW_ACTION_DROP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
-			action_flags |= MLX5_ACTION_OF_POP_VLAN;
+			action_flags |= MLX5_FLOW_ACTION_OF_POP_VLAN;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
-			action_flags |= MLX5_ACTION_OF_PUSH_VLAN;
+			action_flags |= MLX5_FLOW_ACTION_OF_PUSH_VLAN;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
-			action_flags |= MLX5_ACTION_OF_SET_VLAN_VID;
+			action_flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
-			action_flags |= MLX5_ACTION_OF_SET_VLAN_PCP;
+			action_flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
 			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
@@ -809,26 +809,26 @@ flow_tcf_get_actions_and_size(const struct rte_flow_action actions[],
 				SZ_NLATTR_STRZ_OF("mirred") +
 				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
 				SZ_NLATTR_TYPE_OF(struct tc_mirred);
-			flags |= MLX5_ACTION_PORT_ID;
+			flags |= MLX5_FLOW_ACTION_PORT_ID;
 			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			size += SZ_NLATTR_NEST + /* na_act_index. */
 				SZ_NLATTR_STRZ_OF("gact") +
 				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
 				SZ_NLATTR_TYPE_OF(struct tc_gact);
-			flags |= MLX5_ACTION_DROP;
+			flags |= MLX5_FLOW_ACTION_DROP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
-			flags |= MLX5_ACTION_OF_POP_VLAN;
+			flags |= MLX5_FLOW_ACTION_OF_POP_VLAN;
 			goto action_of_vlan;
 		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
-			flags |= MLX5_ACTION_OF_PUSH_VLAN;
+			flags |= MLX5_FLOW_ACTION_OF_PUSH_VLAN;
 			goto action_of_vlan;
 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
-			flags |= MLX5_ACTION_OF_SET_VLAN_VID;
+			flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
 			goto action_of_vlan;
 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
-			flags |= MLX5_ACTION_OF_SET_VLAN_PCP;
+			flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
 			goto action_of_vlan;
 action_of_vlan:
 			size += SZ_NLATTR_NEST + /* na_act_index. */
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 076bb39e6..0ecbc8121 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -755,7 +755,7 @@ flow_verbs_translate_action_drop(uint64_t *action_flags,
 	};
 
 	flow_verbs_spec_add(dev_flow, &drop, size);
-	*action_flags |= MLX5_ACTION_DROP;
+	*action_flags |= MLX5_FLOW_ACTION_DROP;
 }
 
 /**
@@ -781,7 +781,7 @@ flow_verbs_translate_action_queue(const struct rte_flow_action *action,
 	if (flow->queue)
 		(*flow->queue)[0] = queue->index;
 	flow->rss.queue_num = 1;
-	*action_flags |= MLX5_ACTION_QUEUE;
+	*action_flags |= MLX5_FLOW_ACTION_QUEUE;
 }
 
 /**
@@ -811,7 +811,7 @@ flow_verbs_translate_action_rss(const struct rte_flow_action *action,
 	memcpy(flow->key, rss->key, MLX5_RSS_HASH_KEY_LEN);
 	flow->rss.types = rss->types;
 	flow->rss.level = rss->level;
-	*action_flags |= MLX5_ACTION_RSS;
+	*action_flags |= MLX5_FLOW_ACTION_RSS;
 }
 
 /**
@@ -838,7 +838,7 @@ flow_verbs_translate_action_flag
 		.size = size,
 		.tag_id = mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT),
 	};
-	*action_flags |= MLX5_ACTION_MARK;
+	*action_flags |= MLX5_FLOW_ACTION_MARK;
 	flow_verbs_spec_add(dev_flow, &tag, size);
 }
 
@@ -898,14 +898,14 @@ flow_verbs_translate_action_mark(const struct rte_flow_action *action,
 	};
 	struct mlx5_flow_verbs *verbs = &dev_flow->verbs;
 
-	if (*action_flags & MLX5_ACTION_FLAG) {
+	if (*action_flags & MLX5_FLOW_ACTION_FLAG) {
 		flow_verbs_mark_update(verbs, mark->id);
 		size = 0;
 	} else {
 		tag.tag_id = mlx5_flow_mark_set(mark->id);
 		flow_verbs_spec_add(dev_flow, &tag, size);
 	}
-	*action_flags |= MLX5_ACTION_MARK;
+	*action_flags |= MLX5_FLOW_ACTION_MARK;
 }
 
 /**
@@ -954,7 +954,7 @@ flow_verbs_translate_action_count(struct rte_eth_dev *dev,
 						  "cannot get counter"
 						  " context.");
 	}
-	*action_flags |= MLX5_ACTION_COUNT;
+	*action_flags |= MLX5_FLOW_ACTION_COUNT;
 #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
 	counter.counter_set_handle = flow->counter->cs->handle;
 	flow_verbs_spec_add(dev_flow, &counter, size);
@@ -1116,7 +1116,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 							     error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_FLAG;
+			action_flags |= MLX5_FLOW_ACTION_FLAG;
 			break;
 		case RTE_FLOW_ACTION_TYPE_MARK:
 			ret = mlx5_flow_validate_action_mark(actions,
@@ -1124,14 +1124,14 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 							     error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_MARK;
+			action_flags |= MLX5_FLOW_ACTION_MARK;
 			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			ret = mlx5_flow_validate_action_drop(action_flags,
 							     error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_DROP;
+			action_flags |= MLX5_FLOW_ACTION_DROP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_QUEUE:
 			ret = mlx5_flow_validate_action_queue(actions,
@@ -1139,7 +1139,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 							      error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_QUEUE;
+			action_flags |= MLX5_FLOW_ACTION_QUEUE;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RSS:
 			ret = mlx5_flow_validate_action_rss(actions,
@@ -1147,13 +1147,13 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 							    error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_RSS;
+			action_flags |= MLX5_FLOW_ACTION_RSS;
 			break;
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 			ret = mlx5_flow_validate_action_count(dev, error);
 			if (ret < 0)
 				return ret;
-			action_flags |= MLX5_ACTION_COUNT;
+			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
@@ -1191,27 +1191,27 @@ flow_verbs_get_actions_and_size(const struct rte_flow_action actions[],
 			break;
 		case RTE_FLOW_ACTION_TYPE_FLAG:
 			size += sizeof(struct ibv_flow_spec_action_tag);
-			detected_actions |= MLX5_ACTION_FLAG;
+			detected_actions |= MLX5_FLOW_ACTION_FLAG;
 			break;
 		case RTE_FLOW_ACTION_TYPE_MARK:
 			size += sizeof(struct ibv_flow_spec_action_tag);
-			detected_actions |= MLX5_ACTION_MARK;
+			detected_actions |= MLX5_FLOW_ACTION_MARK;
 			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			size += sizeof(struct ibv_flow_spec_action_drop);
-			detected_actions |= MLX5_ACTION_DROP;
+			detected_actions |= MLX5_FLOW_ACTION_DROP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_QUEUE:
-			detected_actions |= MLX5_ACTION_QUEUE;
+			detected_actions |= MLX5_FLOW_ACTION_QUEUE;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RSS:
-			detected_actions |= MLX5_ACTION_RSS;
+			detected_actions |= MLX5_FLOW_ACTION_RSS;
 			break;
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
 			size += sizeof(struct ibv_flow_spec_counter_action);
 #endif
-			detected_actions |= MLX5_ACTION_COUNT;
+			detected_actions |= MLX5_FLOW_ACTION_COUNT;
 			break;
 		default:
 			break;
@@ -1519,7 +1519,7 @@ flow_verbs_remove(struct rte_eth_dev *dev, struct rte_flow *flow)
 			verbs->flow = NULL;
 		}
 		if (verbs->hrxq) {
-			if (flow->actions & MLX5_ACTION_DROP)
+			if (flow->actions & MLX5_FLOW_ACTION_DROP)
 				mlx5_hrxq_drop_release(dev);
 			else
 				mlx5_hrxq_release(dev, verbs->hrxq);
@@ -1578,7 +1578,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 
 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
 		verbs = &dev_flow->verbs;
-		if (flow->actions & MLX5_ACTION_DROP) {
+		if (flow->actions & MLX5_FLOW_ACTION_DROP) {
 			verbs->hrxq = mlx5_hrxq_drop_new(dev);
 			if (!verbs->hrxq) {
 				rte_flow_error_set
@@ -1628,7 +1628,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
 		verbs = &dev_flow->verbs;
 		if (verbs->hrxq) {
-			if (flow->actions & MLX5_ACTION_DROP)
+			if (flow->actions & MLX5_FLOW_ACTION_DROP)
 				mlx5_hrxq_drop_release(dev);
 			else
 				mlx5_hrxq_release(dev, verbs->hrxq);
-- 
2.11.0

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

* [dpdk-dev] [PATCH 4/7] net/mlx5: fix validation of VLAN ID in flow spec
  2018-10-08 18:02 [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine Yongseok Koh
                   ` (2 preceding siblings ...)
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros Yongseok Koh
@ 2018-10-08 18:02 ` Yongseok Koh
  2018-10-09  7:47   ` Ori Kam
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 5/7] net/mlx5: fix flow validation for no fate action Yongseok Koh
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 28+ messages in thread
From: Yongseok Koh @ 2018-10-08 18:02 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, Yongseok Koh, Ori Kam

This can cause crash by null pointer reference.

Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
Cc: Ori Kam <orika@mellanox.com>

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

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 69afd4625..c497cacce 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1012,6 +1012,7 @@ mlx5_flow_validate_item_vlan(const struct rte_flow_item *item,
 		.tci = RTE_BE16(0x0fff),
 		.inner_type = RTE_BE16(0xffff),
 	};
+	uint16_t vlan_tag = 0;
 	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
 	int ret;
 	const uint32_t l34m = tunnel ? (MLX5_FLOW_LAYER_INNER_L3 |
@@ -1037,11 +1038,15 @@ mlx5_flow_validate_item_vlan(const struct rte_flow_item *item,
 					error);
 	if (ret)
 		return ret;
+	if (spec) {
+		vlan_tag = spec->tci;
+		vlan_tag &= mask->tci;
+	}
 	/*
 	 * From verbs perspective an empty VLAN is equivalent
 	 * to a packet without VLAN layer.
 	 */
-	if (!spec->tci)
+	if (!vlan_tag)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM_SPEC,
 					  item->spec,
-- 
2.11.0

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

* [dpdk-dev] [PATCH 5/7] net/mlx5: fix flow validation for no fate action
  2018-10-08 18:02 [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine Yongseok Koh
                   ` (3 preceding siblings ...)
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 4/7] net/mlx5: fix validation of VLAN ID in flow spec Yongseok Koh
@ 2018-10-08 18:02 ` Yongseok Koh
  2018-10-09  7:49   ` Ori Kam
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 6/7] net/mlx5: add missing VLAN action constraints Yongseok Koh
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 28+ messages in thread
From: Yongseok Koh @ 2018-10-08 18:02 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, Yongseok Koh, Ori Kam

Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
Fixes: f7adfffa3de1 ("net/mlx5: add Direct Verbs validation function")
Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
Cc: Ori Kam <orika@mellanox.com>

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

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index c497cacce..30aa95f14 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -742,9 +742,7 @@ mlx5_flow_validate_action_drop(uint64_t action_flags,
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't drop and mark in same flow");
-	if (action_flags &
-	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
-	     MLX5_FLOW_ACTION_RSS))
+	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 fate actions in"
@@ -776,9 +774,7 @@ mlx5_flow_validate_action_queue(const struct rte_flow_action *action,
 	struct priv *priv = dev->data->dev_private;
 	const struct rte_flow_action_queue *queue = action->conf;
 
-	if (action_flags &
-	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
-	     MLX5_FLOW_ACTION_RSS))
+	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 fate actions in"
@@ -821,9 +817,7 @@ mlx5_flow_validate_action_rss(const struct rte_flow_action *action,
 	const struct rte_flow_action_rss *rss = action->conf;
 	unsigned int i;
 
-	if (action_flags &
-	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
-	     MLX5_FLOW_ACTION_RSS))
+	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 fate actions"
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 309e4d4f2..12de841e8 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -79,6 +79,9 @@
 #define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
 #define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
 
+#define MLX5_FLOW_FATE_ACTIONS \
+	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | MLX5_FLOW_ACTION_RSS)
+
 #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 e6c84d444..2fb1d9ee7 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -288,6 +288,10 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 						  "action not supported");
 		}
 	}
+	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					  "no fate action is found");
 	return 0;
 }
 
diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index c87046365..4c660d72b 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -697,6 +697,10 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 						  "action not supported");
 		}
 	}
+	if (!(action_flags & MLX5_TCF_FATE_ACTIONS))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					  "no fate action is found");
 	return 0;
 }
 
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 0ecbc8121..3df467214 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -1162,6 +1162,10 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 						  "action not supported");
 		}
 	}
+	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					  "no fate action is found");
 	return 0;
 }
 
-- 
2.11.0

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

* [dpdk-dev] [PATCH 6/7] net/mlx5: add missing VLAN action constraints
  2018-10-08 18:02 [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine Yongseok Koh
                   ` (4 preceding siblings ...)
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 5/7] net/mlx5: fix flow validation for no fate action Yongseok Koh
@ 2018-10-08 18:02 ` Yongseok Koh
  2018-10-08 19:48   ` Or Gerlitz
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 7/7] net/mlx5: fix errno values for flow engine Yongseok Koh
  2018-10-09  8:58 ` [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new " Shahaf Shuler
  7 siblings, 1 reply; 28+ messages in thread
From: Yongseok Koh @ 2018-10-08 18:02 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, Yongseok Koh

1) VLAN modify isn't supported by driver.

2) FW syndrome (0xA9C090):
	set_flow_table_entry: push vlan action fte in fdb can ONLY be
	forward to the uplink.

3) FW syndrome (0x294609):
	set_flow_table_entry: modify/pop/push actions in fdb flow table are
	supported only while forwarding to vport.

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

diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index 4c660d72b..0406e84f6 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -226,6 +226,9 @@ struct flow_tcf_ptoi {
 };
 
 #define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID)
+#define MLX5_TCF_VLAN_ACTIONS \
+	(MLX5_FLOW_ACTION_OF_POP_VLAN | MLX5_FLOW_ACTION_OF_PUSH_VLAN | \
+	 MLX5_FLOW_ACTION_OF_SET_VLAN_VID | MLX5_FLOW_ACTION_OF_SET_VLAN_PCP)
 
 /**
  * Retrieve mask for pattern item.
@@ -436,6 +439,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 	uint8_t next_protocol = -1;
 	unsigned int tcm_ifindex = 0;
 	struct flow_tcf_ptoi ptoi[PTOI_TABLE_SZ_MAX(dev)];
+	struct rte_eth_dev *port_id_dev = NULL;
 	bool in_port_id_set;
 	int ret;
 
@@ -669,6 +673,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 					 "missing data to convert port ID to"
 					 " ifindex");
 			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
+			port_id_dev = &rte_eth_devices[conf.port_id->id];
 			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			if (action_flags & MLX5_TCF_FATE_ACTIONS)
@@ -685,9 +690,21 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 			action_flags |= MLX5_FLOW_ACTION_OF_PUSH_VLAN;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
+			if (!(action_flags & MLX5_FLOW_ACTION_OF_PUSH_VLAN))
+				return rte_flow_error_set
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					 "vlan modify is not supported,"
+					 " set action must follow push action");
 			action_flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
+			if (!(action_flags & MLX5_FLOW_ACTION_OF_PUSH_VLAN))
+				return rte_flow_error_set
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					 "vlan modify is not supported,"
+					 " set action must follow push action");
 			action_flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
 			break;
 		default:
@@ -697,6 +714,29 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 						  "action not supported");
 		}
 	}
+	/*
+	 * FW syndrome (0xA9C090):
+	 *     set_flow_table_entry: push vlan action fte in fdb can ONLY be
+	 *     forward to the uplink.
+	 */
+	if ((action_flags & MLX5_FLOW_ACTION_OF_PUSH_VLAN) &&
+	    (action_flags & MLX5_FLOW_ACTION_PORT_ID) &&
+	    ((struct priv *)port_id_dev->data->dev_private)->representor)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					  "vlan push can only be applied"
+					  " when forwarding to uplink port");
+	/*
+	 * FW syndrome (0x294609):
+	 *     set_flow_table_entry: modify/pop/push actions in fdb flow table
+	 *     are supported only while forwarding to vport.
+	 */
+	if ((action_flags & MLX5_TCF_VLAN_ACTIONS) &&
+	    !(action_flags & MLX5_FLOW_ACTION_PORT_ID))
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					  "vlan actions are supported"
+					  " only with port_id action");
 	if (!(action_flags & MLX5_TCF_FATE_ACTIONS))
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, actions,
-- 
2.11.0

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

* [dpdk-dev] [PATCH 7/7] net/mlx5: fix errno values for flow engine
  2018-10-08 18:02 [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine Yongseok Koh
                   ` (5 preceding siblings ...)
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 6/7] net/mlx5: add missing VLAN action constraints Yongseok Koh
@ 2018-10-08 18:02 ` Yongseok Koh
  2018-10-09  7:53   ` Ori Kam
  2018-10-09  8:58 ` [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new " Shahaf Shuler
  7 siblings, 1 reply; 28+ messages in thread
From: Yongseok Koh @ 2018-10-08 18:02 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: dev, Yongseok Koh, Ori Kam

Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
Fixes: f7adfffa3de1 ("net/mlx5: add Direct Verbs validation function")
Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
Cc: orika@mellanox.com

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

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 30aa95f14..ed60c40f9 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -452,10 +452,10 @@ mlx5_flow_item_acceptable(const struct rte_flow_item *item,
 		}
 		ret = memcmp(spec, last, size);
 		if (ret != 0)
-			return rte_flow_error_set(error, ENOTSUP,
+			return rte_flow_error_set(error, EINVAL,
 						  RTE_FLOW_ERROR_TYPE_ITEM,
 						  item,
-						  "range is not supported");
+						  "range is not valid");
 	}
 	return 0;
 }
@@ -657,15 +657,15 @@ mlx5_flow_validate_action_flag(uint64_t action_flags,
 {
 
 	if (action_flags & MLX5_FLOW_ACTION_DROP)
-		return rte_flow_error_set(error, ENOTSUP,
+		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, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't mark and flag in same flow");
 	if (action_flags & MLX5_FLOW_ACTION_FLAG)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 flag"
 					  " actions in same flow");
@@ -704,17 +704,17 @@ mlx5_flow_validate_action_mark(const struct rte_flow_action *action,
 					  "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, ENOTSUP,
+		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, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't flag and mark in same flow");
 	if (action_flags & MLX5_FLOW_ACTION_MARK)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "can't have 2 flag actions in same"
+					  "can't have 2 mark actions in same"
 					  " flow");
 	return 0;
 }
@@ -735,15 +735,15 @@ mlx5_flow_validate_action_drop(uint64_t action_flags,
 			       struct rte_flow_error *error)
 {
 	if (action_flags & MLX5_FLOW_ACTION_FLAG)
-		return rte_flow_error_set(error, ENOTSUP,
+		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, ENOTSUP,
+		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)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 fate actions in"
 					  " same flow");
@@ -775,7 +775,7 @@ mlx5_flow_validate_action_queue(const struct rte_flow_action *action,
 	const struct rte_flow_action_queue *queue = action->conf;
 
 	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 fate actions in"
 					  " same flow");
@@ -818,7 +818,7 @@ mlx5_flow_validate_action_rss(const struct rte_flow_action *action,
 	unsigned int i;
 
 	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can't have 2 fate actions"
 					  " in same flow");
@@ -931,7 +931,7 @@ mlx5_flow_validate_attributes(struct rte_eth_dev *dev,
 					  RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
 					  NULL, "transfer is not supported");
 	if (!attributes->ingress)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
 					  NULL,
 					  "ingress attribute is mandatory");
@@ -1017,11 +1017,11 @@ mlx5_flow_validate_item_vlan(const struct rte_flow_item *item,
 					MLX5_FLOW_LAYER_OUTER_VLAN;
 
 	if (item_flags & vlanm)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "VLAN layer already configured");
 	else if ((item_flags & l34m) != 0)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "L2 layer cannot follow L3/L4 layer");
 	if (!mask)
@@ -1085,7 +1085,7 @@ mlx5_flow_validate_item_ipv4(const struct rte_flow_item *item,
 					  "multiple L3 layers not supported");
 	else if (item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L4 :
 					MLX5_FLOW_LAYER_OUTER_L4))
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "L3 cannot follow an L4 layer.");
 	if (!mask)
@@ -1141,7 +1141,7 @@ mlx5_flow_validate_item_ipv6(const struct rte_flow_item *item,
 					  "multiple L3 layers not supported");
 	else if (item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L4 :
 					MLX5_FLOW_LAYER_OUTER_L4))
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "L3 cannot follow an L4 layer.");
 	/*
@@ -1192,18 +1192,18 @@ mlx5_flow_validate_item_udp(const struct rte_flow_item *item,
 	int ret;
 
 	if (target_protocol != 0xff && target_protocol != IPPROTO_UDP)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "protocol filtering not compatible"
 					  " with UDP layer");
 	if (!(item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L3 :
 				     MLX5_FLOW_LAYER_OUTER_L3)))
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "L3 is mandatory to filter on L4");
 	if (item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L4 :
 				   MLX5_FLOW_LAYER_OUTER_L4))
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "L4 layer is already present");
 	if (!mask)
@@ -1243,18 +1243,18 @@ mlx5_flow_validate_item_tcp(const struct rte_flow_item *item,
 	int ret;
 
 	if (target_protocol != 0xff && target_protocol != IPPROTO_TCP)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "protocol filtering not compatible"
 					  " with TCP layer");
 	if (!(item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L3 :
 				     MLX5_FLOW_LAYER_OUTER_L3)))
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "L3 is mandatory to filter on L4");
 	if (item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L4 :
 				   MLX5_FLOW_LAYER_OUTER_L4))
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "L4 layer is already present");
 	if (!mask)
@@ -1307,7 +1307,7 @@ mlx5_flow_validate_item_vxlan(const struct rte_flow_item *item,
 	 * https://tools.ietf.org/html/rfc7348
 	 */
 	if (!(item_flags & MLX5_FLOW_LAYER_OUTER_L4_UDP))
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "no outer UDP layer found");
 	if (!mask)
@@ -1335,11 +1335,11 @@ mlx5_flow_validate_item_vxlan(const struct rte_flow_item *item,
 	 * currently refused.
 	 */
 	if (!vlan_id)
-		return rte_flow_error_set(error, EINVAL,
+		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "VXLAN vni cannot be 0");
 	if (!(item_flags & MLX5_FLOW_LAYER_OUTER))
-		return rte_flow_error_set(error, EINVAL,
+		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "VXLAN tunnel must be fully defined");
 	return 0;
@@ -1393,7 +1393,7 @@ mlx5_flow_validate_item_vxlan_gpe(const struct rte_flow_item *item,
 	 * https://tools.ietf.org/html/rfc7348
 	 */
 	if (!(item_flags & MLX5_FLOW_LAYER_OUTER_L4_UDP))
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "no outer UDP layer found");
 	if (!mask)
@@ -1407,7 +1407,7 @@ mlx5_flow_validate_item_vxlan_gpe(const struct rte_flow_item *item,
 		return ret;
 	if (spec) {
 		if (spec->protocol)
-			return rte_flow_error_set(error, EINVAL,
+			return rte_flow_error_set(error, ENOTSUP,
 						  RTE_FLOW_ERROR_TYPE_ITEM,
 						  item,
 						  "VxLAN-GPE protocol"
@@ -1426,11 +1426,11 @@ mlx5_flow_validate_item_vxlan_gpe(const struct rte_flow_item *item,
 	 * is currently refused.
 	 */
 	if (!vlan_id)
-		return rte_flow_error_set(error, EINVAL,
+		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "VXLAN-GPE vni cannot be 0");
 	if (!(item_flags & MLX5_FLOW_LAYER_OUTER))
-		return rte_flow_error_set(error, EINVAL,
+		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "VXLAN-GPE tunnel must be fully"
 					  " defined");
@@ -1463,7 +1463,7 @@ mlx5_flow_validate_item_gre(const struct rte_flow_item *item,
 	int ret;
 
 	if (target_protocol != 0xff && target_protocol != IPPROTO_GRE)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "protocol filtering not compatible"
 					  " with this GRE layer");
@@ -1520,7 +1520,7 @@ mlx5_flow_validate_item_mpls(const struct rte_flow_item *item __rte_unused,
 	int ret;
 
 	if (target_protocol != 0xff && target_protocol != IPPROTO_MPLS)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ITEM, item,
 					  "protocol filtering not compatible"
 					  " with MPLS layer");
@@ -2336,7 +2336,7 @@ mlx5_flow_query_count(struct rte_flow *flow __rte_unused,
 		}
 		return 0;
 	}
-	return rte_flow_error_set(error, ENOTSUP,
+	return rte_flow_error_set(error, EINVAL,
 				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 				  NULL,
 				  "flow does not have counter");
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 2fb1d9ee7..3bb462ceb 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -79,7 +79,7 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev,
 					  NULL,
 					  "transfer is not supported");
 	if (!attributes->ingress)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
 					  NULL,
 					  "ingress attribute is mandatory");
diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index 0406e84f6..91f6ef678 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -291,7 +291,7 @@ flow_tcf_item_mask(const struct rte_flow_item *item, const void *mask_default,
 		if (item->last &&
 		    (((const uint8_t *)item->spec)[i] & mask[i]) !=
 		    (((const uint8_t *)item->last)[i] & mask[i])) {
-			rte_flow_error_set(error, ENOTSUP,
+			rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM_LAST,
 					   item->last,
 					   "range between \"spec\" and \"last\""
@@ -383,7 +383,7 @@ flow_tcf_validate_attributes(const struct rte_flow_attr *attr,
 					  attr,
 					  "lowest priority level is 0xfffe");
 	if (!attr->ingress)
-		return rte_flow_error_set(error, ENOTSUP,
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
 					  attr, "only ingress is supported");
 	if (attr->egress)
@@ -655,7 +655,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 		case RTE_FLOW_ACTION_TYPE_PORT_ID:
 			if (action_flags & MLX5_TCF_FATE_ACTIONS)
 				return rte_flow_error_set
-					(error, ENOTSUP,
+					(error, EINVAL,
 					 RTE_FLOW_ERROR_TYPE_ACTION, actions,
 					 "can't have multiple fate actions");
 			conf.port_id = actions->conf;
@@ -678,7 +678,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			if (action_flags & MLX5_TCF_FATE_ACTIONS)
 				return rte_flow_error_set
-					(error, ENOTSUP,
+					(error, EINVAL,
 					 RTE_FLOW_ERROR_TYPE_ACTION, actions,
 					 "can't have multiple fate actions");
 			action_flags |= MLX5_FLOW_ACTION_DROP;
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 3df467214..696447674 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -948,7 +948,7 @@ flow_verbs_translate_action_count(struct rte_eth_dev *dev,
 		flow->counter = flow_verbs_counter_new(dev, count->shared,
 						       count->id);
 		if (!flow->counter)
-			return rte_flow_error_set(error, ENOTSUP,
+			return rte_flow_error_set(error, rte_errno,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  action,
 						  "cannot get counter"
@@ -1094,7 +1094,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
 			if (next_protocol != 0xff &&
 			    next_protocol != IPPROTO_MPLS)
 				return rte_flow_error_set
-					(error, ENOTSUP,
+					(error, EINVAL,
 					 RTE_FLOW_ERROR_TYPE_ITEM, items,
 					 "protocol filtering not compatible"
 					 " with MPLS layer");
-- 
2.11.0

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

* Re: [dpdk-dev] [PATCH 6/7] net/mlx5: add missing VLAN action constraints
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 6/7] net/mlx5: add missing VLAN action constraints Yongseok Koh
@ 2018-10-08 19:48   ` Or Gerlitz
  0 siblings, 0 replies; 28+ messages in thread
From: Or Gerlitz @ 2018-10-08 19:48 UTC (permalink / raw)
  To: yskoh; +Cc: Shahaf Shuler, dev

On Mon, Oct 8, 2018 at 9:03 PM Yongseok Koh <yskoh@mellanox.com> wrote:

> 1) VLAN modify isn't supported by driver.
>
>
OVS doesn't support it also @ the DP level, modify is translated to DP
pop + push

How it is done on OVS/AVS-DPDK?



> 2) FW syndrome (0xA9C090):
>         set_flow_table_entry: push vlan action fte in fdb can ONLY be
>         forward to the uplink.
>

also the other way around (pop only to non-uplink), they have a bug there
not erring on that



>
> 3) FW syndrome (0x294609):
>         set_flow_table_entry: modify/pop/push actions in fdb flow table are
>         supported only while forwarding to vport.
>

note that future HWs might support (2) and (3)

>
>

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

* Re: [dpdk-dev] [PATCH 2/7] net/mlx5: use standard IP protocol numbers
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 2/7] net/mlx5: use standard IP protocol numbers Yongseok Koh
@ 2018-10-09  7:37   ` Ori Kam
  2018-10-09  8:02     ` Yongseok Koh
  2018-10-09 15:39   ` Ferruh Yigit
  1 sibling, 1 reply; 28+ messages in thread
From: Ori Kam @ 2018-10-09  7:37 UTC (permalink / raw)
  To: Yongseok Koh, Shahaf Shuler; +Cc: dev

Hi,
PSB

> -----Original Message-----
> From: Yongseok Koh
> Sent: Monday, October 8, 2018 9:02 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 2/7] net/mlx5: use standard IP protocol numbers
> 
> Fixes: 0f8775dd8f1c ("net/mlx5: add support for multiple flow drivers")

I don't think this is a fix since there is no issue with the implementation,
you are just beautify the code.

Otherwise
Acked-by: Ori Kam <orika@mellanox.com>

> Cc: Ori Kam <orika@mellanox.com>
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c       | 9 +++++----
>  drivers/net/mlx5/mlx5_flow.h       | 9 ++++-----
>  drivers/net/mlx5/mlx5_flow_verbs.c | 7 ++++---
>  3 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 8007bf10f..ef5e4684f 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -3,6 +3,7 @@
>   * Copyright 2016 Mellanox Technologies, Ltd
>   */
> 
> +#include <netinet/in.h>
>  #include <sys/queue.h>
>  #include <stdalign.h>
>  #include <stdint.h>
> @@ -1188,7 +1189,7 @@ mlx5_flow_validate_item_udp(const struct
> rte_flow_item *item,
>  	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
>  	int ret;
> 
> -	if (target_protocol != 0xff && target_protocol !=
> MLX5_IP_PROTOCOL_UDP)
> +	if (target_protocol != 0xff && target_protocol != IPPROTO_UDP)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "protocol filtering not compatible"
> @@ -1239,7 +1240,7 @@ mlx5_flow_validate_item_tcp(const struct
> rte_flow_item *item,
>  	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
>  	int ret;
> 
> -	if (target_protocol != 0xff && target_protocol !=
> MLX5_IP_PROTOCOL_TCP)
> +	if (target_protocol != 0xff && target_protocol != IPPROTO_TCP)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "protocol filtering not compatible"
> @@ -1459,7 +1460,7 @@ mlx5_flow_validate_item_gre(const struct
> rte_flow_item *item,
>  	const struct rte_flow_item_gre *mask = item->mask;
>  	int ret;
> 
> -	if (target_protocol != 0xff && target_protocol !=
> MLX5_IP_PROTOCOL_GRE)
> +	if (target_protocol != 0xff && target_protocol != IPPROTO_GRE)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "protocol filtering not compatible"
> @@ -1516,7 +1517,7 @@ mlx5_flow_validate_item_mpls(const struct
> rte_flow_item *item __rte_unused,
>  	const struct rte_flow_item_mpls *mask = item->mask;
>  	int ret;
> 
> -	if (target_protocol != 0xff && target_protocol !=
> MLX5_IP_PROTOCOL_MPLS)
> +	if (target_protocol != 0xff && target_protocol != IPPROTO_MPLS)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "protocol filtering not compatible"
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 79d4a2619..1ac871b3a 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -5,6 +5,7 @@
>  #ifndef RTE_PMD_MLX5_FLOW_H_
>  #define RTE_PMD_MLX5_FLOW_H_
> 
> +#include <netinet/in.h>
>  #include <sys/queue.h>
>  #include <stdalign.h>
>  #include <stdint.h>
> @@ -78,11 +79,9 @@
>  #define MLX5_ACTION_OF_SET_VLAN_VID (1u << 9)
>  #define MLX5_ACTION_OF_SET_VLAN_PCP (1u << 10)
> 
> -/* possible L3 layers protocols filtering. */
> -#define MLX5_IP_PROTOCOL_TCP 6
> -#define MLX5_IP_PROTOCOL_UDP 17
> -#define MLX5_IP_PROTOCOL_GRE 47
> -#define MLX5_IP_PROTOCOL_MPLS 147
> +#ifndef IPPROTO_MPLS
> +#define IPPROTO_MPLS 137
> +#endif
> 
>  /* Internent Protocol versions. */
>  #define MLX5_VXLAN 4789
> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
> b/drivers/net/mlx5/mlx5_flow_verbs.c
> index 05ab5fdad..076bb39e6 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -2,6 +2,7 @@
>   * Copyright 2018 Mellanox Technologies, Ltd
>   */
> 
> +#include <netinet/in.h>
>  #include <sys/queue.h>
>  #include <stdalign.h>
>  #include <stdint.h>
> @@ -683,11 +684,11 @@ flow_verbs_translate_item_gre(const struct
> rte_flow_item *item __rte_unused,
>  	if (*item_flags & MLX5_FLOW_LAYER_OUTER_L3_IPV4)
>  		flow_verbs_item_gre_ip_protocol_update(verbs->attr,
> 
> IBV_FLOW_SPEC_IPV4_EXT,
> -
> MLX5_IP_PROTOCOL_GRE);
> +						       IPPROTO_GRE);
>  	else
>  		flow_verbs_item_gre_ip_protocol_update(verbs->attr,
>  						       IBV_FLOW_SPEC_IPV6,
> -
> MLX5_IP_PROTOCOL_GRE);
> +						       IPPROTO_GRE);
>  	flow_verbs_spec_add(dev_flow, &tunnel, size);
>  	verbs->attr->priority = MLX5_PRIORITY_MAP_L2;
>  	*item_flags |= MLX5_FLOW_LAYER_GRE;
> @@ -1091,7 +1092,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>  			if (ret < 0)
>  				return ret;
>  			if (next_protocol != 0xff &&
> -			    next_protocol != MLX5_IP_PROTOCOL_MPLS)
> +			    next_protocol != IPPROTO_MPLS)
>  				return rte_flow_error_set
>  					(error, ENOTSUP,
>  					 RTE_FLOW_ERROR_TYPE_ITEM,
> items,
> --
> 2.11.0

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

* Re: [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros Yongseok Koh
@ 2018-10-09  7:43   ` Ori Kam
  2018-10-09  8:05     ` Yongseok Koh
  2018-10-09  8:17     ` Yongseok Koh
  2018-10-10 11:57   ` Ferruh Yigit
  1 sibling, 2 replies; 28+ messages in thread
From: Ori Kam @ 2018-10-09  7:43 UTC (permalink / raw)
  To: Yongseok Koh, Shahaf Shuler; +Cc: dev



> -----Original Message-----
> From: Yongseok Koh
> Sent: Monday, October 8, 2018 9:02 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 3/7] net/mlx5: rename flow macros
> 
> MLX5_ACTION_* -> MLX5_FLOW_ACTION_*
> MLX5_VXLAN* -> MLX5_UDP_PORT_VXLAN*
> 
> Fixes: 0f8775dd8f1c ("net/mlx5: add support for multiple flow drivers")
> Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
> Cc: Ori Kam <orika@mellanox.com>
> 
I don't think this is a fix since there is no issue with the implementation,
you are just beautify the code.

Otherwise acked:
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c       | 31 +++++++++++++++------------
>  drivers/net/mlx5/mlx5_flow.h       | 28 ++++++++++++------------
>  drivers/net/mlx5/mlx5_flow_dv.c    | 24 ++++++++++-----------
>  drivers/net/mlx5/mlx5_flow_tcf.c   | 26 +++++++++++-----------
>  drivers/net/mlx5/mlx5_flow_verbs.c | 44 +++++++++++++++++++------------------
> -
>  5 files changed, 78 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index ef5e4684f..69afd4625 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -537,7 +537,7 @@ mlx5_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 &
> -			    (MLX5_ACTION_FLAG | MLX5_ACTION_MARK));
> +			    (MLX5_FLOW_ACTION_FLAG |
> MLX5_FLOW_ACTION_MARK));
>  	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
>  	unsigned int i;
> 
> @@ -581,7 +581,7 @@ mlx5_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 &
> -			    (MLX5_ACTION_FLAG | MLX5_ACTION_MARK));
> +			    (MLX5_FLOW_ACTION_FLAG |
> MLX5_FLOW_ACTION_MARK));
>  	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
>  	unsigned int i;
> 
> @@ -656,15 +656,15 @@ mlx5_flow_validate_action_flag(uint64_t
> action_flags,
>  			       struct rte_flow_error *error)
>  {
> 
> -	if (action_flags & MLX5_ACTION_DROP)
> +	if (action_flags & MLX5_FLOW_ACTION_DROP)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't drop and flag in same flow");
> -	if (action_flags & MLX5_ACTION_MARK)
> +	if (action_flags & MLX5_FLOW_ACTION_MARK)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't mark and flag in same flow");
> -	if (action_flags & MLX5_ACTION_FLAG)
> +	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 flag"
> @@ -703,15 +703,15 @@ mlx5_flow_validate_action_mark(const struct
> rte_flow_action *action,
>  					  &mark->id,
>  					  "mark id must in 0 <= id < "
> 
> RTE_STR(MLX5_FLOW_MARK_MAX));
> -	if (action_flags & MLX5_ACTION_DROP)
> +	if (action_flags & MLX5_FLOW_ACTION_DROP)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't drop and mark in same flow");
> -	if (action_flags & MLX5_ACTION_FLAG)
> +	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't flag and mark in same flow");
> -	if (action_flags & MLX5_ACTION_MARK)
> +	if (action_flags & MLX5_FLOW_ACTION_MARK)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 flag actions in same"
> @@ -734,16 +734,17 @@ int
>  mlx5_flow_validate_action_drop(uint64_t action_flags,
>  			       struct rte_flow_error *error)
>  {
> -	if (action_flags & MLX5_ACTION_FLAG)
> +	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't drop and flag in same flow");
> -	if (action_flags & MLX5_ACTION_MARK)
> +	if (action_flags & MLX5_FLOW_ACTION_MARK)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't drop and mark in same flow");
>  	if (action_flags &
> -		(MLX5_ACTION_DROP | MLX5_ACTION_QUEUE |
> MLX5_ACTION_RSS))
> +	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> +	     MLX5_FLOW_ACTION_RSS))
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions in"
> @@ -776,7 +777,8 @@ mlx5_flow_validate_action_queue(const struct
> rte_flow_action *action,
>  	const struct rte_flow_action_queue *queue = action->conf;
> 
>  	if (action_flags &
> -	    (MLX5_ACTION_DROP | MLX5_ACTION_QUEUE |
> MLX5_ACTION_RSS))
> +	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> +	     MLX5_FLOW_ACTION_RSS))
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions in"
> @@ -820,7 +822,8 @@ mlx5_flow_validate_action_rss(const struct
> rte_flow_action *action,
>  	unsigned int i;
> 
>  	if (action_flags &
> -	    (MLX5_ACTION_DROP | MLX5_ACTION_QUEUE |
> MLX5_ACTION_RSS))
> +	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> +	     MLX5_FLOW_ACTION_RSS))
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions"
> @@ -2304,7 +2307,7 @@ mlx5_flow_query_count(struct rte_flow *flow
> __rte_unused,
>  		      struct rte_flow_error *error)
>  {
>  #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
> -	if (flow->actions & MLX5_ACTION_COUNT) {
> +	if (flow->actions & MLX5_FLOW_ACTION_COUNT) {
>  		struct rte_flow_query_count *qc = data;
>  		uint64_t counters[2] = {0, 0};
>  		struct ibv_query_counter_set_attr query_cs_attr = {
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 1ac871b3a..309e4d4f2 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -67,25 +67,25 @@
>  	 MLX5_FLOW_LAYER_INNER_L4)
> 
>  /* Actions */
> -#define MLX5_ACTION_DROP (1u << 0)
> -#define MLX5_ACTION_QUEUE (1u << 1)
> -#define MLX5_ACTION_RSS (1u << 2)
> -#define MLX5_ACTION_FLAG (1u << 3)
> -#define MLX5_ACTION_MARK (1u << 4)
> -#define MLX5_ACTION_COUNT (1u << 5)
> -#define MLX5_ACTION_PORT_ID (1u << 6)
> -#define MLX5_ACTION_OF_POP_VLAN (1u << 7)
> -#define MLX5_ACTION_OF_PUSH_VLAN (1u << 8)
> -#define MLX5_ACTION_OF_SET_VLAN_VID (1u << 9)
> -#define MLX5_ACTION_OF_SET_VLAN_PCP (1u << 10)
> +#define MLX5_FLOW_ACTION_DROP (1u << 0)
> +#define MLX5_FLOW_ACTION_QUEUE (1u << 1)
> +#define MLX5_FLOW_ACTION_RSS (1u << 2)
> +#define MLX5_FLOW_ACTION_FLAG (1u << 3)
> +#define MLX5_FLOW_ACTION_MARK (1u << 4)
> +#define MLX5_FLOW_ACTION_COUNT (1u << 5)
> +#define MLX5_FLOW_ACTION_PORT_ID (1u << 6)
> +#define MLX5_FLOW_ACTION_OF_POP_VLAN (1u << 7)
> +#define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
> +#define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
> +#define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
> 
>  #ifndef IPPROTO_MPLS
>  #define IPPROTO_MPLS 137
>  #endif
> 
> -/* Internent Protocol versions. */
> -#define MLX5_VXLAN 4789
> -#define MLX5_VXLAN_GPE 4790
> +/* UDP port numbers for VxLAN. */
> +#define MLX5_UDP_PORT_VXLAN 4789
> +#define MLX5_UDP_PORT_VXLAN_GPE 4790
> 
>  /* Priority reserved for default flows. */
>  #define MLX5_FLOW_PRIO_RSVD ((uint32_t)-1)
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 5678dc35f..e6c84d444 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -236,7 +236,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
> rte_flow_attr *attr,
>  							     error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_FLAG;
> +			action_flags |= MLX5_FLOW_ACTION_FLAG;
>  			++actions_n;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_MARK:
> @@ -245,7 +245,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
> rte_flow_attr *attr,
>  							     error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_MARK;
> +			action_flags |= MLX5_FLOW_ACTION_MARK;
>  			++actions_n;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_DROP:
> @@ -253,7 +253,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
> rte_flow_attr *attr,
>  							     error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_DROP;
> +			action_flags |= MLX5_FLOW_ACTION_DROP;
>  			++actions_n;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_QUEUE:
> @@ -262,7 +262,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
> rte_flow_attr *attr,
>  							      error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_QUEUE;
> +			action_flags |= MLX5_FLOW_ACTION_QUEUE;
>  			++actions_n;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_RSS:
> @@ -271,14 +271,14 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
>  							    error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_RSS;
> +			action_flags |= MLX5_FLOW_ACTION_RSS;
>  			++actions_n;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_COUNT:
>  			ret = mlx5_flow_validate_action_count(dev, error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_COUNT;
> +			action_flags |= MLX5_FLOW_ACTION_COUNT;
>  			++actions_n;
>  			break;
>  		default:
> @@ -838,8 +838,8 @@ flow_dv_translate_item_vxlan(void *matcher, void
> *key,
>  					 outer_headers);
>  		headers_v = MLX5_ADDR_OF(fte_match_param, key,
> outer_headers);
>  	}
> -	dport = item->type == RTE_FLOW_ITEM_TYPE_VXLAN ? MLX5_VXLAN :
> -							 MLX5_VXLAN_GPE;
> +	dport = item->type == RTE_FLOW_ITEM_TYPE_VXLAN ?
> +		MLX5_UDP_PORT_VXLAN : MLX5_UDP_PORT_VXLAN_GPE;
>  	if (!MLX5_GET16(fte_match_set_lyr_2_4, headers_v, udp_dport)) {
>  		MLX5_SET(fte_match_set_lyr_2_4, headers_m, udp_dport,
> 0xFFFF);
>  		MLX5_SET(fte_match_set_lyr_2_4, headers_v, udp_dport,
> dport);
> @@ -978,7 +978,7 @@ flow_dv_create_action(const struct rte_flow_action
> *action,
>  		break;
>  	case RTE_FLOW_ACTION_TYPE_DROP:
>  		dev_flow->dv.actions[actions_n].type =
> MLX5DV_FLOW_ACTION_DROP;
> -		flow->actions |= MLX5_ACTION_DROP;
> +		flow->actions |= MLX5_FLOW_ACTION_DROP;
>  		break;
>  	case RTE_FLOW_ACTION_TYPE_QUEUE:
>  		queue = action->conf;
> @@ -1195,7 +1195,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct
> rte_flow *flow,
>  	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>  		dv = &dev_flow->dv;
>  		n = dv->actions_n;
> -		if (flow->actions & MLX5_ACTION_DROP) {
> +		if (flow->actions & MLX5_FLOW_ACTION_DROP) {
>  			dv->hrxq = mlx5_hrxq_drop_new(dev);
>  			if (!dv->hrxq) {
>  				rte_flow_error_set
> @@ -1251,7 +1251,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct
> rte_flow *flow,
>  	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>  		struct mlx5_flow_dv *dv = &dev_flow->dv;
>  		if (dv->hrxq) {
> -			if (flow->actions & MLX5_ACTION_DROP)
> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>  				mlx5_hrxq_drop_release(dev);
>  			else
>  				mlx5_hrxq_release(dev, dv->hrxq);
> @@ -1318,7 +1318,7 @@ flow_dv_remove(struct rte_eth_dev *dev, struct
> rte_flow *flow)
>  			dv->flow = NULL;
>  		}
>  		if (dv->hrxq) {
> -			if (flow->actions & MLX5_ACTION_DROP)
> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>  				mlx5_hrxq_drop_release(dev);
>  			else
>  				mlx5_hrxq_release(dev, dv->hrxq);
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index 14376188e..c87046365 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -225,7 +225,7 @@ struct flow_tcf_ptoi {
>  	unsigned int ifindex; /**< Network interface index. */
>  };
> 
> -#define MLX5_TCF_FATE_ACTIONS (MLX5_ACTION_DROP |
> MLX5_ACTION_PORT_ID)
> +#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP |
> MLX5_FLOW_ACTION_PORT_ID)
> 
>  /**
>   * Retrieve mask for pattern item.
> @@ -668,7 +668,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>  					 conf.port_id,
>  					 "missing data to convert port ID to"
>  					 " ifindex");
> -			action_flags |= MLX5_ACTION_PORT_ID;
> +			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_DROP:
>  			if (action_flags & MLX5_TCF_FATE_ACTIONS)
> @@ -676,19 +676,19 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>  					(error, ENOTSUP,
>  					 RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
>  					 "can't have multiple fate actions");
> -			action_flags |= MLX5_ACTION_DROP;
> +			action_flags |= MLX5_FLOW_ACTION_DROP;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
> -			action_flags |= MLX5_ACTION_OF_POP_VLAN;
> +			action_flags |= MLX5_FLOW_ACTION_OF_POP_VLAN;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
> -			action_flags |= MLX5_ACTION_OF_PUSH_VLAN;
> +			action_flags |=
> MLX5_FLOW_ACTION_OF_PUSH_VLAN;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
> -			action_flags |= MLX5_ACTION_OF_SET_VLAN_VID;
> +			action_flags |=
> MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
> -			action_flags |= MLX5_ACTION_OF_SET_VLAN_PCP;
> +			action_flags |=
> MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
>  			break;
>  		default:
>  			return rte_flow_error_set(error, ENOTSUP,
> @@ -809,26 +809,26 @@ flow_tcf_get_actions_and_size(const struct
> rte_flow_action actions[],
>  				SZ_NLATTR_STRZ_OF("mirred") +
>  				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
>  				SZ_NLATTR_TYPE_OF(struct tc_mirred);
> -			flags |= MLX5_ACTION_PORT_ID;
> +			flags |= MLX5_FLOW_ACTION_PORT_ID;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_DROP:
>  			size += SZ_NLATTR_NEST + /* na_act_index. */
>  				SZ_NLATTR_STRZ_OF("gact") +
>  				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
>  				SZ_NLATTR_TYPE_OF(struct tc_gact);
> -			flags |= MLX5_ACTION_DROP;
> +			flags |= MLX5_FLOW_ACTION_DROP;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
> -			flags |= MLX5_ACTION_OF_POP_VLAN;
> +			flags |= MLX5_FLOW_ACTION_OF_POP_VLAN;
>  			goto action_of_vlan;
>  		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
> -			flags |= MLX5_ACTION_OF_PUSH_VLAN;
> +			flags |= MLX5_FLOW_ACTION_OF_PUSH_VLAN;
>  			goto action_of_vlan;
>  		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
> -			flags |= MLX5_ACTION_OF_SET_VLAN_VID;
> +			flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
>  			goto action_of_vlan;
>  		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
> -			flags |= MLX5_ACTION_OF_SET_VLAN_PCP;
> +			flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
>  			goto action_of_vlan;
>  action_of_vlan:
>  			size += SZ_NLATTR_NEST + /* na_act_index. */
> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
> b/drivers/net/mlx5/mlx5_flow_verbs.c
> index 076bb39e6..0ecbc8121 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -755,7 +755,7 @@ flow_verbs_translate_action_drop(uint64_t
> *action_flags,
>  	};
> 
>  	flow_verbs_spec_add(dev_flow, &drop, size);
> -	*action_flags |= MLX5_ACTION_DROP;
> +	*action_flags |= MLX5_FLOW_ACTION_DROP;
>  }
> 
>  /**
> @@ -781,7 +781,7 @@ flow_verbs_translate_action_queue(const struct
> rte_flow_action *action,
>  	if (flow->queue)
>  		(*flow->queue)[0] = queue->index;
>  	flow->rss.queue_num = 1;
> -	*action_flags |= MLX5_ACTION_QUEUE;
> +	*action_flags |= MLX5_FLOW_ACTION_QUEUE;
>  }
> 
>  /**
> @@ -811,7 +811,7 @@ flow_verbs_translate_action_rss(const struct
> rte_flow_action *action,
>  	memcpy(flow->key, rss->key, MLX5_RSS_HASH_KEY_LEN);
>  	flow->rss.types = rss->types;
>  	flow->rss.level = rss->level;
> -	*action_flags |= MLX5_ACTION_RSS;
> +	*action_flags |= MLX5_FLOW_ACTION_RSS;
>  }
> 
>  /**
> @@ -838,7 +838,7 @@ flow_verbs_translate_action_flag
>  		.size = size,
>  		.tag_id = mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT),
>  	};
> -	*action_flags |= MLX5_ACTION_MARK;
> +	*action_flags |= MLX5_FLOW_ACTION_MARK;
>  	flow_verbs_spec_add(dev_flow, &tag, size);
>  }
> 
> @@ -898,14 +898,14 @@ flow_verbs_translate_action_mark(const struct
> rte_flow_action *action,
>  	};
>  	struct mlx5_flow_verbs *verbs = &dev_flow->verbs;
> 
> -	if (*action_flags & MLX5_ACTION_FLAG) {
> +	if (*action_flags & MLX5_FLOW_ACTION_FLAG) {
>  		flow_verbs_mark_update(verbs, mark->id);
>  		size = 0;
>  	} else {
>  		tag.tag_id = mlx5_flow_mark_set(mark->id);
>  		flow_verbs_spec_add(dev_flow, &tag, size);
>  	}
> -	*action_flags |= MLX5_ACTION_MARK;
> +	*action_flags |= MLX5_FLOW_ACTION_MARK;
>  }
> 
>  /**
> @@ -954,7 +954,7 @@ flow_verbs_translate_action_count(struct rte_eth_dev
> *dev,
>  						  "cannot get counter"
>  						  " context.");
>  	}
> -	*action_flags |= MLX5_ACTION_COUNT;
> +	*action_flags |= MLX5_FLOW_ACTION_COUNT;
>  #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>  	counter.counter_set_handle = flow->counter->cs->handle;
>  	flow_verbs_spec_add(dev_flow, &counter, size);
> @@ -1116,7 +1116,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>  							     error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_FLAG;
> +			action_flags |= MLX5_FLOW_ACTION_FLAG;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_MARK:
>  			ret = mlx5_flow_validate_action_mark(actions,
> @@ -1124,14 +1124,14 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>  							     error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_MARK;
> +			action_flags |= MLX5_FLOW_ACTION_MARK;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_DROP:
>  			ret = mlx5_flow_validate_action_drop(action_flags,
>  							     error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_DROP;
> +			action_flags |= MLX5_FLOW_ACTION_DROP;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_QUEUE:
>  			ret = mlx5_flow_validate_action_queue(actions,
> @@ -1139,7 +1139,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>  							      error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_QUEUE;
> +			action_flags |= MLX5_FLOW_ACTION_QUEUE;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_RSS:
>  			ret = mlx5_flow_validate_action_rss(actions,
> @@ -1147,13 +1147,13 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>  							    error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_RSS;
> +			action_flags |= MLX5_FLOW_ACTION_RSS;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_COUNT:
>  			ret = mlx5_flow_validate_action_count(dev, error);
>  			if (ret < 0)
>  				return ret;
> -			action_flags |= MLX5_ACTION_COUNT;
> +			action_flags |= MLX5_FLOW_ACTION_COUNT;
>  			break;
>  		default:
>  			return rte_flow_error_set(error, ENOTSUP,
> @@ -1191,27 +1191,27 @@ flow_verbs_get_actions_and_size(const struct
> rte_flow_action actions[],
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_FLAG:
>  			size += sizeof(struct ibv_flow_spec_action_tag);
> -			detected_actions |= MLX5_ACTION_FLAG;
> +			detected_actions |= MLX5_FLOW_ACTION_FLAG;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_MARK:
>  			size += sizeof(struct ibv_flow_spec_action_tag);
> -			detected_actions |= MLX5_ACTION_MARK;
> +			detected_actions |= MLX5_FLOW_ACTION_MARK;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_DROP:
>  			size += sizeof(struct ibv_flow_spec_action_drop);
> -			detected_actions |= MLX5_ACTION_DROP;
> +			detected_actions |= MLX5_FLOW_ACTION_DROP;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_QUEUE:
> -			detected_actions |= MLX5_ACTION_QUEUE;
> +			detected_actions |= MLX5_FLOW_ACTION_QUEUE;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_RSS:
> -			detected_actions |= MLX5_ACTION_RSS;
> +			detected_actions |= MLX5_FLOW_ACTION_RSS;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_COUNT:
>  #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>  			size += sizeof(struct ibv_flow_spec_counter_action);
>  #endif
> -			detected_actions |= MLX5_ACTION_COUNT;
> +			detected_actions |= MLX5_FLOW_ACTION_COUNT;
>  			break;
>  		default:
>  			break;
> @@ -1519,7 +1519,7 @@ flow_verbs_remove(struct rte_eth_dev *dev, struct
> rte_flow *flow)
>  			verbs->flow = NULL;
>  		}
>  		if (verbs->hrxq) {
> -			if (flow->actions & MLX5_ACTION_DROP)
> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>  				mlx5_hrxq_drop_release(dev);
>  			else
>  				mlx5_hrxq_release(dev, verbs->hrxq);
> @@ -1578,7 +1578,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct
> rte_flow *flow,
> 
>  	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>  		verbs = &dev_flow->verbs;
> -		if (flow->actions & MLX5_ACTION_DROP) {
> +		if (flow->actions & MLX5_FLOW_ACTION_DROP) {
>  			verbs->hrxq = mlx5_hrxq_drop_new(dev);
>  			if (!verbs->hrxq) {
>  				rte_flow_error_set
> @@ -1628,7 +1628,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct
> rte_flow *flow,
>  	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>  		verbs = &dev_flow->verbs;
>  		if (verbs->hrxq) {
> -			if (flow->actions & MLX5_ACTION_DROP)
> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>  				mlx5_hrxq_drop_release(dev);
>  			else
>  				mlx5_hrxq_release(dev, verbs->hrxq);
> --
> 2.11.0
Acked-by: Ori Kam <orika@mellanox.com>

Thanks,
Ori

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

* Re: [dpdk-dev] [PATCH 1/7] net/mlx5: fix wrong flow action macro usage
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 1/7] net/mlx5: fix wrong flow action macro usage Yongseok Koh
@ 2018-10-09  7:45   ` Ori Kam
  0 siblings, 0 replies; 28+ messages in thread
From: Ori Kam @ 2018-10-09  7:45 UTC (permalink / raw)
  To: Yongseok Koh, Shahaf Shuler; +Cc: dev, Ori Kam



> -----Original Message-----
> From: Yongseok Koh
> Sent: Monday, October 8, 2018 9:02 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 1/7] net/mlx5: fix wrong flow action macro usage
> 
> Fixes: 5c83c536783c ("net/mlx5: add Direct Verbs final functions")
> Cc: Ori Kam <orika@mellanox.com>
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.h    | 10 ----------
>  drivers/net/mlx5/mlx5_flow_dv.c |  2 +-
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index a91f9911d..79d4a2619 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -65,16 +65,6 @@
>  	(MLX5_FLOW_LAYER_INNER_L2 | MLX5_FLOW_LAYER_INNER_L3 | \
>  	 MLX5_FLOW_LAYER_INNER_L4)
> 
> -/* Actions that modify the fate of matching traffic. */
> -#define MLX5_FLOW_FATE_DROP (1u << 0)
> -#define MLX5_FLOW_FATE_QUEUE (1u << 1)
> -#define MLX5_FLOW_FATE_RSS (1u << 2)
> -
> -/* Modify a packet. */
> -#define MLX5_FLOW_MOD_FLAG (1u << 0)
> -#define MLX5_FLOW_MOD_MARK (1u << 1)
> -#define MLX5_FLOW_MOD_COUNT (1u << 2)
> -
>  /* Actions */
>  #define MLX5_ACTION_DROP (1u << 0)
>  #define MLX5_ACTION_QUEUE (1u << 1)
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 7d325320a..5678dc35f 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -1251,7 +1251,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct
> rte_flow *flow,
>  	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>  		struct mlx5_flow_dv *dv = &dev_flow->dv;
>  		if (dv->hrxq) {
> -			if (flow->actions & MLX5_FLOW_FATE_DROP)
> +			if (flow->actions & MLX5_ACTION_DROP)
>  				mlx5_hrxq_drop_release(dev);
>  			else
>  				mlx5_hrxq_release(dev, dv->hrxq);
> --
> 2.11.0

Acked-by: Ori Kam <orika@mellanox.com>

Thanks,
Ori Kam

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

* Re: [dpdk-dev] [PATCH 4/7] net/mlx5: fix validation of VLAN ID in flow spec
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 4/7] net/mlx5: fix validation of VLAN ID in flow spec Yongseok Koh
@ 2018-10-09  7:47   ` Ori Kam
  2018-10-09 15:44     ` Ferruh Yigit
  0 siblings, 1 reply; 28+ messages in thread
From: Ori Kam @ 2018-10-09  7:47 UTC (permalink / raw)
  To: Yongseok Koh, Shahaf Shuler; +Cc: dev, Ori Kam



> -----Original Message-----
> From: Yongseok Koh
> Sent: Monday, October 8, 2018 9:02 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 4/7] net/mlx5: fix validation of VLAN ID in flow spec
> 
> This can cause crash by null pointer reference.
> 
> Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
> Cc: Ori Kam <orika@mellanox.com>
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 69afd4625..c497cacce 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -1012,6 +1012,7 @@ mlx5_flow_validate_item_vlan(const struct
> rte_flow_item *item,
>  		.tci = RTE_BE16(0x0fff),
>  		.inner_type = RTE_BE16(0xffff),
>  	};
> +	uint16_t vlan_tag = 0;
>  	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
>  	int ret;
>  	const uint32_t l34m = tunnel ? (MLX5_FLOW_LAYER_INNER_L3 |
> @@ -1037,11 +1038,15 @@ mlx5_flow_validate_item_vlan(const struct
> rte_flow_item *item,
>  					error);
>  	if (ret)
>  		return ret;
> +	if (spec) {
> +		vlan_tag = spec->tci;
> +		vlan_tag &= mask->tci;
> +	}
>  	/*
>  	 * From verbs perspective an empty VLAN is equivalent
>  	 * to a packet without VLAN layer.
>  	 */
> -	if (!spec->tci)
> +	if (!vlan_tag)
>  		return rte_flow_error_set(error, EINVAL,
> 
> RTE_FLOW_ERROR_TYPE_ITEM_SPEC,
>  					  item->spec,
> --
> 2.11.0

Acked-by: Ori Kam <orika@mellanox.com>

Thanks,
Ori Kam

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

* Re: [dpdk-dev] [PATCH 5/7] net/mlx5: fix flow validation for no fate action
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 5/7] net/mlx5: fix flow validation for no fate action Yongseok Koh
@ 2018-10-09  7:49   ` Ori Kam
  2018-10-10 12:24     ` Ferruh Yigit
  0 siblings, 1 reply; 28+ messages in thread
From: Ori Kam @ 2018-10-09  7:49 UTC (permalink / raw)
  To: Yongseok Koh, Shahaf Shuler; +Cc: dev



> -----Original Message-----
> From: Yongseok Koh
> Sent: Monday, October 8, 2018 9:02 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 5/7] net/mlx5: fix flow validation for no fate action
> 
> Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
> Fixes: f7adfffa3de1 ("net/mlx5: add Direct Verbs validation function")
> Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
> Cc: Ori Kam <orika@mellanox.com>
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c       | 12 +++---------
>  drivers/net/mlx5/mlx5_flow.h       |  3 +++
>  drivers/net/mlx5/mlx5_flow_dv.c    |  4 ++++
>  drivers/net/mlx5/mlx5_flow_tcf.c   |  4 ++++
>  drivers/net/mlx5/mlx5_flow_verbs.c |  4 ++++
>  5 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index c497cacce..30aa95f14 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -742,9 +742,7 @@ mlx5_flow_validate_action_drop(uint64_t action_flags,
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't drop and mark in same flow");
> -	if (action_flags &
> -	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> -	     MLX5_FLOW_ACTION_RSS))
> +	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions in"
> @@ -776,9 +774,7 @@ mlx5_flow_validate_action_queue(const struct
> rte_flow_action *action,
>  	struct priv *priv = dev->data->dev_private;
>  	const struct rte_flow_action_queue *queue = action->conf;
> 
> -	if (action_flags &
> -	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> -	     MLX5_FLOW_ACTION_RSS))
> +	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions in"
> @@ -821,9 +817,7 @@ mlx5_flow_validate_action_rss(const struct
> rte_flow_action *action,
>  	const struct rte_flow_action_rss *rss = action->conf;
>  	unsigned int i;
> 
> -	if (action_flags &
> -	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> -	     MLX5_FLOW_ACTION_RSS))
> +	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions"
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 309e4d4f2..12de841e8 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -79,6 +79,9 @@
>  #define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
>  #define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
> 
> +#define MLX5_FLOW_FATE_ACTIONS \
> +	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> MLX5_FLOW_ACTION_RSS)
> +
>  #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 e6c84d444..2fb1d9ee7 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -288,6 +288,10 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
> rte_flow_attr *attr,
>  						  "action not supported");
>  		}
>  	}
> +	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
> +					  "no fate action is found");
>  	return 0;
>  }
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index c87046365..4c660d72b 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -697,6 +697,10 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>  						  "action not supported");
>  		}
>  	}
> +	if (!(action_flags & MLX5_TCF_FATE_ACTIONS))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
> +					  "no fate action is found");
>  	return 0;
>  }
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
> b/drivers/net/mlx5/mlx5_flow_verbs.c
> index 0ecbc8121..3df467214 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -1162,6 +1162,10 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>  						  "action not supported");
>  		}
>  	}
> +	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
> +					  "no fate action is found");
>  	return 0;
>  }
> 
> --
> 2.11.0

Acked-by: Ori Kam <orika@mellanox.com>

Thanks,
Ori Kam

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

* Re: [dpdk-dev] [PATCH 7/7] net/mlx5: fix errno values for flow engine
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 7/7] net/mlx5: fix errno values for flow engine Yongseok Koh
@ 2018-10-09  7:53   ` Ori Kam
  2018-10-10 12:59     ` Ferruh Yigit
  0 siblings, 1 reply; 28+ messages in thread
From: Ori Kam @ 2018-10-09  7:53 UTC (permalink / raw)
  To: Yongseok Koh, Shahaf Shuler; +Cc: dev



> -----Original Message-----
> From: Yongseok Koh
> Sent: Monday, October 8, 2018 9:02 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 7/7] net/mlx5: fix errno values for flow engine
> 
> Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
> Fixes: f7adfffa3de1 ("net/mlx5: add Direct Verbs validation function")
> Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
> Cc: orika@mellanox.com
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c       | 70 +++++++++++++++++++-------------------
>  drivers/net/mlx5/mlx5_flow_dv.c    |  2 +-
>  drivers/net/mlx5/mlx5_flow_tcf.c   |  8 ++---
>  drivers/net/mlx5/mlx5_flow_verbs.c |  4 +--
>  4 files changed, 42 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 30aa95f14..ed60c40f9 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -452,10 +452,10 @@ mlx5_flow_item_acceptable(const struct
> rte_flow_item *item,
>  		}
>  		ret = memcmp(spec, last, size);
>  		if (ret != 0)
> -			return rte_flow_error_set(error, ENOTSUP,
> +			return rte_flow_error_set(error, EINVAL,
> 
> RTE_FLOW_ERROR_TYPE_ITEM,
>  						  item,
> -						  "range is not supported");
> +						  "range is not valid");
>  	}
>  	return 0;
>  }
> @@ -657,15 +657,15 @@ mlx5_flow_validate_action_flag(uint64_t
> action_flags,
>  {
> 
>  	if (action_flags & MLX5_FLOW_ACTION_DROP)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		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, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't mark and flag in same flow");
>  	if (action_flags & MLX5_FLOW_ACTION_FLAG)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 flag"
>  					  " actions in same flow");
> @@ -704,17 +704,17 @@ mlx5_flow_validate_action_mark(const struct
> rte_flow_action *action,
>  					  "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, ENOTSUP,
> +		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, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't flag and mark in same flow");
>  	if (action_flags & MLX5_FLOW_ACTION_MARK)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> -					  "can't have 2 flag actions in same"
> +					  "can't have 2 mark actions in same"
>  					  " flow");
>  	return 0;
>  }
> @@ -735,15 +735,15 @@ mlx5_flow_validate_action_drop(uint64_t
> action_flags,
>  			       struct rte_flow_error *error)
>  {
>  	if (action_flags & MLX5_FLOW_ACTION_FLAG)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		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, ENOTSUP,
> +		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)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions in"
>  					  " same flow");
> @@ -775,7 +775,7 @@ mlx5_flow_validate_action_queue(const struct
> rte_flow_action *action,
>  	const struct rte_flow_action_queue *queue = action->conf;
> 
>  	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions in"
>  					  " same flow");
> @@ -818,7 +818,7 @@ mlx5_flow_validate_action_rss(const struct
> rte_flow_action *action,
>  	unsigned int i;
> 
>  	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can't have 2 fate actions"
>  					  " in same flow");
> @@ -931,7 +931,7 @@ mlx5_flow_validate_attributes(struct rte_eth_dev
> *dev,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
>  					  NULL, "transfer is not supported");
>  	if (!attributes->ingress)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
>  					  NULL,
>  					  "ingress attribute is mandatory");
> @@ -1017,11 +1017,11 @@ mlx5_flow_validate_item_vlan(const struct
> rte_flow_item *item,
>  					MLX5_FLOW_LAYER_OUTER_VLAN;
> 
>  	if (item_flags & vlanm)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "VLAN layer already configured");
>  	else if ((item_flags & l34m) != 0)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "L2 layer cannot follow L3/L4 layer");
>  	if (!mask)
> @@ -1085,7 +1085,7 @@ mlx5_flow_validate_item_ipv4(const struct
> rte_flow_item *item,
>  					  "multiple L3 layers not supported");
>  	else if (item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L4 :
>  					MLX5_FLOW_LAYER_OUTER_L4))
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "L3 cannot follow an L4 layer.");
>  	if (!mask)
> @@ -1141,7 +1141,7 @@ mlx5_flow_validate_item_ipv6(const struct
> rte_flow_item *item,
>  					  "multiple L3 layers not supported");
>  	else if (item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L4 :
>  					MLX5_FLOW_LAYER_OUTER_L4))
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "L3 cannot follow an L4 layer.");
>  	/*
> @@ -1192,18 +1192,18 @@ mlx5_flow_validate_item_udp(const struct
> rte_flow_item *item,
>  	int ret;
> 
>  	if (target_protocol != 0xff && target_protocol != IPPROTO_UDP)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "protocol filtering not compatible"
>  					  " with UDP layer");
>  	if (!(item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L3 :
>  				     MLX5_FLOW_LAYER_OUTER_L3)))
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "L3 is mandatory to filter on L4");
>  	if (item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L4 :
>  				   MLX5_FLOW_LAYER_OUTER_L4))
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "L4 layer is already present");
>  	if (!mask)
> @@ -1243,18 +1243,18 @@ mlx5_flow_validate_item_tcp(const struct
> rte_flow_item *item,
>  	int ret;
> 
>  	if (target_protocol != 0xff && target_protocol != IPPROTO_TCP)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "protocol filtering not compatible"
>  					  " with TCP layer");
>  	if (!(item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L3 :
>  				     MLX5_FLOW_LAYER_OUTER_L3)))
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "L3 is mandatory to filter on L4");
>  	if (item_flags & (tunnel ? MLX5_FLOW_LAYER_INNER_L4 :
>  				   MLX5_FLOW_LAYER_OUTER_L4))
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "L4 layer is already present");
>  	if (!mask)
> @@ -1307,7 +1307,7 @@ mlx5_flow_validate_item_vxlan(const struct
> rte_flow_item *item,
>  	 * https://tools.ietf.org/html/rfc7348
>  	 */
>  	if (!(item_flags & MLX5_FLOW_LAYER_OUTER_L4_UDP))
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "no outer UDP layer found");
>  	if (!mask)
> @@ -1335,11 +1335,11 @@ mlx5_flow_validate_item_vxlan(const struct
> rte_flow_item *item,
>  	 * currently refused.
>  	 */
>  	if (!vlan_id)
> -		return rte_flow_error_set(error, EINVAL,
> +		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "VXLAN vni cannot be 0");
>  	if (!(item_flags & MLX5_FLOW_LAYER_OUTER))
> -		return rte_flow_error_set(error, EINVAL,
> +		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "VXLAN tunnel must be fully
> defined");
>  	return 0;
> @@ -1393,7 +1393,7 @@ mlx5_flow_validate_item_vxlan_gpe(const struct
> rte_flow_item *item,
>  	 * https://tools.ietf.org/html/rfc7348
>  	 */
>  	if (!(item_flags & MLX5_FLOW_LAYER_OUTER_L4_UDP))
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "no outer UDP layer found");
>  	if (!mask)
> @@ -1407,7 +1407,7 @@ mlx5_flow_validate_item_vxlan_gpe(const struct
> rte_flow_item *item,
>  		return ret;
>  	if (spec) {
>  		if (spec->protocol)
> -			return rte_flow_error_set(error, EINVAL,
> +			return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ITEM,
>  						  item,
>  						  "VxLAN-GPE protocol"
> @@ -1426,11 +1426,11 @@ mlx5_flow_validate_item_vxlan_gpe(const struct
> rte_flow_item *item,
>  	 * is currently refused.
>  	 */
>  	if (!vlan_id)
> -		return rte_flow_error_set(error, EINVAL,
> +		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "VXLAN-GPE vni cannot be 0");
>  	if (!(item_flags & MLX5_FLOW_LAYER_OUTER))
> -		return rte_flow_error_set(error, EINVAL,
> +		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "VXLAN-GPE tunnel must be fully"
>  					  " defined");
> @@ -1463,7 +1463,7 @@ mlx5_flow_validate_item_gre(const struct
> rte_flow_item *item,
>  	int ret;
> 
>  	if (target_protocol != 0xff && target_protocol != IPPROTO_GRE)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "protocol filtering not compatible"
>  					  " with this GRE layer");
> @@ -1520,7 +1520,7 @@ mlx5_flow_validate_item_mpls(const struct
> rte_flow_item *item __rte_unused,
>  	int ret;
> 
>  	if (target_protocol != 0xff && target_protocol != IPPROTO_MPLS)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
>  					  "protocol filtering not compatible"
>  					  " with MPLS layer");
> @@ -2336,7 +2336,7 @@ mlx5_flow_query_count(struct rte_flow *flow
> __rte_unused,
>  		}
>  		return 0;
>  	}
> -	return rte_flow_error_set(error, ENOTSUP,
> +	return rte_flow_error_set(error, EINVAL,
>  				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>  				  NULL,
>  				  "flow does not have counter");
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 2fb1d9ee7..3bb462ceb 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -79,7 +79,7 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev,
>  					  NULL,
>  					  "transfer is not supported");
>  	if (!attributes->ingress)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
>  					  NULL,
>  					  "ingress attribute is mandatory");
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index 0406e84f6..91f6ef678 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -291,7 +291,7 @@ flow_tcf_item_mask(const struct rte_flow_item *item,
> const void *mask_default,
>  		if (item->last &&
>  		    (((const uint8_t *)item->spec)[i] & mask[i]) !=
>  		    (((const uint8_t *)item->last)[i] & mask[i])) {
> -			rte_flow_error_set(error, ENOTSUP,
> +			rte_flow_error_set(error, EINVAL,
> 
> RTE_FLOW_ERROR_TYPE_ITEM_LAST,
>  					   item->last,
>  					   "range between \"spec\" and
> \"last\""
> @@ -383,7 +383,7 @@ flow_tcf_validate_attributes(const struct rte_flow_attr
> *attr,
>  					  attr,
>  					  "lowest priority level is 0xfffe");
>  	if (!attr->ingress)
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(error, EINVAL,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
>  					  attr, "only ingress is supported");
>  	if (attr->egress)
> @@ -655,7 +655,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>  		case RTE_FLOW_ACTION_TYPE_PORT_ID:
>  			if (action_flags & MLX5_TCF_FATE_ACTIONS)
>  				return rte_flow_error_set
> -					(error, ENOTSUP,
> +					(error, EINVAL,
>  					 RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
>  					 "can't have multiple fate actions");
>  			conf.port_id = actions->conf;
> @@ -678,7 +678,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>  		case RTE_FLOW_ACTION_TYPE_DROP:
>  			if (action_flags & MLX5_TCF_FATE_ACTIONS)
>  				return rte_flow_error_set
> -					(error, ENOTSUP,
> +					(error, EINVAL,
>  					 RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
>  					 "can't have multiple fate actions");
>  			action_flags |= MLX5_FLOW_ACTION_DROP;
> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
> b/drivers/net/mlx5/mlx5_flow_verbs.c
> index 3df467214..696447674 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -948,7 +948,7 @@ flow_verbs_translate_action_count(struct rte_eth_dev
> *dev,
>  		flow->counter = flow_verbs_counter_new(dev, count->shared,
>  						       count->id);
>  		if (!flow->counter)
> -			return rte_flow_error_set(error, ENOTSUP,
> +			return rte_flow_error_set(error, rte_errno,
> 
> RTE_FLOW_ERROR_TYPE_ACTION,
>  						  action,
>  						  "cannot get counter"
> @@ -1094,7 +1094,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>  			if (next_protocol != 0xff &&
>  			    next_protocol != IPPROTO_MPLS)
>  				return rte_flow_error_set
> -					(error, ENOTSUP,
> +					(error, EINVAL,
>  					 RTE_FLOW_ERROR_TYPE_ITEM,
> items,
>  					 "protocol filtering not compatible"
>  					 " with MPLS layer");
> --
> 2.11.0

Acked-by: Ori Kam <orika@mellanox.com>

Thanks,
Ori Kam

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

* Re: [dpdk-dev] [PATCH 2/7] net/mlx5: use standard IP protocol numbers
  2018-10-09  7:37   ` Ori Kam
@ 2018-10-09  8:02     ` Yongseok Koh
  0 siblings, 0 replies; 28+ messages in thread
From: Yongseok Koh @ 2018-10-09  8:02 UTC (permalink / raw)
  To: Ori Kam; +Cc: Shahaf Shuler, dev


> On Oct 9, 2018, at 12:37 AM, Ori Kam <orika@mellanox.com> wrote:
> 
> Hi,
> PSB
> 
>> -----Original Message-----
>> From: Yongseok Koh
>> Sent: Monday, October 8, 2018 9:02 PM
>> To: Shahaf Shuler <shahafs@mellanox.com>
>> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
>> <orika@mellanox.com>
>> Subject: [PATCH 2/7] net/mlx5: use standard IP protocol numbers
>> 
>> Fixes: 0f8775dd8f1c ("net/mlx5: add support for multiple flow drivers")
> 
> I don't think this is a fix since there is no issue with the implementation,
> you are just beautify the code.

-#define MLX5_IP_PROTOCOL_MPLS 147

MPLS protocol number was wrong, it should be 137

Thanks,
Yongseok

> Otherwise
> Acked-by: Ori Kam <orika@mellanox.com>
> 
>> Cc: Ori Kam <orika@mellanox.com>
>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> drivers/net/mlx5/mlx5_flow.c       | 9 +++++----
>> drivers/net/mlx5/mlx5_flow.h       | 9 ++++-----
>> drivers/net/mlx5/mlx5_flow_verbs.c | 7 ++++---
>> 3 files changed, 13 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
>> index 8007bf10f..ef5e4684f 100644
>> --- a/drivers/net/mlx5/mlx5_flow.c
>> +++ b/drivers/net/mlx5/mlx5_flow.c
>> @@ -3,6 +3,7 @@
>>  * Copyright 2016 Mellanox Technologies, Ltd
>>  */
>> 
>> +#include <netinet/in.h>
>> #include <sys/queue.h>
>> #include <stdalign.h>
>> #include <stdint.h>
>> @@ -1188,7 +1189,7 @@ mlx5_flow_validate_item_udp(const struct
>> rte_flow_item *item,
>> 	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
>> 	int ret;
>> 
>> -	if (target_protocol != 0xff && target_protocol !=
>> MLX5_IP_PROTOCOL_UDP)
>> +	if (target_protocol != 0xff && target_protocol != IPPROTO_UDP)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ITEM,
>> item,
>> 					  "protocol filtering not compatible"
>> @@ -1239,7 +1240,7 @@ mlx5_flow_validate_item_tcp(const struct
>> rte_flow_item *item,
>> 	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
>> 	int ret;
>> 
>> -	if (target_protocol != 0xff && target_protocol !=
>> MLX5_IP_PROTOCOL_TCP)
>> +	if (target_protocol != 0xff && target_protocol != IPPROTO_TCP)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ITEM,
>> item,
>> 					  "protocol filtering not compatible"
>> @@ -1459,7 +1460,7 @@ mlx5_flow_validate_item_gre(const struct
>> rte_flow_item *item,
>> 	const struct rte_flow_item_gre *mask = item->mask;
>> 	int ret;
>> 
>> -	if (target_protocol != 0xff && target_protocol !=
>> MLX5_IP_PROTOCOL_GRE)
>> +	if (target_protocol != 0xff && target_protocol != IPPROTO_GRE)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ITEM,
>> item,
>> 					  "protocol filtering not compatible"
>> @@ -1516,7 +1517,7 @@ mlx5_flow_validate_item_mpls(const struct
>> rte_flow_item *item __rte_unused,
>> 	const struct rte_flow_item_mpls *mask = item->mask;
>> 	int ret;
>> 
>> -	if (target_protocol != 0xff && target_protocol !=
>> MLX5_IP_PROTOCOL_MPLS)
>> +	if (target_protocol != 0xff && target_protocol != IPPROTO_MPLS)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ITEM,
>> item,
>> 					  "protocol filtering not compatible"
>> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
>> index 79d4a2619..1ac871b3a 100644
>> --- a/drivers/net/mlx5/mlx5_flow.h
>> +++ b/drivers/net/mlx5/mlx5_flow.h
>> @@ -5,6 +5,7 @@
>> #ifndef RTE_PMD_MLX5_FLOW_H_
>> #define RTE_PMD_MLX5_FLOW_H_
>> 
>> +#include <netinet/in.h>
>> #include <sys/queue.h>
>> #include <stdalign.h>
>> #include <stdint.h>
>> @@ -78,11 +79,9 @@
>> #define MLX5_ACTION_OF_SET_VLAN_VID (1u << 9)
>> #define MLX5_ACTION_OF_SET_VLAN_PCP (1u << 10)
>> 
>> -/* possible L3 layers protocols filtering. */
>> -#define MLX5_IP_PROTOCOL_TCP 6
>> -#define MLX5_IP_PROTOCOL_UDP 17
>> -#define MLX5_IP_PROTOCOL_GRE 47
>> -#define MLX5_IP_PROTOCOL_MPLS 147
>> +#ifndef IPPROTO_MPLS
>> +#define IPPROTO_MPLS 137
>> +#endif
>> 
>> /* Internent Protocol versions. */
>> #define MLX5_VXLAN 4789
>> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
>> b/drivers/net/mlx5/mlx5_flow_verbs.c
>> index 05ab5fdad..076bb39e6 100644
>> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
>> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
>> @@ -2,6 +2,7 @@
>>  * Copyright 2018 Mellanox Technologies, Ltd
>>  */
>> 
>> +#include <netinet/in.h>
>> #include <sys/queue.h>
>> #include <stdalign.h>
>> #include <stdint.h>
>> @@ -683,11 +684,11 @@ flow_verbs_translate_item_gre(const struct
>> rte_flow_item *item __rte_unused,
>> 	if (*item_flags & MLX5_FLOW_LAYER_OUTER_L3_IPV4)
>> 		flow_verbs_item_gre_ip_protocol_update(verbs->attr,
>> 
>> IBV_FLOW_SPEC_IPV4_EXT,
>> -
>> MLX5_IP_PROTOCOL_GRE);
>> +						       IPPROTO_GRE);
>> 	else
>> 		flow_verbs_item_gre_ip_protocol_update(verbs->attr,
>> 						       IBV_FLOW_SPEC_IPV6,
>> -
>> MLX5_IP_PROTOCOL_GRE);
>> +						       IPPROTO_GRE);
>> 	flow_verbs_spec_add(dev_flow, &tunnel, size);
>> 	verbs->attr->priority = MLX5_PRIORITY_MAP_L2;
>> 	*item_flags |= MLX5_FLOW_LAYER_GRE;
>> @@ -1091,7 +1092,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>> 			if (ret < 0)
>> 				return ret;
>> 			if (next_protocol != 0xff &&
>> -			    next_protocol != MLX5_IP_PROTOCOL_MPLS)
>> +			    next_protocol != IPPROTO_MPLS)
>> 				return rte_flow_error_set
>> 					(error, ENOTSUP,
>> 					 RTE_FLOW_ERROR_TYPE_ITEM,
>> items,
>> --
>> 2.11.0
> 

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

* Re: [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros
  2018-10-09  7:43   ` Ori Kam
@ 2018-10-09  8:05     ` Yongseok Koh
  2018-10-09  8:17     ` Yongseok Koh
  1 sibling, 0 replies; 28+ messages in thread
From: Yongseok Koh @ 2018-10-09  8:05 UTC (permalink / raw)
  To: Ori Kam, Shahaf Shuler; +Cc: dev


> On Oct 9, 2018, at 12:43 AM, Ori Kam <orika@mellanox.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Yongseok Koh
>> Sent: Monday, October 8, 2018 9:02 PM
>> To: Shahaf Shuler <shahafs@mellanox.com>
>> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
>> <orika@mellanox.com>
>> Subject: [PATCH 3/7] net/mlx5: rename flow macros
>> 
>> MLX5_ACTION_* -> MLX5_FLOW_ACTION_*
>> MLX5_VXLAN* -> MLX5_UDP_PORT_VXLAN*
>> 
>> Fixes: 0f8775dd8f1c ("net/mlx5: add support for multiple flow drivers")
>> Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
>> Cc: Ori Kam <orika@mellanox.com>
>> 
> I don't think this is a fix since there is no issue with the implementation,
> you are just beautify the code.
> 
> Otherwise acked:

Right, but I just added the tags for reference. As this doesn't have "Cc: stable@",
it has no effect.

Shahaf, I'm okay if you drop the "Fixes:" tag when you merge it.
I'll leave it as your choice.

Thanks,
Yongseok

>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> drivers/net/mlx5/mlx5_flow.c       | 31 +++++++++++++++------------
>> drivers/net/mlx5/mlx5_flow.h       | 28 ++++++++++++------------
>> drivers/net/mlx5/mlx5_flow_dv.c    | 24 ++++++++++-----------
>> drivers/net/mlx5/mlx5_flow_tcf.c   | 26 +++++++++++-----------
>> drivers/net/mlx5/mlx5_flow_verbs.c | 44 +++++++++++++++++++------------------
>> -
>> 5 files changed, 78 insertions(+), 75 deletions(-)
>> 
>> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
>> index ef5e4684f..69afd4625 100644
>> --- a/drivers/net/mlx5/mlx5_flow.c
>> +++ b/drivers/net/mlx5/mlx5_flow.c
>> @@ -537,7 +537,7 @@ mlx5_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 &
>> -			    (MLX5_ACTION_FLAG | MLX5_ACTION_MARK));
>> +			    (MLX5_FLOW_ACTION_FLAG |
>> MLX5_FLOW_ACTION_MARK));
>> 	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
>> 	unsigned int i;
>> 
>> @@ -581,7 +581,7 @@ mlx5_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 &
>> -			    (MLX5_ACTION_FLAG | MLX5_ACTION_MARK));
>> +			    (MLX5_FLOW_ACTION_FLAG |
>> MLX5_FLOW_ACTION_MARK));
>> 	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
>> 	unsigned int i;
>> 
>> @@ -656,15 +656,15 @@ mlx5_flow_validate_action_flag(uint64_t
>> action_flags,
>> 			       struct rte_flow_error *error)
>> {
>> 
>> -	if (action_flags & MLX5_ACTION_DROP)
>> +	if (action_flags & MLX5_FLOW_ACTION_DROP)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't drop and flag in same flow");
>> -	if (action_flags & MLX5_ACTION_MARK)
>> +	if (action_flags & MLX5_FLOW_ACTION_MARK)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't mark and flag in same flow");
>> -	if (action_flags & MLX5_ACTION_FLAG)
>> +	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 flag"
>> @@ -703,15 +703,15 @@ mlx5_flow_validate_action_mark(const struct
>> rte_flow_action *action,
>> 					  &mark->id,
>> 					  "mark id must in 0 <= id < "
>> 
>> RTE_STR(MLX5_FLOW_MARK_MAX));
>> -	if (action_flags & MLX5_ACTION_DROP)
>> +	if (action_flags & MLX5_FLOW_ACTION_DROP)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't drop and mark in same flow");
>> -	if (action_flags & MLX5_ACTION_FLAG)
>> +	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't flag and mark in same flow");
>> -	if (action_flags & MLX5_ACTION_MARK)
>> +	if (action_flags & MLX5_FLOW_ACTION_MARK)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 flag actions in same"
>> @@ -734,16 +734,17 @@ int
>> mlx5_flow_validate_action_drop(uint64_t action_flags,
>> 			       struct rte_flow_error *error)
>> {
>> -	if (action_flags & MLX5_ACTION_FLAG)
>> +	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't drop and flag in same flow");
>> -	if (action_flags & MLX5_ACTION_MARK)
>> +	if (action_flags & MLX5_FLOW_ACTION_MARK)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't drop and mark in same flow");
>> 	if (action_flags &
>> -		(MLX5_ACTION_DROP | MLX5_ACTION_QUEUE |
>> MLX5_ACTION_RSS))
>> +	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
>> +	     MLX5_FLOW_ACTION_RSS))
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 fate actions in"
>> @@ -776,7 +777,8 @@ mlx5_flow_validate_action_queue(const struct
>> rte_flow_action *action,
>> 	const struct rte_flow_action_queue *queue = action->conf;
>> 
>> 	if (action_flags &
>> -	    (MLX5_ACTION_DROP | MLX5_ACTION_QUEUE |
>> MLX5_ACTION_RSS))
>> +	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
>> +	     MLX5_FLOW_ACTION_RSS))
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 fate actions in"
>> @@ -820,7 +822,8 @@ mlx5_flow_validate_action_rss(const struct
>> rte_flow_action *action,
>> 	unsigned int i;
>> 
>> 	if (action_flags &
>> -	    (MLX5_ACTION_DROP | MLX5_ACTION_QUEUE |
>> MLX5_ACTION_RSS))
>> +	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
>> +	     MLX5_FLOW_ACTION_RSS))
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 fate actions"
>> @@ -2304,7 +2307,7 @@ mlx5_flow_query_count(struct rte_flow *flow
>> __rte_unused,
>> 		      struct rte_flow_error *error)
>> {
>> #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>> -	if (flow->actions & MLX5_ACTION_COUNT) {
>> +	if (flow->actions & MLX5_FLOW_ACTION_COUNT) {
>> 		struct rte_flow_query_count *qc = data;
>> 		uint64_t counters[2] = {0, 0};
>> 		struct ibv_query_counter_set_attr query_cs_attr = {
>> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
>> index 1ac871b3a..309e4d4f2 100644
>> --- a/drivers/net/mlx5/mlx5_flow.h
>> +++ b/drivers/net/mlx5/mlx5_flow.h
>> @@ -67,25 +67,25 @@
>> 	 MLX5_FLOW_LAYER_INNER_L4)
>> 
>> /* Actions */
>> -#define MLX5_ACTION_DROP (1u << 0)
>> -#define MLX5_ACTION_QUEUE (1u << 1)
>> -#define MLX5_ACTION_RSS (1u << 2)
>> -#define MLX5_ACTION_FLAG (1u << 3)
>> -#define MLX5_ACTION_MARK (1u << 4)
>> -#define MLX5_ACTION_COUNT (1u << 5)
>> -#define MLX5_ACTION_PORT_ID (1u << 6)
>> -#define MLX5_ACTION_OF_POP_VLAN (1u << 7)
>> -#define MLX5_ACTION_OF_PUSH_VLAN (1u << 8)
>> -#define MLX5_ACTION_OF_SET_VLAN_VID (1u << 9)
>> -#define MLX5_ACTION_OF_SET_VLAN_PCP (1u << 10)
>> +#define MLX5_FLOW_ACTION_DROP (1u << 0)
>> +#define MLX5_FLOW_ACTION_QUEUE (1u << 1)
>> +#define MLX5_FLOW_ACTION_RSS (1u << 2)
>> +#define MLX5_FLOW_ACTION_FLAG (1u << 3)
>> +#define MLX5_FLOW_ACTION_MARK (1u << 4)
>> +#define MLX5_FLOW_ACTION_COUNT (1u << 5)
>> +#define MLX5_FLOW_ACTION_PORT_ID (1u << 6)
>> +#define MLX5_FLOW_ACTION_OF_POP_VLAN (1u << 7)
>> +#define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
>> +#define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
>> +#define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
>> 
>> #ifndef IPPROTO_MPLS
>> #define IPPROTO_MPLS 137
>> #endif
>> 
>> -/* Internent Protocol versions. */
>> -#define MLX5_VXLAN 4789
>> -#define MLX5_VXLAN_GPE 4790
>> +/* UDP port numbers for VxLAN. */
>> +#define MLX5_UDP_PORT_VXLAN 4789
>> +#define MLX5_UDP_PORT_VXLAN_GPE 4790
>> 
>> /* Priority reserved for default flows. */
>> #define MLX5_FLOW_PRIO_RSVD ((uint32_t)-1)
>> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
>> b/drivers/net/mlx5/mlx5_flow_dv.c
>> index 5678dc35f..e6c84d444 100644
>> --- a/drivers/net/mlx5/mlx5_flow_dv.c
>> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
>> @@ -236,7 +236,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
>> rte_flow_attr *attr,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_FLAG;
>> +			action_flags |= MLX5_FLOW_ACTION_FLAG;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_MARK:
>> @@ -245,7 +245,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
>> rte_flow_attr *attr,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_MARK;
>> +			action_flags |= MLX5_FLOW_ACTION_MARK;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> @@ -253,7 +253,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
>> rte_flow_attr *attr,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_DROP;
>> +			action_flags |= MLX5_FLOW_ACTION_DROP;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_QUEUE:
>> @@ -262,7 +262,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
>> rte_flow_attr *attr,
>> 							      error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_QUEUE;
>> +			action_flags |= MLX5_FLOW_ACTION_QUEUE;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_RSS:
>> @@ -271,14 +271,14 @@ flow_dv_validate(struct rte_eth_dev *dev, const
>> struct rte_flow_attr *attr,
>> 							    error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_RSS;
>> +			action_flags |= MLX5_FLOW_ACTION_RSS;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_COUNT:
>> 			ret = mlx5_flow_validate_action_count(dev, error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_COUNT;
>> +			action_flags |= MLX5_FLOW_ACTION_COUNT;
>> 			++actions_n;
>> 			break;
>> 		default:
>> @@ -838,8 +838,8 @@ flow_dv_translate_item_vxlan(void *matcher, void
>> *key,
>> 					 outer_headers);
>> 		headers_v = MLX5_ADDR_OF(fte_match_param, key,
>> outer_headers);
>> 	}
>> -	dport = item->type == RTE_FLOW_ITEM_TYPE_VXLAN ? MLX5_VXLAN :
>> -							 MLX5_VXLAN_GPE;
>> +	dport = item->type == RTE_FLOW_ITEM_TYPE_VXLAN ?
>> +		MLX5_UDP_PORT_VXLAN : MLX5_UDP_PORT_VXLAN_GPE;
>> 	if (!MLX5_GET16(fte_match_set_lyr_2_4, headers_v, udp_dport)) {
>> 		MLX5_SET(fte_match_set_lyr_2_4, headers_m, udp_dport,
>> 0xFFFF);
>> 		MLX5_SET(fte_match_set_lyr_2_4, headers_v, udp_dport,
>> dport);
>> @@ -978,7 +978,7 @@ flow_dv_create_action(const struct rte_flow_action
>> *action,
>> 		break;
>> 	case RTE_FLOW_ACTION_TYPE_DROP:
>> 		dev_flow->dv.actions[actions_n].type =
>> MLX5DV_FLOW_ACTION_DROP;
>> -		flow->actions |= MLX5_ACTION_DROP;
>> +		flow->actions |= MLX5_FLOW_ACTION_DROP;
>> 		break;
>> 	case RTE_FLOW_ACTION_TYPE_QUEUE:
>> 		queue = action->conf;
>> @@ -1195,7 +1195,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct
>> rte_flow *flow,
>> 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>> 		dv = &dev_flow->dv;
>> 		n = dv->actions_n;
>> -		if (flow->actions & MLX5_ACTION_DROP) {
>> +		if (flow->actions & MLX5_FLOW_ACTION_DROP) {
>> 			dv->hrxq = mlx5_hrxq_drop_new(dev);
>> 			if (!dv->hrxq) {
>> 				rte_flow_error_set
>> @@ -1251,7 +1251,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct
>> rte_flow *flow,
>> 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>> 		struct mlx5_flow_dv *dv = &dev_flow->dv;
>> 		if (dv->hrxq) {
>> -			if (flow->actions & MLX5_ACTION_DROP)
>> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>> 				mlx5_hrxq_drop_release(dev);
>> 			else
>> 				mlx5_hrxq_release(dev, dv->hrxq);
>> @@ -1318,7 +1318,7 @@ flow_dv_remove(struct rte_eth_dev *dev, struct
>> rte_flow *flow)
>> 			dv->flow = NULL;
>> 		}
>> 		if (dv->hrxq) {
>> -			if (flow->actions & MLX5_ACTION_DROP)
>> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>> 				mlx5_hrxq_drop_release(dev);
>> 			else
>> 				mlx5_hrxq_release(dev, dv->hrxq);
>> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
>> b/drivers/net/mlx5/mlx5_flow_tcf.c
>> index 14376188e..c87046365 100644
>> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
>> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
>> @@ -225,7 +225,7 @@ struct flow_tcf_ptoi {
>> 	unsigned int ifindex; /**< Network interface index. */
>> };
>> 
>> -#define MLX5_TCF_FATE_ACTIONS (MLX5_ACTION_DROP |
>> MLX5_ACTION_PORT_ID)
>> +#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP |
>> MLX5_FLOW_ACTION_PORT_ID)
>> 
>> /**
>>  * Retrieve mask for pattern item.
>> @@ -668,7 +668,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>> 					 conf.port_id,
>> 					 "missing data to convert port ID to"
>> 					 " ifindex");
>> -			action_flags |= MLX5_ACTION_PORT_ID;
>> +			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> 			if (action_flags & MLX5_TCF_FATE_ACTIONS)
>> @@ -676,19 +676,19 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>> 					(error, ENOTSUP,
>> 					 RTE_FLOW_ERROR_TYPE_ACTION,
>> actions,
>> 					 "can't have multiple fate actions");
>> -			action_flags |= MLX5_ACTION_DROP;
>> +			action_flags |= MLX5_FLOW_ACTION_DROP;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
>> -			action_flags |= MLX5_ACTION_OF_POP_VLAN;
>> +			action_flags |= MLX5_FLOW_ACTION_OF_POP_VLAN;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
>> -			action_flags |= MLX5_ACTION_OF_PUSH_VLAN;
>> +			action_flags |=
>> MLX5_FLOW_ACTION_OF_PUSH_VLAN;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
>> -			action_flags |= MLX5_ACTION_OF_SET_VLAN_VID;
>> +			action_flags |=
>> MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
>> -			action_flags |= MLX5_ACTION_OF_SET_VLAN_PCP;
>> +			action_flags |=
>> MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
>> 			break;
>> 		default:
>> 			return rte_flow_error_set(error, ENOTSUP,
>> @@ -809,26 +809,26 @@ flow_tcf_get_actions_and_size(const struct
>> rte_flow_action actions[],
>> 				SZ_NLATTR_STRZ_OF("mirred") +
>> 				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
>> 				SZ_NLATTR_TYPE_OF(struct tc_mirred);
>> -			flags |= MLX5_ACTION_PORT_ID;
>> +			flags |= MLX5_FLOW_ACTION_PORT_ID;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> 			size += SZ_NLATTR_NEST + /* na_act_index. */
>> 				SZ_NLATTR_STRZ_OF("gact") +
>> 				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
>> 				SZ_NLATTR_TYPE_OF(struct tc_gact);
>> -			flags |= MLX5_ACTION_DROP;
>> +			flags |= MLX5_FLOW_ACTION_DROP;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
>> -			flags |= MLX5_ACTION_OF_POP_VLAN;
>> +			flags |= MLX5_FLOW_ACTION_OF_POP_VLAN;
>> 			goto action_of_vlan;
>> 		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
>> -			flags |= MLX5_ACTION_OF_PUSH_VLAN;
>> +			flags |= MLX5_FLOW_ACTION_OF_PUSH_VLAN;
>> 			goto action_of_vlan;
>> 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
>> -			flags |= MLX5_ACTION_OF_SET_VLAN_VID;
>> +			flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
>> 			goto action_of_vlan;
>> 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
>> -			flags |= MLX5_ACTION_OF_SET_VLAN_PCP;
>> +			flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
>> 			goto action_of_vlan;
>> action_of_vlan:
>> 			size += SZ_NLATTR_NEST + /* na_act_index. */
>> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
>> b/drivers/net/mlx5/mlx5_flow_verbs.c
>> index 076bb39e6..0ecbc8121 100644
>> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
>> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
>> @@ -755,7 +755,7 @@ flow_verbs_translate_action_drop(uint64_t
>> *action_flags,
>> 	};
>> 
>> 	flow_verbs_spec_add(dev_flow, &drop, size);
>> -	*action_flags |= MLX5_ACTION_DROP;
>> +	*action_flags |= MLX5_FLOW_ACTION_DROP;
>> }
>> 
>> /**
>> @@ -781,7 +781,7 @@ flow_verbs_translate_action_queue(const struct
>> rte_flow_action *action,
>> 	if (flow->queue)
>> 		(*flow->queue)[0] = queue->index;
>> 	flow->rss.queue_num = 1;
>> -	*action_flags |= MLX5_ACTION_QUEUE;
>> +	*action_flags |= MLX5_FLOW_ACTION_QUEUE;
>> }
>> 
>> /**
>> @@ -811,7 +811,7 @@ flow_verbs_translate_action_rss(const struct
>> rte_flow_action *action,
>> 	memcpy(flow->key, rss->key, MLX5_RSS_HASH_KEY_LEN);
>> 	flow->rss.types = rss->types;
>> 	flow->rss.level = rss->level;
>> -	*action_flags |= MLX5_ACTION_RSS;
>> +	*action_flags |= MLX5_FLOW_ACTION_RSS;
>> }
>> 
>> /**
>> @@ -838,7 +838,7 @@ flow_verbs_translate_action_flag
>> 		.size = size,
>> 		.tag_id = mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT),
>> 	};
>> -	*action_flags |= MLX5_ACTION_MARK;
>> +	*action_flags |= MLX5_FLOW_ACTION_MARK;
>> 	flow_verbs_spec_add(dev_flow, &tag, size);
>> }
>> 
>> @@ -898,14 +898,14 @@ flow_verbs_translate_action_mark(const struct
>> rte_flow_action *action,
>> 	};
>> 	struct mlx5_flow_verbs *verbs = &dev_flow->verbs;
>> 
>> -	if (*action_flags & MLX5_ACTION_FLAG) {
>> +	if (*action_flags & MLX5_FLOW_ACTION_FLAG) {
>> 		flow_verbs_mark_update(verbs, mark->id);
>> 		size = 0;
>> 	} else {
>> 		tag.tag_id = mlx5_flow_mark_set(mark->id);
>> 		flow_verbs_spec_add(dev_flow, &tag, size);
>> 	}
>> -	*action_flags |= MLX5_ACTION_MARK;
>> +	*action_flags |= MLX5_FLOW_ACTION_MARK;
>> }
>> 
>> /**
>> @@ -954,7 +954,7 @@ flow_verbs_translate_action_count(struct rte_eth_dev
>> *dev,
>> 						  "cannot get counter"
>> 						  " context.");
>> 	}
>> -	*action_flags |= MLX5_ACTION_COUNT;
>> +	*action_flags |= MLX5_FLOW_ACTION_COUNT;
>> #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>> 	counter.counter_set_handle = flow->counter->cs->handle;
>> 	flow_verbs_spec_add(dev_flow, &counter, size);
>> @@ -1116,7 +1116,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_FLAG;
>> +			action_flags |= MLX5_FLOW_ACTION_FLAG;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_MARK:
>> 			ret = mlx5_flow_validate_action_mark(actions,
>> @@ -1124,14 +1124,14 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_MARK;
>> +			action_flags |= MLX5_FLOW_ACTION_MARK;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> 			ret = mlx5_flow_validate_action_drop(action_flags,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_DROP;
>> +			action_flags |= MLX5_FLOW_ACTION_DROP;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_QUEUE:
>> 			ret = mlx5_flow_validate_action_queue(actions,
>> @@ -1139,7 +1139,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>> 							      error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_QUEUE;
>> +			action_flags |= MLX5_FLOW_ACTION_QUEUE;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_RSS:
>> 			ret = mlx5_flow_validate_action_rss(actions,
>> @@ -1147,13 +1147,13 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>> 							    error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_RSS;
>> +			action_flags |= MLX5_FLOW_ACTION_RSS;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_COUNT:
>> 			ret = mlx5_flow_validate_action_count(dev, error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_COUNT;
>> +			action_flags |= MLX5_FLOW_ACTION_COUNT;
>> 			break;
>> 		default:
>> 			return rte_flow_error_set(error, ENOTSUP,
>> @@ -1191,27 +1191,27 @@ flow_verbs_get_actions_and_size(const struct
>> rte_flow_action actions[],
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_FLAG:
>> 			size += sizeof(struct ibv_flow_spec_action_tag);
>> -			detected_actions |= MLX5_ACTION_FLAG;
>> +			detected_actions |= MLX5_FLOW_ACTION_FLAG;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_MARK:
>> 			size += sizeof(struct ibv_flow_spec_action_tag);
>> -			detected_actions |= MLX5_ACTION_MARK;
>> +			detected_actions |= MLX5_FLOW_ACTION_MARK;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> 			size += sizeof(struct ibv_flow_spec_action_drop);
>> -			detected_actions |= MLX5_ACTION_DROP;
>> +			detected_actions |= MLX5_FLOW_ACTION_DROP;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_QUEUE:
>> -			detected_actions |= MLX5_ACTION_QUEUE;
>> +			detected_actions |= MLX5_FLOW_ACTION_QUEUE;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_RSS:
>> -			detected_actions |= MLX5_ACTION_RSS;
>> +			detected_actions |= MLX5_FLOW_ACTION_RSS;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_COUNT:
>> #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>> 			size += sizeof(struct ibv_flow_spec_counter_action);
>> #endif
>> -			detected_actions |= MLX5_ACTION_COUNT;
>> +			detected_actions |= MLX5_FLOW_ACTION_COUNT;
>> 			break;
>> 		default:
>> 			break;
>> @@ -1519,7 +1519,7 @@ flow_verbs_remove(struct rte_eth_dev *dev, struct
>> rte_flow *flow)
>> 			verbs->flow = NULL;
>> 		}
>> 		if (verbs->hrxq) {
>> -			if (flow->actions & MLX5_ACTION_DROP)
>> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>> 				mlx5_hrxq_drop_release(dev);
>> 			else
>> 				mlx5_hrxq_release(dev, verbs->hrxq);
>> @@ -1578,7 +1578,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct
>> rte_flow *flow,
>> 
>> 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>> 		verbs = &dev_flow->verbs;
>> -		if (flow->actions & MLX5_ACTION_DROP) {
>> +		if (flow->actions & MLX5_FLOW_ACTION_DROP) {
>> 			verbs->hrxq = mlx5_hrxq_drop_new(dev);
>> 			if (!verbs->hrxq) {
>> 				rte_flow_error_set
>> @@ -1628,7 +1628,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct
>> rte_flow *flow,
>> 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>> 		verbs = &dev_flow->verbs;
>> 		if (verbs->hrxq) {
>> -			if (flow->actions & MLX5_ACTION_DROP)
>> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>> 				mlx5_hrxq_drop_release(dev);
>> 			else
>> 				mlx5_hrxq_release(dev, verbs->hrxq);
>> --
>> 2.11.0
> Acked-by: Ori Kam <orika@mellanox.com>
> 
> Thanks,
> Ori

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

* Re: [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros
  2018-10-09  7:43   ` Ori Kam
  2018-10-09  8:05     ` Yongseok Koh
@ 2018-10-09  8:17     ` Yongseok Koh
  1 sibling, 0 replies; 28+ messages in thread
From: Yongseok Koh @ 2018-10-09  8:17 UTC (permalink / raw)
  To: Ori Kam, Shahaf Shuler; +Cc: dev


> On Oct 9, 2018, at 12:43 AM, Ori Kam <orika@mellanox.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Yongseok Koh
>> Sent: Monday, October 8, 2018 9:02 PM
>> To: Shahaf Shuler <shahafs@mellanox.com>
>> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
>> <orika@mellanox.com>
>> Subject: [PATCH 3/7] net/mlx5: rename flow macros
>> 
>> MLX5_ACTION_* -> MLX5_FLOW_ACTION_*
>> MLX5_VXLAN* -> MLX5_UDP_PORT_VXLAN*
>> 
>> Fixes: 0f8775dd8f1c ("net/mlx5: add support for multiple flow drivers")
>> Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
>> Cc: Ori Kam <orika@mellanox.com>
>> 
> I don't think this is a fix since there is no issue with the implementation,
> you are just beautify the code.
> 
> Otherwise acked:

Right, but I just added the tags for reference. As this doesn't have "Cc: stable@",
it has no effect.

Shahaf, I'm okay if you drop the "Fixes:" tag when you merge it.
I'll leave it as your choice.

Thanks,
Yongseok

>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> drivers/net/mlx5/mlx5_flow.c       | 31 +++++++++++++++------------
>> drivers/net/mlx5/mlx5_flow.h       | 28 ++++++++++++------------
>> drivers/net/mlx5/mlx5_flow_dv.c    | 24 ++++++++++-----------
>> drivers/net/mlx5/mlx5_flow_tcf.c   | 26 +++++++++++-----------
>> drivers/net/mlx5/mlx5_flow_verbs.c | 44 +++++++++++++++++++------------------
>> -
>> 5 files changed, 78 insertions(+), 75 deletions(-)
>> 
>> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
>> index ef5e4684f..69afd4625 100644
>> --- a/drivers/net/mlx5/mlx5_flow.c
>> +++ b/drivers/net/mlx5/mlx5_flow.c
>> @@ -537,7 +537,7 @@ mlx5_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 &
>> -			    (MLX5_ACTION_FLAG | MLX5_ACTION_MARK));
>> +			    (MLX5_FLOW_ACTION_FLAG |
>> MLX5_FLOW_ACTION_MARK));
>> 	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
>> 	unsigned int i;
>> 
>> @@ -581,7 +581,7 @@ mlx5_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 &
>> -			    (MLX5_ACTION_FLAG | MLX5_ACTION_MARK));
>> +			    (MLX5_FLOW_ACTION_FLAG |
>> MLX5_FLOW_ACTION_MARK));
>> 	const int tunnel = !!(flow->layers & MLX5_FLOW_LAYER_TUNNEL);
>> 	unsigned int i;
>> 
>> @@ -656,15 +656,15 @@ mlx5_flow_validate_action_flag(uint64_t
>> action_flags,
>> 			       struct rte_flow_error *error)
>> {
>> 
>> -	if (action_flags & MLX5_ACTION_DROP)
>> +	if (action_flags & MLX5_FLOW_ACTION_DROP)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't drop and flag in same flow");
>> -	if (action_flags & MLX5_ACTION_MARK)
>> +	if (action_flags & MLX5_FLOW_ACTION_MARK)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't mark and flag in same flow");
>> -	if (action_flags & MLX5_ACTION_FLAG)
>> +	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 flag"
>> @@ -703,15 +703,15 @@ mlx5_flow_validate_action_mark(const struct
>> rte_flow_action *action,
>> 					  &mark->id,
>> 					  "mark id must in 0 <= id < "
>> 
>> RTE_STR(MLX5_FLOW_MARK_MAX));
>> -	if (action_flags & MLX5_ACTION_DROP)
>> +	if (action_flags & MLX5_FLOW_ACTION_DROP)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't drop and mark in same flow");
>> -	if (action_flags & MLX5_ACTION_FLAG)
>> +	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't flag and mark in same flow");
>> -	if (action_flags & MLX5_ACTION_MARK)
>> +	if (action_flags & MLX5_FLOW_ACTION_MARK)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 flag actions in same"
>> @@ -734,16 +734,17 @@ int
>> mlx5_flow_validate_action_drop(uint64_t action_flags,
>> 			       struct rte_flow_error *error)
>> {
>> -	if (action_flags & MLX5_ACTION_FLAG)
>> +	if (action_flags & MLX5_FLOW_ACTION_FLAG)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't drop and flag in same flow");
>> -	if (action_flags & MLX5_ACTION_MARK)
>> +	if (action_flags & MLX5_FLOW_ACTION_MARK)
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't drop and mark in same flow");
>> 	if (action_flags &
>> -		(MLX5_ACTION_DROP | MLX5_ACTION_QUEUE |
>> MLX5_ACTION_RSS))
>> +	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
>> +	     MLX5_FLOW_ACTION_RSS))
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 fate actions in"
>> @@ -776,7 +777,8 @@ mlx5_flow_validate_action_queue(const struct
>> rte_flow_action *action,
>> 	const struct rte_flow_action_queue *queue = action->conf;
>> 
>> 	if (action_flags &
>> -	    (MLX5_ACTION_DROP | MLX5_ACTION_QUEUE |
>> MLX5_ACTION_RSS))
>> +	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
>> +	     MLX5_FLOW_ACTION_RSS))
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 fate actions in"
>> @@ -820,7 +822,8 @@ mlx5_flow_validate_action_rss(const struct
>> rte_flow_action *action,
>> 	unsigned int i;
>> 
>> 	if (action_flags &
>> -	    (MLX5_ACTION_DROP | MLX5_ACTION_QUEUE |
>> MLX5_ACTION_RSS))
>> +	    (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
>> +	     MLX5_FLOW_ACTION_RSS))
>> 		return rte_flow_error_set(error, ENOTSUP,
>> 					  RTE_FLOW_ERROR_TYPE_ACTION,
>> NULL,
>> 					  "can't have 2 fate actions"
>> @@ -2304,7 +2307,7 @@ mlx5_flow_query_count(struct rte_flow *flow
>> __rte_unused,
>> 		      struct rte_flow_error *error)
>> {
>> #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>> -	if (flow->actions & MLX5_ACTION_COUNT) {
>> +	if (flow->actions & MLX5_FLOW_ACTION_COUNT) {
>> 		struct rte_flow_query_count *qc = data;
>> 		uint64_t counters[2] = {0, 0};
>> 		struct ibv_query_counter_set_attr query_cs_attr = {
>> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
>> index 1ac871b3a..309e4d4f2 100644
>> --- a/drivers/net/mlx5/mlx5_flow.h
>> +++ b/drivers/net/mlx5/mlx5_flow.h
>> @@ -67,25 +67,25 @@
>> 	 MLX5_FLOW_LAYER_INNER_L4)
>> 
>> /* Actions */
>> -#define MLX5_ACTION_DROP (1u << 0)
>> -#define MLX5_ACTION_QUEUE (1u << 1)
>> -#define MLX5_ACTION_RSS (1u << 2)
>> -#define MLX5_ACTION_FLAG (1u << 3)
>> -#define MLX5_ACTION_MARK (1u << 4)
>> -#define MLX5_ACTION_COUNT (1u << 5)
>> -#define MLX5_ACTION_PORT_ID (1u << 6)
>> -#define MLX5_ACTION_OF_POP_VLAN (1u << 7)
>> -#define MLX5_ACTION_OF_PUSH_VLAN (1u << 8)
>> -#define MLX5_ACTION_OF_SET_VLAN_VID (1u << 9)
>> -#define MLX5_ACTION_OF_SET_VLAN_PCP (1u << 10)
>> +#define MLX5_FLOW_ACTION_DROP (1u << 0)
>> +#define MLX5_FLOW_ACTION_QUEUE (1u << 1)
>> +#define MLX5_FLOW_ACTION_RSS (1u << 2)
>> +#define MLX5_FLOW_ACTION_FLAG (1u << 3)
>> +#define MLX5_FLOW_ACTION_MARK (1u << 4)
>> +#define MLX5_FLOW_ACTION_COUNT (1u << 5)
>> +#define MLX5_FLOW_ACTION_PORT_ID (1u << 6)
>> +#define MLX5_FLOW_ACTION_OF_POP_VLAN (1u << 7)
>> +#define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
>> +#define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
>> +#define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
>> 
>> #ifndef IPPROTO_MPLS
>> #define IPPROTO_MPLS 137
>> #endif
>> 
>> -/* Internent Protocol versions. */
>> -#define MLX5_VXLAN 4789
>> -#define MLX5_VXLAN_GPE 4790
>> +/* UDP port numbers for VxLAN. */
>> +#define MLX5_UDP_PORT_VXLAN 4789
>> +#define MLX5_UDP_PORT_VXLAN_GPE 4790
>> 
>> /* Priority reserved for default flows. */
>> #define MLX5_FLOW_PRIO_RSVD ((uint32_t)-1)
>> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
>> b/drivers/net/mlx5/mlx5_flow_dv.c
>> index 5678dc35f..e6c84d444 100644
>> --- a/drivers/net/mlx5/mlx5_flow_dv.c
>> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
>> @@ -236,7 +236,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
>> rte_flow_attr *attr,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_FLAG;
>> +			action_flags |= MLX5_FLOW_ACTION_FLAG;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_MARK:
>> @@ -245,7 +245,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
>> rte_flow_attr *attr,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_MARK;
>> +			action_flags |= MLX5_FLOW_ACTION_MARK;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> @@ -253,7 +253,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
>> rte_flow_attr *attr,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_DROP;
>> +			action_flags |= MLX5_FLOW_ACTION_DROP;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_QUEUE:
>> @@ -262,7 +262,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct
>> rte_flow_attr *attr,
>> 							      error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_QUEUE;
>> +			action_flags |= MLX5_FLOW_ACTION_QUEUE;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_RSS:
>> @@ -271,14 +271,14 @@ flow_dv_validate(struct rte_eth_dev *dev, const
>> struct rte_flow_attr *attr,
>> 							    error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_RSS;
>> +			action_flags |= MLX5_FLOW_ACTION_RSS;
>> 			++actions_n;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_COUNT:
>> 			ret = mlx5_flow_validate_action_count(dev, error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_COUNT;
>> +			action_flags |= MLX5_FLOW_ACTION_COUNT;
>> 			++actions_n;
>> 			break;
>> 		default:
>> @@ -838,8 +838,8 @@ flow_dv_translate_item_vxlan(void *matcher, void
>> *key,
>> 					 outer_headers);
>> 		headers_v = MLX5_ADDR_OF(fte_match_param, key,
>> outer_headers);
>> 	}
>> -	dport = item->type == RTE_FLOW_ITEM_TYPE_VXLAN ? MLX5_VXLAN :
>> -							 MLX5_VXLAN_GPE;
>> +	dport = item->type == RTE_FLOW_ITEM_TYPE_VXLAN ?
>> +		MLX5_UDP_PORT_VXLAN : MLX5_UDP_PORT_VXLAN_GPE;
>> 	if (!MLX5_GET16(fte_match_set_lyr_2_4, headers_v, udp_dport)) {
>> 		MLX5_SET(fte_match_set_lyr_2_4, headers_m, udp_dport,
>> 0xFFFF);
>> 		MLX5_SET(fte_match_set_lyr_2_4, headers_v, udp_dport,
>> dport);
>> @@ -978,7 +978,7 @@ flow_dv_create_action(const struct rte_flow_action
>> *action,
>> 		break;
>> 	case RTE_FLOW_ACTION_TYPE_DROP:
>> 		dev_flow->dv.actions[actions_n].type =
>> MLX5DV_FLOW_ACTION_DROP;
>> -		flow->actions |= MLX5_ACTION_DROP;
>> +		flow->actions |= MLX5_FLOW_ACTION_DROP;
>> 		break;
>> 	case RTE_FLOW_ACTION_TYPE_QUEUE:
>> 		queue = action->conf;
>> @@ -1195,7 +1195,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct
>> rte_flow *flow,
>> 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>> 		dv = &dev_flow->dv;
>> 		n = dv->actions_n;
>> -		if (flow->actions & MLX5_ACTION_DROP) {
>> +		if (flow->actions & MLX5_FLOW_ACTION_DROP) {
>> 			dv->hrxq = mlx5_hrxq_drop_new(dev);
>> 			if (!dv->hrxq) {
>> 				rte_flow_error_set
>> @@ -1251,7 +1251,7 @@ flow_dv_apply(struct rte_eth_dev *dev, struct
>> rte_flow *flow,
>> 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>> 		struct mlx5_flow_dv *dv = &dev_flow->dv;
>> 		if (dv->hrxq) {
>> -			if (flow->actions & MLX5_ACTION_DROP)
>> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>> 				mlx5_hrxq_drop_release(dev);
>> 			else
>> 				mlx5_hrxq_release(dev, dv->hrxq);
>> @@ -1318,7 +1318,7 @@ flow_dv_remove(struct rte_eth_dev *dev, struct
>> rte_flow *flow)
>> 			dv->flow = NULL;
>> 		}
>> 		if (dv->hrxq) {
>> -			if (flow->actions & MLX5_ACTION_DROP)
>> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>> 				mlx5_hrxq_drop_release(dev);
>> 			else
>> 				mlx5_hrxq_release(dev, dv->hrxq);
>> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
>> b/drivers/net/mlx5/mlx5_flow_tcf.c
>> index 14376188e..c87046365 100644
>> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
>> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
>> @@ -225,7 +225,7 @@ struct flow_tcf_ptoi {
>> 	unsigned int ifindex; /**< Network interface index. */
>> };
>> 
>> -#define MLX5_TCF_FATE_ACTIONS (MLX5_ACTION_DROP |
>> MLX5_ACTION_PORT_ID)
>> +#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP |
>> MLX5_FLOW_ACTION_PORT_ID)
>> 
>> /**
>> * Retrieve mask for pattern item.
>> @@ -668,7 +668,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>> 					 conf.port_id,
>> 					 "missing data to convert port ID to"
>> 					 " ifindex");
>> -			action_flags |= MLX5_ACTION_PORT_ID;
>> +			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> 			if (action_flags & MLX5_TCF_FATE_ACTIONS)
>> @@ -676,19 +676,19 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>> 					(error, ENOTSUP,
>> 					 RTE_FLOW_ERROR_TYPE_ACTION,
>> actions,
>> 					 "can't have multiple fate actions");
>> -			action_flags |= MLX5_ACTION_DROP;
>> +			action_flags |= MLX5_FLOW_ACTION_DROP;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
>> -			action_flags |= MLX5_ACTION_OF_POP_VLAN;
>> +			action_flags |= MLX5_FLOW_ACTION_OF_POP_VLAN;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
>> -			action_flags |= MLX5_ACTION_OF_PUSH_VLAN;
>> +			action_flags |=
>> MLX5_FLOW_ACTION_OF_PUSH_VLAN;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
>> -			action_flags |= MLX5_ACTION_OF_SET_VLAN_VID;
>> +			action_flags |=
>> MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
>> -			action_flags |= MLX5_ACTION_OF_SET_VLAN_PCP;
>> +			action_flags |=
>> MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
>> 			break;
>> 		default:
>> 			return rte_flow_error_set(error, ENOTSUP,
>> @@ -809,26 +809,26 @@ flow_tcf_get_actions_and_size(const struct
>> rte_flow_action actions[],
>> 				SZ_NLATTR_STRZ_OF("mirred") +
>> 				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
>> 				SZ_NLATTR_TYPE_OF(struct tc_mirred);
>> -			flags |= MLX5_ACTION_PORT_ID;
>> +			flags |= MLX5_FLOW_ACTION_PORT_ID;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> 			size += SZ_NLATTR_NEST + /* na_act_index. */
>> 				SZ_NLATTR_STRZ_OF("gact") +
>> 				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
>> 				SZ_NLATTR_TYPE_OF(struct tc_gact);
>> -			flags |= MLX5_ACTION_DROP;
>> +			flags |= MLX5_FLOW_ACTION_DROP;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
>> -			flags |= MLX5_ACTION_OF_POP_VLAN;
>> +			flags |= MLX5_FLOW_ACTION_OF_POP_VLAN;
>> 			goto action_of_vlan;
>> 		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
>> -			flags |= MLX5_ACTION_OF_PUSH_VLAN;
>> +			flags |= MLX5_FLOW_ACTION_OF_PUSH_VLAN;
>> 			goto action_of_vlan;
>> 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
>> -			flags |= MLX5_ACTION_OF_SET_VLAN_VID;
>> +			flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
>> 			goto action_of_vlan;
>> 		case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
>> -			flags |= MLX5_ACTION_OF_SET_VLAN_PCP;
>> +			flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
>> 			goto action_of_vlan;
>> action_of_vlan:
>> 			size += SZ_NLATTR_NEST + /* na_act_index. */
>> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
>> b/drivers/net/mlx5/mlx5_flow_verbs.c
>> index 076bb39e6..0ecbc8121 100644
>> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
>> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
>> @@ -755,7 +755,7 @@ flow_verbs_translate_action_drop(uint64_t
>> *action_flags,
>> 	};
>> 
>> 	flow_verbs_spec_add(dev_flow, &drop, size);
>> -	*action_flags |= MLX5_ACTION_DROP;
>> +	*action_flags |= MLX5_FLOW_ACTION_DROP;
>> }
>> 
>> /**
>> @@ -781,7 +781,7 @@ flow_verbs_translate_action_queue(const struct
>> rte_flow_action *action,
>> 	if (flow->queue)
>> 		(*flow->queue)[0] = queue->index;
>> 	flow->rss.queue_num = 1;
>> -	*action_flags |= MLX5_ACTION_QUEUE;
>> +	*action_flags |= MLX5_FLOW_ACTION_QUEUE;
>> }
>> 
>> /**
>> @@ -811,7 +811,7 @@ flow_verbs_translate_action_rss(const struct
>> rte_flow_action *action,
>> 	memcpy(flow->key, rss->key, MLX5_RSS_HASH_KEY_LEN);
>> 	flow->rss.types = rss->types;
>> 	flow->rss.level = rss->level;
>> -	*action_flags |= MLX5_ACTION_RSS;
>> +	*action_flags |= MLX5_FLOW_ACTION_RSS;
>> }
>> 
>> /**
>> @@ -838,7 +838,7 @@ flow_verbs_translate_action_flag
>> 		.size = size,
>> 		.tag_id = mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT),
>> 	};
>> -	*action_flags |= MLX5_ACTION_MARK;
>> +	*action_flags |= MLX5_FLOW_ACTION_MARK;
>> 	flow_verbs_spec_add(dev_flow, &tag, size);
>> }
>> 
>> @@ -898,14 +898,14 @@ flow_verbs_translate_action_mark(const struct
>> rte_flow_action *action,
>> 	};
>> 	struct mlx5_flow_verbs *verbs = &dev_flow->verbs;
>> 
>> -	if (*action_flags & MLX5_ACTION_FLAG) {
>> +	if (*action_flags & MLX5_FLOW_ACTION_FLAG) {
>> 		flow_verbs_mark_update(verbs, mark->id);
>> 		size = 0;
>> 	} else {
>> 		tag.tag_id = mlx5_flow_mark_set(mark->id);
>> 		flow_verbs_spec_add(dev_flow, &tag, size);
>> 	}
>> -	*action_flags |= MLX5_ACTION_MARK;
>> +	*action_flags |= MLX5_FLOW_ACTION_MARK;
>> }
>> 
>> /**
>> @@ -954,7 +954,7 @@ flow_verbs_translate_action_count(struct rte_eth_dev
>> *dev,
>> 						  "cannot get counter"
>> 						  " context.");
>> 	}
>> -	*action_flags |= MLX5_ACTION_COUNT;
>> +	*action_flags |= MLX5_FLOW_ACTION_COUNT;
>> #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>> 	counter.counter_set_handle = flow->counter->cs->handle;
>> 	flow_verbs_spec_add(dev_flow, &counter, size);
>> @@ -1116,7 +1116,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_FLAG;
>> +			action_flags |= MLX5_FLOW_ACTION_FLAG;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_MARK:
>> 			ret = mlx5_flow_validate_action_mark(actions,
>> @@ -1124,14 +1124,14 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_MARK;
>> +			action_flags |= MLX5_FLOW_ACTION_MARK;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> 			ret = mlx5_flow_validate_action_drop(action_flags,
>> 							     error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_DROP;
>> +			action_flags |= MLX5_FLOW_ACTION_DROP;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_QUEUE:
>> 			ret = mlx5_flow_validate_action_queue(actions,
>> @@ -1139,7 +1139,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>> 							      error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_QUEUE;
>> +			action_flags |= MLX5_FLOW_ACTION_QUEUE;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_RSS:
>> 			ret = mlx5_flow_validate_action_rss(actions,
>> @@ -1147,13 +1147,13 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>> 							    error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_RSS;
>> +			action_flags |= MLX5_FLOW_ACTION_RSS;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_COUNT:
>> 			ret = mlx5_flow_validate_action_count(dev, error);
>> 			if (ret < 0)
>> 				return ret;
>> -			action_flags |= MLX5_ACTION_COUNT;
>> +			action_flags |= MLX5_FLOW_ACTION_COUNT;
>> 			break;
>> 		default:
>> 			return rte_flow_error_set(error, ENOTSUP,
>> @@ -1191,27 +1191,27 @@ flow_verbs_get_actions_and_size(const struct
>> rte_flow_action actions[],
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_FLAG:
>> 			size += sizeof(struct ibv_flow_spec_action_tag);
>> -			detected_actions |= MLX5_ACTION_FLAG;
>> +			detected_actions |= MLX5_FLOW_ACTION_FLAG;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_MARK:
>> 			size += sizeof(struct ibv_flow_spec_action_tag);
>> -			detected_actions |= MLX5_ACTION_MARK;
>> +			detected_actions |= MLX5_FLOW_ACTION_MARK;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_DROP:
>> 			size += sizeof(struct ibv_flow_spec_action_drop);
>> -			detected_actions |= MLX5_ACTION_DROP;
>> +			detected_actions |= MLX5_FLOW_ACTION_DROP;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_QUEUE:
>> -			detected_actions |= MLX5_ACTION_QUEUE;
>> +			detected_actions |= MLX5_FLOW_ACTION_QUEUE;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_RSS:
>> -			detected_actions |= MLX5_ACTION_RSS;
>> +			detected_actions |= MLX5_FLOW_ACTION_RSS;
>> 			break;
>> 		case RTE_FLOW_ACTION_TYPE_COUNT:
>> #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
>> 			size += sizeof(struct ibv_flow_spec_counter_action);
>> #endif
>> -			detected_actions |= MLX5_ACTION_COUNT;
>> +			detected_actions |= MLX5_FLOW_ACTION_COUNT;
>> 			break;
>> 		default:
>> 			break;
>> @@ -1519,7 +1519,7 @@ flow_verbs_remove(struct rte_eth_dev *dev, struct
>> rte_flow *flow)
>> 			verbs->flow = NULL;
>> 		}
>> 		if (verbs->hrxq) {
>> -			if (flow->actions & MLX5_ACTION_DROP)
>> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>> 				mlx5_hrxq_drop_release(dev);
>> 			else
>> 				mlx5_hrxq_release(dev, verbs->hrxq);
>> @@ -1578,7 +1578,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct
>> rte_flow *flow,
>> 
>> 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>> 		verbs = &dev_flow->verbs;
>> -		if (flow->actions & MLX5_ACTION_DROP) {
>> +		if (flow->actions & MLX5_FLOW_ACTION_DROP) {
>> 			verbs->hrxq = mlx5_hrxq_drop_new(dev);
>> 			if (!verbs->hrxq) {
>> 				rte_flow_error_set
>> @@ -1628,7 +1628,7 @@ flow_verbs_apply(struct rte_eth_dev *dev, struct
>> rte_flow *flow,
>> 	LIST_FOREACH(dev_flow, &flow->dev_flows, next) {
>> 		verbs = &dev_flow->verbs;
>> 		if (verbs->hrxq) {
>> -			if (flow->actions & MLX5_ACTION_DROP)
>> +			if (flow->actions & MLX5_FLOW_ACTION_DROP)
>> 				mlx5_hrxq_drop_release(dev);
>> 			else
>> 				mlx5_hrxq_release(dev, verbs->hrxq);
>> --
>> 2.11.0
> Acked-by: Ori Kam <orika@mellanox.com>
> 
> Thanks,
> Ori

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

* Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
  2018-10-08 18:02 [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine Yongseok Koh
                   ` (6 preceding siblings ...)
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 7/7] net/mlx5: fix errno values for flow engine Yongseok Koh
@ 2018-10-09  8:58 ` Shahaf Shuler
  2018-10-09 15:49   ` Ferruh Yigit
  7 siblings, 1 reply; 28+ messages in thread
From: Shahaf Shuler @ 2018-10-09  8:58 UTC (permalink / raw)
  To: Yongseok Koh; +Cc: dev

Monday, October 8, 2018 9:02 PM, Yongseok Koh:
> Subject: [PATCH 0/7] net/mlx5: fixes for the new flow engine
> 
> Minor fixes accumulated since the following two patchsets.
> 
> 	net/mlx5: add Direct Verbs flow driver support [1]
> 	net/mlx5: migrate Linux TC flower driver to new flow engine
> 
> [1] http://patches.dpdk.org/cover/45248
> [2] http://patches.dpdk.org/cover/44897
> 
> Yongseok Koh (7):
>   net/mlx5: fix wrong flow action macro usage
>   net/mlx5: use standard IP protocol numbers
>   net/mlx5: rename flow macros
>   net/mlx5: fix validation of VLAN ID in flow spec
>   net/mlx5: fix flow validation for no fate action
>   net/mlx5: add missing VLAN action constraints
>   net/mlx5: fix errno values for flow engine
> 
>  drivers/net/mlx5/mlx5_flow.c       | 117 +++++++++++++++++++---------------
> ---
>  drivers/net/mlx5/mlx5_flow.h       |  54 ++++++++---------
>  drivers/net/mlx5/mlx5_flow_dv.c    |  30 +++++-----
>  drivers/net/mlx5/mlx5_flow_tcf.c   |  78 +++++++++++++++++++------
>  drivers/net/mlx5/mlx5_flow_verbs.c |  59 ++++++++++---------
>  5 files changed, 193 insertions(+), 145 deletions(-)

Series applied to next-net-mlx, thanks. 
Need to add the missing VLAN limitation of the "pop always to non-uplink" on a separate commit, don't want to stall the entire series. 

> 
> --
> 2.11.0

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

* Re: [dpdk-dev] [PATCH 2/7] net/mlx5: use standard IP protocol numbers
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 2/7] net/mlx5: use standard IP protocol numbers Yongseok Koh
  2018-10-09  7:37   ` Ori Kam
@ 2018-10-09 15:39   ` Ferruh Yigit
  1 sibling, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2018-10-09 15:39 UTC (permalink / raw)
  To: Yongseok Koh, Shahaf Shuler; +Cc: dev, Ori Kam

On 10/8/2018 7:02 PM, Yongseok Koh wrote:
> Fixes: 0f8775dd8f1c ("net/mlx5: add support for multiple flow drivers")
> Cc: Ori Kam <orika@mellanox.com>
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

Squashed into relevant commit in next-net, thanks.

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

* Re: [dpdk-dev] [PATCH 4/7] net/mlx5: fix validation of VLAN ID in flow spec
  2018-10-09  7:47   ` Ori Kam
@ 2018-10-09 15:44     ` Ferruh Yigit
  0 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2018-10-09 15:44 UTC (permalink / raw)
  To: Ori Kam, Yongseok Koh, Shahaf Shuler; +Cc: dev

On 10/9/2018 8:47 AM, Ori Kam wrote:
> 
> 
>> -----Original Message-----
>> From: Yongseok Koh
>> Sent: Monday, October 8, 2018 9:02 PM
>> To: Shahaf Shuler <shahafs@mellanox.com>
>> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
>> <orika@mellanox.com>
>> Subject: [PATCH 4/7] net/mlx5: fix validation of VLAN ID in flow spec
>>
>> This can cause crash by null pointer reference.
>>
>> Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
>> Cc: Ori Kam <orika@mellanox.com>
>>
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

> Acked-by: Ori Kam <orika@mellanox.com>

Squashed into relevant commit in next-net, thanks.

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

* Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
  2018-10-09  8:58 ` [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new " Shahaf Shuler
@ 2018-10-09 15:49   ` Ferruh Yigit
  2018-10-10  5:57     ` Shahaf Shuler
  0 siblings, 1 reply; 28+ messages in thread
From: Ferruh Yigit @ 2018-10-09 15:49 UTC (permalink / raw)
  To: Shahaf Shuler, Yongseok Koh; +Cc: dev

On 10/9/2018 9:58 AM, Shahaf Shuler wrote:
> Monday, October 8, 2018 9:02 PM, Yongseok Koh:
>> Subject: [PATCH 0/7] net/mlx5: fixes for the new flow engine
>>
>> Minor fixes accumulated since the following two patchsets.
>>
>> 	net/mlx5: add Direct Verbs flow driver support [1]
>> 	net/mlx5: migrate Linux TC flower driver to new flow engine
>>
>> [1] http://patches.dpdk.org/cover/45248
>> [2] http://patches.dpdk.org/cover/44897
>>
>> Yongseok Koh (7):
>>   net/mlx5: fix wrong flow action macro usage
>>   net/mlx5: use standard IP protocol numbers
>>   net/mlx5: rename flow macros
>>   net/mlx5: fix validation of VLAN ID in flow spec
>>   net/mlx5: fix flow validation for no fate action
>>   net/mlx5: add missing VLAN action constraints
>>   net/mlx5: fix errno values for flow engine
>>
>>  drivers/net/mlx5/mlx5_flow.c       | 117 +++++++++++++++++++---------------
>> ---
>>  drivers/net/mlx5/mlx5_flow.h       |  54 ++++++++---------
>>  drivers/net/mlx5/mlx5_flow_dv.c    |  30 +++++-----
>>  drivers/net/mlx5/mlx5_flow_tcf.c   |  78 +++++++++++++++++++------
>>  drivers/net/mlx5/mlx5_flow_verbs.c |  59 ++++++++++---------
>>  5 files changed, 193 insertions(+), 145 deletions(-)
> 
> Series applied to next-net-mlx, thanks. 
> Need to add the missing VLAN limitation of the "pop always to non-uplink" on a separate commit, don't want to stall the entire series. 

Hi Shahaf,

These are fixing mlx5 patches which are merged very recently, that is why I
tried to squash these to original commit. This is both for more clean git
history and to have correct Fixes information in commit logs.

I can able to squash: 1/7, 2/7 & 4/7

But 4 are still remaining, main reason is they fixes more than one commit so not
easy to squash into one.

I won't merge remaining ones for now, can you please rebase them on top of
next-net, and try to arrange in a way to squash into next-net, if not able to
make we can get them as it is.

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

* Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
  2018-10-09 15:49   ` Ferruh Yigit
@ 2018-10-10  5:57     ` Shahaf Shuler
  2018-10-10 10:57       ` Ferruh Yigit
  0 siblings, 1 reply; 28+ messages in thread
From: Shahaf Shuler @ 2018-10-10  5:57 UTC (permalink / raw)
  To: Ferruh Yigit, Yongseok Koh; +Cc: dev

Tuesday, October 9, 2018 6:49 PM, Ferruh Yigit:
> Subject: Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
> 
> On 10/9/2018 9:58 AM, Shahaf Shuler wrote:
> > Monday, October 8, 2018 9:02 PM, Yongseok Koh:
> >> Subject: [PATCH 0/7] net/mlx5: fixes for the new flow engine

[...]

> >> djWRGvBzaqpfUVsn8%3D&amp;reserved=0
> >>
> >> Yongseok Koh (7):
> >>   net/mlx5: fix wrong flow action macro usage
> >>   net/mlx5: use standard IP protocol numbers
> >>   net/mlx5: rename flow macros
> >>   net/mlx5: fix validation of VLAN ID in flow spec
> >>   net/mlx5: fix flow validation for no fate action
> >>   net/mlx5: add missing VLAN action constraints
> >>   net/mlx5: fix errno values for flow engine
> >>
> >>  drivers/net/mlx5/mlx5_flow.c       | 117 +++++++++++++++++++-----------
> ----
> >> ---
> >>  drivers/net/mlx5/mlx5_flow.h       |  54 ++++++++---------
> >>  drivers/net/mlx5/mlx5_flow_dv.c    |  30 +++++-----
> >>  drivers/net/mlx5/mlx5_flow_tcf.c   |  78 +++++++++++++++++++------
> >>  drivers/net/mlx5/mlx5_flow_verbs.c |  59 ++++++++++---------
> >>  5 files changed, 193 insertions(+), 145 deletions(-)
> >
> > Series applied to next-net-mlx, thanks.
> > Need to add the missing VLAN limitation of the "pop always to non-uplink"
> on a separate commit, don't want to stall the entire series.
> 
> Hi Shahaf,
> 
> These are fixing mlx5 patches which are merged very recently, that is why I
> tried to squash these to original commit. This is both for more clean git
> history and to have correct Fixes information in commit logs.

I am not sure I agree here. There was a feature which was accepted and later on it had some bug fixes. 
I think it is better to separate between the two, because:
1. I don't think it spams that much the git history
2. if the small fix raised a different bug it will be easier to bisect and track it rather than trying to check what is wrong on the big feature patch

> 
> I can able to squash: 1/7, 2/7 & 4/7
> 
> But 4 are still remaining, main reason is they fixes more than one commit so
> not easy to squash into one.
> 
> I won't merge remaining ones for now, can you please rebase them on top of
> next-net, and try to arrange in a way to squash into next-net, if not able to
> make we can get them as it is.

If it is not critical for you, I suggest we take them as is. It will require some work to re-arrange them + test again.
Let me know. 



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

* Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
  2018-10-10  5:57     ` Shahaf Shuler
@ 2018-10-10 10:57       ` Ferruh Yigit
  2018-10-10 13:01         ` Ferruh Yigit
  0 siblings, 1 reply; 28+ messages in thread
From: Ferruh Yigit @ 2018-10-10 10:57 UTC (permalink / raw)
  To: Shahaf Shuler, Yongseok Koh; +Cc: dev

On 10/10/2018 6:57 AM, Shahaf Shuler wrote:
> Tuesday, October 9, 2018 6:49 PM, Ferruh Yigit:
>> Subject: Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
>>
>> On 10/9/2018 9:58 AM, Shahaf Shuler wrote:
>>> Monday, October 8, 2018 9:02 PM, Yongseok Koh:
>>>> Subject: [PATCH 0/7] net/mlx5: fixes for the new flow engine
> 
> [...]
> 
>>>> djWRGvBzaqpfUVsn8%3D&amp;reserved=0
>>>>
>>>> Yongseok Koh (7):
>>>>   net/mlx5: fix wrong flow action macro usage
>>>>   net/mlx5: use standard IP protocol numbers
>>>>   net/mlx5: rename flow macros
>>>>   net/mlx5: fix validation of VLAN ID in flow spec
>>>>   net/mlx5: fix flow validation for no fate action
>>>>   net/mlx5: add missing VLAN action constraints
>>>>   net/mlx5: fix errno values for flow engine
>>>>
>>>>  drivers/net/mlx5/mlx5_flow.c       | 117 +++++++++++++++++++-----------
>> ----
>>>> ---
>>>>  drivers/net/mlx5/mlx5_flow.h       |  54 ++++++++---------
>>>>  drivers/net/mlx5/mlx5_flow_dv.c    |  30 +++++-----
>>>>  drivers/net/mlx5/mlx5_flow_tcf.c   |  78 +++++++++++++++++++------
>>>>  drivers/net/mlx5/mlx5_flow_verbs.c |  59 ++++++++++---------
>>>>  5 files changed, 193 insertions(+), 145 deletions(-)
>>>
>>> Series applied to next-net-mlx, thanks.
>>> Need to add the missing VLAN limitation of the "pop always to non-uplink"
>> on a separate commit, don't want to stall the entire series.
>>
>> Hi Shahaf,
>>
>> These are fixing mlx5 patches which are merged very recently, that is why I
>> tried to squash these to original commit. This is both for more clean git
>> history and to have correct Fixes information in commit logs.
> 
> I am not sure I agree here. There was a feature which was accepted and later on it had some bug fixes. 
> I think it is better to separate between the two, because:
> 1. I don't think it spams that much the git history
> 2. if the small fix raised a different bug it will be easier to bisect and track it rather than trying to check what is wrong on the big feature patch

I would agree but for this case original feature is too fresh. I think it is
opportunity to merge it into original feature. Eventually final code will be
same, if there is a bug it will be in both ways, just to improve the history.

And I didn't understand why it is better to "fix the fix" instead of merge the
fix to the original feature and "fix the feature" if the fix is wrong.

Also this "fix the fix" chains may make reading/understanding original code
harder in the future.

> 
>>
>> I can able to squash: 1/7, 2/7 & 4/7
>>
>> But 4 are still remaining, main reason is they fixes more than one commit so
>> not easy to squash into one.
>>
>> I won't merge remaining ones for now, can you please rebase them on top of
>> next-net, and try to arrange in a way to squash into next-net, if not able to
>> make we can get them as it is.
> 
> If it is not critical for you, I suggest we take them as is. It will require some work to re-arrange them + test again.
> Let me know. 

No it is not critical, but again patchset doesn't look like too big, so if it is
not too much effort I prefer squashing them. And as a final result code should
be exact same, so testing effort shouldn't change but re-arrange takes effort, I
already did a few of them for you...

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

* Re: [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros
  2018-10-08 18:02 ` [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros Yongseok Koh
  2018-10-09  7:43   ` Ori Kam
@ 2018-10-10 11:57   ` Ferruh Yigit
  1 sibling, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2018-10-10 11:57 UTC (permalink / raw)
  To: Yongseok Koh, Shahaf Shuler; +Cc: dev, Ori Kam

On 10/8/2018 7:02 PM, Yongseok Koh wrote:
> MLX5_ACTION_* -> MLX5_FLOW_ACTION_*
> MLX5_VXLAN* -> MLX5_UDP_PORT_VXLAN*
> 
> Fixes: 0f8775dd8f1c ("net/mlx5: add support for multiple flow drivers")
> Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
> Cc: Ori Kam <orika@mellanox.com>
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

Squashed into relevant commit in next-net, thanks.

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

* Re: [dpdk-dev] [PATCH 5/7] net/mlx5: fix flow validation for no fate action
  2018-10-09  7:49   ` Ori Kam
@ 2018-10-10 12:24     ` Ferruh Yigit
  0 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2018-10-10 12:24 UTC (permalink / raw)
  To: Ori Kam, Yongseok Koh, Shahaf Shuler; +Cc: dev

On 10/9/2018 8:49 AM, Ori Kam wrote:
> 
> 
>> -----Original Message-----
>> From: Yongseok Koh
>> Sent: Monday, October 8, 2018 9:02 PM
>> To: Shahaf Shuler <shahafs@mellanox.com>
>> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
>> <orika@mellanox.com>
>> Subject: [PATCH 5/7] net/mlx5: fix flow validation for no fate action
>>
>> Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
>> Fixes: f7adfffa3de1 ("net/mlx5: add Direct Verbs validation function")
>> Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
>> Cc: Ori Kam <orika@mellanox.com>
>>
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

> Acked-by: Ori Kam <orika@mellanox.com>

Squashed into relevant commit in next-net, thanks.

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

* Re: [dpdk-dev] [PATCH 7/7] net/mlx5: fix errno values for flow engine
  2018-10-09  7:53   ` Ori Kam
@ 2018-10-10 12:59     ` Ferruh Yigit
  0 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2018-10-10 12:59 UTC (permalink / raw)
  To: Ori Kam, Yongseok Koh, Shahaf Shuler; +Cc: dev

On 10/9/2018 8:53 AM, Ori Kam wrote:
> 
> 
>> -----Original Message-----
>> From: Yongseok Koh
>> Sent: Monday, October 8, 2018 9:02 PM
>> To: Shahaf Shuler <shahafs@mellanox.com>
>> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
>> <orika@mellanox.com>
>> Subject: [PATCH 7/7] net/mlx5: fix errno values for flow engine
>>
>> Fixes: 4f07e13d6af5 ("net/mlx5: split flow validation to dedicated function")
>> Fixes: f7adfffa3de1 ("net/mlx5: add Direct Verbs validation function")
>> Fixes: edcdef4e5fe4 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
>> Cc: orika@mellanox.com
>>
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>


> Acked-by: Ori Kam <orika@mellanox.com>

Patches fixes more than it claims above fixes lines, the relevant ones squashed,
remaining ones pushed as a commit with corrected fixes lines.

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

* Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
  2018-10-10 10:57       ` Ferruh Yigit
@ 2018-10-10 13:01         ` Ferruh Yigit
  0 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2018-10-10 13:01 UTC (permalink / raw)
  To: Shahaf Shuler, Yongseok Koh; +Cc: dev

On 10/10/2018 11:57 AM, Ferruh Yigit wrote:
> On 10/10/2018 6:57 AM, Shahaf Shuler wrote:
>> Tuesday, October 9, 2018 6:49 PM, Ferruh Yigit:
>>> Subject: Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
>>>
>>> On 10/9/2018 9:58 AM, Shahaf Shuler wrote:
>>>> Monday, October 8, 2018 9:02 PM, Yongseok Koh:
>>>>> Subject: [PATCH 0/7] net/mlx5: fixes for the new flow engine
>>
>> [...]
>>
>>>>> djWRGvBzaqpfUVsn8%3D&amp;reserved=0
>>>>>
>>>>> Yongseok Koh (7):
>>>>>   net/mlx5: fix wrong flow action macro usage
>>>>>   net/mlx5: use standard IP protocol numbers
>>>>>   net/mlx5: rename flow macros
>>>>>   net/mlx5: fix validation of VLAN ID in flow spec
>>>>>   net/mlx5: fix flow validation for no fate action
>>>>>   net/mlx5: add missing VLAN action constraints
>>>>>   net/mlx5: fix errno values for flow engine
>>>>>
>>>>>  drivers/net/mlx5/mlx5_flow.c       | 117 +++++++++++++++++++-----------
>>> ----
>>>>> ---
>>>>>  drivers/net/mlx5/mlx5_flow.h       |  54 ++++++++---------
>>>>>  drivers/net/mlx5/mlx5_flow_dv.c    |  30 +++++-----
>>>>>  drivers/net/mlx5/mlx5_flow_tcf.c   |  78 +++++++++++++++++++------
>>>>>  drivers/net/mlx5/mlx5_flow_verbs.c |  59 ++++++++++---------
>>>>>  5 files changed, 193 insertions(+), 145 deletions(-)
>>>>
>>>> Series applied to next-net-mlx, thanks.
>>>> Need to add the missing VLAN limitation of the "pop always to non-uplink"
>>> on a separate commit, don't want to stall the entire series.
>>>
>>> Hi Shahaf,
>>>
>>> These are fixing mlx5 patches which are merged very recently, that is why I
>>> tried to squash these to original commit. This is both for more clean git
>>> history and to have correct Fixes information in commit logs.
>>
>> I am not sure I agree here. There was a feature which was accepted and later on it had some bug fixes. 
>> I think it is better to separate between the two, because:
>> 1. I don't think it spams that much the git history
>> 2. if the small fix raised a different bug it will be easier to bisect and track it rather than trying to check what is wrong on the big feature patch
> 
> I would agree but for this case original feature is too fresh. I think it is
> opportunity to merge it into original feature. Eventually final code will be
> same, if there is a bug it will be in both ways, just to improve the history.
> 
> And I didn't understand why it is better to "fix the fix" instead of merge the
> fix to the original feature and "fix the feature" if the fix is wrong.
> 
> Also this "fix the fix" chains may make reading/understanding original code
> harder in the future.
> 
>>
>>>
>>> I can able to squash: 1/7, 2/7 & 4/7
>>>
>>> But 4 are still remaining, main reason is they fixes more than one commit so
>>> not easy to squash into one.
>>>
>>> I won't merge remaining ones for now, can you please rebase them on top of
>>> next-net, and try to arrange in a way to squash into next-net, if not able to
>>> make we can get them as it is.
>>
>> If it is not critical for you, I suggest we take them as is. It will require some work to re-arrange them + test again.
>> Let me know. 
> 
> No it is not critical, but again patchset doesn't look like too big, so if it is
> not too much effort I prefer squashing them. And as a final result code should
> be exact same, so testing effort shouldn't change but re-arrange takes effort, I
> already did a few of them for you...

All squashed except from 6/7 & "7/7 partially", I can confirm final code has not
changed but please double check latest next-net head.

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

end of thread, other threads:[~2018-10-10 13:01 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-08 18:02 [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine Yongseok Koh
2018-10-08 18:02 ` [dpdk-dev] [PATCH 1/7] net/mlx5: fix wrong flow action macro usage Yongseok Koh
2018-10-09  7:45   ` Ori Kam
2018-10-08 18:02 ` [dpdk-dev] [PATCH 2/7] net/mlx5: use standard IP protocol numbers Yongseok Koh
2018-10-09  7:37   ` Ori Kam
2018-10-09  8:02     ` Yongseok Koh
2018-10-09 15:39   ` Ferruh Yigit
2018-10-08 18:02 ` [dpdk-dev] [PATCH 3/7] net/mlx5: rename flow macros Yongseok Koh
2018-10-09  7:43   ` Ori Kam
2018-10-09  8:05     ` Yongseok Koh
2018-10-09  8:17     ` Yongseok Koh
2018-10-10 11:57   ` Ferruh Yigit
2018-10-08 18:02 ` [dpdk-dev] [PATCH 4/7] net/mlx5: fix validation of VLAN ID in flow spec Yongseok Koh
2018-10-09  7:47   ` Ori Kam
2018-10-09 15:44     ` Ferruh Yigit
2018-10-08 18:02 ` [dpdk-dev] [PATCH 5/7] net/mlx5: fix flow validation for no fate action Yongseok Koh
2018-10-09  7:49   ` Ori Kam
2018-10-10 12:24     ` Ferruh Yigit
2018-10-08 18:02 ` [dpdk-dev] [PATCH 6/7] net/mlx5: add missing VLAN action constraints Yongseok Koh
2018-10-08 19:48   ` Or Gerlitz
2018-10-08 18:02 ` [dpdk-dev] [PATCH 7/7] net/mlx5: fix errno values for flow engine Yongseok Koh
2018-10-09  7:53   ` Ori Kam
2018-10-10 12:59     ` Ferruh Yigit
2018-10-09  8:58 ` [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new " Shahaf Shuler
2018-10-09 15:49   ` Ferruh Yigit
2018-10-10  5:57     ` Shahaf Shuler
2018-10-10 10:57       ` Ferruh Yigit
2018-10-10 13:01         ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).