Hi Helin:
> -----Original Message-----
> From: Zhang, Helin
> Sent: Tuesday, January 17, 2017 9:31 AM
> To: Zhang, Qi Z <qi.z.zh...@intel.com>; Lu, Wenzhuo
> <wenzhuo...@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2] net/ixgbe: fix interrupt block issue
>
>
>
> > -----Original Message-----
> > From: Zhang, Qi Z
> > Sent: Tuesday, January 17, 2017 2:02 AM
> > To: Lu, Wenzhuo; Zhang, Helin
> > Cc: dev@dpdk.org; Zhang, Qi Z
> > Subject: [PATCH v2] net/ixgbe: fix interrupt block issue
> >
> > When handle link status change interrupt, interrupt will be blocked
> > until delayed handler finish, the duration is at least 1 second, this
> > may cause following VF to PF mailbox traffic be blocked and sometimes
> > PF can't ack to VF in time before VF think it's time out.
> > This patch remove this limitation, interrupt will be enabled before
> > interrupt handler finish, and a flag is used to prevent re-entering delayed
> handler.
> >
> > Fixes: 0a45657a6794 ("pci: rework interrupt handling")
> >
> > Signed-off-by: Qi Zhang <qi.z.zh...@intel.com>
> > ---
> > v2:
> > - rebase to dpdk-next-net
> >
> > drivers/net/ixgbe/ixgbe_ethdev.c | 19 +++++++++----------
> > drivers/net/ixgbe/ixgbe_ethdev.h | 2 ++
> > 2 files changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index c66f432..f36ce89 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -3762,7 +3762,6 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev
> > *dev,
> > IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
> > int64_t timeout;
> > struct rte_eth_link link;
> > - int intr_enable_delay = false;
> > struct ixgbe_hw *hw =
> > IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> >
> > @@ -3778,7 +3777,7 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev
> > *dev,
> > intr->flags &= ~IXGBE_FLAG_PHY_INTERRUPT;
> > }
> >
> > - if (intr->flags & IXGBE_FLAG_NEED_LINK_UPDATE) {
> > + if (intr->flags & IXGBE_FLAG_NEED_LINK_UPDATE && !intr->delay) {
> > /* get the link status before link update, for predicting later
> > */
> > memset(&link, 0, sizeof(link));
> > rte_ixgbe_dev_atomic_read_link_status(dev, &link); @@ -
> > 3795,20 +3794,16 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev
> *dev,
> > timeout = IXGBE_LINK_DOWN_CHECK_TIMEOUT;
> >
> > ixgbe_dev_link_status_print(dev);
> > + intr->delay = true;
> >
> > - intr_enable_delay = true;
> > - }
> > -
> > - if (intr_enable_delay) {
> > if (rte_eal_alarm_set(timeout * 1000,
> > ixgbe_dev_interrupt_delayed_handler,
> > (void *)dev) < 0)
> > PMD_DRV_LOG(ERR, "Error setting alarm");
> > - } else {
> > - PMD_DRV_LOG(DEBUG, "enable intr immediately");
> > - ixgbe_enable_intr(dev);
> > - rte_intr_enable(intr_handle);
> > }
> >
> > + PMD_DRV_LOG(DEBUG, "enable intr immediately");
> > + ixgbe_enable_intr(dev);
> > + rte_intr_enable(intr_handle);
> I think we should still disable LSC interrupt before the delayed handling, to
> avoid any chaos on potential too many interrupts being reported.
> Of cause other interrupts can be enabled here, to fix the issue you found.
Agree, will add this in v3.
>
> Thanks,
> Helin
