DPDK patches and discussions
 help / color / mirror / Atom feed
From: Akhil Goyal <akhil.goyal@nxp.com>
To: "Zhang, Roy Fan" <roy.fan.zhang@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"Doherty, Declan" <declan.doherty@intel.com>,
	"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
Subject: Re: [dpdk-dev] [RFC PATCH 1/9] security: introduce CPU Crypto action type and API
Date: Tue, 10 Sep 2019 11:25:26 +0000	[thread overview]
Message-ID: <VE1PR04MB66398D6133890E025AF3E80BE6B60@VE1PR04MB6639.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <9F7182E3F746AB4EA17801C148F3C604336AF46E@IRSMSX101.ger.corp.intel.com>

Hi Fan,
> 
> Hi Akhil,
> 
> You are right, the new API will process the crypto workload, no heavy enqueue
> Dequeue operations required.
> 
> Cryptodev tends to support multiple crypto devices, including HW and SW.
> The 3-cache line access, iova address computation and assignment, simulation
> of async enqueue/dequeue operations, allocate and free crypto ops, even the
> mbuf linked-list for scatter-gather buffers are too heavy for SW crypto PMDs.

Why cant we have a cryptodev synchronous API which work on plain bufs as your suggested
API and use the same crypto sym_session creation logic as it was before? It will perform
same as it is doing in this series.

> 
> To create this new synchronous API in cryptodev cannot avoid the problem
> listed above:  first the API shall not serve only to part of the crypto (SW) PMDs -
> as you know, it is Cryptodev. The users can expect some PMD only support part
> of the overall algorithms, but not the workload processing API.

Why cant we have an optional data path in cryptodev for synchronous behavior if the
underlying PMD support it. It depends on the PMD to decide whether it can have it supported or not.
Only a feature flag will be needed to decide that.
One more option could be a PMD API which the application can directly call if the
mode is only supported in very few PMDs. This could be a backup if there is a 
requirement of deprecation notice etc.

> 
> Another reason is, there is assumption made, first when creating a crypto op
> we have to allocate the memory to hold crypto op + sym op + iv, - we cannot
> simply declare an array of crypto ops in the run-time and discard it when
> processing
> is done. Also we need to fill aad and digest HW address, which is not required for
> SW at all.

We are defining a new API which may have its own parameters and requirements which
Need to be fulfilled. In case it was a rte_security API, then also you are defining a new way
Of packet execution and API params. So it would be same.
You can reduce the cache line accesses as you need in the new API.
The session logic need not be changed from crypto session to security session.
Only the data patch need to be altered as per the new API.

> 
> Bottom line: using crypto op will still have 3 cache-line access performance
> problem.
> 
> So if we to create the new API in Cryptodev instead of rte_security, we need to
> create new crypto op structure only for the SW PMDs, carefully document them
> to not confuse with existing cryptodev APIs, make new device feature flags to
> indicate the API is not supported by some PMDs, and again carefully document
> them of these device feature flags.

The explanation of the new API will also happen in case it is a security API. Instead you need
to add more explanation for session also which is already there in cryptodev.

> 
> So, to push these changes to rte_security instead the above problem can be
> resolved,
> and the performance improvement because of this change is big for smaller
> packets
> - I attached a performance test app in the patchset.

I believe there wont be any perf gap in case the optimized new cryptodev API is used.

> 
> For rte_security, we already have inline-crypto type that works quite close to the
> this
> new API, the only difference is that it is processed by the CPU cycles. As you may
> have already seen the ipsec-library has wrapped these changes, and ipsec-secgw
> has only minimum updates to adopt this change too. So to the end user, if they
> use IPSec this patchset can seamlessly enabled with just commandline update
> when
> creating an SA.

In the IPSec application I do not see the changes wrt the new execution API.
So the data path is not getting handled there. It looks incomplete. The user experience
to use the new API will definitely be changed.

So I believe this patchset is not required in rte_security, we can have it in cryptodev unless
I have missed something.

