From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 99BC0A04BC; Tue, 29 Sep 2020 18:02:51 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 71AC01D9B2; Tue, 29 Sep 2020 18:02:50 +0200 (CEST) Received: from new1-smtp.messagingengine.com (new1-smtp.messagingengine.com [66.111.4.221]) by dpdk.org (Postfix) with ESMTP id 705C71D9A2 for ; Tue, 29 Sep 2020 18:02:48 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailnew.nyi.internal (Postfix) with ESMTP id A4E75580143; Tue, 29 Sep 2020 12:02:46 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Tue, 29 Sep 2020 12:02:46 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm2; bh= GGq+2uNocWbaFFWzrWUm/Jee0lnBJbfGiJkRxCcwSHk=; b=t/v+QVpVBkV95rzV U5DVEe+oKZVrb7rTD2zYiPO06JuO2Xwtruf75mi3KbqOPmvuAW2aTHCdaMwo1xYK B44PUiKj2HCldmkjUXDAQDViwDrB53B02swYT4MAqMnGOKz8horeyvn8R3U3CMFG 1zieXk40f/kMQJfYb2qhhtUUnJHf9+LNIZ47sak84dyIzLGx6h8o+dYvKzYqOrrI RH/DX9nfRfEWZPOxYOSo+biq/L+cuoel7AdDngk+ht37/VAcWT1m6CNT52SrGhVH 0uJuoaXkqX4c0KJpQWw4tKZ5xVzCRjlVPRODyG6+stO5f6O69lkRlCOOMkwS0uwC jFHoMw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=GGq+2uNocWbaFFWzrWUm/Jee0lnBJbfGiJkRxCcwS Hk=; b=EI3uMvyGCt6SIwu7SaqIKBmQmib/NRUP2z+WVQJzZMTO/JeMDK1FH38H+ tC1KHuTerc2QTrsV+5deBqJwZkxD/F9NoV/dPe244jBfRPkSiLfuPJhTrPPXOOlE MfGk1tYydjs9VU7HbMyGCruLH6ZEPVpm0CVzemQiimusmLinre57PhCtSs9eLQXO alhSOOIl+xYpMJXECKJX8EyoE6ZziN2/dvAXyp/Y+5kJrkSySJoJ6h1APcYXZuXo NvL20akSmhyKWJC9Tmo8T85TASeAriwhzqxPJEGnU/VKCCcONSHL38XMhG9weBgk 1+RVzGRXQer7v6D1Qp0j/mFCNeWWg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrvdekgdeliecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpeffvdffjeeuteelfeeileduudeugfetjeelveefkeejfeeigeehteff vdekfeegudenucffohhmrghinhepughpughkrdhorhhgnecukfhppeejjedrudefgedrvd dtfedrudekgeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhr ohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 84A893280065; Tue, 29 Sep 2020 12:02:38 -0400 (EDT) From: Thomas Monjalon To: "Wang, Haiyue" , Maxime Coquelin , Ferruh Yigit Cc: "dev@dpdk.org" , "arybchenko@solarflare.com" , Shepard Siegel , Ed Czeck , John Miller , Igor Russkikh , Pavel Belous , Somalapuram Amaranath , Ajit Khaparde , Somnath Kotur , Chas Williams , "Wei Hu (Xavier)" , Hemant Agrawal , Sachin Saxena , "Guo, Jia" , Marcin Wojtas , Michal Krawczyk , Guy Tzalik , Evgeny Schemeilin , Igor Chauskin , "Zhang, Qi Z" , "Wang, Xiao W" , Ziyang Xuan , Xiaoyun Wang , Guoyang Zhou , "Min Hu (Connor)" , Yisen Zhuang , "Xing, Beilei" , "Wu, Jingjing" , "Yang, Qiming" , Alfredo Cardigliano , Shijith Thotton , Srisivasubramanian Srinivasan , Stephen Hemminger , "K. Y. Srinivasan" , Haiyang Zhang , Long Li , Harman Kalra , Rasesh Mody , Shahed Shaikh , "Wiles, Keith" , "Xia, Chenbo" , "Wang, Zhihong" , Yong Wang , "matan@nvidia.com" Date: Tue, 29 Sep 2020 18:02:36 +0200 Message-ID: <26589070.GSGI34Fg33@thomas> In-Reply-To: <4c4ae719-f870-c219-babc-c34ff67b25c7@intel.com> References: <20200913220711.3768597-1-thomas@monjalon.net> <4c4ae719-f870-c219-babc-c34ff67b25c7@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v3 28/29] ethdev: reset all when releasing a port 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 29/09/2020 17:50, Ferruh Yigit: > On 9/29/2020 12:58 PM, Wang, Haiyue wrote: > > From: Thomas Monjalon > >> 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 > >>> , > >>> 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/ I forgot this patch :) > That one is also waiting because I remember it was not safe for hotplug. I don't think there is a safety issue. It makes harder to play with dangling pointers, which is good. Note: the .device pointer was already resetted by rte_eth_dev_pci_generic_remove(), and generalized in patch 1. > Instead of a big memset, I am for selectively set pointers to NULL. I will prepare the patch with selective resets.