[dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

[dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

[Rory Sexton]

- RTE_FLOW_ITEM_TYPE_L2TPV3: matches a L2TPv3 header

Signed-off-by: Rory Sexton <rory.sexton@intel.com>
Signed-off-by: Dariusz Jagus <dariuszx.jagus@intel.com>
---
 app/test-pmd/cmdline_flow.c                 | 28 +++++++++++++++++++++
 doc/guides/prog_guide/rte_flow.rst          |  8 ++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 +++
 lib/librte_ethdev/rte_flow.c                |  1 +
 lib/librte_ethdev/rte_flow.h                | 27 ++++++++++++++++++++
 5 files changed, 68 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 99dade7d8..a7fe7a7a8 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -213,6 +213,8 @@ enum index {
 	ITEM_TAG,
 	ITEM_TAG_DATA,
 	ITEM_TAG_INDEX,
+	ITEM_L2TPV3,
+	ITEM_L2TPV3_SESSION_ID,
 
 	/* Validate/create actions. */
 	ACTIONS,
@@ -746,6 +748,7 @@ static const enum index next_item[] = {
 	ITEM_PPPOE_PROTO_ID,
 	ITEM_HIGIG2,
 	ITEM_TAG,
+	ITEM_L2TPV3,
 	END_SET,
 	ZERO,
 };
@@ -1030,6 +1033,12 @@ static const enum index item_tag[] = {
 	ZERO,
 };
 
+static const enum index item_l2tpv3[] = {
+	ITEM_L2TPV3_SESSION_ID,
+	ITEM_NEXT,
+	ZERO,
+};
+
 static const enum index next_action[] = {
 	ACTION_END,
 	ACTION_VOID,
@@ -2593,6 +2602,21 @@ static const struct token token_list[] = {
 			     NEXT_ENTRY(ITEM_PARAM_IS)),
 		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_tag, index)),
 	},
+	[ITEM_L2TPV3] = {
+		.name = "l2tpv3",
+		.help = "match L2TPv3 header",
+		.priv = PRIV_ITEM(L2TPV3, sizeof(struct rte_flow_item_l2tpv3)),
+		.next = NEXT(item_l2tpv3),
+		.call = parse_vc,
+	},
+	[ITEM_L2TPV3_SESSION_ID] = {
+		.name = "session_id",
+		.help = "session identifier",
+		.next = NEXT(item_l2tpv3, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_l2tpv3,
+					     session_id)),
+	},
+
 	/* Validate/create actions. */
 	[ACTIONS] = {
 		.name = "actions",
@@ -6238,6 +6262,10 @@ flow_item_default_mask(const struct rte_flow_item *item)
 		break;
 	case RTE_FLOW_ITEM_TYPE_PPPOE_PROTO_ID:
 		mask = &rte_flow_item_pppoe_proto_id_mask;
+		break;
+	case RTE_FLOW_ITEM_TYPE_L2TPV3:
+		mask = &rte_flow_item_l2tpv3_mask;
+		break;
 	default:
 		break;
 	}
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index a254c81ef..c7ddb38c5 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1336,6 +1336,14 @@ Broadcom switches.
 
 - Default ``mask`` matches classification and vlan.
 
+Item: ``L2TPV3``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Matches a L2TPv3 header.
+
+- ``session_id``: L2TPv3 session identifier.
+- Default ``mask`` matches session_id only.
+
 
 Actions
 ~~~~~~~
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 73ef0b41d..a48e77619 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3954,6 +3954,10 @@ This section lists supported pattern items and their attributes, if any.
 
   - ``proto_id {unsigned}``: PPP protocol identifier.
 
+- ``l2tpv3``: match L2TPv3 header.
+
+  - ``session_id {unsigned}``: L2TPv3 session identifier.
+
 Actions list
 ^^^^^^^^^^^^
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 87a3e8c4c..fcda73320 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -93,6 +93,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = {
 	MK_FLOW_ITEM(IGMP, sizeof(struct rte_flow_item_igmp)),
 	MK_FLOW_ITEM(AH, sizeof(struct rte_flow_item_ah)),
 	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
+	MK_FLOW_ITEM(L2TPV3, sizeof(struct rte_flow_item_l2tpv3)),
 };
 
 /** Generate flow_action[] entry. */
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 452d359a1..5ee055c28 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -510,6 +510,16 @@ enum rte_flow_item_type {
 	 * See struct rte_flow_item_tag.
 	 */
 	RTE_FLOW_ITEM_TYPE_TAG,
+
+	/*
+	 * Matches a L2TPv3 header.
+	 *
+	 * Configure flow for L2TPv3 packets.
+	 *
+	 * See struct rte_flow_item_l2tpv3.
+	 */
+	RTE_FLOW_ITEM_TYPE_L2TPV3,
+
 };
 
 /**
@@ -1373,6 +1383,23 @@ static const struct rte_flow_item_tag rte_flow_item_tag_mask = {
 };
 #endif
 
+/**
+ * RTE_FLOW_ITEM_TYPE_L2TPV3.
+ *
+ * Matches a L2TPv3 header.
+ */
+struct rte_flow_item_l2tpv3 {
+	rte_be32_t session_id; /**< Session ID. */
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_L2TPV3. */
+#ifndef __cplusplus
+static const struct rte_flow_item_l2tpv3 rte_flow_item_l2tpv3_mask = {
+	.session_id = RTE_BE32(UINT32_MAX),
+};
+#endif
+
+
 /**
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
-- 
2.17.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[dpdk-dev] [PATCH] net/i40e: Add new customized pctype for l2tpv3

[Rory Sexton]

Signed-off-by: Rory Sexton <rory.sexton@intel.com>
Signed-off-by: Dariusz Jagus <dariuszx.jagus@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 11 ++++++-
 drivers/net/i40e/i40e_ethdev.h |  9 ++++++
 drivers/net/i40e/i40e_fdir.c   | 34 +++++++++++++++++----
 drivers/net/i40e/i40e_flow.c   | 55 ++++++++++++++++++++++++++++++++++
 4 files changed, 103 insertions(+), 6 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5999c964b..80a46916c 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12351,6 +12351,14 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			new_pctype =
 				i40e_find_customized_pctype(pf,
 						      I40E_CUSTOMIZED_GTPU);
+		else if (!strcmp(name, "IPV4_L2TPV3"))
+			new_pctype =
+				i40e_find_customized_pctype(pf,
+						I40E_CUSTOMIZED_IPV4_L2TPV3);
+		else if (!strcmp(name, "IPV6_L2TPV3"))
+			new_pctype =
+				i40e_find_customized_pctype(pf,
+						I40E_CUSTOMIZED_IPV6_L2TPV3);
 		if (new_pctype) {
 			if (op == RTE_PMD_I40E_PKG_OP_WR_ADD) {
 				new_pctype->pctype = pctype_value;
@@ -12544,7 +12552,8 @@ i40e_update_customized_ptype(struct rte_eth_dev *dev, uint8_t *pkg,
 						RTE_PTYPE_TUNNEL_GRENAT;
 					in_tunnel = true;
 				} else if (!strncasecmp(name, "L2TPV2CTL", 9) ||
-					   !strncasecmp(name, "L2TPV2", 6)) {
+					   !strncasecmp(name, "L2TPV2", 6) ||
+					   !strncasecmp(name, "L2TPV3", 6)) {
 					ptype_mapping[i].sw_ptype |=
 						RTE_PTYPE_TUNNEL_L2TP;
 					in_tunnel = true;
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 295ad593b..569a5a1e5 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -508,6 +508,11 @@ struct i40e_raw_flow {
 	uint32_t length;
 };
 
+/* A structure used to define the input for L2TPv3 flow */
+struct i40e_l2tpv3_flow {
+	uint32_t session_id; /* Session ID in big endian. */
+};
+
 /*
  * A union contains the inputs for all types of flow
  * items in flows need to be in big endian
@@ -526,6 +531,7 @@ union i40e_fdir_flow {
 	struct i40e_gtp_ipv4_flow  gtp_ipv4_flow;
 	struct i40e_gtp_ipv6_flow  gtp_ipv6_flow;
 	struct i40e_raw_flow       raw_flow;
+	struct i40e_l2tpv3_flow    l2tpv3_flow;
 };
 
 enum i40e_fdir_ip_type {
@@ -542,6 +548,7 @@ struct i40e_fdir_flow_ext {
 	uint16_t dst_id; /* VF ID, available when is_vf is 1*/
 	bool inner_ip;   /* If there is inner ip */
 	enum i40e_fdir_ip_type iip_type; /* ip type for inner ip */
+	enum i40e_fdir_ip_type oip_type; /* ip type for outer ip */
 	bool customized_pctype; /* If customized pctype is used */
 	bool pkt_template; /* If raw packet template is used */
 };
