DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v4 0/4] crypto: add asym crypto support
@ 2018-07-03 15:24 Shally Verma
  2018-07-03 15:24 ` [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev Shally Verma
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: Shally Verma @ 2018-07-03 15:24 UTC (permalink / raw)
  To: pablo.de.lara.guarch; +Cc: dev, pathreya, nmurthy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3558 bytes --]

This patch series add support for asymmetric crypto in DPDK
librte_cryptodev framework along with documentation updates.

This patch series is divided in to following categories:
1. library patches with asymmetric API, xform and capability
   definitions
2. Programmer Guide updates with asymmetric description

openssl pmd and test app to be added as separate patch series
with 1.1.0 support.

changes in v4:
- add of asym specific session header size get API
- add asym function pointer NULL check for asym_session_configure/destroy,
  for the devices that doesn’t support asymmetric
- remove redundant asym_op_reset and asym_op_attach functions from lib
- fix ordering in version.map
- simplified check_modlen function
- simplified logic in op_pool_create
- corrections in doc
- renaming rte_cryptodev_asym_session_get_private_data to 
  rte_cryptodev_asym_session_get_app_private_data 
- contains only lib changes

changes in v3:
- correct rte_cryptodev_asym_session_create to pass void** to
  rte_mempool_get() and add support for private_data_size flag
- remove redundant xform_type from rte_cryptodev_asymmetric_capability
- added rte_cryptodev_asym_session_set/get_private_data for app to setup
  private data in a session as per latest dpdk-next-crypto spec
- rename few APIs to be consistent with other API names
- update test meson.build to include asym unit test file

changes in v2:
-addresses patch apply failure
raised on asym crypto v1 patch series:
https://dpdk.org/dev/patchwork/patch/36575/
https://dpdk.org/dev/patchwork/patch/36576/
https://dpdk.org/dev/patchwork/patch/36577/

And, unit test and PMD patch series:
https://dpdk.org/dev/patchwork/patch/36928/
https://dpdk.org/dev/patchwork/patch/36929/
https://dpdk.org/dev/patchwork/patch/36930/
-- resolve git apply patch error on patch id 36575
-- resolve git apply patch error on patch id 36929

Changes in v1:
- removal of dedicated sym and asym qp setup,
- remove asym qp count and attach/detach_session apis
- re-org xforms params for deffie-hellman to allow
  public key and optional private key generations
- move elliptic curve changes into another separate patch/patch series

TBD:
- add elliptic curve support
- rename of existing session_configure/clear APIs to
  sym_session_configure/clear/init APIs

It is based on review discussion on RFC v1 asym crypto patch
http://dpdk.org/patch/34308.

RFC v1 patch http://dpdk.org/patch/34308 is further a derivative of
earlier reviewed  RFC v2 patch series:
http://dpdk.org/dev/patchwork/patch/24245/
http://dpdk.org/dev/patchwork/patch/24246/
http://dpdk.org/dev/patchwork/patch/24247/

Shally Verma (3):
  lib/cryptodev: add asymmetric algos in cryptodev
  cryptodev: support asymmetric operations
  doc: add asym crypto in cryptodev programmer guide

Sunila Sahu (1):
  lib/cryptodev: add asymmetric crypto capability in cryptodev

 doc/guides/prog_guide/cryptodev_lib.rst        | 290 ++++++++++++++-
 lib/librte_cryptodev/Makefile                  |   1 +
 lib/librte_cryptodev/meson.build               |   3 +-
 lib/librte_cryptodev/rte_crypto.h              |  37 +-
 lib/librte_cryptodev/rte_crypto_asym.h         | 496 +++++++++++++++++++++++++
 lib/librte_cryptodev/rte_cryptodev.c           | 277 +++++++++++++-
 lib/librte_cryptodev/rte_cryptodev.h           | 227 ++++++++++-
 lib/librte_cryptodev/rte_cryptodev_pmd.h       |  58 ++-
 lib/librte_cryptodev/rte_cryptodev_version.map |  15 +-
 9 files changed, 1384 insertions(+), 20 deletions(-)
 create mode 100644 lib/librte_cryptodev/rte_crypto_asym.h

-- 
2.9.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  2018-07-03 15:24 [dpdk-dev] [PATCH v4 0/4] crypto: add asym crypto support Shally Verma
@ 2018-07-03 15:24 ` Shally Verma
  2018-07-05 14:54   ` Doherty, Declan
                     ` (2 more replies)
  2018-07-03 15:24 ` [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations Shally Verma
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 22+ messages in thread
From: Shally Verma @ 2018-07-03 15:24 UTC (permalink / raw)
  To: pablo.de.lara.guarch
  Cc: dev, pathreya, nmurthy, Sunila Sahu, Ashish Gupta, Umesh Kartha

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_cryptodev/Makefile
index bba8dee..c114888 100644
--- a/lib/librte_cryptodev/Makefile
+++ b/lib/librte_cryptodev/Makefile
@@ -23,6 +23,7 @@ SYMLINK-y-include += rte_crypto.h
 SYMLINK-y-include += rte_crypto_sym.h
 SYMLINK-y-include += rte_cryptodev.h
 SYMLINK-y-include += rte_cryptodev_pmd.h
+SYMLINK-y-include += rte_crypto_asym.h
 
 # versioning export map
 EXPORT_MAP := rte_cryptodev_version.map
diff --git a/lib/librte_cryptodev/meson.build b/lib/librte_cryptodev/meson.build
index bd5fed8..295f509 100644
--- a/lib/librte_cryptodev/meson.build
+++ b/lib/librte_cryptodev/meson.build
@@ -6,5 +6,6 @@ sources = files('rte_cryptodev.c', 'rte_cryptodev_pmd.c')
 headers = files('rte_cryptodev.h',
 	'rte_cryptodev_pmd.h',
 	'rte_crypto.h',
-	'rte_crypto_sym.h')
+	'rte_crypto_sym.h',
+	'rte_crypto_asym.h')
 deps += ['kvargs', 'mbuf']
diff --git a/lib/librte_cryptodev/rte_crypto_asym.h b/lib/librte_cryptodev/rte_crypto_asym.h
new file mode 100644
index 0000000..7f88b57
--- /dev/null
+++ b/lib/librte_cryptodev/rte_crypto_asym.h
@@ -0,0 +1,496 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Cavium Networks
+ */
+
+#ifndef _RTE_CRYPTO_ASYM_H_
+#define _RTE_CRYPTO_ASYM_H_
+
+/**
+ * @file rte_crypto_asym.h
+ *
+ * RTE Definitions for Asymmetric Cryptography
+ *
+ * Defines asymmetric algorithms and modes, as well as supported
+ * asymmetric crypto operations.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <string.h>
+#include <stdint.h>
+
+#include <rte_memory.h>
+#include <rte_mempool.h>
+#include <rte_common.h>
+
+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;
+
+/** 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,
+	/**< Modular Inverse
+	 * Perform Modulus inverse b^(-1) mod n
+	 */
+	RTE_CRYPTO_ASYM_XFORM_MODEX,
+	/**< 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
+};
+
+/**
+ * Padding types for RSA signature.
+ */
+enum rte_crypto_rsa_padding_type {
+	RTE_CRYPTO_RSA_PADDING_NONE = 0,
+	/**< RSA no padding scheme */
+	RTE_CRYPTO_RSA_PKCS1_V1_5_BT0,
+	/**< RSA PKCS#1 V1.5 Block Type 0 padding scheme
+	 * as descibed in rfc2313
+	 */
+	RTE_CRYPTO_RSA_PKCS1_V1_5_BT1,
+	/**< RSA PKCS#1 V1.5 Block Type 01 padding scheme
+	 * as descibed in rfc2313
+	 */
+	RTE_CRYPTO_RSA_PKCS1_V1_5_BT2,
+	/**< RSA PKCS#1 V1.5 Block Type 02 padding scheme
+	 * as descibed in rfc2313
+	 */
+	RTE_CRYPTO_RSA_PADDING_OAEP,
+	/**< RSA PKCS#1 OAEP padding scheme */
+	RTE_CRYPTO_RSA_PADDING_PSS,
+	/**< RSA PKCS#1 PSS padding scheme */
+	RTE_CRYPTO_RSA_PADDING_TYPE_LIST_END
+};
+
+/**
+ * RSA private key type enumeration
+ *
+ * enumerates private key format required to perform RSA crypto
+ * transform.
+ *
+ */
+enum rte_crypto_rsa_priv_key_type {
+	RTE_RSA_KEY_TYPE_EXP,
+	/**< RSA private key is an exponent */
+	RTE_RSA_KET_TYPE_QT,
+	/**< RSA private key is in quintuple format
+	 * See rte_crypto_rsa_priv_key_qt
+	 */
+};
+
+/**
+ * Structure describing RSA private key in quintuple format.
+ * See PKCS V1.5 RSA Cryptography Standard.
+ */
+struct rte_crypto_rsa_priv_key_qt {
+	rte_crypto_param p;
+	/**< p - Private key component P
+	 * Private key component of RSA parameter  required for CRT method
+	 * of private key operations in Octet-string network byte order
+	 * format.
+	 */
+
+	rte_crypto_param q;
+	/**< q - Private key component Q
+	 * Private key component of RSA parameter  required for CRT method
+	 * of private key operations in Octet-string network byte order
+	 * format.
+	 */
+
+	rte_crypto_param dP;
+	/**< dP - Private CRT component
+	 * Private CRT component of RSA parameter  required for CRT method
+	 * RSA private key operations in Octet-string network byte order
+	 * format.
+	 * dP = d mod ( p - 1 )
+	 */
+
+	rte_crypto_param dQ;
+	/**< dQ - Private CRT component
+	 * Private CRT component of RSA parameter  required for CRT method
+	 * RSA private key operations in Octet-string network byte order
+	 * format.
+	 * dQ = d mod ( q - 1 )
+	 */
+
+	rte_crypto_param qInv;
+	/**< qInv - Private CRT component
+	 * Private CRT component of RSA parameter  required for CRT method
+	 * RSA private key operations in Octet-string network byte order
+	 * format.
+	 * qInv = inv q mod p
+	 */
+};
+
+/**
+ * Asymmetric RSA transform data
+ *
+ * Structure describing RSA xform params
+ *
+ */
+struct rte_crypto_rsa_xform {
+	rte_crypto_param n;
+	/**< n - Prime modulus
+	 * Prime modulus data of RSA operation in Octet-string network
+	 * byte order format.
+	 */
+
+	rte_crypto_param e;
+	/**< e - Public key exponent
+	 * Public key exponent used for RSA public key operations in Octet-
+	 * string network byte order format.
+	 */
+
+	enum rte_crypto_rsa_priv_key_type key_type;
+
+	__extension__
+	union {
+			rte_crypto_param d;
+			/**< d - Private key exponent
+			 * Private key exponent used for RSA
+			 * private key operations in
+			 * Octet-string  network byte order format.
+			 */
+
+			struct rte_crypto_rsa_priv_key_qt qt;
+			/**< qt - Private key in quintuple format */
+	};
+};
+
+/**
+ * Asymmetric Modular exponentiation transform data
+ *
+ * Structure describing modular exponentation xform param
+ *
+ */
+struct rte_crypto_modex_xform {
+	rte_crypto_param modulus;
+	/**< modulus
+	 * Prime modulus of the modexp transform operation in octet-string
+	 * network byte order format.
+	 */
+
+	rte_crypto_param exponent;
+	/**< exponent
+	 * Private exponent of the modexp transform operation in
+	 * octet-string network byte order format.
+	 */
+};
+
+/**
+ * Asymmetric modular inverse transform operation
+ *
+ * Structure describing modulus inverse xform params
+ *
+ */
+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.
+	 */
+};
+
+/**
+ * Asymmetric DH transform data
+ *
+ * Structure describing deffie-hellman xform params
+ *
+ */
+struct rte_crypto_dh_xform {
+	enum rte_crypto_asym_op_type type;
+	/**< Setup xform for key generate or shared secret compute */
+
+	rte_crypto_param p;
+	/**< p : Prime modulus data
+	 * DH prime modulous data in octet-string network byte order format.
+	 *
+	 */
+
+	rte_crypto_param g;
+	/**< g : Generator
+	 * DH group generator data in octet-string network byte order
+	 * format.
+	 *
+	 */
+};
+
+/**
+ * Asymmetric Digital Signature transform operation
+ *
+ * Structure describing DSA xform params
+ *
+ */
+struct rte_crypto_dsa_xform {
+	rte_crypto_param p;
+	/**< p - Prime modulus
+	 * Prime modulus data for DSA operation in Octet-string network byte
+	 * order format.
+	 */
+	rte_crypto_param q;
+	/**< q : Order of the subgroup.
+	 * Order of the subgroup data in Octet-string network byte order
+	 * format.
+	 * (p-1) % q = 0
+	 */
+	rte_crypto_param g;
+	/**< g: Generator of the subgroup
+	 * Generator  data in Octet-string network byte order format.
+	 */
+	rte_crypto_param x;
+	/**< x: Private key of the signer in octet-string network
+	 * byte order format.
+	 * Used when app has pre-defined private key.
+	 * Valid only when xform chain is DSA ONLY.
+	 * if xform chain is DH private key generate + DSA, then DSA sign
+	 * compute will use internally generated key.
+	 */
+};
+
+/**
+ * Operations params for modular operations:
+ * exponentiation and invert
+ *
+ */
+struct rte_crypto_mod_op_param {
+	rte_crypto_param base;
+	/**<
+	 * Pointer to base of modular exponentiation/inversion data in
+	 * Octet-string network byte order format.
+	 */
+};
+
+/**
+ * Asymmetric crypto transform data
+ *
+ * Structure describing asym xforms.
+ */
+struct rte_crypto_asym_xform {
+	struct rte_crypto_asym_xform *next;
+	/**< Pointer to next xform to set up xform chain.*/
+	enum rte_crypto_asym_xform_type xform_type;
+	/**< Asymmetric crypto transform */
+
+	__extension__
+	union {
+		struct rte_crypto_rsa_xform rsa;
+		/**< RSA xform parameters */
+
+		struct rte_crypto_modex_xform modex;
+		/**< Modular Exponentiation xform parameters */
+
+		struct rte_crypto_modinv_xform modinv;
+		/**< Modulus Inverse xform parameters */
+
+		struct rte_crypto_dh_xform dh;
+		/**< DH xform parameters */
+
+		struct rte_crypto_dsa_xform dsa;
+		/**< DSA xform parameters */
+	};
+};
+
+struct rte_cryptodev_asym_session;
+
+/**
+ * RSA operation params
+ *
+ */
+struct rte_crypto_rsa_op_param {
+	enum rte_crypto_asym_op_type op_type;
+	/**< Type of RSA operation for transform */;
+
+	rte_crypto_param message;
+	/**<
+	 * Pointer to data
+	 * - to be encrypted for RSA public encrypt.
+	 * - to be decrypted for RSA private decrypt.
+	 * - to be signed for RSA sign generation.
+	 * - to be authenticated for RSA sign verification.
+	 */
+
+	rte_crypto_param sign;
+	/**<
+	 * Pointer to RSA signature data. If operation is RSA
+	 * sign @ref RTE_CRYPTO_RSA_OP_SIGN, buffer will be
+	 * over-written with generated signature.
+	 *
+	 * Length of the signature data will be equal to the
+	 * RSA prime modulus length.
+	 */
+
+	enum rte_crypto_rsa_padding_type pad;
+	/**< RSA padding scheme to be used for transform */
+
+	enum rte_crypto_auth_algorithm md;
+	/**< Hash algorithm to be used for data hash if padding
+	 * scheme is either OAEP or PSS. Valid hash algorithms
+	 * are:
+	 * MD5, SHA1, SHA224, SHA256, SHA384, SHA512
+	 */
+
+	enum rte_crypto_auth_algorithm mgf1md;
+	/**<
+	 * Hash algorithm to be used for mask generation if
+	 * padding scheme is either OAEP or PSS. If padding
+	 * scheme is unspecified data hash algorithm is used
+	 * for mask generation. Valid hash algorithms are:
+	 * MD5, SHA1, SHA224, SHA256, SHA384, SHA512
+	 */
+};
+
+/**
+ * Deffie-Hellman Operations params.
+ * @note:
+ */
+struct rte_crypto_dh_op_param {
+	rte_crypto_param pub_key;
+	/**<
+	 * Output generated public key when xform type is
+	 * DH PUB_KEY_GENERATION.
+	 * Input peer public key when xform type is DH
+	 * SHARED_SECRET_COMPUTATION
+	 * pub_key is in octet-string network byte order format.
+	 *
+	 */
+
+	rte_crypto_param priv_key;
+	/**<
+	 * Output generated private key if xform type is
+	 * DH PRIVATE_KEY_GENERATION
+	 * Input when xform type is DH SHARED_SECRET_COMPUTATION.
+	 * priv_key is in octet-string network byte order format.
+	 *
+	 */
+
+	rte_crypto_param shared_secret;
+	/**<
+	 * Output with calculated shared secret
+	 * when dh xform set up with op type = SHARED_SECRET_COMPUTATION.
+	 * shared_secret is an octet-string network byte order format.
+	 *
+	 */
+};
+
+/**
+ * DSA Operations params
+ *
+ */
+struct rte_crypto_dsa_op_param {
+	enum rte_crypto_asym_op_type op_type;
+	/**< Signature Generation or Verification */
+	rte_crypto_param message;
+	/**< input message to be signed or verified */
+	rte_crypto_param r;
+	/**< dsa sign component 'r' value
+	 *
+	 * output if op_type = sign generate,
+	 * input if op_type = sign verify
+	 */
+	rte_crypto_param s;
+	/**< dsa sign component 's' value
+	 *
+	 * output if op_type = sign generate,
+	 * input if op_type = sign verify
+	 */
+	rte_crypto_param y;
+	/**< y : Public key of the signer.
+	 * Public key data of the signer in Octet-string network byte order
+	 * format.
+	 * y = g^x mod p
+	 */
+};
+
+/**
+ * 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;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_CRYPTO_ASYM_H_ */
-- 
2.9.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations
  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-03 15:24 ` Shally Verma
  2018-07-06 13:41   ` 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-03 15:24 ` [dpdk-dev] [PATCH v4 4/4] doc: add asym crypto in cryptodev programmer guide Shally Verma
  3 siblings, 1 reply; 22+ messages in thread
