From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 29684A0C46;
Tue, 24 Aug 2021 04:10:45 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id B5EC8406A3;
Tue, 24 Aug 2021 04:10:44 +0200 (CEST)
Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187])
by mails.dpdk.org (Postfix) with ESMTP id 8B77F4014D
for <dev@dpdk.org>; Tue, 24 Aug 2021 04:10:43 +0200 (CEST)
Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.56])
by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Gtsy90xdjzYt01;
Tue, 24 Aug 2021 10:10:09 +0800 (CST)
Received: from dggema767-chm.china.huawei.com (10.1.198.209) by
dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server
(version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id
15.1.2176.2; Tue, 24 Aug 2021 10:10:40 +0800
Received: from [10.66.74.184] (10.66.74.184) by dggema767-chm.china.huawei.com
(10.1.198.209) with Microsoft SMTP Server (version=TLS1_2,
cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Tue, 24
Aug 2021 10:10:40 +0800
To: "Singh, Aman Deep" <aman.deep.singh@intel.com>, <dev@dpdk.org>
CC: <thomas@monjalon.net>, <ferruh.yigit@intel.com>
References: <1627908397-51565-1-git-send-email-lihuisong@huawei.com>
<44a1b455-31e8-7909-3011-1643293f3210@intel.com>
From: Huisong Li <lihuisong@huawei.com>
Message-ID: <91cf7c95-038f-bba2-4e05-b921c3e0427e@huawei.com>
Date: Tue, 24 Aug 2021 10:10:40 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
Thunderbird/78.8.0
MIME-Version: 1.0
In-Reply-To: <44a1b455-31e8-7909-3011-1643293f3210@intel.com>
X-Originating-IP: [10.66.74.184]
X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To
dggema767-chm.china.huawei.com (10.1.198.209)
X-CFilter-Loop: Reflected
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 8bit
X-Content-Filtered-By: Mailman/MimeDel 2.1.29
Subject: Re: [dpdk-dev] [RFC V1] ethdev: fix the issue that dev uninit may
be called twice
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Hi, Singh, Aman Deep
Sorry, I missed your review. Thank you for your review.😁
在 2021/8/18 17:47, Singh, Aman Deep 写道:
>
> Hi Huison,
>
> On 8/2/2021 6:16 PM, Huisong Li wrote:
>> Ethernet devices in DPDK can be released by rte_eth_dev_close() and
>> rte_dev_remove(). However, these two APIs do not have explicit invocation
>> restrictions. In other words, at the ethdev layer, calling
>> rte_eth_dev_close() and then rte_dev_remove() or rte_eal_hotplug_remove()
>> is allowed. In such a bad scenario, the primary process may be fine, but it
>> may cause that dev_unint() in the secondary process will be called twice,
> Shouldn't dev_unint() for Secondary process, simply return with no-action.
The prerequisite is that the secondary process does not have any
resources that need to be released.
However, some resources from secondary process may also need to be
released. For example, mp action
registered by rte_mp_action_register() is used to for multi-process
communication. It should be unloaded
when all eth devices driven by one PMD in a process are removed. In
order to achieve the above purpose,
secondary process may have data recording the number of device to decide
when to deregister the action.
Of course, this is just the case. In short, secondary process may have
its own private data or resources to
be released.
It is mentioned in RFC v2. Please go to discussion line of RFC v2.
>> and even other serious problems. So this patch fixes it.
>>
>> Fixes: 99a2dd955fba ("lib: remove librte_ prefix from directory names")
>>
>> Signed-off-by: Huisong Li<lihuisong@huawei.com>
>> ---
>> lib/ethdev/ethdev_pci.h | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
>> index 8edca82..14a0e01 100644
>> --- a/lib/ethdev/ethdev_pci.h
>> +++ b/lib/ethdev/ethdev_pci.h
>> @@ -151,6 +151,19 @@ rte_eth_dev_pci_generic_remove(struct rte_pci_device *pci_dev,
>> if (!eth_dev)
>> return 0;
>>
>> + /*
>> + * The eth_dev->data->name doesn't be cleared by the secondary precess,
> Can we reprase above sentence "doesn't be cleared "
ok
>> + * so above "eth_dev" isn't NULL after rte_eth_dev_close() called.
>> + * Namely, whether "eth_dev" is NULL cannot be used to determine whether
>> + * an ethdev port has been released.
>> + * For both primary precess and secondary precess, eth_dev->state is
> s/ precess / process
Thanks. RFC v2 has corrected it.
>> + * RTE_ETH_DEV_UNUSED, which means the ethdev port has been released.
>> + */
>> + if (eth_dev->state == RTE_ETH_DEV_UNUSED) {
>> + RTE_ETHDEV_LOG(INFO, "The ethdev port has been released.");
>> + return 0;
>> + }
>> +
>> if (dev_uninit) {
>> ret = dev_uninit(eth_dev);
>> if (ret)