DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Xing, Beilei" <beilei.xing@intel.com>
To: "Sun, GuinanX" <guinanx.sun@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Guo, Jia" <jia.guo@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: enable port filter by switch filter
Date: Tue, 30 Jun 2020 06:15:36 +0000	[thread overview]
Message-ID: <MN2PR11MB3807B2A111A6A5CC2F236C2CF76F0@MN2PR11MB3807.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20200630044205.54900-1-guinanx.sun@intel.com>



> -----Original Message-----
> From: Sun, GuinanX <guinanx.sun@intel.com>
> Sent: Tuesday, June 30, 2020 12:42 PM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Guo, Jia <jia.guo@intel.com>; Sun,
> GuinanX <guinanx.sun@intel.com>
> Subject: [PATCH v2] net/i40e: enable port filter by switch filter

How about 'support cloud filter with l4 port'?

Please also fix all the warnings in patchwork: http://mails.dpdk.org/archives/test-report/2020-June/139555.html.

> 
> This patch enables the filter that supports to create following two rules for
> the same packet type:

Better to clarify which packet types will be supported.

> One is to select source port only as input set and the other is for destination
> port only.
> 
> Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> ---
> v2:
> * Fixed code style and variable naming
> ---
>  doc/guides/rel_notes/release_20_08.rst |   8 +
>  drivers/net/i40e/i40e_ethdev.c         | 195 ++++++++++++++++++++-
>  drivers/net/i40e/i40e_ethdev.h         |  17 ++
>  drivers/net/i40e/i40e_flow.c           | 223 +++++++++++++++++++++++++
>  4 files changed, 442 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/rel_notes/release_20_08.rst
> b/doc/guides/rel_notes/release_20_08.rst
> index 3c40424cc..c4d094eac 100644
> --- a/doc/guides/rel_notes/release_20_08.rst
> +++ b/doc/guides/rel_notes/release_20_08.rst
> @@ -87,6 +87,14 @@ New Features
> 
>    * Added support for DCF datapath configuration.
> 
> +* **Updated Intel i40e driver.**
> +
> +  Updated i40e PMD with new features and improvements, including:
> +
> +  * Added a new type of cloud filter to support the coexistence of the
> +    following two rules. One selects L4 destination as input set and

L4 destination port

> +    the other one selects L4 source port.
> +

BTW, replace filter is used for the patch, so which protocol is impacted with this change?
e.g. If using this feature, can cloud filter for vxlan work well?
If there's any impact, please add the limitation in the doc.

