From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v2] ethdev: complete closing of port
Date: Wed, 10 Oct 2018 09:44:46 +0200
Message-ID: <7172106.4TAESsr7Yr@xps>
References: <20180907233929.21950-1-thomas@monjalon.net>
 <20181009221732.17377-1-thomas@monjalon.net>
 <c85fe1fb-2135-b5c1-ffdc-ce19d1f4304c@solarflare.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: ferruh.yigit@intel.com, dev@dpdk.org, ophirmu@mellanox.com
To: Andrew Rybchenko <arybchenko@solarflare.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com
 [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 6BDAF1B202
 for <dev@dpdk.org>; Wed, 10 Oct 2018 09:44:49 +0200 (CEST)
In-Reply-To: <c85fe1fb-2135-b5c1-ffdc-ce19d1f4304c@solarflare.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

10/10/2018 08:15, Andrew Rybchenko:
> On 10/10/18 1:17 AM, Thomas Monjalon wrote:
> > After closing a port, it cannot be restarted.
> > So there is no reason to not free all associated resources.
> >
> > The last step was done with rte_eth_dev_detach() which is deprecated.
> > Instead of blindly removing the associated rte_device, the driver should
> > check if no more port (ethdev, cryptodev, etc) is open for the device.
> >
> > The last ethdev freeing (dev_private and final release), which were done
> > by rte_eth_dev_detach(), are now done at the end of rte_eth_dev_close().
> >
> > If the driver is trying to free the port again, the function
> > rte_eth_dev_release_port() will abort with -ENODEV error.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >   lib/librte_ethdev/rte_ethdev.c | 6 ++++++
> >   lib/librte_ethdev/rte_ethdev.h | 3 +--
> >   2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> > index ed83e5954..3062dc711 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -506,6 +506,8 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
> >   {
> >   	if (eth_dev == NULL)
> >   		return -EINVAL;
> > +	if (eth_dev->state == RTE_ETH_DEV_UNUSED)
> > +		return -ENODEV;
> >   
> >   	rte_eth_dev_shared_data_prepare();
> >   
> > @@ -1441,6 +1443,10 @@ rte_eth_dev_close(uint16_t port_id)
> >   	dev->data->nb_tx_queues = 0;
> >   	rte_free(dev->data->tx_queues);
> >   	dev->data->tx_queues = NULL;
> > +
> > +	rte_free(dev->data->dev_private);
> 
> It is used by, for example, PCI device uninit functions.
> What does guarantee that uninit is done and we can free the private data.

The state of the port is set to UNUSED and the name is NULL.
So nobody should try to use it anymore.
There are already some checks before calling uninit functions.
For instance, in rte_eth_dev_pci_generic_remove(),
rte_eth_dev_allocated() will return NULL and won't call uninit function.