[dpdk-dev] [PATCH v3 28/29] ethdev: reset all when releasing a port

Ferruh Yigit ferruh.yigit at intel.com
Tue Sep 29 17:50:48 CEST 2020


On 9/29/2020 12:58 PM, Wang, Haiyue wrote:
>> -----Original Message-----
>> From: Thomas Monjalon <thomas at monjalon.net>
>> Sent: Tuesday, September 29, 2020 18:36
>> To: Maxime Coquelin <maxime.coquelin at redhat.com>
>> Cc: dev at dpdk.org; Yigit, Ferruh <ferruh.yigit at intel.com>; arybchenko at solarflare.com; Shepard Siegel
>> <shepard.siegel at atomicrules.com>; Ed Czeck <ed.czeck at atomicrules.com>; John Miller
>> <john.miller at atomicrules.com>; Igor Russkikh <igor.russkikh at aquantia.com>; Pavel Belous
>> <pavel.belous at aquantia.com>; Somalapuram Amaranath <asomalap at amd.com>; Ajit Khaparde
>> <ajit.khaparde at broadcom.com>; Somnath Kotur <somnath.kotur at broadcom.com>; Chas Williams
>> <chas3 at att.com>; Wei Hu (Xavier) <xavier.huwei at huawei.com>; Hemant Agrawal <hemant.agrawal at nxp.com>;
>> Sachin Saxena <sachin.saxena at oss.nxp.com>; Guo, Jia <jia.guo at intel.com>; Wang, Haiyue
>> <haiyue.wang at intel.com>; Marcin Wojtas <mw at semihalf.com>; Michal Krawczyk <mk at semihalf.com>; Guy
>> Tzalik <gtzalik at amazon.com>; Evgeny Schemeilin <evgenys at amazon.com>; Igor Chauskin <igorch at amazon.com>;
>> Zhang, Qi Z <qi.z.zhang at intel.com>; Wang, Xiao W <xiao.w.wang at intel.com>; Ziyang Xuan
>> <xuanziyang2 at huawei.com>; Xiaoyun Wang <cloud.wangxiaoyun at huawei.com>; Guoyang Zhou
>> <zhouguoyang at huawei.com>; Min Hu (Connor) <humin29 at huawei.com>; Yisen Zhuang <yisen.zhuang at huawei.com>;
>> Xing, Beilei <beilei.xing at intel.com>; Wu, Jingjing <jingjing.wu at intel.com>; Yang, Qiming
>> <qiming.yang at intel.com>; Alfredo Cardigliano <cardigliano at ntop.org>; Shijith Thotton
>> <sthotton at marvell.com>; Srisivasubramanian Srinivasan <srinivasan at marvell.com>; Stephen Hemminger
>> <sthemmin at microsoft.com>; K. Y. Srinivasan <kys at microsoft.com>; Haiyang Zhang <haiyangz at microsoft.com>;
>> Long Li <longli at microsoft.com>; Harman Kalra <hkalra at marvell.com>; Rasesh Mody <rmody at marvell.com>;
>> Shahed Shaikh <shshaikh at marvell.com>; Wiles, Keith <keith.wiles at intel.com>; Xia, Chenbo
>> <chenbo.xia at intel.com>; Wang, Zhihong <zhihong.wang at intel.com>; Yong Wang <yongwang at vmware.com>;
>> matan at nvidia.com
>> Subject: Re: [PATCH v3 28/29] ethdev: reset all when releasing a port
>>
>> 29/09/2020 12:26, Maxime Coquelin:
>>> On 9/29/20 1:14 AM, Thomas Monjalon wrote:
>>>> The function rte_eth_dev_release_port() was resetting partially
>>>> the struct rte_eth_dev. The drivers were completing it
>>>> with more pointers set to NULL in the close or remove operations.
>>>>
>>>> A full memset is done so most of those assignments become useless.
>> [...]
>>> With this patch, I get following segfault at init time with Virtio PMD:
>>>
>>>          Program received signal SIGSEGV, Segmentation fault.
>>>          0x0000000000854c9b in rte_eth_dev_callback_register (port_id=32,
>>>              event=RTE_ETH_EVENT_UNKNOWN, cb_fn=0x4b24de
>>> <eth_event_callback>,
>>>              cb_arg=0x0) at ../lib/librte_ethdev/rte_ethdev.c:4042
>>>          4042
>>> TAILQ_INSERT_TAIL(&(dev->link_intr_cbs),
>>
>> Yes this is because after closing a port, everything is resetted,
>> including .link_intr_cbs which is set only once in a constructor:
>> http://git.dpdk.org/dpdk/commit/?id=9ec0b3869d8
>>
>> I can change this patch to selectively set pointers to NULL.
>>
>> Or if we prefer a big memset 0, we need to rework how RTE_ETH_ALL
>> is managed to register a callback for any event.
>> Instead of setting the callback for all ports, we could have
>> a special catch-call callback list which is called for all events.
>> This way we could revert initializing .link_intr_cbs in eth_dev_get().
>>
>>
> 
> Move 'struct rte_eth_dev_cb_list link_intr_cbs' to the end of 'struct rte_eth_dev',
> and starting from link_intr_cbs, these members will be kept after closed ? :-)
> 
> memset(eth_dev, 0, offsetof(struct rte_eth_dev, link_intr_cbs));
> 

This is similar version of a previous patch:
https://patches.dpdk.org/patch/65795/

That one is also waiting because I remember it was not safe for hotplug.

Instead of a big memset, I am for selectively set pointers to NULL.


More information about the dev mailing list