DPDK patches and discussions
 help / color / mirror / Atom feed
From: Huisong Li <lihuisong@huawei.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC V2] ethdev: fix issue that dev close in PMD calls twice
Date: Fri, 13 Aug 2021 16:16:20 +0800
Message-ID: <eeb4a052-0320-7add-44bf-1dcdbb90fc04@huawei.com> (raw)
In-Reply-To: <21383486.34YfpWhNxb@thomas>


在 2021/8/13 14:12, Thomas Monjalon 写道:
> 13/08/2021 04:11, Huisong Li:
>> Hi, all
>>
>> This patch can enhance the security of device uninstallation to
>> eliminate dependency on user usage methods.
>>
>> Can you check this patch?
>>
>>
>> 在 2021/8/3 10:30, Huisong Li 写道:
>>> Ethernet devices in DPDK can be released by rte_eth_dev_close() and
>>> rte_dev_remove(). These APIs both call xxx_dev_close() in PMD layer
>>> to uninstall hardware. However, the two APIs do not have explicit
>>> invocation restrictions. In other words, at the ethdev layer, it is
>>> possible to call rte_eth_dev_close() before calling rte_dev_remove()
>>> or rte_eal_hotplug_remove(). In such a bad scenario,
> It is not a bad scenario.
> If there is no more port for the device after calling close,
> the device should be removed automatically.
> Keep in mind "close" is for one port, "remove" is for the entire device
> which can have more than one port.

I know.

dev_close() is for removing an eth device. And rte_dev_remove() can be used

for removing the rte device and all its eth devices belonging to the rte 
device.

In rte_dev_remove(), "remove" is executed in primary or one of secondary,

all eth devices having same pci address will be closed and removed.

>
>>> the primary
>>> process may be fine, but it may cause that xxx_dev_close() in the PMD
>>> layer will be called twice in the secondary process. So this patch
>>> fixes it.
> If a port is closed in primary, it should be the same in secondary.
>
>
>>> +	/*
>>> +	 * The eth_dev->data->name doesn't be cleared by the secondary process,
>>> +	 * so above "eth_dev" isn't NULL after rte_eth_dev_close() called.
> This assumption is not clear. All should be closed together.

However, dev_close() does not have the feature similar to rte_dev_remove().

Namely, it is not guaranteed that all eth devices are closed together in 
ethdev layer. It depends on app or user.

If the app does not close together, the operation of repeatedly 
uninstalling an eth device in the secondary process

will be triggered when dev_close() is first called by one secondary 
process, and then rte_dev_remove() is called.

So I think it should be avoided.

>
>>> +	 * Namely, whether "eth_dev" is NULL cannot be used to determine whether
>>> +	 * an ethdev port has been released.
>>> +	 * For both primary process and secondary process, eth_dev->state is
>>> +	 * 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;
>>> +	}
>
>
> .

  reply	other threads:[~2021-08-13  8:16 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02 12:46 [dpdk-dev] [RFC V1] ethdev: fix the issue that dev uninit may be called twice 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 [this message]
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
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=eeb4a052-0320-7add-44bf-1dcdbb90fc04@huawei.com \
    --to=lihuisong@huawei.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --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