net/i40e: Add new customized pctype for l2tpv3

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

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

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,
+};
  

> -----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,
> +};
>
  

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.