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 71A6EA04B1; Thu, 24 Sep 2020 13:56:39 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 50E0B1D9EE; Thu, 24 Sep 2020 13:56:39 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id A8E0F1D671 for ; Thu, 24 Sep 2020 13:56:37 +0200 (CEST) IronPort-SDR: bbf2MNQvMfT6TIUerx4gIAblwwU7XEtlvtScXO4d/U7r+tSLpKZ7zYcx9lCaQnhtOxhOD4FnRM dxjRqR1kTsjA== X-IronPort-AV: E=McAfee;i="6000,8403,9753"; a="222775677" X-IronPort-AV: E=Sophos;i="5.77,297,1596524400"; d="scan'208";a="222775677" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Sep 2020 04:56:36 -0700 IronPort-SDR: ulSITzGCDFYSLxmdC//527zdZMdQxqQymqRIAHbIlBnqWC/0NUpHzDvGAgVuCb11yTgZEjjCW7 U/ciUYYI4I/A== X-IronPort-AV: E=Sophos;i="5.77,297,1596524400"; d="scan'208";a="512130170" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.251.85.112]) ([10.251.85.112]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Sep 2020 04:56:35 -0700 To: Thomas Monjalon Cc: dev@dpdk.org, arybchenko@solarflare.com, Anatoly Burakov References: <20200913220711.3768597-1-thomas@monjalon.net> <20200913220711.3768597-13-thomas@monjalon.net> <721acbe3-9f67-c98c-e964-6be2e1ec3f2c@intel.com> <6917397.Q1i2GfNnKL@thomas> From: Ferruh Yigit Message-ID: <54abc523-47cc-6283-51a5-e0b319e5ba21@intel.com> Date: Thu, 24 Sep 2020 12:56:31 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.2.2 MIME-Version: 1.0 In-Reply-To: <6917397.Q1i2GfNnKL@thomas> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 12/20] net/pcap: release port upon close 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" On 9/23/2020 9:44 PM, Thomas Monjalon wrote: > 23/09/2020 18:44, Ferruh Yigit: >> On 9/13/2020 11:07 PM, Thomas Monjalon wrote: >>> The flag RTE_ETH_DEV_CLOSE_REMOVE is set so all port resources >>> can be freed by rte_eth_dev_close(). >>> >>> Freeing of private port resources is moved >>> from the ".remove(device)" to the ".dev_close(port)" operation. >>> >>> Signed-off-by: Thomas Monjalon >>> --- >>> drivers/net/pcap/rte_eth_pcap.c | 29 ++++++++++++++--------------- >>> 1 file changed, 14 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c >>> index 76e704a65a..a946fa9a1a 100644 >>> --- a/drivers/net/pcap/rte_eth_pcap.c >>> +++ b/drivers/net/pcap/rte_eth_pcap.c >>> @@ -734,6 +734,12 @@ eth_dev_close(struct rte_eth_dev *dev) >>> unsigned int i; >>> struct pmd_internals *internals = dev->data->dev_private; >>> >>> + if (internals == NULL) >>> + return 0; >> >> Not sure if this check needed, can 'internals' be null at this stage? > > I think yes we need to protect against a double close. > 'eth_dev_close()' can be called by 'pmd_pcap_remove()' or 'rte_eth_dev_close()' both should be blocked to call 'eth_dev_close()' after first close. And same thing for all PMDs, if they don't need, this one also shouldn't need. > >> But perhaps need to add 'RTE_PROC_PRIMARY' check. > > Yes but that's not the goal of this patch. > Yes, the check should be there already, but now more stuff added to close dev_ops, and calling it from secondary will cause problem. Since you are already here, I think would be nice to add it. >>> + rte_free(dev->process_private); >> >> Can we move freeing 'process_private' to 'rte_eth_dev_release_port()' > > Yes we could. > >