Re: [dpdk-dev] [EXT] [PATCH v5 2/7] cryptodev: add cipher field to RSA op

[Shally Verma <shallyv@marvell.com>]

> -----Original Message-----
> From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> Sent: Thursday, July 18, 2019 9:40 PM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com; Shally Verma
> <shallyv@marvell.com>; damianx.nowak@intel.com; Arek Kusztal
> <arkadiuszx.kusztal@intel.com>
> Subject: [EXT] [PATCH v5 2/7] cryptodev: add cipher field to RSA op
> 
> External Email
> 
> ----------------------------------------------------------------------
> Asymmetric nature of RSA algorithm suggest to use additional field for
> output. In place operations still can be done by setting cipher and message
> pointers with the same memory address.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto_asym.h | 43
> ++++++++++++++++++++++++++++------
>  1 file changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> b/lib/librte_cryptodev/rte_crypto_asym.h
> index 02ec304..1d4ec80 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -395,21 +395,50 @@ struct rte_crypto_rsa_op_param {
> 
>  	rte_crypto_param message;
>  	/**<
> -	 * Pointer to data
> +	 * Pointer to input data
>  	 * - to be encrypted for RSA public encrypt.
> -	 * - to be decrypted for RSA private decrypt.
>  	 * - to be signed for RSA sign generation.
>  	 * - to be authenticated for RSA sign verification.
> +	 *
> +	 * Pointer to output data
> +	 * - for RSA private decrypt.
> +	 * In this case the underlying array should have been
> +	 * allocated with enough memory to hold plaintext output
> +	 * (i.e. must be at least RSA key size). The message.length
> +	 * field should be 0 and will be overwritten by the PMD
> +	 * with the decrypted length.
> +	 *
> +	 * All data is in Octet-string network byte order format.
> +	 */
As per Fiona feedback in another email, for PMD it does not matter what output buffer length is set to. All matters if it should be allocated large enough as per description in spec.
Given that, there is no need to mention specifically, that length should be set to 0. App can leave it to anything as PMD don't care. It does not and should not check for any valid params here.
Ditto is my feedback on cipher.length description below. There is no need to mention, it should be set to 0 specifically

If we agree, this change can be taken as part of next patch set. Current one can still go on.

Thanks
Shally



> +
> +	rte_crypto_param cipher;
> +	/**<
> +	 * Pointer to input data
> +	 * - to be decrypted for RSA private decrypt.
> +	 *
> +	 * Pointer to output data
> +	 * - for RSA public encrypt.
> +	 * In this case the underlying array should have been allocated
> +	 * with enough memory to hold ciphertext output (i.e. must be
> +	 * at least RSA key size). The cipher.length field should
> +	 * be 0 and will be overwritten by the PMD with the encrypted
> length.
> +	 *
> +	 * All data is in Octet-string network byte order format.
>  	 */
> 
>  	rte_crypto_param sign;
>  	/**<
> -	 * Pointer to RSA signature data. If operation is RSA
> -	 * sign @ref RTE_CRYPTO_ASYM_OP_SIGN, buffer will be
> -	 * over-written with generated signature.
> +	 * Pointer to input data
> +	 * - to be verified for RSA public decrypt.
> +	 *
> +	 * Pointer to output data
> +	 * - for RSA private encrypt.
> +	 * In this case the underlying array should have been allocated
> +	 * with enough memory to hold signature output (i.e. must be
> +	 * at least RSA key size). The sign.length field should
> +	 * be 0 and will be overwritten by the PMD with the signature length.
>  	 *
> -	 * Length of the signature data will be equal to the
> -	 * RSA modulus length.
> +	 * All data is in Octet-string network byte order format.
>  	 */
> 
>  	enum rte_crypto_rsa_padding_type pad;
> --
> 2.1.0