[dpdk-dev] [PATCH v2] net/ixgbe: fix interrupt block issue

Zhang, Qi Z qi.z.zhang at intel.com
Tue Jan 17 03:16:20 CET 2017


Hi Helin:

> -----Original Message-----
> From: Zhang, Helin
> Sent: Tuesday, January 17, 2017 9:31 AM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>; Lu, Wenzhuo
> <wenzhuo.lu at intel.com>
> Cc: dev at 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 at 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.zhang at 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


More information about the dev mailing list