[dpdk-dev] [PATCH 1/5] net/mlx5: add warning message for Direct Verbs flow

[dpdk-dev] [PATCH 1/5] net/mlx5: add warning message for Direct Verbs flow

[Yongseok Koh]

In case that the library doesn't support DV flow, if enabled by
'dv_flow_en=1', print out a warning message and disable it.

Fixes: 51e72d386c99 ("net/mlx5: add runtime parameter to enable Direct Verbs")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5.c      |  6 ++++++
 drivers/net/mlx5/mlx5_flow.c | 20 +++++++-------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 795a219771..70b8626126 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1016,6 +1016,12 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 		cs_desc.counter_type, cs_desc.num_of_cs,
 		cs_desc.attributes);
 #endif
+#ifndef HAVE_IBV_FLOW_DV_SUPPORT
+	if (config.dv_flow_en) {
+		DRV_LOG(WARNING, "DV flow is not supported");
+		config.dv_flow_en = 0;
+	}
+#endif
 	config.ind_table_max_size =
 		attr.rss_caps.max_rwq_indirection_table_size;
 	/*
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index bd70fce248..04e287f160 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1673,25 +1673,19 @@ const struct mlx5_flow_driver_ops mlx5_flow_null_drv_ops = {
  *   Pointer to the flow attributes.
  *
  * @return
- *   flow driver type if supported, MLX5_FLOW_TYPE_MAX otherwise.
+ *   flow driver type, MLX5_FLOW_TYPE_MAX otherwise.
  */
 static enum mlx5_flow_drv_type
-flow_get_drv_type(struct rte_eth_dev *dev __rte_unused,
-		  const struct rte_flow_attr *attr)
+flow_get_drv_type(struct rte_eth_dev *dev, const struct rte_flow_attr *attr)
 {
-	struct priv *priv __rte_unused = dev->data->dev_private;
+	struct priv *priv = dev->data->dev_private;
 	enum mlx5_flow_drv_type type = MLX5_FLOW_TYPE_MAX;
 
-	if (attr->transfer) {
+	if (attr->transfer)
 		type = MLX5_FLOW_TYPE_TCF;
-	} else {
-#ifdef HAVE_IBV_FLOW_DV_SUPPORT
-		type = priv->config.dv_flow_en ?  MLX5_FLOW_TYPE_DV :
-						  MLX5_FLOW_TYPE_VERBS;
-#else
-		type = MLX5_FLOW_TYPE_VERBS;
-#endif
-	}
+	else
+		type = priv->config.dv_flow_en ? MLX5_FLOW_TYPE_DV :
+						 MLX5_FLOW_TYPE_VERBS;
 	return type;
 }
 
-- 
2.11.0

[dpdk-dev] [PATCH 2/5] net/mlx5: fix UDP hash field flag in Direct Verbs

[Yongseok Koh]

Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index becbc57b55..2b362dae11 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -925,9 +925,9 @@ flow_dv_create_item(void *matcher, void *key,
 		tmatcher->priority = MLX5_PRIORITY_MAP_L4;
 		dev_flow->verbs.hash_fields |=
 			mlx5_flow_hashfields_adjust(dev_flow, inner,
-						    ETH_RSS_TCP,
-						    (IBV_RX_HASH_SRC_PORT_TCP |
-						     IBV_RX_HASH_DST_PORT_TCP));
+						    ETH_RSS_UDP,
+						    (IBV_RX_HASH_SRC_PORT_UDP |
+						     IBV_RX_HASH_DST_PORT_UDP));
 		break;
 	case RTE_FLOW_ITEM_TYPE_NVGRE:
 		flow_dv_translate_item_nvgre(tmatcher->mask.buf, key, item,
-- 
2.11.0

Re: [dpdk-dev] [PATCH 2/5] net/mlx5: fix UDP hash field flag in Direct Verbs

[Ori Kam]

> -----Original Message-----
> From: Yongseok Koh
> Sent: Wednesday, October 17, 2018 5:08 AM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 2/5] net/mlx5: fix UDP hash field flag in Direct Verbs
> 
> Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
> Cc: orika@mellanox.com
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> Acked-by: Ori Kam <orika@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index becbc57b55..2b362dae11 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -925,9 +925,9 @@ flow_dv_create_item(void *matcher, void *key,
>  		tmatcher->priority = MLX5_PRIORITY_MAP_L4;
>  		dev_flow->verbs.hash_fields |=
>  			mlx5_flow_hashfields_adjust(dev_flow, inner,
> -						    ETH_RSS_TCP,
> -
> (IBV_RX_HASH_SRC_PORT_TCP |
> -
> IBV_RX_HASH_DST_PORT_TCP));
> +						    ETH_RSS_UDP,
> +
> (IBV_RX_HASH_SRC_PORT_UDP |
> +
> IBV_RX_HASH_DST_PORT_UDP));
>  		break;
>  	case RTE_FLOW_ITEM_TYPE_NVGRE:
>  		flow_dv_translate_item_nvgre(tmatcher->mask.buf, key, item,
> --
> 2.11.0

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

Thanks,
Ori

[dpdk-dev] [PATCH 3/5] net/mlx5: fix item validation in Direct Verbs

[Yongseok Koh]

1) remove MPLS item in validation as it doesn't have a translator.

2) add missing NVGRE item to validation

3) match switch-case order between validation and translation.

Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
Fixes: 3d69434113d1 ("net/mlx5: add Direct Verbs validation function")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 50 +++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 2b362dae11..ea8e1f4831 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -164,6 +164,17 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 					((const struct rte_flow_item_ipv6 *)
 					 items->spec)->hdr.proto;
 			break;
