From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id D8A0E1B447 for ; Wed, 10 Oct 2018 09:51:32 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 22685A80061; Wed, 10 Oct 2018 07:51:31 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 10 Oct 2018 08:51:23 +0100 To: Thomas Monjalon CC: , , References: <20180907233929.21950-1-thomas@monjalon.net> <20181009221732.17377-1-thomas@monjalon.net> <7172106.4TAESsr7Yr@xps> From: Andrew Rybchenko Message-ID: <5f013fa6-762c-0d7e-a057-a8225bed7634@solarflare.com> Date: Wed, 10 Oct 2018 10:50:42 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <7172106.4TAESsr7Yr@xps> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24146.003 X-TM-AS-Result: No-17.880300-8.000000-10 X-TMASE-MatchedRID: vEvJ7Rh1lGgOwH4pD14DsPHkpkyUphL9wuIWIvQEbW6vPoND+wakFlPK Q4g0ENBTz0kHebusWqG/OcTZGXKzdinPugGsN3p5bc297PAGtWbhKQh1LCmGBhqB+wKK9uZeyU7 XgTs6W4LuzcUA0q2gBeKOmN63egZIOVzKEd+ERcomEURBmKrZlG803dnzHHML+Cckfm+bb6AomI 3JawtquaRXh45fPtHvOduZmbQPXWnVFtf7bVfns7Sw7varainhKx5ICGp/WtFkBeQk1bVWX/NO7 flRFqXmIGSHKu90xKWSU848M/hs6Me4Woyb+kVFyDp+jSvEtWsl3afZehJEWedlU2K5Jm9bLMlY avN/+VJdQJ0TvOSfrgUIzG/5oa1p31upjFk7WXTkGAR1SqoA1H0tCKdnhB58vqq8s2MNhPCy5/t FZu9S3GNgbF0oRDZ0MrJHH5VFFAwxjujMh/WMy4gt4DLVP3itXoIfgoo1xoWInmLynhFpWkT/8P RYMWAmy9BQq8s/JL7Uq+9w/evblB1mq8u4nNjiq4X/HlZlHuc= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--17.880300-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24146.003 X-MDID: 1539157892-yM2RViPduEd4 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2] ethdev: complete closing of port 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: Wed, 10 Oct 2018 07:51:33 -0000 On 10/10/18 10:44 AM, Thomas Monjalon wrote: > 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 >>> --- >>> 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. The questions are: Is application allowed to call the function? When? Who calls uninit in this case? (What does guarantee that uninit is done before close)