DPDK patches and discussions
 help / color / mirror / Atom feed
From: Huisong Li <lihuisong@huawei.com>
To: "Singh, Aman Deep" <aman.deep.singh@intel.com>, <dev@dpdk.org>
Cc: <thomas@monjalon.net>, <ferruh.yigit@intel.com>
Subject: Re: [dpdk-dev] [RFC V1] ethdev: fix the issue that dev uninit may be called twice
Date: Tue, 24 Aug 2021 10:10:40 +0800
Message-ID: <91cf7c95-038f-bba2-4e05-b921c3e0427e@huawei.com> (raw)
In-Reply-To: <44a1b455-31e8-7909-3011-1643293f3210@intel.com>

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)

  reply	other threads:[~2021-08-24  2:10 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02 12:46 Huisong Li
2021-08-03  2:30 ` [dpdk-dev] [RFC V2] ethdev: fix issue that dev close in PMD calls twice Huisong Li
2021-08-13  2:11   ` Huisong Li
2021-08-13  6:12     ` Thomas Monjalon
2021-08-13  8:16       ` Huisong Li
2021-08-18 11:24         ` Ferruh Yigit
2021-08-19  3:45           ` Huisong Li
2021-08-24 14:42             ` Ferruh Yigit
2021-08-25  9:53               ` Huisong Li
2021-09-04  1:23                 ` Huisong Li
2021-09-18  3:31                 ` Huisong Li
2021-09-20 14:07                 ` Ferruh Yigit
2021-09-22  3:31                   ` Huisong Li
2021-09-28  7:19                     ` Singh, Aman Deep
2021-09-30 10:54                       ` Huisong Li
2021-09-30 11:01                         ` Ferruh Yigit
2021-10-08  6:13                           ` lihuisong (C)
2021-08-18  9:47 ` [dpdk-dev] [RFC V1] ethdev: fix the issue that dev uninit may be called twice Singh, Aman Deep
2021-08-24  2:10   ` Huisong Li [this message]
2021-10-08  8:21 ` [dpdk-dev] [PATCH] ethdev: fix eth device released repeatedly Huisong Li
2021-10-08 10:23   ` Thomas Monjalon
2021-10-09  1:29     ` lihuisong (C)
2021-10-12 11:39 ` [dpdk-dev] [PATCH V2] " Huisong Li
2021-10-12 15:33   ` Thomas Monjalon
2021-10-14  3:50     ` lihuisong (C)
2021-10-14 12:32     ` lihuisong (C)
2021-10-14 12:50       ` Thomas Monjalon
2021-10-15  3:03         ` lihuisong (C)
2021-10-15  3:44 ` [dpdk-dev] [PATCH V3] " Huisong Li
2021-10-19 13:09   ` Ferruh Yigit
2021-10-21  2:31     ` lihuisong (C)
2021-10-21  2:24 ` [dpdk-dev] [PATCH V4] " Huisong Li
2021-10-21 21:19   ` 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=91cf7c95-038f-bba2-4e05-b921c3e0427e@huawei.com \
    --to=lihuisong@huawei.com \
    --cc=aman.deep.singh@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=thomas@monjalon.net \
    /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 http://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/ http://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