From: "Verma, Shally" <Shally.Verma@cavium.com>
To: "Doherty, Declan" <declan.doherty@intel.com>,
"pablo.de.lara.guarch@intel.com" <pablo.de.lara.guarch@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Athreya, Narayana Prasad" <NarayanaPrasad.Athreya@cavium.com>,
"Murthy, Nidadavolu" <Nidadavolu.Murthy@cavium.com>,
"Sahu, Sunila" <Sunila.Sahu@cavium.com>,
"Gupta, Ashish" <Ashish.Gupta@cavium.com>,
"Kartha, Umesh" <Umesh.Kartha@cavium.com>
Subject: Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
Date: Mon, 9 Jul 2018 05:45:23 +0000 [thread overview]
Message-ID: <CY4PR0701MB3634970C93B23184EDF9F0C5F0440@CY4PR0701MB3634.namprd07.prod.outlook.com> (raw)
In-Reply-To: <CY4PR0701MB36346190238DA7ED707F255DF0470@CY4PR0701MB3634.namprd07.prod.outlook.com>
HI Declan, Pablo
Could we please close on this asap?
Thanks
Shally
>-----Original Message-----
>From: Verma, Shally
>Sent: 06 July 2018 19:59
>To: 'Doherty, Declan' <declan.doherty@intel.com>; pablo.de.lara.guarch@intel.com
>Cc: dev@dpdk.org; Athreya, Narayana Prasad <NarayanaPrasad.Athreya@cavium.com>; Murthy, Nidadavolu
><Nidadavolu.Murthy@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>; Kartha,
>Umesh <Umesh.Kartha@cavium.com>
>Subject: RE: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
>
>Hi Declan
>
>>-----Original Message-----
>>From: Doherty, Declan [mailto:declan.doherty@intel.com]
>>Sent: 05 July 2018 20:24
>>To: Verma, Shally <Shally.Verma@cavium.com>; pablo.de.lara.guarch@intel.com
>>Cc: dev@dpdk.org; Athreya, Narayana Prasad <NarayanaPrasad.Athreya@cavium.com>; Murthy, Nidadavolu
>><Nidadavolu.Murthy@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>; Kartha,
>>Umesh <Umesh.Kartha@cavium.com>
>>Subject: Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
>>
>>External Email
>>
>>Hey Shally,
>>
>>just a few things inline below mainly concerned with the need to be able
>>to support session-less operations in future PMDs. I think with a few
>>minor changes to the API now it should allow session-less to be
>>supported later without the need for a major rework of the APIs, I don't
>>think this should cause any major rework to your PMD just the adoption
>>of some new more explicit op types.
>>
>>Thanks
>>Declan
>>
>>On 03/07/2018 4:24 PM, Shally Verma wrote:
>>> Add rte_crypto_asym.h with supported xfrms
>>> and associated op structures and APIs
>>>
>>> API currently supports:
>>> - RSA Encrypt, Decrypt, Sign and Verify
>>> - Modular Exponentiation and Inversion
>>> - DSA Sign and Verify
>>> - Deffie-hellman private key exchange
>>> - Deffie-hellman public key exchange
>>> - Deffie-hellman shared secret compute
>>> - Deffie-hellman public/private key pair generation
>>> using xform chain
>>>
>>> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
>>> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
>>> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
>>> Signed-off-by: Umesh Kartha <umesh.kartha@caviumnetworks.com>
>>> ---
>>> lib/librte_cryptodev/Makefile | 1 +
>>> lib/librte_cryptodev/meson.build | 3 +-
>>> lib/librte_cryptodev/rte_crypto_asym.h | 496 +++++++++++++++++++++++++++++++++
>>> 3 files changed, 499 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/librte_cryptodev/Makefile b/lib/librte_cr
>>
>>...
>>
>>> +typedef struct rte_crypto_param_t {
>>> + uint8_t *data;
>>> + /**< pointer to buffer holding data */
>>> + rte_iova_t iova;
>>> + /**< IO address of data buffer */
>>> + size_t length;
>>> + /**< length of data in bytes */
>>> +} rte_crypto_param;
>>
>>What is the intended way for this memory to be allocated,
>
>[Shally] It should be pointer to flat buffers and added only to input/output data to/from
>asymmetric crypto engine.
>
>> it seems like
>>there might be a more general requirement in DPDK for IO addressable
>>memory (compression? other hardware acceleators implemented on FPGAs)
>>than just asymmetric crypto, will we end up needing to support features
>>like scatter gather lists in this structure?
>
>[Shally] I don’t anticipate that we would need to support scatter-gather data buffers as far as it is used for asymmetric.
>And I'm not aware if we have requirement to support it for asymmetric processing since data size is usually small for
>such operations. Thus, app is expected to send linear buffers for input/output.
>
>Does that answer your question? Or did I miss anything?
>
>
>>btw I think this is
>>probably fine for the moment as it will be expermential but I think it
>>will need to be addressed before the removal of the expermential tag.
>>
>
>...
>
>>> + RTE_CRYPTO_ASYM_XFORM_MODINV,
>>
>>Would prefer if this was _MOD_INV :)
>>
>>> + /**< Modular Inverse
>>> + * Perform Modulus inverse b^(-1) mod n
>>> + */
>>> + RTE_CRYPTO_ASYM_XFORM_MODEX,
>>
>>any this was _MOD_EX :)
>
>[Shally] fine will do name change.
>
>>
>>> + /**< Modular Exponentiation
>>> + * Perform Modular Exponentiation b^e mod n
>>> + */
>>> + RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
>>> + /**< End of list */
>>> +};
>>> +
>>> +/**
>>> + * Asymmetric crypto operation type variants
>>> + */
>>> +enum rte_crypto_asym_op_type {
>>> + RTE_CRYPTO_ASYM_OP_ENCRYPT,
>>> + /**< Asymmetric Encrypt operation */
>>> + RTE_CRYPTO_ASYM_OP_DECRYPT,
>>> + /**< Asymmetric Decrypt operation */
>>> + RTE_CRYPTO_ASYM_OP_SIGN,
>>> + /**< Signature Generation operation */
>>> + RTE_CRYPTO_ASYM_OP_VERIFY,
>>> + /**< Signature Verification operation */
>>> + RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE,
>>> + /**< DH Private Key generation operation */
>>> + RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE,
>>> + /**< DH Public Key generation operation */
>>> + RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE,
>>> + /**< DH Shared Secret compute operation */
>>> + RTE_CRYPTO_ASYM_OP_LIST_END
>>> +};
>>> +
>>
>>I think that having generic operation types which may or may not apply
>>to all of the defined xforms is confusing from a user perspective and in
>>the longer term will make it impossible to support session-less
>>asymmetric operations. If we instead do something like
>>
>> RTE_CRYPTO_ASYM_OP_RSA_ENCRYPT,
>> RTE_CRYPTO_ASYM_OP_RSA_DECRYPT,
>> RTE_CRYPTO_ASYM_OP_RSA_SIGN,
>> RTE_CRYPTO_ASYM_OP_RSA_VERIFY,
>> RTE_CRYPTO_ASYM_OP_DH_KEY_GENERATE,
>> RTE_CRYPTO_ASYM_OP_DH_SHARED_SECRET_COMPUTE,
>> etc...
>>
>>Then the op type becomes very explicit and will allow session-less
>>operations to be supported by PMDs. This shouldn't have any impact on
>>your current implementation other than updating the op type.
>>
>
>[Shally] Ok, so you suggest to merge xform and op_type (including keeping op_type in one place) for simplicity sake.
>
>If we take this change, then I will prefer to keep it as xforms rather than op_types, such as,
>
>enum rte_crypto_asym_xform_types {
>RTE_CRYPTO_ASYM_XFORM_RSA_ENCRYPT,
>RTE_CRYPTO_ASYM_XFORM_RSA_SIGN,
>RTE_CRYPTO_ASYM_XFORM_DSA_SIGN,
>RTE_CRYPTO_ASYM_XFORM_DSA_VERIFY,
>RTE_CRYPTO_ASYM_XFORM_DH_PRIV_KEY_GENERATE,
>RTE_CRYPTO_ASYM_XFORM_DH_PUB_KEY_GENERATE,
>RTE_CRYPTO_ASYM_XFORM_DH_SHARED_SECRET_COMPUTE, .... and so on..
>}
>
>These, when set on a rte_crypto_asym_xform , will work seamlessly for both session and session-less.
>I also see advantage here to support xform chaining. In this case, op_type in rte_crypto_asym_xx_op_params isn't needed and will be
>removed.
>It will add bit of repetition though, like RSA_SIGN / DSA_SIGN, but that would occur in any case, if we are merging two types.
>Does that sound okay?
>
>We were about to submit Openssl PMD with asym support today. But I would hold back till we align on this.
>...
>
>>....
>>
>>
>>> + */
>>> +struct rte_crypto_dh_xform {
>>> + enum rte_crypto_asym_op_type type;
>>> + /**< Setup xform for key generate or shared secret compute */
>>> +
>>
>>there is an inconsistency here in terms of were the op_type is defined,
>>in this case it is in the xform but it other cases RSA, DSA it is
>>defined in the operation information itself. I don't know of any reason
>>why it is needed in the xform but I think it must be consistent across
>>all operations/xforms. Ideally from my perspective it would be in the
>>rte_crypto_asym_op structure, see below, as this would allow
>>session/session-less operations to be supported seamlessly.
>>
>
>[Shally] Reason was xform chaining. As per Fiona feedback, we identified requirement to break DH Key pair
>generation in to two : PRIV_KEY_GENERATION and PUB_KEY_GENERATION, so I had to move this param around.
>But this will be addressed once we change xform_types as per suggestion above.
>
>...
>
>>
>>...
>>
>>> +/**
>>> + * Asymmetric Cryptographic Operation.
>>> + *
>>> + * Structure describing asymmetric crypto operation params.
>>> + *
>>> + */
>>> +struct rte_crypto_asym_op {
>>> + struct rte_cryptodev_asym_session *session;
>>> + /**< Handle for the initialised session context */
>>> +
>>> + __extension__
>>> + union {
>>> + struct rte_crypto_rsa_op_param rsa;
>>> + struct rte_crypto_mod_op_param modex;
>>> + struct rte_crypto_mod_op_param modinv;
>>> + struct rte_crypto_dh_op_param dh;
>>> + struct rte_crypto_dsa_op_param dsa;
>>> + };
>>> +} __rte_cache_aligned;
>>> +
>>Relating to my comment on position of the op_type and the minor change
>>of having an union of session/xform in the rte_crypto_asym_op structure
>>would then enable sessionless support to be added seamless in the future
>>with minimal effect to the current proposal.
>
>[Shally] Again, this will also be resolved with change to xform_types
>
>>
>>struct rte_crypto_asym_op {
>>- struct rte_cryptodev_asym_session *session;
>>- /**< Handle for the initialised session context */
>>+ enum rte_crypto_asym_op_type op_type;
>>+
>>+ union {
>>+ struct rte_cryptodev_asym_session *session;
>>+ /**< Handle for the initialised session context */
>>+ struct rte_crypto_asym_xform *xform;
>>+ };
>>
>[Shally] Ok. Will add this change. But then I'll have to check if crypto PMD has support to reflect which type of mode it supports?
>
>Thanks for review.
>Shally
>
>> __extension__
>>
>>
>>> +#ifdef __cplusplus
>>> +}
>>> +#endif
>>> +
>>> +#endif /* _RTE_CRYPTO_ASYM_H_ */
>>>
next prev parent reply other threads:[~2018-07-09 5:50 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-03 15:24 [dpdk-dev] [PATCH v4 0/4] crypto: add asym crypto support Shally Verma
2018-07-03 15:24 ` [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev Shally Verma
2018-07-05 14:54 ` Doherty, Declan
2018-07-06 14:28 ` Verma, Shally
2018-07-09 5:45 ` Verma, Shally [this message]
2018-07-09 14:54 ` Doherty, Declan
2018-07-09 16:44 ` Trahe, Fiona
2018-07-09 17:12 ` Verma, Shally
2018-07-09 17:16 ` De Lara Guarch, Pablo
2018-07-09 17:42 ` Verma, Shally
2018-07-12 18:16 ` Verma, Shally
2018-07-09 22:23 ` De Lara Guarch, Pablo
2018-07-10 5:22 ` Verma, Shally
2018-07-10 8:08 ` De Lara Guarch, Pablo
2018-07-09 22:30 ` De Lara Guarch, Pablo
2018-07-03 15:24 ` [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations Shally Verma
2018-07-06 13:41 ` Trahe, Fiona
2018-07-06 13:48 ` Verma, Shally
2018-07-06 14:13 ` Trahe, Fiona
2018-07-03 15:24 ` [dpdk-dev] [PATCH v4 3/4] lib/cryptodev: add asymmetric crypto capability in cryptodev Shally Verma
2018-07-09 22:34 ` De Lara Guarch, Pablo
2018-07-03 15:24 ` [dpdk-dev] [PATCH v4 4/4] doc: add asym crypto in cryptodev programmer guide Shally Verma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CY4PR0701MB3634970C93B23184EDF9F0C5F0440@CY4PR0701MB3634.namprd07.prod.outlook.com \
--to=shally.verma@cavium.com \
--cc=Ashish.Gupta@cavium.com \
--cc=NarayanaPrasad.Athreya@cavium.com \
--cc=Nidadavolu.Murthy@cavium.com \
--cc=Sunila.Sahu@cavium.com \
--cc=Umesh.Kartha@cavium.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=pablo.de.lara.guarch@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).