DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch
@ 2018-12-27 15:34 Viacheslav Ovsiienko
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 1/5] net/mlx5: remove checks for outer tunnel items " Viacheslav Ovsiienko
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Viacheslav Ovsiienko @ 2018-12-27 15:34 UTC (permalink / raw)
  To: shahafs; +Cc: dev, stable

The generic Flow rule for tunnels looks like:

flow create <attributes> <port> \
            <tunnel outer items pattern> \
            <tunnel vni item> \
            <tunnel inner items pattern>
			    
Current design supports only L2 addresses as inner pattern
items. This patchset adds support for L3 (IPv4/IPv6) addresses
and L4 (TCP/UDP) ports items as inner tunnel parameters.
	
Also this patchset adds support for inner and outer ethernet
types for the E-Switch Flows with tunnels. Inner and outer ethernet
type match  can be specified with ethernet items, vlan items, or
implicitly deduced from IP address items. The tcm_info field 
in Netlink message tcm structure is filled always with outer
protocol.
	 
Cc: stable@dpdk.org
	
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

Viacheslav Ovsiienko (5):
  net/mlx5: remove checks for outer tunnel items on E-Switch
  net/mlx5: add tunnel inner items validation on E-Switch
  net/mlx5: add tunnel inner items support on E-Switch
  net/mlx5: add ethernet type validation on E-Switch
  net/mlx5: add ethernet type support for tunnels on E-Switch

 drivers/net/mlx5/mlx5_flow_tcf.c | 690 ++++++++++++++++++++++-----------------
 1 file changed, 399 insertions(+), 291 deletions(-)

-- 
1.8.3.1

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

* [dpdk-dev] [PATCH 1/5] net/mlx5: remove checks for outer tunnel items on E-Switch
  2018-12-27 15:34 [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch Viacheslav Ovsiienko
@ 2018-12-27 15:34 ` Viacheslav Ovsiienko
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 2/5] net/mlx5: add tunnel inner items validation " Viacheslav Ovsiienko
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Viacheslav Ovsiienko @ 2018-12-27 15:34 UTC (permalink / raw)
  To: shahafs; +Cc: dev, stable

This patch removes unnecessary outer tunnel parameters check in the
validation routine for the E-Switch Flows. IPv4/IPv6 may have any
spec and mask, and transferred to tc without changes, all checks
are performed by kernel.

We are going to support Flows matching with outer tunnel items
and not containing the explicit tunnel decap action (this one
might be drop, redirect or table jump, for exapmle). So we can
not rely on presence of tunnel decap action in the list to decide
whether the Flow is for tunnel, instead we will use the presence
of tunnel item (like RTE_FLOW_ITEM_TYPE_VXLAN) in the item list.
The tunnel pattern checks within Flow validation routine are
rebound to presence of tunnel item. VXLAN decap action checks
for presence of VXLAN VNI item.

The tunnel UDP item is checked at the point of processing the tunnel
item (i.e. VXLAN). We can not perform UDP item check as tunnel once
UDP item encountered in the list, because it is not known yet whether
the tunnel item follows. The pointer to UDP item is saved and
checked as outer ones if tunnel item found.

Cc: stable@dpdk.org

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_tcf.c | 239 +++++++++++----------------------------
 1 file changed, 63 insertions(+), 176 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index fb284c3..e59e638 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -1584,141 +1584,8 @@ struct pedit_parser {
 }
 
 /**
- * Validate RTE_FLOW_ITEM_TYPE_IPV4 item if VXLAN_DECAP action
- * is present in actions list.
- *
- * @param[in] ipv4
- *   Outer IPv4 address item (if any, NULL otherwise).
- * @param[out] error
- *   Pointer to the error structure.
- *
- * @return
- *   0 on success, a negative errno value otherwise and rte_ernno is set.
- **/
-static int
-flow_tcf_validate_vxlan_decap_ipv4(const struct rte_flow_item *ipv4,
-				   struct rte_flow_error *error)
-{
-	const struct rte_flow_item_ipv4 *spec = ipv4->spec;
-	const struct rte_flow_item_ipv4 *mask = ipv4->mask;
-
-	if (!spec) {
-		/*
-		 * Specification for IP addresses cannot be empty
-		 * because it is required as decap parameter.
-		 */
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ITEM, ipv4,
-					  "NULL outer ipv4 address"
-					  " specification for vxlan"
-					  " for vxlan decapsulation");
-	}
-	if (!mask)
-		mask = &rte_flow_item_ipv4_mask;
-	if (mask->hdr.dst_addr != RTE_BE32(0x00000000)) {
-		if (mask->hdr.dst_addr != RTE_BE32(0xffffffff))
-			return rte_flow_error_set
-					(error, ENOTSUP,
-					 RTE_FLOW_ERROR_TYPE_ITEM_MASK, mask,
-					 "no support for partial mask on"
-					 " \"ipv4.hdr.dst_addr\" field");
-		/* More IP address validations can be put here. */
-	} else {
-		/*
-		 * Kernel uses the destination IP address
-		 * to determine the ingress network interface
-		 * for traffic being decapsulated.
-		 */
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ITEM, ipv4,
-					  "outer ipv4 destination address"
-					  " must be specified for"
-					  " vxlan decapsulation");
-	}
-	/* Source IP address is optional for decap. */
-	if (mask->hdr.src_addr != RTE_BE32(0x00000000) &&
-	    mask->hdr.src_addr != RTE_BE32(0xffffffff))
-		return rte_flow_error_set(error, ENOTSUP,
-					  RTE_FLOW_ERROR_TYPE_ITEM_MASK, mask,
-					  "no support for partial mask on"
-					  " \"ipv4.hdr.src_addr\" field");
-	return 0;
-}
-
-/**
- * Validate RTE_FLOW_ITEM_TYPE_IPV6 item if VXLAN_DECAP action
- * is present in actions list.
- *
- * @param[in] ipv6
- *   Outer IPv6 address item (if any, NULL otherwise).
- * @param[out] error
- *   Pointer to the error structure.
- *
- * @return
- *   0 on success, a negative errno value otherwise and rte_ernno is set.
- **/
-static int
-flow_tcf_validate_vxlan_decap_ipv6(const struct rte_flow_item *ipv6,
-				   struct rte_flow_error *error)
-{
-	const struct rte_flow_item_ipv6 *spec = ipv6->spec;
-	const struct rte_flow_item_ipv6 *mask = ipv6->mask;
-
-	if (!spec) {
-		/*
-		 * Specification for IP addresses cannot be empty
-		 * because it is required as decap parameter.
-		 */
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ITEM, ipv6,
-					  "NULL outer ipv6 address"
-					  " specification for vxlan"
-					  " decapsulation");
-	}
-	if (!mask)
-		mask = &rte_flow_item_ipv6_mask;
-	if (memcmp(&mask->hdr.dst_addr,
-		   &flow_tcf_mask_empty.ipv6.hdr.dst_addr,
-		   IPV6_ADDR_LEN)) {
-		if (memcmp(&mask->hdr.dst_addr,
-			&rte_flow_item_ipv6_mask.hdr.dst_addr,
-			IPV6_ADDR_LEN))
-			return rte_flow_error_set
-					(error, ENOTSUP,
-					 RTE_FLOW_ERROR_TYPE_ITEM_MASK, mask,
-					 "no support for partial mask on"
-					 " \"ipv6.hdr.dst_addr\" field");
-		/* More IP address validations can be put here. */
-	} else {
-		/*
-		 * Kernel uses the destination IP address
-		 * to determine the ingress network interface
-		 * for traffic being decapsulated.
-		 */
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ITEM, ipv6,
-					  "outer ipv6 destination address must be "
-					  "specified for vxlan decapsulation");
-	}
-	/* Source IP address is optional for decap. */
-	if (memcmp(&mask->hdr.src_addr,
-		   &flow_tcf_mask_empty.ipv6.hdr.src_addr,
-		   IPV6_ADDR_LEN)) {
-		if (memcmp(&mask->hdr.src_addr,
-			   &rte_flow_item_ipv6_mask.hdr.src_addr,
-			   IPV6_ADDR_LEN))
-			return rte_flow_error_set
-					(error, ENOTSUP,
-					 RTE_FLOW_ERROR_TYPE_ITEM_MASK, mask,
-					 "no support for partial mask on"
-					 " \"ipv6.hdr.src_addr\" field");
-	}
-	return 0;
-}
-
-/**
- * Validate RTE_FLOW_ITEM_TYPE_UDP item if VXLAN_DECAP action
- * is present in actions list.
+ * Validate outer RTE_FLOW_ITEM_TYPE_UDP item if tunnel item
+ * RTE_FLOW_ITEM_TYPE_VXLAN is present in item list.
  *
  * @param[in] udp
  *   Outer UDP layer item (if any, NULL otherwise).
@@ -1726,7 +1593,7 @@ struct pedit_parser {
  *   Pointer to the error structure.
  *
  * @return
- *   0 on success, a negative errno value otherwise and rte_ernno is set.
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
  **/
 static int
 flow_tcf_validate_vxlan_decap_udp(const struct rte_flow_item *udp,
@@ -1825,6 +1692,7 @@ struct pedit_parser {
 		const struct rte_flow_action_set_ipv4 *set_ipv4;
 		const struct rte_flow_action_set_ipv6 *set_ipv6;
 	} conf;
+	const struct rte_flow_item *outer_udp = NULL;
 	uint64_t item_flags = 0;
 	uint64_t action_flags = 0;
 	uint8_t next_protocol = -1;
@@ -2151,12 +2019,6 @@ struct pedit_parser {
 				next_protocol =
 					((const struct rte_flow_item_ipv4 *)
 					 (items->spec))->hdr.next_proto_id;
-			if (action_flags & MLX5_FLOW_ACTION_VXLAN_DECAP) {
-				ret = flow_tcf_validate_vxlan_decap_ipv4
-								(items, error);
-				if (ret < 0)
-					return ret;
-			}
 			break;
 		case RTE_FLOW_ITEM_TYPE_IPV6:
 			ret = mlx5_flow_validate_item_ipv6(items, item_flags,
@@ -2184,12 +2046,6 @@ struct pedit_parser {
 				next_protocol =
 					((const struct rte_flow_item_ipv6 *)
 					 (items->spec))->hdr.proto;
-			if (action_flags & MLX5_FLOW_ACTION_VXLAN_DECAP) {
-				ret = flow_tcf_validate_vxlan_decap_ipv6
-								(items, error);
-				if (ret < 0)
-					return ret;
-			}
 			break;
 		case RTE_FLOW_ITEM_TYPE_UDP:
 			ret = mlx5_flow_validate_item_udp(items, item_flags,
@@ -2205,12 +2061,12 @@ struct pedit_parser {
 				 error);
 			if (!mask.udp)
 				return -rte_errno;
-			if (action_flags & MLX5_FLOW_ACTION_VXLAN_DECAP) {
-				ret = flow_tcf_validate_vxlan_decap_udp
-								(items, error);
-				if (ret < 0)
-					return ret;
-			}
+			/*
+			 * Save the presumed outer UDP item for extra check
+			 * if the tunnel item will be found later in the list.
+			 */
+			if (!(item_flags & MLX5_FLOW_LAYER_TUNNEL))
+				outer_udp = items;
 			break;
 		case RTE_FLOW_ITEM_TYPE_TCP:
 			ret = mlx5_flow_validate_item_tcp
@@ -2259,6 +2115,45 @@ struct pedit_parser {
 					 mask.vxlan,
 					 "no support for partial or "
 					 "empty mask on \"vxlan.vni\" field");
+			/*
+			 * The VNI item assumes the VXLAN tunnel, it requires
+			 * at least the outer destination UDP port must be
+			 * specified without wildcards to allow kernel select
+			 * the virtual VXLAN device by port. Also outer IPv4
+			 * or IPv6 item must be specified (wilcards or even
+			 * zero mask are allowed) to let driver know the tunnel
+			 * IP version and process UDP traffic correctly.
+			 */
+			if (!(item_flags &
+			     (MLX5_FLOW_LAYER_OUTER_L3_IPV4 |
+			      MLX5_FLOW_LAYER_OUTER_L3_IPV6)))
+				return rte_flow_error_set
+						 (error, EINVAL,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "no outer IP pattern found"
+						  " for vxlan tunnel");
+			if (!(item_flags & MLX5_FLOW_LAYER_OUTER_L4_UDP))
+				return rte_flow_error_set
+						 (error, EINVAL,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "no outer UDP pattern found"
+						  " for vxlan tunnel");
+			/*
+			 * All items preceding the tunnel item become outer
+			 * ones and we should do extra validation for them
+			 * due to tc limitations for tunnel outer parameters.
+			 * Currently only outer UDP item requres extra check,
+			 * use the saved pointer instead of item list rescan.
+			 */
+			assert(outer_udp);
+			ret = flow_tcf_validate_vxlan_decap_udp
+						(outer_udp, error);
+			if (ret < 0)
+				return ret;
+			/* Reset L4 protocol for inner parameters. */
+			next_protocol = 0xff;
 			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
@@ -2361,28 +2256,20 @@ struct pedit_parser {
 						  "no ethernet found in"
 						  " pattern");
 	}
-	if (action_flags & MLX5_FLOW_ACTION_VXLAN_DECAP) {
-		if (!(item_flags &
-		     (MLX5_FLOW_LAYER_OUTER_L3_IPV4 |
-		      MLX5_FLOW_LAYER_OUTER_L3_IPV6)))
-			return rte_flow_error_set(error, EINVAL,
-						  RTE_FLOW_ERROR_TYPE_ACTION,
-						  NULL,
-						  "no outer IP pattern found"
-						  " for vxlan decap action");
-		if (!(item_flags & MLX5_FLOW_LAYER_OUTER_L4_UDP))
-			return rte_flow_error_set(error, EINVAL,
-						  RTE_FLOW_ERROR_TYPE_ACTION,
-						  NULL,
-						  "no outer UDP pattern found"
-						  " for vxlan decap action");
-		if (!(item_flags & MLX5_FLOW_LAYER_VXLAN))
-			return rte_flow_error_set(error, EINVAL,
-						  RTE_FLOW_ERROR_TYPE_ACTION,
-						  NULL,
-						  "no VNI pattern found"
-						  " for vxlan decap action");
-	}
+	if ((action_flags & MLX5_FLOW_ACTION_VXLAN_DECAP) &&
+	    !(item_flags & MLX5_FLOW_LAYER_VXLAN))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION,
+					  NULL,
+					  "no VNI pattern found"
+					  " for vxlan decap action");
+	if ((action_flags & MLX5_FLOW_ACTION_VXLAN_ENCAP) &&
+	    (item_flags & MLX5_FLOW_LAYER_TUNNEL))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION,
+					  NULL,
+					  "vxlan encap not supported"
+					  " for tunneled traffic");
 	return 0;
 }
 
-- 
1.8.3.1

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

* [dpdk-dev] [PATCH 2/5] net/mlx5: add tunnel inner items validation on E-Switch
  2018-12-27 15:34 [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch Viacheslav Ovsiienko
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 1/5] net/mlx5: remove checks for outer tunnel items " Viacheslav Ovsiienko
@ 2018-12-27 15:34 ` Viacheslav Ovsiienko
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 3/5] net/mlx5: add tunnel inner items support " Viacheslav Ovsiienko
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Viacheslav Ovsiienko @ 2018-12-27 15:34 UTC (permalink / raw)
  To: shahafs; +Cc: dev, stable

