From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <arybchenko@solarflare.com>
Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com
[67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 57EE91B6E3
for <dev@dpdk.org>; Wed, 10 Oct 2018 17:02:27 +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
50505400074; Wed, 10 Oct 2018 15:02:24 +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 16:02:18 +0100
To: Thomas Monjalon <thomas@monjalon.net>
CC: <ferruh.yigit@intel.com>, <dev@dpdk.org>, <ophirmu@mellanox.com>
References: <20180907233929.21950-1-thomas@monjalon.net>
<7172106.4TAESsr7Yr@xps>
<5f013fa6-762c-0d7e-a057-a8225bed7634@solarflare.com>
<1961538.UcvSjTfz0W@xps>
From: Andrew Rybchenko <arybchenko@solarflare.com>
Message-ID: <f0c3b435-c458-f1c5-8b47-25da6036e49f@solarflare.com>
Date: Wed, 10 Oct 2018 18:01:37 +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: <1961538.UcvSjTfz0W@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-20.965000-8.000000-10
X-TMASE-MatchedRID: gzVbiXtWD9sOwH4pD14DsPHkpkyUphL9wuIWIvQEbW6vPoND+wakFlPK
Q4g0ENBTz0kHebusWqG/OcTZGXKzdinPugGsN3p5bc297PAGtWbhKQh1LCmGBhqB+wKK9uZeyU7
XgTs6W4LuzcUA0q2gBeKOmN63egZIOVzKEd+ERcomEURBmKrZlG803dnzHHML+Cckfm+bb6AomI
3JawtquaRXh45fPtHvOduZmbQPXWnVFtf7bVfns7Sw7varainhKx5ICGp/WtFtw+n+iKWyyDsFp
QORr8vEKx+bRc8nrpP0B9yN5j90xE0eOhDHYyZWRLhbOV0MntSbKpAlY2y6SRjQD3m2MCf7YZcj
YPntmcoyVRDcQBa7IFDQ43dkW3a5NuVzoZ6LieUiPTMUjkOgkl4KsHfYo5LQowtRP8whCK8K8Tr
y7xT2TxHh35S+GXSq/VWmtOMs6xHeWaJDH5WqDQ0QY5VnQyANLZWWNVgH0Y07LF3pX3rdVLjxa5
EVBV1q4vM1YF6AJbZFi+KwZZttL7ew1twePJJBOwBXM346/+zqlKfmetRka03N73j17SVB6J5UX
o16ZAL5GEFSyCjOMgqQhzexJ+dB
X-TM-AS-User-Approved-Sender: Yes
X-TM-AS-User-Blocked-Sender: No
X-TMASE-Result: 10--20.965000-8.000000
X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24146.003
X-MDID: 1539183745-aXnf7bb_AhCT
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 <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>
X-List-Received-Date: Wed, 10 Oct 2018 15:02:27 -0000
On 10/10/18 11:39 AM, Thomas Monjalon wrote:
> 10/10/2018 09:50, Andrew Rybchenko:
>> 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 <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.
>> 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)
> So far, everything is allowed:
> - The application can close a port and remove the rte_device later.
If the patch is applied, close frees dev_private which is used by uninit.
So, uninit must be done first. Who does it?
(it looks like I'm missing something obvious, but still can't find it)
> - The application can remove the rte_device and expect the PMD is closing
> associated ports.
>
> In other words, when rte_device is removed, the ports should be closed
> by the PMD, except if the application has already closed the ports.
> It means ethdev port close is optional, but EAL removal is always required.
> The behaviour is not changed.
>
> If we want to go further, we could change the behaviour of the close op,
> by asking the PMD to remove the rte_device automatically if all associated
> ports are closed. It would allow the application to manage only ports
> at ethdev layer without bothering with low-level EAL management.
> We can think about it as a planned change for next releases.
>
>