+		case RTE_FLOW_ITEM_TYPE_TCP:
+			ret = mlx5_flow_validate_item_tcp
+						(items, item_flags,
+						 next_protocol,
+						 &rte_flow_item_tcp_mask,
+						 error);
+			if (ret < 0)
+				return ret;
+			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L4_TCP :
+					       MLX5_FLOW_LAYER_OUTER_L4_TCP;
+			break;
 		case RTE_FLOW_ITEM_TYPE_UDP:
 			ret = mlx5_flow_validate_item_udp(items, item_flags,
 							  next_protocol,
@@ -173,16 +184,13 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L4_UDP :
 					       MLX5_FLOW_LAYER_OUTER_L4_UDP;
 			break;
-		case RTE_FLOW_ITEM_TYPE_TCP:
-			ret = mlx5_flow_validate_item_tcp
-						(items, item_flags,
-						 next_protocol,
-						 &rte_flow_item_tcp_mask,
-						 error);
+		case RTE_FLOW_ITEM_TYPE_GRE:
+		case RTE_FLOW_ITEM_TYPE_NVGRE:
+			ret = mlx5_flow_validate_item_gre(items, item_flags,
+							  next_protocol, error);
 			if (ret < 0)
 				return ret;
-			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L4_TCP :
-					       MLX5_FLOW_LAYER_OUTER_L4_TCP;
+			item_flags |= MLX5_FLOW_LAYER_GRE;
 			break;
 		case RTE_FLOW_ITEM_TYPE_VXLAN:
 			ret = mlx5_flow_validate_item_vxlan(items, item_flags,
@@ -199,21 +207,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 				return ret;
 			item_flags |= MLX5_FLOW_LAYER_VXLAN_GPE;
 			break;
-		case RTE_FLOW_ITEM_TYPE_GRE:
-			ret = mlx5_flow_validate_item_gre(items, item_flags,
-							  next_protocol, error);
-			if (ret < 0)
-				return ret;
-			item_flags |= MLX5_FLOW_LAYER_GRE;
-			break;
-		case RTE_FLOW_ITEM_TYPE_MPLS:
-			ret = mlx5_flow_validate_item_mpls(items, item_flags,
-							   next_protocol,
-							   error);
-			if (ret < 0)
-				return ret;
-			item_flags |= MLX5_FLOW_LAYER_MPLS;
-			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
 						  RTE_FLOW_ERROR_TYPE_ITEM,
@@ -879,9 +872,6 @@ flow_dv_create_item(void *matcher, void *key,
 	struct mlx5_flow_dv_matcher *tmatcher = matcher;
 
 	switch (item->type) {
-	case RTE_FLOW_ITEM_TYPE_VOID:
-	case RTE_FLOW_ITEM_TYPE_END:
-		break;
 	case RTE_FLOW_ITEM_TYPE_ETH:
 		flow_dv_translate_item_eth(tmatcher->mask.buf, key, item,
 					   inner);
@@ -929,14 +919,14 @@ flow_dv_create_item(void *matcher, void *key,
 						    (IBV_RX_HASH_SRC_PORT_UDP |
 						     IBV_RX_HASH_DST_PORT_UDP));
 		break;
-	case RTE_FLOW_ITEM_TYPE_NVGRE:
-		flow_dv_translate_item_nvgre(tmatcher->mask.buf, key, item,
-					     inner);
-		break;
 	case RTE_FLOW_ITEM_TYPE_GRE:
 		flow_dv_translate_item_gre(tmatcher->mask.buf, key, item,
 					   inner);
 		break;
+	case RTE_FLOW_ITEM_TYPE_NVGRE:
+		flow_dv_translate_item_nvgre(tmatcher->mask.buf, key, item,
+					     inner);
+		break;
 	case RTE_FLOW_ITEM_TYPE_VXLAN:
 	case RTE_FLOW_ITEM_TYPE_VXLAN_GPE:
 		flow_dv_translate_item_vxlan(tmatcher->mask.buf, key, item,
-- 
2.11.0

[dpdk-dev] [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs

[Yongseok Koh]

If a network layer is specified with no spec, it means wildcard match.
flow_dv_translate_item_*() returns without writing anything if spec is
null and it causes creation of wrong flow. E.g., the following flow has to
patch with any ipv4 packet.

  flow create 0 ingress pattern eth / ipv4 / end actions ...

But, with the current code, it matches any packet because PMD doesn't write
anything about IPv4. The matcher value and mask becomes completely zero. It
should have written the IP version at least. It is same for the rest of
items.

Even if the spec is null, PMD has to write constant fields before return,
e.g. IP version and IP protocol number.

Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 55 ++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index ea8e1f4831..0c09ac8026 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2,7 +2,6 @@
  * Copyright 2018 Mellanox Technologies, Ltd
  */
 
-
 #include <sys/queue.h>
 #include <stdalign.h>
 #include <stdint.h>
@@ -474,10 +473,6 @@ flow_dv_translate_item_ipv4(void *matcher, void *key,
 	char *l24_v;
 	uint8_t tos;
 
-	if (!ipv4_v)
-		return;
-	if (!ipv4_m)
-		ipv4_m = &nic_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -489,6 +484,10 @@ flow_dv_translate_item_ipv4(void *matcher, void *key,
 	}
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 4);
+	if (!ipv4_v)
+		return;
+	if (!ipv4_m)
+		ipv4_m = &nic_mask;
 	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
 			     dst_ipv4_dst_ipv6.ipv4_layout.ipv4);
 	l24_v = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v,
@@ -557,10 +556,6 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
 	int i;
 	int size;
 
-	if (!ipv6_v)
-		return;
-	if (!ipv6_m)
-		ipv6_m = &nic_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -570,6 +565,12 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
 					 outer_headers);
 		headers_v = MLX5_ADDR_OF(fte_match_param, key, outer_headers);
 	}
+	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
+	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
+	if (!ipv6_v)
+		return;
+	if (!ipv6_m)
+		ipv6_m = &nic_mask;
 	size = sizeof(ipv6_m->hdr.dst_addr);
 	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
 			     dst_ipv4_dst_ipv6.ipv6_layout.ipv6);
@@ -585,8 +586,6 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
 	memcpy(l24_m, ipv6_m->hdr.src_addr, size);
 	for (i = 0; i < size; ++i)
 		l24_v[i] = l24_m[i] & ipv6_v->hdr.src_addr[i];
-	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
-	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
 	/* TOS. */
 	vtc_m = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow);
 	vtc_v = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow & ipv6_v->hdr.vtc_flow);
