diff mbox series

[V3] ethdev: fix eth device released repeatedly

Message ID 20211015034447.26368-1-lihuisong@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [V3] ethdev: fix eth device released repeatedly |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/Intel-compilation fail Compilation issues
ci/intel-Testing success Testing PASS

Commit Message

lihuisong (C) Oct. 15, 2021, 3:44 a.m. UTC 
  In secondary process, rte_eth_dev_close() doesn't clear eth_dev->data.
If calling rte_dev_remove() after rte_eth_dev_close(),
in rte_eth_dev_pci_generic_remove() function, the released eth device still
can be found by its name in shared memory. As a result, the eth device will
be released repeatedly. The state of the eth device is modified to
RTE_ETH_DEV_UNUSED after rte_eth_dev_close(). So this state can be used to
avoid this problem.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
v2 -> v3:
 * fix the commit log description and the comment inside the code.
v1 -> v2:                                                                  
  * fix the commit log description.   
RFC -> v1:                                                                 
  * fix commit log and add a judgment for secondary process.
---
 lib/ethdev/ethdev_pci.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Ferruh Yigit Oct. 19, 2021, 1:09 p.m. UTC | #1 
On 10/15/2021 4:44 AM, Huisong Li wrote:
> In secondary process, rte_eth_dev_close() doesn't clear eth_dev->data.
> If calling rte_dev_remove() after rte_eth_dev_close(),
> in rte_eth_dev_pci_generic_remove() function, the released eth device still
> can be found by its name in shared memory. As a result, the eth device will
> be released repeatedly. The state of the eth device is modified to
> RTE_ETH_DEV_UNUSED after rte_eth_dev_close(). So this state can be used to
> avoid this problem.
> 

Hi Huisong,

Can you please add Fixes line, and stable tag if the change is requested
for backport?


> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
> v2 -> v3:
>   * fix the commit log description and the comment inside the code.
> v1 -> v2:
>    * fix the commit log description.
> RFC -> v1:
>    * fix commit log and add a judgment for secondary process.
> ---
>   lib/ethdev/ethdev_pci.h | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
> index 8edca82ce8..fcabae02fa 100644
> --- a/lib/ethdev/ethdev_pci.h
> +++ b/lib/ethdev/ethdev_pci.h
> @@ -151,6 +151,16 @@ rte_eth_dev_pci_generic_remove(struct rte_pci_device *pci_dev,
>   	if (!eth_dev)
>   		return 0;
>   
> +	/*
> +	 * In secondary process, a released eth device can be found by its name
> +	 * in shared memory.
> +	 * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
> +	 * eth device has been released.
> +	 */
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
> +	    eth_dev->state == RTE_ETH_DEV_UNUSED)
> +		return 0;
> +
>   	if (dev_uninit) {
>   		ret = dev_uninit(eth_dev);
>   		if (ret)
>
lihuisong (C) Oct. 21, 2021, 2:31 a.m. UTC | #2 
在 2021/10/19 21:09, Ferruh Yigit 写道:
> On 10/15/2021 4:44 AM, Huisong Li wrote:
>> In secondary process, rte_eth_dev_close() doesn't clear eth_dev->data.
>> If calling rte_dev_remove() after rte_eth_dev_close(),
>> in rte_eth_dev_pci_generic_remove() function, the released eth device 
>> still
>> can be found by its name in shared memory. As a result, the eth 
>> device will
>> be released repeatedly. The state of the eth device is modified to
>> RTE_ETH_DEV_UNUSED after rte_eth_dev_close(). So this state can be 
>> used to
>> avoid this problem.
>>
>
> Hi Huisong,
>
> Can you please add Fixes line, and stable tag if the change is requested
> for backport?
ok. It has been added. Please review v4.
>
>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>> v2 -> v3:
>>   * fix the commit log description and the comment inside the code.
>> v1 -> v2:
>>    * fix the commit log description.
>> RFC -> v1:
>>    * fix commit log and add a judgment for secondary process.
>> ---
>>   lib/ethdev/ethdev_pci.h | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
>> index 8edca82ce8..fcabae02fa 100644
>> --- a/lib/ethdev/ethdev_pci.h
>> +++ b/lib/ethdev/ethdev_pci.h
>> @@ -151,6 +151,16 @@ rte_eth_dev_pci_generic_remove(struct 
>> rte_pci_device *pci_dev,
>>       if (!eth_dev)
>>           return 0;
>>   +    /*
>> +     * In secondary process, a released eth device can be found by 
>> its name
>> +     * in shared memory.
>> +     * If the state of the eth device is RTE_ETH_DEV_UNUSED, it 
>> means the
>> +     * eth device has been released.
>> +     */
>> +    if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
>> +        eth_dev->state == RTE_ETH_DEV_UNUSED)
>> +        return 0;
>> +
>>       if (dev_uninit) {
>>           ret = dev_uninit(eth_dev);
>>           if (ret)
>>
>
> .
diff mbox series

Patch

diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
index 8edca82ce8..fcabae02fa 100644
--- a/lib/ethdev/ethdev_pci.h
+++ b/lib/ethdev/ethdev_pci.h
@@ -151,6 +151,16 @@  rte_eth_dev_pci_generic_remove(struct rte_pci_device *pci_dev,
 	if (!eth_dev)
 		return 0;
 
+	/*
+	 * In secondary process, a released eth device can be found by its name
+	 * in shared memory.
+	 * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
+	 * eth device has been released.
+	 */
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+	    eth_dev->state == RTE_ETH_DEV_UNUSED)
+		return 0;
+
 	if (dev_uninit) {
 		ret = dev_uninit(eth_dev);
 		if (ret)