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.
> >
> > Yes, instead it just call rte_security_process_cpu_crypto_bulk(...)
> >
> > > It would be a new API something like process_packets and it will have the
> > crypto processed packets while returning from the API?
> >
> > Yes, though the plan is that API will operate on raw data buffers, not
> > mbufs.
> >
> > >
> > > 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.
> >
> > I suppose your question is why not to have
> > rte_crypot_process_cpu_crypto_bulk(...) instead?
> > The main reason is that would require disruptive changes in existing
> > cryptodev
> > API
> > (would cause ABI/API breakage).
> > Session for RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO need some extra
> > information
> > that normal crypto_sym_xform doesn't contain
> > (cipher offset from the start of the buffer, might be something extra in
> > future).
>
> Cipher offset will be part of rte_crypto_op.
fill/read (+ alloc/free) is one of the main things that slowdown current
crypto-op approach.
That's why the general idea - have all data that wouldn't change from packet to
packet
included into the session and setup it once at session_init().
> If you intend not to use rte_crypto_op
> You can pass this as an argument in the new cryptodev API.
You mean extra parameter in rte_security_process_cpu_crypto_bulk()?
It can be in theory, but that solution looks a bit ugly:
why to pass for each call something that would be constant per session?
Again having that value constant per session might allow some extra
optimisations
That would be hard to achieve for dynamic case.
and not extendable:
Suppose tomorrow will need to add something extra (some new algorithm support
or so).
With what you proposing will need to new parameter to the function,
which means API breakage.
> Something extra will also cause ABI breakage in security as well.
> So it will be same.
I don't think it would.
AFAIK, right now this patch doesn't introduce any API/ABI breakage.
Iinside struct rte_security_session_conf we have a union of xforms
depending on session type.
So as long as cpu_crypto_xform wouldn't exceed sizes of other xform -
I believe no ABI breakage will appear.
>
> > Also right now there is no way to add new type of crypto_sym_session without
> > either breaking existing crypto-dev ABI/API or introducing new structure
> > (rte_crypto_sym_cpu_session or so) for that.
>
> What extra info is required in rte_cryptodev_sym_session to get the
> rte_crypto_sym_cpu_session.
Right now - just cipher_offset (see above).
What else in future (if any) - don't know.
> I don't think there is any.
> I believe the same crypto session will be able to work synchronously as well.
Exactly the same - problematically, see above.
> We would only need a new API to perform synchronous actions.
> That will reduce the duplication code significantly
> in the driver to support 2 different kind of APIs with similar code inside.
> Please correct me in case I am missing something.
To add new API into crypto-dev would also require changes in the PMD,
it wouldn't come totally free and I believe would require roughly the same
amount of changes.
>
>
> > While rte_security is designed in a way that we can add new session types
> > and
> > related parameters without causing API/ABI breakage.
>
> Yes the intent is to add new sessions based on various protocols that can be
> supported by the driver.
Various protocols and different types of sessions (and devices they belong to).
Let say right now we have INLINE_CRYPTO, INLINE_PROTO, LOOKASIDE_PROTO, etc.
Here we introduce new type of session.
> It is not that we should find it as an alternative to cryptodev and using it
> just because it will not cause
> ABI/API breakage.
I am considering this new API as an alternative to existing ones, but as an
extension.
Existing crypto-op API has its own advantages (generic), and I think we should
keep it supported by all crypto-devs.
>From other side rte_security is an extendable framework that suits the purpose:
allows easily (and yes without ABI breakage) introduce new API for special type
of crypto-dev (SW based).
> IMO the code should be placed where its intent is.
>
> >
> > BTW, what is your concern with proposed approach (via rte_security)?
> > From my perspective it is a lightweight change and it is totally optional
> > for the crypto PMDs to support it or not.
> > Konstantin
> >
> > > >
> > > > 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.go...@nxp.com]
> > > > > Sent: Wednesday, September 4, 2019 11:33 AM
> > > > > To: Zhang, Roy Fan <roy.fan.zh...@intel.com>; dev@dpdk.org
> > > > > Cc: Ananyev, Konstantin <konstantin.anan...@intel.com>; Doherty,
> > Declan
> > > > > <declan.dohe...@intel.com>; De Lara Guarch, Pablo
> > > > > <pablo.de.lara.gua...@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
