Re: [dpdk-dev] [PATCH] net/ixgbe: fix crash when on remove

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

Hi:

> -----Original Message-----
> From: wangyunjian [mailto:wangyunjian@huawei.com]
> Sent: Wednesday, February 13, 2019 10:49 AM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; xudingke@huawei.com; Yunjian Wang
> <wangyunjian@huawei.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/ixgbe: fix crash when on remove
> 
> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> The nic's interrupt source has some active handler, when the port remove. We
> should cancel the delay handler before remove dev to prevent executing the
> delay handler.

Agree, thanks to capture this.

> 
> Call Trace:
>   #0  ixgbe_disable_intr (hw=0x0, hw=0x0)
>       at /usr/src/debug/dpdk-18.11/drivers/net/ixgbe/ixgbe_ethdev.c:852
>   #1  ixgbe_dev_interrupt_delayed_handler (param=0xadb9c0
>       <rte_eth_devices@@DPDK_2.2+33024>)
>       at /usr/src/debug/dpdk-18.11/drivers/net/ixgbe/ixgbe_ethdev.c:4386
>   #2  0x00007f05782147af in eal_alarm_callback (arg=<optimized out>)
>       at /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/
>       eal_alarm.c:90
>   #3  0x00007f057821320a in eal_intr_process_interrupts (nfds=1,
>       events=0x7f056cbf3e88) at /usr/src/debug/dpdk-18.11/lib/
>       librte_eal/linuxapp/eal/eal_interrupts.c:838
>   #4  eal_intr_handle_interrupts (totalfds=<optimized out>, pfd=18)
>       at /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/
>       eal_interrupts.c:885
>   #5  eal_intr_thread_main (arg=<optimized out>)
>       at /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/
>       eal_interrupts.c:965
>   #6  0x00007f05708a0e45 in start_thread () from /usr/lib64/libpthread.so.0
>   #7  0x00007f056eb4ab5d in clone () from /usr/lib64/libc.so.6
> 
> Fixes: 2866c5f1b87e ("ixgbe: support port hotplug")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 7493110..e9533e5 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1336,6 +1336,9 @@ struct rte_ixgbe_xstats_name_off {
>  		rte_delay_ms(100);
>  	} while (retries++ < (10 + IXGBE_LINK_UP_TIME));
> 
> +	/* cancel the delay handler before remove dev */
> +	rte_eal_alarm_cancel(ixgbe_dev_interrupt_delayed_handler, eth_dev);
> +

I think it will be more safe to move this call ahead,
the delayed handler may invoked application callback which may also invoke the ethdev API, but at this moment, we already reset ethdev, we still have chance to get problem, right?
Is it better that we add this call at the beginning of dev_close?

Regards
Qi


>  	/* uninitialize PF if max_vfs not zero */
>  	ixgbe_pf_host_uninit(eth_dev);
> 
> --
> 1.8.3.1
>