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

["Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>]

Hi Shally, Fiona,

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Thursday, December 20, 2018 11:53 AM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; Kanaka
> Durga Kotamarthy <kkotamarthy@marvell.com>; Sunila Sahu
> <ssahu@marvell.com>; Kotamarthy, Kanaka
> <Kanaka.Kotamarthy@cavium.com>; Sahu, Sunila
> <Sunila.Sahu@cavium.com>; Cel, TomaszX <tomaszx.cel@intel.com>;
> Jozwiak, TomaszX <tomaszx.jozwiak@intel.com>
> Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and
> modinv API
> 
> 
> 
> >-----Original Message-----
> >From: Trahe, Fiona <fiona.trahe@intel.com>
> >Sent: 18 December 2018 21:23
> >To: Verma, Shally <Shally.Verma@cavium.com>; Kusztal, ArkadiuszX
> ><arkadiuszx.kusztal@intel.com>
> >Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; Kanaka
> >Durga Kotamarthy <kkotamarthy@marvell.com>; Sunila Sahu
> ><ssahu@marvell.com>; Kotamarthy, Kanaka
> <Kanaka.Kotamarthy@cavium.com>;
> >Sahu, Sunila <Sunila.Sahu@cavium.com>; Cel, TomaszX
> ><tomaszx.cel@intel.com>; Jozwiak, TomaszX
> <tomaszx.jozwiak@intel.com>;
> >Trahe, Fiona <fiona.trahe@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?

Yes, I will send patches.

Thanks,
Arek

> 
> 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
> >> >>
> >> >>