Hi Tonghao: Would you submit v4 with Ferruh' s fix and also do a rebase on next-net? Thanks Qi
> -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit > Sent: Thursday, April 19, 2018 12:10 AM > To: Tonghao Zhang <xiangxia.m....@gmail.com> > Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Ananyev, Konstantin > <konstantin.anan...@intel.com>; Xing, Beilei <beilei.x...@intel.com>; Dai, > Wei <wei....@intel.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt > interval for EITR. > > On 4/18/2018 2:01 AM, Tonghao Zhang wrote: > > On Tue, Apr 17, 2018 at 7:00 PM, Ferruh Yigit <ferruh.yi...@intel.com> > wrote: > >> On 3/22/2018 1:01 PM, xiangxia.m....@gmail.com wrote: > >>> From: Tonghao Zhang <xiangxia.m....@gmail.com> > >>> > >>> Set EITR interval as default. This patch can improve the performance > >>> when we enable the rx-intrrupt to process the packets because we > >>> hope rx-intrrupt reduce CPU. For example, the 200us value of EITR > >>> makes the performance better with the low CPU. > >>> > >>> Users can configure the value of ITR via DPDK configuration. > >>> > >>> The default value of ITR is 500us, compatible with RSC of ixgbe PF, > >>> and next patch will use the default value. > >>> > >>> Signed-off-by: Tonghao Zhang <xiangxia.m....@gmail.com> > >>> --- > >>> v1 --> v2: > >>> use the configure file, for different user. > >>> suggested by Beilei Xing, http://dpdk.org/dev/patchwork/patch/32989 > >>> --- > >>> config/common_base | 2 ++ > >>> drivers/net/ixgbe/ixgbe_ethdev.c | 7 +++++++ > >>> drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++++++++ > >>> 3 files changed, 21 insertions(+) > >>> > >>> diff --git a/config/common_base b/config/common_base index > >>> e74febe..2e9fded 100644 > >>> --- a/config/common_base > >>> +++ b/config/common_base > >>> @@ -196,6 +196,8 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n > >>> CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n > >>> CONFIG_RTE_IXGBE_INC_VECTOR=y > >>> CONFIG_RTE_LIBRTE_IXGBE_BYPASS=n > >>> +# interval up to 1024 us > >>> +CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL=-1 > >> > >> I can understand this is to let user to ability to fine tune this > >> value, the down side is when number of these kind of tune parameters > >> increased, it become confusing and config options because less useful or > perhaps useless. > >> > >> And overall we are trying to reduce config options we have. > >> > >> I can see there is already some default values in the header file, > >> what do you think removing the config option and go with default values > defined in header? > > v2 used the default value in header, but for configuring it like as > > i40e option (CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL). > > so v3 makes some changes. These patches may have been applied. For > > reducing config option, should we do it in individual patches to fix > > the ixgbe/i40e option ? > > Patches are still in Helin's tree, so this up to him how manage. > Since patches are not in main repo yet I would suggest making new version if > fits to Helin. > > > > >>> > >>> # > >>> # Compile burst-oriented I40E PMD driver diff --git > >>> a/drivers/net/ixgbe/ixgbe_ethdev.c > >>> b/drivers/net/ixgbe/ixgbe_ethdev.c > >>> index e67389f..495e72c 100644 > >>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c > >>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > >>> @@ -5780,6 +5780,13 @@ static void ixgbevf_set_vfta_all(struct > rte_eth_dev *dev, bool on) > >>> if (vector_idx < base + intr_handle->nb_efd - 1) > >>> vector_idx++; > >>> } > >>> + > >>> + /* As RX queue setting above show, all queues use the vector 0. > >>> + * Set only the ITR value of IXGBE_MISC_VEC_ID. > >>> + */ > >>> + IXGBE_WRITE_REG(hw, IXGBE_VTEITR(IXGBE_MISC_VEC_ID), > >>> + > ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL) > >>> + | IXGBE_EITR_CNT_WDIS); > >>> } > >>> > >>> /** > >>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h > >>> b/drivers/net/ixgbe/ixgbe_ethdev.h > >>> index 1db29bd..c779001 100644 > >>> --- a/drivers/net/ixgbe/ixgbe_ethdev.h > >>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h > >>> @@ -58,6 +58,18 @@ > >>> IXGBE_EITR_ITR_INT_MASK) > >>> > >>> > >>> +#define IXGBE_QUEUE_ITR_INTERVAL_MAX 1024 /* 1024us */ > >>> +#define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT 500 /* 500us */ > >>> + > >>> +static inline uint16_t > >>> +ixgbe_calc_itr_interval(int16_t interval) { > >>> + if (interval < 0 || interval > IXGBE_QUEUE_ITR_INTERVAL_MAX) > >>> + interval = IXGBE_QUEUE_ITR_INTERVAL_DEFAULT; > >>> + > >>> + return IXGBE_EITR_INTERVAL_US(interval); } > >>> + > >>> /* Loopback operation modes */ > >>> /* 82599 specific loopback operation types */ > >>> #define IXGBE_LPBK_82599_NONE 0x0 /* Default value. Loopback > is disabled. */ > >>> > >>