[PATCH] ethdev: fix dev close in secondary process

[PATCH] ethdev: fix dev close in secondary process

[Min Hu (Connor)]

From: Min Hu <humin29@huawei.com>

Shared memory like port private resources should only be reserved
by primary process. Secondary process should not start dev, and
the state of 'dev_started' is only meaningful to primary process.
While secondary process need to close dev to release process private
resources.

This patch limited the scope of 'dev_started'.

Fixes: febc855b358e ("ethdev: forbid closing started device")
Cc: stable@dpdk.org

Signed-off-by: Min Hu <humin29@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 09abee6345..f34c6580a4 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1574,7 +1574,8 @@ rte_eth_dev_close(uint16_t port_id)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
-	if (dev->data->dev_started) {
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
+			dev->data->dev_started) {
 		RTE_ETHDEV_LOG(ERR, "Cannot close started device (port %u)\n",
 			       port_id);
 		return -EINVAL;
-- 
2.33.0

Re: [PATCH] ethdev: fix dev close in secondary process

[Andrew Rybchenko]

On 5/27/22 05:35, Min Hu (Connor) wrote:
> From: Min Hu <humin29@huawei.com>
> 
> Shared memory like port private resources should only be reserved
> by primary process. Secondary process should not start dev, and
> the state of 'dev_started' is only meaningful to primary process.
> While secondary process need to close dev to release process private
> resources.
> 
> This patch limited the scope of 'dev_started'.

I agree with the patch since secondary process should not be
obliged to wait for device stop before closing ethdev. In any
case closing ethdev in secondary process should do nothing
harmful to the primary process.

However, the patch description pretends to limit dev_started
scope for secondary processes in general. It is wrong since
secondary processes need the information and that's why it is
stored in shared memory.

> 
> Fixes: febc855b358e ("ethdev: forbid closing started device")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu <humin29@huawei.com>
> ---
>   lib/ethdev/rte_ethdev.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 09abee6345..f34c6580a4 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -1574,7 +1574,8 @@ rte_eth_dev_close(uint16_t port_id)
>   	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>   	dev = &rte_eth_devices[port_id];
>   
> -	if (dev->data->dev_started) {
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
> +			dev->data->dev_started) {
>   		RTE_ETHDEV_LOG(ERR, "Cannot close started device (port %u)\n",
>   			       port_id);
>   		return -EINVAL;

Re: [PATCH] ethdev: fix dev close in secondary process

[Stephen Hemminger]

On Tue, 31 May 2022 20:08:55 +0300
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote:

> On 5/27/22 05:35, Min Hu (Connor) wrote:
> > From: Min Hu <humin29@huawei.com>
> > 
> > Shared memory like port private resources should only be reserved
> > by primary process. Secondary process should not start dev, and
> > the state of 'dev_started' is only meaningful to primary process.
> > While secondary process need to close dev to release process private
> > resources.
> > 
> > This patch limited the scope of 'dev_started'.  
> 
> I agree with the patch since secondary process should not be
> obliged to wait for device stop before closing ethdev. In any
> case closing ethdev in secondary process should do nothing
> harmful to the primary process.
> 
> However, the patch description pretends to limit dev_started
> scope for secondary processes in general. It is wrong since
> secondary processes need the information and that's why it is
> stored in shared memory.
> 
> > 
> > Fixes: febc855b358e ("ethdev: forbid closing started device")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Min Hu <humin29@huawei.com>
> > ---

Also secondary processes are used differently by different application models.

Some applications only use secondary process for information.
But some have a primary process that only inits DPDK and do everything
in a secondary process.

Re: [PATCH] ethdev: fix dev close in secondary process

[Min Hu (Connor)]

Hi,Andrew, Stephen ,
	I fixed the comment, thanks for your comment.

在 2022/6/1 1:40, Stephen Hemminger 写道:
> On Tue, 31 May 2022 20:08:55 +0300
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote:
> 
>> On 5/27/22 05:35, Min Hu (Connor) wrote:
>>> From: Min Hu <humin29@huawei.com>
>>>
>>> Shared memory like port private resources should only be reserved
>>> by primary process. Secondary process should not start dev, and
>>> the state of 'dev_started' is only meaningful to primary process.
>>> While secondary process need to close dev to release process private
>>> resources.
>>>
>>> This patch limited the scope of 'dev_started'.
>>
>> I agree with the patch since secondary process should not be
>> obliged to wait for device stop before closing ethdev. In any
>> case closing ethdev in secondary process should do nothing
>> harmful to the primary process.
>>
>> However, the patch description pretends to limit dev_started
>> scope for secondary processes in general. It is wrong since
>> secondary processes need the information and that's why it is
>> stored in shared memory.
>>
>>>
>>> Fixes: febc855b358e ("ethdev: forbid closing started device")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Min Hu <humin29@huawei.com>
>>> ---
> 
> Also secondary processes are used differently by different application models.
> 
> Some applications only use secondary process for information.
> But some have a primary process that only inits DPDK and do everything
> in a secondary process.
> .
>