[dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[Verma, Shally]

>-----Original Message-----
>From: Trahe, Fiona <fiona.trahe at intel.com>
>Sent: 18 December 2018 21:23
>To: Verma, Shally <Shally.Verma at cavium.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal at intel.com>
>Cc: dev at dpdk.org; Doherty, Declan <declan.doherty at intel.com>; Kanaka Durga Kotamarthy <kkotamarthy at marvell.com>; Sunila
>Sahu <ssahu at marvell.com>; Kotamarthy, Kanaka <Kanaka.Kotamarthy at cavium.com>; Sahu, Sunila <Sunila.Sahu at cavium.com>; Cel,
>TomaszX <tomaszx.cel at intel.com>; Jozwiak, TomaszX <tomaszx.jozwiak at intel.com>; Trahe, Fiona <fiona.trahe at intel.com>
>Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and modinv API
>
>External Email
>
>Hi Shally, Arek,
>
...

>> >>
>> >>               rte_crypto_asym.h:323
>> >>                              struct rte_crypto_mod_op_param {
>> >>                              [AK] - There should be a result field. It size should be equal to
>> >> the size
>> >>                              of modulus. Same apply to mod mult inverse. It should be
>> >> driver responsability to check if result
>> >>                              will not overflow [Shally] so these are in-place operation.
>> >> Output will be written back to base param. It also imply length of allocated
>> >> array should be >= modulus length which is passed in session param.
>> >[AK-v2] I would abandon in-place/oop approach at all in asymmetric. For symmetric reason for in-
>> place is that very often structure of
>> >packet is almost intact (macs, ip addresses, ttl etc are changed but structure remains the same, it
>> may differ for IPSec ESP mode etc).
>> >For asymmetric it is mainly used for handshakes (for example in TLS RSA use case client will send
>> 48byte of pre master secret which
>> >server will use to generate master secret after decryption). I generally don't think asymmetric crypto
>> can be used as symmetric
>> >especially that only RSA would be (to some extent) capable of it.
>>
>> [Shally] So you suggest all asym ops should be out of place? Am okay with add that. However would
>> like to ask if anyone has preference to keep in-place option in Asym.
>> If so, then we would need to add Feature flag indicating in-place processing capability.
>[Fiona] I'm ok with dropping all in-place for asymm. As there are multiple inputs and output for various
>algos it would be quite difficult to craft set of capabilities to reflect which field(s) was used for in-place result.
>I don't see a need for in-place, would use out-of-place for all.
>
[Shally] Am okay with that too. So, Arek will you send patches with these changes?

Thanks
Shally

>> >>                              [AK] - Any particular reason modulus and exponent is in
>> >> session? Not saying
>> >>                              it is wrong but is it for DH, RSA use cases only?
>> >> [Shally] no that's not the intent. For RSA and DH respective xforms have been
>> >> defined. It is kept in session envisioning modulus and exponent wont change
>> >> frequently across operation but only base value.
>> >> So once context is loaded with modulus and exponent , app can call modexp
>> >> on different base values.
>> >>
>> >>                              rte_crypto.h:39
>> >>                              enum rte_crypto_op_status {
>> >>                              [AK] - There will be many more status options in asymmetric,
>> >>                              could we probably create new one for asymmetric crypto?
>> >> Even if asymmetric and symmetric
>> >>                              overlap?
>> >>                              For mod exp, mod inv potentially it will be:
>> >>                             DIVIDING_BY_ZERO_ERROR, INVERSE_NOT_EXISTS_ERROR...
>> >> [Shally] So far RTE_CRYPTO_OP_STATUS_INVALID_PARAM has been
>> >> sufficient for such cases. Do you have any use-cases where you need specific
>> >> error code to indicate asym specific error codes?
>> >There would be many examples, one of which:
>> >[AK-v2] Ok, still to discussion i think though.
>> >>
>> >>               rte_crypto_asym.h:33
>> >>                              size_t length;
>> >>                              /**< length of data in bytes */
>> >>                              [AK] - Is it guaranteed to be length of actual data, not
>> >> allocated memory (i mean no leading 0ed bytes), so the most significant bit
>> >> will be in data[0]?
>> >> [Shally] it should be length of actual data not length of allocated memory to
>> >> an array.
>> >> However it might create bit confusion on modular exponentiation op param
>> >> as that expect length passed should tell actual data length in base array but
>> >> array itself should be allocated upto modulus length.
>> >[AK-v2] - so it is maybe good idea to have allocated data in bytes and actual len in bits here.
>>
>> [Shally] No that will make it complex and breaks compatibility too. I would propose to keep it in bytes
>> which states length of actual data present in array.
>> Any confusion around it will be resolved if we add out of place or proper documentation if in-place is
>> retained.
>>
>> I would suggest you to send a patch with things that we agree or you propose. We can discuss on that
>> further.
>[Fiona] yes, agreed, Arek will send a patch.
>
>> Thanks
>> Shally
>> >>
>> >>                              [AK] - could it be uint16/32_t instead as size_t can have
>> >> different sizes in different implementations, uint16_t should be enough
>> >>                              for all algorithms big integer sizes [Shally] no hard choices
>> >> here though. But size_t would never be less than uint16_t so it guarantee to
>> >> be large enough for any machines
>> >>
>> >>               rte_crypto_asym.h:74, 250, 257, 351
>> >>                              /**< Modular Inverse
>> >>                              [AK] - Modular Multiplicative Inverse
>> >>     [Shally] Ack.
>> >>
>> >> Thanks,
>> >> Arek
>> >>
>> >>