[dpdk-dev,v2,01/18] net/ixgbe: store SYN filter

Message ID 1483084390-53159-2-git-send-email-wei.zhao1@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Zhao1, Wei Dec. 30, 2016, 7:52 a.m. UTC
  Add support for storing SYN filter in SW.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---

v2:
--synqf assignment location change
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 14 +++++++++++---
 drivers/net/ixgbe/ixgbe_ethdev.h |  2 ++
 2 files changed, 13 insertions(+), 3 deletions(-)
  

Comments

Wei Dai Jan. 3, 2017, 2:33 p.m. UTC | #1
Hi, Wei Zhao 

I think that you had better give a cover letter for such a series of patches.
You can give the changes between v2 and v1 in cover letter 
and maybe no need describe it in each one.

Thanks &Best Regards
-Wei

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 30, 2016 3:53 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v2 01/18] net/ixgbe: store SYN filter
> 
> Add support for storing SYN filter in SW.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
> 
> v2:
> --synqf assignment location change
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 14 +++++++++++---
> drivers/net/ixgbe/ixgbe_ethdev.h |  2 ++
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index a25bac8..316e560 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1274,6 +1274,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
>  	memset(filter_info->fivetuple_mask, 0,
>  	       sizeof(uint32_t) * IXGBE_5TUPLE_ARRAY_SIZE);
> 
> +	/* initialize SYN filter */
> +	filter_info->syn_info = 0;
>  	return 0;
>  }
> 
> @@ -5580,15 +5582,18 @@ ixgbe_syn_filter_set(struct rte_eth_dev *dev,
>  			bool add)
>  {
>  	struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct ixgbe_filter_info *filter_info =
> +		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
> +	uint32_t syn_info;
>  	uint32_t synqf;
> 
>  	if (filter->queue >= IXGBE_MAX_RX_QUEUE_NUM)
>  		return -EINVAL;
> 
> -	synqf = IXGBE_READ_REG(hw, IXGBE_SYNQF);
> +	syn_info = filter_info->syn_info;
> 
>  	if (add) {
> -		if (synqf & IXGBE_SYN_FILTER_ENABLE)
> +		if (syn_info & IXGBE_SYN_FILTER_ENABLE)
>  			return -EINVAL;
>  		synqf = (uint32_t)(((filter->queue <<
> IXGBE_SYN_FILTER_QUEUE_SHIFT) &
>  			IXGBE_SYN_FILTER_QUEUE) | IXGBE_SYN_FILTER_ENABLE);
> @@ -5598,10 +5603,13 @@ ixgbe_syn_filter_set(struct rte_eth_dev *dev,
>  		else
>  			synqf &= ~IXGBE_SYN_FILTER_SYNQFP;
>  	} else {
> -		if (!(synqf & IXGBE_SYN_FILTER_ENABLE))
> +		synqf = IXGBE_READ_REG(hw, IXGBE_SYNQF);
> +		if (!(syn_info & IXGBE_SYN_FILTER_ENABLE))
>  			return -ENOENT;
>  		synqf &= ~(IXGBE_SYN_FILTER_QUEUE |
> IXGBE_SYN_FILTER_ENABLE);
>  	}
> +
> +	filter_info->syn_info = synqf;
>  	IXGBE_WRITE_REG(hw, IXGBE_SYNQF, synqf);
>  	IXGBE_WRITE_FLUSH(hw);
>  	return 0;
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 4ff6338..827026c 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -262,6 +262,8 @@ struct ixgbe_filter_info {
>  	/* Bit mask for every used 5tuple filter */
>  	uint32_t fivetuple_mask[IXGBE_5TUPLE_ARRAY_SIZE];
>  	struct ixgbe_5tuple_filter_list fivetuple_list;
> +	/* store the SYN filter info */
> +	uint32_t syn_info;
>  };
> 
>  /*
> --
> 2.5.5
  
Zhao1, Wei Jan. 4, 2017, 1:46 a.m. UTC | #2
Hi, Daiwei

> -----Original Message-----
> From: Dai, Wei
> Sent: Tuesday, January 3, 2017 10:33 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhao1, Wei
> <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 01/18] net/ixgbe: store SYN filter
> 
> Hi, Wei Zhao
> 
> I think that you had better give a cover letter for such a series of patches.
> You can give the changes between v2 and v1 in cover letter and maybe no
> need describe it in each one.
> 

Ok, I will add cover letter in v3

> Thanks &Best Regards
> -Wei
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> > Sent: Friday, December 30, 2016 3:53 PM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhao1, Wei
> > <wei.zhao1@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 01/18] net/ixgbe: store SYN filter
> >
> > Add support for storing SYN filter in SW.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > ---
> >
> > v2:
> > --synqf assignment location change
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 14 +++++++++++---
> > drivers/net/ixgbe/ixgbe_ethdev.h |  2 ++
> >  2 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index a25bac8..316e560 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -1274,6 +1274,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
> >  	memset(filter_info->fivetuple_mask, 0,
> >  	       sizeof(uint32_t) * IXGBE_5TUPLE_ARRAY_SIZE);
> >
> > +	/* initialize SYN filter */
> > +	filter_info->syn_info = 0;
> >  	return 0;
> >  }
> >
> > @@ -5580,15 +5582,18 @@ ixgbe_syn_filter_set(struct rte_eth_dev *dev,
> >  			bool add)
> >  {
> >  	struct ixgbe_hw *hw =
> > IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > +	struct ixgbe_filter_info *filter_info =
> > +		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data-
> >dev_private);
> > +	uint32_t syn_info;
> >  	uint32_t synqf;
> >
> >  	if (filter->queue >= IXGBE_MAX_RX_QUEUE_NUM)
> >  		return -EINVAL;
> >
> > -	synqf = IXGBE_READ_REG(hw, IXGBE_SYNQF);
> > +	syn_info = filter_info->syn_info;
> >
> >  	if (add) {
> > -		if (synqf & IXGBE_SYN_FILTER_ENABLE)
> > +		if (syn_info & IXGBE_SYN_FILTER_ENABLE)
> >  			return -EINVAL;
> >  		synqf = (uint32_t)(((filter->queue <<
> > IXGBE_SYN_FILTER_QUEUE_SHIFT) &
> >  			IXGBE_SYN_FILTER_QUEUE) |
> IXGBE_SYN_FILTER_ENABLE); @@ -5598,10
> > +5603,13 @@ ixgbe_syn_filter_set(struct rte_eth_dev *dev,
> >  		else
> >  			synqf &= ~IXGBE_SYN_FILTER_SYNQFP;
> >  	} else {
> > -		if (!(synqf & IXGBE_SYN_FILTER_ENABLE))
> > +		synqf = IXGBE_READ_REG(hw, IXGBE_SYNQF);
> > +		if (!(syn_info & IXGBE_SYN_FILTER_ENABLE))
> >  			return -ENOENT;
> >  		synqf &= ~(IXGBE_SYN_FILTER_QUEUE |
> IXGBE_SYN_FILTER_ENABLE);
> >  	}
> > +
> > +	filter_info->syn_info = synqf;
> >  	IXGBE_WRITE_REG(hw, IXGBE_SYNQF, synqf);
> >  	IXGBE_WRITE_FLUSH(hw);
> >  	return 0;
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> > b/drivers/net/ixgbe/ixgbe_ethdev.h
> > index 4ff6338..827026c 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> > @@ -262,6 +262,8 @@ struct ixgbe_filter_info {
> >  	/* Bit mask for every used 5tuple filter */
> >  	uint32_t fivetuple_mask[IXGBE_5TUPLE_ARRAY_SIZE];
> >  	struct ixgbe_5tuple_filter_list fivetuple_list;
> > +	/* store the SYN filter info */
> > +	uint32_t syn_info;
> >  };
> >
> >  /*
> > --
> > 2.5.5
  
Ferruh Yigit Jan. 6, 2017, 4:28 p.m. UTC | #3
On 12/30/2016 7:52 AM, Wei Zhao wrote:
> Add support for storing SYN filter in SW.

I think you have mentioned in previous review that you will update this
to "TCP SYN", I would like to remind in case it is missed.

> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
> 
> v2:
> --synqf assignment location change
> ---
<...>
  
Zhao1, Wei Jan. 10, 2017, 5:33 a.m. UTC | #4
Hi, yigit 

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Saturday, January 7, 2017 12:29 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 01/18] net/ixgbe: store SYN filter
> 
> On 12/30/2016 7:52 AM, Wei Zhao wrote:
> > Add support for storing SYN filter in SW.
> 
> I think you have mentioned in previous review that you will update this to
> "TCP SYN", I would like to remind in case it is missed.
> 
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > ---
> >
> > v2:
> > --synqf assignment location change
> > ---
> <...>

There is something wrong when I update code in v2.
I PROMISE it will be changed in v3.
  
Zhao1, Wei Jan. 12, 2017, 8:12 a.m. UTC | #5
The patches mainly finish following functions:
1) Store and restore all kinds of filters.
2) Parse all kinds of filters.
3) Add flow validate function.
4) Add flow create function.
5) Add flow destroy function.
6) Add flow flush function.

v2 changes:
 fix git log error
 Modify some function call relationship
 Change return value type of all parse flow functions
 Update error info for all flow ops
 Add ixgbe_filterlist_flush to flush flows and rules created

v3 change:
 add new file ixgbe_flow.c to store generic API parser related functions
 add more comment about pattern and action rules
 add attr check in parser functions
 change struct name ixgbe_flow to rte_flow
 change SYN to TCP SYN
 change to use memset initizlize struct ixgbe_filter_info
 break down filter uninit process to 3 indepedent functions in eth_ixgbe_dev_uninit()
 change struct rte_flow_item_nvgre definition
 change struct rte_flow_item_e_tag definition
 fix one bug in function ixgbe_dev_filter_ctrl
 add goto in function ixgbe_flow_create
 delete some useless initialization 
 eliminate some git log check warning

zhao wei (18):
  net/ixgbe: store TCP SYN filter
  net/ixgbe: store flow director filter
  net/ixgbe: store L2 tunnel filter
  net/ixgbe: restore n-tuple filter Add support for restoring n-tuple
    filter in SW.
  net/ixgbe: restore ether type filter
  net/ixgbe: restore TCP SYN filter
  net/ixgbe: restore flow director filter
  net/ixgbe: restore L2 tunnel filter
  net/ixgbe: store and restore L2 tunnel configuration
  net/ixgbe: flush all the filters
  net/ixgbe: parse n-tuple filter
  net/ixgbe: parse ethertype filter
  net/ixgbe: parse TCP SYN filter     check if the rule is a TCP SYN
    rule, and get the SYN info.
  net/ixgbe: parse L2 tunnel filter     check if the rule is a L2 tunnel
    rule, and get the L2 tunnel info.
  net/ixgbe: parse flow director filter
  net/ixgbe: create consistent filter
  net/ixgbe: destroy consistent filter
  net/ixgbe: flush all the filter list

 drivers/net/ixgbe/Makefile       |    2 +
 drivers/net/ixgbe/ixgbe_ethdev.c |  667 +++++++--
 drivers/net/ixgbe/ixgbe_ethdev.h |  203 ++-
 drivers/net/ixgbe/ixgbe_fdir.c   |  407 ++++--
 drivers/net/ixgbe/ixgbe_flow.c   | 2811 ++++++++++++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_pf.c     |   26 +-
 lib/librte_ether/rte_flow.h      |   48 +
 7 files changed, 3953 insertions(+), 211 deletions(-)
 create mode 100644 drivers/net/ixgbe/ixgbe_flow.c
  

Patch

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a25bac8..316e560 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1274,6 +1274,8 @@  eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
 	memset(filter_info->fivetuple_mask, 0,
 	       sizeof(uint32_t) * IXGBE_5TUPLE_ARRAY_SIZE);
 
+	/* initialize SYN filter */
+	filter_info->syn_info = 0;
 	return 0;
 }
 
@@ -5580,15 +5582,18 @@  ixgbe_syn_filter_set(struct rte_eth_dev *dev,
 			bool add)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_filter_info *filter_info =
+		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
+	uint32_t syn_info;
 	uint32_t synqf;
 
 	if (filter->queue >= IXGBE_MAX_RX_QUEUE_NUM)
 		return -EINVAL;
 
-	synqf = IXGBE_READ_REG(hw, IXGBE_SYNQF);
+	syn_info = filter_info->syn_info;
 
 	if (add) {
-		if (synqf & IXGBE_SYN_FILTER_ENABLE)
+		if (syn_info & IXGBE_SYN_FILTER_ENABLE)
 			return -EINVAL;
 		synqf = (uint32_t)(((filter->queue << IXGBE_SYN_FILTER_QUEUE_SHIFT) &
 			IXGBE_SYN_FILTER_QUEUE) | IXGBE_SYN_FILTER_ENABLE);
@@ -5598,10 +5603,13 @@  ixgbe_syn_filter_set(struct rte_eth_dev *dev,
 		else
 			synqf &= ~IXGBE_SYN_FILTER_SYNQFP;
 	} else {
-		if (!(synqf & IXGBE_SYN_FILTER_ENABLE))
+		synqf = IXGBE_READ_REG(hw, IXGBE_SYNQF);
+		if (!(syn_info & IXGBE_SYN_FILTER_ENABLE))
 			return -ENOENT;
 		synqf &= ~(IXGBE_SYN_FILTER_QUEUE | IXGBE_SYN_FILTER_ENABLE);
 	}
+
+	filter_info->syn_info = synqf;
 	IXGBE_WRITE_REG(hw, IXGBE_SYNQF, synqf);
 	IXGBE_WRITE_FLUSH(hw);
 	return 0;
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 4ff6338..827026c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -262,6 +262,8 @@  struct ixgbe_filter_info {
 	/* Bit mask for every used 5tuple filter */
 	uint32_t fivetuple_mask[IXGBE_5TUPLE_ARRAY_SIZE];
 	struct ixgbe_5tuple_filter_list fivetuple_list;
+	/* store the SYN filter info */
+	uint32_t syn_info;
 };
 
 /*