[PATCH] eal: fix bus cleanup in secondary process

[PATCH] eal: fix bus cleanup in secondary process

[myang]

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 our crypto devices
will execute ipsec_mb_remove to rte_cryptodev_pmd_destroy.

Finally error logs occur as below:
CRYPTODEV: rte_cryptodev_close() line 1453: Device 0 must be
stopped before closing
EAL: failed to send to (/tmp/dpdk/l2hicu/mp_socket) due to Bad
file descriptor
EAL: Fail to send request /tmp/dpdk/l2hicu/mp_socket:ipsec_mb_mp_msg
USER1: Create MR request to primary process failed.

Function call trace: rte_eal_cleanup->eal_bus_cleanup->
vdev_cleanup->rte_vdev_driver->ipsec_mb_remove->
1. ipsec_mb_remove->rte_cryptodev_pmd_destroy->
rte_cryptodev_pmd_release_device->rte_cryptodev_close
2. ipsec_mb_remove->ipsec_mb_qp_release->ipsec_mb_secondary_qp_op
->rte_mp_request_async->mp_request_async

Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
Cc: stable@dpdk.org

Signed-off-by: myang <ming.1.yang@nokia-sbell.com>
---
 lib/eal/linux/eal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index a6220524a4..eec791ce1e 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1320,7 +1320,8 @@ rte_eal_cleanup(void)
 	vfio_mp_sync_cleanup();
 #endif
 	rte_mp_channel_cleanup();
-	eal_bus_cleanup();
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		eal_bus_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
 	eal_mp_dev_hotplug_cleanup();
-- 
2.34.1

Re: [PATCH] eal: fix bus cleanup in secondary process

[Stephen Hemminger]

On Thu, 28 Nov 2024 13:48:29 +0800
myang <ming.1.yang@nokia-sbell.com> wrote:

> 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 our crypto devices
> will execute ipsec_mb_remove to rte_cryptodev_pmd_destroy.
> 
> Finally error logs occur as below:
> CRYPTODEV: rte_cryptodev_close() line 1453: Device 0 must be
> stopped before closing
> EAL: failed to send to (/tmp/dpdk/l2hicu/mp_socket) due to Bad
> file descriptor
> EAL: Fail to send request /tmp/dpdk/l2hicu/mp_socket:ipsec_mb_mp_msg
> USER1: Create MR request to primary process failed.
> 
> Function call trace: rte_eal_cleanup->eal_bus_cleanup->
> vdev_cleanup->rte_vdev_driver->ipsec_mb_remove->
> 1. ipsec_mb_remove->rte_cryptodev_pmd_destroy->
> rte_cryptodev_pmd_release_device->rte_cryptodev_close
> 2. ipsec_mb_remove->ipsec_mb_qp_release->ipsec_mb_secondary_qp_op
> ->rte_mp_request_async->mp_request_async  
> 
> Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
> Cc: stable@dpdk.org
> 
> Signed-off-by: myang <ming.1.yang@nokia-sbell.com>

There is was a reason for calling cleanup on shutdown.
It looks like more of a bug in the crypto device not the EAL.


Freebsd and Windows also calls eal_bus_cleanup.

RE: [External] Re: [PATCH] eal: fix bus cleanup in secondary process

[Ming 1. Yang (NSB)]

Hi Stephen,

Yes, You're right. I'm making a new patch for improving in crypto device to solve this issue. And the modification has already worked in our cases.
Anyway, I will upload the patch soon and will mark the status of this patch to Superseded. Thanks.

Brs,
Yang Ming

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org> 
Sent: 2024年11月29日 1:16
To: Ming 1. Yang (NSB) <ming.1.yang@nokia-sbell.com>
Cc: Anatoly Burakov <anatoly.burakov@intel.com>; Bruce Richardson <bruce.richardson@intel.com>; Kevin Laatz <kevin.laatz@intel.com>; Morten Brørup <mb@smartsharesystems.com>; dev@dpdk.org; stable@dpdk.org
Subject: [External] Re: [PATCH] eal: fix bus cleanup in secondary process

Caution: This is an external email. Please be very careful when clicking links or opening attachments. See http://nok.it/nsb for additional information.

On Thu, 28 Nov 2024 13:48:29 +0800
myang <ming.1.yang@nokia-sbell.com> wrote:

> 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 our crypto devices will execute 
> ipsec_mb_remove to rte_cryptodev_pmd_destroy.
> 
> Finally error logs occur as below:
> CRYPTODEV: rte_cryptodev_close() line 1453: Device 0 must be stopped 
> before closing
> EAL: failed to send to (/tmp/dpdk/l2hicu/mp_socket) due to Bad file 
> descriptor
> EAL: Fail to send request /tmp/dpdk/l2hicu/mp_socket:ipsec_mb_mp_msg
> USER1: Create MR request to primary process failed.
> 
> Function call trace: rte_eal_cleanup->eal_bus_cleanup->
> vdev_cleanup->rte_vdev_driver->ipsec_mb_remove->
> 1. ipsec_mb_remove->rte_cryptodev_pmd_destroy->
> rte_cryptodev_pmd_release_device->rte_cryptodev_close
> 2. ipsec_mb_remove->ipsec_mb_qp_release->ipsec_mb_secondary_qp_op
> ->rte_mp_request_async->mp_request_async
> 
> Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
> Cc: stable@dpdk.org
> 
> Signed-off-by: myang <ming.1.yang@nokia-sbell.com>

There is was a reason for calling cleanup on shutdown.
It looks like more of a bug in the crypto device not the EAL.


Freebsd and Windows also calls eal_bus_cleanup.

[PATCH v2] cryptodev: not close device when secondary exit

[Yang Ming]

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;
-- 
2.34.1