[dpdk-dev] [PATCH v2] net/i40e: fix wrong handle when enable interrupt

[Zhang, Qi Z]

Hi Thomas:
	

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Tuesday, March 21, 2017 8:10 PM
> To: Wu, Jingjing <jingjing.wu at intel.com>; Zhang, Helin
> <helin.zhang at intel.com>
> Cc: dev at dpdk.org; Zhang, Qi Z <qi.z.zhang at intel.com>; stable at dpdk.org
> Subject: [PATCH v2] net/i40e: fix wrong handle when enable interrupt
> 
> In i40e_dev_interrupt_handler, when call rte_intr_enable, the intr_handle is
> the copy when we registered.
> According to interrupt handle framework, if the requirement of intr_handle
> is changed, we need to unregister then register a new copy. This happens on
> i40e driver when bind to vfio-pci, the rte_intr_efd_enable function will
> modify the max_intr according the queue number, so a new copy of
> intr_handle need to be registered.
> Without this fix, we saw lw3fwd-power does not work due to wrong
> interrupt count in vfio_irq_set when set vfio interrupt.
> 
> Fixes: 2ce7a1ed09fc ("net/i40e: localize mapping of ethdev to PCI device")
> 
> Cc: stable at dpdk.org
> 
> Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
> ---
> v2:
> - follow current design, when intr_handle is modified, unregister
>   the old one and register the new one.
> - there should be a patch set to fix on other devices.
> 
>  drivers/net/i40e/i40e_ethdev.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 9c76baa..e7bbea5 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1886,6 +1886,14 @@ i40e_dev_start(struct rte_eth_dev *dev)
>  		ret = rte_intr_efd_enable(intr_handle, intr_vector);
>  		if (ret)
>  			return ret;
> +		 /**
> +		  * intr_handle may be modified in rte_intr_efd_enable
> +		  * so unregster the old one and register the new one.
> +		  */
> +		rte_intr_callback_unregister(intr_handle,
> +			i40e_dev_interrupt_handler, dev);
> +		rte_intr_callback_register(intr_handle,
> +			i40e_dev_interrupt_handler, dev);
To me, this fix looks a little bit weird.
May I know why we need to register an copy of intr_handle ?
What do you think about my previous clean up patch.
http://dpdk.org/dev/patchwork/patch/21529/
Thanks
Qi.
>  	}
> 
>  	if (rte_intr_dp_is_en(intr_handle) && !intr_handle->intr_vec) {
> --
> 2.9.3