From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR. Date: Wed, 18 Apr 2018 17:10:12 +0100 Message-ID: <4261b067-f381-8857-f4ca-648c3f715c24@intel.com> References: <1521723718-93761-1-git-send-email-xiangxia.m.yue@gmail.com> <1521723718-93761-2-git-send-email-xiangxia.m.yue@gmail.com> <3b7d6b8b-270b-18e4-13a0-3676d02ef686@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "Lu, Wenzhuo" , konstantin.ananyev@intel.com, "Xing, Beilei" , "Dai, Wei" , dev@dpdk.org To: Tonghao Zhang Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 56D2E7D0E for ; Wed, 18 Apr 2018 18:10:16 +0200 (CEST) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 4/18/2018 2:01 AM, Tonghao Zhang wrote: > On Tue, Apr 17, 2018 at 7:00 PM, Ferruh Yigit wrote: >> On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote: >>> From: Tonghao Zhang >>> >>> 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 >>> --- >>> 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. */ >>> >>