Hi Aviad, > -----Original Message----- > From: Aviad Yehezkel [mailto:avia...@dev.mellanox.co.il] > Sent: Sunday, October 15, 2017 1:54 PM > To: dev@dpdk.org; Gonzalez Monroy, Sergio > <sergio.gonzalez.mon...@intel.com>; De Lara Guarch, Pablo > <pablo.de.lara.gua...@intel.com>; avia...@mellanox.com > Cc: bor...@mellanox.com; akhil.go...@nxp.com; > hemant.agra...@nxp.com; Nicolau, Radu <radu.nico...@intel.com>; > Doherty, Declan <declan.dohe...@intel.com>; lir...@mellanox.com; > nelio.laranje...@6wind.com; tho...@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, avia...@dev.mellanox.co.il wrote: > > From: Aviad Yehezkel <avia...@mellanox.com> > > > > This was broken since new aead xfrom was introduced > > > > Signed-off-by: Aviad Yehezkel <avia...@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; > > + }
