Re: [dpdk-dev] [PATCH v3 1/2] cryptodev: change queue pair configure structure

["Zhang, Roy Fan" <roy.fan.zhang@intel.com>]

Hi Pablo,

Thanks for the review.
Comments inline.

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Tuesday, January 8, 2019 11:21 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>
> Subject: RE: [PATCH v3 1/2] cryptodev: change queue pair configure
> structure
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Roy Fan
> > Sent: Friday, December 21, 2018 1:56 PM
> > To: dev@dpdk.org
> > Cc: akhil.goyal@nxp.com; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>
> > Subject: [PATCH v3 1/2] cryptodev: change queue pair configure
> > structure
> >
> > This patch changes the cryptodev queue pair configure structure to
> > enable two mempool passed into cryptodev PMD simutaneously.
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > Acked-by: Fiona Trahe <fiona.trahe@@intel.com>
> 
> ...
> 
> > diff --git a/doc/guides/rel_notes/release_19_02.rst
> > b/doc/guides/rel_notes/release_19_02.rst
> > index 47768288a..4420c2441 100644
> > --- a/doc/guides/rel_notes/release_19_02.rst
> > +++ b/doc/guides/rel_notes/release_19_02.rst
> > @@ -130,6 +130,11 @@ API Changes
> >    ``rte_pdump_init()`` and enum ``rte_pdump_socktype`` were deprecated
> >    since 18.05 and are removed in this release.
> >
> > +* cryptodev: as shown in the the 18.08 deprecation notice, the
> > +structure
> 
> Typo. "the 18.11" deprecation notice.
[Fan: Will change] 
> 
> > +  ``rte_cryptodev_qp_conf`` has been added two parameters of
> > + symmetric
> > session
> > +  mempool and symmetric session private data mempool, and the last
> > parameter of
> > +  ``rte_cryptodev_queue_pair_setup()`` is removed.
> > +
> >
> >  ABI Changes
> >  -----------
> 
> I think we need to bump the ABI version of cryptodev due to this, in release
> notes.
> Also, the deprecation notice added for this change, should be removed in
> this patch.

[Fan: Will do] 
> 
> > diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > index ebdf7c35a..abc7a6d5f 100644
> 
> ...
> 
> > --- a/examples/fips_validation/main.c
> > +++ b/examples/fips_validation/main.c
> > @@ -39,7 +39,7 @@ static int
> >  cryptodev_fips_validate_app_int(void)
> >  {
> >  	struct rte_cryptodev_config conf = {rte_socket_id(), 1};
> > -	struct rte_cryptodev_qp_conf qp_conf = {128};
> > +	struct rte_cryptodev_qp_conf qp_conf = {128, NULL, NULL};
> 
> Is this OK? Below, it looks like a mempool was passes (env.mpool), but now it
> is NULL here.
[Fan: we do not need a mempool here as none of the session-less scheme will be tested] 
> 
> >  	int ret;
> >
> >  	ret = rte_cryptodev_configure(env.dev_id, &conf); @@ -52,7 +52,7
> @@
> > cryptodev_fips_validate_app_int(void)
> >  		return ret;
> >
> >  	ret = rte_cryptodev_queue_pair_setup(env.dev_id, 0, &qp_conf,
> > -			rte_socket_id(), env.mpool);
> > +			rte_socket_id());
> >  	if (ret < 0)
> >  		return ret;
> >