@@ -897,6 +904,8 @@ enum i40e_new_pctype {
 	I40E_CUSTOMIZED_GTPU_IPV4,
 	I40E_CUSTOMIZED_GTPU_IPV6,
 	I40E_CUSTOMIZED_GTPU,
+	I40E_CUSTOMIZED_IPV4_L2TPV3,
+	I40E_CUSTOMIZED_IPV6_L2TPV3,
 	I40E_CUSTOMIZED_MAX,
 };
 
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index dee007daa..b18301eec 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -33,6 +33,10 @@
 #define IPV6_ADDR_LEN              16
 #endif
 
+#ifndef IPPROTO_L2TP
+#define IPPROTO_L2TP		  115
+#endif
+
 #define I40E_FDIR_PKT_LEN                   512
 #define I40E_FDIR_IP_DEFAULT_LEN            420
 #define I40E_FDIR_IP_DEFAULT_TTL            0x40
@@ -1026,7 +1030,12 @@ i40e_flow_fdir_fill_eth_ip_head(struct i40e_pf *pf,
 		 pctype == I40E_FILTER_PCTYPE_NONF_IPV4_SCTP ||
 		 pctype == I40E_FILTER_PCTYPE_NONF_IPV4_OTHER ||
 		 pctype == I40E_FILTER_PCTYPE_FRAG_IPV4 ||
-		 is_customized_pctype) {
+		 ((is_customized_pctype) &&
+		  ((cus_pctype->index == I40E_CUSTOMIZED_GTPC) ||
+		   (cus_pctype->index == I40E_CUSTOMIZED_GTPU_IPV4) ||
+		   (cus_pctype->index == I40E_CUSTOMIZED_GTPU_IPV6) ||
+		   (cus_pctype->index == I40E_CUSTOMIZED_GTPU) ||
+		   (cus_pctype->index == I40E_CUSTOMIZED_IPV4_L2TPV3)))) {
 		ip = (struct rte_ipv4_hdr *)raw_pkt;
 
 		*ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
@@ -1054,12 +1063,16 @@ i40e_flow_fdir_fill_eth_ip_head(struct i40e_pf *pf,
 			 cus_pctype->index == I40E_CUSTOMIZED_GTPU_IPV6 ||
 			 cus_pctype->index == I40E_CUSTOMIZED_GTPU)
 			ip->next_proto_id = IPPROTO_UDP;
+		else if (cus_pctype->index == I40E_CUSTOMIZED_IPV4_L2TPV3)
+			ip->next_proto_id = IPPROTO_L2TP;
 		len += sizeof(struct rte_ipv4_hdr);
 	} else if (pctype == I40E_FILTER_PCTYPE_NONF_IPV6_TCP ||
 		   pctype == I40E_FILTER_PCTYPE_NONF_IPV6_UDP ||
 		   pctype == I40E_FILTER_PCTYPE_NONF_IPV6_SCTP ||
 		   pctype == I40E_FILTER_PCTYPE_NONF_IPV6_OTHER ||
-		   pctype == I40E_FILTER_PCTYPE_FRAG_IPV6) {
+		   pctype == I40E_FILTER_PCTYPE_FRAG_IPV6 ||
+		   ((is_customized_pctype) &&
+		    (cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3))) {
 		ip6 = (struct rte_ipv6_hdr *)raw_pkt;
 
 		*ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV6);
@@ -1069,9 +1082,12 @@ i40e_flow_fdir_fill_eth_ip_head(struct i40e_pf *pf,
 					  I40E_FDIR_IPv6_TC_OFFSET));
 		ip6->payload_len =
 			rte_cpu_to_be_16(I40E_FDIR_IPv6_PAYLOAD_LEN);
-		ip6->proto = fdir_input->flow.ipv6_flow.proto ?
-			fdir_input->flow.ipv6_flow.proto :
-			next_proto[fdir_input->pctype];
+		if (!is_customized_pctype)
+			ip6->proto = fdir_input->flow.ipv6_flow.proto ?
+				fdir_input->flow.ipv6_flow.proto :
+				next_proto[fdir_input->pctype];
+		else if (cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3)
+			ip6->proto = IPPROTO_L2TP;
 		ip6->hop_limits = fdir_input->flow.ipv6_flow.hop_limits ?
 			fdir_input->flow.ipv6_flow.hop_limits :
 			I40E_FDIR_IPv6_DEFAULT_HOP_LIMITS;
