[dpdk-dev] [PATCH] net/i40evf: regression fix - reenable interrupts in handler

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jankowski, Konrad
> Sent: Thursday, April 5, 2018 1:12 AM
> To: Zhang, Qi Z; Dai, Wei; Xing, Beilei; Wu, Jingjing; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: regression fix - reenable interrupts
> in handler
> 
> Hi Zhang,
> 
> If you look at the source of interrupt handlers in both the igb_uio and
> uio_pci_generic drivers (look for irqreturn_t type), you will see they can
> disable interrupts immediately after receipt of one.
> It's up to the user to make sure they're re-enabled. Also please compare with
> code in i40e_ethdev.c, which still does this correctly - there's an explicit
> rte_intr_enable(dev->intr_handle) call at the end of
> i40e_dev_interrupt_handler().
> Probably a cleaner approach would be to leave them disabled as is, but only
> enable them for a once-off receipt when sending an AdminQ message, maybe
> that was the assumption here. However I've added some tracing to
> igbuio_pci_irqcontrol() and I'm sure this isn't happening on my system.
> (nothing is enabling those interrupts post device init). Looks like code path
> might be different with the newest igb_uio driver and MSI enabled, but the
> current code will still not work for all cases (like with uio_pci_generic).

Hi Konrad
Please don't use uio_pci_generic, as it is not supported by X710/XL710/XXV710 series,
as below. With VFIO, do you see similar issue?
http://www.dpdk.org/doc/guides/rel_notes/known_issues.html#uio-pci-generic-module-bind-failed-in-x710-xl710-xxv710

> There can also be cases when you have and older igb_uio driver which disables
> interrupts in all cases and running it with with a new DPDK. (vendor provided
> compiled driver) I think for full compatibility we need to keep re-enabling
> those interrupts.
> 
> Regards,
> Konrad
> 
> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Wednesday, March 28, 2018 4:37 AM
> To: Jankowski, Konrad <konrad.jankowski at intel.com>; Dai, Wei
> <wei.dai at intel.com>; Xing, Beilei <beilei.xing at intel.com>; Wu, Jingjing
> <jingjing.wu at intel.com>; dev at dpdk.org
> Subject: RE: [PATCH] net/i40evf: regression fix - reenable interrupts in handler
> 
> Hi Jankowski:
> 
> > -----Original Message-----
> > From: Jankowski, KonradX
> > Sent: Thursday, February 15, 2018 2:33 AM
> > To: Dai, Wei <wei.dai at intel.com>; Xing, Beilei
> > <beilei.xing at intel.com>; Zhang, Qi Z <qi.z.zhang at intel.com>; Wu,
> > Jingjing <jingjing.wu at intel.com>; dev at dpdk.org
> > Cc: Jankowski, KonradX <konradx.jankowski at intel.com>
> > Subject: [PATCH] net/i40evf: regression fix - reenable interrupts in
> > handler
> >
> > Commit 66b8304f removed the rte_intr_enable() call from
> > i40evf_dev_interrupt_handler() as a "bonus". On one of my systems this
> > causes the AdminQ messages to stop beeing delivered to the VF. This
> > results in unability to initialize and use the port. With this patch it works
> again.
> >
> > System in question:
> > Wind River OVP6 running kernel 3.10.58-ovp-rt58-WR6.0.0.13_preempt-rt
> >
> > Signed-off-by: Konrad Jankowski <konrad.jankowski at intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev_vf.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index fd003fe..b927a35 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -1404,6 +1404,7 @@ i40evf_dev_interrupt_handler(void *param)
> >
> >  done:
> >  	i40evf_enable_irq0(hw);
> > +	rte_intr_enable(dev->intr_handle);'
> 
> Would you explain more about why the patch fix the issue?
> Usually we will not accept a fix just because it work but not understand the
> root cause.
> 
> Regards
> Qi
> 
> >  }
> >
> >  static int
> > --
> > 2.5.5
> 
> --------------------------------------------------------------
> Intel Research and Development Ireland Limited Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
> 
> 
> This e-mail and any attachments may contain confidential material for the sole
> use of the intended recipient(s). Any review or distribution by others is strictly
> prohibited. If you are not the intended recipient, please contact the sender
> and delete all copies.