> 
> Regards,
> Fan
> 
> 
> > -----Original Message-----
> > From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> > Sent: Friday, September 6, 2019 10:01 AM
> > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Doherty, Declan
> > <declan.doherty@intel.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>
> > Subject: RE: [RFC PATCH 1/9] security: introduce CPU Crypto action type and
> > API
> >
> >
> > Hi Fan,
> > >
> > > Hi Akhil,
> > >
> > > This action type allows the burst of symmetric crypto workload using
> > > the same algorithm, key, and direction being processed by CPU cycles
> > synchronously.
> > > This flexible action type does not require external hardware
> > > involvement, having the crypto workload processed synchronously, and
> > > is more performant than Cryptodev SW PMD due to the saved cycles on
> > > removed "async mode simulation" as well as 3 cacheline access of the
> > crypto ops.
> >
> > Does that mean application will not call the cryptodev_enqueue_burst and
> > corresponding dequeue burst.
> > It would be a new API something like process_packets and it will have the
> > crypto processed packets while returning from the API?
> >
> > I still do not understand why we cannot do with the conventional crypto lib
> > only.
> > As far as I can understand, you are not doing any protocol processing or any
> > value add To the crypto processing. IMO, you just need a synchronous crypto
> > processing API which Can be defined in cryptodev, you don't need to re-
> > create a crypto session in the name of Security session in the driver just to do
> > a synchronous processing.
> >
> > >
> > > AESNI-GCM and AESNI-MB PMDs are updated with this support. There is a
> > > small performance test app under
> > > app/test/security_aesni_gcm(mb)_perftest to prove.
> > >
> > > For the new API
> > > The packet is sent to the crypto device for symmetric crypto
> > > processing. The device will encrypt or decrypt the buffer based on the
> > > session data specified and preprocessed in the security session.
> > > Different than the inline or lookaside modes, when the function exits,
> > > the user will expect the buffers are either processed successfully, or
> > > having the error number assigned to the appropriate index of the status
> > array.
> > >
> > > Will update the program's guide in the v1 patch.
> > >
> > > Regards,
> > > Fan
> > >
> > > > -----Original Message-----
> > > > From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> > > > Sent: Wednesday, September 4, 2019 11:33 AM
> > > > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> > > > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Doherty,
> > > > Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo
> > > > <pablo.de.lara.guarch@intel.com>
> > > > Subject: RE: [RFC PATCH 1/9] security: introduce CPU Crypto action
> > > > type and API
> > > >
> > > > Hi Fan,
> > > >
> > > > >
> > > > > This patch introduce new RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO
> > > > action
> > > > > type to security library. The type represents performing crypto
> > > > > operation with CPU cycles. The patch also includes a new API to
> > > > > process crypto operations in bulk and the function pointers for PMDs.
> > > > >
> > > > I am not able to get the flow of execution for this action type.
> > > > Could you please elaborate the flow in the documentation. If not in
> > > > documentation right now, then please elaborate the flow in cover letter.
> > > > Also I see that there are new APIs for processing crypto operations in
> > bulk.
> > > > What does that mean. How are they different from the existing APIs
> > > > which are also handling bulk crypto ops depending on the budget.
> > > >
> > > >
> > > > -Akhil


  reply	other threads:[~2019-09-10 11:25 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-03 15:40 [dpdk-dev] [RFC PATCH 0/9] security: add software synchronous crypto process Fan Zhang
2019-09-03 15:40 ` [dpdk-dev] [RFC PATCH 1/9] security: introduce CPU Crypto action type and API Fan Zhang
2019-09-04 10:32   ` Akhil Goyal
2019-09-04 13:06     ` Zhang, Roy Fan
2019-09-06  9:01       ` Akhil Goyal
2019-09-06 13:12         ` Zhang, Roy Fan
2019-09-10 11:25           ` Akhil Goyal [this message]
2019-09-11 13:01             ` Ananyev, Konstantin
2019-09-06 13:27         ` Ananyev, Konstantin
2019-09-10 10:44           ` Akhil Goyal
2019-09-11 12:29             ` Ananyev, Konstantin
2019-09-12 14:12               ` Akhil Goyal
2019-09-16 14:53                 ` Ananyev, Konstantin
2019-09-16 15:08                   ` Ananyev, Konstantin
2019-09-17  6:02                   ` Akhil Goyal
2019-09-18  7:44                     ` Ananyev, Konstantin
2019-09-25 18:24                       ` Ananyev, Konstantin
2019-09-27  9:26                         ` Akhil Goyal
2019-09-30 12:22                           ` Ananyev, Konstantin
2019-09-30 13:43                             ` Akhil Goyal
2019-10-01 14:49                               ` Ananyev, Konstantin
2019-10-03 13:24                                 ` Akhil Goyal
2019-10-07 12:53                                   ` Ananyev, Konstantin
2019-10-09  7:20                                     ` Akhil Goyal
2019-10-09 13:43                                       ` Ananyev, Konstantin
2019-10-11 13:23                                         ` Akhil Goyal
2019-10-13 23:07                                           ` Zhang, Roy Fan
2019-10-14 11:10                                             ` Ananyev, Konstantin
2019-10-15 15:02                                               ` Akhil Goyal
2019-10-16 13:04                                                 ` Ananyev, Konstantin
2019-10-15 15:00                                             ` Akhil Goyal
2019-10-16 22:07                                           ` Ananyev, Konstantin
2019-10-17 12:49                                             ` Ananyev, Konstantin
2019-10-18 13:17                                             ` Akhil Goyal
2019-10-21 13:47                                               ` Ananyev, Konstantin
2019-10-22 13:31                                                 ` Akhil Goyal
2019-10-22 17:44                                                   ` Ananyev, Konstantin
2019-10-22 22:21                                                     ` Ananyev, Konstantin
2019-10-23 10:05                                                     ` Akhil Goyal
2019-10-30 14:23                                                       ` Ananyev, Konstantin
2019-11-01 13:53                                                         ` Akhil Goyal
2019-09-03 15:40 ` [dpdk-dev] [RFC PATCH 2/9] crypto/aesni_gcm: add rte_security handler Fan Zhang
2019-09-03 15:40 ` [dpdk-dev] [RFC PATCH 3/9] app/test: add security cpu crypto autotest Fan Zhang
2019-09-03 15:40 ` [dpdk-dev] [RFC PATCH 4/9] app/test: add security cpu crypto perftest Fan Zhang
2019-09-03 15:40 ` [dpdk-dev] [RFC PATCH 5/9] crypto/aesni_mb: add rte_security handler Fan Zhang
2019-09-03 15:40 ` [dpdk-dev] [RFC PATCH 6/9] app/test: add aesni_mb security cpu crypto autotest Fan Zhang
2019-09-03 15:40 ` [dpdk-dev] [RFC PATCH 7/9] app/test: add aesni_mb security cpu crypto perftest Fan Zhang
2019-09-03 15:40 ` [dpdk-dev] [RFC PATCH 8/9] ipsec: add rte_security cpu_crypto action support Fan Zhang
2019-09-03 15:40 ` [dpdk-dev] [RFC PATCH 9/9] examples/ipsec-secgw: add security " Fan Zhang
2019-09-06 13:13 ` [dpdk-dev] [PATCH 00/10] security: add software synchronous crypto process Fan Zhang
2019-09-06 13:13   ` [dpdk-dev] [PATCH 01/10] security: introduce CPU Crypto action type and API Fan Zhang
2019-09-18 12:45     ` Ananyev, Konstantin
2019-09-29  6:00     ` Hemant Agrawal
2019-09-29 16:59       ` Ananyev, Konstantin
2019-09-30  9:43         ` Hemant Agrawal
2019-10-01 15:27           ` Ananyev, Konstantin
2019-10-02  2:47             ` Hemant Agrawal
2019-09-06 13:13   ` [dpdk-dev] [PATCH 02/10] crypto/aesni_gcm: add rte_security handler Fan Zhang
2019-09-18 10:24     ` Ananyev, Konstantin
2019-09-06 13:13   ` [dpdk-dev] [PATCH 03/10] app/test: add security cpu crypto autotest Fan Zhang
2019-09-06 13:13   ` [dpdk-dev] [PATCH 04/10] app/test: add security cpu crypto perftest Fan Zhang
2019-09-06 13:13   ` [dpdk-dev] [PATCH 05/10] crypto/aesni_mb: add rte_security handler Fan Zhang
2019-09-18 15:20     ` Ananyev, Konstantin
2019-09-06 13:13   ` [dpdk-dev] [PATCH 06/10] app/test: add aesni_mb security cpu crypto autotest Fan Zhang
2019-09-06 13:13   ` [dpdk-dev] [PATCH 07/10] app/test: add aesni_mb security cpu crypto perftest Fan Zhang
2019-09-06 13:13   ` [dpdk-dev] [PATCH 08/10] ipsec: add rte_security cpu_crypto action support Fan Zhang
2019-09-26 23:20     ` Ananyev, Konstantin
2019-09-27 10:38     ` Ananyev, Konstantin
2019-09-06 13:13   ` [dpdk-dev] [PATCH 09/10] examples/ipsec-secgw: add security " Fan Zhang
2019-09-06 13:13   ` [dpdk-dev] [PATCH 10/10] doc: update security cpu process description Fan Zhang
2019-09-09 12:43   ` [dpdk-dev] [PATCH 00/10] security: add software synchronous crypto process Aaron Conole
2019-10-07 16:28   ` [dpdk-dev] [PATCH v2 " Fan Zhang
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 01/10] security: introduce CPU Crypto action type and API Fan Zhang
2019-10-08 13:42       ` Ananyev, Konstantin
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 02/10] crypto/aesni_gcm: add rte_security handler Fan Zhang
2019-10-08 13:44       ` Ananyev, Konstantin
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 03/10] app/test: add security cpu crypto autotest Fan Zhang
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 04/10] app/test: add security cpu crypto perftest Fan Zhang
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 05/10] crypto/aesni_mb: add rte_security handler Fan Zhang
2019-10-08 16:23       ` Ananyev, Konstantin
2019-10-09  8:29       ` Ananyev, Konstantin
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 06/10] app/test: add aesni_mb security cpu crypto autotest Fan Zhang
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 07/10] app/test: add aesni_mb security cpu crypto perftest Fan Zhang
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 08/10] ipsec: add rte_security cpu_crypto action support Fan Zhang
2019-10-08 23:28       ` Ananyev, Konstantin
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 09/10] examples/ipsec-secgw: add security " Fan Zhang
2019-10-07 16:28     ` [dpdk-dev] [PATCH v2 10/10] doc: update security cpu process description Fan Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VE1PR04MB66398D6133890E025AF3E80BE6B60@VE1PR04MB6639.eurprd04.prod.outlook.com \
    --to=akhil.goyal@nxp.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=pablo.de.lara.guarch@intel.com \
    --cc=roy.fan.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).