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

["Jankowski, Konrad" <konrad.jankowski@intel.com>]

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).
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@intel.com>; Dai, Wei <wei.dai@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; dev@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@intel.com>; Xing, Beilei 
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, 
> Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Cc: Jankowski, KonradX <konradx.jankowski@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@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.