[v4,0/3] cryptodev: move dh type from xform to dh op

  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.

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(-)
  

> -----Original Message-----
> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Sent: Wednesday, April 27, 2022 8:44 AM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH v4 0/3] cryptodev: move dh type from xform to dh op
> 
> 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.
> 
> 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
Series-acked-by: Fan Zhang <roy.fan.zhang@intel.com>
  

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?

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.


> 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
  

> -----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.

> 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
  

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
  

Series-acked-by: Kai Ji <kai.ji@intel.com>

> -----Original Message-----
> From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> Sent: Wednesday, April 27, 2022 8:44 AM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH v4 0/3] cryptodev: move dh type from xform to dh op
> 
> 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.
> 
> 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