@@ -635,10 +634,6 @@ flow_dv_translate_item_tcp(void *matcher, void *key,
 	void *headers_m;
 	void *headers_v;
 
-	if (!tcp_v)
-		return;
-	if (!tcp_m)
-		tcp_m = &rte_flow_item_tcp_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -650,6 +645,10 @@ flow_dv_translate_item_tcp(void *matcher, void *key,
 	}
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol, IPPROTO_TCP);
+	if (!tcp_v)
+		return;
+	if (!tcp_m)
+		tcp_m = &rte_flow_item_tcp_mask;
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, tcp_sport,
 		 rte_be_to_cpu_16(tcp_m->hdr.src_port));
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, tcp_sport,
@@ -682,10 +681,6 @@ flow_dv_translate_item_udp(void *matcher, void *key,
 	void *headers_m;
 	void *headers_v;
 
-	if (!udp_v)
-		return;
-	if (!udp_m)
-		udp_m = &rte_flow_item_udp_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -697,6 +692,10 @@ flow_dv_translate_item_udp(void *matcher, void *key,
 	}
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol, IPPROTO_UDP);
+	if (!udp_v)
+		return;
+	if (!udp_m)
+		udp_m = &rte_flow_item_udp_mask;
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, udp_sport,
 		 rte_be_to_cpu_16(udp_m->hdr.src_port));
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, udp_sport,
@@ -731,10 +730,6 @@ flow_dv_translate_item_gre(void *matcher, void *key,
 	void *misc_m = MLX5_ADDR_OF(fte_match_param, matcher, misc_parameters);
 	void *misc_v = MLX5_ADDR_OF(fte_match_param, key, misc_parameters);
 
-	if (!gre_v)
-		return;
-	if (!gre_m)
-		gre_m = &rte_flow_item_gre_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -746,6 +741,10 @@ flow_dv_translate_item_gre(void *matcher, void *key,
 	}
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol, IPPROTO_GRE);
+	if (!gre_v)
+		return;
+	if (!gre_m)
+		gre_m = &rte_flow_item_gre_mask;
 	MLX5_SET(fte_match_set_misc, misc_m, gre_protocol,
 		 rte_be_to_cpu_16(gre_m->protocol));
 	MLX5_SET(fte_match_set_misc, misc_v, gre_protocol,
@@ -780,6 +779,7 @@ flow_dv_translate_item_nvgre(void *matcher, void *key,
 	int size;
 	int i;
 
+	flow_dv_translate_item_gre(matcher, key, item, inner);
 	if (!nvgre_v)
 		return;
 	if (!nvgre_m)
@@ -790,7 +790,6 @@ flow_dv_translate_item_nvgre(void *matcher, void *key,
 	memcpy(gre_key_m, tni_flow_id_m, size);
 	for (i = 0; i < size; ++i)
 		gre_key_v[i] = gre_key_m[i] & tni_flow_id_v[i];
-	flow_dv_translate_item_gre(matcher, key, item, inner);
 }
 
 /**
@@ -822,10 +821,6 @@ flow_dv_translate_item_vxlan(void *matcher, void *key,
 	int size;
 	int i;
 
-	if (!vxlan_v)
-		return;
-	if (!vxlan_m)
-		vxlan_m = &rte_flow_item_vxlan_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -841,6 +836,10 @@ flow_dv_translate_item_vxlan(void *matcher, void *key,
 		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);
 	}
+	if (!vxlan_v)
+		return;
+	if (!vxlan_m)
+		vxlan_m = &rte_flow_item_vxlan_mask;
 	size = sizeof(vxlan_m->vni);
 	vni_m = MLX5_ADDR_OF(fte_match_set_misc, misc_m, vxlan_vni);
 	vni_v = MLX5_ADDR_OF(fte_match_set_misc, misc_v, vxlan_vni);
-- 
2.11.0

Re: [dpdk-dev] [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs

[Shahaf Shuler]

Wednesday, October 17, 2018 5:08 AM, Yongseok Koh:
> Subject: [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs
> 
> If a network layer is specified with no spec, it means wildcard match.
> flow_dv_translate_item_*() returns without writing anything if spec is null
> and it causes creation of wrong flow. E.g., the following flow has to patch
> with any ipv4 packet.
> 
>   flow create 0 ingress pattern eth / ipv4 / end actions ...
> 
> But, with the current code, it matches any packet because PMD doesn't write
> anything about IPv4. The matcher value and mask becomes completely zero.
> It should have written the IP version at least. It is same for the rest of items.
> 
> Even if the spec is null, PMD has to write constant fields before return, e.g. IP
> version and IP protocol number.
> 
> Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
> Cc: orika@mellanox.com
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> Acked-by: Ori Kam <orika@mellanox.com>

[...]

>  #include <sys/queue.h>
>  #include <stdalign.h>
>  #include <stdint.h>
> @@ -474,10 +473,6 @@ flow_dv_translate_item_ipv4(void *matcher, void
> *key,
>  	char *l24_v;
>  	uint8_t tos;
> 
> -	if (!ipv4_v)
> -		return;
> -	if (!ipv4_m)
> -		ipv4_m = &nic_mask;
>  	if (inner) {
>  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>  					 inner_headers);
> @@ -489,6 +484,10 @@ flow_dv_translate_item_ipv4(void *matcher, void
> *key,
>  	}
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 4);

Is matching on the ip version is enough? Don't we need to match also the ethertype? 
Meaning maybe the value on the IP offset can be 4 even though it is not a IPv4 header.  

Same question for IPv6. 

> +	if (!ipv4_v)
> +		return;
> +	if (!ipv4_m)
> +		ipv4_m = &nic_mask;
>  	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
>  			     dst_ipv4_dst_ipv6.ipv4_layout.ipv4);
>  	l24_v = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v, @@ -
> 557,10 +556,6 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
>  	int i;
>  	int size;
> 
> -	if (!ipv6_v)
> -		return;
> -	if (!ipv6_m)
> -		ipv6_m = &nic_mask;
>  	if (inner) {
>  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>  					 inner_headers);
> @@ -570,6 +565,12 @@ flow_dv_translate_item_ipv6(void *matcher, void
> *key,
>  					 outer_headers);
>  		headers_v = MLX5_ADDR_OF(fte_match_param, key,
> outer_headers);
>  	}
> +	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> +	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
> +	if (!ipv6_v)
> +		return;
> +	if (!ipv6_m)
> +		ipv6_m = &nic_mask;
>  	size = sizeof(ipv6_m->hdr.dst_addr);
>  	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
>  			     dst_ipv4_dst_ipv6.ipv6_layout.ipv6);
> @@ -585,8 +586,6 @@ flow_dv_translate_item_ipv6(void *matcher, void
> *key,
>  	memcpy(l24_m, ipv6_m->hdr.src_addr, size);
>  	for (i = 0; i < size; ++i)
>  		l24_v[i] = l24_m[i] & ipv6_v->hdr.src_addr[i];
> -	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> -	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
>  	/* TOS. */
>  	vtc_m = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow);
>  	vtc_v = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow & ipv6_v-
> >hdr.vtc_flow); @@ -635,10 +634,6 @@ flow_dv_translate_item_tcp(void
> *matcher, void *key,
>  	void *headers_m;
>  	void *headers_v;
> 
> -	if (!tcp_v)
> -		return;
> -	if (!tcp_m)
> -		tcp_m = &rte_flow_item_tcp_mask;
>  	if (inner) {
>  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>  					 inner_headers);
> @@ -650,6 +645,10 @@ flow_dv_translate_item_tcp(void *matcher, void
> *key,
>  	}
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> IPPROTO_TCP);
> +	if (!tcp_v)
> +		return;
> +	if (!tcp_m)
> +		tcp_m = &rte_flow_item_tcp_mask;
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, tcp_sport,
>  		 rte_be_to_cpu_16(tcp_m->hdr.src_port));
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, tcp_sport, @@ -
> 682,10 +681,6 @@ flow_dv_translate_item_udp(void *matcher, void *key,
>  	void *headers_m;
>  	void *headers_v;
> 
> -	if (!udp_v)
> -		return;
> -	if (!udp_m)
> -		udp_m = &rte_flow_item_udp_mask;
>  	if (inner) {
>  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>  					 inner_headers);
> @@ -697,6 +692,10 @@ flow_dv_translate_item_udp(void *matcher, void
> *key,
>  	}
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> IPPROTO_UDP);
> +	if (!udp_v)
> +		return;
> +	if (!udp_m)
> +		udp_m = &rte_flow_item_udp_mask;
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, udp_sport,
>  		 rte_be_to_cpu_16(udp_m->hdr.src_port));
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, udp_sport, @@ -
> 731,10 +730,6 @@ flow_dv_translate_item_gre(void *matcher, void *key,
>  	void *misc_m = MLX5_ADDR_OF(fte_match_param, matcher,
> misc_parameters);
>  	void *misc_v = MLX5_ADDR_OF(fte_match_param, key,
> misc_parameters);
> 
> -	if (!gre_v)
> -		return;
> -	if (!gre_m)
> -		gre_m = &rte_flow_item_gre_mask;
>  	if (inner) {
>  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>  					 inner_headers);
> @@ -746,6 +741,10 @@ flow_dv_translate_item_gre(void *matcher, void
> *key,
>  	}
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> IPPROTO_GRE);
> +	if (!gre_v)
> +		return;
> +	if (!gre_m)
> +		gre_m = &rte_flow_item_gre_mask;
>  	MLX5_SET(fte_match_set_misc, misc_m, gre_protocol,
>  		 rte_be_to_cpu_16(gre_m->protocol));
>  	MLX5_SET(fte_match_set_misc, misc_v, gre_protocol, @@ -780,6
> +779,7 @@ flow_dv_translate_item_nvgre(void *matcher, void *key,
>  	int size;
>  	int i;
> 
> +	flow_dv_translate_item_gre(matcher, key, item, inner);
>  	if (!nvgre_v)
>  		return;
>  	if (!nvgre_m)
> @@ -790,7 +790,6 @@ flow_dv_translate_item_nvgre(void *matcher, void
> *key,
>  	memcpy(gre_key_m, tni_flow_id_m, size);
>  	for (i = 0; i < size; ++i)
>  		gre_key_v[i] = gre_key_m[i] & tni_flow_id_v[i];
> -	flow_dv_translate_item_gre(matcher, key, item, inner);
>  }
> 
>  /**
> @@ -822,10 +821,6 @@ flow_dv_translate_item_vxlan(void *matcher, void
> *key,
>  	int size;
>  	int i;
> 
> -	if (!vxlan_v)
> -		return;
> -	if (!vxlan_m)
> -		vxlan_m = &rte_flow_item_vxlan_mask;
>  	if (inner) {
>  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>  					 inner_headers);
> @@ -841,6 +836,10 @@ flow_dv_translate_item_vxlan(void *matcher, void
> *key,
>  		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);
>  	}
> +	if (!vxlan_v)
> +		return;
> +	if (!vxlan_m)
> +		vxlan_m = &rte_flow_item_vxlan_mask;
>  	size = sizeof(vxlan_m->vni);
>  	vni_m = MLX5_ADDR_OF(fte_match_set_misc, misc_m, vxlan_vni);
>  	vni_v = MLX5_ADDR_OF(fte_match_set_misc, misc_v, vxlan_vni);
> --
> 2.11.0

Re: [dpdk-dev] [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs

[Ori Kam]

PSB

> -----Original Message-----
> From: Shahaf Shuler
> Sent: Tuesday, October 23, 2018 10:42 AM
> To: Yongseok Koh <yskoh@mellanox.com>
> Cc: dev@dpdk.org; Ori Kam <orika@mellanox.com>
> Subject: RE: [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs
> 
> Wednesday, October 17, 2018 5:08 AM, Yongseok Koh:
> > Subject: [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs
> >
> > If a network layer is specified with no spec, it means wildcard match.
> > flow_dv_translate_item_*() returns without writing anything if spec is null
> > and it causes creation of wrong flow. E.g., the following flow has to patch
> > with any ipv4 packet.
> >
> >   flow create 0 ingress pattern eth / ipv4 / end actions ...
> >
> > But, with the current code, it matches any packet because PMD doesn't write
> > anything about IPv4. The matcher value and mask becomes completely zero.
> > It should have written the IP version at least. It is same for the rest of items.
> >
> > Even if the spec is null, PMD has to write constant fields before return, e.g. IP
> > version and IP protocol number.
> >
> > Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
> > Cc: orika@mellanox.com
> >
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > Acked-by: Ori Kam <orika@mellanox.com>
> 
> [...]
> 
> >  #include <sys/queue.h>
> >  #include <stdalign.h>
> >  #include <stdint.h>
> > @@ -474,10 +473,6 @@ flow_dv_translate_item_ipv4(void *matcher, void
> > *key,
> >  	char *l24_v;
> >  	uint8_t tos;
> >
> > -	if (!ipv4_v)
> > -		return;
> > -	if (!ipv4_m)
> > -		ipv4_m = &nic_mask;
> >  	if (inner) {
> >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> >  					 inner_headers);
> > @@ -489,6 +484,10 @@ flow_dv_translate_item_ipv4(void *matcher, void
> > *key,
> >  	}
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 4);
> 
> Is matching on the ip version is enough? Don't we need to match also the
> ethertype?
> Meaning maybe the value on the IP offset can be 4 even though it is not a IPv4
> header.
> 
> Same question for IPv6.

I think you are correct, 
We should also test the ethertype.
> 
> > +	if (!ipv4_v)
> > +		return;
> > +	if (!ipv4_m)
> > +		ipv4_m = &nic_mask;
> >  	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
> >  			     dst_ipv4_dst_ipv6.ipv4_layout.ipv4);
> >  	l24_v = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v, @@ -
> > 557,10 +556,6 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
> >  	int i;
> >  	int size;
> >
> > -	if (!ipv6_v)
> > -		return;
> > -	if (!ipv6_m)
> > -		ipv6_m = &nic_mask;
> >  	if (inner) {
> >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> >  					 inner_headers);
> > @@ -570,6 +565,12 @@ flow_dv_translate_item_ipv6(void *matcher, void
> > *key,
> >  					 outer_headers);
> >  		headers_v = MLX5_ADDR_OF(fte_match_param, key,
> > outer_headers);
> >  	}
> > +	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> > +	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
> > +	if (!ipv6_v)
> > +		return;
> > +	if (!ipv6_m)
> > +		ipv6_m = &nic_mask;
> >  	size = sizeof(ipv6_m->hdr.dst_addr);
> >  	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
> >  			     dst_ipv4_dst_ipv6.ipv6_layout.ipv6);
> > @@ -585,8 +586,6 @@ flow_dv_translate_item_ipv6(void *matcher, void
> > *key,
> >  	memcpy(l24_m, ipv6_m->hdr.src_addr, size);
> >  	for (i = 0; i < size; ++i)
> >  		l24_v[i] = l24_m[i] & ipv6_v->hdr.src_addr[i];
> > -	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> > -	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
> >  	/* TOS. */
> >  	vtc_m = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow);
> >  	vtc_v = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow & ipv6_v-
> > >hdr.vtc_flow); @@ -635,10 +634,6 @@ flow_dv_translate_item_tcp(void
> > *matcher, void *key,
> >  	void *headers_m;
> >  	void *headers_v;
> >
> > -	if (!tcp_v)
> > -		return;
> > -	if (!tcp_m)
> > -		tcp_m = &rte_flow_item_tcp_mask;
> >  	if (inner) {
> >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> >  					 inner_headers);
> > @@ -650,6 +645,10 @@ flow_dv_translate_item_tcp(void *matcher, void
> > *key,
> >  	}
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> > IPPROTO_TCP);
> > +	if (!tcp_v)
> > +		return;
> > +	if (!tcp_m)
> > +		tcp_m = &rte_flow_item_tcp_mask;
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, tcp_sport,
> >  		 rte_be_to_cpu_16(tcp_m->hdr.src_port));
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, tcp_sport, @@ -
> > 682,10 +681,6 @@ flow_dv_translate_item_udp(void *matcher, void *key,
> >  	void *headers_m;
> >  	void *headers_v;
> >
> > -	if (!udp_v)
> > -		return;
> > -	if (!udp_m)
> > -		udp_m = &rte_flow_item_udp_mask;
> >  	if (inner) {
> >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> >  					 inner_headers);
> > @@ -697,6 +692,10 @@ flow_dv_translate_item_udp(void *matcher, void
> > *key,
> >  	}
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> > IPPROTO_UDP);
> > +	if (!udp_v)
> > +		return;
> > +	if (!udp_m)
> > +		udp_m = &rte_flow_item_udp_mask;
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, udp_sport,
> >  		 rte_be_to_cpu_16(udp_m->hdr.src_port));
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, udp_sport, @@ -
> > 731,10 +730,6 @@ flow_dv_translate_item_gre(void *matcher, void *key,
> >  	void *misc_m = MLX5_ADDR_OF(fte_match_param, matcher,
> > misc_parameters);
> >  	void *misc_v = MLX5_ADDR_OF(fte_match_param, key,
> > misc_parameters);
> >
> > -	if (!gre_v)
> > -		return;
> > -	if (!gre_m)
> > -		gre_m = &rte_flow_item_gre_mask;
> >  	if (inner) {
> >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> >  					 inner_headers);
> > @@ -746,6 +741,10 @@ flow_dv_translate_item_gre(void *matcher, void
> > *key,
> >  	}
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
> >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> > IPPROTO_GRE);
> > +	if (!gre_v)
> > +		return;
> > +	if (!gre_m)
> > +		gre_m = &rte_flow_item_gre_mask;
> >  	MLX5_SET(fte_match_set_misc, misc_m, gre_protocol,
> >  		 rte_be_to_cpu_16(gre_m->protocol));
> >  	MLX5_SET(fte_match_set_misc, misc_v, gre_protocol, @@ -780,6
> > +779,7 @@ flow_dv_translate_item_nvgre(void *matcher, void *key,
> >  	int size;
> >  	int i;
> >
> > +	flow_dv_translate_item_gre(matcher, key, item, inner);
> >  	if (!nvgre_v)
> >  		return;
> >  	if (!nvgre_m)
> > @@ -790,7 +790,6 @@ flow_dv_translate_item_nvgre(void *matcher, void
> > *key,
> >  	memcpy(gre_key_m, tni_flow_id_m, size);
> >  	for (i = 0; i < size; ++i)
> >  		gre_key_v[i] = gre_key_m[i] & tni_flow_id_v[i];
> > -	flow_dv_translate_item_gre(matcher, key, item, inner);
> >  }
> >
> >  /**
> > @@ -822,10 +821,6 @@ flow_dv_translate_item_vxlan(void *matcher, void
> > *key,
> >  	int size;
> >  	int i;
> >
> > -	if (!vxlan_v)
> > -		return;
> > -	if (!vxlan_m)
> > -		vxlan_m = &rte_flow_item_vxlan_mask;
> >  	if (inner) {
> >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> >  					 inner_headers);
> > @@ -841,6 +836,10 @@ flow_dv_translate_item_vxlan(void *matcher, void
> > *key,
> >  		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);
> >  	}
> > +	if (!vxlan_v)
> > +		return;
> > +	if (!vxlan_m)
> > +		vxlan_m = &rte_flow_item_vxlan_mask;
> >  	size = sizeof(vxlan_m->vni);
> >  	vni_m = MLX5_ADDR_OF(fte_match_set_misc, misc_m, vxlan_vni);
> >  	vni_v = MLX5_ADDR_OF(fte_match_set_misc, misc_v, vxlan_vni);
> > --
> > 2.11.0

Re: [dpdk-dev] [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs

[Yongseok Koh]

On Tue, Oct 23, 2018 at 08:25:15AM -0700, Ori Kam wrote:
> PSB
> 
> > -----Original Message-----
> > From: Shahaf Shuler
> > Sent: Tuesday, October 23, 2018 10:42 AM
> > To: Yongseok Koh <yskoh@mellanox.com>
> > Cc: dev@dpdk.org; Ori Kam <orika@mellanox.com>
> > Subject: RE: [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs
> > 
> > Wednesday, October 17, 2018 5:08 AM, Yongseok Koh:
> > > Subject: [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs
> > >
> > > If a network layer is specified with no spec, it means wildcard match.
> > > flow_dv_translate_item_*() returns without writing anything if spec is null
> > > and it causes creation of wrong flow. E.g., the following flow has to patch
> > > with any ipv4 packet.
> > >
> > >   flow create 0 ingress pattern eth / ipv4 / end actions ...
> > >
> > > But, with the current code, it matches any packet because PMD doesn't write
> > > anything about IPv4. The matcher value and mask becomes completely zero.
> > > It should have written the IP version at least. It is same for the rest of items.
> > >
> > > Even if the spec is null, PMD has to write constant fields before return, e.g. IP
> > > version and IP protocol number.
> > >
> > > Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
> > > Cc: orika@mellanox.com
> > >
> > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > > Acked-by: Ori Kam <orika@mellanox.com>
> > 
> > [...]
> > 
> > >  #include <sys/queue.h>
> > >  #include <stdalign.h>
> > >  #include <stdint.h>
> > > @@ -474,10 +473,6 @@ flow_dv_translate_item_ipv4(void *matcher, void
> > > *key,
> > >  	char *l24_v;
> > >  	uint8_t tos;
> > >
> > > -	if (!ipv4_v)
> > > -		return;
> > > -	if (!ipv4_m)
> > > -		ipv4_m = &nic_mask;
> > >  	if (inner) {
> > >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> > >  					 inner_headers);
> > > @@ -489,6 +484,10 @@ flow_dv_translate_item_ipv4(void *matcher, void
> > > *key,
> > >  	}
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 4);
> > 
> > Is matching on the ip version is enough? Don't we need to match also the
> > ethertype?
> > Meaning maybe the value on the IP offset can be 4 even though it is not a IPv4
> > header.
> > 
> > Same question for IPv6.
> 
> I think you are correct, 
> We should also test the ethertype.

Nope, we should test either one if kernel driver code is right. When I wrote
this fix, I checked the driver code.


static int parse_flow_attr(struct mlx5_core_dev *mdev, u32 *match_c,
			   u32 *match_v, const union ib_flow_spec *ib_spec,
			   const struct ib_flow_attr *flow_attr,
			   struct mlx5_flow_act *action, u32 prev_type)
{
[...]

	if (ib_spec->type & IB_FLOW_SPEC_INNER) {
		headers_c = MLX5_ADDR_OF(fte_match_param, match_c,
					 inner_headers);
		headers_v = MLX5_ADDR_OF(fte_match_param, match_v,
					 inner_headers);
		match_ipv = MLX5_CAP_FLOWTABLE_NIC_RX(mdev,
					ft_field_support.inner_ip_version);
	} else {
		headers_c = MLX5_ADDR_OF(fte_match_param, match_c,
					 outer_headers);
		headers_v = MLX5_ADDR_OF(fte_match_param, match_v,
					 outer_headers);
		match_ipv = MLX5_CAP_FLOWTABLE_NIC_RX(mdev,
					ft_field_support.outer_ip_version);
	}
[...]

	switch (ib_spec->type & ~IB_FLOW_SPEC_INNER) {
[...]
	case IB_FLOW_SPEC_IPV4:
		if (match_ipv) {
			MLX5_SET(fte_match_set_lyr_2_4, headers_c,
				 ip_version, 0xf);
			MLX5_SET(fte_match_set_lyr_2_4, headers_v,
				 ip_version, MLX5_FS_IPV4_VERSION);
		} else {
			MLX5_SET(fte_match_set_lyr_2_4, headers_c,
				 ethertype, 0xffff);
			MLX5_SET(fte_match_set_lyr_2_4, headers_v,
				 ethertype, ETH_P_IP);
		}

It does look like it depends on device capability. So, I thought Xueming/Ori
already knew the result and made a decision to not check the capability. Any
comment?


Thanks,
Yongseok

> > 
> > > +	if (!ipv4_v)
> > > +		return;
> > > +	if (!ipv4_m)
> > > +		ipv4_m = &nic_mask;
> > >  	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
> > >  			     dst_ipv4_dst_ipv6.ipv4_layout.ipv4);
> > >  	l24_v = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v, @@ -
> > > 557,10 +556,6 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
> > >  	int i;
> > >  	int size;
> > >
> > > -	if (!ipv6_v)
> > > -		return;
> > > -	if (!ipv6_m)
> > > -		ipv6_m = &nic_mask;
> > >  	if (inner) {
> > >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> > >  					 inner_headers);
> > > @@ -570,6 +565,12 @@ flow_dv_translate_item_ipv6(void *matcher, void
> > > *key,
> > >  					 outer_headers);
> > >  		headers_v = MLX5_ADDR_OF(fte_match_param, key,
> > > outer_headers);
> > >  	}
> > > +	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> > > +	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
> > > +	if (!ipv6_v)
> > > +		return;
> > > +	if (!ipv6_m)
> > > +		ipv6_m = &nic_mask;
> > >  	size = sizeof(ipv6_m->hdr.dst_addr);
> > >  	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
> > >  			     dst_ipv4_dst_ipv6.ipv6_layout.ipv6);
> > > @@ -585,8 +586,6 @@ flow_dv_translate_item_ipv6(void *matcher, void
> > > *key,
> > >  	memcpy(l24_m, ipv6_m->hdr.src_addr, size);
> > >  	for (i = 0; i < size; ++i)
> > >  		l24_v[i] = l24_m[i] & ipv6_v->hdr.src_addr[i];
> > > -	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> > > -	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
> > >  	/* TOS. */
> > >  	vtc_m = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow);
> > >  	vtc_v = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow & ipv6_v-
> > > >hdr.vtc_flow); @@ -635,10 +634,6 @@ flow_dv_translate_item_tcp(void
> > > *matcher, void *key,
> > >  	void *headers_m;
> > >  	void *headers_v;
> > >
> > > -	if (!tcp_v)
> > > -		return;
> > > -	if (!tcp_m)
> > > -		tcp_m = &rte_flow_item_tcp_mask;
> > >  	if (inner) {
> > >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> > >  					 inner_headers);
> > > @@ -650,6 +645,10 @@ flow_dv_translate_item_tcp(void *matcher, void
> > > *key,
> > >  	}
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> > > IPPROTO_TCP);
> > > +	if (!tcp_v)
> > > +		return;
> > > +	if (!tcp_m)
> > > +		tcp_m = &rte_flow_item_tcp_mask;
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, tcp_sport,
> > >  		 rte_be_to_cpu_16(tcp_m->hdr.src_port));
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, tcp_sport, @@ -
> > > 682,10 +681,6 @@ flow_dv_translate_item_udp(void *matcher, void *key,
> > >  	void *headers_m;
> > >  	void *headers_v;
> > >
> > > -	if (!udp_v)
> > > -		return;
> > > -	if (!udp_m)
> > > -		udp_m = &rte_flow_item_udp_mask;
> > >  	if (inner) {
> > >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> > >  					 inner_headers);
> > > @@ -697,6 +692,10 @@ flow_dv_translate_item_udp(void *matcher, void
> > > *key,
> > >  	}
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> > > IPPROTO_UDP);
> > > +	if (!udp_v)
> > > +		return;
> > > +	if (!udp_m)
> > > +		udp_m = &rte_flow_item_udp_mask;
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, udp_sport,
> > >  		 rte_be_to_cpu_16(udp_m->hdr.src_port));
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, udp_sport, @@ -
> > > 731,10 +730,6 @@ flow_dv_translate_item_gre(void *matcher, void *key,
> > >  	void *misc_m = MLX5_ADDR_OF(fte_match_param, matcher,
> > > misc_parameters);
> > >  	void *misc_v = MLX5_ADDR_OF(fte_match_param, key,
> > > misc_parameters);
> > >
> > > -	if (!gre_v)
> > > -		return;
> > > -	if (!gre_m)
> > > -		gre_m = &rte_flow_item_gre_mask;
> > >  	if (inner) {
> > >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> > >  					 inner_headers);
> > > @@ -746,6 +741,10 @@ flow_dv_translate_item_gre(void *matcher, void
> > > *key,
> > >  	}
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> > > IPPROTO_GRE);
> > > +	if (!gre_v)
> > > +		return;
> > > +	if (!gre_m)
> > > +		gre_m = &rte_flow_item_gre_mask;
> > >  	MLX5_SET(fte_match_set_misc, misc_m, gre_protocol,
> > >  		 rte_be_to_cpu_16(gre_m->protocol));
> > >  	MLX5_SET(fte_match_set_misc, misc_v, gre_protocol, @@ -780,6
> > > +779,7 @@ flow_dv_translate_item_nvgre(void *matcher, void *key,
> > >  	int size;
> > >  	int i;
> > >
> > > +	flow_dv_translate_item_gre(matcher, key, item, inner);
> > >  	if (!nvgre_v)
> > >  		return;
> > >  	if (!nvgre_m)
> > > @@ -790,7 +790,6 @@ flow_dv_translate_item_nvgre(void *matcher, void
> > > *key,
> > >  	memcpy(gre_key_m, tni_flow_id_m, size);
> > >  	for (i = 0; i < size; ++i)
> > >  		gre_key_v[i] = gre_key_m[i] & tni_flow_id_v[i];
> > > -	flow_dv_translate_item_gre(matcher, key, item, inner);
> > >  }
> > >
> > >  /**
> > > @@ -822,10 +821,6 @@ flow_dv_translate_item_vxlan(void *matcher, void
> > > *key,
> > >  	int size;
> > >  	int i;
> > >
> > > -	if (!vxlan_v)
> > > -		return;
> > > -	if (!vxlan_m)
> > > -		vxlan_m = &rte_flow_item_vxlan_mask;
> > >  	if (inner) {
> > >  		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> > >  					 inner_headers);
> > > @@ -841,6 +836,10 @@ flow_dv_translate_item_vxlan(void *matcher, void
> > > *key,
> > >  		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);
> > >  	}
> > > +	if (!vxlan_v)
> > > +		return;
> > > +	if (!vxlan_m)
> > > +		vxlan_m = &rte_flow_item_vxlan_mask;
> > >  	size = sizeof(vxlan_m->vni);
> > >  	vni_m = MLX5_ADDR_OF(fte_match_set_misc, misc_m, vxlan_vni);
> > >  	vni_v = MLX5_ADDR_OF(fte_match_set_misc, misc_v, vxlan_vni);
> > > --
> > > 2.11.0
> 

[dpdk-dev] [PATCH 5/5] net/mlx5: fix flow mark ID conversion in Direct Verbs

[Yongseok Koh]

Fixes: d02cb0691299 ("net/mlx5: add Direct Verbs translate actions")
Cc: orika@mellanox.com

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

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 0c09ac8026..cbda05455a 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -959,15 +959,16 @@ flow_dv_create_action(const struct rte_flow_action *action,
 	case RTE_FLOW_ACTION_TYPE_FLAG:
 		dev_flow->dv.actions[actions_n].type = MLX5DV_FLOW_ACTION_TAG;
 		dev_flow->dv.actions[actions_n].tag_value =
-			MLX5_FLOW_MARK_DEFAULT;
+			mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT);
 		actions_n++;
 		flow->actions |= MLX5_FLOW_ACTION_FLAG;
 		break;
 	case RTE_FLOW_ACTION_TYPE_MARK:
 		dev_flow->dv.actions[actions_n].type = MLX5DV_FLOW_ACTION_TAG;
 		dev_flow->dv.actions[actions_n].tag_value =
-			((const struct rte_flow_action_mark *)
-			 (action->conf))->id;
+			mlx5_flow_mark_set
+			(((const struct rte_flow_action_mark *)
+			  (action->conf))->id);
 		flow->actions |= MLX5_FLOW_ACTION_MARK;
 		actions_n++;
 		break;
-- 
2.11.0

Re: [dpdk-dev] [PATCH 5/5] net/mlx5: fix flow mark ID conversion in Direct Verbs

[Ori Kam]

> -----Original Message-----
> From: Yongseok Koh
> Sent: Wednesday, October 17, 2018 5:08 AM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 5/5] net/mlx5: fix flow mark ID conversion in Direct Verbs
> 
> Fixes: d02cb0691299 ("net/mlx5: add Direct Verbs translate actions")
> Cc: orika@mellanox.com
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 0c09ac8026..cbda05455a 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -959,15 +959,16 @@ flow_dv_create_action(const struct rte_flow_action
> *action,
>  	case RTE_FLOW_ACTION_TYPE_FLAG:
>  		dev_flow->dv.actions[actions_n].type =
> MLX5DV_FLOW_ACTION_TAG;
>  		dev_flow->dv.actions[actions_n].tag_value =
> -			MLX5_FLOW_MARK_DEFAULT;
> +			mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT);
>  		actions_n++;
>  		flow->actions |= MLX5_FLOW_ACTION_FLAG;
>  		break;
>  	case RTE_FLOW_ACTION_TYPE_MARK:
>  		dev_flow->dv.actions[actions_n].type =
> MLX5DV_FLOW_ACTION_TAG;
>  		dev_flow->dv.actions[actions_n].tag_value =
> -			((const struct rte_flow_action_mark *)
> -			 (action->conf))->id;
> +			mlx5_flow_mark_set
> +			(((const struct rte_flow_action_mark *)
> +			  (action->conf))->id);
>  		flow->actions |= MLX5_FLOW_ACTION_MARK;
>  		actions_n++;
>  		break;
> --
> 2.11.0

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

Thanks,
Ori

Re: [dpdk-dev] [PATCH 1/5] net/mlx5: add warning message for Direct Verbs flow

[Ori Kam]

> -----Original Message-----
> From: Yongseok Koh
> Sent: Wednesday, October 17, 2018 5:08 AM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: [PATCH 1/5] net/mlx5: add warning message for Direct Verbs flow
> 
> In case that the library doesn't support DV flow, if enabled by
> 'dv_flow_en=1', print out a warning message and disable it.
> 
> Fixes: 51e72d386c99 ("net/mlx5: add runtime parameter to enable Direct
> Verbs")
> Cc: orika@mellanox.com
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> Acked-by: Ori Kam <orika@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5.c      |  6 ++++++
>  drivers/net/mlx5/mlx5_flow.c | 20 +++++++-------------
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 795a219771..70b8626126 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -1016,6 +1016,12 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>  		cs_desc.counter_type, cs_desc.num_of_cs,
>  		cs_desc.attributes);
>  #endif
> +#ifndef HAVE_IBV_FLOW_DV_SUPPORT
> +	if (config.dv_flow_en) {
> +		DRV_LOG(WARNING, "DV flow is not supported");
> +		config.dv_flow_en = 0;
> +	}
> +#endif
>  	config.ind_table_max_size =
>  		attr.rss_caps.max_rwq_indirection_table_size;
>  	/*
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index bd70fce248..04e287f160 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -1673,25 +1673,19 @@ const struct mlx5_flow_driver_ops
> mlx5_flow_null_drv_ops = {
>   *   Pointer to the flow attributes.
>   *
>   * @return
> - *   flow driver type if supported, MLX5_FLOW_TYPE_MAX otherwise.
> + *   flow driver type, MLX5_FLOW_TYPE_MAX otherwise.
>   */
>  static enum mlx5_flow_drv_type
> -flow_get_drv_type(struct rte_eth_dev *dev __rte_unused,
> -		  const struct rte_flow_attr *attr)
> +flow_get_drv_type(struct rte_eth_dev *dev, const struct rte_flow_attr *attr)
>  {
> -	struct priv *priv __rte_unused = dev->data->dev_private;
> +	struct priv *priv = dev->data->dev_private;
>  	enum mlx5_flow_drv_type type = MLX5_FLOW_TYPE_MAX;
> 
> -	if (attr->transfer) {
> +	if (attr->transfer)
>  		type = MLX5_FLOW_TYPE_TCF;
> -	} else {
> -#ifdef HAVE_IBV_FLOW_DV_SUPPORT
> -		type = priv->config.dv_flow_en ?  MLX5_FLOW_TYPE_DV :
> -						  MLX5_FLOW_TYPE_VERBS;
> -#else
> -		type = MLX5_FLOW_TYPE_VERBS;
> -#endif
> -	}
> +	else
> +		type = priv->config.dv_flow_en ? MLX5_FLOW_TYPE_DV :
> +						 MLX5_FLOW_TYPE_VERBS;
>  	return type;
>  }
> 
> --
> 2.11.0

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

Thanks,
Ori

[dpdk-dev] [PATCH v2 0/5] net/mlx5: fixes for Direct Verbs flow

[Yongseok Koh]

v2:
* rebase on top of the new branch tip


Yongseok Koh (5):
  net/mlx5: add warning message for Direct Verbs flow
  net/mlx5: fix UDP hash field flag in Direct Verbs
  net/mlx5: fix item validation in Direct Verbs
  net/mlx5: fix wildcard item for Direct Verbs
  net/mlx5: fix flow mark ID conversion in Direct Verbs

 drivers/net/mlx5/mlx5.c         |   6 ++
 drivers/net/mlx5/mlx5_flow.c    |  20 +++----
 drivers/net/mlx5/mlx5_flow_dv.c | 118 ++++++++++++++++++----------------------
 3 files changed, 67 insertions(+), 77 deletions(-)

-- 
2.11.0

[dpdk-dev] [PATCH v2 1/5] net/mlx5: add warning message for Direct Verbs flow

[Yongseok Koh]

In case that the library doesn't support DV flow, if enabled by
'dv_flow_en=1', print out a warning message and disable it.

Fixes: 51e72d386c99 ("net/mlx5: add runtime parameter to enable Direct Verbs")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5.c      |  6 ++++++
 drivers/net/mlx5/mlx5_flow.c | 20 +++++++-------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 97be494de9..10a724e944 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1016,6 +1016,12 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 		cs_desc.counter_type, cs_desc.num_of_cs,
 		cs_desc.attributes);
 #endif
+#ifndef HAVE_IBV_FLOW_DV_SUPPORT
+	if (config.dv_flow_en) {
+		DRV_LOG(WARNING, "DV flow is not supported");
+		config.dv_flow_en = 0;
+	}
+#endif
 	config.ind_table_max_size =
 		attr.rss_caps.max_rwq_indirection_table_size;
 	/*
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index df5c34e81c..6219c9d68a 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1685,25 +1685,19 @@ const struct mlx5_flow_driver_ops mlx5_flow_null_drv_ops = {
  *   Pointer to the flow attributes.
  *
  * @return
- *   flow driver type if supported, MLX5_FLOW_TYPE_MAX otherwise.
+ *   flow driver type, MLX5_FLOW_TYPE_MAX otherwise.
  */
 static enum mlx5_flow_drv_type
-flow_get_drv_type(struct rte_eth_dev *dev __rte_unused,
-		  const struct rte_flow_attr *attr)
+flow_get_drv_type(struct rte_eth_dev *dev, const struct rte_flow_attr *attr)
 {
-	struct priv *priv __rte_unused = dev->data->dev_private;
+	struct priv *priv = dev->data->dev_private;
 	enum mlx5_flow_drv_type type = MLX5_FLOW_TYPE_MAX;
 
-	if (attr->transfer) {
+	if (attr->transfer)
 		type = MLX5_FLOW_TYPE_TCF;
-	} else {
-#ifdef HAVE_IBV_FLOW_DV_SUPPORT
-		type = priv->config.dv_flow_en ?  MLX5_FLOW_TYPE_DV :
-						  MLX5_FLOW_TYPE_VERBS;
-#else
-		type = MLX5_FLOW_TYPE_VERBS;
-#endif
-	}
+	else
+		type = priv->config.dv_flow_en ? MLX5_FLOW_TYPE_DV :
+						 MLX5_FLOW_TYPE_VERBS;
 	return type;
 }
 
-- 
2.11.0

[dpdk-dev] [PATCH v2 2/5] net/mlx5: fix UDP hash field flag in Direct Verbs

[Yongseok Koh]

Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index e8f409fd07..57884e9f98 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1028,9 +1028,9 @@ flow_dv_create_item(void *matcher, void *key,
 		tmatcher->priority = MLX5_PRIORITY_MAP_L4;
 		dev_flow->verbs.hash_fields |=
 			mlx5_flow_hashfields_adjust(dev_flow, inner,
-						    ETH_RSS_TCP,
-						    (IBV_RX_HASH_SRC_PORT_TCP |
-						     IBV_RX_HASH_DST_PORT_TCP));
+						    ETH_RSS_UDP,
+						    (IBV_RX_HASH_SRC_PORT_UDP |
+						     IBV_RX_HASH_DST_PORT_UDP));
 		break;
 	case RTE_FLOW_ITEM_TYPE_NVGRE:
 		flow_dv_translate_item_nvgre(tmatcher->mask.buf, key, item,
-- 
2.11.0

[dpdk-dev] [PATCH v2 3/5] net/mlx5: fix item validation in Direct Verbs

[Yongseok Koh]

1) remove MPLS item in validation as it doesn't have a translator.

2) add missing NVGRE item to validation

