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

[Yang Ming <ming.1.yang@nokia-sbell.com>]

On 2025/1/8 20:21, Akhil Goyal wrote:
> Caution: This is an external email. Please be very careful when clicking links or opening attachments. See http://nok.it/nsb for additional information.
>
>> 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.
>
>
Hi Akhil, I suppose the change of removing `dev_close` in 
`pmd_release_device` requires much more effort and commit size because 
this change will impact a lot of examples and cases. As there is no more 
comments from other people, can we accept this patch as a little 
improvement for the second process's scenario and wait for the entire 
improvement from anyone who is interested in this entire work?