RE: [EXT] [PATCH v4 0/3] cryptodev: move dh type from xform to dh op

["Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>]

Hi Akhil,

> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Friday, April 29, 2022 8:26 AM
> To: Akhil Goyal <gakhil@marvell.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH v4 0/3] cryptodev: move dh type from xform to dh op
> 
> 
> 
> > -----Original Message-----
> > From: Akhil Goyal <gakhil@marvell.com>
> > Sent: Wednesday, April 27, 2022 5:58 PM
> > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > Subject: RE: [EXT] [PATCH v4 0/3] cryptodev: move dh type from xform
> > to dh op
> >
> > Hi Arek,
> > > Operation type (PUBLIC_KEY_GENERATION, SHARED_SECRET) should be free
> > > to choose for any operation. One xform/session should be enough to
> > > perform both DH operations, if op_type would be xform member,
> > > session would have to be to be created twice for the same group.
> > > Similar problem would be observed in sessionless case.
> > > Additionally, it will help extend DH to support Elliptic Curves.
> > >
> > rte_crypto_asym_op_type is moved to rte_crypto_dh_op_param.
> > But why not move to rte_crypto_asym_op? I see that in other ops also,
> > Op_type is there, we can move that out. Right?
> >
> Yes, we could. Although some of the operations do not use op type
> (POINT_MULT, MODEX) so we would have to extend asym_op_type to contain
> RTE_CRYPTO_ASYM_OP_DEFAULT /**< Default operation */.
> Another proposal was to split op type to:
> CRYPTO and KEY_EXCHANGE_OP
> like I described in here:
> https://patchwork.dpdk.org/project/dpdk/patch/20220407134248.20178-1-
> arkadiuszx.kusztal@intel.com/
> then op stays in algorithm_op.
If op_type will eventually be placed in op_param or in asym_op can be changed later, as it is of less importance.
I would say first we need to decide if we are going to extend this Diffie Hellman struct to support Elliptic Curves (for Montgomery/Edwards there will be another extension, but it is fine, would be in union).
So in this case op_type should not be in xform as:
- DH op will be used with EC xform.
- We would have to create separate sessions for single group.
Then we can add 'point verification' to this or, have separate API structs for all these but then DH would be redundant.

> 
> > Also, I see one more potential issue.
> > There is a union of various ops in rte_crypto_asym_op, but how will
> > User identify which one to use. There should be a union to identify
> > which Struct to choose from.
> Could you show how this union would look like?
> Normally PMD will reject operations that are incorrectly setup, for example
> DH_op + ECDSA_xform or incorrect op type like ENCRYPT.
> 
> >
> >
> > > v4:
> > > - changed op_type coment
> > > - added openssl fix
> > >
> > > Arek Kusztal (3):
> > >   cryptodev: move dh type from xform to dh op
> > >   crypto/openssl: move dh type from xform to dh op
> > >   test/crypto: move dh type from xform to dh op
> > >
> > >  app/test/test_cryptodev_asym.c               | 11 +++---
> > >  drivers/crypto/openssl/rte_openssl_pmd.c     | 54 ++--------------------------
> > >  drivers/crypto/openssl/rte_openssl_pmd_ops.c | 26 --------------
> > >  lib/cryptodev/rte_crypto_asym.h              | 14 ++++----
> > >  4 files changed, 16 insertions(+), 89 deletions(-)
> > >
> > > --
> > > 2.13.6