3) match switch-case order between validation and translation.

Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
Fixes: 3d69434113d1 ("net/mlx5: add Direct Verbs validation function")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 50 +++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 57884e9f98..15603401cf 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -225,6 +225,17 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 					((const struct rte_flow_item_ipv6 *)
 					 items->spec)->hdr.proto;
 			break;
+		case RTE_FLOW_ITEM_TYPE_TCP:
+			ret = mlx5_flow_validate_item_tcp
+						(items, item_flags,
+						 next_protocol,
+						 &rte_flow_item_tcp_mask,
+						 error);
+			if (ret < 0)
+				return ret;
+			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L4_TCP :
+					       MLX5_FLOW_LAYER_OUTER_L4_TCP;
+			break;
 		case RTE_FLOW_ITEM_TYPE_UDP:
 			ret = mlx5_flow_validate_item_udp(items, item_flags,
 							  next_protocol,
@@ -234,16 +245,13 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L4_UDP :
 					       MLX5_FLOW_LAYER_OUTER_L4_UDP;
 			break;
-		case RTE_FLOW_ITEM_TYPE_TCP:
-			ret = mlx5_flow_validate_item_tcp
-						(items, item_flags,
-						 next_protocol,
-						 &rte_flow_item_tcp_mask,
-						 error);
+		case RTE_FLOW_ITEM_TYPE_GRE:
+		case RTE_FLOW_ITEM_TYPE_NVGRE:
+			ret = mlx5_flow_validate_item_gre(items, item_flags,
+							  next_protocol, error);
 			if (ret < 0)
 				return ret;
-			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L4_TCP :
-					       MLX5_FLOW_LAYER_OUTER_L4_TCP;
+			item_flags |= MLX5_FLOW_LAYER_GRE;
 			break;
 		case RTE_FLOW_ITEM_TYPE_VXLAN:
 			ret = mlx5_flow_validate_item_vxlan(items, item_flags,
@@ -260,21 +268,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 				return ret;
 			item_flags |= MLX5_FLOW_LAYER_VXLAN_GPE;
 			break;
-		case RTE_FLOW_ITEM_TYPE_GRE:
-			ret = mlx5_flow_validate_item_gre(items, item_flags,
-							  next_protocol, error);
-			if (ret < 0)
-				return ret;
-			item_flags |= MLX5_FLOW_LAYER_GRE;
-			break;
-		case RTE_FLOW_ITEM_TYPE_MPLS:
-			ret = mlx5_flow_validate_item_mpls(items, item_flags,
-							   next_protocol,
-							   error);
-			if (ret < 0)
-				return ret;
-			item_flags |= MLX5_FLOW_LAYER_MPLS;
-			break;
 		case RTE_FLOW_ITEM_TYPE_META:
 			ret = flow_dv_validate_item_meta(dev, items, attr,
 							 error);
@@ -982,9 +975,6 @@ flow_dv_create_item(void *matcher, void *key,
 	struct mlx5_flow_dv_matcher *tmatcher = matcher;
 
 	switch (item->type) {
-	case RTE_FLOW_ITEM_TYPE_VOID:
-	case RTE_FLOW_ITEM_TYPE_END:
-		break;
 	case RTE_FLOW_ITEM_TYPE_ETH:
 		flow_dv_translate_item_eth(tmatcher->mask.buf, key, item,
 					   inner);
@@ -1032,14 +1022,14 @@ flow_dv_create_item(void *matcher, void *key,
 						    (IBV_RX_HASH_SRC_PORT_UDP |
 						     IBV_RX_HASH_DST_PORT_UDP));
 		break;
-	case RTE_FLOW_ITEM_TYPE_NVGRE:
-		flow_dv_translate_item_nvgre(tmatcher->mask.buf, key, item,
-					     inner);
-		break;
 	case RTE_FLOW_ITEM_TYPE_GRE:
 		flow_dv_translate_item_gre(tmatcher->mask.buf, key, item,
 					   inner);
 		break;
+	case RTE_FLOW_ITEM_TYPE_NVGRE:
+		flow_dv_translate_item_nvgre(tmatcher->mask.buf, key, item,
+					     inner);
+		break;
 	case RTE_FLOW_ITEM_TYPE_VXLAN:
 	case RTE_FLOW_ITEM_TYPE_VXLAN_GPE:
 		flow_dv_translate_item_vxlan(tmatcher->mask.buf, key, item,
-- 
2.11.0

[dpdk-dev] [PATCH v2 4/5] net/mlx5: fix wildcard item for Direct Verbs

[Yongseok Koh]

If a network layer is specified with no spec, it means wildcard match.
flow_dv_translate_item_*() returns without writing anything if spec is
null and it causes creation of wrong flow. E.g., the following flow has to
patch with any ipv4 packet.

  flow create 0 ingress pattern eth / ipv4 / end actions ...

But, with the current code, it matches any packet because PMD doesn't write
anything about IPv4. The matcher value and mask becomes completely zero. It
should have written the IP version at least. It is same for the rest of
items.

Even if the spec is null, PMD has to write constant fields before return,
e.g. IP version and IP protocol number.

Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 55 ++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 15603401cf..57b01a78ee 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2,7 +2,6 @@
  * Copyright 2018 Mellanox Technologies, Ltd
  */
 
-
 #include <sys/queue.h>
 #include <stdalign.h>
 #include <stdint.h>
@@ -542,10 +541,6 @@ flow_dv_translate_item_ipv4(void *matcher, void *key,
 	char *l24_v;
 	uint8_t tos;
 
-	if (!ipv4_v)
-		return;
-	if (!ipv4_m)
-		ipv4_m = &nic_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -557,6 +552,10 @@ flow_dv_translate_item_ipv4(void *matcher, void *key,
 	}
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 4);
+	if (!ipv4_v)
+		return;
+	if (!ipv4_m)
+		ipv4_m = &nic_mask;
 	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
 			     dst_ipv4_dst_ipv6.ipv4_layout.ipv4);
 	l24_v = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v,
@@ -625,10 +624,6 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
 	int i;
 	int size;
 
-	if (!ipv6_v)
-		return;
-	if (!ipv6_m)
-		ipv6_m = &nic_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -638,6 +633,12 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
 					 outer_headers);
 		headers_v = MLX5_ADDR_OF(fte_match_param, key, outer_headers);
 	}
+	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
+	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
+	if (!ipv6_v)
+		return;
+	if (!ipv6_m)
+		ipv6_m = &nic_mask;
 	size = sizeof(ipv6_m->hdr.dst_addr);
 	l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
 			     dst_ipv4_dst_ipv6.ipv6_layout.ipv6);
@@ -653,8 +654,6 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
 	memcpy(l24_m, ipv6_m->hdr.src_addr, size);
 	for (i = 0; i < size; ++i)
 		l24_v[i] = l24_m[i] & ipv6_v->hdr.src_addr[i];
-	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
-	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
 	/* TOS. */
 	vtc_m = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow);
 	vtc_v = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow & ipv6_v->hdr.vtc_flow);
