Re: [dpdk-dev] [PATCH v7 01/11] crypto/dpaa: replace rte_panic instances in crypto/dpaa driver

[Kevin Traynor <ktraynor@redhat.com>]

On 04/26/2018 05:05 PM, Kevin Traynor wrote:
> On 04/24/2018 11:16 PM, Arnon Warshavsky wrote:
>> replace panic calls with log and return value.
>>

Replied to wrong version. Comments are still relevant for v9,

Kevin.

>> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
>> ---
>>  drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c |  8 +++++---
>>  drivers/crypto/dpaa_sec/dpaa_sec.c          | 10 ++++++----
>>  2 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
>> index 58cbce8..a78f3a2 100644
>> --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
>> +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
>> @@ -2883,9 +2883,11 @@ struct rte_security_ops dpaa2_sec_security_ops = {
>>  					RTE_CACHE_LINE_SIZE,
>>  					rte_socket_id());
>>  
>> -		if (cryptodev->data->dev_private == NULL)
>> -			rte_panic("Cannot allocate memzone for private "
>> -				  "device data");
>> +		if (cryptodev->data->dev_private == NULL) {
>> +			DPAA_SEC_ERR("%s() Cannot allocate memzone for private device data",
>> +			__func__);
>> +			return -ENOMEM;
> 
> I'm not familiar with the code but there was a successful allocate
> already, so it seems you should jump to the cleanup section at the end
> of the function before returning.
> 
>> +		}
>>  	}
>>  
>>  	dpaa2_dev->cryptodev = cryptodev;
>> diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c b/drivers/crypto/dpaa_sec/dpaa_sec.c
>> index e456fd5..a4670bf 100644
>> --- a/drivers/crypto/dpaa_sec/dpaa_sec.c
>> +++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
>> @@ -2352,7 +2352,7 @@ struct rte_security_ops dpaa_sec_security_ops = {
>>  		}
>>  	}
>>  
>> -	RTE_LOG(INFO, PMD, "%s cryptodev init\n", cryptodev->data->name);
>> +	DPAA_SEC_INFO("%s cryptodev init\n", cryptodev->data->name);
> 
> This fix is an unrelated to the patchset. Perhaps as it's trivial the
> maintainer will allow it
> 
>>  	return 0;
>>  
>>  init_error:
>> @@ -2384,9 +2384,11 @@ struct rte_security_ops dpaa_sec_security_ops = {
>>  					RTE_CACHE_LINE_SIZE,
>>  					rte_socket_id());
>>  
>> -		if (cryptodev->data->dev_private == NULL)
>> -			rte_panic("Cannot allocate memzone for private "
>> -					"device data");
>> +		if (cryptodev->data->dev_private == NULL) {
>> +			DPAA_SEC_ERR("%s() Cannot allocate memzone for private device data",
>> +			__func__);
>> +			return -ENOMEM;
> 
> Same comment as above
> 
>> +		}
>>  	}
>>  
>>  	dpaa_dev->crypto_dev = cryptodev;
>>
>