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

Ferruh Yigit ferruh.yigit at intel.com
Tue Dec 20 17:55:42 CET 2016


On 12/2/2016 10:42 AM, Wei Zhao wrote:
> From: wei zhao1 <wei.zhao1 at intel.com>
> 
> Add support for storing SYN filter in SW.

Do you think does it makes more clear to refer as TCP SYN filter? Or SYN
filter is clear enough?

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

Can you please update sign-off to your actual name?

> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 12 ++++++++++--
>  drivers/net/ixgbe/ixgbe_ethdev.h |  2 ++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index edc9b22..7f10cca 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1287,6 +1287,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;

can it be an option to memset all filter_info? (and of course move list
init after memset)

>  	return 0;
>  }
>  
> @@ -5509,15 +5511,19 @@ 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;
>  
> +	syn_info = filter_info->syn_info;
>  	synqf = IXGBE_READ_REG(hw, IXGBE_SYNQF);
>  
>  	if (add) {
> -		if (synqf & IXGBE_SYN_FILTER_ENABLE)
> +		if (syn_info & IXGBE_SYN_FILTER_ENABLE)

If these checks will be done on syn_info, shouldn't syn_info be assigned
to synqf before this. Specially for first usage, synqf may be different
than hw register.

Or perhaps can keep continue to use synqf. Since synqf assigned to
filter_info->syn_info after updated.

>  			return -EINVAL;
>  		synqf = (uint32_t)(((filter->queue << IXGBE_SYN_FILTER_QUEUE_SHIFT) &
>  			IXGBE_SYN_FILTER_QUEUE) | IXGBE_SYN_FILTER_ENABLE);
> @@ -5527,10 +5533,12 @@ ixgbe_syn_filter_set(struct rte_eth_dev *dev,
>  		else
>  			synqf &= ~IXGBE_SYN_FILTER_SYNQFP;
>  	} else {
> -		if (!(synqf & IXGBE_SYN_FILTER_ENABLE))
> +		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;
<...>




More information about the dev mailing list