Re: [dpdk-dev] [RFC PATCH 1/9] security: introduce CPU Crypto action type and API

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Tuesday, October 15, 2019 4:02 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; 'dev@dpdk.org' <dev@dpdk.org>;
> De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; 'Thomas Monjalon' <thomas@monjalon.net>; Doherty, Declan
> <declan.doherty@intel.com>
> Cc: 'Anoob Joseph' <anoobj@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
> Subject: RE: [RFC PATCH 1/9] security: introduce CPU Crypto action type and API
> 
> 
> 
> >
> >
> > > Hi Akhil,
> > >
> > > Thanks for the review and comments!
> > > Knowing you are extremely busy. Here is my point in brief:
> > > I think placing the CPU synchronous crypto in the rte_security make sense, as
> > >
> > > 1. rte_security contains inline crypto and lookaside crypto action type already,
> > adding cpu_crypto action type is reasonable.
> > > 2. rte_security contains the security features may not supported by all devices,
> > such as crypto, ipsec, and PDCP. cpu_crypto follow this
> > > category, again crypto.
> > > 3. placing CPU synchronous crypto API in rte_security is natural - as inline
> > mode works synchronously, too. However cryptodev doesn't.
> > > 4. placing CPU synchronous crypto API in rte_security helps boosting SW
> > crypto performance, I have already provided a simple perf test
> > > inside the unit test in the patchset for the user to try out - just comparing its
> > output against DPDK crypto perf app output.
> > > 5. placing CPU synchronous crypto API in cryptodev will never serve HW
> > lookaside crypto PMDs, as making them to work synchronously
> > > have huge performance penalty. However Cryptodev framework's existing
> > design is providing APIs that will work in all crypto PMDs
> > > (rte_cryptodev_enqueue_burst / dequeue_burst for example), this does not fit
> > in cryptodev's principle.
> > > 6. placing CPU synchronous crypto API in cryptodev confuses the user, as:
> > > 	- the session created for async mode may not work in sync mode
> > > 	- both enqueue/dequeue and cpu_crypto_process does the same crypto
> > processing, but one PMD may support only one API (set),
> > > the other may support another, and the third PMD supports both. We have to
> > provide another API to let the user query which one to
> > > support which.
> > > 	- two completely different code paths for async/sync mode.
> > > 7. You said in the end of the email - placing CPU synchronous crypto API into
> > rte_security is not acceptable as it does not do any
> > > rte_security stuff - crypto isn't? You may call this a quibble, but in my idea, in
> > the patchset both PMDs' implementations did offload the work
> > > to the CPU's special circuit designed dedicated to accelerate the crypto
> > processing.
> > >
> > > To me cryptodev is the one CPU synchronous crypto API should not go into,
> > rte_security is.
> >
> > I also don't understand why rte_security is not an option here.
> > We do have inline-crypto right now, why we can't have cpu-crypto with new
> > process() API here?
> > Actually would like to hear more opinions from the community here -
> > what other interested parties think is the best way for introducing cpu-crypto
> > specific API?
> 
> I have raised this concern in the weekly status meeting as well. But it looks like nobody
> is interested.

That's really a pity...
CC-ing it to TB members, hopefully someone would be interested,
or at least can forward to interested person.
Konstantin