@@ -703,10 +702,6 @@ flow_dv_translate_item_tcp(void *matcher, void *key,
 	void *headers_m;
 	void *headers_v;
 
-	if (!tcp_v)
-		return;
-	if (!tcp_m)
-		tcp_m = &rte_flow_item_tcp_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -718,6 +713,10 @@ flow_dv_translate_item_tcp(void *matcher, void *key,
 	}
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol, IPPROTO_TCP);
+	if (!tcp_v)
+		return;
+	if (!tcp_m)
+		tcp_m = &rte_flow_item_tcp_mask;
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, tcp_sport,
 		 rte_be_to_cpu_16(tcp_m->hdr.src_port));
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, tcp_sport,
@@ -750,10 +749,6 @@ flow_dv_translate_item_udp(void *matcher, void *key,
 	void *headers_m;
 	void *headers_v;
 
-	if (!udp_v)
-		return;
-	if (!udp_m)
-		udp_m = &rte_flow_item_udp_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -765,6 +760,10 @@ flow_dv_translate_item_udp(void *matcher, void *key,
 	}
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol, IPPROTO_UDP);
+	if (!udp_v)
+		return;
+	if (!udp_m)
+		udp_m = &rte_flow_item_udp_mask;
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, udp_sport,
 		 rte_be_to_cpu_16(udp_m->hdr.src_port));
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, udp_sport,
@@ -799,10 +798,6 @@ flow_dv_translate_item_gre(void *matcher, void *key,
 	void *misc_m = MLX5_ADDR_OF(fte_match_param, matcher, misc_parameters);
 	void *misc_v = MLX5_ADDR_OF(fte_match_param, key, misc_parameters);
 
-	if (!gre_v)
-		return;
-	if (!gre_m)
-		gre_m = &rte_flow_item_gre_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -814,6 +809,10 @@ flow_dv_translate_item_gre(void *matcher, void *key,
 	}
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol, IPPROTO_GRE);
+	if (!gre_v)
+		return;
+	if (!gre_m)
+		gre_m = &rte_flow_item_gre_mask;
 	MLX5_SET(fte_match_set_misc, misc_m, gre_protocol,
 		 rte_be_to_cpu_16(gre_m->protocol));
 	MLX5_SET(fte_match_set_misc, misc_v, gre_protocol,
@@ -848,6 +847,7 @@ flow_dv_translate_item_nvgre(void *matcher, void *key,
 	int size;
 	int i;
 
+	flow_dv_translate_item_gre(matcher, key, item, inner);
 	if (!nvgre_v)
 		return;
 	if (!nvgre_m)
@@ -858,7 +858,6 @@ flow_dv_translate_item_nvgre(void *matcher, void *key,
 	memcpy(gre_key_m, tni_flow_id_m, size);
 	for (i = 0; i < size; ++i)
 		gre_key_v[i] = gre_key_m[i] & tni_flow_id_v[i];
-	flow_dv_translate_item_gre(matcher, key, item, inner);
 }
 
 /**
@@ -890,10 +889,6 @@ flow_dv_translate_item_vxlan(void *matcher, void *key,
 	int size;
 	int i;
 
-	if (!vxlan_v)
-		return;
-	if (!vxlan_m)
-		vxlan_m = &rte_flow_item_vxlan_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -909,6 +904,10 @@ flow_dv_translate_item_vxlan(void *matcher, void *key,
 		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);
 	}
+	if (!vxlan_v)
+		return;
+	if (!vxlan_m)
+		vxlan_m = &rte_flow_item_vxlan_mask;
 	size = sizeof(vxlan_m->vni);
 	vni_m = MLX5_ADDR_OF(fte_match_set_misc, misc_m, vxlan_vni);
 	vni_v = MLX5_ADDR_OF(fte_match_set_misc, misc_v, vxlan_vni);
-- 
2.11.0

[dpdk-dev] [PATCH v2 5/5] net/mlx5: fix flow mark ID conversion in Direct Verbs

[Yongseok Koh]

Fixes: d02cb0691299 ("net/mlx5: add Direct Verbs translate actions")
Cc: orika@mellanox.com

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 57b01a78ee..071f31d0fd 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1065,15 +1065,16 @@ flow_dv_create_action(const struct rte_flow_action *action,
 	case RTE_FLOW_ACTION_TYPE_FLAG:
 		dev_flow->dv.actions[actions_n].type = MLX5DV_FLOW_ACTION_TAG;
 		dev_flow->dv.actions[actions_n].tag_value =
-			MLX5_FLOW_MARK_DEFAULT;
+			mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT);
 		actions_n++;
 		flow->actions |= MLX5_FLOW_ACTION_FLAG;
 		break;
 	case RTE_FLOW_ACTION_TYPE_MARK:
 		dev_flow->dv.actions[actions_n].type = MLX5DV_FLOW_ACTION_TAG;
 		dev_flow->dv.actions[actions_n].tag_value =
-			((const struct rte_flow_action_mark *)
-			 (action->conf))->id;
+			mlx5_flow_mark_set
+			(((const struct rte_flow_action_mark *)
+			  (action->conf))->id);
 		flow->actions |= MLX5_FLOW_ACTION_MARK;
 		actions_n++;
 		break;
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2 0/5] net/mlx5: fixes for Direct Verbs flow

[Shahaf Shuler]

Tuesday, October 23, 2018 7:52 PM, Yongseok Koh:
> Subject: [PATCH v2 0/5] net/mlx5: fixes for Direct Verbs flow
> 
> v2:
> * rebase on top of the new branch tip
> 
> 
> Yongseok Koh (5):
>   net/mlx5: add warning message for Direct Verbs flow
>   net/mlx5: fix UDP hash field flag in Direct Verbs
>   net/mlx5: fix item validation in Direct Verbs
>   net/mlx5: fix wildcard item for Direct Verbs
>   net/mlx5: fix flow mark ID conversion in Direct Verbs
> 
>  drivers/net/mlx5/mlx5.c         |   6 ++
>  drivers/net/mlx5/mlx5_flow.c    |  20 +++----
>  drivers/net/mlx5/mlx5_flow_dv.c | 118 ++++++++++++++++++---------------
> -------
>  3 files changed, 67 insertions(+), 77 deletions(-)

Applied to next-net-mlx, thanks. 

> 
> --
> 2.11.0