@@ -1115,6 +1131,7 @@ i40e_flow_fdir_construct_pkt(struct i40e_pf *pf,
 	struct rte_flow_item_gtp *gtp;
 	struct rte_ipv4_hdr *gtp_ipv4;
 	struct rte_ipv6_hdr *gtp_ipv6;
+	struct rte_flow_item_l2tpv3 *l2tpv3;
 	uint8_t size, dst = 0;
 	uint8_t i, pit_idx, set_idx = I40E_FLXPLD_L4_IDX; /* use l4 by default*/
 	int len;
@@ -1285,6 +1302,13 @@ i40e_flow_fdir_construct_pkt(struct i40e_pf *pf,
 			} else
 				payload = (unsigned char *)gtp +
 					sizeof(struct rte_flow_item_gtp);
+		} else if (cus_pctype->index == I40E_CUSTOMIZED_IPV4_L2TPV3 ||
+			   cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3) {
+			l2tpv3 = (struct rte_flow_item_l2tpv3 *)(raw_pkt + len);
+			l2tpv3->session_id =
+				fdir_input->flow.l2tpv3_flow.session_id;
+			payload = (unsigned char *)l2tpv3 +
+				sizeof(struct rte_flow_item_l2tpv3);
 		}
 	} else {
 		PMD_DRV_LOG(ERR, "unknown pctype %u.",
diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index 61021037c..2a5d1d0b6 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -1615,6 +1615,20 @@ static enum rte_flow_item_type pattern_qinq_1[] = {
 	RTE_FLOW_ITEM_TYPE_END,
 };
 
+static enum rte_flow_item_type pattern_fdir_ipv4_l2tpv3[] = {
+	RTE_FLOW_ITEM_TYPE_ETH,
+	RTE_FLOW_ITEM_TYPE_IPV4,
+	RTE_FLOW_ITEM_TYPE_L2TPV3,
+	RTE_FLOW_ITEM_TYPE_END,
+};
+
+static enum rte_flow_item_type pattern_fdir_ipv6_l2tpv3[] = {
+	RTE_FLOW_ITEM_TYPE_ETH,
+	RTE_FLOW_ITEM_TYPE_IPV6,
+	RTE_FLOW_ITEM_TYPE_L2TPV3,
+	RTE_FLOW_ITEM_TYPE_END,
+};
+
 static struct i40e_valid_pattern i40e_supported_patterns[] = {
 	/* Ethertype */
 	{ pattern_ethertype, i40e_flow_parse_ethertype_filter },
@@ -1795,6 +1809,9 @@ static struct i40e_valid_pattern i40e_supported_patterns[] = {
 	{ pattern_fdir_ipv6_gtpu, i40e_flow_parse_gtp_filter },
 	/* QINQ */
 	{ pattern_qinq_1, i40e_flow_parse_qinq_filter },
+	/* L2TPv3 */
+	{ pattern_fdir_ipv4_l2tpv3, i40e_flow_parse_fdir_filter },
+	{ pattern_fdir_ipv6_l2tpv3, i40e_flow_parse_fdir_filter },
 };
 
 #define NEXT_ITEM_OF_ACTION(act, actions, index)                        \
@@ -2420,6 +2437,15 @@ i40e_flow_fdir_get_pctype_value(struct i40e_pf *pf,
 			cus_pctype = i40e_find_customized_pctype(pf,
 						 I40E_CUSTOMIZED_GTPU_IPV6);
 		break;
+	case RTE_FLOW_ITEM_TYPE_L2TPV3:
+		if (filter->input.flow_ext.oip_type == I40E_FDIR_IPTYPE_IPV4)
+			cus_pctype = i40e_find_customized_pctype(pf,
+						I40E_CUSTOMIZED_IPV4_L2TPV3);
+		else if (filter->input.flow_ext.oip_type ==
+			 I40E_FDIR_IPTYPE_IPV6)
+			cus_pctype = i40e_find_customized_pctype(pf,
+						I40E_CUSTOMIZED_IPV6_L2TPV3);
+		break;
 	default:
 		PMD_DRV_LOG(ERR, "Unsupported item type");
 		break;
@@ -2461,6 +2487,7 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
 	const struct rte_flow_item_gtp *gtp_spec, *gtp_mask;
 	const struct rte_flow_item_raw *raw_spec, *raw_mask;
 	const struct rte_flow_item_vf *vf_spec;
+	const struct rte_flow_item_l2tpv3 *l2tpv3_spec, *l2tpv3_mask;
 
 	uint8_t pctype = 0;
 	uint64_t input_set = I40E_INSET_NONE;
@@ -3012,6 +3039,34 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
 				return -rte_errno;
 			}
 			break;
+		case RTE_FLOW_ITEM_TYPE_L2TPV3:
+			l2tpv3_spec = item->spec;
+			l2tpv3_mask = item->mask;
+
+			if (!l2tpv3_spec || !l2tpv3_mask)
+				break;
+
+			if (l2tpv3_mask->session_id != UINT32_MAX) {
+				rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ITEM,
+					item,
+					"Invalid L2TPv3 mask");
+				return -rte_errno;
+			}
+
+			filter->input.flow.l2tpv3_flow.session_id =
+				l2tpv3_spec->session_id;
+
+			if (l3 == RTE_FLOW_ITEM_TYPE_IPV4)
+				filter->input.flow_ext.oip_type =
+					I40E_FDIR_IPTYPE_IPV4;
+			else if (l3 == RTE_FLOW_ITEM_TYPE_IPV6)
+				filter->input.flow_ext.oip_type =
+					I40E_FDIR_IPTYPE_IPV6;
+
+			filter->input.flow_ext.customized_pctype = true;
+			cus_proto = item_type;
+			break;
 		default:
 			break;
 		}
-- 
2.17.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

[Ori Kam]

Hi Rory,

One general question why do we want to support only v3 and not also v2?

PSB for some other comments.

Best,
Ori Kam

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Rory Sexton
> Sent: Wednesday, December 4, 2019 4:11 PM
> To: dev@dpdk.org
> Cc: qi.z.zhang@intel.com; beilei.xing@intel.com; rory.sexton@intel.com;
> Adrien Mazarguil <adrien.mazarguil@6wind.com>; Dariusz Jagus
> <dariuszx.jagus@intel.com>
> Subject: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API
> 
> - RTE_FLOW_ITEM_TYPE_L2TPV3: matches a L2TPv3 header
> 
> Signed-off-by: Rory Sexton <rory.sexton@intel.com>
> Signed-off-by: Dariusz Jagus <dariuszx.jagus@intel.com>
> ---
>  app/test-pmd/cmdline_flow.c                 | 28 +++++++++++++++++++++
>  doc/guides/prog_guide/rte_flow.rst          |  8 ++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 +++
>  lib/librte_ethdev/rte_flow.c                |  1 +
>  lib/librte_ethdev/rte_flow.h                | 27 ++++++++++++++++++++
>  5 files changed, 68 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 99dade7d8..a7fe7a7a8 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -213,6 +213,8 @@ enum index {
>  	ITEM_TAG,
>  	ITEM_TAG_DATA,
>  	ITEM_TAG_INDEX,
> +	ITEM_L2TPV3,
> +	ITEM_L2TPV3_SESSION_ID,
> 
>  	/* Validate/create actions. */
>  	ACTIONS,
> @@ -746,6 +748,7 @@ static const enum index next_item[] = {
>  	ITEM_PPPOE_PROTO_ID,
>  	ITEM_HIGIG2,
>  	ITEM_TAG,
> +	ITEM_L2TPV3,
>  	END_SET,
>  	ZERO,
>  };
> @@ -1030,6 +1033,12 @@ static const enum index item_tag[] = {
>  	ZERO,
>  };
> 
> +static const enum index item_l2tpv3[] = {
> +	ITEM_L2TPV3_SESSION_ID,
> +	ITEM_NEXT,
> +	ZERO,
> +};
> +
>  static const enum index next_action[] = {
>  	ACTION_END,
>  	ACTION_VOID,
> @@ -2593,6 +2602,21 @@ static const struct token token_list[] = {
>  			     NEXT_ENTRY(ITEM_PARAM_IS)),
>  		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_tag,
> index)),
>  	},
> +	[ITEM_L2TPV3] = {
> +		.name = "l2tpv3",
> +		.help = "match L2TPv3 header",
> +		.priv = PRIV_ITEM(L2TPV3, sizeof(struct
> rte_flow_item_l2tpv3)),
> +		.next = NEXT(item_l2tpv3),
> +		.call = parse_vc,
> +	},
> +	[ITEM_L2TPV3_SESSION_ID] = {
> +		.name = "session_id",
> +		.help = "session identifier",
> +		.next = NEXT(item_l2tpv3, NEXT_ENTRY(UNSIGNED),
> item_param),
> +		.args = ARGS(ARGS_ENTRY_HTON(struct
> rte_flow_item_l2tpv3,
> +					     session_id)),
> +	},
> +
>  	/* Validate/create actions. */
>  	[ACTIONS] = {
>  		.name = "actions",
> @@ -6238,6 +6262,10 @@ flow_item_default_mask(const struct
> rte_flow_item *item)
>  		break;
>  	case RTE_FLOW_ITEM_TYPE_PPPOE_PROTO_ID:
>  		mask = &rte_flow_item_pppoe_proto_id_mask;
> +		break;
> +	case RTE_FLOW_ITEM_TYPE_L2TPV3:
> +		mask = &rte_flow_item_l2tpv3_mask;
> +		break;
>  	default:
>  		break;
>  	}
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index a254c81ef..c7ddb38c5 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1336,6 +1336,14 @@ Broadcom switches.
> 
>  - Default ``mask`` matches classification and vlan.
> 
> +Item: ``L2TPV3``
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Matches a L2TPv3 header.
> +
> +- ``session_id``: L2TPv3 session identifier.
> +- Default ``mask`` matches session_id only.
> +
> 
>  Actions
>  ~~~~~~~
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 73ef0b41d..a48e77619 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3954,6 +3954,10 @@ This section lists supported pattern items and their
> attributes, if any.
> 
>    - ``proto_id {unsigned}``: PPP protocol identifier.
> 
> +- ``l2tpv3``: match L2TPv3 header.
> +
> +  - ``session_id {unsigned}``: L2TPv3 session identifier.
> +
>  Actions list
>  ^^^^^^^^^^^^
> 
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 87a3e8c4c..fcda73320 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -93,6 +93,7 @@ static const struct rte_flow_desc_data
> rte_flow_desc_item[] = {
>  	MK_FLOW_ITEM(IGMP, sizeof(struct rte_flow_item_igmp)),
>  	MK_FLOW_ITEM(AH, sizeof(struct rte_flow_item_ah)),
>  	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
> +	MK_FLOW_ITEM(L2TPV3, sizeof(struct rte_flow_item_l2tpv3)),
>  };
> 
>  /** Generate flow_action[] entry. */
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 452d359a1..5ee055c28 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -510,6 +510,16 @@ enum rte_flow_item_type {
>  	 * See struct rte_flow_item_tag.
>  	 */
>  	RTE_FLOW_ITEM_TYPE_TAG,
> +
> +	/*

Missing *. It should be /**

> +	 * Matches a L2TPv3 header.
> +	 *
> +	 * Configure flow for L2TPv3 packets.
> +	 *
> +	 * See struct rte_flow_item_l2tpv3.
> +	 */
> +	RTE_FLOW_ITEM_TYPE_L2TPV3,
> +
>  };
> 
>  /**
> @@ -1373,6 +1383,23 @@ static const struct rte_flow_item_tag
> rte_flow_item_tag_mask = {
>  };
>  #endif
> 
> +/**
> + * RTE_FLOW_ITEM_TYPE_L2TPV3.
> + *
> + * Matches a L2TPv3 header.
> + */
> +struct rte_flow_item_l2tpv3 {
> +	rte_be32_t session_id; /**< Session ID. */
> +};

