From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 45DF24CA7; Thu, 7 Mar 2019 14:39:25 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Mar 2019 05:39:24 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,451,1544515200"; d="scan'208";a="280585420" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga004.jf.intel.com with ESMTP; 07 Mar 2019 05:39:23 -0800 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 7 Mar 2019 05:39:23 -0800 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 7 Mar 2019 05:39:23 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.134]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.57]) with mapi id 14.03.0415.000; Thu, 7 Mar 2019 21:38:04 +0800 From: "Zhang, Qi Z" To: wangyunjian , "dev@dpdk.org" CC: xudingke , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] net/ixgbe: fix crash when on remove Thread-Index: AQHUw0bKc7SJagazuUKnaI8tvTQ2DaXxogcA//+ZGACADxHX4A== Date: Thu, 7 Mar 2019 13:38:04 +0000 Message-ID: <039ED4275CED7440929022BC67E7061153349104@SHSMSX103.ccr.corp.intel.com> References: <1550026132-9244-1-git-send-email-wangyunjian@huawei.com> <039ED4275CED7440929022BC67E706115333A9D7@SHSMSX103.ccr.corp.intel.com> <34EFBCA9F01B0748BEB6B629CE643AE60CA392C3@DGGEMM533-MBX.china.huawei.com> In-Reply-To: <34EFBCA9F01B0748BEB6B629CE643AE60CA392C3@DGGEMM533-MBX.china.huawei.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNWUwMDI1ZjYtMjg3Zi00Mzk4LWE0NjUtNzllY2M5MjM4MjY2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiWnhUNVwvUndaNDhmQ3ZsR1hQelh3d21yMm1XZWRIRVJrOUF3eG5RMVJ2clRER29HcitFdkVKZkxBNnc3TmJhTUQifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix crash when on remove X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Mar 2019 13:39:26 -0000 > -----Original Message----- > From: wangyunjian [mailto:wangyunjian@huawei.com] > Sent: Tuesday, February 26, 2019 3:22 PM > To: Zhang, Qi Z ; dev@dpdk.org > Cc: xudingke ; stable@dpdk.org > Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: fix crash when on remove >=20 >=20 >=20 > > -----Original Message----- > > From: Zhang, Qi Z [mailto:qi.z.zhang@intel.com] > > Sent: Tuesday, February 26, 2019 1:35 PM > > To: wangyunjian ; dev@dpdk.org > > Cc: xudingke ; stable@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: fix crash when on remove > > > > 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 ; xudingke@huawei.com; Yunjian > > > Wang ; stable@dpdk.org > > > Subject: [dpdk-dev] [PATCH] net/ixgbe: fix crash when on remove > > > > > > From: Yunjian Wang > > > > > > 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=3D0x0, hw=3D0x0) > > > at /usr/src/debug/dpdk-18.11/drivers/net/ixgbe/ixgbe_ethdev.c:8= 52 > > > #1 ixgbe_dev_interrupt_delayed_handler (param=3D0xadb9c0 > > > ) > > > at > /usr/src/debug/dpdk-18.11/drivers/net/ixgbe/ixgbe_ethdev.c:4386 > > > #2 0x00007f05782147af in eal_alarm_callback (arg=3D= ) > > > at /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/ > > > eal_alarm.c:90 > > > #3 0x00007f057821320a in eal_intr_process_interrupts (nfds=3D1, > > > events=3D0x7f056cbf3e88) at /usr/src/debug/dpdk-18.11/lib/ > > > librte_eal/linuxapp/eal/eal_interrupts.c:838 > > > #4 eal_intr_handle_interrupts (totalfds=3D, pfd=3D1= 8) > > > at /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/ > > > eal_interrupts.c:885 > > > #5 eal_intr_thread_main (arg=3D) > > > at /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/ > > > eal_interrupts.c:965 > > > #6 0x00007f05708a0e45 in start_thread () from /usr/lib64/libpthrea= d.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 > > > --- > > > 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 >=20 > The delay handler callback was canceled after unregistered interrupt hand= ler and > the interrupt handler callback was unregistered after disabled uio. So I = added the > call after rte_intr_callback_unregister. >=20 > I am not sure if we can add the disable uio, unregister interrupt handler= at the > beginning of dev_close. OK, I think the idea scenario is Disable interrupt -> unregister interrupt handler -> unregister delayed han= dler -> reset ethdev ... Now we reset ethdev too early give potential risk in interrupt handler, but= that could be fixed by a separate patch. So=20 Acked-by: Qi Zhang Applied to dpdk-next-net-intel. Thanks Qi >=20 > Thanks > Yunjian > > > > > > > /* uninitialize PF if max_vfs not zero */ > > > ixgbe_pf_host_uninit(eth_dev); > > > > > > -- > > > 1.8.3.1 > > >