RE: [EXT] [PATCH v2 02/14] cryptodev: reduce number of comments in asym xform

[Akhil Goyal <gakhil@marvell.com>]

> - Reduced number of comments in asymmetric xform.
> Information describing basic functionality of well known
> algorithms are unnecessary.
> - Removed NONE asymetric xform.

I commented on v1 not to remove this and I do not see comment from your side
for removing it.

> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  app/test/test_cryptodev_asym.c  |   2 -
>  lib/cryptodev/rte_crypto_asym.h | 114 ++++++++++++++++------------------------
>  lib/cryptodev/rte_cryptodev.c   |   1 -
>  3 files changed, 46 insertions(+), 71 deletions(-)
> 
> diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
> index 573af2a537..5aa9d65395 100644
> --- a/app/test/test_cryptodev_asym.c
> +++ b/app/test/test_cryptodev_asym.c
> @@ -288,7 +288,6 @@ test_cryptodev_asym_ver(struct rte_crypto_op *op,
>  		break;
>  	case RTE_CRYPTO_ASYM_XFORM_DH:
>  	case RTE_CRYPTO_ASYM_XFORM_DSA:
> -	case RTE_CRYPTO_ASYM_XFORM_NONE:
>  	case RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED:
>  	default:
>  		break;
> @@ -440,7 +439,6 @@ test_cryptodev_asym_op(struct
> crypto_testsuite_params_asym *ts_params,
>  		break;
>  	case RTE_CRYPTO_ASYM_XFORM_DH:
>  	case RTE_CRYPTO_ASYM_XFORM_DSA:
> -	case RTE_CRYPTO_ASYM_XFORM_NONE:
>  	case RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED:
>  	default:
>  		snprintf(test_msg, ASYM_TEST_MSG_LEN,
> diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
> index 7206652458..66ffb29743 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -38,6 +38,40 @@ extern const char *
>  rte_crypto_asym_op_strings[];
> 
>  /**
> + * Buffer to hold crypto params required for asym operations.
> + *
> + * These buffers can be used for both input to PMD and output from PMD.
> When
> + * used for output from PMD, application has to ensure the buffer is large
> + * enough to hold the target data.
> + *
> + * If an operation requires the PMD to generate a random number,
> + * and the device supports CSRNG, 'data' should be set to NULL.
> + * The crypto parameter in question will not be used by the PMD,
> + * as it is internally generated.
> + */
> +typedef struct rte_crypto_param_t {
> +	uint8_t *data;
> +	/**< pointer to buffer holding data */
> +	rte_iova_t iova;
> +	/**< IO address of data buffer */
> +	size_t length;
> +	/**< length of data in bytes */
> +} rte_crypto_param;
> +
> +/** Unsigned big-integer in big-endian format */
> +typedef rte_crypto_param rte_crypto_uint;
> +
> +/**
> + * Structure for elliptic curve point
> + */
> +struct rte_crypto_ec_point {
> +	rte_crypto_param x;
> +	/**< X coordinate */
> +	rte_crypto_param y;
> +	/**< Y coordinate */
> +};

Why is this movement of code done?

> +
> +/**
>   * List of elliptic curves. This enum aligns with
>   * TLS "Supported Groups" registry (previously known  as
>   * NamedCurve registry). FFDH groups are not, and will not
> @@ -55,46 +89,23 @@ enum rte_crypto_curve_id {
>  };
> 
>  /**
> - * Asymmetric crypto transformation types.
> - * Each xform type maps to one asymmetric algorithm
> - * performing specific operation
> - *
> + * Asymmetric crypto algorithms

Since you are deferring the change for xform, it is better to keep the
Above description as well. So no need for this above change.

>   */
>  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 */

I believe it is better to have a short one line description is good to have
For someone who is new to asymmetric cryptography.

You can remove "* Refer to rte_crypto_asym_op_type "

>  	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 */
>  	RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
> @@ -126,11 +137,12 @@ enum rte_crypto_asym_op_type {
>   * Padding types for RSA signature.
>   */
>  enum rte_crypto_rsa_padding_type {
> -	RTE_CRYPTO_RSA_PADDING_NONE = 0,
> +	RTE_CRYPTO_RSA_PADDING_NONE,
>  	/**< RSA no padding scheme */
>  	RTE_CRYPTO_RSA_PADDING_PKCS1_5,
> -	/**< RSA PKCS#1 PKCS1-v1_5 padding scheme. For signatures block type
> 01,
> -	 * for encryption block type 02 are used.
> +	/**< RSA PKCS#1 PKCS1-v1_5 padding scheme.
> +	 * For signatures block type 01, for encryption
> +	 * block type 02 are used.
>  	 */
>  	RTE_CRYPTO_RSA_PADDING_OAEP,
>  	/**< RSA PKCS#1 OAEP padding scheme */
> @@ -156,40 +168,6 @@ enum rte_crypto_rsa_priv_key_type {
>  };
> 
>  /**
> - * Buffer to hold crypto params required for asym operations.
> - *
> - * These buffers can be used for both input to PMD and output from PMD.
> When
> - * used for output from PMD, application has to ensure the buffer is large
> - * enough to hold the target data.
> - *
> - * If an operation requires the PMD to generate a random number,
> - * and the device supports CSRNG, 'data' should be set to NULL.
> - * The crypto parameter in question will not be used by the PMD,
> - * as it is internally generated.
> - */
> -typedef struct rte_crypto_param_t {
> -	uint8_t *data;
> -	/**< pointer to buffer holding data */
> -	rte_iova_t iova;
> -	/**< IO address of data buffer */
> -	size_t length;
> -	/**< length of data in bytes */
> -} rte_crypto_param;
> -
> -/** Unsigned big-integer in big-endian format */
> -typedef rte_crypto_param rte_crypto_uint;
> -
> -/**
> - * Structure for elliptic curve point
> - */
> -struct rte_crypto_ec_point {
> -	rte_crypto_param x;
> -	/**< X coordinate */
> -	rte_crypto_param y;
> -	/**< Y coordinate */
> -};
> -
> -/**
>   * Structure describing RSA private key in quintuple format.
>   * See PKCS V1.5 RSA Cryptography Standard.
>   */
> 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