From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id A31791B14F for ; Thu, 27 Sep 2018 23:31:54 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id E044A219FE; Thu, 27 Sep 2018 17:31:53 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 27 Sep 2018 17:31:53 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=e9xNR5bIWtPDPhtmxdjQtAIZEJ mx70c4myrX8fZEXwM=; b=C2ATHEGsOsylOah9Bo5xmCreLtCC8RakJkiUhkFmwU dtsOto4q2nH/XWJqfUOy1xBi2SrqyXOvsd9fFPjN6/jLkJWziNCZWcTR7x9t0Upg A01AoPsllV0i7j42BtEdSDd3MOfq+3W6frNuMGFd/P/cJGwv0MvJvWZmp/lw2rEz g= 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-sender:x-me-sender:x-sasl-enc; s=fm3; bh=e9xNR5 bIWtPDPhtmxdjQtAIZEJmx70c4myrX8fZEXwM=; b=fLu7jOM45UXL7yO5+YY/Ws TKvfmJpuPUcWFLI0PAIEob7tnooxOkodvl4PT14QVs2zXRQ0wbMmTVta9Sl+Mpnm tK4GPe46nCGLl7/uU/FVUA02cmY0vViTo3Z32Lj7LpUkoD1WWCpy67MRQ6ukYJ+7 6YgFG02Ofd8guHFG/85onAbmQnjW4x+5/XulQbvVJrtF1C2p3RsfrVkXNyMH/0OK 6WsDHAzAZu6VMZLOxTJnFjbIy3kfr7MccuxyDk0bCqttZnXomyw0SOpxabD0XRG+ z2Fyfv403jmAZjy9gCfR/pO3OQxrVlARLisFPAHANeCyAtAPP2thuvl31IsJpHFw == X-ME-Proxy: X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id C7B71E405E; Thu, 27 Sep 2018 17:31:51 -0400 (EDT) From: Thomas Monjalon To: Ophir Munk Cc: "dev@dpdk.org" , "gaetan.rivet@6wind.com" , "qi.z.zhang@intel.com" , "ferruh.yigit@intel.com" , "ktraynor@redhat.com" , Shahaf Shuler , Olga Shern Date: Thu, 27 Sep 2018 23:31:50 +0200 Message-ID: <4883182.eS8zcPg0Ii@xps> In-Reply-To: References: <20180907222727.20521-1-thomas@monjalon.net> <20180926214759.1856-3-thomas@monjalon.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2 2/4] devargs: simplify parameters of removal function 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, 27 Sep 2018 21:31:55 -0000 Hi, 27/09/2018 10:24, Ophir Munk: > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > The function rte_devargs_remove(), which is intended to be internal, can > > take a devargs structure as argument. > > The matching is still using string comparison of bus name and device name. > > It is simpler and may allow a different devargs matching in future. [...] > > --- a/lib/librte_eal/common/eal_common_dev.c > > +++ b/lib/librte_eal/common/eal_common_dev.c > > @@ -186,7 +186,7 @@ int __rte_experimental rte_eal_hotplug_add(const > > char *busname, const char *devn > > return 0; > > > > err_devarg: > > - if (rte_devargs_remove(busname, devname)) { > > + if (rte_devargs_remove(da)) { > > Can you please explain why calling with 'da' parameter > (which may have different internal fields of busname and devname > than the explicit busname and devname parameters) - is correct? Before devargs is parsed and inserted in the global list, devargs values are NULL, so nothing is found (as before this patch). rte_devargs_remove(da) will have no effect and will return 1, so the devargs will be freed by code in the if block. When devargs is inserted in the list, it will have the values of busname and devname, so it is the same as before this patch. However, there may be an issue dereferencing devargs->bus->name when devargs is not parsed. I must add this code in rte_devargs_remove: if (devargs->bus == NULL) return 1; [...] > > @@ -227,7 +227,7 @@ rte_eal_hotplug_remove(const char *busname, > > const char *devname) > > if (ret) > > RTE_LOG(ERR, EAL, "Driver cannot detach the device > > (%s)\n", > > dev->name); > > - rte_devargs_remove(busname, devname); > > + rte_devargs_remove(dev->devargs); > > Can you please explain why calling with 'dev->devargs' parameter > (which may have different internal fields of busname and devname > than the explicit busname and devname parameters) - is correct? The device is found by checking the busname and devname. I don't see how the devargs values may be different.