Re: [dpdk-dev] [PATCH v3 0/4] add AESNI-MB rawdev for multi-function processing

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi David,
> Hi Akhil,
> 
> > -----Original Message-----
> > From: Akhil Goyal <akhil.goyal@nxp.com>
> > Sent: Wednesday, April 22, 2020 11:51 AM
> > Hi David,
> > > > >>
> > > > >> I don't agree rte_security addresses the problem of different
> > > > >> device types supporting the same services. The problem being
> > > > >> addressed here is a single device which supports the chaining of
> > > > >> multiple services (sym crypto & error detection)
> > > > >
> > > > > Doing IPsec processing in Rx or Tx of a NIC is not chaining?
> > > > >
> > > > I wouldn't consider an inline crypto offload or full IPsec offload a
> > > > chained operation in the vein being proposed here where completely
> > > > independent services (in the view of DPDK which are currently on
> > > > independent devices and APIs) are linked together.
> > > >
> > > > We did look at using rte_security here but it wasn't considered
> > > > suitable for a chaining of non-crypto operations such as CRC or
> > > > possibly compression in the future, as it would still run into the
> > > > issue of having to use the cryptodev enq/deq API in the lookaside offload
> > case.
> > > >
> > > >
> > I did not look at your patches completely, but looking at the ops that you
> > have added For rawdev are pretty much same as that of a crypto device.
> >
> > I see that there are 2 types of ops that you need
> > - session create/destroy
> > - enq/deq
> >
> > On the first impression of your patchset, I see that you want to enq to driver
> > only once for both The operations - CRC and crypto.
> >
> > So what is the issue in using the cryptodev_enqueue for processing in the
> > existing AESNI-MB driver.
> > For session creation, the cryptodev layer will not give flexibility to add
> > CRC+crypto kind of sessions.
> > But in case of rte_security, you can define your new session xform based on
> > your requirement.
> >
> > And while doing the cryptodev enq/deq, based on the session type, you can
> > process the packet Specific to your usecase in your aesni-mb PMD
> >
> > Now if you want to add compression also along with crypto, then you can
> > define another xform which Will be combination of crypto+compression and
> > the aesni-mb PMD can have another mode which Can make sessions based
> > on the new xform and the enq and deq can be done using the cryptodev
> > enq/deq.
> > For all your cases you will be having only one action type - lookaside protocol
> > and can define different Protocols (that may not be standard).
> >
> > So to conclude, your AESNI-MB will have 3 types of operations
> > - plain crypto
> > - crc+crypto
> > - compression+crypto
> >
> > I believe this is doable or did I miss something very obvious?
> 
> [DC] Thank you for this feedback
> 
> I have done this exact same analysis on rte_security and how we could use it.
> 
> The main issue of this approach (and it may be possible to easily overcome) is
> that ultimately crypto_op's need
> to be enqueued into cryptodev. This means we can't easily control the CRC (or
> compression in the future) at the
> operation level - application developers using this API would create a
> Crypto+CRC security xform session  for a
> particular flow but may want to turn off the CRC part for some packets in that
> flow.
> 
> There are a number of ways this issue could possibly be overcome:
> 1) the auth offset/length fields in a rte_crypto_op could be overloaded to
> control the CRC part of the combined operation
>     - this is not the cleanest approach
> 2) we add a "security" op struct of some type to the union at end of the
> rte_crypto_op
>     - to avoid any circular dependencies, this would need to be opaque to
> rte_cryptodev
>     - rte_cryptodev should not be aware of rte_security
> 
> Number 2 above is probably the cleaner and more preferable approach.

Yes, it is preferred, but it should be a union to rte_crypto_sym_op/rte_crypto_asym_op.
Crypto_op->type as RTE_CRYPTO_OP_TYPE_SECURITY and sess_type as
RTE_CRYPTO_OP_SECURITY_SESSION
The size of rte_crypto_op will remain as is and there will be no ABI breakage I guess.

One more thing that can be looked into is the recently added CPU crypto process API
If that could of any use, we may extend that if need be.

> 
> The other approach is that CRC is either on/off at the session level. That
> limitation would then need to be adhered
> by application developers, which is something we would ideally like to avoid.

You mean that CRC can be on/off per session as well as per packet?
I think that can also be handled when you are defining your own security_op for per packet.

> 
> The rawdev multi-function approach did not have these issues which is one of
> the reasons we have pursued this
> approach to date.
> 
> However, we think the rte_security approach is workable.
> It still requires some deeper analysis but with your support, we think we can
> overcome the challenges.
> 
Yes, please let me know where ever my help is required.