[dpdk-dev] [PATCH 02/11] examples/ipsec-secgw: Fixed init of aead crypto devices

[De Lara Guarch, Pablo]

Hi Aviad,

> -----Original Message-----
> From: Aviad Yehezkel [mailto:aviadye at dev.mellanox.co.il]
> Sent: Sunday, October 15, 2017 1:54 PM
> To: dev at dpdk.org; Gonzalez Monroy, Sergio
> <sergio.gonzalez.monroy at intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch at intel.com>; aviadye at mellanox.com
> Cc: borisp at mellanox.com; akhil.goyal at nxp.com;
> hemant.agrawal at nxp.com; Nicolau, Radu <radu.nicolau at intel.com>;
> Doherty, Declan <declan.doherty at intel.com>; liranl at mellanox.com;
> nelio.laranjeiro at 6wind.com; thomas at monjalon.net
> Subject: Re: [dpdk-dev][PATCH 02/11] examples/ipsec-secgw: Fixed init of
> aead crypto devices
> 

Commit titles should start with infinitive and not with lowercase.
e.g. examples/ipsec-secgw: fix init of aead crypto devices

Also, since this is a fix, you should include a Fixes line with the commit id
where the issues was introduced, and CC stable, if the issue was not introduced 
in the current release.

Take a look at the following document, that explains in detail the contribution guidelines:
http://dpdk.org/doc/guides/contributing/patches.html

Also, I have a comment below.

Thanks,
Pablo


> 
> 
> On 10/14/2017 4:27 PM, aviadye at dev.mellanox.co.il wrote:
> > From: Aviad Yehezkel <aviadye at mellanox.com>
> >
> > This was broken since new aead xfrom was introduced
> >
> > Signed-off-by: Aviad Yehezkel <aviadye at mellanox.com>

...

> >   	if (ret != -ENOENT)
> > @@ -1192,19 +1195,25 @@ add_cdev_mapping(struct
> rte_cryptodev_info *dev_info, uint16_t cdev_id,
> >   		if (i->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC)
> >   			continue;
> >


I think it is simpler to leave the code as it is, and add:

+		if (i->sym.xform_type == RTE_CRYPTO_SYM_XFORM_AEAD) {
+			ret |= add_mapping(map, str, cdev_id, qp, params,
+					ipsec_ctx, NULL, NULL, i);
+			continue;
+		}

And just add NULL in the existing add_mapping() function, without modifying the for loop.
The other changes were OK to me.

> > -		if (i->sym.xform_type !=
> RTE_CRYPTO_SYM_XFORM_CIPHER)
> > +		if (i->sym.xform_type == RTE_CRYPTO_SYM_XFORM_AEAD)
> {
> > +			ret |= add_mapping(map, str, cdev_id, qp, params,
> > +					ipsec_ctx, NULL, NULL, i);
> >   			continue;
> > +		}