Re: [dpdk-dev] [PATCH v2] cryptodev: rework mod exp and mod inv comments

[Shally Verma <shallyv@marvell.com>]

>-----Original Message-----
>From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
>Sent: 06 February 2019 15:04
>To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org
>Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>; sunila.sahu@caviumnetworks.com;
>ashish.gupta@caviumnetworks.com; umesh.kartha@caviumnetworks.com
>Subject: [EXT] RE: [PATCH v2] cryptodev: rework mod exp and mod inv comments
>
>External Email
>
>----------------------------------------------------------------------
>Hi Shally,
>
>Thanks for your feedback.
>
>> -----Original Message-----
>> From: Shally Verma [mailto:shallyv@marvell.com]
>> Sent: Wednesday, February 6, 2019 6:08 AM
>> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
>> Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
>> sunila.sahu@caviumnetworks.com; ashish.gupta@caviumnetworks.com;
>> umesh.kartha@caviumnetworks.com
>> Subject: RE: [PATCH v2] cryptodev: rework mod exp and mod inv comments
>>
>> HI Arek,
>>
>> Acked with minor feedback.
>>
>> >-----Original Message-----
>> >From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
>> >Sent: 05 February 2019 23:50
>> >To: dev@dpdk.org
>> >Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com;
>> >shally.verma@caviumnetworks.com; sunila.sahu@caviumnetworks.com;
>> >ashish.gupta@caviumnetworks.com; umesh.kartha@caviumnetworks.com;
>> Arek
>> >Kusztal <arkadiuszx.kusztal@intel.com>
>> >Subject: [PATCH v2] cryptodev: rework mod exp and mod inv comments
>> >
>> >External Email
>> >
>> >This patch changes modular exponentiation and modular multiplicative
>> >inverse API comments to make it more precise.
>> >
>> >Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
>> >---
>> >v2:
>> >- grammar fixes
>> >
>> > lib/librte_cryptodev/rte_crypto_asym.h | 44
>> > ++++++++++++++++++++++------------
>> > 1 file changed, 29 insertions(+), 15 deletions(-)
>> >
>> Acked-by: Shally Verma <shally.verma@marvell.com>
>>
>> >diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
>> >b/lib/librte_cryptodev/rte_crypto_asym.h
>> >index 5e185b2..9582ee3 100644
>> >--- a/lib/librte_cryptodev/rte_crypto_asym.h
>> >+++ b/lib/librte_cryptodev/rte_crypto_asym.h
>> >@@ -72,8 +72,8 @@ enum rte_crypto_asym_xform_type {
>> >         * Refer to rte_crypto_asym_op_type
>> >         */
>> >        RTE_CRYPTO_ASYM_XFORM_MODINV,
>> >-       /**< Modular Inverse
>> >-        * Perform Modulus inverse b^(-1) mod n
>> >+       /**< Modular Multiplicative Inverse
>> >+        * Perform Modular Multiplicative Inverse b^(-1) mod n
>> >         */
>> >        RTE_CRYPTO_ASYM_XFORM_MODEX,
>> >        /**< Modular Exponentiation
>> >@@ -233,29 +233,39 @@ struct rte_crypto_rsa_xform {  struct
>> >rte_crypto_modex_xform {
>> >        rte_crypto_param modulus;
>> >        /**< modulus
>> >-        * Prime modulus of the modexp transform operation in octet-string
>> >-        * network byte order format.
>> >+        * Pointer to the modulus data for modexp transform operation
>> >+        * in octet-string network byte order format, any positive integer
>> >+        *
>> This statement looks incomplete. You can write it "it should be positive
>> integer " but is it required to be mentioned? Coz input is array of uint8_t so
>> assumption is it will be positive.
>>
>Actually by this we meant that there is no constraint  (if should be prime, co-prime, semi-prime, multi-prime, less than... etc),
>But yes this is only informational and probably could even be omitted
So, will you be re-wording these comments for better clarity?

Thanks
Shally
>
>> ...
>> > struct rte_crypto_modinv_xform {
>> >        rte_crypto_param modulus;
>> >        /**<
>> >-        * Pointer to the prime modulus data for modular
>> >-        * inverse operation in octet-string network byte
>> >-        * order format.
>> >+        * Pointer to the modulus data for modular multiplicative inverse
>> >+        * operation in octet-string network byte order format,
>> >+        * positive integer
>> Same comment as above.
>>
>> >+        *
>> >+        * In case this number is equal to zero the driver shall set
>> >+        * the crypto op status field to RTE_CRYPTO_OP_STATUS_ERROR
>> >+        *
>> >+        * This number shall be relatively prime to base
>> >+        * in corresponding Modular Multiplicative Inverse
>> >+        * rte_crypto_mod_op_param
>> >         */
>> > };
>> >
>> >@@ -317,14 +327,18 @@ struct rte_crypto_dsa_xform {
>> >
>> ...
>>
>> >2.1.0