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 CAB5AA04C8; Fri, 18 Sep 2020 15:32:42 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AE9341DA71; Fri, 18 Sep 2020 15:32:42 +0200 (CEST) Received: from stargate.chelsio.com (stargate.chelsio.com [12.32.117.8]) by dpdk.org (Postfix) with ESMTP id 729F41DA70 for ; Fri, 18 Sep 2020 15:32:41 +0200 (CEST) Received: from localhost (scalar.blr.asicdesigners.com [10.193.185.94]) by stargate.chelsio.com (8.13.8/8.13.8) with ESMTP id 08IDWbao012755; Fri, 18 Sep 2020 06:32:38 -0700 Date: Fri, 18 Sep 2020 18:48:19 +0530 From: Rahul Lakkireddy To: Ferruh Yigit Cc: dev@dpdk.org, kaara.satwik@chelsio.com Message-ID: <20200918131818.GA18801@chelsio.com> References: <8fb4bbbf-9bbd-b3e0-b96b-aae6647fe211@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8fb4bbbf-9bbd-b3e0-b96b-aae6647fe211@intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [dpdk-dev] [PATCH 2/2] net/cxgbe: release port resources during port 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 Thursday, September 09/17/20, 2020 at 13:46:24 +0100, Ferruh Yigit wrote: > On 9/1/2020 6:16 PM, Rahul Lakkireddy wrote: > >Enable RTE_ETH_DEV_CLOSE_REMOVE during PCI probe for all ports > >enumerated under the PF. Free up the underlying port Virtual > >Identifier (VI) and associated resources during port close. > >Once all the ports under the PF are closed, free up the PF-wide > >shared resources. Invoke port close function of all ports under > >the PF, in PCI remove too. > > > >Signed-off-by: Rahul Lakkireddy > > <...> > > >@@ -1204,11 +1222,13 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev) > > static int eth_cxgbe_dev_uninit(struct rte_eth_dev *eth_dev) > > { > >- struct port_info *pi = eth_dev->data->dev_private; > >- struct adapter *adap = pi->adapter; > >+ struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev); > >+ uint16_t port_id; > > /* Free up other ports and all resources */ > >- cxgbe_close(adap); > >+ RTE_ETH_FOREACH_DEV_OF(port_id, &pci_dev->device) > >+ rte_eth_dev_close(port_id); > > Is there a reason to call the ethdev wrapper API here? Why not calling > 'cxgbe_dev_close()' directly? Chelsio NICs enumerate all physical ports under a single Physical Function 4 (PF4). When PCI remove is invoked without rte_eth_dev_close() first, the rte_eth_dev_release_port() is only invoked for the first port instantiated on PF4. So, to ensure rte_eth_dev_release_port() is invoked for the remaining ports, we're explicitly calling rte_eth_dev_close() for all ports here. Here's a testpmd example that leads to the problem. testpmd> port stop all Stopping ports... Checking link statuses... Done testpmd> device detach 0000:03:00.4 Removing a device... Port 0 is now closed Port 1 is now closed Device 0000:03:00.4 is detached Now total ports is 1 Done testpmd> The Chelsio NIC has 2 ports enumerated under 0000:03:00.4. When PCI remove is invoked, rte_eth_dev_release_port() is only invoked once for the first port. You can see that the other port is still active, as printed by "Now total ports is 1". The sequence of calls is as follows from driver's PCI remove: eth_cxgbe_pci_remove() rte_eth_dev_pci_generic_remove() rte_eth_dev_pci_release() rte_eth_dev_release_port() <--- Only invoked once If I explicitly invoke rte_eth_dev_close() in PCI remove for all ports, as done in this patch, rte_eth_dev_release_port() is invoked for both the ports. testpmd> port stop all Stopping ports... Checking link statuses... Done testpmd> device detach 0000:03:00.4 Removing a device... Port 0 is now closed Port 1 is now closed Device 0000:03:00.4 is detached Now total ports is 0 Done Now, both ports are released properly as printed by "Now total ports is 0". Thanks, Rahul