From: "Doherty, Declan" <declan.doherty@intel.com>
To: Shally Verma <shally.verma@caviumnetworks.com>,
pablo.de.lara.guarch@intel.com
Cc: dev@dpdk.org, pathreya@caviumnetworks.com,
nmurthy@caviumnetworks.com,
Sunila Sahu <sunila.sahu@caviumnetworks.com>,
Ashish Gupta <ashish.gupta@caviumnetworks.com>,
Umesh Kartha <umesh.kartha@caviumnetworks.com>
Subject: Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
Date: Thu, 5 Jul 2018 15:54:29 +0100 [thread overview]
Message-ID: <9905afc5-3571-d6c8-c2b0-7c9804f0dbd6@intel.com> (raw)
In-Reply-To: <1530631466-26427-2-git-send-email-shally.verma@caviumnetworks.com>
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, 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? 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.
> +
> +/** asym xform type name strings */
> +extern const char *
> +rte_crypto_asym_xform_strings[];
> +
> +/** asym operations type name strings */
> +extern const char *
> +rte_crypto_asym_op_strings[];
> +
> +/**
> + * Asymmetric crypto transformation types.
> + * Each xform type maps to one asymmetric algorithm
> + * performing specific operation
> + *
> + */
> +enum rte_crypto_asym_xform_type {
> + RTE_CRYPTO_ASYM_XFORM_UNSPECIFIED = 0,
> + /**< Invalid xform. */
> + RTE_CRYPTO_ASYM_XFORM_NONE,
> + /**< Xform type None.
> + * May be supported by PMD to support
> + * passthrough op for debugging purpose.
> + * if xform_type none , op_type is disregarded.
> + */
> + RTE_CRYPTO_ASYM_XFORM_RSA,
> + /**< RSA. Performs Encrypt, Decrypt, Sign and Verify.
> + * Refer to rte_crypto_asym_op_type
> + */
> + RTE_CRYPTO_ASYM_XFORM_DH,
> + /**< Deffie-Hellman.
> + * Performs Key Generate and Shared Secret Compute.
> + * Refer to rte_crypto_asym_op_type
> + */
> + RTE_CRYPTO_ASYM_XFORM_DSA,
> + /**< Digital Signature Algorithm
> + * Performs Signature Generation and Verification.
> + * Refer to rte_crypto_asym_op_type
> + */
> + 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 :)
> + /**< 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.
> +/**
....
> + */
> +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.
> + rte_crypto_param p;
...
> +struct rte_crypto_rsa_op_param {
> + enum rte_crypto_asym_op_type op_type;
> + /**< Type of RSA operation for transform */;
> +
see previous comment above
...
> +
> +/**
> + * DSA Operations params
> + *
> + */
> +struct rte_crypto_dsa_op_param {
> + enum rte_crypto_asym_op_type op_type;
> + /**< Signature Generation or Verification */
see previous comment 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.
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;
+ };
__extension__
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_CRYPTO_ASYM_H_ */
>
next prev parent reply other threads:[~2018-07-05 14:54 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 [this message]
2018-07-06 14:28 ` Verma, Shally
2018-07-09 5:45 ` Verma, Shally
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=9905afc5-3571-d6c8-c2b0-7c9804f0dbd6@intel.com \
--to=declan.doherty@intel.com \
--cc=ashish.gupta@caviumnetworks.com \
--cc=dev@dpdk.org \
--cc=nmurthy@caviumnetworks.com \
--cc=pablo.de.lara.guarch@intel.com \
--cc=pathreya@caviumnetworks.com \
--cc=shally.verma@caviumnetworks.com \
--cc=sunila.sahu@caviumnetworks.com \
--cc=umesh.kartha@caviumnetworks.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).