From: Shally Verma @ 2018-07-03 15:24 UTC (permalink / raw)
  To: pablo.de.lara.guarch
  Cc: dev, pathreya, nmurthy, Umesh Kartha, Sunila Sahu, Ashish Gupta

From: Umesh Kartha <umesh.kartha@caviumnetworks.com>

Extend DPDK librte_cryptodev to:
- define asym op type in rte_crypto_op_type and associated
  op pool create/alloc APIs
- define asym session and associated session APIs

If PMD shows in its feature flag that it supports both sym and
asym then it must support those on all its qps.

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/rte_crypto.h              |  37 ++++-
 lib/librte_cryptodev/rte_cryptodev.c           | 201 ++++++++++++++++++++++++-
 lib/librte_cryptodev/rte_cryptodev.h           | 124 ++++++++++++++-
 lib/librte_cryptodev/rte_cryptodev_pmd.h       |  58 ++++++-
 lib/librte_cryptodev/rte_cryptodev_version.map |  10 +-
 5 files changed, 423 insertions(+), 7 deletions(-)

diff --git a/lib/librte_cryptodev/rte_crypto.h b/lib/librte_cryptodev/rte_crypto.h
index a16be65..fd5ef3a 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -23,6 +23,7 @@ extern "C" {
 #include <rte_common.h>
 
 #include "rte_crypto_sym.h"
+#include "rte_crypto_asym.h"
 
 /** Crypto operation types */
 enum rte_crypto_op_type {
@@ -30,6 +31,8 @@ enum rte_crypto_op_type {
 	/**< Undefined operation type */
 	RTE_CRYPTO_OP_TYPE_SYMMETRIC,
 	/**< Symmetric operation */
+	RTE_CRYPTO_OP_TYPE_ASYMMETRIC
+	/**< Asymmetric operation */
 };
 
 /** Status of crypto operation */
@@ -114,6 +117,10 @@ struct rte_crypto_op {
 	union {
 		struct rte_crypto_sym_op sym[0];
 		/**< Symmetric operation parameters */
+
+		struct rte_crypto_asym_op asym[0];
+		/**< Asymmetric operation parameters */
+
 	}; /**< operation specific parameters */
 };
 
@@ -134,6 +141,9 @@ __rte_crypto_op_reset(struct rte_crypto_op *op, enum rte_crypto_op_type type)
 	case RTE_CRYPTO_OP_TYPE_SYMMETRIC:
 		__rte_crypto_sym_op_reset(op->sym);
 		break;
+	case RTE_CRYPTO_OP_TYPE_ASYMMETRIC:
+		memset(op->asym, 0, sizeof(struct rte_crypto_asym_op));
+	break;
 	case RTE_CRYPTO_OP_TYPE_UNDEFINED:
 	default:
 		break;
@@ -300,9 +310,14 @@ __rte_crypto_op_get_priv_data(struct rte_crypto_op *op, uint32_t size)
 	if (likely(op->mempool != NULL)) {
 		priv_size = __rte_crypto_op_get_priv_data_size(op->mempool);
 
-		if (likely(priv_size >= size))
-			return (void *)((uint8_t *)(op + 1) +
+		if (likely(priv_size >= size)) {
+			if (op->type == RTE_CRYPTO_OP_TYPE_SYMMETRIC)
+				return (void *)((uint8_t *)(op + 1) +
 					sizeof(struct rte_crypto_sym_op));
+			if (op->type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC)
+				return (void *)((uint8_t *)(op + 1) +
+					sizeof(struct rte_crypto_asym_op));
+		}
 	}
 
 	return NULL;
@@ -405,6 +420,24 @@ rte_crypto_op_attach_sym_session(struct rte_crypto_op *op,
 	return __rte_crypto_sym_op_attach_sym_session(op->sym, sess);
 }
 
+/**
+ * Attach a asymmetric session to a crypto operation
+ *
+ * @param	op	crypto operation, must be of type asymmetric
+ * @param	sess	cryptodev session
+ */
+static inline int
+rte_crypto_op_attach_asym_session(struct rte_crypto_op *op,
+		struct rte_cryptodev_asym_session *sess)
+{
+	if (unlikely(op->type != RTE_CRYPTO_OP_TYPE_ASYMMETRIC))
+		return -1;
+
+	op->sess_type = RTE_CRYPTO_OP_WITH_SESSION;
+	op->asym->session = sess;
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 7e58212..47fc2e5 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -166,6 +166,31 @@ rte_crypto_aead_operation_strings[] = {
 	[RTE_CRYPTO_AEAD_OP_DECRYPT]	= "decrypt"
 };
 
+/**
+ * Asymmetric crypto transform operation strings identifiers.
+ */
+const char *rte_crypto_asym_xform_strings[] = {
+	[RTE_CRYPTO_ASYM_XFORM_NONE]	= "none",
+	[RTE_CRYPTO_ASYM_XFORM_RSA]	= "rsa",
+	[RTE_CRYPTO_ASYM_XFORM_MODEX]	= "modexp",
+	[RTE_CRYPTO_ASYM_XFORM_MODINV]	= "modinv",
+	[RTE_CRYPTO_ASYM_XFORM_DH]	= "dh",
+	[RTE_CRYPTO_ASYM_XFORM_DSA]	= "dsa",
+};
+
+/**
+ * Asymmetric crypto operation strings identifiers.
+ */
+const char *rte_crypto_asym_op_strings[] = {
+	[RTE_CRYPTO_ASYM_OP_ENCRYPT]	= "encrypt",
+	[RTE_CRYPTO_ASYM_OP_DECRYPT]	= "decrypt",
+	[RTE_CRYPTO_ASYM_OP_SIGN]	= "sign",
+	[RTE_CRYPTO_ASYM_OP_VERIFY]	= "verify",
+	[RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE]	= "priv_key_generate",
+	[RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE] = "pub_key_generate",
+	[RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE] = "sharedsecret_compute",
+};
+
 int
 rte_cryptodev_get_cipher_algo_enum(enum rte_crypto_cipher_algorithm *algo_enum,
 		const char *algo_string)
@@ -1111,6 +1136,41 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
 	return 0;
 }
 
+int __rte_experimental
+rte_cryptodev_asym_session_init(uint8_t dev_id,
+		struct rte_cryptodev_asym_session *sess,
+		struct rte_crypto_asym_xform *xforms,
+		struct rte_mempool *mp)
+{
+	struct rte_cryptodev *dev;
+	uint8_t index;
+	int ret;
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+
+	if (sess == NULL || xforms == NULL || dev == NULL)
+		return -EINVAL;
+
+	index = dev->driver_id;
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->asym_session_configure,
+				-ENOTSUP);
+
+	if (sess->sess_private_data[index] == NULL) {
+		ret = dev->dev_ops->asym_session_configure(dev,
+							xforms,
+							sess, mp);
+		if (ret < 0) {
+			CDEV_LOG_ERR(
+				"dev_id %d failed to configure session details",
+				dev_id);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 struct rte_cryptodev_sym_session *
 rte_cryptodev_sym_session_create(struct rte_mempool *mp)
 {
@@ -1130,6 +1190,25 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mp)
 	return sess;
 }
 
+struct rte_cryptodev_asym_session * __rte_experimental
+rte_cryptodev_asym_session_create(struct rte_mempool *mp)
+{
+	struct rte_cryptodev_asym_session *sess;
+
+	/* Allocate a session structure from the session pool */
+	if (rte_mempool_get(mp, (void **)&sess)) {
+		CDEV_LOG_ERR("couldn't get object from session mempool");
+		return NULL;
+	}
+
+	/* Clear device session pointer.
+	 * Include the flag indicating presence of private data
+	 */
+	memset(sess, 0, (sizeof(void *) * nb_drivers) + sizeof(uint8_t));
+
+	return sess;
+}
+
 int
 rte_cryptodev_queue_pair_attach_sym_session(uint8_t dev_id, uint16_t qp_id,
 		struct rte_cryptodev_sym_session *sess)
@@ -1200,6 +1279,24 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
 	return 0;
 }
 
+int __rte_experimental
+rte_cryptodev_asym_session_clear(uint8_t dev_id,
+		struct rte_cryptodev_asym_session *sess)
+{
+	struct rte_cryptodev *dev;
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+
+	if (dev == NULL || sess == NULL)
+		return -EINVAL;
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->asym_session_clear, -ENOTSUP);
+
+	dev->dev_ops->asym_session_clear(dev, sess);
+
+	return 0;
+}
+
 int
 rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
 {
@@ -1224,6 +1321,31 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
 	return 0;
 }
 
+int __rte_experimental
+rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
+{
+	uint8_t i;
+	void *sess_priv;
+	struct rte_mempool *sess_mp;
+
+	if (sess == NULL)
+		return -EINVAL;
+
+	/* Check that all device private data has been freed */
+	for (i = 0; i < nb_drivers; i++) {
+		sess_priv = get_asym_session_private_data(sess, i);
+		if (sess_priv != NULL)
+			return -EBUSY;
+	}
+
+	/* Return session to mempool */
+	sess_mp = rte_mempool_from_obj(sess);
+	rte_mempool_put(sess_mp, sess);
+
+	return 0;
+}
+
+
 unsigned int
 rte_cryptodev_get_header_session_size(void)
 {
@@ -1241,6 +1363,17 @@ rte_cryptodev_sym_get_header_session_size(void)
 	return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
 }
 
+unsigned int __rte_experimental
+rte_cryptodev_asym_get_header_session_size(void)
+{
+	/*
+	 * Header contains pointers to the private data
+	 * of all registered drivers, and a flag which
+	 * indicates presence of private data
+	 */
+	return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
+}
+
 unsigned int
 rte_cryptodev_get_private_session_size(uint8_t dev_id)
 {
@@ -1276,6 +1409,29 @@ rte_cryptodev_sym_get_private_session_size(uint8_t dev_id)
 
 }
 
+unsigned int __rte_experimental
+rte_cryptodev_asym_get_private_session_size(uint8_t dev_id)
+{
+	struct rte_cryptodev *dev;
+	unsigned int header_size = sizeof(void *) * nb_drivers;
+	unsigned int priv_sess_size;
+
+	if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
+		return 0;
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+
+	if (*dev->dev_ops->asym_session_get_size == NULL)
+		return 0;
+
+	priv_sess_size = (*dev->dev_ops->asym_session_get_size)(dev);
+	if (priv_sess_size < header_size)
+		return header_size;
+
+	return priv_sess_size;
+
+}
+
 int __rte_experimental
 rte_cryptodev_sym_session_set_private_data(
 					struct rte_cryptodev_sym_session *sess,
@@ -1308,6 +1464,39 @@ rte_cryptodev_sym_session_get_private_data(
 	return (uint8_t *)sess + off_set;
 }
 
+
+int __rte_experimental
+rte_cryptodev_asym_session_set_private_data(
+					struct rte_cryptodev_asym_session *sess,
+					void *data,
+					uint16_t size)
+{
+	uint16_t off_set = sizeof(void *) * nb_drivers;
+	uint8_t *private_data_present = (uint8_t *)sess + off_set;
+
+	if (sess == NULL)
+		return -EINVAL;
+
+	*private_data_present = 1;
+	off_set += sizeof(uint8_t);
+	rte_memcpy((uint8_t *)sess + off_set, data, size);
+	return 0;
+}
+
+void * __rte_experimental
+rte_cryptodev_asym_session_get_app_private_data(
+				struct rte_cryptodev_asym_session *sess)
+{
+	uint16_t off_set = sizeof(void *) * nb_drivers;
+	uint8_t *private_data_present = (uint8_t *)sess + off_set;
+
+	if (sess == NULL || !*private_data_present)
+		return NULL;
+
+	off_set += sizeof(uint8_t);
+	return (uint8_t *)sess + off_set;
+}
+
 /** Initialise rte_crypto_op mempool element */
 static void
 rte_crypto_op_init(struct rte_mempool *mempool,
@@ -1335,8 +1524,16 @@ rte_crypto_op_pool_create(const char *name, enum rte_crypto_op_type type,
 	struct rte_crypto_op_pool_private *priv;
 
 	unsigned elt_size = sizeof(struct rte_crypto_op) +
-			sizeof(struct rte_crypto_sym_op) +
-			priv_size;
+			    priv_size;
+
+	if (type == RTE_CRYPTO_OP_TYPE_SYMMETRIC) {
+		elt_size += sizeof(struct rte_crypto_sym_op);
+	} else if (type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
+		elt_size += sizeof(struct rte_crypto_asym_op);
+	} else {
+		CDEV_LOG_ERR("Invalid op_type\n");
+		return NULL;
+	}
 
 	/* lookup mempool in case already allocated */
 	struct rte_mempool *mp = rte_mempool_lookup(name);
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index ccc0f73..89dcd40 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -867,9 +867,14 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
  */
 struct rte_cryptodev_sym_session {
 	__extension__ void *sess_private_data[0];
-	/**< Private session material */
+	/**< Private symmetric session material */
 };
 
+/** Cryptodev asymmetric crypto session */
+struct rte_cryptodev_asym_session {
+	__extension__ void *sess_private_data[0];
+	/**< Private asymmetric session material */
+};
 
 /**
  * Create symmetric crypto session header (generic with no private data)
@@ -884,6 +889,18 @@ struct rte_cryptodev_sym_session *
 rte_cryptodev_sym_session_create(struct rte_mempool *mempool);
 
 /**
+ * Create asymmetric crypto session header (generic with no private data)
+ *
+ * @param   mempool    mempool to allocate asymmetric session
+ *                     objects from
+ * @return
+ *  - On success return pointer to asym-session
+ *  - On failure returns NULL
+ */
+struct rte_cryptodev_asym_session * __rte_experimental
+rte_cryptodev_asym_session_create(struct rte_mempool *mempool);
+
+/**
  * Frees symmetric crypto session header, after checking that all
  * the device private data has been freed, returning it
  * to its original mempool.
@@ -899,6 +916,21 @@ int
 rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess);
 
 /**
+ * Frees asymmetric crypto session header, after checking that all
+ * the device private data has been freed, returning it
+ * to its original mempool.
+ *
+ * @param   sess     Session header to be freed.
+ *
+ * @return
+ *  - 0 if successful.
+ *  - -EINVAL if session is NULL.
+ *  - -EBUSY if not all device private data has been freed.
+ */
+int __rte_experimental
+rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess);
+
+/**
  * Fill out private data for the device id, based on its device type.
  *
  * @param   dev_id   ID of device that we want the session to be used on
@@ -920,6 +952,27 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
 			struct rte_mempool *mempool);
 
 /**
+ * Initialize asymmetric session on a device with specific asymmetric xform
+ *
+ * @param   dev_id   ID of device that we want the session to be used on
+ * @param   sess     Session to be set up on a device
+ * @param   xforms   Asymmetric crypto transform operations to apply on flow
+ *                   processed with this session
+ * @param   mempool  Mempool to be used for internal allocation.
+ *
+ * @return
+ *  - On success, zero.
+ *  - -EINVAL if input parameters are invalid.
+ *  - -ENOTSUP if crypto device does not support the crypto transform.
+ *  - -ENOMEM if the private session could not be allocated.
+ */
+int __rte_experimental
+rte_cryptodev_asym_session_init(uint8_t dev_id,
+			struct rte_cryptodev_asym_session *sess,
+			struct rte_crypto_asym_xform *xforms,
+			struct rte_mempool *mempool);
+
+/**
  * Frees private data for the device id, based on its device type,
  * returning it to its mempool. It is the application's responsibility
  * to ensure that private session data is not cleared while there are
@@ -937,6 +990,20 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
 			struct rte_cryptodev_sym_session *sess);
 
 /**
+ * Frees resources held by asymmetric session during rte_cryptodev_session_init
+ *
+ * @param   dev_id   ID of device that uses the asymmetric session.
+ * @param   sess     Asymmetric session setup on device using
+ *					 rte_cryptodev_session_init
+ * @return
+ *  - 0 if successful.
+ *  - -EINVAL if device is invalid or session is NULL.
+ */
+int __rte_experimental
+rte_cryptodev_asym_session_clear(uint8_t dev_id,
+			struct rte_cryptodev_asym_session *sess);
+
+/**
  * @deprecated
  * Get the size of the header session, for all registered drivers.
  *
@@ -971,6 +1038,15 @@ unsigned int
 rte_cryptodev_sym_get_header_session_size(void);
 
 /**
+ * Get the size of the asymmetric session header, for all registered drivers.
+ *
+ * @return
+ *   Size of the asymmetric header session.
+ */
+unsigned int __rte_experimental
+rte_cryptodev_asym_get_header_session_size(void);
+
+/**
  * Get the size of the private symmetric session data
  * for a device.
  *
@@ -985,6 +1061,19 @@ unsigned int
 rte_cryptodev_sym_get_private_session_size(uint8_t dev_id);
 
 /**
+ * Get the size of the private data for asymmetric session
+ * on device
+ *
+ * @param	dev_id		The device identifier.
+ *
+ * @return
+ *   - Size of the asymmetric private data, if successful
+ *   - 0 if device is invalid or does not have private session
+ */
+unsigned int __rte_experimental
+rte_cryptodev_asym_get_private_session_size(uint8_t dev_id);
+
+/**
  * @deprecated
  * Attach queue pair with sym session.
  *
@@ -1072,6 +1161,39 @@ void * __rte_experimental
 rte_cryptodev_sym_session_get_private_data(
 					struct rte_cryptodev_sym_session *sess);
 
+/**
+ * Set private data for a session.
+ *
+ * @param	sess		Session pointer allocated by
+ *				*rte_cryptodev_asym_session_create*.
+ * @param	data		Pointer to the private data.
+ * @param	size		Size of the private data.
+ *
+ * @return
+ *  - On success, zero.
+ *  - On failure, a negative value.
+ */
+int __rte_experimental
+rte_cryptodev_asym_session_set_private_data(
+					struct rte_cryptodev_asym_session *sess,
+					void *data,
+					uint16_t size);
+
+/**
+ * Get private data of a session.
+ *
+ * @param	sess		Session pointer allocated by
+ *				*rte_cryptodev_asym_session_create*.
+ *
+ * @return
+ *  - On success return pointer to private data.
+ *  - On failure returns NULL.
+ */
+void * __rte_experimental
+rte_cryptodev_asym_session_get_app_private_data(
+				struct rte_cryptodev_asym_session *sess);
+
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h b/lib/librte_cryptodev/rte_cryptodev_pmd.h
index db8b976..b94e844 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
@@ -275,6 +275,17 @@ typedef int (*cryptodev_sym_create_session_pool_t)(
  */
 typedef unsigned (*cryptodev_sym_get_session_private_size_t)(
 		struct rte_cryptodev *dev);
+/**
+ * Get the size of a asymmetric cryptodev session
+ *
+ * @param	dev		Crypto device pointer
+ *
+ * @return
+ *  - On success returns the size of the session structure for device
+ *  - On failure returns 0
+ */
+typedef unsigned int (*cryptodev_asym_get_session_private_size_t)(
+		struct rte_cryptodev *dev);
 
 /**
  * Configure a Crypto session on a device.
@@ -294,7 +305,24 @@ typedef int (*cryptodev_sym_configure_session_t)(struct rte_cryptodev *dev,
 		struct rte_crypto_sym_xform *xform,
 		struct rte_cryptodev_sym_session *session,
 		struct rte_mempool *mp);
-
+/**
+ * Configure a Crypto asymmetric session on a device.
+ *
+ * @param	dev		Crypto device pointer
+ * @param	xform		Single or chain of crypto xforms
+ * @param	priv_sess	Pointer to cryptodev's private session structure
+ * @param	mp		Mempool where the private session is allocated
+ *
+ * @return
+ *  - Returns 0 if private session structure have been created successfully.
+ *  - Returns -EINVAL if input parameters are invalid.
+ *  - Returns -ENOTSUP if crypto device does not support the crypto transform.
+ *  - Returns -ENOMEM if the private session could not be allocated.
+ */
+typedef int (*cryptodev_asym_configure_session_t)(struct rte_cryptodev *dev,
+		struct rte_crypto_asym_xform *xform,
+		struct rte_cryptodev_asym_session *session,
+		struct rte_mempool *mp);
 /**
  * Free driver private session data.
  *
@@ -305,6 +333,15 @@ typedef void (*cryptodev_sym_free_session_t)(struct rte_cryptodev *dev,
 		struct rte_cryptodev_sym_session *sess);
 
 /**
+ * Free asymmetric session private data.
+ *
+ * @param	dev		Crypto device pointer
+ * @param	sess		Cryptodev session structure
+ */
+typedef void (*cryptodev_asym_free_session_t)(struct rte_cryptodev *dev,
+		struct rte_cryptodev_asym_session *sess);
+
+/**
  * Optional API for drivers to attach sessions with queue pair.
  * @param	dev		Crypto device pointer
  * @param	qp_id		queue pair id for attaching session
@@ -357,10 +394,16 @@ struct rte_cryptodev_ops {
 
 	cryptodev_sym_get_session_private_size_t session_get_size;
 	/**< Return private session. */
+	cryptodev_asym_get_session_private_size_t asym_session_get_size;
+	/**< Return asym session private size. */
 	cryptodev_sym_configure_session_t session_configure;
 	/**< Configure a Crypto session. */
+	cryptodev_asym_configure_session_t asym_session_configure;
+	/**< Configure asymmetric Crypto session. */
 	cryptodev_sym_free_session_t session_clear;
 	/**< Clear a Crypto sessions private data. */
+	cryptodev_asym_free_session_t asym_session_clear;
+	/**< Clear a Crypto sessions private data. */
 	cryptodev_sym_queue_pair_attach_session_t qp_attach_session;
 	/**< Attach session to queue pair. */
 	cryptodev_sym_queue_pair_detach_session_t qp_detach_session;
@@ -508,6 +551,19 @@ set_session_private_data(struct rte_cryptodev_sym_session *sess,
 	sess->sess_private_data[driver_id] = private_data;
 }
 
+static inline void *
+get_asym_session_private_data(const struct rte_cryptodev_asym_session *sess,
+		uint8_t driver_id) {
+	return sess->sess_private_data[driver_id];
+}
+
+static inline void
+set_asym_session_private_data(struct rte_cryptodev_asym_session *sess,
+		uint8_t driver_id, void *private_data)
+{
+	sess->sess_private_data[driver_id] = private_data;
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map
index be8f4c1..9cdc0ee 100644
--- a/lib/librte_cryptodev/rte_cryptodev_version.map
+++ b/lib/librte_cryptodev/rte_cryptodev_version.map
@@ -95,8 +95,16 @@ DPDK_18.05 {
 } DPDK_17.11;
 
 EXPERIMENTAL {
-        global:
+	global:
 
+	rte_cryptodev_asym_get_header_session_size;
+	rte_cryptodev_asym_get_private_session_size;
+	rte_cryptodev_asym_session_clear;
+	rte_cryptodev_asym_session_create;
+	rte_cryptodev_asym_session_free;
+	rte_cryptodev_asym_session_get_app_private_data;
+	rte_cryptodev_asym_session_init;
+	rte_cryptodev_asym_session_set_private_data;
 	rte_cryptodev_sym_session_get_private_data;
 	rte_cryptodev_sym_session_set_private_data;
 };
-- 
2.9.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v4 3/4] lib/cryptodev: add asymmetric crypto capability in cryptodev
  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-03 15:24 ` [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations Shally Verma
@ 2018-07-03 15:24 ` 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
  3 siblings, 1 reply; 22+ messages in thread
From: Shally Verma @ 2018-07-03 15:24 UTC (permalink / raw)
  To: pablo.de.lara.guarch
  Cc: dev, pathreya, nmurthy, Ashish Gupta, Sunila Sahu, Umesh Kartha

From: Ashish Gupta <ashish.gupta@caviumnetworks.com>

Extend cryptodev with asymmetric capability APIs and
definitions.

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/rte_cryptodev.c           |  76 ++++++++++++++++++
 lib/librte_cryptodev/rte_cryptodev.h           | 103 ++++++++++++++++++++++++-
 lib/librte_cryptodev/rte_cryptodev_version.map |   5 ++
 3 files changed, 183 insertions(+), 1 deletion(-)

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 47fc2e5..2e4b128 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -242,6 +242,24 @@ rte_cryptodev_get_aead_algo_enum(enum rte_crypto_aead_algorithm *algo_enum,
 	return -1;
 }
 
+int __rte_experimental
+rte_cryptodev_asym_get_xform_enum(enum rte_crypto_asym_xform_type *xform_enum,
+		const char *xform_string)
+{
+	unsigned int i;
+
+	for (i = 1; i < RTE_DIM(rte_crypto_asym_xform_strings); i++) {
+		if (strcmp(xform_string,
+			   rte_crypto_asym_xform_strings[i]) == 0) {
+			*xform_enum = (enum rte_crypto_asym_xform_type) i;
+			return 0;
+		}
+	}
+
+	/* Invalid string */
+	return -1;
+}
+
 /**
  * The crypto auth operation strings identifiers.
  * It could be used in application command line.
@@ -312,6 +330,28 @@ param_range_check(uint16_t size, const struct rte_crypto_param_range *range)
 	return -1;
 }
 
+const struct rte_cryptodev_asymmetric_xform_capability * __rte_experimental
+rte_cryptodev_asym_capability_get(uint8_t dev_id,
+		const struct rte_cryptodev_asym_capability_idx *idx)
+{
+	const struct rte_cryptodev_capabilities *capability;
+	struct rte_cryptodev_info dev_info;
+	unsigned int i = 0;
+
+	memset(&dev_info, 0, sizeof(struct rte_cryptodev_info));
+	rte_cryptodev_info_get(dev_id, &dev_info);
+
+	while ((capability = &dev_info.capabilities[i++])->op !=
+			RTE_CRYPTO_OP_TYPE_UNDEFINED) {
+		if (capability->op != RTE_CRYPTO_OP_TYPE_ASYMMETRIC)
+			continue;
+
+		if (capability->asym.xform_capa.xform_type == idx->type)
+			return &capability->asym.xform_capa;
+	}
+	return NULL;
+};
+
 int
 rte_cryptodev_sym_capability_check_cipher(
 		const struct rte_cryptodev_symmetric_capability *capability,
@@ -363,6 +403,42 @@ rte_cryptodev_sym_capability_check_aead(
 
 	return 0;
 }
+int __rte_experimental
+rte_cryptodev_asym_xform_capability_check_optype(
+	const struct rte_cryptodev_asymmetric_xform_capability *capability,
+	enum rte_crypto_asym_op_type op_type)
+{
+	if (capability->op_types & (1 << op_type))
+		return 1;
+
+	return 0;
+}
+
+int __rte_experimental
+rte_cryptodev_asym_xform_capability_check_modlen(
+	const struct rte_cryptodev_asymmetric_xform_capability *capability,
+	uint16_t modlen)
+{
+	/* no need to check for limits, if min or max = 0 */
+	if (capability->modlen.min != 0) {
+		if (modlen < capability->modlen.min)
+			return -1;
+	}
+
+	if (capability->modlen.max != 0) {
+		if (modlen > capability->modlen.max)
+			return -1;
+	}
+
+	/* in any case, check if given modlen is module increment */
+	if (capability->modlen.increment != 0) {
+		if (modlen % (capability->modlen.increment))
+			return -1;
+	}
+
+	return 0;
+}
+
 
 const char *
 rte_cryptodev_get_feature_name(uint64_t flag)
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index 89dcd40..ec87a77 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -143,6 +143,35 @@ struct rte_cryptodev_symmetric_capability {
 	};
 };
 
+/**
+ * Asymmetric Xform Crypto Capability
+ *
+ */
+struct rte_cryptodev_asymmetric_xform_capability {
+	enum rte_crypto_asym_xform_type xform_type;
+	/**< Transform type: RSA/MODEXP/DH/DSA/MODINV */
+
+	uint32_t op_types;
+	/**< bitmask for supported rte_crypto_asym_op_type */
+
+	__extension__
+	union {
+		struct rte_crypto_param_range modlen;
+		/**< Range of modulus length supported by modulus based xform.
+		 * Value 0 mean implementation default
+		 */
+	};
+};
+
+/**
+ * Asymmetric Crypto Capability
+ *
+ */
+struct rte_cryptodev_asymmetric_capability {
+	struct rte_cryptodev_asymmetric_xform_capability xform_capa;
+};
+
+
 /** Structure used to capture a capability of a crypto device */
 struct rte_cryptodev_capabilities {
 	enum rte_crypto_op_type op;
@@ -152,6 +181,8 @@ struct rte_cryptodev_capabilities {
 	union {
 		struct rte_cryptodev_symmetric_capability sym;
 		/**< Symmetric operation capability parameters */
+		struct rte_cryptodev_asymmetric_capability asym;
+		/**< Asymmetric operation capability parameters */
 	};
 };
 
@@ -166,7 +197,17 @@ struct rte_cryptodev_sym_capability_idx {
 };
 
 /**
- *  Provide capabilities available for defined device and algorithm
+ * Structure used to describe asymmetric crypto xforms
+ * Each xform maps to one asym algorithm.
+ *
+ */
+struct rte_cryptodev_asym_capability_idx {
+	enum rte_crypto_asym_xform_type type;
+	/**< Asymmetric xform (algo) type */
+};
+
+/**
+ * Provide capabilities available for defined device and algorithm
  *
  * @param	dev_id		The identifier of the device.
  * @param	idx		Description of crypto algorithms.
@@ -180,6 +221,20 @@ rte_cryptodev_sym_capability_get(uint8_t dev_id,
 		const struct rte_cryptodev_sym_capability_idx *idx);
 
 /**
+ *  Provide capabilities available for defined device and algorithm
+ *
+ * @param	dev_id		The identifier of the device.
+ * @param	algo		Description of crypto algorithms.
+ *
+ * @return
+ *   - Return description of the asymmetric crypto capability if exist.
+ *   - Return NULL if the capability not exist.
+ */
+const struct rte_cryptodev_asymmetric_xform_capability * __rte_experimental
+rte_cryptodev_asym_capability_get(uint8_t dev_id,
+		const struct rte_cryptodev_asym_capability_idx *idx);
+
+/**
  * Check if key size and initial vector are supported
  * in crypto cipher capability
  *
@@ -235,6 +290,36 @@ rte_cryptodev_sym_capability_check_aead(
 		uint16_t iv_size);
 
 /**
+ * Check if op type is supported
+ *
+ * @param	capability	Description of the asymmetric crypto capability.
+ * @param	op_type		op type
+ *
+ * @return
+ *   - Return 1 if the op type is supported
+ *   - Return 0 if unsupported
+ */
+int __rte_experimental
+rte_cryptodev_asym_xform_capability_check_optype(
+	const struct rte_cryptodev_asymmetric_xform_capability *capability,
+		enum rte_crypto_asym_op_type op_type);
+
+/**
+ * Check if modulus length is in supported range
+ *
+ * @param	capability	Description of the asymmetric crypto capability.
+ * @param	modlen		modulus length.
+ *
+ * @return
+ *   - Return 0 if the parameters are in range of the capability.
+ *   - Return -1 if the parameters are out of range of the capability.
+ */
+int __rte_experimental
+rte_cryptodev_asym_xform_capability_check_modlen(
+	const struct rte_cryptodev_asymmetric_xform_capability *capability,
+		uint16_t modlen);
+
+/**
  * Provide the cipher algorithm enum, given an algorithm string
  *
  * @param	algo_enum	A pointer to the cipher algorithm
@@ -279,6 +364,22 @@ int
 rte_cryptodev_get_aead_algo_enum(enum rte_crypto_aead_algorithm *algo_enum,
 		const char *algo_string);
 
+/**
+ * Provide the Asymmetric xform enum, given an xform string
+ *
+ * @param	xform_enum	A pointer to the xform type
+ *				enum to be filled
+ * @param	xform_string	xform string
+ *
+ * @return
+ * - Return -1 if string is not valid
+ * - Return 0 if the string is valid
+ */
+int __rte_experimental
+rte_cryptodev_asym_get_xform_enum(enum rte_crypto_asym_xform_type *xform_enum,
+		const char *xform_string);
+
+
 /** Macro used at end of crypto PMD list */
 #define RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST() \
 	{ RTE_CRYPTO_OP_TYPE_UNDEFINED }
diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map
index 9cdc0ee..aac1498 100644
--- a/lib/librte_cryptodev/rte_cryptodev_version.map
+++ b/lib/librte_cryptodev/rte_cryptodev_version.map
@@ -97,14 +97,19 @@ DPDK_18.05 {
 EXPERIMENTAL {
 	global:
 
+	rte_cryptodev_asym_capability_get;
 	rte_cryptodev_asym_get_header_session_size;
 	rte_cryptodev_asym_get_private_session_size;
+	rte_cryptodev_asym_get_xform_enum;
 	rte_cryptodev_asym_session_clear;
 	rte_cryptodev_asym_session_create;
 	rte_cryptodev_asym_session_free;
 	rte_cryptodev_asym_session_get_app_private_data;
 	rte_cryptodev_asym_session_init;
 	rte_cryptodev_asym_session_set_private_data;
+	rte_cryptodev_asym_xform_capability_check_optype;
 	rte_cryptodev_sym_session_get_private_data;
 	rte_cryptodev_sym_session_set_private_data;
+	rte_crypto_asym_op_strings;
+	rte_crypto_asym_xform_strings;
 };
-- 
2.9.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v4 4/4] doc: add asym crypto in cryptodev programmer guide
  2018-07-03 15:24 [dpdk-dev] [PATCH v4 0/4] crypto: add asym crypto support Shally Verma
                   ` (2 preceding siblings ...)
  2018-07-03 15:24 ` [dpdk-dev] [PATCH v4 3/4] lib/cryptodev: add asymmetric crypto capability in cryptodev Shally Verma
@ 2018-07-03 15:24 ` Shally Verma
  3 siblings, 0 replies; 22+ messages in thread
From: Shally Verma @ 2018-07-03 15:24 UTC (permalink / raw)
  To: pablo.de.lara.guarch
  Cc: dev, pathreya, nmurthy, Sunila Sahu, Ashish Gupta, Umesh Kartha

From: Sunila Sahu <sunila.sahu@caviumnetworks.com>

Update cryptodev programmer guide with description of
asymmetric crypto framework in lib cryptodev.

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>
---
 doc/guides/prog_guide/cryptodev_lib.rst | 290 ++++++++++++++++++++++++++++++--
 1 file changed, 279 insertions(+), 11 deletions(-)

diff --git a/doc/guides/prog_guide/cryptodev_lib.rst b/doc/guides/prog_guide/cryptodev_lib.rst
index 30f0bcf..f6f0405 100644
--- a/doc/guides/prog_guide/cryptodev_lib.rst
+++ b/doc/guides/prog_guide/cryptodev_lib.rst
@@ -8,7 +8,7 @@ The cryptodev library provides a Crypto device framework for management and
 provisioning of hardware and software Crypto poll mode drivers, defining generic
 APIs which support a number of different Crypto operations. The framework
 currently only supports cipher, authentication, chained cipher/authentication
-and AEAD symmetric Crypto operations.
+and AEAD symmetric and asymmetric Crypto operations.
 
 
 Design Principles
@@ -159,8 +159,8 @@ Device Features and Capabilities
 Crypto devices define their functionality through two mechanisms, global device
 features and algorithm capabilities. Global devices features identify device
 wide level features which are applicable to the whole device such as
-the device having hardware acceleration or supporting symmetric Crypto
-operations,
+the device having hardware acceleration or supporting symmetric and/or asymmetric
+Crypto operations,
 
 The capabilities mechanism defines the individual algorithms/functions which
 the device supports, such as a specific symmetric Crypto cipher,
@@ -199,7 +199,7 @@ scope of the Crypto capability see the definition of the structure in the
 Each Crypto poll mode driver defines its own private array of capabilities
 for the operations it supports. Below is an example of the capabilities for a
 PMD which supports the authentication algorithm SHA1_HMAC and the cipher
-algorithm AES_CBC.
+algorithm AES_CBC and RSA operations.
 
 .. code-block:: c
 
@@ -245,7 +245,29 @@ algorithm AES_CBC.
                     }
                 }
             }
-        }
+        },
+		{	/* RSA */
+		.op = RTE_CRYPTO_OP_TYPE_ASYMMETRIC,
+		{.asym = {
+			.xform_type = RTE_CRYPTO_ASYM_XFORM_RSA,
+			.xfrm_capa = {
+				.xform_type = RTE_CRYPTO_ASYM_XFORM_RSA,
+				.op_types = ((1 << RTE_CRYPTO_ASYM_OP_SIGN) |
+					(1 << RTE_CRYPTO_ASYM_OP_VERIFY) |
+					(1 << RTE_CRYPTO_ASYM_OP_ENCRYPT) |
+					(1 << RTE_CRYPTO_ASYM_OP_DECRYPT)),
+				{
+				.modlen = {
+				/* min length is based on openssl rsa keygen */
+				.min = 30,
+				/* value 0 symbolizes no limit on max length */
+				.max = 0,
+				.increment = 1
+				}, }
+			}
+		},
+		}
+	}
     }
 
 
@@ -446,7 +468,7 @@ Crypto workloads.
 
 .. figure:: img/cryptodev_sym_sess.*
 
-The Crypto device framework provides APIs to allocate and initizalize sessions
+The Crypto device framework provides APIs to allocate and initialize sessions
 for crypto devices, where sessions are mempool objects.
 It is the application's responsibility to create and manage the session mempools.
 This approach allows for different scenarios such as having a single session
@@ -788,14 +810,260 @@ using one of the crypto PMDs available in DPDK.
                                             num_dequeued_ops);
     } while (total_num_dequeued_ops < num_enqueued_ops);
 
-
 Asymmetric Cryptography
 -----------------------
 
-Asymmetric functionality is currently not supported by the cryptodev API.
+The cryptodev library currently provides support for the following asymmetric
+Crypto operations; RSA, Modular exponentiation and inversion, Deffie-hellman
+public and/or private key generation and shared secret compute, DSA Signature
+generation and verification.
+
+Session and Session Management
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Sessions are used in asymmetric cryptographic processing to store the immutable
+data defined in asymmetric cryptographic transform which is further used in the
+operation processing. Sessions typically stores information, such as, public
+and private key information or domain params or prime modulus data i.e. immutable
+across data sets. Crypto sessions cache this immutable data in a optimal way for the
+underlying PMD and this allows further acceleration of the offload of Crypto workloads.
+
+Like symmetric, the Crypto device framework provides APIs to allocate and initialize
+asymmetric sessions for crypto devices, where sessions are mempool objects.
+It is the application's responsibility to create and manage the session mempools.
+Application using both symmetric and asymmetric sessions should allocate and maintain
+different sessions pools for each type.
+
+An application can use ``rte_cryptodev_get_asym_session_private_size()`` to
+get the private size of asymmetric session on a given crypto device. This
+function would allow an application to calculate the max device asymmetric
+session size of all crypto devices to create a single session mempool.
+If instead an application creates multiple asymmetric session mempools,
+the Crypto device framework also provides ``rte_cryptodev_asym_get_header_session_size()`` to get
+the size of an uninitialized session.
+
+Once the session mempools have been created, ``rte_cryptodev_asym_session_create()``
+is used to allocate an uninitialized asymmetric session from the given mempool.
+The session then must be initialized using ``rte_cryptodev_asym_session_init()``
+for each of the required crypto devices. An asymmetric transform chain
+is used to specify the operation and its parameters. See the section below for
+details on transforms.
+
+When a session is no longer used, user must call ``rte_cryptodev_asym_session_clear()``
+for each of the crypto devices that are using the session, to free all driver
+private asymmetric session data. Once this is done, session should be freed using
+``rte_cryptodev_asym_session_free()`` which returns them to their mempool.
+
+Asymmetric Sessionless Support
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Currently asymmetric crypto framework does not support sessionless.
+
+Transforms and Transform Chaining
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Asymmetric Crypto transforms (``rte_crypto_asym_xform``) are the mechanism used
+to specify the details of the asymmetric Crypto operation. Next pointer within
+xform allows transform to be chained together. Also it is important to note that
+the order in which the transforms are passed indicates the order of the chaining.
+
+Not all asymmetric crypto xforms are supported for chaining. Currently supported
+asymmetric crypto chaining is Deffie-hellman private key generation followed by
+public generation. Also, currently API does not support chaining of symmetric and
+asymmetric crypto xfroms.
+
+Each xform defines specific asymmetric crypto algo. Currently supported are:
+* RSA
+* Modular operations (Exponentiation and Inverse)
+* Deffie-hellman
+* DSA
+* None - special case where PMD may support a passthrough mode. More for diagnostic purpose
+
+See *DPDK API Reference* for details on each rte_crypto_xxx_xform struct
+
+Asymmetric Operations
+~~~~~~~~~~~~~~~~~~~~~
+
+The asymmetric Crypto operation structure contains all the mutable data relating
+to asymmetric cryptographic processing on an input data buffer. It uses either
+RSA, Modular, Deffie-hellman or DSA operations depending upon session it is attached
+to.
+
+Every operation must carry a valid session handle which further carries information
+on xform or xform-chain to be performed on op. Every xform type defines its own set
+of operational params in their respective rte_crypto_xxx_op_param struct. Depending
+on xform information within session, PMD picks up and process respective op_param
+struct.
+Unlike symmetric, asymmetric operations do not use mbufs for input/output.
+They operate on data buffer of type ``rte_crypto_param``.
+
+See *DPDK API Reference* for details on each rte_crypto_xxx_op_param struct
+
+Asymmetric crypto Sample code
+-----------------------------
+
+There's a unit test application test_cryptodev_asym.c inside unit test framework that
+show how to setup and process asymmetric operations using cryptodev library.
+
+The following sample code shows the basic steps to compute modular exponentiation
+using 1024-bit modulus length using openssl PMD available in DPDK (performing other
+crypto operations is similar except change to respective op and xform setup).
+
+.. code-block:: c
+
+    /*
+     * Simple example to compute modular exponentiation with 1024-bit key
+     *
+     */
+    #define MAX_ASYM_SESSIONS	10
+    #define NUM_ASYM_BUFS	10
+
+    struct rte_mempool *crypto_op_pool, *asym_session_pool;
+    unsigned int asym_session_size;
+    int ret;
+
+    /* Initialize EAL. */
+    ret = rte_eal_init(argc, argv);
+    if (ret < 0)
+        rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n");
+
+    uint8_t socket_id = rte_socket_id();
+
+    /* Create crypto operation pool. */
+    crypto_op_pool = rte_crypto_op_pool_create(
+                                    "crypto_op_pool",
+                                    RTE_CRYPTO_OP_TYPE_ASYMMETRIC,
+                                    NUM_ASYM_BUFS, 0, 0,
+                                    socket_id);
+    if (crypto_op_pool == NULL)
+        rte_exit(EXIT_FAILURE, "Cannot create crypto op pool\n");
+
+    /* Create the virtual crypto device. */
+    char args[128];
+    const char *crypto_name = "crypto_openssl";
+    snprintf(args, sizeof(args), "socket_id=%d", socket_id);
+    ret = rte_vdev_init(crypto_name, args);
+    if (ret != 0)
+        rte_exit(EXIT_FAILURE, "Cannot create virtual device");
+
+    uint8_t cdev_id = rte_cryptodev_get_dev_id(crypto_name);
+
+    /* Get private asym session data size. */
+    asym_session_size = rte_cryptodev_get_asym_private_session_size(cdev_id);
+
+    /*
+     * Create session mempool, with two objects per session,
+     * one for the session header and another one for the
+     * private asym session data for the crypto device.
+     */
+    asym_session_pool = rte_mempool_create("asym_session_pool",
+                                    MAX_ASYM_SESSIONS * 2,
+                                    asym_session_size,
+                                    0,
+                                    0, NULL, NULL, NULL,
+                                    NULL, socket_id,
+                                    0);
+
+    /* Configure the crypto device. */
+    struct rte_cryptodev_config conf = {
+        .nb_queue_pairs = 1,
+        .socket_id = socket_id
+    };
+    struct rte_cryptodev_qp_conf qp_conf = {
+        .nb_descriptors = 2048
+    };
+
+    if (rte_cryptodev_configure(cdev_id, &conf) < 0)
+        rte_exit(EXIT_FAILURE, "Failed to configure cryptodev %u", cdev_id);
+
+    if (rte_cryptodev_queue_pair_setup(cdev_id, 0, &qp_conf,
+                            socket_id, asym_session_pool) < 0)
+        rte_exit(EXIT_FAILURE, "Failed to setup queue pair\n");
+
+    if (rte_cryptodev_start(cdev_id) < 0)
+        rte_exit(EXIT_FAILURE, "Failed to start device\n");
+
+    /* Setup crypto xform to do modular exponentiation with 1024 bit
+	 * length modulus
+	 */
+    struct rte_crypto_asym_xform modex_xform = {
+		.next = NULL,
+		.xform_type = RTE_CRYPTO_ASYM_XFORM_MODEX,
+		.modex = {
+			.modulus = {
+				.data =
+				(uint8_t *)
+				("\xb3\xa1\xaf\xb7\x13\x08\x00\x0a\x35\xdc\x2b\x20\x8d"
+				"\xa1\xb5\xce\x47\x8a\xc3\x80\xf4\x7d\x4a\xa2\x62\xfd\x61\x7f"
+				"\xb5\xa8\xde\x0a\x17\x97\xa0\xbf\xdf\x56\x5a\x3d\x51\x56\x4f"
+				"\x70\x70\x3f\x63\x6a\x44\x5b\xad\x84\x0d\x3f\x27\x6e\x3b\x34"
+				"\x91\x60\x14\xb9\xaa\x72\xfd\xa3\x64\xd2\x03\xa7\x53\x87\x9e"
+				"\x88\x0b\xc1\x14\x93\x1a\x62\xff\xb1\x5d\x74\xcd\x59\x63\x18"
+				"\x11\x3d\x4f\xba\x75\xd4\x33\x4e\x23\x6b\x7b\x57\x44\xe1\xd3"
+				"\x03\x13\xa6\xf0\x8b\x60\xb0\x9e\xee\x75\x08\x9d\x71\x63\x13"
+				"\xcb\xa6\x81\x92\x14\x03\x22\x2d\xde\x55"),
+				.length = 128
+			},
+			.exponent = {
+				.data = (uint8_t *)("\x01\x00\x01"),
+				.length = 3
+			}
+		}
+    };
+    /* Create asym crypto session and initialize it for the crypto device. */
+    struct rte_cryptodev_asym_session *asym_session;
+    asym_session = rte_cryptodev_asym_session_create(asym_session_pool);
+    if (asym_session == NULL)
+        rte_exit(EXIT_FAILURE, "Session could not be created\n");
+
+    if (rte_cryptodev_asym_session_init(cdev_id, asym_session,
+                    &modex_xform, asym_session_pool) < 0)
+        rte_exit(EXIT_FAILURE, "Session could not be initialized "
+                    "for the crypto device\n");
+
+    /* Get a burst of crypto operations. */
+    struct rte_crypto_op *crypto_ops[1];
+    if (rte_crypto_op_bulk_alloc(crypto_op_pool,
+                            RTE_CRYPTO_OP_TYPE_ASYMMETRIC,
+                            crypto_ops, 1) == 0)
+        rte_exit(EXIT_FAILURE, "Not enough crypto operations available\n");
+
+    /* Set up the crypto operations. */
+    struct rte_crypto_asym_op *asym_op = crypto_ops[0]->asym;
+
+	/* calculate mod exp of value 0xf8 */
+    static unsigned char base[] = {0xF8};
+    asym_op->modex.base.data = base;
+    asym_op->modex.base.length = sizeof(base);
+	asym_op->modex.base.iova = base;
+
+    /* Attach the asym crypto session to the operation */
+    rte_crypto_op_attach_asym_session(op, asym_session);
+
+    /* Enqueue the crypto operations in the crypto device. */
+    uint16_t num_enqueued_ops = rte_cryptodev_enqueue_burst(cdev_id, 0,
+                                            crypto_ops, 1);
+
+    /*
+     * Dequeue the crypto operations until all the operations
+     * are processed in the crypto device.
+     */
+    uint16_t num_dequeued_ops, total_num_dequeued_ops = 0;
+    do {
+        struct rte_crypto_op *dequeued_ops[1];
+        num_dequeued_ops = rte_cryptodev_dequeue_burst(cdev_id, 0,
+                                        dequeued_ops, 1);
+        total_num_dequeued_ops += num_dequeued_ops;
 
+        /* Check if operation was processed successfully */
+        if (dequeued_ops[0]->status != RTE_CRYPTO_OP_STATUS_SUCCESS)
+                rte_exit(EXIT_FAILURE,
+                        "Some operations were not processed correctly");
 
-Crypto Device API
-~~~~~~~~~~~~~~~~~
+    } while (total_num_dequeued_ops < num_enqueued_ops);
+
+
+Asymmetric Crypto Device API
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-The cryptodev Library API is described in the *DPDK API Reference* document.
+The cryptodev Library API is described in the
+`DPDK API Reference <http://dpdk.org/doc/api/>`_
-- 
2.9.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  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 22:23   ` De Lara Guarch, Pablo
  2018-07-09 22:30   ` De Lara Guarch, Pablo
  2 siblings, 1 reply; 22+ messages in thread
From: Doherty, Declan @ 2018-07-05 14:54 UTC (permalink / raw)
  To: Shally Verma, pablo.de.lara.guarch
  Cc: dev, pathreya, nmurthy, Sunila Sahu, Ashish Gupta, Umesh Kartha

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_ */
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations
  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
  0 siblings, 1 reply; 22+ messages in thread
From: Trahe, Fiona @ 2018-07-06 13:41 UTC (permalink / raw)
  To: Shally Verma, De Lara Guarch, Pablo
  Cc: dev, pathreya, nmurthy, Umesh Kartha, Sunila Sahu, Ashish Gupta,
	Trahe, Fiona

Hi Shally, Umesh,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shally Verma
> Sent: Tuesday, July 3, 2018 4:24 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com; nmurthy@caviumnetworks.com; Umesh Kartha
> <umesh.kartha@caviumnetworks.com>; Sunila Sahu <sunila.sahu@caviumnetworks.com>; Ashish
> Gupta <ashish.gupta@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations


//snip//
> +
> +int __rte_experimental
> +rte_cryptodev_asym_session_set_private_data(
> +					struct rte_cryptodev_asym_session *sess,
> +					void *data,
> +					uint16_t size)
> +{
> +	uint16_t off_set = sizeof(void *) * nb_drivers;
> +	uint8_t *private_data_present = (uint8_t *)sess + off_set;
> +
> +	if (sess == NULL)
> +		return -EINVAL;
> +
> +	*private_data_present = 1;
> +	off_set += sizeof(uint8_t);
> +	rte_memcpy((uint8_t *)sess + off_set, data, size);
> +	return 0;
> +}
> +
> +void * __rte_experimental
> +rte_cryptodev_asym_session_get_app_private_data(
> +				struct rte_cryptodev_asym_session *sess)
[Fiona] The set api should be renamed if the get function is renamed. 
However, I'd suggest leaving out these functions unless they're really needed for asymm.
Are they just here for consistency with the sym functions?
The sym functions are still experimental and I think the names should be changed to
use user_data instead of private_data.
I've just sent a patch to the mailing list about this - it would be better to resolve that naming
issue first and add corresponding fns later to this api if needed. 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations
  2018-07-06 13:41   ` Trahe, Fiona
@ 2018-07-06 13:48     ` Verma, Shally
  2018-07-06 14:13       ` Trahe, Fiona
  0 siblings, 1 reply; 22+ messages in thread
From: Verma, Shally @ 2018-07-06 13:48 UTC (permalink / raw)
  To: Trahe, Fiona, De Lara Guarch, Pablo
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Kartha, Umesh,
	Sahu, Sunila, Gupta, Ashish

Hi Fiona

>-----Original Message-----
>From: Trahe, Fiona [mailto:fiona.trahe@intel.com]
>Sent: 06 July 2018 19:11
>To: Verma, Shally <Shally.Verma@cavium.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>Cc: dev@dpdk.org; Athreya, Narayana Prasad <NarayanaPrasad.Athreya@cavium.com>; Murthy, Nidadavolu
><Nidadavolu.Murthy@cavium.com>; Kartha, Umesh <Umesh.Kartha@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Gupta,
>Ashish <Ashish.Gupta@cavium.com>; Trahe, Fiona <fiona.trahe@intel.com>
>Subject: RE: [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations
>
>External Email
>
>Hi Shally, Umesh,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shally Verma
>> Sent: Tuesday, July 3, 2018 4:24 PM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org; pathreya@caviumnetworks.com; nmurthy@caviumnetworks.com; Umesh Kartha
>> <umesh.kartha@caviumnetworks.com>; Sunila Sahu <sunila.sahu@caviumnetworks.com>; Ashish
>> Gupta <ashish.gupta@caviumnetworks.com>
>> Subject: [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations
>
>
>//snip//
>> +
>> +int __rte_experimental
>> +rte_cryptodev_asym_session_set_private_data(
>> +                                     struct rte_cryptodev_asym_session *sess,
>> +                                     void *data,
>> +                                     uint16_t size)
>> +{
>> +     uint16_t off_set = sizeof(void *) * nb_drivers;
>> +     uint8_t *private_data_present = (uint8_t *)sess + off_set;
>> +
>> +     if (sess == NULL)
>> +             return -EINVAL;
>> +
>> +     *private_data_present = 1;
>> +     off_set += sizeof(uint8_t);
>> +     rte_memcpy((uint8_t *)sess + off_set, data, size);
>> +     return 0;
>> +}
>> +
>> +void * __rte_experimental
>> +rte_cryptodev_asym_session_get_app_private_data(
>> +                             struct rte_cryptodev_asym_session *sess)
>[Fiona] The set api should be renamed if the get function is renamed.
>However, I'd suggest leaving out these functions unless they're really needed for asymm.
>Are they just here for consistency with the sym functions?
>The sym functions are still experimental and I think the names should be changed to
>use user_data instead of private_data.
>I've just sent a patch to the mailing list about this - it would be better to resolve that naming
>issue first and add corresponding fns later to this api if needed.

Ya . right now they were there for consistency. You prefer to remove them?

Thanks
Shally

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 2/4] cryptodev: support asymmetric operations
  2018-07-06 13:48     ` Verma, Shally
@ 2018-07-06 14:13       ` Trahe, Fiona
  0 siblings, 0 replies; 22+ messages in thread
From: Trahe, Fiona @ 2018-07-06 14:13 UTC (permalink / raw)
  To: Verma, Shally, De Lara Guarch, Pablo
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Kartha, Umesh,
	Sahu, Sunila, Gupta, Ashish, Trahe, Fiona

Hi Shally
 
> Ya . right now they were there for consistency. You prefer to remove them?
 
Yes.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  2018-07-05 14:54   ` Doherty, Declan
@ 2018-07-06 14:28     ` Verma, Shally
  2018-07-09  5:45       ` Verma, Shally
  2018-07-09 14:54       ` Doherty, Declan
  0 siblings, 2 replies; 22+ messages in thread
From: Verma, Shally @ 2018-07-06 14:28 UTC (permalink / raw)
  To: Doherty, Declan, pablo.de.lara.guarch
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh

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_ */
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  2018-07-06 14:28     ` Verma, Shally
@ 2018-07-09  5:45       ` Verma, Shally
  2018-07-09 14:54       ` Doherty, Declan
  1 sibling, 0 replies; 22+ messages in thread
From: Verma, Shally @ 2018-07-09  5:45 UTC (permalink / raw)
  To: Doherty, Declan, pablo.de.lara.guarch
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh

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_ */
>>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  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-12 18:16         ` Verma, Shally
  1 sibling, 2 replies; 22+ messages in thread
From: Doherty, Declan @ 2018-07-09 14:54 UTC (permalink / raw)
  To: Verma, Shally, pablo.de.lara.guarch
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh

On 06/07/2018 3:28 PM, Verma, Shally wrote:
> 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?
> 

Sure I understand the rationale.

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

I would be a bit concerned with dropping the 
rte_crypto_asym_xx_op_params as if we are following the symmetric model, 
the xform should only specify the immutable data in relation to a 
transform and the operation should provide the mutable input data. I'm 
not sure how this would look like for the different asymmetric 
operations but if there are transforms that don't have immutable data 
then it would make more sense not to have a specific xform data 
structure for that and pass the data through that transformations 
operation structure.

> 
> We were about to submit Openssl PMD with asym support today. But I would hold back till we align on this.
> ...
> 

Ok, I think I had missed part of the complexity of the chained used 
case. I think if we are to follow the symmetric crypto model we end up 
with something along the lines of a fundamental op types - dh, dsa, mod, 
rsa etc.

enum rte_crypto_asym_op_types {
	RTE_CRYPTO_ASYM_OP_DH,
	RTE_CRYPTO_ASYM_OP_DSA,
	RTE_CRYPTO_ASYM_OP_MOD,
	RTE_CRYPTO_ASYM_OP_RSA,
};

with a corresponding asym_op as follows.

struct rte_crypto_asym_op {
	union {
		struct rte_cryptodev_asym_session *session;
		/**< Handle for the initialized session context */
		struct rte_crypto_asym_xform *xform;
	};

	union {
		struct rte_crypto_asym_dh_op_data dh;
		struct rte_crypto_asym_dsa_op_data dsa;
		struct rte_crypto_asyn_mod_op_data mod;
		struct rte_crypto_asym_rsa_op_data rsa;
	} op_data;
};


In terms of the xform definitions I'm not sure that we should have all 
the different transforms defined in the same enum. If we take the below 
approach, all the specifics of each transform could be split out into 
separate headers, giving a cleaner API. This would see a xform types 
enum which would mirror the operation types:

enum rte_crypto_asym_xform_types {
	RTE_CRYPTO_ASYM_XFORM_DH,
	RTE_CRYPTO_ASYM_XFORM_DSA,
	RTE_CRYPTO_ASYM_XFORM_MOD,
	RTE_CRYPTO_ASYM_XFORM_RSA,
}

with corresponding xforms structures

struct rte_crypto_asym_xform dh;
struct rte_crypto_asym_xform dsa;
struct rte_crypto_asym_xform modlus;
struct rte_crypto_asym_xform rsa;

then within the xforms would define the sub-type of that xform and have 
definitions of  the relevant session data which is required, with the 
rsa it might look like something like this:

enum rte_crypto_asym_xform_rsa_types {
	RTE_CRYPTO_ASYM_XFORM_RSA_ENCRYPT,
	RTE_CRYPTO_ASYM_XFORM_RSA_DECRYPT,
	RTE_CRYPTO_ASYM_XFORM_RSA_SIGN,
	RTE_CRYPTO_ASYM_XFORM_RSA_VERIFY,
}

struct rte_crypto_asym_xform rsa {
	enum rte_crypto_asym_op_rsa_types type;
	union {
		struct rte_crypto_asym_xform_rsa_encrypt_data encrypt;
		struct rte_crypto_asym_xform_rsa_decrypt_data decrypt;
		etc...
	} data;
};

The need for xform sub-type data structure would be dependent on whether 
there was immutable data associated with a particular transform. Also if 
we take this approach it would allow each operation type to be separated 
out into there own headers.

>> ....
>>
>>
>>> + */
>>> +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?
> 
The sess_type=RTE_CRYPTO_OP_WITH_SESSION/RTE_CRYPTO_OP_SESSIONLESS 
should be set in the outer crypto_op struct, so if the PMD doesn't 
support the selected sess_type, it should just fail the enqueue and 
possibly log an error.

> Thanks for review.
> Shally
> 
>>          __extension__
>>
>>
>>> +#ifdef __cplusplus
>>> +}
>>> +#endif
>>> +
>>> +#endif /* _RTE_CRYPTO_ASYM_H_ */
>>>
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  2018-07-09 14:54       ` Doherty, Declan
@ 2018-07-09 16:44         ` Trahe, Fiona
  2018-07-09 17:12           ` Verma, Shally
  2018-07-12 18:16         ` Verma, Shally
  1 sibling, 1 reply; 22+ messages in thread
From: Trahe, Fiona @ 2018-07-09 16:44 UTC (permalink / raw)
  To: Doherty, Declan, Verma, Shally, De Lara Guarch, Pablo
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh, Trahe, Fiona

Hi Shally,  Declan, Pablo,

I'm concerned about rushing in significant last-minute changes, but would like to see this API in 18.08.
So I suggest the patchset is applied with the caveat that it is experimental and will continue to be so
for the next release, in which the remaining open issues should be addressed. 
The main areas of concern are:
 - the structures for xforms and ops and rework needed to cater for sessionless
 - capabilities


Regards,
Fiona


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Doherty, Declan
> Sent: Monday, July 9, 2018 3:55 PM
> To: Verma, Shally <Shally.Verma@cavium.com>; De Lara Guarch, Pablo
> <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
> 
> On 06/07/2018 3:28 PM, Verma, Shally wrote:
> > 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?
> >
> 
> Sure I understand the rationale.
> 
> >
> >> 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?
> 
> I would be a bit concerned with dropping the
> rte_crypto_asym_xx_op_params as if we are following the symmetric model,
> the xform should only specify the immutable data in relation to a
> transform and the operation should provide the mutable input data. I'm
> not sure how this would look like for the different asymmetric
> operations but if there are transforms that don't have immutable data
> then it would make more sense not to have a specific xform data
> structure for that and pass the data through that transformations
> operation structure.
> 
> >
> > We were about to submit Openssl PMD with asym support today. But I would hold back till we align on
> this.
> > ...
> >
> 
> Ok, I think I had missed part of the complexity of the chained used
> case. I think if we are to follow the symmetric crypto model we end up
> with something along the lines of a fundamental op types - dh, dsa, mod,
> rsa etc.
> 
> enum rte_crypto_asym_op_types {
> 	RTE_CRYPTO_ASYM_OP_DH,
> 	RTE_CRYPTO_ASYM_OP_DSA,
> 	RTE_CRYPTO_ASYM_OP_MOD,
> 	RTE_CRYPTO_ASYM_OP_RSA,
> };
> 
> with a corresponding asym_op as follows.
> 
> struct rte_crypto_asym_op {
> 	union {
> 		struct rte_cryptodev_asym_session *session;
> 		/**< Handle for the initialized session context */
> 		struct rte_crypto_asym_xform *xform;
> 	};
> 
> 	union {
> 		struct rte_crypto_asym_dh_op_data dh;
> 		struct rte_crypto_asym_dsa_op_data dsa;
> 		struct rte_crypto_asyn_mod_op_data mod;
> 		struct rte_crypto_asym_rsa_op_data rsa;
> 	} op_data;
> };
> 
> 
> In terms of the xform definitions I'm not sure that we should have all
> the different transforms defined in the same enum. If we take the below
> approach, all the specifics of each transform could be split out into
> separate headers, giving a cleaner API. This would see a xform types
> enum which would mirror the operation types:
> 
> enum rte_crypto_asym_xform_types {
> 	RTE_CRYPTO_ASYM_XFORM_DH,
> 	RTE_CRYPTO_ASYM_XFORM_DSA,
> 	RTE_CRYPTO_ASYM_XFORM_MOD,
> 	RTE_CRYPTO_ASYM_XFORM_RSA,
> }
> 
> with corresponding xforms structures
> 
> struct rte_crypto_asym_xform dh;
> struct rte_crypto_asym_xform dsa;
> struct rte_crypto_asym_xform modlus;
> struct rte_crypto_asym_xform rsa;
> 
> then within the xforms would define the sub-type of that xform and have
> definitions of  the relevant session data which is required, with the
> rsa it might look like something like this:
> 
> enum rte_crypto_asym_xform_rsa_types {
> 	RTE_CRYPTO_ASYM_XFORM_RSA_ENCRYPT,
> 	RTE_CRYPTO_ASYM_XFORM_RSA_DECRYPT,
> 	RTE_CRYPTO_ASYM_XFORM_RSA_SIGN,
> 	RTE_CRYPTO_ASYM_XFORM_RSA_VERIFY,
> }
> 
> struct rte_crypto_asym_xform rsa {
> 	enum rte_crypto_asym_op_rsa_types type;
> 	union {
> 		struct rte_crypto_asym_xform_rsa_encrypt_data encrypt;
> 		struct rte_crypto_asym_xform_rsa_decrypt_data decrypt;
> 		etc...
> 	} data;
> };
> 
> The need for xform sub-type data structure would be dependent on whether
> there was immutable data associated with a particular transform. Also if
> we take this approach it would allow each operation type to be separated
> out into there own headers.
> 
> >> ....
> >>
> >>
> >>> + */
> >>> +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?
> >
> The sess_type=RTE_CRYPTO_OP_WITH_SESSION/RTE_CRYPTO_OP_SESSIONLESS
> should be set in the outer crypto_op struct, so if the PMD doesn't
> support the selected sess_type, it should just fail the enqueue and
> possibly log an error.
> 
> > Thanks for review.
> > Shally
> >
> >>          __extension__
> >>
> >>
> >>> +#ifdef __cplusplus
> >>> +}
> >>> +#endif
> >>> +
> >>> +#endif /* _RTE_CRYPTO_ASYM_H_ */
> >>>
> >


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  2018-07-09 16:44         ` Trahe, Fiona
@ 2018-07-09 17:12           ` Verma, Shally
  2018-07-09 17:16             ` De Lara Guarch, Pablo
  0 siblings, 1 reply; 22+ messages in thread
From: Verma, Shally @ 2018-07-09 17:12 UTC (permalink / raw)
  To: Trahe, Fiona, Doherty, Declan, De Lara Guarch, Pablo
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh



>-----Original Message-----
>From: Trahe, Fiona [mailto:fiona.trahe@intel.com]
>Sent: 09 July 2018 22:15
>To: Doherty, Declan <declan.doherty@intel.com>; Verma, Shally <Shally.Verma@cavium.com>; De Lara Guarch, Pablo
><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>; Trahe, Fiona <fiona.trahe@intel.com>
>Subject: RE: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
>
>External Email
>
>Hi Shally,  Declan, Pablo,
>
>I'm concerned about rushing in significant last-minute changes, but would like to see this API in 18.08.
>So I suggest the patchset is applied with the caveat that it is experimental and will continue to be so
>for the next release, in which the remaining open issues should be addressed.
>The main areas of concern are:
> - the structures for xforms and ops and rework needed to cater for sessionless
> - capabilities
>
Sounds good to me. If that’s fine with everyone, I will send current openssl PMD patch. Please confirm.

Thanks
Shally
>
>Regards,
>Fiona
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Doherty, Declan
>> Sent: Monday, July 9, 2018 3:55 PM
>> To: Verma, Shally <Shally.Verma@cavium.com>; De Lara Guarch, Pablo
>> <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
>>
>> On 06/07/2018 3:28 PM, Verma, Shally wrote:
>> > 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?
>> >
>>
>> Sure I understand the rationale.
>>
>> >
>> >> 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?
>>
>> I would be a bit concerned with dropping the
>> rte_crypto_asym_xx_op_params as if we are following the symmetric model,
>> the xform should only specify the immutable data in relation to a
>> transform and the operation should provide the mutable input data. I'm
>> not sure how this would look like for the different asymmetric
>> operations but if there are transforms that don't have immutable data
>> then it would make more sense not to have a specific xform data
>> structure for that and pass the data through that transformations
>> operation structure.
>>
>> >
>> > We were about to submit Openssl PMD with asym support today. But I would hold back till we align on
>> this.
>> > ...
>> >
>>
>> Ok, I think I had missed part of the complexity of the chained used
>> case. I think if we are to follow the symmetric crypto model we end up
>> with something along the lines of a fundamental op types - dh, dsa, mod,
>> rsa etc.
>>
>> enum rte_crypto_asym_op_types {
>>       RTE_CRYPTO_ASYM_OP_DH,
>>       RTE_CRYPTO_ASYM_OP_DSA,
>>       RTE_CRYPTO_ASYM_OP_MOD,
>>       RTE_CRYPTO_ASYM_OP_RSA,
>> };
>>
>> with a corresponding asym_op as follows.
>>
>> struct rte_crypto_asym_op {
>>       union {
>>               struct rte_cryptodev_asym_session *session;
>>               /**< Handle for the initialized session context */
>>               struct rte_crypto_asym_xform *xform;
>>       };
>>
>>       union {
>>               struct rte_crypto_asym_dh_op_data dh;
>>               struct rte_crypto_asym_dsa_op_data dsa;
>>               struct rte_crypto_asyn_mod_op_data mod;
>>               struct rte_crypto_asym_rsa_op_data rsa;
>>       } op_data;
>> };
>>
>>
>> In terms of the xform definitions I'm not sure that we should have all
>> the different transforms defined in the same enum. If we take the below
>> approach, all the specifics of each transform could be split out into
>> separate headers, giving a cleaner API. This would see a xform types
>> enum which would mirror the operation types:
>>
>> enum rte_crypto_asym_xform_types {
>>       RTE_CRYPTO_ASYM_XFORM_DH,
>>       RTE_CRYPTO_ASYM_XFORM_DSA,
>>       RTE_CRYPTO_ASYM_XFORM_MOD,
>>       RTE_CRYPTO_ASYM_XFORM_RSA,
>> }
>>
>> with corresponding xforms structures
>>
>> struct rte_crypto_asym_xform dh;
>> struct rte_crypto_asym_xform dsa;
>> struct rte_crypto_asym_xform modlus;
>> struct rte_crypto_asym_xform rsa;
>>
>> then within the xforms would define the sub-type of that xform and have
>> definitions of  the relevant session data which is required, with the
>> rsa it might look like something like this:
>>
>> enum rte_crypto_asym_xform_rsa_types {
>>       RTE_CRYPTO_ASYM_XFORM_RSA_ENCRYPT,
>>       RTE_CRYPTO_ASYM_XFORM_RSA_DECRYPT,
>>       RTE_CRYPTO_ASYM_XFORM_RSA_SIGN,
>>       RTE_CRYPTO_ASYM_XFORM_RSA_VERIFY,
>> }
>>
>> struct rte_crypto_asym_xform rsa {
>>       enum rte_crypto_asym_op_rsa_types type;
>>       union {
>>               struct rte_crypto_asym_xform_rsa_encrypt_data encrypt;
>>               struct rte_crypto_asym_xform_rsa_decrypt_data decrypt;
>>               etc...
>>       } data;
>> };
>>
>> The need for xform sub-type data structure would be dependent on whether
>> there was immutable data associated with a particular transform. Also if
>> we take this approach it would allow each operation type to be separated
>> out into there own headers.
>>
>> >> ....
>> >>
>> >>
>> >>> + */
>> >>> +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?
>> >
>> The sess_type=RTE_CRYPTO_OP_WITH_SESSION/RTE_CRYPTO_OP_SESSIONLESS
>> should be set in the outer crypto_op struct, so if the PMD doesn't
>> support the selected sess_type, it should just fail the enqueue and
>> possibly log an error.
>>
>> > Thanks for review.
>> > Shally
>> >
>> >>          __extension__
>> >>
>> >>
>> >>> +#ifdef __cplusplus
>> >>> +}
>> >>> +#endif
>> >>> +
>> >>> +#endif /* _RTE_CRYPTO_ASYM_H_ */
>> >>>
>> >


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  2018-07-09 17:12           ` Verma, Shally
@ 2018-07-09 17:16             ` De Lara Guarch, Pablo
  2018-07-09 17:42               ` Verma, Shally
  0 siblings, 1 reply; 22+ messages in thread
From: De Lara Guarch, Pablo @ 2018-07-09 17:16 UTC (permalink / raw)
  To: Verma, Shally, Trahe, Fiona, Doherty, Declan
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh

Hi Shally,

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Monday, July 9, 2018 6:12 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; De Lara Guarch, Pablo
> <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
> 
> 
> 
> >-----Original Message-----
> >From: Trahe, Fiona [mailto:fiona.trahe@intel.com]
> >Sent: 09 July 2018 22:15
> >To: Doherty, Declan <declan.doherty@intel.com>; Verma, Shally
> ><Shally.Verma@cavium.com>; De Lara Guarch, Pablo
> ><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>; Trahe, Fiona <fiona.trahe@intel.com>
> >Subject: RE: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric
> >algos in cryptodev
> >
> >External Email
> >
> >Hi Shally,  Declan, Pablo,
> >
> >I'm concerned about rushing in significant last-minute changes, but would like
> to see this API in 18.08.
> >So I suggest the patchset is applied with the caveat that it is
> >experimental and will continue to be so for the next release, in which the
> remaining open issues should be addressed.
> >The main areas of concern are:
> > - the structures for xforms and ops and rework needed to cater for
> >sessionless
> > - capabilities
> >
> Sounds good to me. If that’s fine with everyone, I will send current openssl PMD
> patch. Please confirm.
> 

I agree with Fiona. This was postponed one release, so it is fair that it makes it into 18.08,
knowing that there will be substantial changes in the next release.

About OpenSSL, please make sure that it works on 1.1.0.

Thanks,
Pablo

> Thanks
> Shally
> >
> >Regards,
> >Fiona
> >
> >


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  2018-07-09 17:16             ` De Lara Guarch, Pablo
@ 2018-07-09 17:42               ` Verma, Shally
  0 siblings, 0 replies; 22+ messages in thread
From: Verma, Shally @ 2018-07-09 17:42 UTC (permalink / raw)
  To: De Lara Guarch, Pablo, Trahe, Fiona, Doherty, Declan
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh



>-----Original Message-----
>From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
>Sent: 09 July 2018 22:46
>To: Verma, Shally <Shally.Verma@cavium.com>; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan <declan.doherty@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
>
>Hi Shally,
>
>> -----Original Message-----
>> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
>> Sent: Monday, July 9, 2018 6:12 PM
>> To: Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
>> <declan.doherty@intel.com>; De Lara Guarch, Pablo
>> <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
>>
>>
>>
>> >-----Original Message-----
>> >From: Trahe, Fiona [mailto:fiona.trahe@intel.com]
>> >Sent: 09 July 2018 22:15
>> >To: Doherty, Declan <declan.doherty@intel.com>; Verma, Shally
>> ><Shally.Verma@cavium.com>; De Lara Guarch, Pablo
>> ><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>; Trahe, Fiona <fiona.trahe@intel.com>
>> >Subject: RE: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric
>> >algos in cryptodev
>> >
>> >External Email
>> >
>> >Hi Shally,  Declan, Pablo,
>> >
>> >I'm concerned about rushing in significant last-minute changes, but would like
>> to see this API in 18.08.
>> >So I suggest the patchset is applied with the caveat that it is
>> >experimental and will continue to be so for the next release, in which the
>> remaining open issues should be addressed.
>> >The main areas of concern are:
>> > - the structures for xforms and ops and rework needed to cater for
>> >sessionless
>> > - capabilities
>> >
>> Sounds good to me. If that’s fine with everyone, I will send current openssl PMD
>> patch. Please confirm.
>>
>
>I agree with Fiona. This was postponed one release, so it is fair that it makes it into 18.08,
>knowing that there will be substantial changes in the next release.
>
We will continue to discuss through open items  to find level of change.

>About OpenSSL, please make sure that it works on 1.1.0.
Yup. It will support 1.1.0 with compatibility to 1.0.2

Thanks
Shally

>
>Thanks,
>Pablo
>
>> Thanks
>> Shally
>> >
>> >Regards,
>> >Fiona
>> >
>> >


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  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-09 22:23   ` De Lara Guarch, Pablo
  2018-07-10  5:22     ` Verma, Shally
  2018-07-09 22:30   ` De Lara Guarch, Pablo
  2 siblings, 1 reply; 22+ messages in thread
From: De Lara Guarch, Pablo @ 2018-07-09 22:23 UTC (permalink / raw)
  To: Shally Verma
  Cc: dev, pathreya, nmurthy, Sunila Sahu, Ashish Gupta, Umesh Kartha

Hi Shally,

A few minor comments:

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Tuesday, July 3, 2018 4:24 PM
> To: De Lara Guarch, Pablo <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: [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
> 

Change title to: "cryptodev: add basic asymmetric structures"?
At least, don't use lib, and cryptodev twice.

> 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

Diffie-Hellman

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  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-09 22:23   ` De Lara Guarch, Pablo
@ 2018-07-09 22:30   ` De Lara Guarch, Pablo
  2 siblings, 0 replies; 22+ messages in thread
From: De Lara Guarch, Pablo @ 2018-07-09 22:30 UTC (permalink / raw)
  To: Shally Verma
  Cc: dev, pathreya, nmurthy, Sunila Sahu, Ashish Gupta, Umesh Kartha



> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Tuesday, July 3, 2018 4:24 PM
> To: De Lara Guarch, Pablo <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: [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
> 
> Add rte_crypto_asym.h with supported xfrms and associated op structures and
> APIs

One more comment that I missed.

> +++ b/lib/librte_cryptodev/rte_crypto_asym.h

...

> +struct rte_crypto_rsa_op_param {

...

> +	rte_crypto_param sign;
> +	/**<
> +	 * Pointer to RSA signature data. If operation is RSA
> +	 * sign @ref RTE_CRYPTO_RSA_OP_SIGN, buffer will be
> +	 * over-written with generated signature.

There is an error when building the API documentation here:

lib/librte_cryptodev/rte_crypto_asym.h:383: warning:
unable to resolve reference to `RTE_CRYPTO_RSA_OP_SIGN' for \ref command

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 3/4] lib/cryptodev: add asymmetric crypto capability in cryptodev
  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
  0 siblings, 0 replies; 22+ messages in thread
From: De Lara Guarch, Pablo @ 2018-07-09 22:34 UTC (permalink / raw)
  To: Shally Verma
  Cc: dev, pathreya, nmurthy, Ashish Gupta, Sunila Sahu, Umesh Kartha



> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Tuesday, July 3, 2018 4:24 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> nmurthy@caviumnetworks.com; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>; Sunila Sahu
> <sunila.sahu@caviumnetworks.com>; Umesh Kartha
> <umesh.kartha@caviumnetworks.com>
> Subject: [PATCH v4 3/4] lib/cryptodev: add asymmetric crypto capability in
> cryptodev
> 

Remove "in cryptodev" from title, as it is redundant (and lib/).


> From: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> 
> Extend cryptodev with asymmetric capability APIs and definitions.
> 
> 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>
>

...

 +++ b/lib/librte_cryptodev/rte_cryptodev.c

...

> +	for (i = 1; i < RTE_DIM(rte_crypto_asym_xform_strings); i++) {
> +		if (strcmp(xform_string,
> +			   rte_crypto_asym_xform_strings[i]) == 0) {

Add indentation here.

> +			*xform_enum = (enum rte_crypto_asym_xform_type) i;
> +			return 0;
> +		}
> +	}
> +
> +	/* Invalid string */
> +	return -1;
> +}

...

> +++ b/lib/librte_cryptodev/rte_cryptodev.h

...

>  /**
> + *  Provide capabilities available for defined device and algorithm
> + *
> + * @param	dev_id		The identifier of the device.
> + * @param	algo		Description of crypto algorithms.
> + *

Build error on API docs:

lib/librte_cryptodev/rte_cryptodev.h:224: warning: argument 'algo' of command
@param is not found in the argument list of rte_cryptodev_asym_capability_get(uint8_t dev_id,
const struct rte_cryptodev_asym_capability_idx *idx)
lib/librte_cryptodev/rte_cryptodev.h:234: warning: The following parameters of
rte_cryptodev_asym_capability_get(uint8_t dev_id, const struct rte_cryptodev_asym_capability_idx *idx)
are not documented:  parameter 'idx'

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  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
  0 siblings, 1 reply; 22+ messages in thread
From: Verma, Shally @ 2018-07-10  5:22 UTC (permalink / raw)
  To: De Lara Guarch, Pablo
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh

Hi Pablo

>-----Original Message-----
>From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
>Sent: 10 July 2018 03:54
>To: Verma, Shally <Shally.Verma@cavium.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: [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
>
>External Email
>
>Hi Shally,
>
>A few minor comments:
>
>> -----Original Message-----
>> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
>> Sent: Tuesday, July 3, 2018 4:24 PM
>> To: De Lara Guarch, Pablo <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: [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
>>
>
>Change title to: "cryptodev: add basic asymmetric structures"?
>At least, don't use lib, and cryptodev twice.

So what's convention for all patches? Just cryptodev: <actual title> ?

Thanks
Shally

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  2018-07-10  5:22     ` Verma, Shally
@ 2018-07-10  8:08       ` De Lara Guarch, Pablo
  0 siblings, 0 replies; 22+ messages in thread
From: De Lara Guarch, Pablo @ 2018-07-10  8:08 UTC (permalink / raw)
  To: Verma, Shally
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh



> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Tuesday, July 10, 2018 6:22 AM
> To: De Lara Guarch, Pablo <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: [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
> 
> Hi Pablo
> 
> >-----Original Message-----
> >From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
> >Sent: 10 July 2018 03:54
> >To: Verma, Shally <Shally.Verma@cavium.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: [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in
> >cryptodev
> >
> >External Email
> >
> >Hi Shally,
> >
> >A few minor comments:
> >
> >> -----Original Message-----
> >> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> >> Sent: Tuesday, July 3, 2018 4:24 PM
> >> To: De Lara Guarch, Pablo <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: [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in
> >> cryptodev
> >>
> >
> >Change title to: "cryptodev: add basic asymmetric structures"?
> >At least, don't use lib, and cryptodev twice.
> 
> So what's convention for all patches? Just cryptodev: <actual title> ?

Correct. For libraries, title prefix should be just the name of the library.
For other components, we usually need two names, the folder where the belong and the component name.
For example, for drivers, we use the folder where they belong and their name (e.g. crypto/openssl).

Pablo

> 
> Thanks
> Shally
> 
> >
> >> 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
> >
> >Diffie-Hellman
> >
> >> - 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>
> >

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/4] lib/cryptodev: add asymmetric algos in cryptodev
  2018-07-09 14:54       ` Doherty, Declan
  2018-07-09 16:44         ` Trahe, Fiona
@ 2018-07-12 18:16         ` Verma, Shally
  1 sibling, 0 replies; 22+ messages in thread
From: Verma, Shally @ 2018-07-12 18:16 UTC (permalink / raw)
  To: Doherty, Declan, pablo.de.lara.guarch
  Cc: dev, Athreya, Narayana Prasad, Murthy, Nidadavolu, Sahu, Sunila,
	Gupta, Ashish, Kartha, Umesh

HI Declan

>-----Original Message-----
>From: Doherty, Declan [mailto:declan.doherty@intel.com]
>Sent: 09 July 2018 20:25
>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
>
>On 06/07/2018 3:28 PM, Verma, Shally wrote:
>> 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?
>>
>
>Sure I understand the rationale.
>
>>
>>> 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?
>
>I would be a bit concerned with dropping the
>rte_crypto_asym_xx_op_params as if we are following the symmetric model,
>the xform should only specify the immutable data in relation to a
>transform and the operation should provide the mutable input data. I'm
>not sure how this would look like for the different asymmetric
>operations but if there are transforms that don't have immutable data
>then it would make more sense not to have a specific xform data
>structure for that and pass the data through that transformations
>operation structure.

[Shally] Believe there's some misunderstanding here. My suggestion was not to drop asym_xx_op_params, but just an op_type field from op_params if xform types are redefined as suggested above. However, I see below you have another proposal so I will clarify more there.
As a side note, Asym works on similar principle as symmetric i.e. immutable data in xform and mutable in op_params. So, op_params definitely will be there.

>
>>
>> We were about to submit Openssl PMD with asym support today. But I would hold back till we align on this.
>> ...
>>
>
>Ok, I think I had missed part of the complexity of the chained used
>case. I think if we are to follow the symmetric crypto model we end up
>with something along the lines of a fundamental op types - dh, dsa, mod,
>rsa etc.
>
>enum rte_crypto_asym_op_types {
>        RTE_CRYPTO_ASYM_OP_DH,
>        RTE_CRYPTO_ASYM_OP_DSA,
>        RTE_CRYPTO_ASYM_OP_MOD,
>        RTE_CRYPTO_ASYM_OP_RSA,
>};
>
>with a corresponding asym_op as follows.
>
>struct rte_crypto_asym_op {
>        union {
>                struct rte_cryptodev_asym_session *session;
>                /**< Handle for the initialized session context */
>                struct rte_crypto_asym_xform *xform;
>        };
>
>        union {
>                struct rte_crypto_asym_dh_op_data dh;
>                struct rte_crypto_asym_dsa_op_data dsa;
>                struct rte_crypto_asyn_mod_op_data mod;
>                struct rte_crypto_asym_rsa_op_data rsa;
>        } op_data;
>};
>
>
>In terms of the xform definitions I'm not sure that we should have all
>the different transforms defined in the same enum. If we take the below
>approach, all the specifics of each transform could be split out into
>separate headers, giving a cleaner API. This would see a xform types
>enum which would mirror the operation types:
>
>enum rte_crypto_asym_xform_types {
>        RTE_CRYPTO_ASYM_XFORM_DH,
>        RTE_CRYPTO_ASYM_XFORM_DSA,
>        RTE_CRYPTO_ASYM_XFORM_MOD,
>        RTE_CRYPTO_ASYM_XFORM_RSA,
>}
>
>with corresponding xforms structures
>
>struct rte_crypto_asym_xform dh;
>struct rte_crypto_asym_xform dsa;
>struct rte_crypto_asym_xform modlus;
>struct rte_crypto_asym_xform rsa;
>
>then within the xforms would define the sub-type of that xform and have
>definitions of  the relevant session data which is required, with the
>rsa it might look like something like this:
>
>enum rte_crypto_asym_xform_rsa_types {
>        RTE_CRYPTO_ASYM_XFORM_RSA_ENCRYPT,
>        RTE_CRYPTO_ASYM_XFORM_RSA_DECRYPT,
>        RTE_CRYPTO_ASYM_XFORM_RSA_SIGN,
>        RTE_CRYPTO_ASYM_XFORM_RSA_VERIFY,
>}
>
>struct rte_crypto_asym_xform rsa {
>        enum rte_crypto_asym_op_rsa_types type;
>        union {
>                struct rte_crypto_asym_xform_rsa_encrypt_data encrypt;
>                struct rte_crypto_asym_xform_rsa_decrypt_data decrypt;
>                etc...
>        } data;
>};
>
>The need for xform sub-type data structure would be dependent on whether
>there was immutable data associated with a particular transform. Also if
>we take this approach it would allow each operation type to be separated
>out into there own headers.
>

[Shally] I'm assuming you are not suggesting any changes to existing fundamental rte_crypto_asym_op and rte_crypto_asym_xform structures here, as they're 
defined the way you mentioned.  Only change I see having a new xform sub-type in per-xform struct.
Since we already have an enum rte_crypto_asym_op_type {
RTE_CRYPTO_ASYM_OP_ENCRYPT,
RTE_CRYPTO_ASYM_OP_SIGN
RTE_CRYPTO_ASYM_OP_VERIFY
RTE_CRYPTO_ASYM_OP_KEY_PAIR_GENERATE and so on...
}
So, we can use existing one into asym_xx_xform structure so every rte_crypto_asym_xx_xform struct would look like:

struct rte_crypto_xxx_xform {
	enum rte_crypto_asym_op_type;
	/**< Sign/Verify/Encrypt/Decrypt/key generate/ */

	/* other xform params such as public and private key */
}

Since every xform returns bitmask of op_types supported in capability structure. So, app would know which op types are supported on xform.
Also, I don’t see a need a new define such as OP_RSA/OP_DSA as every op come associated with session/xform with same information, so relevant xform op params will be referenced based on that.

Thanks
Shally

>>> ....
>>>
>>>
>>>> + */
>>>> +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?
>>
>The sess_type=RTE_CRYPTO_OP_WITH_SESSION/RTE_CRYPTO_OP_SESSIONLESS
>should be set in the outer crypto_op struct, so if the PMD doesn't
>support the selected sess_type, it should just fail the enqueue and
>possibly log an error.
>
>> Thanks for review.
>> Shally
>>
>>>          __extension__
>>>
>>>
>>>> +#ifdef __cplusplus
>>>> +}
>>>> +#endif
>>>> +
>>>> +#endif /* _RTE_CRYPTO_ASYM_H_ */
>>>>
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-07-12 18:16 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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