Does it make sense that in future we will want to match on the 
T / L / ver / Ns / Nr?

Please also consider that any change will be ABI / API breakage, which will be allowed
only next year.

> +
> +/** Default mask for RTE_FLOW_ITEM_TYPE_L2TPV3. */
> +#ifndef __cplusplus
> +static const struct rte_flow_item_l2tpv3 rte_flow_item_l2tpv3_mask = {
> +	.session_id = RTE_BE32(UINT32_MAX),
> +};
> +#endif
> +
> +
>  /**
>   * @warning
>   * @b EXPERIMENTAL: this structure may change without prior notice
> --
> 2.17.1
> 
> --------------------------------------------------------------
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
> 
> 
> This e-mail and any attachments may contain confidential material for the
> sole
> use of the intended recipient(s). Any review or distribution by others is
> strictly prohibited. If you are not the intended recipient, please contact the
> sender and delete all copies.

Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

[Sexton, Rory]

Hi Ori,

> One general question why do we want to support only v3 and not also v2?
l2tpv3 is more widely used as a tunneling protocol hence it's inclusion.
A specific example is the cable industry where DOCSIS cable traffic is encapsulated using depi and uepi protocols which both make use of l2tpv3 session ids.
Directing flows based on l2tpv3 can be very useful in these cases.

Some more comments inline below.

Rory

>> diff --git a/lib/librte_ethdev/rte_flow.h 
>> b/lib/librte_ethdev/rte_flow.h index 452d359a1..5ee055c28 100644
>> --- a/lib/librte_ethdev/rte_flow.h
>> +++ b/lib/librte_ethdev/rte_flow.h
>> @@ -510,6 +510,16 @@ enum rte_flow_item_type {
>>  	 * See struct rte_flow_item_tag.
>>  	 */
>>  	RTE_FLOW_ITEM_TYPE_TAG,
>> +
>> +	/*
>
>Missing *. It should be /**
>

Will correct this in another version of this patch.

>> +/**
>> + * RTE_FLOW_ITEM_TYPE_L2TPV3.
>> + *
>> + * Matches a L2TPv3 header.
>> + */
>> +struct rte_flow_item_l2tpv3 {
>> +	rte_be32_t session_id; /**< Session ID. */ };
>
>Does it make sense that in future we will want to match on the T / L / ver / Ns / Nr?
>
>Please also consider that any change will be ABI / API breakage, which will be allowed only next year. 
>

I don't foresee us wanting to match on any of the other fields, T / L / ver / Ns/ Nr.
The session id would typically be the only field of interest in the l2tpv3 header.

Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

[Ori Kam]

Hi Rory,

> -----Original Message-----
> From: Sexton, Rory <rory.sexton@intel.com>
> Sent: Tuesday, December 10, 2019 4:52 PM
> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Adrien Mazarguil <adrien.mazarguil@6wind.com>; Jagus, DariuszX
> <dariuszx.jagus@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API
> 
> Hi Ori,
> 
> > One general question why do we want to support only v3 and not also v2?
> l2tpv3 is more widely used as a tunneling protocol hence it's inclusion.
> A specific example is the cable industry where DOCSIS cable traffic is
> encapsulated using depi and uepi protocols which both make use of l2tpv3
> session ids.
> Directing flows based on l2tpv3 can be very useful in these cases.
> 

I'm not saying that v3 is not used more, I just thought from looking at the spec
that both can be supported and the only difference is the version, so matching on
the version we will be able to support both versions.
Your decision.