>  Removed Items
>  -------------
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 970a31cb2..cea7f6b59 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -7956,6 +7956,13 @@ i40e_dev_tunnel_filter_set(struct i40e_pf *pf,
>  #define I40E_TR_GRE_KEY_MASK			0x400
>  #define I40E_TR_GRE_KEY_WITH_XSUM_MASK		0x800
>  #define I40E_TR_GRE_NO_KEY_MASK			0x8000
> +#define I40E_AQC_REPLACE_CLOUD_CMD_INPUT_PORT_TR_WORD0 0x49
> #define
> +I40E_AQC_REPLACE_CLOUD_CMD_INPUT_DIRECTION_WORD0 0x41
> #define
> +I40E_AQC_REPLACE_CLOUD_CMD_INPUT_INGRESS_WORD0 0x80
> +#define I40E_DIRECTION_INGRESS_KEY		0x8000
> +#define I40E_TR_L4_TYPE_TCP			0x2
> +#define I40E_TR_L4_TYPE_UDP			0x4
> +#define I40E_TR_L4_TYPE_SCTP			0x8
> 
>  static enum
>  i40e_status_code i40e_replace_mpls_l1_filter(struct i40e_pf *pf) @@ -
> 8254,6 +8261,131 @@ i40e_status_code i40e_replace_gtp_cloud_filter(struct
> i40e_pf *pf)
>  	return status;
>  }
> 
> +static enum i40e_status_code
> +i40e_replace_port_l1_filter(struct i40e_pf *pf, enum i40e_l4_port_type
> +l4_port_type) {
> +	struct i40e_aqc_replace_cloud_filters_cmd_buf  filter_replace_buf;
> +	struct i40e_aqc_replace_cloud_filters_cmd  filter_replace;
> +	enum i40e_status_code status = I40E_SUCCESS;
> +	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> +	struct rte_eth_dev *dev = ((struct i40e_adapter *)hw->back)-
> >eth_dev;
> +
> +	if (pf->support_multi_driver) {
> +		PMD_DRV_LOG(ERR, "Replace l1 filter is not supported.");
> +		return I40E_NOT_SUPPORTED;
> +	}
> +
> +	memset(&filter_replace, 0,
> +	       sizeof(struct i40e_aqc_replace_cloud_filters_cmd));
> +	memset(&filter_replace_buf, 0,
> +	       sizeof(struct i40e_aqc_replace_cloud_filters_cmd_buf));
> +
> +	/* create L1 filter */
> +	if (l4_port_type == I40E_L4_PORT_TYPE_SRC) {
> +		filter_replace.old_filter_type =
> +
> 	I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_TUNNLE_KEY;
> +		filter_replace.new_filter_type =
> I40E_AQC_ADD_CLOUD_FILTER_0X11;

To create L1 filter, so should be I40E_AQC_ADD_L1_FILTER_0X11 here?

> +		filter_replace_buf.data[8] =
> +
> 	I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_SRC_PORT;
> +	} else {
> +		filter_replace.old_filter_type =
> +
> 	I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_STAG_IVLAN;
> +		filter_replace.new_filter_type =
> I40E_AQC_ADD_CLOUD_FILTER_0X10;

Same as above.

> +		filter_replace_buf.data[8] =
> +
> 	I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_DST_PORT;
> +	}
> +
> +	filter_replace.tr_bit = 0;
> +	/* Prepare the buffer, 3 entries */
> +	filter_replace_buf.data[0] =
> +
> 	I40E_AQC_REPLACE_CLOUD_CMD_INPUT_DIRECTION_WORD0;
> +	filter_replace_buf.data[0] |=
> +		I40E_AQC_REPLACE_CLOUD_CMD_INPUT_VALIDATED;
> +	filter_replace_buf.data[2] = 0x00;
> +	filter_replace_buf.data[3] =
> +		I40E_AQC_REPLACE_CLOUD_CMD_INPUT_INGRESS_WORD0;
> +	filter_replace_buf.data[4] =
> +		I40E_AQC_REPLACE_CLOUD_CMD_INPUT_PORT_TR_WORD0;
> +	filter_replace_buf.data[4] |=
> +		I40E_AQC_REPLACE_CLOUD_CMD_INPUT_VALIDATED;
> +	filter_replace_buf.data[5] = 0x00;
> +	filter_replace_buf.data[6] = I40E_TR_L4_TYPE_UDP |
> +		I40E_TR_L4_TYPE_TCP |
> +		I40E_TR_L4_TYPE_SCTP;
> +	filter_replace_buf.data[7] = 0x00;
> +	filter_replace_buf.data[8] |=
> +		I40E_AQC_REPLACE_CLOUD_CMD_INPUT_VALIDATED;
> +	filter_replace_buf.data[9] = 0x00;
> +	filter_replace_buf.data[10] = 0xFF;
> +	filter_replace_buf.data[11] = 0xFF;
> +
> +	status = i40e_aq_replace_cloud_filters(hw, &filter_replace,
> +					       &filter_replace_buf);
> +	if (!status && (filter_replace.old_filter_type !=
> +		filter_replace.new_filter_type))
> +		PMD_DRV_LOG(WARNING, "i40e device %s changed cloud l1
> type."
> +			    " original: 0x%x, new: 0x%x",
> +			    dev->device->name,
> +			    filter_replace.old_filter_type,
> +			    filter_replace.new_filter_type);
> +
> +	return status;
> +}
> +
> +static enum
> +i40e_status_code i40e_replace_port_cloud_filter(struct i40e_pf *pf,
> +					enum i40e_l4_port_type
> l4_port_type) {
> +	struct i40e_aqc_replace_cloud_filters_cmd_buf  filter_replace_buf;
> +	struct i40e_aqc_replace_cloud_filters_cmd  filter_replace;
> +	enum i40e_status_code status = I40E_SUCCESS;
> +	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> +	struct rte_eth_dev *dev = ((struct i40e_adapter *)hw->back)-
> >eth_dev;
> +
> +	if (pf->support_multi_driver) {
> +		PMD_DRV_LOG(ERR, "Replace cloud filter is not supported.");
> +		return I40E_NOT_SUPPORTED;
> +	}
> +
> +	memset(&filter_replace, 0,
> +	       sizeof(struct i40e_aqc_replace_cloud_filters_cmd));
> +	memset(&filter_replace_buf, 0,
> +	       sizeof(struct i40e_aqc_replace_cloud_filters_cmd_buf));
> +
> +	if (l4_port_type == I40E_L4_PORT_TYPE_SRC) {
> +		filter_replace.old_filter_type =
> I40E_AQC_ADD_CLOUD_FILTER_IIP;
> +		filter_replace.new_filter_type =
> +			I40E_AQC_ADD_L1_FILTER_0X11;
> +		filter_replace_buf.data[4] =
> I40E_AQC_ADD_CLOUD_FILTER_0X11;
> +	} else {
> +		filter_replace.old_filter_type =
> I40E_AQC_ADD_CLOUD_FILTER_OIP;
> +		filter_replace.new_filter_type =
> +			I40E_AQC_ADD_CLOUD_FILTER_0X10;
> +		filter_replace_buf.data[4] =
> I40E_AQC_ADD_CLOUD_FILTER_0X10;
> +	}
> +
> +	filter_replace.valid_flags = I40E_AQC_REPLACE_CLOUD_FILTER;
> +	filter_replace.tr_bit = 0;
> +	/* Prepare the buffer, 2 entries */
> +	filter_replace_buf.data[0] =
> I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_STAG;
> +	filter_replace_buf.data[0] |=
> +		I40E_AQC_REPLACE_CLOUD_CMD_INPUT_VALIDATED;
> +	filter_replace_buf.data[4] |=
> +		I40E_AQC_REPLACE_CLOUD_CMD_INPUT_VALIDATED;
> +	status = i40e_aq_replace_cloud_filters(hw, &filter_replace,
> +					       &filter_replace_buf);
> +
> +	if (!status && (filter_replace.old_filter_type !=
> +			filter_replace.new_filter_type))
> +		PMD_DRV_LOG(WARNING, "i40e device %s changed cloud
> filter type."
> +			    " original: 0x%x, new: 0x%x",
> +			    dev->device->name,
> +			    filter_replace.old_filter_type,
> +			    filter_replace.new_filter_type);
> +
> +	return status;
> +}
> +
>  int
>  i40e_dev_consistent_tunnel_filter_set(struct i40e_pf *pf,
>  		      struct i40e_tunnel_filter_conf *tunnel_filter, @@ -8401,6
> +8533,58 @@ i40e_dev_consistent_tunnel_filter_set(struct i40e_pf *pf,
>  		pfilter->general_fields[0] = tunnel_filter->inner_vlan;
>  		pfilter->general_fields[1] = tunnel_filter->outer_vlan;
>  		big_buffer = 1;
> +		break;
> +	case I40E_TUNNEL_TYPE_UDP:
> +	case I40E_TUNNEL_TYPE_TCP:
> +	case I40E_TUNNEL_TYPE_SCTP:

What's the tunnel_type_udp/tcp/sctp? It's not for tunnel, just normal UDP/TCP/SCTP. 

> +		if (tunnel_filter->l4_port_type == I40E_L4_PORT_TYPE_SRC) {
> +			if (!pf->sport_replace_flag) {
> +				i40e_replace_port_l1_filter(pf, tunnel_filter-
> >l4_port_type);
> +				i40e_replace_port_cloud_filter(pf,
> tunnel_filter->l4_port_type);
> +				pf->sport_replace_flag = 1;
> +			}
> +			teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> +			pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> +				I40E_DIRECTION_INGRESS_KEY;
> +
> +			if (tunnel_filter->tunnel_type ==
> I40E_TUNNEL_TYPE_UDP)
> +				pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> +					I40E_TR_L4_TYPE_UDP;
> +			else if (tunnel_filter->tunnel_type ==
> I40E_TUNNEL_TYPE_TCP)
> +				pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> +					I40E_TR_L4_TYPE_TCP;
> +			else
> +				pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> +					I40E_TR_L4_TYPE_SCTP;
> +
> +			pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> +				(teid_le >> 16) & 0xFFFF;
> +			big_buffer = 1;
> +		} else {
> +			if (!pf->dport_replace_flag) {
> +				i40e_replace_port_l1_filter(pf, tunnel_filter-
> >l4_port_type);
> +				i40e_replace_port_cloud_filter(pf,
> tunnel_filter->l4_port_type);
> +				pf->dport_replace_flag = 1;
> +			}
> +			teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> +			pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X10_WORD0] =
> +				I40E_DIRECTION_INGRESS_KEY;
> +
> +			if (tunnel_filter->tunnel_type ==
> I40E_TUNNEL_TYPE_UDP)
> +				pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X10_WORD1] =
> +					I40E_TR_L4_TYPE_UDP;
> +			else if (tunnel_filter->tunnel_type ==
> I40E_TUNNEL_TYPE_TCP)
> +				pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X10_WORD1] =
> +					I40E_TR_L4_TYPE_TCP;
> +			else
> +				pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X10_WORD1] =
> +					I40E_TR_L4_TYPE_SCTP;
> +
> +			pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X10_WORD2] =
> +				(teid_le >> 16) & 0xFFFF;
> +			big_buffer = 1;
> +		}
> +
>  		break;
>  	default:
>  		/* Other tunnel types is not supported. */ @@ -8424,7
> +8608,16 @@ i40e_dev_consistent_tunnel_filter_set(struct i40e_pf *pf,
>  	else if (tunnel_filter->tunnel_type == I40E_TUNNEL_TYPE_QINQ)
>  		pfilter->element.flags |=
>  			I40E_AQC_ADD_CLOUD_FILTER_0X10;
> -	else {
> +	else if (tunnel_filter->tunnel_type == I40E_TUNNEL_TYPE_UDP ||
> +		 tunnel_filter->tunnel_type == I40E_TUNNEL_TYPE_TCP ||
> +		 tunnel_filter->tunnel_type == I40E_TUNNEL_TYPE_SCTP) {
> +		if (tunnel_filter->l4_port_type == I40E_L4_PORT_TYPE_SRC)
> +			pfilter->element.flags |=
> +				I40E_AQC_ADD_L1_FILTER_0X11;
> +		else
> +			pfilter->element.flags |=
> +				I40E_AQC_ADD_CLOUD_FILTER_0X10;
> +	} else {
>  		val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
>  						&pfilter->element.flags);
>  		if (val < 0) {
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index e5d0ce53f..56955068b 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -767,6 +767,8 @@ struct i40e_rss_pattern_info {
> 
>  #define I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_TEID_WORD0 44
> #define I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_TEID_WORD1 45
> +#define I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_SRC_PORT 29 #define
> +I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_DST_PORT 30
>  #define I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSOUDP	8
>  #define I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSOGRE	9
>  #define I40E_AQC_ADD_CLOUD_FILTER_0X10		0x10
> @@ -828,9 +830,20 @@ enum i40e_tunnel_type {
>  	I40E_TUNNEL_TYPE_GTPU,
>  	I40E_TUNNEL_TYPE_ESPoUDP,
>  	I40E_TUNNEL_TYPE_ESPoIP,
> +	I40E_TUNNEL_TYPE_UDP,
> +	I40E_TUNNEL_TYPE_TCP,
> +	I40E_TUNNEL_TYPE_SCTP,

the macro name should be changed.

>  	I40E_TUNNEL_TYPE_MAX,
>  };
> 
> +/**
> + * L4 port type.
> + */
> +enum i40e_l4_port_type {
> +	I40E_L4_PORT_TYPE_SRC = 0,
> +	I40E_L4_PORT_TYPE_DST,
> +};
> +
...


  reply	other threads:[~2020-06-30  6:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-11  5:24 [dpdk-dev] [PATCH] " Guinan Sun
2020-06-15  6:12 ` Zhao1, Wei
2020-06-15  6:28   ` Sun, GuinanX
2020-06-21 12:28 ` Jeff Guo
2020-06-24  3:09   ` Sun, GuinanX
2020-06-30  4:42 ` [dpdk-dev] [PATCH v2] " Guinan Sun
2020-06-30  6:15   ` Xing, Beilei [this message]
2020-07-07  6:11     ` Sun, GuinanX
2020-07-07  6:33 ` [dpdk-dev] [PATCH v3] net/i40e: support cloud filter with L4 port Guinan Sun
2020-07-07  8:15   ` Xing, Beilei
2020-07-08  3:11 ` [dpdk-dev] [PATCH v4] " Guinan Sun
2020-07-08  6:01   ` Xing, Beilei
2020-07-08  6:27 ` [dpdk-dev] [PATCH v5] " Guinan Sun
2020-07-08  7:40 ` [dpdk-dev] [PATCH v6] " Guinan Sun
2020-07-08  8:20   ` Xing, Beilei
2020-07-08  8:52     ` Zhang, Qi Z

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MN2PR11MB3807B2A111A6A5CC2F236C2CF76F0@MN2PR11MB3807.namprd11.prod.outlook.com \
    --to=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=guinanx.sun@intel.com \
    --cc=jia.guo@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).