[dpdk-dev,4/5] net/ixgbevf: add check for rte_intr_enable.
Checks
Commit Message
When we bind the ixgbevf to vfio and call the rte_eth_dev_rx_intr_enable
and rte_eth_dev_rx_intr_disable frequently, the interrupt setting
(msi_set_mask_bit) will take more CPU as show below. rte_intr_enable
call 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.
This patch add checks for using VFIO. With the patch, msi_set_mask_bit is
not listed in perl any more. Any suggestion will be welcome.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
Comments
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tonghao Zhang
> Sent: Friday, January 5, 2018 10:11 PM
> To: dev@dpdk.org
> Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Subject: [dpdk-dev] [PATCH 4/5] net/ixgbevf: add check for rte_intr_enable.
The patch is not only for ixgbevf, but also for ixgbe, right?
so how about changing the title with net/ixgbe started?
> When we bind the ixgbevf to vfio and call the rte_eth_dev_rx_intr_enable
> and rte_eth_dev_rx_intr_disable frequently, the interrupt setting
> (msi_set_mask_bit) will take more CPU as show below. rte_intr_enable call
> 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.
>
> This patch add checks for using VFIO. With the patch, msi_set_mask_bit is
> not listed in perl any more. Any suggestion will be welcome.
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index e929235..79e4097 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -5610,7 +5610,9 @@ 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);
> + if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
> + intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
> + rte_intr_enable(intr_handle);
For igb_uio, did you check if it's necessary to call rte_intr_enable every time? Since rte interrupt is not disabled during ixgbevf_dev_rx_queue_intr_disable.
>
> return 0;
> }
> @@ -5659,7 +5661,10 @@ 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);
> +
> + if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
> + intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
> + rte_intr_enable(intr_handle);
The same comment as above.
>
> return 0;
> }
> --
> 1.8.3.1
On Thu, Jan 11, 2018 at 3:06 PM, Xing, Beilei <beilei.xing@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tonghao Zhang
>> Sent: Friday, January 5, 2018 10:11 PM
>> To: dev@dpdk.org
>> Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> Subject: [dpdk-dev] [PATCH 4/5] net/ixgbevf: add check for rte_intr_enable.
>
> The patch is not only for ixgbevf, but also for ixgbe, right?
> so how about changing the title with net/ixgbe started?
>
OK
>> When we bind the ixgbevf to vfio and call the rte_eth_dev_rx_intr_enable
>> and rte_eth_dev_rx_intr_disable frequently, the interrupt setting
>> (msi_set_mask_bit) will take more CPU as show below. rte_intr_enable call
>> 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.
>>
>> This patch add checks for using VFIO. With the patch, msi_set_mask_bit is
>> not listed in perl any more. Any suggestion will be welcome.
>>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> ---
>> drivers/net/ixgbe/ixgbe_ethdev.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index e929235..79e4097 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -5610,7 +5610,9 @@ 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);
>> + if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
>> + intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
>> + rte_intr_enable(intr_handle);
>
> For igb_uio, did you check if it's necessary to call rte_intr_enable every time? Since rte interrupt is not disabled during ixgbevf_dev_rx_queue_intr_disable.
good, remove the rte_intr_enable directly, because intr has been
disable/enable via writing the register in
ixgbevf_dev_rx_queue_intr_disable/enable.
>>
>> return 0;
>> }
>> @@ -5659,7 +5661,10 @@ 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);
>> +
>> + if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
>> + intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
>> + rte_intr_enable(intr_handle);
>
> The same comment as above.
>
>>
>> return 0;
>> }
>> --
>> 1.8.3.1
>
Hi, Tonghao
Thanks for your patch.
It looks that same change can be applied to ixgbe_dev_interrupt_action( )
and ixgbe_dev_interrupt_delayed_handler( ).
Anyway, you can test all these changes with example/l3fwd-power.
-Wei
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xing, Beilei
> Sent: Thursday, January 11, 2018 3:06 PM
> To: Tonghao Zhang <xiangxia.m.yue@gmail.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 4/5] net/ixgbevf: add check for
> rte_intr_enable.
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tonghao Zhang
> > Sent: Friday, January 5, 2018 10:11 PM
> > To: dev@dpdk.org
> > Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > Subject: [dpdk-dev] [PATCH 4/5] net/ixgbevf: add check for
> rte_intr_enable.
>
> The patch is not only for ixgbevf, but also for ixgbe, right?
> so how about changing the title with net/ixgbe started?
>
> > When we bind the ixgbevf to vfio and call the
> > rte_eth_dev_rx_intr_enable and rte_eth_dev_rx_intr_disable frequently,
> > the interrupt setting
> > (msi_set_mask_bit) will take more CPU as show below. rte_intr_enable
> > call 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.
> >
> > This patch add checks for using VFIO. With the patch,
> > msi_set_mask_bit is not listed in perl any more. Any suggestion will be
> welcome.
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > ---
> > drivers/net/ixgbe/ixgbe_ethdev.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index e929235..79e4097 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -5610,7 +5610,9 @@ 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);
> > + if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
> > + intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
> > + rte_intr_enable(intr_handle);
>
> For igb_uio, did you check if it's necessary to call rte_intr_enable every time?
> Since rte interrupt is not disabled during ixgbevf_dev_rx_queue_intr_disable.
>
> >
> > return 0;
> > }
> > @@ -5659,7 +5661,10 @@ 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);
> > +
> > + if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
> > + intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
> > + rte_intr_enable(intr_handle);
>
> The same comment as above.
>
> >
> > return 0;
> > }
> > --
> > 1.8.3.1
On Fri, Jan 12, 2018 at 8:10 PM, Dai, Wei <wei.dai@intel.com> wrote:
> Hi, Tonghao
> Thanks for your patch.
> It looks that same change can be applied to ixgbe_dev_interrupt_action( )
> and ixgbe_dev_interrupt_delayed_handler( ).
Yes, but the irq (e.g mailbox irq) is not same as rx interrupt
handled frequently.
and it will not affect the performance.
> Anyway, you can test all these changes with example/l3fwd-power.
> -Wei
I tested them with example/l3fwd-power and our apps. It is ok.
I will send v3 to you.
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xing, Beilei
>> Sent: Thursday, January 11, 2018 3:06 PM
>> To: Tonghao Zhang <xiangxia.m.yue@gmail.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 4/5] net/ixgbevf: add check for
>> rte_intr_enable.
>>
>>
>>
>> > -----Original Message-----
>> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tonghao Zhang
>> > Sent: Friday, January 5, 2018 10:11 PM
>> > To: dev@dpdk.org
>> > Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> > Subject: [dpdk-dev] [PATCH 4/5] net/ixgbevf: add check for
>> rte_intr_enable.
>>
>> The patch is not only for ixgbevf, but also for ixgbe, right?
>> so how about changing the title with net/ixgbe started?
>>
>> > When we bind the ixgbevf to vfio and call the
>> > rte_eth_dev_rx_intr_enable and rte_eth_dev_rx_intr_disable frequently,
>> > the interrupt setting
>> > (msi_set_mask_bit) will take more CPU as show below. rte_intr_enable
>> > call 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.
>> >
>> > This patch add checks for using VFIO. With the patch,
>> > msi_set_mask_bit is not listed in perl any more. Any suggestion will be
>> welcome.
>> >
>> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> > ---
>> > drivers/net/ixgbe/ixgbe_ethdev.c | 9 +++++++--
>> > 1 file changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> > b/drivers/net/ixgbe/ixgbe_ethdev.c
>> > index e929235..79e4097 100644
>> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> > @@ -5610,7 +5610,9 @@ 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);
>> > + if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
>> > + intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
>> > + rte_intr_enable(intr_handle);
>>
>> For igb_uio, did you check if it's necessary to call rte_intr_enable every time?
>> Since rte interrupt is not disabled during ixgbevf_dev_rx_queue_intr_disable.
>>
>> >
>> > return 0;
>> > }
>> > @@ -5659,7 +5661,10 @@ 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);
>> > +
>> > + if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
>> > + intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
>> > + rte_intr_enable(intr_handle);
>>
>> The same comment as above.
>>
>> >
>> > return 0;
>> > }
>> > --
>> > 1.8.3.1
>
@@ -5610,7 +5610,9 @@ 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);
+ if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
+ intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
+ rte_intr_enable(intr_handle);
return 0;
}
@@ -5659,7 +5661,10 @@ 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);
+
+ if (intr_handle->type == RTE_INTR_HANDLE_UIO ||
+ intr_handle->type == RTE_INTR_HANDLE_UIO_INTX)
+ rte_intr_enable(intr_handle);
return 0;
}