RE: [EXTERNAL] [PATCH v5 2/4] cryptodev: add ec points to sm2 op

[Akhil Goyal <gakhil@marvell.com>]

> 
> 
> > -----Original Message-----
> > From: Akhil Goyal <gakhil@marvell.com>
> > Sent: Friday, October 11, 2024 2:18 PM
> > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> > Cc: Dooley, Brian <brian.dooley@intel.com>
> > Subject: RE: [EXTERNAL] [PATCH v5 2/4] cryptodev: add ec points to sm2 op
> >
> > > In the case when PMD cannot support the full process of the SM2, but
> > > elliptic curve computation only, additional fields are needed to
> > > handle such a case.
> > >
> > > Points C1, kP therefore were added to the SM2 crypto operation struct.
> > >
> > > Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> > > ---
> > >  lib/cryptodev/rte_crypto_asym.h | 53
> > > ++++++++++++++++++++++++++++++------
> > > -----
> > >  1 file changed, 39 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > > b/lib/cryptodev/rte_crypto_asym.h index 2af6a307f6..65b1a081b1 100644
> > > --- a/lib/cryptodev/rte_crypto_asym.h
> > > +++ b/lib/cryptodev/rte_crypto_asym.h
> > > @@ -607,6 +607,8 @@ enum rte_crypto_sm2_op_capa {
> > >  	/**< Random number generator supported in SM2 ops. */
> > >  	RTE_CRYPTO_SM2_PH,
> > >  	/**< Prehash message before crypto op. */
> > > +	RTE_CRYPTO_SM2_PARTIAL,
> > > +	/**< Calculate elliptic curve points only. */
> > >  };
> > >
> > >  /**
> > > @@ -634,20 +636,43 @@ struct rte_crypto_sm2_op_param {
> > >  	 * will be overwritten by the PMD with the decrypted length.
> > >  	 */
> > >
> > > -	rte_crypto_param cipher;
> > > -	/**<
> > > -	 * Pointer to input data
> > > -	 * - to be decrypted for SM2 private decrypt.
> > > -	 *
> > > -	 * Pointer to output data
> > > -	 * - for SM2 public encrypt.
> > > -	 * In this case the underlying array should have been allocated
> > > -	 * with enough memory to hold ciphertext output (at least X bytes
> > > -	 * for prime field curve of N bytes and for message M bytes,
> > > -	 * where X = (C1 || C2 || C3) and computed based on SM2 RFC as
> > > -	 * C1 (1 + N + N), C2 = M, C3 = N. The cipher.length field will
> > > -	 * be overwritten by the PMD with the encrypted length.
> > > -	 */
> > > +	union {
> > > +		rte_crypto_param cipher;
> > > +		/**<
> > > +		 * Pointer to input data
> > > +		 * - to be decrypted for SM2 private decrypt.
> > > +		 *
> > > +		 * Pointer to output data
> > > +		 * - for SM2 public encrypt.
> > > +		 * In this case the underlying array should have been allocated
> > > +		 * with enough memory to hold ciphertext output (at least X
> > > bytes
> > > +		 * for prime field curve of N bytes and for message M bytes,
> > > +		 * where X = (C1 || C2 || C3) and computed based on SM2 RFC
> > > as
> > > +		 * C1 (1 + N + N), C2 = M, C3 = N. The cipher.length field will
> > > +		 * be overwritten by the PMD with the encrypted length.
> > > +		 */
> > > +		struct {
> > > +			struct rte_crypto_ec_point C1;
> > > +			/**<
> > > +			 * This field is used only when PMD does not support
> > the
> > > full
> > > +			 * process of the SM2 encryption/decryption, but the
> > > elliptic
> > > +			 * curve part only.
> > > +			 *
> > > +			 * In the case of encryption, it is an output - point C1 =
> > > (x1,y1).
> > > +			 * In the case of decryption, if is an input - point C1 =
> > > (x1,y1)
> > > +			 *
> > > +			 */
> > > +			struct rte_crypto_ec_point kP;
> > > +			/**<
> > > +			 * This field is used only when PMD does not support
> > the
> > > full
> > > +			 * process of the SM2 encryption/decryption, but the
> > > elliptic
> > > +			 * curve part only.
> > > +			 *
> > > +			 * It is an output in the encryption case, it is a point
> > > +			 * [k]P = (x2,y2)
> > > +			 */
> >
> > It is better to keep the variable names in lower case.
> > c1 and kp should be fine.
> 
> The reason for keeping some of the letters in uppercase is that it corresponds to
> the general convention of naming for these types.
> That's why we have dQ, qInv in RSA key for example, not dq, qinv.

Those were missed in the review. We should not add new ones.