Hi Shally,
> -----Original Message-----
> From: Shally Verma [mailto:shally.ve...@caviumnetworks.com]
> Sent: Wednesday, May 16, 2018 7:05 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com>
> Cc: Trahe, Fiona <fiona.tr...@intel.com>; akhil.go...@nxp.com;
> dev@dpdk.org; pathr...@caviumnetworks.com; Sunila Sahu
> <sunila.s...@caviumnetworks.com>; Ashish Gupta
> <ashish.gu...@caviumnetworks.com>
> Subject: [PATCH v3 1/6] lib/cryptodev: add asymmetric algos in cryptodev
>
> 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.ve...@caviumnetworks.com>
> Signed-off-by: Sunila Sahu <sunila.s...@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gu...@caviumnetworks.com>
I have some comments below, but apart from those, could you send a rebased
version of this API?
Thanks,
Pablo
> ---
> lib/librte_cryptodev/Makefile | 2 +-
> lib/librte_cryptodev/meson.build | 3 +-
> lib/librte_cryptodev/rte_crypto_asym.h | 519
> +++++++++++++++++++++++++++++++++
> 3 files changed, 522 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_cryptodev/Makefile b/lib/librte_cryptodev/Makefile
> index
> bba8dee9f..138e627dc 100644
> --- a/lib/librte_cryptodev/Makefile
> +++ b/lib/librte_cryptodev/Makefile
> @@ -23,7 +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
Leave the blank space that was present before.
> # versioning export map
> EXPORT_MAP := rte_cryptodev_version.map
>
...
> --- /dev/null
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
...
> +
> +#include <string.h>
> +#include <stdint.h>
Leave a blank space between non-DPDK and DPDK libraries.
> +#include <rte_memory.h>
> +#include <rte_mempool.h>
> +#include <rte_common.h>
> +
...
> +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;
> +
Needs RTE_STD_C11/extension, before the union.
> + 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 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 */
> +
Looking at the xform structure, it looks like a chain of xforms is possible.
Looking at this union, this case wouldn't be possible, as only one item from
the union can be set.
> + __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;
> +
> +/**
> + * Reset the fields of an asymmetric operation to their default values.
> + *
> + * @param op The crypto operation to be reset.
> + */
> +static inline void
> +__rte_crypto_asym_op_reset(struct rte_crypto_asym_op *op) {
> + memset(op, 0, sizeof(*op));
> +}
> +
> +/**
> + * Attach a session to an asymmetric crypto operation
> + *
> + * @param asym_op crypto operation
> + * @param sess cryptodev session
> + */
> +static inline int
> +__rte_crypto_op_attach_asym_session(struct rte_crypto_asym_op *asym_op,
> + struct rte_cryptodev_asym_session *sess) {
> + asym_op->session = sess;
> + return 0;
> +}
I think we should get rid of these two functions, as they are just one line,
used just once in the code.
I know it is also done in symmetric, I think it can be removed from there too.
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_CRYPTO_ASYM_H_ */
> --
> 2.14.3
