[dpdk-dev,v1,06/11] ixgbe: fix rx intr compatible issue with PF mbox
Commit Message
When ixgbe runs as a PF, mbox interrupt is prerequisite to make VF start normally.
And PF sometimes won't 'dev_start', so the mbox interrupt register during 'dev_init' is required.
The patch rolls back the interrupt register for mbox,lsc to the 'dev_init'.
As UIO doesn't support multiple vector, mbox has to occupy the only one.
It adds condition check on 'dev_start', rxq interrupt is not allowed when PF running in IOV mode via UIO.
Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 45 +++++++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 14 deletions(-)
Comments
On Thu, Sep 24, 2015 at 7:33 AM, Cunming Liang <cunming.liang@intel.com>
wrote:
> When ixgbe runs as a PF, mbox interrupt is prerequisite to make VF start
> normally.
> And PF sometimes won't 'dev_start', so the mbox interrupt register during
> 'dev_init' is required.
>
Can you describe the cases/situations where you would want a device to
handle interrupts while not started ?
Thanks.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Marchand
> Sent: Monday, November 02, 2015 4:03 PM
> To: Liang, Cunming
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 06/11] ixgbe: fix rx intr compatible issue with PF mbox
>
> On Thu, Sep 24, 2015 at 7:33 AM, Cunming Liang <cunming.liang@intel.com>
> wrote:
>
> > When ixgbe runs as a PF, mbox interrupt is prerequisite to make VF start
> > normally.
> > And PF sometimes won't 'dev_start', so the mbox interrupt register during
> > 'dev_init' is required.
> >
>
> Can you describe the cases/situations where you would want a device to
> handle interrupts while not started ?
When PF and VF are both controlled by DPDK process(es).
And user doesn't really want to do any RX/TX through PF - uses PF just to control/configure VF(s).
>
>
> Thanks.
> --
> David Marchand
On Mon, Nov 2, 2015 at 5:09 PM, Ananyev, Konstantin <
konstantin.ananyev@intel.com> wrote:
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Marchand
> > Sent: Monday, November 02, 2015 4:03 PM
> > To: Liang, Cunming
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1 06/11] ixgbe: fix rx intr compatible
> issue with PF mbox
> >
> > On Thu, Sep 24, 2015 at 7:33 AM, Cunming Liang <cunming.liang@intel.com>
> > wrote:
> >
> > > When ixgbe runs as a PF, mbox interrupt is prerequisite to make VF
> start
> > > normally.
> > > And PF sometimes won't 'dev_start', so the mbox interrupt register
> during
> > > 'dev_init' is required.
> > >
> >
> > Can you describe the cases/situations where you would want a device to
> > handle interrupts while not started ?
>
> When PF and VF are both controlled by DPDK process(es).
> And user doesn't really want to do any RX/TX through PF - uses PF just to
> control/configure VF(s).
>
>
Ok, but the user still needs to whitelist the PF (or ensure the PF is not
blacklisted) in one of these processes.
Then, the application would do a "partial" initialisation ?
If you don't want rx/tx, don't poll the port.
Anyway, this is your code :-)
>
> From: David Marchand [mailto:david.marchand@6wind.com]
> Sent: Monday, November 02, 2015 4:22 PM
> To: Ananyev, Konstantin
> Cc: Liang, Cunming; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 06/11] ixgbe: fix rx intr compatible issue with PF mbox
>
> On Mon, Nov 2, 2015 at 5:09 PM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Marchand
> > Sent: Monday, November 02, 2015 4:03 PM
> > To: Liang, Cunming
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1 06/11] ixgbe: fix rx intr compatible issue with PF mbox
> >
> > On Thu, Sep 24, 2015 at 7:33 AM, Cunming Liang <cunming.liang@intel.com>
> > wrote:
> >
> > > When ixgbe runs as a PF, mbox interrupt is prerequisite to make VF start
> > > normally.
> > > And PF sometimes won't 'dev_start', so the mbox interrupt register during
> > > 'dev_init' is required.
> > >
> >
> > Can you describe the cases/situations where you would want a device to
> > handle interrupts while not started ?
>
> When PF and VF are both controlled by DPDK process(es).
> And user doesn't really want to do any RX/TX through PF - uses PF just to control/configure VF(s).
>
> Ok, but the user still needs to whitelist the PF (or ensure the PF is not blacklisted) in one of these processes.
Yes, at least dev_init() need to be called for that device.
> Then, the application would do a "partial" initialisation ?
Yep, sort of.
> If you don't want rx/tx, don't poll the port.
Well, the question is why to add an extra restriction here?
Probably user deliberately doesn't want to call dev_start() for PF device -
as he doesn't plan to use it for RX/TX.
Or might be dev_stop() was called just to do some re-configuration
(allow to TX scattered packets on the PF queues or so).
Or dev_start() for PF has not yet been called.
Why VF should stop working properly because of that?
Konstantin
> Anyway, this is your code :-)
>
>
> --
> David Marchand
On Mon, Nov 2, 2015 at 5:41 PM, Ananyev, Konstantin <
konstantin.ananyev@intel.com> wrote:
> > If you don't want rx/tx, don't poll the port.
>
> Well, the question is why to add an extra restriction here?
>
Well, until I start a port, I would expect it to do nothing.
Probably user deliberately doesn't want to call dev_start() for PF device -
> as he doesn't plan to use it for RX/TX.
> Or might be dev_stop() was called just to do some re-configuration
> (allow to TX scattered packets on the PF queues or so).
> Or dev_start() for PF has not yet been called.
> Why VF should stop working properly because of that?
>
Why not.
>>> If you don't want rx/tx, don't poll the port.
>>Well, the question is why to add an extra restriction here?
>Well, until I start a port, I would expect it to do nothing.
>>Probably user deliberately doesn't want to call dev_start() for PF device -
>>as he doesn't plan to use it for RX/TX.
>>Or might be dev_stop() was called just to do some re-configuration
>>(allow to TX scattered packets on the PF queues or so).
>>Or dev_start() for PF has not yet been called.
>>Why VF should stop working properly because of that?
>Why not.
I thought I explained it above.
Basically it means that you can't stop your PF without forcing to stop all VFs first.
And you can't start any of your VFs without starting PF first.
I think that adds an unnecessary restrictions and limits systems availability quite significantly.
Konstantin
On Nov 2, 2015 6:23 PM, "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
wrote:
> >>Probably user deliberately doesn't want to call dev_start() for PF
device -
> >>as he doesn't plan to use it for RX/TX.
> >>Or might be dev_stop() was called just to do some re-configuration
> >>(allow to TX scattered packets on the PF queues or so).
> >>Or dev_start() for PF has not yet been called.
> >>Why VF should stop working properly because of that?
>
> >Why not.
>
> I thought I explained it above.
Sorry wrong choice of words with this "Why not".
I agree with you for this case.
@@ -1028,6 +1028,13 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
eth_dev->data->port_id, pci_dev->id.vendor_id,
pci_dev->id.device_id);
+ rte_intr_callback_register(&(pci_dev->intr_handle),
+ ixgbe_dev_interrupt_handler,
+ (void *)eth_dev);
+
+ /* enable uio/vfio intr/eventfd mapping */
+ rte_intr_enable(&(pci_dev->intr_handle));
+
/* enable support intr */
ixgbe_enable_intr(eth_dev);
@@ -1704,17 +1711,19 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
ixgbe_pf_host_configure(dev);
/* check and configure queue intr-vector mapping */
- if (dev->data->dev_conf.intr_conf.rxq != 0)
+ if (((RTE_ETH_DEV_SRIOV(dev).active &&
+ rte_intr_cap_multiple(intr_handle)) ||
+ !RTE_ETH_DEV_SRIOV(dev).active) &&
+ dev->data->dev_conf.intr_conf.rxq != 0) {
intr_vector = dev->data->nb_rx_queues;
-
- if (rte_intr_efd_enable(intr_handle, intr_vector))
- return -1;
+ if (rte_intr_efd_enable(intr_handle, intr_vector))
+ return -1;
+ }
if (rte_intr_dp_is_en(intr_handle) && !intr_handle->intr_vec) {
intr_handle->intr_vec =
rte_zmalloc("intr_vec",
- dev->data->nb_rx_queues * sizeof(int),
- 0);
+ dev->data->nb_rx_queues * sizeof(int), 0);
if (intr_handle->intr_vec == NULL) {
PMD_INIT_LOG(ERR, "Failed to allocate %d rx_queues"
" intr_vec\n", dev->data->nb_rx_queues);
@@ -1801,20 +1810,22 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
skip_link_setup:
- /* check if lsc interrupt is enabled */
- if (dev->data->dev_conf.intr_conf.lsc != 0) {
- if (rte_intr_allow_others(intr_handle)) {
- rte_intr_callback_register(intr_handle,
- ixgbe_dev_interrupt_handler,
- (void *)dev);
+ if (rte_intr_allow_others(intr_handle)) {
+ /* check if lsc interrupt is enabled */
+ if (dev->data->dev_conf.intr_conf.lsc != 0)
ixgbe_dev_lsc_interrupt_setup(dev);
- } else
+ } else {
+ rte_intr_callback_unregister(intr_handle,
+ ixgbe_dev_interrupt_handler,
+ (void *)dev);
+ if (dev->data->dev_conf.intr_conf.lsc != 0)
PMD_INIT_LOG(INFO, "lsc won't enable because of"
" no intr multiplex\n");
}
/* check if rxq interrupt is enabled */
- if (dev->data->dev_conf.intr_conf.rxq != 0)
+ if (dev->data->dev_conf.intr_conf.rxq != 0 &&
+ rte_intr_dp_is_en(intr_handle))
ixgbe_dev_rxq_interrupt_setup(dev);
/* enable uio/vfio intr/eventfd mapping */
@@ -1926,6 +1937,12 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
memset(filter_info->fivetuple_mask, 0,
sizeof(uint32_t) * IXGBE_5TUPLE_ARRAY_SIZE);
+ if (!rte_intr_allow_others(intr_handle))
+ /* resume to the default handler */
+ rte_intr_callback_register(intr_handle,
+ ixgbe_dev_interrupt_handler,
+ (void *)dev);
+
/* Clean datapath event and queue/vec mapping */
rte_intr_efd_disable(intr_handle);
if (intr_handle->intr_vec != NULL) {