[v2] cryptodev: not close device when secondary exit

Message ID 20250103030957.1117-1-ming.1.yang@nokia-sbell.com (mailing list archive)
State New
Delegated to: akhil goyal
Headers
Series [v2] cryptodev: not close device when secondary exit |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-unit-arm64-testing pending Testing pending
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing pending Testing pending
ci/iol-unit-amd64-testing pending Testing pending
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

Yang Ming Jan. 3, 2025, 3:09 a.m. UTC
The secordary process should not close the crypto device when
it exits because the primary process still manage the device.
There is no reason with occurring error log below when
secordary process exits without any operation on the crypto
device while primary process starts the device.

Case situation:
eal_bus_cleanup has been added in rte_eal_cleanup. But for
secondary process, eal_bus_cleanup will trigger vdev_cleanup
which trigger rte_vdev_driver to remove. Then crypto devices
will execute ipsec_mb_remove to rte_cryptodev_pmd_destroy.
Finially, rte_cryptodev_close will be called by secordary
process exit.

Error logs occur as below when the secordary process exit:
CRYPTODEV: rte_cryptodev_close() line 1453: Device 0 must be
stopped before closing

Function call trace: rte_eal_cleanup->eal_bus_cleanup->
vdev_cleanup->rte_vdev_driver_remove->ipsec_mb_remove->
rte_cryptodev_pmd_destroy->rte_cryptodev_pmd_release_device->
rte_cryptodev_close

Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>
---
 lib/cryptodev/rte_cryptodev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Akhil Goyal Jan. 8, 2025, 12:21 p.m. UTC | #1
> The secordary process should not close the crypto device when
> it exits because the primary process still manage the device.
> There is no reason with occurring error log below when
> secordary process exits without any operation on the crypto
> device while primary process starts the device.
> 
> Case situation:
> eal_bus_cleanup has been added in rte_eal_cleanup. But for
> secondary process, eal_bus_cleanup will trigger vdev_cleanup
> which trigger rte_vdev_driver to remove. Then crypto devices
> will execute ipsec_mb_remove to rte_cryptodev_pmd_destroy.
> Finially, rte_cryptodev_close will be called by secordary
> process exit.
> 
> Error logs occur as below when the secordary process exit:
> CRYPTODEV: rte_cryptodev_close() line 1453: Device 0 must be
> stopped before closing
> 
> Function call trace: rte_eal_cleanup->eal_bus_cleanup->
> vdev_cleanup->rte_vdev_driver_remove->ipsec_mb_remove->
> rte_cryptodev_pmd_destroy->rte_cryptodev_pmd_release_device->
> rte_cryptodev_close
> 
> Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>
> ---
>  lib/cryptodev/rte_cryptodev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index 85a4b46ac9..ed1021f635 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -1142,7 +1142,7 @@ rte_cryptodev_pmd_release_device(struct
> rte_cryptodev *cryptodev)
>  	cryptodev_fp_ops_reset(rte_crypto_fp_ops + dev_id);
> 
>  	/* Close device only if device operations have been set */
> -	if (cryptodev->dev_ops) {
> +	if (cryptodev->dev_ops && (rte_eal_process_type() ==
> RTE_PROC_PRIMARY)) {
>  		ret = rte_cryptodev_close(dev_id);
>  		if (ret < 0)
>  			return ret;
I believe dev_close is actually not required in pmd_release_device.
Dev_close need to be called from the application separately before it is released
which I think is already happening. 

Adding more people for review.
  

Patch

diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index 85a4b46ac9..ed1021f635 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -1142,7 +1142,7 @@  rte_cryptodev_pmd_release_device(struct rte_cryptodev *cryptodev)
 	cryptodev_fp_ops_reset(rte_crypto_fp_ops + dev_id);
 
 	/* Close device only if device operations have been set */
-	if (cryptodev->dev_ops) {
+	if (cryptodev->dev_ops && (rte_eal_process_type() == RTE_PROC_PRIMARY)) {
 		ret = rte_cryptodev_close(dev_id);
 		if (ret < 0)
 			return ret;