From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id C2CDAFBB9 for ; Tue, 20 Dec 2016 17:55:50 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 20 Dec 2016 08:55:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,379,1477983600"; d="scan'208";a="20797070" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.29]) ([10.237.220.29]) by orsmga002.jf.intel.com with ESMTP; 20 Dec 2016 08:55:43 -0800 From: Ferruh Yigit To: Wei Zhao , dev@dpdk.org References: <1480675394-59179-1-git-send-email-wei.zhao1@intel.com> <1480675394-59179-2-git-send-email-wei.zhao1@intel.com> Cc: wenzhuo.lu@intel.com Message-ID: Date: Tue, 20 Dec 2016 16:55:42 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1480675394-59179-2-git-send-email-wei.zhao1@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 01/18] net/ixgbe: store SYN filter X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Dec 2016 16:55:51 -0000 On 12/2/2016 10:42 AM, Wei Zhao wrote: > From: wei zhao1 > > 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 > Signed-off-by: wei zhao1 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; <...>