> Some more comments inline below.
> 
> Rory
> 
> >> diff --git a/lib/librte_ethdev/rte_flow.h
> >> b/lib/librte_ethdev/rte_flow.h index 452d359a1..5ee055c28 100644
> >> --- a/lib/librte_ethdev/rte_flow.h
> >> +++ b/lib/librte_ethdev/rte_flow.h
> >> @@ -510,6 +510,16 @@ enum rte_flow_item_type {
> >>  	 * See struct rte_flow_item_tag.
> >>  	 */
> >>  	RTE_FLOW_ITEM_TYPE_TAG,
> >> +
> >> +	/*
> >
> >Missing *. It should be /**
> >
> 
> Will correct this in another version of this patch.
> 
> >> +/**
> >> + * RTE_FLOW_ITEM_TYPE_L2TPV3.
> >> + *
> >> + * Matches a L2TPv3 header.
> >> + */
> >> +struct rte_flow_item_l2tpv3 {
> >> +	rte_be32_t session_id; /**< Session ID. */ };
> >
> >Does it make sense that in future we will want to match on the T / L / ver /
> Ns / Nr?
> >
> >Please also consider that any change will be ABI / API breakage, which will
> be allowed only next year.
> >
> 
> I don't foresee us wanting to match on any of the other fields, T / L / ver /
> Ns/ Nr.
> The session id would typically be the only field of interest in the l2tpv3
> header.

I think that adding all fields to the structure will solve the possible issue with adding matching later.
Also and even more important you will be able to use it for creating the  raw_encap / raw_decap buffers.

What do you think?

Best,
Ori

Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

[Sexton, Rory]

Hi Ori,

See my comments below.

Regards,
Rory

>> 
>>> One general question why do we want to support only v3 and not also v2?
>> l2tpv3 is more widely used as a tunneling protocol hence it's inclusion.
>> A specific example is the cable industry where DOCSIS cable traffic is 
>> encapsulated using depi and uepi protocols which both make use of 
>> l2tpv3 session ids.
>> Directing flows based on l2tpv3 can be very useful in these cases.
>> 
>
>I'm not saying that v3 is not used more, I just thought from looking at the spec that both can be supported and the only difference is the version, so matching on the version we will be able to support both versions.
>Your decision.
>

There is more difference between l2tpv2 and l2tpv3 than just the version number.
In L2TPv2 there exists a 16 bit Tunnel ID and 16 bit Session ID. This is changed to a 32 bit Session ID in L2TPv3
Please see difference in headers below for v2 and v3.
Due to the differences between v2 and v3 I don't think both can be supported with same flow item.

                                                          L2TPv2
    0...............................................15, 16......................................31
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |T|L|x|x|S|x|O|P|x|x|x|x|  Ver  |          Length (opt)                  |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |           Tunnel ID                               |           Session ID                     |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |             Ns (opt)                               |             Nr (opt)                        |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |      Offset Size (opt)                        |    Offset pad... (opt)               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                                                          L2TPv3
    0...............................................15, 16......................................31
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                           Session ID                                                                  |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |               Cookie (optional, maximum 64 bits)...
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
                                                                                                                    |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>> >> +/**
>> >> + * RTE_FLOW_ITEM_TYPE_L2TPV3.
>> >> + *
>> >> + * Matches a L2TPv3 header.
>> >> + */
>> >> +struct rte_flow_item_l2tpv3 {
>> >> +	rte_be32_t session_id; /**< Session ID. */ };
>> >
>> >Does it make sense that in future we will want to match on the T / L 
>> >/ ver /
>> Ns / Nr?
>> >
>> >Please also consider that any change will be ABI / API breakage, 
>> >which will
>> be allowed only next year.
>> >
>> 
>> I don't foresee us wanting to match on any of the other fields, T / L 
>> / ver / Ns/ Nr.
>> The session id would typically be the only field of interest in the 
>> l2tpv3 header.
>
> I think that adding all fields to the structure will solve the possible issue with adding matching later.
> Also and even more important you will be able to use it for creating the  raw_encap / raw_decap buffers.
>
>What do you think?

Based on the differences between v2 and v3 the only field of interest in l2tpv3 over IP is the Session ID.
I agree it would make sense to add all fields if we were implementing l2tpv2 however v2 would require a different implementation to v3 due to the differences between both Protocols.

Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

[Ori Kam]

Hi Rory,

PSB

> -----Original Message-----
> From: Sexton, Rory <rory.sexton@intel.com>
> Sent: Wednesday, December 11, 2019 1:36 PM
> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Adrien Mazarguil <adrien.mazarguil@6wind.com>; Jagus, DariuszX
> <dariuszx.jagus@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API
> 
> Hi Ori,
> 
> See my comments below.
> 
> Regards,
> Rory
> 
> >>
> >>> One general question why do we want to support only v3 and not also
> v2?
> >> l2tpv3 is more widely used as a tunneling protocol hence it's inclusion.
> >> A specific example is the cable industry where DOCSIS cable traffic is
> >> encapsulated using depi and uepi protocols which both make use of
> >> l2tpv3 session ids.
> >> Directing flows based on l2tpv3 can be very useful in these cases.
> >>
> >
> >I'm not saying that v3 is not used more, I just thought from looking at the
> spec that both can be supported and the only difference is the version, so
> matching on the version we will be able to support both versions.
> >Your decision.
> >
> 
> There is more difference between l2tpv2 and l2tpv3 than just the version
> number.
> In L2TPv2 there exists a 16 bit Tunnel ID and 16 bit Session ID. This is changed
> to a 32 bit Session ID in L2TPv3
> Please see difference in headers below for v2 and v3.
> Due to the differences between v2 and v3 I don't think both can be
> supported with same flow item.
> 
>                                                           L2TPv2
>     0...............................................15, 16......................................31
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |T|L|x|x|S|x|O|P|x|x|x|x|  Ver  |          Length (opt)                  |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |           Tunnel ID                               |           Session ID                     |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |             Ns (opt)                               |             Nr (opt)                        |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |      Offset Size (opt)                        |    Offset pad... (opt)               |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>                                                           L2TPv3
>     0...............................................15, 16......................................31
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |                           Session ID                                                                  |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |               Cookie (optional, maximum 64 bits)...
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>                                                                                                                     |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 

Since we have the version, they can by using union,
But like I said your call.

> >> >> +/**
> >> >> + * RTE_FLOW_ITEM_TYPE_L2TPV3.
> >> >> + *
> >> >> + * Matches a L2TPv3 header.
> >> >> + */
> >> >> +struct rte_flow_item_l2tpv3 {
> >> >> +	rte_be32_t session_id; /**< Session ID. */ };
> >> >
> >> >Does it make sense that in future we will want to match on the T / L
> >> >/ ver /
> >> Ns / Nr?
> >> >
> >> >Please also consider that any change will be ABI / API breakage,
> >> >which will
> >> be allowed only next year.
> >> >
> >>
> >> I don't foresee us wanting to match on any of the other fields, T / L
> >> / ver / Ns/ Nr.
> >> The session id would typically be the only field of interest in the
> >> l2tpv3 header.
> >
> > I think that adding all fields to the structure will solve the possible issue
> with adding matching later.
> > Also and even more important you will be able to use it for creating the
> raw_encap / raw_decap buffers.
> >
> >What do you think?
> 
> Based on the differences between v2 and v3 the only field of interest in
> l2tpv3 over IP is the Session ID.
> I agree it would make sense to add all fields if we were implementing l2tpv2
> however v2 would require a different implementation to v3 due to the
> differences between both Protocols.

Even if we just support v3, I think that it is a good idea to add all fields of v3.
This will allow the use of the raw_encap / raw_decap actions.
Please also note that you didn't add the new item to  cmd_set_raw_parsed function.
this function is used in order to create raw_encap/ raw_decap encapsulations.

Best,
Ori

Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

[Sexton, Rory]

Hi Ori,

See comments below.

Regards,
Rory

> > > > > > +/**
> > > > > > + * RTE_FLOW_ITEM_TYPE_L2TPV3.
> > > > > > + *
> > > > > > + * Matches a L2TPv3 header.
> > > > > > + */
> > > > > > +struct rte_flow_item_l2tpv3 {
> > > > > > +	rte_be32_t session_id; /**< Session ID. */ };
> > > > >
> > > > >Does it make sense that in future we will want to match on the T / 
> > > > >L / ver /
> > > > Ns / Nr?
> > > > >
> > > > >Please also consider that any change will be ABI / API breakage, 
> > > > >which will
> > > > >be allowed only next year.
> > > > >
> > > >
> > > > I don't foresee us wanting to match on any of the other fields, T / 
> > > > L / ver / Ns/ Nr.
> > > > The session id would typically be the only field of interest in the
> > > > l2tpv3 header.
> > >
> > > I think that adding all fields to the structure will solve the 
> > > possible issue
> > with adding matching later.
> > > Also and even more important you will be able to use it for creating 
> > > the
> > raw_encap / raw_decap buffers.
> > >
> > >What do you think?
> > 
> > Based on the differences between v2 and v3 the only field of interest 
> > in
> > l2tpv3 over IP is the Session ID.
> > I agree it would make sense to add all fields if we were implementing 
> > l2tpv2 however v2 would require a different implementation to v3 due 
> > to the differences between both Protocols.
>
> Even if we just support v3, I think that it is a good idea to add all fields of v3. 
> This will allow the use of the raw_encap / raw_decap actions.
> Please also note that you didn't add the new item to  cmd_set_raw_parsed function.
> this function is used in order to create raw_encap/ raw_decap encapsulations.

I think the confusion here is based on the fact that there are 2 separate types of l2tpv3.
- l2tpv3 over UDP, which is very similar to l2tpv2 with only change being 16 bit Tunnel ID 
  and 16 bit Session ID changing to 32 bit Session ID. All other fields remain (T/L/Ver/Ns/Nr).
- l2tpv3 over IP is another type of l2tpv3 that is carried over IP rather than UDP and as such
  the message format is entirely different and consists of just a 32 bit Session ID. The other
  fields mentioned above do not exist at all in this l2tpv3 header.

This patch was targeted at creating a flow to handle l2tpv3 over IP exclusively. This is why
the Session ID is the only field in the flow item.

I can add the item to cmd_set_raw_parsed function.

Re: [dpdk-dev] [PATCH] net/i40e: Add new customized pctype for l2tpv3

[Xing, Beilei]

> -----Original Message-----
> From: Sexton, Rory <rory.sexton@intel.com>
> Sent: Wednesday, December 4, 2019 6:11 AM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Sexton, Rory <rory.sexton@intel.com>; adrien.mazarguil@6wind.com; Jagus,
> DariuszX <dariuszx.jagus@intel.com>
> Subject: [PATCH] net/i40e: Add new customized pctype for l2tpv3
It's not only add new customized pctype, but mainly enable FDIR for l2ipv3, so how about " net/i40e: support FDIR for L2TPv3"?

Detailed commit log is also needed.

> 
> Signed-off-by: Rory Sexton <rory.sexton@intel.com>
> Signed-off-by: Dariusz Jagus <dariuszx.jagus@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 11 ++++++-
> drivers/net/i40e/i40e_ethdev.h |  9 ++++++
>  drivers/net/i40e/i40e_fdir.c   | 34 +++++++++++++++++----
>  drivers/net/i40e/i40e_flow.c   | 55
> ++++++++++++++++++++++++++++++++++
>  4 files changed, 103 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5999c964b..80a46916c 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -12351,6 +12351,14 @@ i40e_update_customized_pctype(struct
> rte_eth_dev *dev, uint8_t *pkg,
>  			new_pctype =
>  				i40e_find_customized_pctype(pf,
>  						      I40E_CUSTOMIZED_GTPU);
> +		else if (!strcmp(name, "IPV4_L2TPV3"))
> +			new_pctype =
> +				i40e_find_customized_pctype(pf,
> +						I40E_CUSTOMIZED_IPV4_L2TPV3);
> +		else if (!strcmp(name, "IPV6_L2TPV3"))
> +			new_pctype =
> +				i40e_find_customized_pctype(pf,
> +						I40E_CUSTOMIZED_IPV6_L2TPV3);
>  		if (new_pctype) {
>  			if (op == RTE_PMD_I40E_PKG_OP_WR_ADD) {
>  				new_pctype->pctype = pctype_value;
> @@ -12544,7 +12552,8 @@ i40e_update_customized_ptype(struct
> rte_eth_dev *dev, uint8_t *pkg,
>  						RTE_PTYPE_TUNNEL_GRENAT;
>  					in_tunnel = true;
>  				} else if (!strncasecmp(name, "L2TPV2CTL", 9) ||
> -					   !strncasecmp(name, "L2TPV2", 6)) {
> +					   !strncasecmp(name, "L2TPV2", 6) ||
> +					   !strncasecmp(name, "L2TPV3", 6)) {
>  					ptype_mapping[i].sw_ptype |=
>  						RTE_PTYPE_TUNNEL_L2TP;
>  					in_tunnel = true;
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index 295ad593b..569a5a1e5 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -508,6 +508,11 @@ struct i40e_raw_flow {
>  	uint32_t length;
>  };
> 
> +/* A structure used to define the input for L2TPv3 flow */ struct
> +i40e_l2tpv3_flow {

Seems missed struct rte_eth_ipv4_flow or struct rte_eth_ipv6_flow here?

> +	uint32_t session_id; /* Session ID in big endian. */ };
> +
>  /*
>   * A union contains the inputs for all types of flow
>   * items in flows need to be in big endian @@ -526,6 +531,7 @@ union
> i40e_fdir_flow {
>  	struct i40e_gtp_ipv4_flow  gtp_ipv4_flow;
>  	struct i40e_gtp_ipv6_flow  gtp_ipv6_flow;
>  	struct i40e_raw_flow       raw_flow;
> +	struct i40e_l2tpv3_flow    l2tpv3_flow;
>  };
> 
>  enum i40e_fdir_ip_type {
> @@ -542,6 +548,7 @@ struct i40e_fdir_flow_ext {
>  	uint16_t dst_id; /* VF ID, available when is_vf is 1*/
>  	bool inner_ip;   /* If there is inner ip */
>  	enum i40e_fdir_ip_type iip_type; /* ip type for inner ip */
> +	enum i40e_fdir_ip_type oip_type; /* ip type for outer ip */
>  	bool customized_pctype; /* If customized pctype is used */
>  	bool pkt_template; /* If raw packet template is used */  }; @@ -897,6
> +904,8 @@ enum i40e_new_pctype {
>  	I40E_CUSTOMIZED_GTPU_IPV4,
>  	I40E_CUSTOMIZED_GTPU_IPV6,
>  	I40E_CUSTOMIZED_GTPU,
> +	I40E_CUSTOMIZED_IPV4_L2TPV3,
> +	I40E_CUSTOMIZED_IPV6_L2TPV3,
>  	I40E_CUSTOMIZED_MAX,
>  };
> 
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c index
> dee007daa..b18301eec 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -33,6 +33,10 @@
>  #define IPV6_ADDR_LEN              16
>  #endif
> 
> +#ifndef IPPROTO_L2TP
> +#define IPPROTO_L2TP		  115
> +#endif
> +
>  #define I40E_FDIR_PKT_LEN                   512
>  #define I40E_FDIR_IP_DEFAULT_LEN            420
>  #define I40E_FDIR_IP_DEFAULT_TTL            0x40
> @@ -1026,7 +1030,12 @@ i40e_flow_fdir_fill_eth_ip_head(struct i40e_pf
> *pf,
>  		 pctype == I40E_FILTER_PCTYPE_NONF_IPV4_SCTP ||
>  		 pctype == I40E_FILTER_PCTYPE_NONF_IPV4_OTHER ||
>  		 pctype == I40E_FILTER_PCTYPE_FRAG_IPV4 ||
> -		 is_customized_pctype) {
> +		 ((is_customized_pctype) &&
> +		  ((cus_pctype->index == I40E_CUSTOMIZED_GTPC) ||
> +		   (cus_pctype->index == I40E_CUSTOMIZED_GTPU_IPV4) ||
> +		   (cus_pctype->index == I40E_CUSTOMIZED_GTPU_IPV6) ||
> +		   (cus_pctype->index == I40E_CUSTOMIZED_GTPU) ||
> +		   (cus_pctype->index == I40E_CUSTOMIZED_IPV4_L2TPV3)))) {
>  		ip = (struct rte_ipv4_hdr *)raw_pkt;
> 
>  		*ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
> @@ -1054,12 +1063,16 @@ i40e_flow_fdir_fill_eth_ip_head(struct i40e_pf
> *pf,
>  			 cus_pctype->index == I40E_CUSTOMIZED_GTPU_IPV6 ||
>  			 cus_pctype->index == I40E_CUSTOMIZED_GTPU)
>  			ip->next_proto_id = IPPROTO_UDP;
> +		else if (cus_pctype->index == I40E_CUSTOMIZED_IPV4_L2TPV3)
> +			ip->next_proto_id = IPPROTO_L2TP;
>  		len += sizeof(struct rte_ipv4_hdr);
>  	} else if (pctype == I40E_FILTER_PCTYPE_NONF_IPV6_TCP ||
>  		   pctype == I40E_FILTER_PCTYPE_NONF_IPV6_UDP ||
>  		   pctype == I40E_FILTER_PCTYPE_NONF_IPV6_SCTP ||
>  		   pctype == I40E_FILTER_PCTYPE_NONF_IPV6_OTHER ||
> -		   pctype == I40E_FILTER_PCTYPE_FRAG_IPV6) {
> +		   pctype == I40E_FILTER_PCTYPE_FRAG_IPV6 ||
> +		   ((is_customized_pctype) &&
> +		    (cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3))) {
>  		ip6 = (struct rte_ipv6_hdr *)raw_pkt;
> 
>  		*ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV6);
> @@ -1069,9 +1082,12 @@ i40e_flow_fdir_fill_eth_ip_head(struct i40e_pf
> *pf,
>  					  I40E_FDIR_IPv6_TC_OFFSET));
>  		ip6->payload_len =
>  			rte_cpu_to_be_16(I40E_FDIR_IPv6_PAYLOAD_LEN);
> -		ip6->proto = fdir_input->flow.ipv6_flow.proto ?
> -			fdir_input->flow.ipv6_flow.proto :
> -			next_proto[fdir_input->pctype];
> +		if (!is_customized_pctype)
> +			ip6->proto = fdir_input->flow.ipv6_flow.proto ?
> +				fdir_input->flow.ipv6_flow.proto :
> +				next_proto[fdir_input->pctype];
> +		else if (cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3)
> +			ip6->proto = IPPROTO_L2TP;
>  		ip6->hop_limits = fdir_input->flow.ipv6_flow.hop_limits ?
>  			fdir_input->flow.ipv6_flow.hop_limits :
>  			I40E_FDIR_IPv6_DEFAULT_HOP_LIMITS;
> @@ -1115,6 +1131,7 @@ i40e_flow_fdir_construct_pkt(struct i40e_pf *pf,
>  	struct rte_flow_item_gtp *gtp;
>  	struct rte_ipv4_hdr *gtp_ipv4;
>  	struct rte_ipv6_hdr *gtp_ipv6;
> +	struct rte_flow_item_l2tpv3 *l2tpv3;
>  	uint8_t size, dst = 0;
>  	uint8_t i, pit_idx, set_idx = I40E_FLXPLD_L4_IDX; /* use l4 by default*/
>  	int len;
> @@ -1285,6 +1302,13 @@ i40e_flow_fdir_construct_pkt(struct i40e_pf *pf,
>  			} else
>  				payload = (unsigned char *)gtp +
>  					sizeof(struct rte_flow_item_gtp);
> +		} else if (cus_pctype->index == I40E_CUSTOMIZED_IPV4_L2TPV3 ||
> +			   cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3) {
> +			l2tpv3 = (struct rte_flow_item_l2tpv3 *)(raw_pkt + len);
> +			l2tpv3->session_id =
> +				fdir_input->flow.l2tpv3_flow.session_id;
> +			payload = (unsigned char *)l2tpv3 +
> +				sizeof(struct rte_flow_item_l2tpv3);
>  		}
>  	} else {
>  		PMD_DRV_LOG(ERR, "unknown pctype %u.", diff --git
> a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> 61021037c..2a5d1d0b6 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -1615,6 +1615,20 @@ static enum rte_flow_item_type pattern_qinq_1[]
> = {
>  	RTE_FLOW_ITEM_TYPE_END,
>  };
> 
> +static enum rte_flow_item_type pattern_fdir_ipv4_l2tpv3[] = {
> +	RTE_FLOW_ITEM_TYPE_ETH,
> +	RTE_FLOW_ITEM_TYPE_IPV4,
> +	RTE_FLOW_ITEM_TYPE_L2TPV3,
> +	RTE_FLOW_ITEM_TYPE_END,
> +};
> +
> +static enum rte_flow_item_type pattern_fdir_ipv6_l2tpv3[] = {
> +	RTE_FLOW_ITEM_TYPE_ETH,
> +	RTE_FLOW_ITEM_TYPE_IPV6,
> +	RTE_FLOW_ITEM_TYPE_L2TPV3,
> +	RTE_FLOW_ITEM_TYPE_END,
> +};
> +
>  static struct i40e_valid_pattern i40e_supported_patterns[] = {
>  	/* Ethertype */
>  	{ pattern_ethertype, i40e_flow_parse_ethertype_filter }, @@ -1795,6
> +1809,9 @@ static struct i40e_valid_pattern i40e_supported_patterns[] = {
>  	{ pattern_fdir_ipv6_gtpu, i40e_flow_parse_gtp_filter },
>  	/* QINQ */
>  	{ pattern_qinq_1, i40e_flow_parse_qinq_filter },
> +	/* L2TPv3 */
> +	{ pattern_fdir_ipv4_l2tpv3, i40e_flow_parse_fdir_filter },
> +	{ pattern_fdir_ipv6_l2tpv3, i40e_flow_parse_fdir_filter },
>  };
> 
>  #define NEXT_ITEM_OF_ACTION(act, actions, index)
> \
> @@ -2420,6 +2437,15 @@ i40e_flow_fdir_get_pctype_value(struct i40e_pf
> *pf,
>  			cus_pctype = i40e_find_customized_pctype(pf,
>  						 I40E_CUSTOMIZED_GTPU_IPV6);
>  		break;
> +	case RTE_FLOW_ITEM_TYPE_L2TPV3:
> +		if (filter->input.flow_ext.oip_type == I40E_FDIR_IPTYPE_IPV4)
> +			cus_pctype = i40e_find_customized_pctype(pf,
> +						I40E_CUSTOMIZED_IPV4_L2TPV3);
> +		else if (filter->input.flow_ext.oip_type ==
> +			 I40E_FDIR_IPTYPE_IPV6)
> +			cus_pctype = i40e_find_customized_pctype(pf,
> +						I40E_CUSTOMIZED_IPV6_L2TPV3);
> +		break;
>  	default:
>  		PMD_DRV_LOG(ERR, "Unsupported item type");
>  		break;
> @@ -2461,6 +2487,7 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev
> *dev,
>  	const struct rte_flow_item_gtp *gtp_spec, *gtp_mask;
>  	const struct rte_flow_item_raw *raw_spec, *raw_mask;
>  	const struct rte_flow_item_vf *vf_spec;
> +	const struct rte_flow_item_l2tpv3 *l2tpv3_spec, *l2tpv3_mask;
> 
>  	uint8_t pctype = 0;
>  	uint64_t input_set = I40E_INSET_NONE;
> @@ -3012,6 +3039,34 @@ i40e_flow_parse_fdir_pattern(struct
> rte_eth_dev *dev,
>  				return -rte_errno;
>  			}
>  			break;
> +		case RTE_FLOW_ITEM_TYPE_L2TPV3:
> +			l2tpv3_spec = item->spec;
> +			l2tpv3_mask = item->mask;
> +
> +			if (!l2tpv3_spec || !l2tpv3_mask)
> +				break;
> +
> +			if (l2tpv3_mask->session_id != UINT32_MAX) {
> +				rte_flow_error_set(error, EINVAL,
> +					RTE_FLOW_ERROR_TYPE_ITEM,
> +					item,
> +					"Invalid L2TPv3 mask");
> +				return -rte_errno;
> +			}
> +
> +			filter->input.flow.l2tpv3_flow.session_id =
> +				l2tpv3_spec->session_id;
> +
> +			if (l3 == RTE_FLOW_ITEM_TYPE_IPV4)
> +				filter->input.flow_ext.oip_type =
> +					I40E_FDIR_IPTYPE_IPV4;
> +			else if (l3 == RTE_FLOW_ITEM_TYPE_IPV6)
> +				filter->input.flow_ext.oip_type =
> +					I40E_FDIR_IPTYPE_IPV6;
> +
> +			filter->input.flow_ext.customized_pctype = true;
> +			cus_proto = item_type;
> +			break;
>  		default:
>  			break;
>  		}
> --
> 2.17.1

Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

[Sexton, Rory]

Hi Ori,

Let me rework the patch to make it clearer that this is supporting new flow type for l2tpv3 over IP, rather than l2tpv2/v3 over UDP which is how you interpreted it.
Will take into account all your feedback. Please review v2 once I submit.

Regards,
Rory

-----Original Message-----
From: dev <dev-bounces@dpdk.org> On Behalf Of Sexton, Rory
Sent: Wednesday, December 11, 2019 4:31 PM
To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Adrien Mazarguil <adrien.mazarguil@6wind.com>; Jagus, DariuszX <dariuszx.jagus@intel.com>
Subject: Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

Hi Ori,

See comments below.

Regards,
Rory

> > > > > > +/**
> > > > > > + * RTE_FLOW_ITEM_TYPE_L2TPV3.
> > > > > > + *
> > > > > > + * Matches a L2TPv3 header.
> > > > > > + */
> > > > > > +struct rte_flow_item_l2tpv3 {
> > > > > > +	rte_be32_t session_id; /**< Session ID. */ };
> > > > >
> > > > >Does it make sense that in future we will want to match on the 
> > > > >T / L / ver /
> > > > Ns / Nr?
> > > > >
> > > > >Please also consider that any change will be ABI / API 
> > > > >breakage, which will be allowed only next year.
> > > > >
> > > >
> > > > I don't foresee us wanting to match on any of the other fields, 
> > > > T / L / ver / Ns/ Nr.
> > > > The session id would typically be the only field of interest in 
> > > > the
> > > > l2tpv3 header.
> > >
> > > I think that adding all fields to the structure will solve the 
> > > possible issue
> > with adding matching later.
> > > Also and even more important you will be able to use it for 
> > > creating the
> > raw_encap / raw_decap buffers.
> > >
> > >What do you think?
> > 
> > Based on the differences between v2 and v3 the only field of 
> > interest in
> > l2tpv3 over IP is the Session ID.
> > I agree it would make sense to add all fields if we were 
> > implementing
> > l2tpv2 however v2 would require a different implementation to v3 due 
> > to the differences between both Protocols.
>
> Even if we just support v3, I think that it is a good idea to add all fields of v3. 
> This will allow the use of the raw_encap / raw_decap actions.
> Please also note that you didn't add the new item to  cmd_set_raw_parsed function.
> this function is used in order to create raw_encap/ raw_decap encapsulations.

I think the confusion here is based on the fact that there are 2 separate types of l2tpv3.
- l2tpv3 over UDP, which is very similar to l2tpv2 with only change being 16 bit Tunnel ID
  and 16 bit Session ID changing to 32 bit Session ID. All other fields remain (T/L/Ver/Ns/Nr).
- l2tpv3 over IP is another type of l2tpv3 that is carried over IP rather than UDP and as such
  the message format is entirely different and consists of just a 32 bit Session ID. The other
  fields mentioned above do not exist at all in this l2tpv3 header.

This patch was targeted at creating a flow to handle l2tpv3 over IP exclusively. This is why the Session ID is the only field in the flow item.

I can add the item to cmd_set_raw_parsed function.

Re: [dpdk-dev] [PATCH] net/i40e: Add new customized pctype for l2tpv3

[Sexton, Rory]

Hi Beilei,

See comments below.

Regards,
Rory

> > Subject: [PATCH] net/i40e: Add new customized pctype for l2tpv3
> It's not only add new customized pctype, but mainly enable FDIR for l2ipv3, so how about " net/i40e: support FDIR for L2TPv3"?
>
> Detailed commit log is also needed.

Of course can update this in v2.

> > +/* A structure used to define the input for L2TPv3 flow */ struct 
> > +i40e_l2tpv3_flow {
>
> Seems missed struct rte_eth_ipv4_flow or struct rte_eth_ipv6_flow here?
> 

I'm not convinced we need struct rte_eth_ipv4_flow or struct rte_eth_ipv6_flow to be part of the struct i40e_l2tpv3_flow.
The rte_eth_ipv4/6_flow struct will be included in the flow director pattern from the following additions.
Please advice so I can update in a v2 of the patch if required.

+static enum rte_flow_item_type pattern_fdir_ipv4_l2tpv3[] = {
+       RTE_FLOW_ITEM_TYPE_ETH,
+       RTE_FLOW_ITEM_TYPE_IPV4,
+       RTE_FLOW_ITEM_TYPE_L2TPV3,
+       RTE_FLOW_ITEM_TYPE_END,
+};
+
+static enum rte_flow_item_type pattern_fdir_ipv6_l2tpv3[] = {
+       RTE_FLOW_ITEM_TYPE_ETH,
+       RTE_FLOW_ITEM_TYPE_IPV6,
+       RTE_FLOW_ITEM_TYPE_L2TPV3,
+       RTE_FLOW_ITEM_TYPE_END,
+};

Re: [dpdk-dev] [PATCH] net/i40e: Add new customized pctype for l2tpv3

[Xing, Beilei]

> -----Original Message-----
> From: Sexton, Rory <rory.sexton@intel.com>
> Sent: Friday, December 13, 2019 3:18 AM
> To: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; adrien.mazarguil@6wind.com;
> Jagus, DariuszX <dariuszx.jagus@intel.com>
> Subject: RE: [PATCH] net/i40e: Add new customized pctype for l2tpv3
> 
> Hi Beilei,
> 
> See comments below.
> 
> Regards,
> Rory
> 
> > > Subject: [PATCH] net/i40e: Add new customized pctype for l2tpv3
> > It's not only add new customized pctype, but mainly enable FDIR for l2ipv3,
> so how about " net/i40e: support FDIR for L2TPv3"?
> >
> > Detailed commit log is also needed.
> 
> Of course can update this in v2.
> 
> > > +/* A structure used to define the input for L2TPv3 flow */ struct
> > > +i40e_l2tpv3_flow {
> >
> > Seems missed struct rte_eth_ipv4_flow or struct rte_eth_ipv6_flow here?
> >
> 
> I'm not convinced we need struct rte_eth_ipv4_flow or struct
> rte_eth_ipv6_flow to be part of the struct i40e_l2tpv3_flow.
> The rte_eth_ipv4/6_flow struct will be included in the flow director pattern
> from the following additions.
> Please advice so I can update in a v2 of the patch if required.

Please refer to union i40e_fdir_flow: A union contains the inputs for all types of flow items in flows need to be in big endian.
Pattern is part of rte flow, but not the packet sent to HW to create/destroy a FDIR rule.

Beilei

> 
> +static enum rte_flow_item_type pattern_fdir_ipv4_l2tpv3[] = {
> +       RTE_FLOW_ITEM_TYPE_ETH,
> +       RTE_FLOW_ITEM_TYPE_IPV4,
> +       RTE_FLOW_ITEM_TYPE_L2TPV3,
> +       RTE_FLOW_ITEM_TYPE_END,
> +};
> +
> +static enum rte_flow_item_type pattern_fdir_ipv6_l2tpv3[] = {
> +       RTE_FLOW_ITEM_TYPE_ETH,
> +       RTE_FLOW_ITEM_TYPE_IPV6,
> +       RTE_FLOW_ITEM_TYPE_L2TPV3,
> +       RTE_FLOW_ITEM_TYPE_END,
> +};
> 

Re: [dpdk-dev] [PATCH] net/i40e: Add new customized pctype for l2tpv3

[Sexton, Rory]

Hi Beilei,

See below,

Rory

> > > > +/* A structure used to define the input for L2TPv3 flow */ struct 
> > > > +i40e_l2tpv3_flow {
> > >
> > > Seems missed struct rte_eth_ipv4_flow or struct rte_eth_ipv6_flow here?
> > >
> > 
> > I'm not convinced we need struct rte_eth_ipv4_flow or struct 
> > rte_eth_ipv6_flow to be part of the struct i40e_l2tpv3_flow.
> > The rte_eth_ipv4/6_flow struct will be included in the flow director 
> > pattern from the following additions.
> > Please advice so I can update in a v2 of the patch if required.
>
> Please refer to union i40e_fdir_flow: A union contains the inputs for all types of flow items in flows need to be in big endian.
> Pattern is part of rte flow, but not the packet sent to HW to create/destroy a FDIR rule.
>
> Beilei

Ok, I follow you now. Thanks for clarification.
Please see updates in v2.