* [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.
2018-03-22 13:01 [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve xiangxia.m.yue
@ 2018-03-22 13:01 ` xiangxia.m.yue
2018-04-17 11:00 ` Ferruh Yigit
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 2/5] net/ixgbe: set the ITR via configuration xiangxia.m.yue
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: xiangxia.m.yue @ 2018-03-22 13:01 UTC (permalink / raw)
To: wenzhuo.lu, konstantin.ananyev, beilei.xing, wei.dai; +Cc: dev, Tonghao Zhang
From: Tonghao Zhang <xiangxia.m.yue@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.yue@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
#
# 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. */
--
1.8.3.1
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR xiangxia.m.yue
@ 2018-04-17 11:00 ` Ferruh Yigit
2018-04-18 1:01 ` Tonghao Zhang
0 siblings, 1 reply; 12+ messages in thread
From: Ferruh Yigit @ 2018-04-17 11:00 UTC (permalink / raw)
To: xiangxia.m.yue, wenzhuo.lu, konstantin.ananyev, beilei.xing, wei.dai; +Cc: dev
On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@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.yue@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?
>
> #
> # 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. */
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.
2018-04-17 11:00 ` Ferruh Yigit
@ 2018-04-18 1:01 ` Tonghao Zhang
2018-04-18 16:10 ` Ferruh Yigit
0 siblings, 1 reply; 12+ messages in thread
From: Tonghao Zhang @ 2018-04-18 1:01 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Lu, Wenzhuo, konstantin.ananyev, Xing, Beilei, Dai, Wei, dev
On Tue, Apr 17, 2018 at 7:00 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
>> From: Tonghao Zhang <xiangxia.m.yue@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.yue@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. */
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.
2018-04-18 1:01 ` Tonghao Zhang
@ 2018-04-18 16:10 ` Ferruh Yigit
2018-04-26 13:14 ` Zhang, Qi Z
0 siblings, 1 reply; 12+ messages in thread
From: Ferruh Yigit @ 2018-04-18 16:10 UTC (permalink / raw)
To: Tonghao Zhang
Cc: Lu, Wenzhuo, konstantin.ananyev, Xing, Beilei, Dai, Wei, dev
On 4/18/2018 2:01 AM, Tonghao Zhang wrote:
> On Tue, Apr 17, 2018 at 7:00 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>> On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
>>> From: Tonghao Zhang <xiangxia.m.yue@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.yue@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. */
>>>
>>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.
2018-04-18 16:10 ` Ferruh Yigit
@ 2018-04-26 13:14 ` Zhang, Qi Z
2018-04-26 13:54 ` Tonghao Zhang
0 siblings, 1 reply; 12+ messages in thread
From: Zhang, Qi Z @ 2018-04-26 13:14 UTC (permalink / raw)
To: Tonghao Zhang; +Cc: dev, Zhang, Helin, Yigit, Ferruh
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-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Thursday, April 19, 2018 12:10 AM
> To: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Dai,
> Wei <wei.dai@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.yigit@intel.com>
> wrote:
> >> On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
> >>> From: Tonghao Zhang <xiangxia.m.yue@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.yue@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. */
> >>>
> >>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.
2018-04-26 13:14 ` Zhang, Qi Z
@ 2018-04-26 13:54 ` Tonghao Zhang
0 siblings, 0 replies; 12+ messages in thread
From: Tonghao Zhang @ 2018-04-26 13:54 UTC (permalink / raw)
To: Zhang, Qi Z; +Cc: dev, Zhang, Helin, Yigit, Ferruh
On Thu, Apr 26, 2018 at 9:14 PM, Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> Hi Tonghao:
> Would you submit v4 with Ferruh' s fix and also do a rebase on next-net?
Yes
> Thanks
> Qi
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Thursday, April 19, 2018 12:10 AM
>> To: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Dai,
>> Wei <wei.dai@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.yigit@intel.com>
>> wrote:
>> >> On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
>> >>> From: Tonghao Zhang <xiangxia.m.yue@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.yue@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. */
>> >>>
>> >>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [dpdk-dev] [PATCH v3 2/5] net/ixgbe: set the ITR via configuration.
2018-03-22 13:01 [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve xiangxia.m.yue
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR xiangxia.m.yue
@ 2018-03-22 13:01 ` xiangxia.m.yue
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 3/5] net/ixgbe: write disable to ITR counter xiangxia.m.yue
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: xiangxia.m.yue @ 2018-03-22 13:01 UTC (permalink / raw)
To: wenzhuo.lu, konstantin.ananyev, beilei.xing, wei.dai; +Cc: dev, Tonghao Zhang
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
With this patch, the ITR value of ixgbe PF, can be
configured as wanted.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 5 +----
drivers/net/ixgbe/ixgbe_rxtx.c | 3 ++-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 495e72c..14296ea 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -60,9 +60,6 @@
*/
#define IXGBE_FC_LO 0x40
-/* Default minimum inter-interrupt interval for EITR configuration */
-#define IXGBE_MIN_INTER_INTERRUPT_INTERVAL_DEFAULT 0x79E
-
/* Timer value included in XOFF frames. */
#define IXGBE_FC_PAUSE 0x680
@@ -5856,7 +5853,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
break;
}
IXGBE_WRITE_REG(hw, IXGBE_EITR(IXGBE_MISC_VEC_ID),
- IXGBE_MIN_INTER_INTERRUPT_INTERVAL_DEFAULT & 0xFFF);
+ ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL));
/* set up to autoclear timer, and the vectors */
mask = IXGBE_EIMS_ENABLE_MASK;
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 9bc8462..cb51f46 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -4695,7 +4695,8 @@ void __attribute__((cold))
* at most 500us latency for a single RSC aggregation.
*/
eitr &= ~IXGBE_EITR_ITR_INT_MASK;
- eitr |= IXGBE_EITR_INTERVAL_US(500) | IXGBE_EITR_CNT_WDIS;
+ eitr |= ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL);
+ eitr |= IXGBE_EITR_CNT_WDIS;
IXGBE_WRITE_REG(hw, IXGBE_SRRCTL(rxq->reg_idx), srrctl);
IXGBE_WRITE_REG(hw, IXGBE_RSCCTL(rxq->reg_idx), rscctl);
--
1.8.3.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [dpdk-dev] [PATCH v3 3/5] net/ixgbe: write disable to ITR counter.
2018-03-22 13:01 [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve xiangxia.m.yue
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR xiangxia.m.yue
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 2/5] net/ixgbe: set the ITR via configuration xiangxia.m.yue
@ 2018-03-22 13:01 ` xiangxia.m.yue
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 4/5] net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance xiangxia.m.yue
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 5/5] net/ixgbe: remove the unnecessary call rte_intr_enable xiangxia.m.yue
4 siblings, 0 replies; 12+ messages in thread
From: xiangxia.m.yue @ 2018-03-22 13:01 UTC (permalink / raw)
To: wenzhuo.lu, konstantin.ananyev, beilei.xing, wei.dai; +Cc: dev, Tonghao Zhang
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
ixgbe doesn't write the ITR counter, disable it now.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 14296ea..0ea8fe8 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5853,7 +5853,8 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
break;
}
IXGBE_WRITE_REG(hw, IXGBE_EITR(IXGBE_MISC_VEC_ID),
- ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL));
+ ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL)
+ | IXGBE_EITR_CNT_WDIS);
/* set up to autoclear timer, and the vectors */
mask = IXGBE_EIMS_ENABLE_MASK;
--
1.8.3.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [dpdk-dev] [PATCH v3 4/5] net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
2018-03-22 13:01 [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve xiangxia.m.yue
` (2 preceding siblings ...)
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 3/5] net/ixgbe: write disable to ITR counter xiangxia.m.yue
@ 2018-03-22 13:01 ` xiangxia.m.yue
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 5/5] net/ixgbe: remove the unnecessary call rte_intr_enable xiangxia.m.yue
4 siblings, 0 replies; 12+ messages in thread
From: xiangxia.m.yue @ 2018-03-22 13:01 UTC (permalink / raw)
To: wenzhuo.lu, konstantin.ananyev, beilei.xing, wei.dai; +Cc: dev, Tonghao Zhang
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
If dpdk APPs call the rte_eth_dev_rx_intr_enable or
rte_eth_dev_rx_intr_disable frequently, and ixgbe vf will read
the IXGBE_VTEIMS register everytime. The patch saves the IXGBE_VTEIMS
to mask to avoid read frequently.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Acked-by: Beilei Xing <beilei.xing@intel.com>
Acked-by: Wei Dai <wei.dai@intel.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 55 +++++++++++++++++++++++++---------------
1 file changed, 35 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0ea8fe8..7928144 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -238,8 +238,8 @@ static int ixgbevf_dev_link_update(struct rte_eth_dev *dev,
static void ixgbevf_dev_stop(struct rte_eth_dev *dev);
static void ixgbevf_dev_close(struct rte_eth_dev *dev);
static int ixgbevf_dev_reset(struct rte_eth_dev *dev);
-static void ixgbevf_intr_disable(struct ixgbe_hw *hw);
-static void ixgbevf_intr_enable(struct ixgbe_hw *hw);
+static void ixgbevf_intr_disable(struct rte_eth_dev *dev);
+static void ixgbevf_intr_enable(struct rte_eth_dev *dev);
static int ixgbevf_dev_stats_get(struct rte_eth_dev *dev,
struct rte_eth_stats *stats);
static void ixgbevf_dev_stats_reset(struct rte_eth_dev *dev);
@@ -1633,7 +1633,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
ixgbevf_dev_stats_reset(eth_dev);
/* Disable the interrupts for VF */
- ixgbevf_intr_disable(hw);
+ ixgbevf_intr_disable(eth_dev);
hw->mac.num_rar_entries = 128; /* The MAX of the underlying PF */
diag = hw->mac.ops.reset_hw(hw);
@@ -1702,7 +1702,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
rte_intr_callback_register(intr_handle,
ixgbevf_dev_interrupt_handler, eth_dev);
rte_intr_enable(intr_handle);
- ixgbevf_intr_enable(hw);
+ ixgbevf_intr_enable(eth_dev);
PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
eth_dev->data->port_id, pci_dev->id.vendor_id,
@@ -1735,7 +1735,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
eth_dev->tx_pkt_burst = NULL;
/* Disable the interrupts for VF */
- ixgbevf_intr_disable(hw);
+ ixgbevf_intr_disable(eth_dev);
rte_free(eth_dev->data->mac_addrs);
eth_dev->data->mac_addrs = NULL;
@@ -4921,19 +4921,32 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
* Virtual Function operations
*/
static void
-ixgbevf_intr_disable(struct ixgbe_hw *hw)
+ixgbevf_intr_disable(struct rte_eth_dev *dev)
{
+ struct ixgbe_interrupt *intr =
+ IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+ struct ixgbe_hw *hw =
+ IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
PMD_INIT_FUNC_TRACE();
/* Clear interrupt mask to stop from interrupts being generated */
IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, IXGBE_VF_IRQ_CLEAR_MASK);
IXGBE_WRITE_FLUSH(hw);
+
+ /* Clear mask value. */
+ intr->mask = 0;
}
static void
-ixgbevf_intr_enable(struct ixgbe_hw *hw)
+ixgbevf_intr_enable(struct rte_eth_dev *dev)
{
+ struct ixgbe_interrupt *intr =
+ IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+ struct ixgbe_hw *hw =
+ IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
PMD_INIT_FUNC_TRACE();
/* VF enable interrupt autoclean */
@@ -4942,6 +4955,9 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, IXGBE_VF_IRQ_ENABLE_MASK);
IXGBE_WRITE_FLUSH(hw);
+
+ /* Save IXGBE_VTEIMS value to mask. */
+ intr->mask = IXGBE_VF_IRQ_ENABLE_MASK;
}
static int
@@ -5067,7 +5083,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
rte_intr_enable(intr_handle);
/* Re-enable interrupt for VF */
- ixgbevf_intr_enable(hw);
+ ixgbevf_intr_enable(dev);
return 0;
}
@@ -5081,7 +5097,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
PMD_INIT_FUNC_TRACE();
- ixgbevf_intr_disable(hw);
+ ixgbevf_intr_disable(dev);
hw->adapter_stopped = 1;
ixgbe_stop_adapter(hw);
@@ -5579,17 +5595,17 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
{
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
- uint32_t mask;
+ struct ixgbe_interrupt *intr =
+ IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
struct ixgbe_hw *hw =
IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
uint32_t vec = IXGBE_MISC_VEC_ID;
- mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
if (rte_intr_allow_others(intr_handle))
vec = IXGBE_RX_VEC_START;
- mask |= (1 << vec);
+ intr->mask |= (1 << vec);
RTE_SET_USED(queue_id);
- IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
+ IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
rte_intr_enable(intr_handle);
@@ -5599,19 +5615,19 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
static int
ixgbevf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id)
{
- uint32_t mask;
+ struct ixgbe_interrupt *intr =
+ IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
struct ixgbe_hw *hw =
IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
uint32_t vec = IXGBE_MISC_VEC_ID;
- mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
if (rte_intr_allow_others(intr_handle))
vec = IXGBE_RX_VEC_START;
- mask &= ~(1 << vec);
+ intr->mask &= ~(1 << vec);
RTE_SET_USED(queue_id);
- IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
+ IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
return 0;
}
@@ -8172,7 +8188,7 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct ixgbe_interrupt *intr =
IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
- ixgbevf_intr_disable(hw);
+ ixgbevf_intr_disable(dev);
/* read-on-clear nic registers here */
eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR);
@@ -8189,7 +8205,6 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
static int
ixgbevf_dev_interrupt_action(struct rte_eth_dev *dev)
{
- struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct ixgbe_interrupt *intr =
IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
@@ -8198,7 +8213,7 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
intr->flags &= ~IXGBE_FLAG_MAILBOX;
}
- ixgbevf_intr_enable(hw);
+ ixgbevf_intr_enable(dev);
return 0;
}
--
1.8.3.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [dpdk-dev] [PATCH v3 5/5] net/ixgbe: remove the unnecessary call rte_intr_enable.
2018-03-22 13:01 [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve xiangxia.m.yue
` (3 preceding siblings ...)
2018-03-22 13:01 ` [dpdk-dev] [PATCH v3 4/5] net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance xiangxia.m.yue
@ 2018-03-22 13:01 ` xiangxia.m.yue
4 siblings, 0 replies; 12+ messages in thread
From: xiangxia.m.yue @ 2018-03-22 13:01 UTC (permalink / raw)
To: wenzhuo.lu, konstantin.ananyev, beilei.xing, wei.dai; +Cc: dev, Tonghao Zhang
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
When binding the ixgbe pf or vf to vfio and call the
rte_eth_dev_rx_intr_enable frequently, the interrupt setting
(msi_set_mask_bit) will take more CPU as show below. rte_intr_enable
calls the ioctl to map the fd to interrupts frequently.
perf top:
5.45% [kernel] [k] msi_set_mask_bit
It is unnecessary to call the rte_intr_enable in
ixgbe_dev_rx_queue_intr_enable. Because the fds has been mapped to
interrupt and not unmapped in ixgbe_dev_rx_queue_intr_disable.
With the patch, msi_set_mask_bit is not listed in perl anymore.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7928144..ef01e0c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4253,7 +4253,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
*/
static int
ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
- struct rte_intr_handle *intr_handle)
+ __rte_unused struct rte_intr_handle *intr_handle)
{
struct ixgbe_interrupt *intr =
IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
@@ -4304,7 +4304,6 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
PMD_DRV_LOG(DEBUG, "enable intr immediately");
ixgbe_enable_intr(dev);
- rte_intr_enable(intr_handle);
return 0;
}
@@ -4327,8 +4326,6 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
ixgbe_dev_interrupt_delayed_handler(void *param)
{
struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
- struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
- struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
struct ixgbe_interrupt *intr =
IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
struct ixgbe_hw *hw =
@@ -4366,7 +4363,6 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
PMD_DRV_LOG(DEBUG, "enable intr in delayed handler S[%08x]", eicr);
ixgbe_enable_intr(dev);
- rte_intr_enable(intr_handle);
}
/**
@@ -5607,8 +5603,6 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
RTE_SET_USED(queue_id);
IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
- rte_intr_enable(intr_handle);
-
return 0;
}
@@ -5635,8 +5629,6 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
static int
ixgbe_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id)
{
- struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
- struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
uint32_t mask;
struct ixgbe_hw *hw =
IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -5656,7 +5648,6 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
mask &= (1 << (queue_id - 32));
IXGBE_WRITE_REG(hw, IXGBE_EIMS_EX(1), mask);
}
- rte_intr_enable(intr_handle);
return 0;
}
--
1.8.3.1
^ permalink raw reply [flat|nested] 12+ messages in thread