Re: [dpdk-dev] [PATCH 4/8] common/cpt: add helper functions for asymmetric crypto

[Anoob Joseph <anoobj@marvell.com>]

Hi Akhil,

Minor correction. Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Anoob Joseph
> Sent: Wednesday, October 2, 2019 4:44 PM
> To: Akhil Goyal <akhil.goyal@nxp.com>; Pablo de Lara
> <pablo.de.lara.guarch@intel.com>
> Cc: Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; Narayana Prasad Raju Athreya
> <pathreya@marvell.com>; Fiona Trahe <fiona.trahe@intel.com>; Shally
> Verma <shallyv@marvell.com>; Sunila Sahu <ssahu@marvell.com>;
> dev@dpdk.org
> Subject: RE: [PATCH 4/8] common/cpt: add helper functions for asymmetric
> crypto
> 
> Hi Akhil,
> 
> Please see inline.
> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: Akhil Goyal <akhil.goyal@nxp.com>
> > Sent: Tuesday, October 1, 2019 6:35 PM
> > To: Anoob Joseph <anoobj@marvell.com>; Pablo de Lara
> > <pablo.de.lara.guarch@intel.com>
> > Cc: Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Jerin Jacob
> > Kollanukkaran <jerinj@marvell.com>; Narayana Prasad Raju Athreya
> > <pathreya@marvell.com>; Fiona Trahe <fiona.trahe@intel.com>; Shally
> > Verma <shallyv@marvell.com>; Sunila Sahu <ssahu@marvell.com>;
> > dev@dpdk.org
> > Subject: RE: [PATCH 4/8] common/cpt: add helper functions for
> > asymmetric crypto
> >
> > Hi Anoob,
> > >
> > > From: Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>
> > >
> > > Add helper functions to get meta len for asymmetric operations
> > >
> > > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > > Signed-off-by: Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>
> > > Signed-off-by: Sunila Sahu <ssahu@marvell.com>
> > > ---
> >
> > <snip>
> >
> > > diff --git a/drivers/crypto/octeontx/otx_cryptodev_hw_access.c
> > > b/drivers/crypto/octeontx/otx_cryptodev_hw_access.c
> > > index ad64bf4..0543494 100644
> > > --- a/drivers/crypto/octeontx/otx_cryptodev_hw_access.c
> > > +++ b/drivers/crypto/octeontx/otx_cryptodev_hw_access.c
> > > @@ -412,24 +412,34 @@ otx_cpt_metabuf_mempool_create(const
> struct
> > > rte_cryptodev *dev,
> > >  			       int nb_elements)
> > >  {
> > >  	char mempool_name[RTE_MEMPOOL_NAMESIZE];
> > > -	int sg_mlen, lb_mlen, max_mlen, ret;
> > >  	struct cpt_qp_meta_info *meta_info;
> > >  	struct rte_mempool *pool;
> > > +	int max_mlen = 0;
> > > +	int sg_mlen = 0;
> > > +	int lb_mlen = 0;
> > > +	int ret;
> > >
> > > -	/* Get meta len for scatter gather mode */
> > > -	sg_mlen = cpt_pmd_ops_helper_get_mlen_sg_mode();
> > > +	if (dev->feature_flags & RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO)
> {
> > >
> > > -	/* Extra 32B saved for future considerations */
> > > -	sg_mlen += 4 * sizeof(uint64_t);
> > > +		/* Get meta len for scatter gather mode */
> > > +		sg_mlen = cpt_pmd_ops_helper_get_mlen_sg_mode();
> > >
> > > -	/* Get meta len for linear buffer (direct) mode */
> > > -	lb_mlen = cpt_pmd_ops_helper_get_mlen_direct_mode();
> > > +		/* Extra 32B saved for future considerations */
> > > +		sg_mlen += 4 * sizeof(uint64_t);
> > >
> > > -	/* Extra 32B saved for future considerations */
> > > -	lb_mlen += 4 * sizeof(uint64_t);
> > > +		/* Get meta len for linear buffer (direct) mode */
> > > +		lb_mlen = cpt_pmd_ops_helper_get_mlen_direct_mode();
> > >
> > > -	/* Check max requirement for meta buffer */
> > > -	max_mlen = RTE_MAX(lb_mlen, sg_mlen);
> > > +		/* Extra 32B saved for future considerations */
> > > +		lb_mlen += 4 * sizeof(uint64_t);
> > > +
> > > +		/* Check max requirement for meta buffer */
> > > +		max_mlen = RTE_MAX(lb_mlen, sg_mlen);
> > > +	} else if (dev->feature_flags &
> > > RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO) {
> > > +
> > > +		/* Get meta len for asymmetric operations */
> > > +		max_mlen = cpt_pmd_ops_helper_asym_get_mlen();
> > > +	}
> >
> > I am not sure how this is working.
> > In your octeontx device, you will have both
> > RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO
> > and RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO will be set together. So it
> will
> > not go in else in any case.
> > Please check in rest of the PMD also if this assumption is made
> > somewhere else also.
> >
> 
> [Anoob] The crypto VFs on octeontx would be either sym/asym and cannot
> be both. Hence the above would work. But, you are right. In the above, it
> should be,
> 
> sym_lens = 0;
> asym_lens = 0;
> 
> If (SYM) {
>     /* set various lens for sym */
> }
> 
> If (ASYM) {
>     /* set various lens for asym */
> }
> 
> len = MAX(sym_len, asym_len);
> 
> I'll make the required changes in v2. Hope this clarifies.

[Anoob] The above code snippet is only applicable for crypto_octeontx PMD. And hence the current code should be fine. Initially, I mistook that the suggestion was for a common routine. Nevertheless, I'll add a comment stating that only one feature is supported. 
 
> 
> > >
> > >  	/* Allocate mempool */
> > >
> > > --
> > > 2.7.4