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

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> Sent: Wednesday, January 10, 2018 2:22 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>;
> dev at dpdk.org
> Cc: hemant.agrawal at nxp.com; Gonzalez Monroy, Sergio
> <sergio.gonzalez.monroy at intel.com>; Nicolau, Radu
> <radu.nicolau at intel.com>
> Subject: Re: [PATCH] examples/ipsec-secgw: add cryptodev mask option
> 
> 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.

Right, actually I am seeing PCI devices first on my system, so clearly the statement below was wrong.

Looks ok to me then :)

Thanks,
Pablo

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