[dpdk-dev] [PATCH] examples/ipsec-secgw: add cryptodev mask option

[Akhil Goyal]

Hi Pablo,
On 1/10/2018 6:17 PM, De Lara Guarch, Pablo wrote:
> Hi Akhil,
> 
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
>> Sent: Thursday, December 14, 2017 6:52 AM
>> To: dev at dpdk.org
>> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>;
>> hemant.agrawal at nxp.com; Gonzalez Monroy, Sergio
>> <sergio.gonzalez.monroy at intel.com>; Nicolau, Radu
>> <radu.nicolau at intel.com>; Akhil Goyal <akhil.goyal at nxp.com>
>> Subject: [PATCH] examples/ipsec-secgw: add cryptodev mask option
>>
>> Previously, ipsec-secgw application did not give user the flexibility to decide
>> which crypto device(s) will be used.
>>
>> In this patch, a new cryptodev_mask option is added to the application.
>> Same as portmask, the cryptodev_mask avails the user to mask out the
>> unwanted crypto devices in the system.
>>
>> This patch is similar to the support added in l2fwd-crypto
>> (d2797f51cc63: examples/l2fwd-crypto: add cryptodev mask option)
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal at nxp.com>
>> ---
> 
> ...
> 
> Not sure if you should change the order of the crypto devices that was set previously
> (starting from the end and not from the beginning). Shouldn't we keep it as it was?
Actually as per the current code base there is no fix order of the 
devices to be available. In bus scan, all(pci,fslmc,vdev) have same 
priority(110), which means whatever is first recognized/linked will come 
first.

So the assumption that last cdev_id is HW doesn't seem to be correct.
I just wanted to make the code similar to l2fwd-crypto and the behavior 
of cryptodev_mask similar to what l2fwd-crypto understands.

Please let me know if my understanding is not correct.

> 
>>   	idx = 0;
>> -	/* Start from last cdev id to give HW priority */
>> -	for (cdev_id = rte_cryptodev_count() - 1; cdev_id >= 0; cdev_id--) {
>> +	for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
>>   		struct rte_cryptodev_info cdev_info;
>>
>> +		if (check_cryptodev_mask((uint8_t)cdev_id))
>> +			continue;
>> +
>>   		rte_cryptodev_info_get(cdev_id, &cdev_info);
>>
>>   		if (nb_lcore_params > cdev_info.max_nb_queue_pairs)
>> --
>> 2.9.3
> 
> For the rest, I don't have other objections, so apart from the comment above:
> 
> Acked-by: Pablo de Lara <pablo.de.lara.guarch at intel.com>
> 
> 

Thanks,
Akhil