This patch updates the validation routine for the E-Switch Flows.
The inner/outer item flags are added and set correctly, the
validation routine will accept and check the inner items
which follow the tunnel item (like VNI).

Cc: stable@dpdk.org

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_tcf.c | 48 +++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index e59e638..5fc50c2 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -1879,17 +1879,16 @@ struct pedit_parser {
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
 		unsigned int i;
 
-		if ((item_flags & MLX5_FLOW_LAYER_TUNNEL) &&
-		    items->type != RTE_FLOW_ITEM_TYPE_ETH)
-			return rte_flow_error_set(error, ENOTSUP,
-						  RTE_FLOW_ERROR_TYPE_ITEM,
-						  items,
-						  "only L2 inner item"
-						  " is supported");
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
 			break;
 		case RTE_FLOW_ITEM_TYPE_PORT_ID:
+			if (item_flags & MLX5_FLOW_LAYER_TUNNEL)
+				return rte_flow_error_set
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_ITEM, items,
+					 "inner tunnel port id"
+					 " item is not supported");
 			mask.port_id = flow_tcf_item_mask
 				(items, &rte_flow_item_port_id_mask,
 				 &flow_tcf_mask_supported.port_id,
@@ -1940,8 +1939,8 @@ struct pedit_parser {
 			if (ret < 0)
 				return ret;
 			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
-					MLX5_FLOW_LAYER_INNER_L2 :
-					MLX5_FLOW_LAYER_OUTER_L2;
+				      MLX5_FLOW_LAYER_INNER_L2 :
+				      MLX5_FLOW_LAYER_OUTER_L2;
 			/* TODO:
 			 * Redundant check due to different supported mask.
 			 * Same for the rest of items.
@@ -1964,6 +1963,12 @@ struct pedit_parser {
 					 " \"type\" field");
 			break;
 		case RTE_FLOW_ITEM_TYPE_VLAN:
+			if (item_flags & MLX5_FLOW_LAYER_TUNNEL)
+				return rte_flow_error_set
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_ITEM, items,
+					 "inner tunnel VLAN"
+					 " is not supported");
 			ret = mlx5_flow_validate_item_vlan(items, item_flags,
 							   error);
 			if (ret < 0)
@@ -1998,7 +2003,9 @@ struct pedit_parser {
 							   error);
 			if (ret < 0)
 				return ret;
-			item_flags |= MLX5_FLOW_LAYER_OUTER_L3_IPV4;
+			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
+				      MLX5_FLOW_LAYER_INNER_L3_IPV4 :
+				      MLX5_FLOW_LAYER_OUTER_L3_IPV4;
 			mask.ipv4 = flow_tcf_item_mask
 				(items, &rte_flow_item_ipv4_mask,
 				 &flow_tcf_mask_supported.ipv4,
@@ -2025,7 +2032,9 @@ struct pedit_parser {
 							   error);
 			if (ret < 0)
 				return ret;
-			item_flags |= MLX5_FLOW_LAYER_OUTER_L3_IPV6;
+			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
+				      MLX5_FLOW_LAYER_INNER_L3_IPV6 :
+				      MLX5_FLOW_LAYER_OUTER_L3_IPV6;
 			mask.ipv6 = flow_tcf_item_mask
 				(items, &rte_flow_item_ipv6_mask,
 				 &flow_tcf_mask_supported.ipv6,
@@ -2052,7 +2061,9 @@ struct pedit_parser {
 							  next_protocol, error);
 			if (ret < 0)
 				return ret;
-			item_flags |= MLX5_FLOW_LAYER_OUTER_L4_UDP;
+			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
+				      MLX5_FLOW_LAYER_INNER_L4_UDP :
+				      MLX5_FLOW_LAYER_OUTER_L4_UDP;
 			mask.udp = flow_tcf_item_mask
 				(items, &rte_flow_item_udp_mask,
 				 &flow_tcf_mask_supported.udp,
@@ -2076,7 +2087,9 @@ struct pedit_parser {
 					      error);
 			if (ret < 0)
 				return ret;
-			item_flags |= MLX5_FLOW_LAYER_OUTER_L4_TCP;
+			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
+				      MLX5_FLOW_LAYER_INNER_L4_TCP :
+				      MLX5_FLOW_LAYER_OUTER_L4_TCP;
 			mask.tcp = flow_tcf_item_mask
 				(items, &rte_flow_item_tcp_mask,
 				 &flow_tcf_mask_supported.tcp,
@@ -2087,13 +2100,12 @@ struct pedit_parser {
 				return -rte_errno;
 			break;
 		case RTE_FLOW_ITEM_TYPE_VXLAN:
-			if (!(action_flags & MLX5_FLOW_ACTION_VXLAN_DECAP))
+			if (item_flags & MLX5_FLOW_LAYER_OUTER_VLAN)
 				return rte_flow_error_set
 					(error, ENOTSUP,
-					 RTE_FLOW_ERROR_TYPE_ITEM,
-					 items,
-					 "vni pattern should be followed by"
-					 " vxlan decapsulation action");
+					 RTE_FLOW_ERROR_TYPE_ITEM, items,
+					 "vxlan tunnel over vlan"
+					 " is not supported");
 			ret = mlx5_flow_validate_item_vxlan(items,
 							    item_flags, error);
 			if (ret < 0)
-- 
1.8.3.1

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

* [dpdk-dev] [PATCH 3/5] net/mlx5: add tunnel inner items support on E-Switch
  2018-12-27 15:34 [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch Viacheslav Ovsiienko
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 1/5] net/mlx5: remove checks for outer tunnel items " Viacheslav Ovsiienko
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 2/5] net/mlx5: add tunnel inner items validation " Viacheslav Ovsiienko
@ 2018-12-27 15:34 ` Viacheslav Ovsiienko
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 4/5] net/mlx5: add ethernet type validation " Viacheslav Ovsiienko
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Viacheslav Ovsiienko @ 2018-12-27 15:34 UTC (permalink / raw)
  To: shahafs; +Cc: dev, stable

This patch updates the translation routine for the E-Switch Flows.
Inner tunnel pattern items are translated into Netlink message,
support for tunnel inner IP addresses (v4 or v6), IP protocol,
and TCP and UDP ports is added.

We are going to support Flows matching with outer tunnel items
and not containing the explicit tunnel decap action (this one
might be drop, redirect or table jump, for exapmle).
So we can not rely on presence of tunnel decap action in the
list to decide whether the Flow is for tunnel, instead we will
use the presence of tunnel item. Item translation is rebound
to presence of tunnel items, instead of relying on decap action.

There is no way to tell kernel driver the outer address type
(IPv4 or IPv6) but specify the address flower key. The outer
address key is put on Netlink with zero mask if there is no
RTE item is specified in the list.

Cc: stable@dpdk.org

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_tcf.c | 174 ++++++++++++++++++++++++++++-----------
 1 file changed, 125 insertions(+), 49 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index 5fc50c2..688422d 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -463,7 +463,9 @@ struct flow_tcf_stats_basic {
 	struct rte_flow_item_tcp tcp;
 	struct rte_flow_item_udp udp;
 	struct rte_flow_item_vxlan vxlan;
-} flow_tcf_mask_empty;
+} flow_tcf_mask_empty = {
+	{0},
+};
 
 /** Supported masks for known item types. */
 static const struct {
@@ -2292,13 +2294,16 @@ struct pedit_parser {
  *   Pointer to the flow attributes.
  * @param[in] items
  *   Pointer to the list of items.
+ * @param[out] action_flags
+ *   Pointer to the detected actions.
  *
  * @return
  *   Maximum size of memory for items.
  */
 static int
 flow_tcf_get_items_size(const struct rte_flow_attr *attr,
-			const struct rte_flow_item items[])
+			const struct rte_flow_item items[],
+			uint64_t *action_flags)
 {
 	int size = 0;
 
@@ -2349,6 +2354,16 @@ struct pedit_parser {
 			break;
 		case RTE_FLOW_ITEM_TYPE_VXLAN:
 			size += SZ_NLATTR_TYPE_OF(uint32_t);
+			/*
+			 * There might be no VXLAN decap action in the action
+			 * list, nonetheless the VXLAN tunnel flow requires
+			 * the decap structure to be correctly applied to
+			 * VXLAN device, set the flag to create the structure.
+			 * Translation routine will not put the decap action
+			 * in tne Netlink message if there is no actual action
+			 * in the list.
+			 */
+			*action_flags |= MLX5_FLOW_ACTION_VXLAN_DECAP;
 			break;
 		default:
 			DRV_LOG(WARNING,
@@ -2597,7 +2612,7 @@ struct pedit_parser {
 	struct tcmsg *tcm;
 	uint8_t *sp, *tun = NULL;
 
-	size += flow_tcf_get_items_size(attr, items);
+	size += flow_tcf_get_items_size(attr, items, &action_flags);
 	size += flow_tcf_get_actions_and_size(actions, &action_flags);
 	dev_flow = rte_zmalloc(__func__, size, MNL_ALIGNTO);
 	if (!dev_flow) {
@@ -3001,6 +3016,7 @@ struct pedit_parser {
 	bool vlan_present = 0;
 	bool vlan_eth_type_set = 0;
 	bool ip_proto_set = 0;
+	bool tunnel_outer = 0;
 	struct nlattr *na_flower;
 	struct nlattr *na_flower_act;
 	struct nlattr *na_vlan_id = NULL;
@@ -3014,6 +3030,7 @@ struct pedit_parser {
 		switch (dev_flow->tcf.tunnel->type) {
 		case FLOW_TCF_TUNACT_VXLAN_DECAP:
 			decap.vxlan = dev_flow->tcf.vxlan_decap;
+			tunnel_outer = 1;
 			break;
 		case FLOW_TCF_TUNACT_VXLAN_ENCAP:
 			encap.vxlan = dev_flow->tcf.vxlan_encap;
@@ -3068,7 +3085,7 @@ struct pedit_parser {
 			tcm->tcm_ifindex = ptoi[i].ifindex;
 			break;
 		case RTE_FLOW_ITEM_TYPE_ETH:
-			item_flags |= (item_flags & MLX5_FLOW_LAYER_VXLAN) ?
+			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
 				      MLX5_FLOW_LAYER_INNER_L2 :
 				      MLX5_FLOW_LAYER_OUTER_L2;
 			mask.eth = flow_tcf_item_mask
@@ -3081,12 +3098,11 @@ struct pedit_parser {
 			if (mask.eth == &flow_tcf_mask_empty.eth)
 				break;
 			spec.eth = items->spec;
-			if (decap.vxlan &&
-			    !(item_flags & MLX5_FLOW_LAYER_VXLAN)) {
+			if (tunnel_outer) {
 				DRV_LOG(WARNING,
-					"outer L2 addresses cannot be forced"
-					" for vxlan decapsulation, parameter"
-					" ignored");
+					"outer L2 addresses cannot be"
+					" forced is outer ones for tunnel,"
+					" parameter is ignored");
 				break;
 			}
 			if (mask.eth->type) {
@@ -3115,6 +3131,7 @@ struct pedit_parser {
 		case RTE_FLOW_ITEM_TYPE_VLAN:
 			assert(!encap.hdr);
 			assert(!decap.hdr);
+			assert(!tunnel_outer);
 			item_flags |= MLX5_FLOW_LAYER_OUTER_VLAN;
 			mask.vlan = flow_tcf_item_mask
 				(items, &rte_flow_item_vlan_mask,
@@ -3149,7 +3166,9 @@ struct pedit_parser {
 			assert(dev_flow->tcf.nlsize >= nlh->nlmsg_len);
 			break;
 		case RTE_FLOW_ITEM_TYPE_IPV4:
-			item_flags |= MLX5_FLOW_LAYER_OUTER_L3_IPV4;
+			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
+				      MLX5_FLOW_LAYER_INNER_L3_IPV4 :
+				      MLX5_FLOW_LAYER_OUTER_L3_IPV4;
 			mask.ipv4 = flow_tcf_item_mask
 				(items, &rte_flow_item_ipv4_mask,
 				 &flow_tcf_mask_supported.ipv4,
@@ -3158,7 +3177,7 @@ struct pedit_parser {
 				 error);
 			assert(mask.ipv4);
 			spec.ipv4 = items->spec;
-			if (!decap.vxlan) {
+			if (!tunnel_outer) {
 				if (!eth_type_set ||
 				    (!vlan_eth_type_set && vlan_present))
 					mnl_attr_put_u16
@@ -3169,45 +3188,70 @@ struct pedit_parser {
 						 RTE_BE16(ETH_P_IP));
 				eth_type_set = 1;
 				vlan_eth_type_set = 1;
-				if (mask.ipv4 == &flow_tcf_mask_empty.ipv4)
+			}
+			if (!tunnel_outer && mask.ipv4->hdr.next_proto_id) {
+				/*
+				 * No way to set IP protocol for outer tunnel
+				 * layers. Usually it is fixed, for example,
+				 * to UDP for VXLAN/GPE.
+				 */
+				assert(spec.ipv4); /* Mask is not empty. */
+				mnl_attr_put_u8(nlh, TCA_FLOWER_KEY_IP_PROTO,
+						spec.ipv4->hdr.next_proto_id);
+				ip_proto_set = 1;
+			}
+			if (mask.ipv4 == &flow_tcf_mask_empty.ipv4 ||
+			     (!mask.ipv4->hdr.src_addr &&
+			      !mask.ipv4->hdr.dst_addr)) {
+				if (!tunnel_outer)
 					break;
-				if (mask.ipv4->hdr.next_proto_id) {
-					mnl_attr_put_u8
-						(nlh, TCA_FLOWER_KEY_IP_PROTO,
-						 spec.ipv4->hdr.next_proto_id);
-					ip_proto_set = 1;
-				}
-			} else {
-				assert(mask.ipv4 != &flow_tcf_mask_empty.ipv4);
+				/*
+				 * For tunnel outer we must set outer IP key
+				 * anyway, even if the specification/mask is
+				 * empty. There is no another way to tell
+				 * kernel about he outer layer protocol.
+				 */
+				mnl_attr_put_u32
+					(nlh, TCA_FLOWER_KEY_ENC_IPV4_SRC,
+					 mask.ipv4->hdr.src_addr);
+				mnl_attr_put_u32
+					(nlh, TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK,
+					 mask.ipv4->hdr.src_addr);
+				assert(dev_flow->tcf.nlsize >= nlh->nlmsg_len);
+				break;
 			}
 			if (mask.ipv4->hdr.src_addr) {
 				mnl_attr_put_u32
-					(nlh, decap.vxlan ?
+					(nlh, tunnel_outer ?
 					 TCA_FLOWER_KEY_ENC_IPV4_SRC :
 					 TCA_FLOWER_KEY_IPV4_SRC,
 					 spec.ipv4->hdr.src_addr);
 				mnl_attr_put_u32
-					(nlh, decap.vxlan ?
+					(nlh, tunnel_outer ?
 					 TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK :
 					 TCA_FLOWER_KEY_IPV4_SRC_MASK,
 					 mask.ipv4->hdr.src_addr);
 			}
 			if (mask.ipv4->hdr.dst_addr) {
 				mnl_attr_put_u32
-					(nlh, decap.vxlan ?
+					(nlh, tunnel_outer ?
 					 TCA_FLOWER_KEY_ENC_IPV4_DST :
 					 TCA_FLOWER_KEY_IPV4_DST,
 					 spec.ipv4->hdr.dst_addr);
 				mnl_attr_put_u32
-					(nlh, decap.vxlan ?
+					(nlh, tunnel_outer ?
 					 TCA_FLOWER_KEY_ENC_IPV4_DST_MASK :
 					 TCA_FLOWER_KEY_IPV4_DST_MASK,
 					 mask.ipv4->hdr.dst_addr);
 			}
 			assert(dev_flow->tcf.nlsize >= nlh->nlmsg_len);
 			break;
-		case RTE_FLOW_ITEM_TYPE_IPV6:
-			item_flags |= MLX5_FLOW_LAYER_OUTER_L3_IPV6;
+		case RTE_FLOW_ITEM_TYPE_IPV6: {
+			bool ipv6_src, ipv6_dst;
+
+			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
+				      MLX5_FLOW_LAYER_INNER_L3_IPV6 :
+				      MLX5_FLOW_LAYER_OUTER_L3_IPV6;
 			mask.ipv6 = flow_tcf_item_mask
 				(items, &rte_flow_item_ipv6_mask,
 				 &flow_tcf_mask_supported.ipv6,
@@ -3216,7 +3260,7 @@ struct pedit_parser {
 				 error);
 			assert(mask.ipv6);
 			spec.ipv6 = items->spec;
-			if (!decap.vxlan) {
+			if (!tunnel_outer) {
 				if (!eth_type_set ||
 				    (!vlan_eth_type_set && vlan_present))
 					mnl_attr_put_u16
@@ -3227,36 +3271,62 @@ struct pedit_parser {
 						 RTE_BE16(ETH_P_IPV6));
 				eth_type_set = 1;
 				vlan_eth_type_set = 1;
-				if (mask.ipv6 == &flow_tcf_mask_empty.ipv6)
+			}
+			if (!tunnel_outer && mask.ipv6->hdr.proto) {
+				/*
+				 * No way to set IP protocol for outer tunnel
+				 * layers. Usually it is fixed, for example,
+				 * to UDP for VXLAN/GPE.
+				 */
+				assert(spec.ipv6); /* Mask is not empty. */
+				mnl_attr_put_u8(nlh, TCA_FLOWER_KEY_IP_PROTO,
+						spec.ipv6->hdr.proto);
+				ip_proto_set = 1;
+			}
+			ipv6_dst = !IN6_IS_ADDR_UNSPECIFIED
+						(mask.ipv6->hdr.dst_addr);
+			ipv6_src = !IN6_IS_ADDR_UNSPECIFIED
+						(mask.ipv6->hdr.src_addr);
+			if (mask.ipv6 == &flow_tcf_mask_empty.ipv6 ||
+			     (!ipv6_dst && !ipv6_src)) {
+				if (!tunnel_outer)
 					break;
-				if (mask.ipv6->hdr.proto) {
-					mnl_attr_put_u8
-						(nlh, TCA_FLOWER_KEY_IP_PROTO,
-						 spec.ipv6->hdr.proto);
-					ip_proto_set = 1;
-				}
-			} else {
-				assert(mask.ipv6 != &flow_tcf_mask_empty.ipv6);
+				/*
+				 * For tunnel outer we must set outer IP key
+				 * anyway, even if the specification/mask is
+				 * empty. There is no another way to tell
+				 * kernel about he outer layer protocol.
+				 */
+				mnl_attr_put(nlh,
+					     TCA_FLOWER_KEY_ENC_IPV6_SRC,
+					     IPV6_ADDR_LEN,
+					     mask.ipv6->hdr.src_addr);
+				mnl_attr_put(nlh,
+					     TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK,
+					     IPV6_ADDR_LEN,
+					     mask.ipv6->hdr.src_addr);
+				assert(dev_flow->tcf.nlsize >= nlh->nlmsg_len);
+				break;
 			}
-			if (!IN6_IS_ADDR_UNSPECIFIED(mask.ipv6->hdr.src_addr)) {
-				mnl_attr_put(nlh, decap.vxlan ?
+			if (ipv6_src) {
+				mnl_attr_put(nlh, tunnel_outer ?
 					     TCA_FLOWER_KEY_ENC_IPV6_SRC :
 					     TCA_FLOWER_KEY_IPV6_SRC,
 					     IPV6_ADDR_LEN,
 					     spec.ipv6->hdr.src_addr);
-				mnl_attr_put(nlh, decap.vxlan ?
+				mnl_attr_put(nlh, tunnel_outer ?
 					     TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK :
 					     TCA_FLOWER_KEY_IPV6_SRC_MASK,
 					     IPV6_ADDR_LEN,
 					     mask.ipv6->hdr.src_addr);
 			}
-			if (!IN6_IS_ADDR_UNSPECIFIED(mask.ipv6->hdr.dst_addr)) {
-				mnl_attr_put(nlh, decap.vxlan ?
+			if (ipv6_dst) {
+				mnl_attr_put(nlh, tunnel_outer ?
 					     TCA_FLOWER_KEY_ENC_IPV6_DST :
 					     TCA_FLOWER_KEY_IPV6_DST,
 					     IPV6_ADDR_LEN,
 					     spec.ipv6->hdr.dst_addr);
-				mnl_attr_put(nlh, decap.vxlan ?
+				mnl_attr_put(nlh, tunnel_outer ?
 					     TCA_FLOWER_KEY_ENC_IPV6_DST_MASK :
 					     TCA_FLOWER_KEY_IPV6_DST_MASK,
 					     IPV6_ADDR_LEN,
@@ -3264,8 +3334,11 @@ struct pedit_parser {
 			}
 			assert(dev_flow->tcf.nlsize >= nlh->nlmsg_len);
 			break;
+		}
 		case RTE_FLOW_ITEM_TYPE_UDP:
-			item_flags |= MLX5_FLOW_LAYER_OUTER_L4_UDP;
+			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
+				      MLX5_FLOW_LAYER_INNER_L4_UDP :
+				      MLX5_FLOW_LAYER_OUTER_L4_UDP;
 			mask.udp = flow_tcf_item_mask
 				(items, &rte_flow_item_udp_mask,
 				 &flow_tcf_mask_supported.udp,
@@ -3274,7 +3347,7 @@ struct pedit_parser {
 				 error);
 			assert(mask.udp);
 			spec.udp = items->spec;
-			if (!decap.vxlan) {
+			if (!tunnel_outer) {
 				if (!ip_proto_set)
 					mnl_attr_put_u8
 						(nlh, TCA_FLOWER_KEY_IP_PROTO,
@@ -3289,24 +3362,24 @@ struct pedit_parser {
 			}
 			if (mask.udp->hdr.src_port) {
 				mnl_attr_put_u16
-					(nlh, decap.vxlan ?
+					(nlh, tunnel_outer ?
 					 TCA_FLOWER_KEY_ENC_UDP_SRC_PORT :
 					 TCA_FLOWER_KEY_UDP_SRC,
 					 spec.udp->hdr.src_port);
 				mnl_attr_put_u16
-					(nlh, decap.vxlan ?
+					(nlh, tunnel_outer ?
 					 TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK :
 					 TCA_FLOWER_KEY_UDP_SRC_MASK,
 					 mask.udp->hdr.src_port);
 			}
 			if (mask.udp->hdr.dst_port) {
 				mnl_attr_put_u16
-					(nlh, decap.vxlan ?
+					(nlh, tunnel_outer ?
 					 TCA_FLOWER_KEY_ENC_UDP_DST_PORT :
 					 TCA_FLOWER_KEY_UDP_DST,
 					 spec.udp->hdr.dst_port);
 				mnl_attr_put_u16
-					(nlh, decap.vxlan ?
+					(nlh, tunnel_outer ?
 					 TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK :
 					 TCA_FLOWER_KEY_UDP_DST_MASK,
 					 mask.udp->hdr.dst_port);
@@ -3314,7 +3387,9 @@ struct pedit_parser {
 			assert(dev_flow->tcf.nlsize >= nlh->nlmsg_len);
 			break;
 		case RTE_FLOW_ITEM_TYPE_TCP:
-			item_flags |= MLX5_FLOW_LAYER_OUTER_L4_TCP;
+			item_flags |= (item_flags & MLX5_FLOW_LAYER_TUNNEL) ?
+				      MLX5_FLOW_LAYER_INNER_L4_TCP :
+				      MLX5_FLOW_LAYER_OUTER_L4_TCP;
 			mask.tcp = flow_tcf_item_mask
 				(items, &rte_flow_item_tcp_mask,
 				 &flow_tcf_mask_supported.tcp,
@@ -3358,6 +3433,7 @@ struct pedit_parser {
 			break;
 		case RTE_FLOW_ITEM_TYPE_VXLAN:
 			assert(decap.vxlan);
+			tunnel_outer = 0;
 			item_flags |= MLX5_FLOW_LAYER_VXLAN;
 			spec.vxlan = items->spec;
 			mnl_attr_put_u32(nlh,
-- 
1.8.3.1

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

* [dpdk-dev] [PATCH 4/5] net/mlx5: add ethernet type validation on E-Switch
  2018-12-27 15:34 [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch Viacheslav Ovsiienko
                   ` (2 preceding siblings ...)
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 3/5] net/mlx5: add tunnel inner items support " Viacheslav Ovsiienko
@ 2018-12-27 15:34 ` Viacheslav Ovsiienko
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 5/5] net/mlx5: add ethernet type support for tunnels " Viacheslav Ovsiienko
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Viacheslav Ovsiienko @ 2018-12-27 15:34 UTC (permalink / raw)
  To: shahafs; +Cc: dev, stable

This patch updates the validation routine for the E-Switch Flows.
The ethernet type field can be specified within inner and outer
tunnel ethernet items, by vlan item or implicitly deduced from
IP address items. The validation routine checks all these items
and their combinations for mutual compatibility issues and possible
conflicts.

Cc: stable@dpdk.org

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_tcf.c | 114 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 113 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index 688422d..e70c377 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -1695,9 +1695,12 @@ struct pedit_parser {
 		const struct rte_flow_action_set_ipv6 *set_ipv6;
 	} conf;
 	const struct rte_flow_item *outer_udp = NULL;
+	rte_be16_t inner_etype = RTE_BE16(ETH_P_ALL);
+	rte_be16_t outer_etype = RTE_BE16(ETH_P_ALL);
+	rte_be16_t vlan_etype = RTE_BE16(ETH_P_ALL);
 	uint64_t item_flags = 0;
 	uint64_t action_flags = 0;
-	uint8_t next_protocol = -1;
+	uint8_t next_protocol = 0xff;
 	unsigned int tcm_ifindex = 0;
 	uint8_t pedit_validated = 0;
 	struct flow_tcf_ptoi ptoi[PTOI_TABLE_SZ_MAX(dev)];
@@ -1963,6 +1966,32 @@ struct pedit_parser {
 					 mask.eth,
 					 "no support for partial mask on"
 					 " \"type\" field");
+			assert(items->spec);
+			spec.eth = items->spec;
+			if (mask.eth->type &&
+			    (item_flags & MLX5_FLOW_LAYER_TUNNEL) &&
+			    inner_etype != RTE_BE16(ETH_P_ALL) &&
+			    inner_etype != spec.eth->type)
+				return rte_flow_error_set
+					(error, EINVAL,
+					 RTE_FLOW_ERROR_TYPE_ITEM,
+					 items,
+					 "inner eth_type conflict");
+			if (mask.eth->type &&
+			    !(item_flags & MLX5_FLOW_LAYER_TUNNEL) &&
+			    outer_etype != RTE_BE16(ETH_P_ALL) &&
+			    outer_etype != spec.eth->type)
+				return rte_flow_error_set
+					(error, EINVAL,
+					 RTE_FLOW_ERROR_TYPE_ITEM,
+					 items,
+					 "outer eth_type conflict");
+			if (mask.eth->type) {
+				if (item_flags & MLX5_FLOW_LAYER_TUNNEL)
+					inner_etype = spec.eth->type;
+				else
+					outer_etype = spec.eth->type;
+			}
 			break;
 		case RTE_FLOW_ITEM_TYPE_VLAN:
 			if (item_flags & MLX5_FLOW_LAYER_TUNNEL)
@@ -1999,6 +2028,27 @@ struct pedit_parser {
 					 "no support for partial masks on"
 					 " \"tci\" (PCP and VID parts) and"
 					 " \"inner_type\" fields");
+			if (outer_etype != RTE_BE16(ETH_P_ALL) &&
+			    outer_etype != RTE_BE16(ETH_P_8021Q))
+				return rte_flow_error_set
+					(error, EINVAL,
+					 RTE_FLOW_ERROR_TYPE_ITEM,
+					 items,
+					 "outer eth_type conflict,"
+					 " must be 802.1Q");
+			outer_etype = RTE_BE16(ETH_P_8021Q);
+			assert(items->spec);
+			spec.vlan = items->spec;
+			if (mask.vlan->inner_type &&
+			    vlan_etype != RTE_BE16(ETH_P_ALL) &&
+			    vlan_etype != spec.vlan->inner_type)
+				return rte_flow_error_set
+					(error, EINVAL,
+					 RTE_FLOW_ERROR_TYPE_ITEM,
+					 items,
+					 "vlan eth_type conflict");
+			if (mask.vlan->inner_type)
+				vlan_etype = spec.vlan->inner_type;
 			break;
 		case RTE_FLOW_ITEM_TYPE_IPV4:
 			ret = mlx5_flow_validate_item_ipv4(items, item_flags,
@@ -2028,6 +2078,37 @@ struct pedit_parser {
 				next_protocol =
 					((const struct rte_flow_item_ipv4 *)
 					 (items->spec))->hdr.next_proto_id;
+			if (item_flags & MLX5_FLOW_LAYER_TUNNEL) {
+				if (inner_etype != RTE_BE16(ETH_P_ALL) &&
+				    inner_etype != RTE_BE16(ETH_P_IP))
+					return rte_flow_error_set
+						(error, EINVAL,
+						 RTE_FLOW_ERROR_TYPE_ITEM,
+						 items,
+						 "inner eth_type conflict,"
+						 " IPv4 is required");
+				inner_etype = RTE_BE16(ETH_P_IP);
+			} else if (item_flags & MLX5_FLOW_LAYER_OUTER_VLAN) {
+				if (vlan_etype != RTE_BE16(ETH_P_ALL) &&
+				    vlan_etype != RTE_BE16(ETH_P_IP))
+					return rte_flow_error_set
+						(error, EINVAL,
+						 RTE_FLOW_ERROR_TYPE_ITEM,
+						 items,
+						 "vlan eth_type conflict,"
+						 " IPv4 is required");
+				vlan_etype = RTE_BE16(ETH_P_IP);
+			} else {
+				if (outer_etype != RTE_BE16(ETH_P_ALL) &&
+				    outer_etype != RTE_BE16(ETH_P_IP))
+					return rte_flow_error_set
+						(error, EINVAL,
+						 RTE_FLOW_ERROR_TYPE_ITEM,
+						 items,
+						 "eth_type conflict,"
+						 " IPv4 is required");
+				outer_etype = RTE_BE16(ETH_P_IP);
+			}
 			break;
 		case RTE_FLOW_ITEM_TYPE_IPV6:
 			ret = mlx5_flow_validate_item_ipv6(items, item_flags,
@@ -2057,6 +2138,37 @@ struct pedit_parser {
 				next_protocol =
 					((const struct rte_flow_item_ipv6 *)
 					 (items->spec))->hdr.proto;
+			if (item_flags & MLX5_FLOW_LAYER_TUNNEL) {
+				if (inner_etype != RTE_BE16(ETH_P_ALL) &&
+				    inner_etype != RTE_BE16(ETH_P_IPV6))
+					return rte_flow_error_set
+						(error, EINVAL,
+						 RTE_FLOW_ERROR_TYPE_ITEM,
+						 items,
+						 "inner eth_type conflict,"
+						 " IPv6 is required");
+				inner_etype = RTE_BE16(ETH_P_IPV6);
+			} else if (item_flags & MLX5_FLOW_LAYER_OUTER_VLAN) {
+				if (vlan_etype != RTE_BE16(ETH_P_ALL) &&
+				    vlan_etype != RTE_BE16(ETH_P_IPV6))
+					return rte_flow_error_set
+						(error, EINVAL,
+						 RTE_FLOW_ERROR_TYPE_ITEM,
+						 items,
+						 "vlan eth_type conflict,"
+						 " IPv6 is required");
+				vlan_etype = RTE_BE16(ETH_P_IPV6);
+			} else {
+				if (outer_etype != RTE_BE16(ETH_P_ALL) &&
+				    outer_etype != RTE_BE16(ETH_P_IPV6))
+					return rte_flow_error_set
+						(error, EINVAL,
+						 RTE_FLOW_ERROR_TYPE_ITEM,
+						 items,
+						 "eth_type conflict,"
+						 " IPv6 is required");
+				outer_etype = RTE_BE16(ETH_P_IPV6);
+			}
 			break;
 		case RTE_FLOW_ITEM_TYPE_UDP:
 			ret = mlx5_flow_validate_item_udp(items, item_flags,
-- 
1.8.3.1

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

* [dpdk-dev] [PATCH 5/5] net/mlx5: add ethernet type support for tunnels on E-Switch
  2018-12-27 15:34 [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch Viacheslav Ovsiienko
                   ` (3 preceding siblings ...)
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 4/5] net/mlx5: add ethernet type validation " Viacheslav Ovsiienko
@ 2018-12-27 15:34 ` Viacheslav Ovsiienko
  2019-01-13 12:11 ` [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support " Shahaf Shuler
  2019-01-31 14:52 ` Kevin Traynor
  6 siblings, 0 replies; 11+ messages in thread
From: Viacheslav Ovsiienko @ 2018-12-27 15:34 UTC (permalink / raw)
  To: shahafs; +Cc: dev, stable

This patch add support for inner and outer ethernet types for the
E-Switch Flows with tunnels. Inner and outer ethernet type match
can be specified with ethernet items, vlan items, or implicitly
deduced from IP address items. The tcm_info field in Netlink message
tcm structure is filled always with outer protocol.

Cc: stable@dpdk.org

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_tcf.c | 127 +++++++++++++++++++++++----------------
 1 file changed, 74 insertions(+), 53 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index e70c377..9e5d947 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -2420,6 +2420,7 @@ struct pedit_parser {
 	int size = 0;
 
 	size += SZ_NLATTR_STRZ_OF("flower") +
+		SZ_NLATTR_TYPE_OF(uint16_t) + /* Outer ether type. */
 		SZ_NLATTR_NEST + /* TCA_OPTIONS. */
 		SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CLS_FLAGS_SKIP_SW. */
 	if (attr->group > 0)
@@ -2431,26 +2432,22 @@ struct pedit_parser {
 		case RTE_FLOW_ITEM_TYPE_PORT_ID:
 			break;
 		case RTE_FLOW_ITEM_TYPE_ETH:
-			size += SZ_NLATTR_TYPE_OF(uint16_t) + /* Ether type. */
-				SZ_NLATTR_DATA_OF(ETHER_ADDR_LEN) * 4;
+			size += SZ_NLATTR_DATA_OF(ETHER_ADDR_LEN) * 4;
 				/* dst/src MAC addr and mask. */
 			break;
 		case RTE_FLOW_ITEM_TYPE_VLAN:
-			size += SZ_NLATTR_TYPE_OF(uint16_t) + /* Ether type. */
-				SZ_NLATTR_TYPE_OF(uint16_t) +
+			size +=	SZ_NLATTR_TYPE_OF(uint16_t) +
 				/* VLAN Ether type. */
 				SZ_NLATTR_TYPE_OF(uint8_t) + /* VLAN prio. */
 				SZ_NLATTR_TYPE_OF(uint16_t); /* VLAN ID. */
 			break;
 		case RTE_FLOW_ITEM_TYPE_IPV4:
-			size += SZ_NLATTR_TYPE_OF(uint16_t) + /* Ether type. */
-				SZ_NLATTR_TYPE_OF(uint8_t) + /* IP proto. */
+			size +=	SZ_NLATTR_TYPE_OF(uint8_t) + /* IP proto. */
 				SZ_NLATTR_TYPE_OF(uint32_t) * 4;
 				/* dst/src IP addr and mask. */
 			break;
 		case RTE_FLOW_ITEM_TYPE_IPV6:
-			size += SZ_NLATTR_TYPE_OF(uint16_t) + /* Ether type. */
-				SZ_NLATTR_TYPE_OF(uint8_t) + /* IP proto. */
+			size +=	SZ_NLATTR_TYPE_OF(uint8_t) + /* IP proto. */
 				SZ_NLATTR_DATA_OF(IPV6_ADDR_LEN) * 4;
 				/* dst/src IP addr and mask. */
 			break;
@@ -3124,9 +3121,9 @@ struct pedit_parser {
 	struct nlmsghdr *nlh = dev_flow->tcf.nlh;
 	struct tcmsg *tcm = dev_flow->tcf.tcm;
 	uint32_t na_act_index_cur;
-	bool eth_type_set = 0;
-	bool vlan_present = 0;
-	bool vlan_eth_type_set = 0;
+	rte_be16_t inner_etype = RTE_BE16(ETH_P_ALL);
+	rte_be16_t outer_etype = RTE_BE16(ETH_P_ALL);
+	rte_be16_t vlan_etype = RTE_BE16(ETH_P_ALL);
 	bool ip_proto_set = 0;
 	bool tunnel_outer = 0;
 	struct nlattr *na_flower;
@@ -3164,8 +3161,7 @@ struct pedit_parser {
 	 * Priority cannot be zero to prevent the kernel from picking one
 	 * automatically.
 	 */
-	tcm->tcm_info = TC_H_MAKE((attr->priority + 1) << 16,
-				  RTE_BE16(ETH_P_ALL));
+	tcm->tcm_info = TC_H_MAKE((attr->priority + 1) << 16, outer_etype);
 	if (attr->group > 0)
 		mnl_attr_put_u32(nlh, TCA_CHAIN, attr->group);
 	mnl_attr_put_strz(nlh, TCA_KIND, "flower");
@@ -3210,6 +3206,12 @@ struct pedit_parser {
 			if (mask.eth == &flow_tcf_mask_empty.eth)
 				break;
 			spec.eth = items->spec;
+			if (mask.eth->type) {
+				if (item_flags & MLX5_FLOW_LAYER_TUNNEL)
+					inner_etype = spec.eth->type;
+				else
+					outer_etype = spec.eth->type;
+			}
 			if (tunnel_outer) {
 				DRV_LOG(WARNING,
 					"outer L2 addresses cannot be"
@@ -3217,11 +3219,6 @@ struct pedit_parser {
 					" parameter is ignored");
 				break;
 			}
-			if (mask.eth->type) {
-				mnl_attr_put_u16(nlh, TCA_FLOWER_KEY_ETH_TYPE,
-						 spec.eth->type);
-				eth_type_set = 1;
-			}
 			if (!is_zero_ether_addr(&mask.eth->dst)) {
 				mnl_attr_put(nlh, TCA_FLOWER_KEY_ETH_DST,
 					     ETHER_ADDR_LEN,
@@ -3252,20 +3249,14 @@ struct pedit_parser {
 				 sizeof(flow_tcf_mask_supported.vlan),
 				 error);
 			assert(mask.vlan);
-			if (!eth_type_set)
-				mnl_attr_put_u16(nlh, TCA_FLOWER_KEY_ETH_TYPE,
-						 RTE_BE16(ETH_P_8021Q));
-			eth_type_set = 1;
-			vlan_present = 1;
 			if (mask.vlan == &flow_tcf_mask_empty.vlan)
 				break;
 			spec.vlan = items->spec;
-			if (mask.vlan->inner_type) {
-				mnl_attr_put_u16(nlh,
-						 TCA_FLOWER_KEY_VLAN_ETH_TYPE,
-						 spec.vlan->inner_type);
-				vlan_eth_type_set = 1;
-			}
+			assert(outer_etype == RTE_BE16(ETH_P_ALL) ||
+			       outer_etype == RTE_BE16(ETH_P_8021Q));
+			outer_etype = RTE_BE16(ETH_P_8021Q);
+			if (mask.vlan->inner_type)
+				vlan_etype = spec.vlan->inner_type;
 			if (mask.vlan->tci & RTE_BE16(0xe000))
 				mnl_attr_put_u8(nlh, TCA_FLOWER_KEY_VLAN_PRIO,
 						(rte_be_to_cpu_16
@@ -3288,19 +3279,20 @@ struct pedit_parser {
 				 sizeof(flow_tcf_mask_supported.ipv4),
 				 error);
 			assert(mask.ipv4);
-			spec.ipv4 = items->spec;
-			if (!tunnel_outer) {
-				if (!eth_type_set ||
-				    (!vlan_eth_type_set && vlan_present))
-					mnl_attr_put_u16
-						(nlh,
-						 vlan_present ?
-						 TCA_FLOWER_KEY_VLAN_ETH_TYPE :
-						 TCA_FLOWER_KEY_ETH_TYPE,
-						 RTE_BE16(ETH_P_IP));
-				eth_type_set = 1;
-				vlan_eth_type_set = 1;
+			if (item_flags & MLX5_FLOW_LAYER_TUNNEL) {
+				assert(inner_etype == RTE_BE16(ETH_P_ALL) ||
+				       inner_etype == RTE_BE16(ETH_P_IP));
+				inner_etype = RTE_BE16(ETH_P_IP);
+			} else if (outer_etype == RTE_BE16(ETH_P_8021Q)) {
+				assert(vlan_etype == RTE_BE16(ETH_P_ALL) ||
+				       vlan_etype == RTE_BE16(ETH_P_IP));
+				vlan_etype = RTE_BE16(ETH_P_IP);
+			} else {
+				assert(outer_etype == RTE_BE16(ETH_P_ALL) ||
+				       outer_etype == RTE_BE16(ETH_P_IP));
+				outer_etype = RTE_BE16(ETH_P_IP);
 			}
+			spec.ipv4 = items->spec;
 			if (!tunnel_outer && mask.ipv4->hdr.next_proto_id) {
 				/*
 				 * No way to set IP protocol for outer tunnel
@@ -3371,19 +3363,20 @@ struct pedit_parser {
 				 sizeof(flow_tcf_mask_supported.ipv6),
 				 error);
 			assert(mask.ipv6);
-			spec.ipv6 = items->spec;
-			if (!tunnel_outer) {
-				if (!eth_type_set ||
-				    (!vlan_eth_type_set && vlan_present))
-					mnl_attr_put_u16
-						(nlh,
-						 vlan_present ?
-						 TCA_FLOWER_KEY_VLAN_ETH_TYPE :
-						 TCA_FLOWER_KEY_ETH_TYPE,
-						 RTE_BE16(ETH_P_IPV6));
-				eth_type_set = 1;
-				vlan_eth_type_set = 1;
+			if (item_flags & MLX5_FLOW_LAYER_TUNNEL) {
+				assert(inner_etype == RTE_BE16(ETH_P_ALL) ||
+				       inner_etype == RTE_BE16(ETH_P_IPV6));
+				inner_etype = RTE_BE16(ETH_P_IPV6);
+			} else if (outer_etype == RTE_BE16(ETH_P_8021Q)) {
+				assert(vlan_etype == RTE_BE16(ETH_P_ALL) ||
+				       vlan_etype == RTE_BE16(ETH_P_IPV6));
+				vlan_etype = RTE_BE16(ETH_P_IPV6);
+			} else {
+				assert(outer_etype == RTE_BE16(ETH_P_ALL) ||
+				       outer_etype == RTE_BE16(ETH_P_IPV6));
+				outer_etype = RTE_BE16(ETH_P_IPV6);
 			}
+			spec.ipv6 = items->spec;
 			if (!tunnel_outer && mask.ipv6->hdr.proto) {
 				/*
 				 * No way to set IP protocol for outer tunnel
@@ -3559,6 +3552,34 @@ struct pedit_parser {
 						  NULL, "item not supported");
 		}
 	}
+	/*
+	 * Set the ether_type flower key and tc rule protocol:
+	 * - if there is nor VLAN neither VXLAN the key is taken from
+	 *   eth item directly or deduced from L3 items.
+	 * - if there is vlan item then key is fixed to 802.1q.
+	 * - if there is vxlan item then key is set to inner tunnel type.
+	 * - simultaneous vlan and vxlan items are prohibited.
+	 */
+	if (outer_etype != RTE_BE16(ETH_P_ALL)) {
+		tcm->tcm_info = TC_H_MAKE((attr->priority + 1) << 16,
+					   outer_etype);
+		if (item_flags & MLX5_FLOW_LAYER_TUNNEL) {
+			if (inner_etype != RTE_BE16(ETH_P_ALL))
+				mnl_attr_put_u16(nlh,
+						 TCA_FLOWER_KEY_ETH_TYPE,
+						 inner_etype);
+		} else {
+			mnl_attr_put_u16(nlh,
+					 TCA_FLOWER_KEY_ETH_TYPE,
+					 outer_etype);
+			if (outer_etype == RTE_BE16(ETH_P_8021Q) &&
+			    vlan_etype != RTE_BE16(ETH_P_ALL))
+				mnl_attr_put_u16(nlh,
+						 TCA_FLOWER_KEY_VLAN_ETH_TYPE,
+						 vlan_etype);
+		}
+		assert(dev_flow->tcf.nlsize >= nlh->nlmsg_len);
+	}
 	na_flower_act = mnl_attr_nest_start(nlh, TCA_FLOWER_ACT);
 	na_act_index_cur = 1;
 	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
-- 
1.8.3.1

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

* Re: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch
  2018-12-27 15:34 [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch Viacheslav Ovsiienko
                   ` (4 preceding siblings ...)
  2018-12-27 15:34 ` [dpdk-dev] [PATCH 5/5] net/mlx5: add ethernet type support for tunnels " Viacheslav Ovsiienko
@ 2019-01-13 12:11 ` Shahaf Shuler
  2019-01-31 14:52 ` Kevin Traynor
  6 siblings, 0 replies; 11+ messages in thread
From: Shahaf Shuler @ 2019-01-13 12:11 UTC (permalink / raw)
  To: Slava Ovsiienko; +Cc: dev, stable

Thursday, December 27, 2018 5:35 PM, Viacheslav Ovsiienko:
> Subject: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support
> on E-Switch
> 
> The generic Flow rule for tunnels looks like:
> 
> flow create <attributes> <port> \
>             <tunnel outer items pattern> \
>             <tunnel vni item> \
>             <tunnel inner items pattern>
> 
> Current design supports only L2 addresses as inner pattern items. This
> patchset adds support for L3 (IPv4/IPv6) addresses and L4 (TCP/UDP) ports
> items as inner tunnel parameters.
> 
> Also this patchset adds support for inner and outer ethernet types for the E-
> Switch Flows with tunnels. Inner and outer ethernet type match  can be
> specified with ethernet items, vlan items, or implicitly deduced from IP
> address items. The tcm_info field in Netlink message tcm structure is filled
> always with outer protocol.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

Applied to next-net-mlx, thanks. 

> 
> Viacheslav Ovsiienko (5):
>   net/mlx5: remove checks for outer tunnel items on E-Switch
>   net/mlx5: add tunnel inner items validation on E-Switch
>   net/mlx5: add tunnel inner items support on E-Switch
>   net/mlx5: add ethernet type validation on E-Switch
>   net/mlx5: add ethernet type support for tunnels on E-Switch
> 
>  drivers/net/mlx5/mlx5_flow_tcf.c | 690 ++++++++++++++++++++++---------
> --------
>  1 file changed, 399 insertions(+), 291 deletions(-)
> 
> --
> 1.8.3.1

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

* Re: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch
  2018-12-27 15:34 [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch Viacheslav Ovsiienko
                   ` (5 preceding siblings ...)
  2019-01-13 12:11 ` [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support " Shahaf Shuler
@ 2019-01-31 14:52 ` Kevin Traynor
  2019-01-31 16:13   ` Slava Ovsiienko
  6 siblings, 1 reply; 11+ messages in thread
From: Kevin Traynor @ 2019-01-31 14:52 UTC (permalink / raw)
  To: Viacheslav Ovsiienko, shahafs
  Cc: dev, stable, Thomas Monjalon, Yongseok Koh, Luca Boccassi

On 12/27/2018 03:34 PM, Viacheslav Ovsiienko wrote:
> The generic Flow rule for tunnels looks like:
> 
> flow create <attributes> <port> \
>             <tunnel outer items pattern> \
>             <tunnel vni item> \
>             <tunnel inner items pattern>
> 			    
> Current design supports only L2 addresses as inner pattern
> items. This patchset adds support for L3 (IPv4/IPv6) addresses
> and L4 (TCP/UDP) ports items as inner tunnel parameters.
> 	
> Also this patchset adds support for inner and outer ethernet
> types for the E-Switch Flows with tunnels. Inner and outer ethernet
> type match  can be specified with ethernet items, vlan items, or
> implicitly deduced from IP address items. The tcm_info field 
> in Netlink message tcm structure is filled always with outer
> protocol.
> 	 
> Cc: stable@dpdk.org
> 	

Hi Viacheslav - these are new features, not a bugfixes. Are the
stable@dpdk.org tags intentional?

From
http://doc.dpdk.org/guides/contributing/stable.html#what-changes-should-be-backported

--
Features should not be backported to stable releases. It may be
acceptable, in limited cases, to back port features for the LTS release
where:

    There is a justifiable use case (for example a new PMD).
    The change is non-invasive.
    The work of preparing the backport is done by the proposer.
    There is support within the community.
--

Kevin.


> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> 
> Viacheslav Ovsiienko (5):
>   net/mlx5: remove checks for outer tunnel items on E-Switch
>   net/mlx5: add tunnel inner items validation on E-Switch
>   net/mlx5: add tunnel inner items support on E-Switch
>   net/mlx5: add ethernet type validation on E-Switch
>   net/mlx5: add ethernet type support for tunnels on E-Switch
> 
>  drivers/net/mlx5/mlx5_flow_tcf.c | 690 ++++++++++++++++++++++-----------------
>  1 file changed, 399 insertions(+), 291 deletions(-)
> 

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

* Re: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch
  2019-01-31 14:52 ` Kevin Traynor
@ 2019-01-31 16:13   ` Slava Ovsiienko
  2019-02-06 11:00     ` Kevin Traynor
  0 siblings, 1 reply; 11+ messages in thread
From: Slava Ovsiienko @ 2019-01-31 16:13 UTC (permalink / raw)
  To: Kevin Traynor, Shahaf Shuler
  Cc: dev, stable, Thomas Monjalon, Yongseok Koh, Luca Boccassi

Hi, Kevin

It is rather refactoring, not new feature, tunnel inner items support is partially present in 18.11.
Yes, this patchset is too big to be simple fix, and updates not so much to be new feature. 
It is very early patch (after 18.11 release), just mailed later, rebase on the top was done 
automatically and quite simple, because it touches merely E-Switch subsystem (merge should
not be difficult - mostly mlx5_flow_tcf.c is changed) and highly desirable to be backported. 
It fixes serious bugs with ether_type field, which limit VLAN and VXLAN functionality on E-Switch.

With best regards,
Slava (aka Viacheslav)

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Thursday, January 31, 2019 16:52
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Thomas Monjalon
> <thomas@monjalon.net>; Yongseok Koh <yskoh@mellanox.com>; Luca
> Boccassi <bluca@debian.org>
> Subject: Re: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support
> on E-Switch
> 
> On 12/27/2018 03:34 PM, Viacheslav Ovsiienko wrote:
> > The generic Flow rule for tunnels looks like:
> >
> > flow create <attributes> <port> \
> >             <tunnel outer items pattern> \
> >             <tunnel vni item> \
> >             <tunnel inner items pattern>
> >
> > Current design supports only L2 addresses as inner pattern items. This
> > patchset adds support for L3 (IPv4/IPv6) addresses and L4 (TCP/UDP)
> > ports items as inner tunnel parameters.
> >
> > Also this patchset adds support for inner and outer ethernet types for
> > the E-Switch Flows with tunnels. Inner and outer ethernet type match
> > can be specified with ethernet items, vlan items, or implicitly
> > deduced from IP address items. The tcm_info field in Netlink message
> > tcm structure is filled always with outer protocol.
> >
> > Cc: stable@dpdk.org
> >
> 
> Hi Viacheslav - these are new features, not a bugfixes. Are the
> stable@dpdk.org tags intentional?
> 
> From
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdoc.dp
> dk.org%2Fguides%2Fcontributing%2Fstable.html%23what-changes-should-be-
> backported&amp;data=02%7C01%7Cviacheslavo%40mellanox.com%7C1d6e08
> fe6f7f47d83f7e08d6878bb56d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0
> %7C0%7C636845431441557730&amp;sdata=%2Fmgoj2Il%2Fu4CCrkAcETdoMp
> d7Ri5ash%2FUhzGv1dLXLY%3D&amp;reserved=0
> 
> --
> Features should not be backported to stable releases. It may be acceptable, in
> limited cases, to back port features for the LTS release
> where:
> 
>     There is a justifiable use case (for example a new PMD).
>     The change is non-invasive.
>     The work of preparing the backport is done by the proposer.
>     There is support within the community.
> --
> 
> Kevin.
> 
> 
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> >
> > Viacheslav Ovsiienko (5):
> >   net/mlx5: remove checks for outer tunnel items on E-Switch
> >   net/mlx5: add tunnel inner items validation on E-Switch
> >   net/mlx5: add tunnel inner items support on E-Switch
> >   net/mlx5: add ethernet type validation on E-Switch
> >   net/mlx5: add ethernet type support for tunnels on E-Switch
> >
> >  drivers/net/mlx5/mlx5_flow_tcf.c | 690
> > ++++++++++++++++++++++-----------------
> >  1 file changed, 399 insertions(+), 291 deletions(-)
> >


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

* Re: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch
  2019-01-31 16:13   ` Slava Ovsiienko
@ 2019-02-06 11:00     ` Kevin Traynor
  2019-02-06 11:56       ` Slava Ovsiienko
  0 siblings, 1 reply; 11+ messages in thread
From: Kevin Traynor @ 2019-02-06 11:00 UTC (permalink / raw)
  To: Slava Ovsiienko, Shahaf Shuler
  Cc: dev, stable, Thomas Monjalon, Yongseok Koh, Luca Boccassi, stable

On 01/31/2019 04:13 PM, Slava Ovsiienko wrote:
> Hi, Kevin
> 

Hi Slava,

> It is rather refactoring, not new feature, tunnel inner items support is partially present in 18.11.
> Yes, this patchset is too big to be simple fix, and updates not so much to be new feature. 
> It is very early patch (after 18.11 release), just mailed later, rebase on the top was done 
> automatically and quite simple, because it touches merely E-Switch subsystem (merge should
> not be difficult - mostly mlx5_flow_tcf.c is changed) and highly desirable to be backported. 
> It fixes serious bugs with ether_type field, which limit VLAN and VXLAN functionality on E-Switch.
> 

The issue I have is it also adding functionality and more importantly
reworking how some existing behavior is handled (e.g. outer tunnel
validation moving in 1/5). But, yes, it is completely isolated to
mlx5/tcf and applies cleanly, so that is a positive.

As it's a bit grey, I will backport to 18.11 on the understanding that
it will be supported by Mellanox and you will test one of the
18.11.1-RC's to make sure there are no regressions on the existing
behavior - is that reasonable?

thanks,
Kevin.

> With best regards,
> Slava (aka Viacheslav)
> 
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Thursday, January 31, 2019 16:52
>> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Shahaf Shuler
>> <shahafs@mellanox.com>
>> Cc: dev@dpdk.org; stable@dpdk.org; Thomas Monjalon
>> <thomas@monjalon.net>; Yongseok Koh <yskoh@mellanox.com>; Luca
>> Boccassi <bluca@debian.org>
>> Subject: Re: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support
>> on E-Switch
>>
>> On 12/27/2018 03:34 PM, Viacheslav Ovsiienko wrote:
>>> The generic Flow rule for tunnels looks like:
>>>
>>> flow create <attributes> <port> \
>>>             <tunnel outer items pattern> \
>>>             <tunnel vni item> \
>>>             <tunnel inner items pattern>
>>>
>>> Current design supports only L2 addresses as inner pattern items. This
>>> patchset adds support for L3 (IPv4/IPv6) addresses and L4 (TCP/UDP)
>>> ports items as inner tunnel parameters.
>>>
>>> Also this patchset adds support for inner and outer ethernet types for
>>> the E-Switch Flows with tunnels. Inner and outer ethernet type match
>>> can be specified with ethernet items, vlan items, or implicitly
>>> deduced from IP address items. The tcm_info field in Netlink message
>>> tcm structure is filled always with outer protocol.
>>>
>>> Cc: stable@dpdk.org
>>>
>>
>> Hi Viacheslav - these are new features, not a bugfixes. Are the
>> stable@dpdk.org tags intentional?
>>
>> From
>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdoc.dp
>> dk.org%2Fguides%2Fcontributing%2Fstable.html%23what-changes-should-be-
>> backported&amp;data=02%7C01%7Cviacheslavo%40mellanox.com%7C1d6e08
>> fe6f7f47d83f7e08d6878bb56d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0
>> %7C0%7C636845431441557730&amp;sdata=%2Fmgoj2Il%2Fu4CCrkAcETdoMp
>> d7Ri5ash%2FUhzGv1dLXLY%3D&amp;reserved=0
>>
>> --
>> Features should not be backported to stable releases. It may be acceptable, in
>> limited cases, to back port features for the LTS release
>> where:
>>
>>     There is a justifiable use case (for example a new PMD).
>>     The change is non-invasive.
>>     The work of preparing the backport is done by the proposer.
>>     There is support within the community.
>> --
>>
>> Kevin.
>>
>>
>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>>>
>>> Viacheslav Ovsiienko (5):
>>>   net/mlx5: remove checks for outer tunnel items on E-Switch
>>>   net/mlx5: add tunnel inner items validation on E-Switch
>>>   net/mlx5: add tunnel inner items support on E-Switch
>>>   net/mlx5: add ethernet type validation on E-Switch
>>>   net/mlx5: add ethernet type support for tunnels on E-Switch
>>>
>>>  drivers/net/mlx5/mlx5_flow_tcf.c | 690
>>> ++++++++++++++++++++++-----------------
>>>  1 file changed, 399 insertions(+), 291 deletions(-)
>>>
> 

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

* Re: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch
  2019-02-06 11:00     ` Kevin Traynor
@ 2019-02-06 11:56       ` Slava Ovsiienko
  0 siblings, 0 replies; 11+ messages in thread
From: Slava Ovsiienko @ 2019-02-06 11:56 UTC (permalink / raw)
  To: Kevin Traynor, Shahaf Shuler
  Cc: dev, stable, Thomas Monjalon, Yongseok Koh, Luca Boccassi, stable

Hi, Kevin

> As it's a bit grey, I will backport to 18.11 on the understanding that it will be
> supported by Mellanox and you will test one of the 18.11.1-RC's to make sure
> there are no regressions on the existing behavior - is that reasonable?
>
Yes, it is quite reasonable, I will test it. This code is also merged in 19.02, so
the related problems are not very likely.

WBR,
Slava (aka Viacheslav)

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Wednesday, February 6, 2019 13:01
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Thomas Monjalon
> <thomas@monjalon.net>; Yongseok Koh <yskoh@mellanox.com>; Luca
> Boccassi <bluca@debian.org>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support
> on E-Switch
> 
> On 01/31/2019 04:13 PM, Slava Ovsiienko wrote:
> > Hi, Kevin
> >
> 
> Hi Slava,
> 
> > It is rather refactoring, not new feature, tunnel inner items support is
> partially present in 18.11.
> > Yes, this patchset is too big to be simple fix, and updates not so much to be
> new feature.
> > It is very early patch (after 18.11 release), just mailed later,
> > rebase on the top was done automatically and quite simple, because it
> > touches merely E-Switch subsystem (merge should not be difficult - mostly
> mlx5_flow_tcf.c is changed) and highly desirable to be backported.
> > It fixes serious bugs with ether_type field, which limit VLAN and VXLAN
> functionality on E-Switch.
> >
> 
> The issue I have is it also adding functionality and more importantly
> reworking how some existing behavior is handled (e.g. outer tunnel validation
> moving in 1/5). But, yes, it is completely isolated to mlx5/tcf and applies
> cleanly, so that is a positive.
> 
> As it's a bit grey, I will backport to 18.11 on the understanding that it will be
> supported by Mellanox and you will test one of the 18.11.1-RC's to make sure
> there are no regressions on the existing behavior - is that reasonable?
> 
> thanks,
> Kevin.
> 
> > With best regards,
> > Slava (aka Viacheslav)
> >
> >> -----Original Message-----
> >> From: Kevin Traynor <ktraynor@redhat.com>
> >> Sent: Thursday, January 31, 2019 16:52
> >> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Shahaf Shuler
> >> <shahafs@mellanox.com>
> >> Cc: dev@dpdk.org; stable@dpdk.org; Thomas Monjalon
> >> <thomas@monjalon.net>; Yongseok Koh <yskoh@mellanox.com>; Luca
> >> Boccassi <bluca@debian.org>
> >> Subject: Re: [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items
> >> support on E-Switch
> >>
> >> On 12/27/2018 03:34 PM, Viacheslav Ovsiienko wrote:
> >>> The generic Flow rule for tunnels looks like:
> >>>
> >>> flow create <attributes> <port> \
> >>>             <tunnel outer items pattern> \
> >>>             <tunnel vni item> \
> >>>             <tunnel inner items pattern>
> >>>
> >>> Current design supports only L2 addresses as inner pattern items.
> >>> This patchset adds support for L3 (IPv4/IPv6) addresses and L4
> >>> (TCP/UDP) ports items as inner tunnel parameters.
> >>>
> >>> Also this patchset adds support for inner and outer ethernet types
> >>> for the E-Switch Flows with tunnels. Inner and outer ethernet type
> >>> match can be specified with ethernet items, vlan items, or
> >>> implicitly deduced from IP address items. The tcm_info field in
> >>> Netlink message tcm structure is filled always with outer protocol.
> >>>
> >>> Cc: stable@dpdk.org
> >>>
> >>
> >> Hi Viacheslav - these are new features, not a bugfixes. Are the
> >> stable@dpdk.org tags intentional?
> >>
> >> From
> >> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdoc
> >> .dp
> >> dk.org%2Fguides%2Fcontributing%2Fstable.html%23what-changes-should-
> be
> >> -
> >>
> backported&amp;data=02%7C01%7Cviacheslavo%40mellanox.com%7C1d6e08
> >>
> fe6f7f47d83f7e08d6878bb56d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0
> >>
> %7C0%7C636845431441557730&amp;sdata=%2Fmgoj2Il%2Fu4CCrkAcETdoMp
> >> d7Ri5ash%2FUhzGv1dLXLY%3D&amp;reserved=0
> >>
> >> --
> >> Features should not be backported to stable releases. It may be
> >> acceptable, in limited cases, to back port features for the LTS
> >> release
> >> where:
> >>
> >>     There is a justifiable use case (for example a new PMD).
> >>     The change is non-invasive.
> >>     The work of preparing the backport is done by the proposer.
> >>     There is support within the community.
> >> --
> >>
> >> Kevin.
> >>
> >>
> >>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> >>>
> >>> Viacheslav Ovsiienko (5):
> >>>   net/mlx5: remove checks for outer tunnel items on E-Switch
> >>>   net/mlx5: add tunnel inner items validation on E-Switch
> >>>   net/mlx5: add tunnel inner items support on E-Switch
> >>>   net/mlx5: add ethernet type validation on E-Switch
> >>>   net/mlx5: add ethernet type support for tunnels on E-Switch
> >>>
> >>>  drivers/net/mlx5/mlx5_flow_tcf.c | 690
> >>> ++++++++++++++++++++++-----------------
> >>>  1 file changed, 399 insertions(+), 291 deletions(-)
> >>>
> >


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

end of thread, other threads:[~2019-02-06 11:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-27 15:34 [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support on E-Switch Viacheslav Ovsiienko
2018-12-27 15:34 ` [dpdk-dev] [PATCH 1/5] net/mlx5: remove checks for outer tunnel items " Viacheslav Ovsiienko
2018-12-27 15:34 ` [dpdk-dev] [PATCH 2/5] net/mlx5: add tunnel inner items validation " Viacheslav Ovsiienko
2018-12-27 15:34 ` [dpdk-dev] [PATCH 3/5] net/mlx5: add tunnel inner items support " Viacheslav Ovsiienko
2018-12-27 15:34 ` [dpdk-dev] [PATCH 4/5] net/mlx5: add ethernet type validation " Viacheslav Ovsiienko
2018-12-27 15:34 ` [dpdk-dev] [PATCH 5/5] net/mlx5: add ethernet type support for tunnels " Viacheslav Ovsiienko
2019-01-13 12:11 ` [dpdk-dev] [PATCH 0/5] net/mlx5: add inner tunnel items support " Shahaf Shuler
2019-01-31 14:52 ` Kevin Traynor
2019-01-31 16:13   ` Slava Ovsiienko
2019-02-06 11:00     ` Kevin Traynor
2019-02-06 11:56       ` Slava Ovsiienko

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