Re: [PATCH v7 05/13] crypto: ecc - Add nbits field to ecc_curve structure

From: Stefan Berger <stefanb@linux.ibm.com>
To: Jarkko Sakkinen <jarkko@kernel.org>,
	keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
	herbert@gondor.apana.org.au, davem@davemloft.net
Cc: linux-kernel@vger.kernel.org, saulo.alessandre@tse.jus.br,
	lukas@wunner.de, bbhushan2@marvell.com
Subject: Re: [PATCH v7 05/13] crypto: ecc - Add nbits field to ecc_curve structure
Date: Thu, 21 Mar 2024 13:42:02 -0400	[thread overview]
Message-ID: <58166047-b339-4234-832b-a1aad6bf49de@linux.ibm.com> (raw)
In-Reply-To: <CZZLKGMM0B9E.7J1CGE8EIGQX@kernel.org>

On 3/21/24 13:17, Jarkko Sakkinen wrote:
> On Wed Mar 20, 2024 at 1:47 PM EET, Stefan Berger wrote:
>> Add the number of bits a curve has to the ecc_curve definition to be able
>> to derive the number of bytes a curve requires for its coordinates from it.
>> It also allows one to identify a curve by its particular size. Set the
>> number of bits on all curve definitions.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> Tested-by: Lukas Wunner <lukas@wunner.de>
>> ---

>>   		.y = tc512b_g_y,
>> diff --git a/include/crypto/ecc_curve.h b/include/crypto/ecc_curve.h
>> index 70964781eb68..63d5754e7614 100644
>> --- a/include/crypto/ecc_curve.h
>> +++ b/include/crypto/ecc_curve.h
>> @@ -23,6 +23,7 @@ struct ecc_point {
>>    * struct ecc_curve - definition of elliptic curve
>>    *
>>    * @name:	Short name of the curve.
>> + * @nbits:	The number of bits of a curve.
>>    * @g:		Generator point of the curve.
>>    * @p:		Prime number, if Barrett's reduction is used for this curve
>>    *		pre-calculated value 'mu' is appended to the @p after ndigits.
>> @@ -34,6 +35,7 @@ struct ecc_point {
>>    */
>>   struct ecc_curve {
>>   	char *name;
>> +	unsigned int nbits;
> 
> Nit:
> 
> Hmm not strongly opionated here but wouldn't it be more consistent to
> use u32 here as the types below are also exact bitsize types?

I will change this for v8. I will probably delay v8 until this patch 
here has been queued for upstreaming because 11/13 will need a similar 
module alias.

https://lore.kernel.org/linux-crypto/20240321144433.1671394-1-stefanb@linux.ibm.com/T/#u

> 
>>   	struct ecc_point g;
>>   	u64 *p;
>>   	u64 *n;
> 
> BR, Jarkko
> 