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 ?
>> >> # >> # 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. >> */ >> >