diff mbox series

[04/40] cryptodev: reduce number of comments in asym xform

Message ID 20220520055445.40063-5-arkadiuszx.kusztal@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series cryptodev: rsa, dh, ecdh changes |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Arkadiusz Kusztal May 20, 2022, 5:54 a.m. UTC 
  - Reduced number of comments in asymmetric xform.
Information describing basic functionality of well known
algorithms are unnecessary.
- Added information about data memory lifetime.
It was specified how user should work with private data,
and it is user's responsability to clear it.
- Removed NONE asymetric xform.

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 lib/cryptodev/rte_crypto_asym.h | 45 +++++++++++++----------------------------
 lib/cryptodev/rte_cryptodev.c   |  1 -
 2 files changed, 14 insertions(+), 32 deletions(-)

Comments

Akhil Goyal May 24, 2022, 10:59 a.m. UTC | #1 
> - Reduced number of comments in asymmetric xform.
> Information describing basic functionality of well known
> algorithms are unnecessary.
> - Added information about data memory lifetime.
> It was specified how user should work with private data,
> and it is user's responsability to clear it.
> - Removed NONE asymetric xform.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/cryptodev/rte_crypto_asym.h | 45 +++++++++++++----------------------------
>  lib/cryptodev/rte_cryptodev.c   |  1 -
>  2 files changed, 14 insertions(+), 32 deletions(-)
> 
> diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
> index a474b6acd1..0251e8caae 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -55,46 +55,29 @@ enum rte_crypto_curve_id {
>  };
> 
>  /**
> - * Asymmetric crypto transformation types.
> - * Each xform type maps to one asymmetric algorithm
> - * performing specific operation
> - *
> + * Asymmetric crypto algorithm static data.
> + * Data that may be used more than once (e.g. RSA private key).
> + * It is the USER responsibility to keep track of private data memory
> + * lifetime and security of the this data in xform. The same way
> + * it is the USER responsibility to call cryptodev session_clear()
> + * function if a session was created. If session-less not used
> + * xform data should be cleared after successful session creation.
>   */
>  enum rte_crypto_asym_xform_type {
> -	RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED = 0,
> +	RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED,
>  	/**< Invalid xform. */
> -	RTE_CRYPTO_ASYM_XFORM_NONE,
> -	/**< Xform type None.
> -	 * May be supported by PMD to support
> -	 * passthrough op for debugging purpose.
> -	 * if xform_type none , op_type is disregarded.
> -	 */
I believe removing this is not a good idea. As stated, it will help in
Debugging.

>  	RTE_CRYPTO_ASYM_XFORM_RSA,
> -	/**< RSA. Performs Encrypt, Decrypt, Sign and Verify.
> -	 * Refer to rte_crypto_asym_op_type
> -	 */
> +	/**< RSA */
>  	RTE_CRYPTO_ASYM_XFORM_DH,
> -	/**< Diffie-Hellman.
> -	 * Performs Key Generate and Shared Secret Compute.
> -	 * Refer to rte_crypto_asym_op_type
> -	 */
> +	/**< Diffie-Hellman */
>  	RTE_CRYPTO_ASYM_XFORM_DSA,
> -	/**< Digital Signature Algorithm
> -	 * Performs Signature Generation and Verification.
> -	 * Refer to rte_crypto_asym_op_type
> -	 */
> +	/**< Digital Signature Algorithm */
>  	RTE_CRYPTO_ASYM_XFORM_MODINV,
> -	/**< Modular Multiplicative Inverse
> -	 * Perform Modular Multiplicative Inverse b^(-1) mod n
> -	 */
> +	/**< Modular Multiplicative Inverse */
>  	RTE_CRYPTO_ASYM_XFORM_MODEX,
> -	/**< Modular Exponentiation
> -	 * Perform Modular Exponentiation b^e mod n
> -	 */
> +	/**< Modular Exponentiation */
>  	RTE_CRYPTO_ASYM_XFORM_ECDSA,
> -	/**< Elliptic Curve Digital Signature Algorithm
> -	 * Perform Signature Generation and Verification.
> -	 */
> +	/**< Elliptic Curve Digital Signature Algorithm */
>  	RTE_CRYPTO_ASYM_XFORM_ECPM
>  	/**< Elliptic Curve Point Multiplication */
>  };
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index e16e6802aa..691625bd04 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -160,7 +160,6 @@ rte_crypto_aead_operation_strings[] = {
>   * Asymmetric crypto transform operation strings identifiers.
>   */
>  const char *rte_crypto_asym_xform_strings[] = {
> -	[RTE_CRYPTO_ASYM_XFORM_NONE]	= "none",
>  	[RTE_CRYPTO_ASYM_XFORM_RSA]	= "rsa",
>  	[RTE_CRYPTO_ASYM_XFORM_MODEX]	= "modexp",
>  	[RTE_CRYPTO_ASYM_XFORM_MODINV]	= "modinv",
> --
> 2.13.6
Arkadiusz Kusztal May 24, 2022, 5:37 p.m. UTC | #2 
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, May 24, 2022 12:59 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Anoob Joseph <anoobj@marvell.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH 04/40] cryptodev: reduce number of comments in
> asym xform
> 
> > - Reduced number of comments in asymmetric xform.
> > Information describing basic functionality of well known algorithms
> > are unnecessary.
> > - Added information about data memory lifetime.
> > It was specified how user should work with private data, and it is
> > user's responsability to clear it.
> > - Removed NONE asymetric xform.
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/cryptodev/rte_crypto_asym.h | 45 +++++++++++++----------------------------
> >  lib/cryptodev/rte_cryptodev.c   |  1 -
> >  2 files changed, 14 insertions(+), 32 deletions(-)
> >
> > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > b/lib/cryptodev/rte_crypto_asym.h index a474b6acd1..0251e8caae 100644
> > --- a/lib/cryptodev/rte_crypto_asym.h
> > +++ b/lib/cryptodev/rte_crypto_asym.h
> > @@ -55,46 +55,29 @@ enum rte_crypto_curve_id {  };
> >
> >  /**
> > - * Asymmetric crypto transformation types.
> > - * Each xform type maps to one asymmetric algorithm
> > - * performing specific operation
> > - *
> > + * Asymmetric crypto algorithm static data.
> > + * Data that may be used more than once (e.g. RSA private key).
> > + * It is the USER responsibility to keep track of private data memory
> > + * lifetime and security of the this data in xform. The same way
> > + * it is the USER responsibility to call cryptodev session_clear()
> > + * function if a session was created. If session-less not used
> > + * xform data should be cleared after successful session creation.
> >   */
> >  enum rte_crypto_asym_xform_type {
> > -	RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED = 0,
> > +	RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED,
> >  	/**< Invalid xform. */
> > -	RTE_CRYPTO_ASYM_XFORM_NONE,
> > -	/**< Xform type None.
> > -	 * May be supported by PMD to support
> > -	 * passthrough op for debugging purpose.
> > -	 * if xform_type none , op_type is disregarded.
> > -	 */
> I believe removing this is not a good idea. As stated, it will help in Debugging.
> 
> >  	RTE_CRYPTO_ASYM_XFORM_RSA,
> > -	/**< RSA. Performs Encrypt, Decrypt, Sign and Verify.
> > -	 * Refer to rte_crypto_asym_op_type
> > -	 */
> > +	/**< RSA */
> >  	RTE_CRYPTO_ASYM_XFORM_DH,
> > -	/**< Diffie-Hellman.
> > -	 * Performs Key Generate and Shared Secret Compute.
> > -	 * Refer to rte_crypto_asym_op_type
> > -	 */
> > +	/**< Diffie-Hellman */
> >  	RTE_CRYPTO_ASYM_XFORM_DSA,
> > -	/**< Digital Signature Algorithm
> > -	 * Performs Signature Generation and Verification.
> > -	 * Refer to rte_crypto_asym_op_type
> > -	 */
> > +	/**< Digital Signature Algorithm */
> >  	RTE_CRYPTO_ASYM_XFORM_MODINV,
> > -	/**< Modular Multiplicative Inverse
> > -	 * Perform Modular Multiplicative Inverse b^(-1) mod n
> > -	 */
> > +	/**< Modular Multiplicative Inverse */
> >  	RTE_CRYPTO_ASYM_XFORM_MODEX,
> > -	/**< Modular Exponentiation
> > -	 * Perform Modular Exponentiation b^e mod n
> > -	 */
> > +	/**< Modular Exponentiation */
> >  	RTE_CRYPTO_ASYM_XFORM_ECDSA,
> > -	/**< Elliptic Curve Digital Signature Algorithm
> > -	 * Perform Signature Generation and Verification.
> > -	 */
> > +	/**< Elliptic Curve Digital Signature Algorithm */
> >  	RTE_CRYPTO_ASYM_XFORM_ECPM
[Arek] - maybe this is not that important but could not we have asym_algorithm instead of xform_type? There is not ECDSA/ECPM xform at all.
> >  	/**< Elliptic Curve Point Multiplication */  }; diff --git
> > a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c index
> > e16e6802aa..691625bd04 100644
> > --- a/lib/cryptodev/rte_cryptodev.c
> > +++ b/lib/cryptodev/rte_cryptodev.c
> > @@ -160,7 +160,6 @@ rte_crypto_aead_operation_strings[] = {
> >   * Asymmetric crypto transform operation strings identifiers.
> >   */
> >  const char *rte_crypto_asym_xform_strings[] = {
> > -	[RTE_CRYPTO_ASYM_XFORM_NONE]	= "none",
> >  	[RTE_CRYPTO_ASYM_XFORM_RSA]	= "rsa",
> >  	[RTE_CRYPTO_ASYM_XFORM_MODEX]	= "modexp",
> >  	[RTE_CRYPTO_ASYM_XFORM_MODINV]	= "modinv",
> > --
> > 2.13.6
Akhil Goyal May 25, 2022, 5:44 a.m. UTC | #3 
> > > - Reduced number of comments in asymmetric xform.
> > > Information describing basic functionality of well known algorithms
> > > are unnecessary.
> > > - Added information about data memory lifetime.
> > > It was specified how user should work with private data, and it is
> > > user's responsability to clear it.
> > > - Removed NONE asymetric xform.
> > >
> > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > ---
> > >  lib/cryptodev/rte_crypto_asym.h | 45 +++++++++++++--------------------------
> --
> > >  lib/cryptodev/rte_cryptodev.c   |  1 -
> > >  2 files changed, 14 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > > b/lib/cryptodev/rte_crypto_asym.h index a474b6acd1..0251e8caae 100644
> > > --- a/lib/cryptodev/rte_crypto_asym.h
> > > +++ b/lib/cryptodev/rte_crypto_asym.h
> > > @@ -55,46 +55,29 @@ enum rte_crypto_curve_id {  };
> > >
> > >  /**
> > > - * Asymmetric crypto transformation types.
> > > - * Each xform type maps to one asymmetric algorithm
> > > - * performing specific operation
> > > - *
> > > + * Asymmetric crypto algorithm static data.
> > > + * Data that may be used more than once (e.g. RSA private key).
> > > + * It is the USER responsibility to keep track of private data memory
> > > + * lifetime and security of the this data in xform. The same way
> > > + * it is the USER responsibility to call cryptodev session_clear()
> > > + * function if a session was created. If session-less not used
> > > + * xform data should be cleared after successful session creation.
> > >   */
> > >  enum rte_crypto_asym_xform_type {
> > > -	RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED = 0,
> > > +	RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED,
> > >  	/**< Invalid xform. */
> > > -	RTE_CRYPTO_ASYM_XFORM_NONE,
> > > -	/**< Xform type None.
> > > -	 * May be supported by PMD to support
> > > -	 * passthrough op for debugging purpose.
> > > -	 * if xform_type none , op_type is disregarded.
> > > -	 */
> > I believe removing this is not a good idea. As stated, it will help in Debugging.
> >
> > >  	RTE_CRYPTO_ASYM_XFORM_RSA,
> > > -	/**< RSA. Performs Encrypt, Decrypt, Sign and Verify.
> > > -	 * Refer to rte_crypto_asym_op_type
> > > -	 */
> > > +	/**< RSA */
> > >  	RTE_CRYPTO_ASYM_XFORM_DH,
> > > -	/**< Diffie-Hellman.
> > > -	 * Performs Key Generate and Shared Secret Compute.
> > > -	 * Refer to rte_crypto_asym_op_type
> > > -	 */
> > > +	/**< Diffie-Hellman */
> > >  	RTE_CRYPTO_ASYM_XFORM_DSA,
> > > -	/**< Digital Signature Algorithm
> > > -	 * Performs Signature Generation and Verification.
> > > -	 * Refer to rte_crypto_asym_op_type
> > > -	 */
> > > +	/**< Digital Signature Algorithm */
> > >  	RTE_CRYPTO_ASYM_XFORM_MODINV,
> > > -	/**< Modular Multiplicative Inverse
> > > -	 * Perform Modular Multiplicative Inverse b^(-1) mod n
> > > -	 */
> > > +	/**< Modular Multiplicative Inverse */
> > >  	RTE_CRYPTO_ASYM_XFORM_MODEX,
> > > -	/**< Modular Exponentiation
> > > -	 * Perform Modular Exponentiation b^e mod n
> > > -	 */
> > > +	/**< Modular Exponentiation */
> > >  	RTE_CRYPTO_ASYM_XFORM_ECDSA,
> > > -	/**< Elliptic Curve Digital Signature Algorithm
> > > -	 * Perform Signature Generation and Verification.
> > > -	 */
> > > +	/**< Elliptic Curve Digital Signature Algorithm */
> > >  	RTE_CRYPTO_ASYM_XFORM_ECPM
> [Arek] - maybe this is not that important but could not we have asym_algorithm
> instead of xform_type? There is not ECDSA/ECPM xform at all.

Converting everything to asym algo may not be good. As they have different xforms
And different type of operations.
May be we could split into xform type and algo similar to symmetric crypto?

> > >  	/**< Elliptic Curve Point Multiplication */  }; diff --git
> > > a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c index
> > > e16e6802aa..691625bd04 100644
> > > --- a/lib/cryptodev/rte_cryptodev.c
> > > +++ b/lib/cryptodev/rte_cryptodev.c
> > > @@ -160,7 +160,6 @@ rte_crypto_aead_operation_strings[] = {
> > >   * Asymmetric crypto transform operation strings identifiers.
> > >   */
> > >  const char *rte_crypto_asym_xform_strings[] = {
> > > -	[RTE_CRYPTO_ASYM_XFORM_NONE]	= "none",
> > >  	[RTE_CRYPTO_ASYM_XFORM_RSA]	= "rsa",
> > >  	[RTE_CRYPTO_ASYM_XFORM_MODEX]	= "modexp",
> > >  	[RTE_CRYPTO_ASYM_XFORM_MODINV]	= "modinv",
> > > --
> > > 2.13.6
diff mbox series

Patch

diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
index a474b6acd1..0251e8caae 100644
--- a/lib/cryptodev/rte_crypto_asym.h
+++ b/lib/cryptodev/rte_crypto_asym.h
@@ -55,46 +55,29 @@  enum rte_crypto_curve_id {
 };
 
 /**
- * Asymmetric crypto transformation types.
- * Each xform type maps to one asymmetric algorithm
- * performing specific operation
- *
+ * Asymmetric crypto algorithm static data.
+ * Data that may be used more than once (e.g. RSA private key).
+ * It is the USER responsibility to keep track of private data memory
+ * lifetime and security of the this data in xform. The same way
+ * it is the USER responsibility to call cryptodev session_clear()
+ * function if a session was created. If session-less not used
+ * xform data should be cleared after successful session creation.
  */
 enum rte_crypto_asym_xform_type {
-	RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED = 0,
+	RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED,
 	/**< Invalid xform. */
-	RTE_CRYPTO_ASYM_XFORM_NONE,
-	/**< Xform type None.
-	 * May be supported by PMD to support
-	 * passthrough op for debugging purpose.
-	 * if xform_type none , op_type is disregarded.
-	 */
 	RTE_CRYPTO_ASYM_XFORM_RSA,
-	/**< RSA. Performs Encrypt, Decrypt, Sign and Verify.
-	 * Refer to rte_crypto_asym_op_type
-	 */
+	/**< RSA */
 	RTE_CRYPTO_ASYM_XFORM_DH,
-	/**< Diffie-Hellman.
-	 * Performs Key Generate and Shared Secret Compute.
-	 * Refer to rte_crypto_asym_op_type
-	 */
+	/**< Diffie-Hellman */
 	RTE_CRYPTO_ASYM_XFORM_DSA,
-	/**< Digital Signature Algorithm
-	 * Performs Signature Generation and Verification.
-	 * Refer to rte_crypto_asym_op_type
-	 */
+	/**< Digital Signature Algorithm */
 	RTE_CRYPTO_ASYM_XFORM_MODINV,
-	/**< Modular Multiplicative Inverse
-	 * Perform Modular Multiplicative Inverse b^(-1) mod n
-	 */
+	/**< Modular Multiplicative Inverse */
 	RTE_CRYPTO_ASYM_XFORM_MODEX,
-	/**< Modular Exponentiation
-	 * Perform Modular Exponentiation b^e mod n
-	 */
+	/**< Modular Exponentiation */
 	RTE_CRYPTO_ASYM_XFORM_ECDSA,
-	/**< Elliptic Curve Digital Signature Algorithm
-	 * Perform Signature Generation and Verification.
-	 */
+	/**< Elliptic Curve Digital Signature Algorithm */
 	RTE_CRYPTO_ASYM_XFORM_ECPM
 	/**< Elliptic Curve Point Multiplication */
 };
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index e16e6802aa..691625bd04 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -160,7 +160,6 @@  rte_crypto_aead_operation_strings[] = {
  * Asymmetric crypto transform operation strings identifiers.
  */
 const char *rte_crypto_asym_xform_strings[] = {
-	[RTE_CRYPTO_ASYM_XFORM_NONE]	= "none",
 	[RTE_CRYPTO_ASYM_XFORM_RSA]	= "rsa",
 	[RTE_CRYPTO_ASYM_XFORM_MODEX]	= "modexp",
 	[RTE_CRYPTO_ASYM_XFORM_MODINV]	= "modinv",