DPDK patches and discussions
 help / color / mirror / Atom feed
From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: dev@dpdk.org, kaara.satwik@chelsio.com
Subject: Re: [dpdk-dev] [PATCH 2/2] net/cxgbe: release port resources during port close
Date: Fri, 18 Sep 2020 18:48:19 +0530
Message-ID: <20200918131818.GA18801@chelsio.com> (raw)
In-Reply-To: <8fb4bbbf-9bbd-b3e0-b96b-aae6647fe211@intel.com>

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 <rahul.lakkireddy@chelsio.com>
> 
> <...>
> 
> >@@ -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

  reply	other threads:[~2020-09-18 13:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 17:16 [dpdk-dev] [PATCH 0/2] " Rahul Lakkireddy
2020-09-01 17:16 ` [dpdk-dev] [PATCH 1/2] net/cxgbe: fix queue DMA ring leaks " Rahul Lakkireddy
2020-09-01 17:16 ` [dpdk-dev] [PATCH 2/2] net/cxgbe: release port resources " Rahul Lakkireddy
2020-09-17 12:46   ` Ferruh Yigit
2020-09-18 13:18     ` Rahul Lakkireddy [this message]
2020-09-18 14:38       ` Ferruh Yigit
2020-09-18 14:57 ` [dpdk-dev] [PATCH 0/2] " Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200918131818.GA18801@chelsio.com \
    --to=rahul.lakkireddy@chelsio.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=kaara.satwik@chelsio.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git