Re: [dpdk-dev] [PATCH] cryptodev: add capabilities discovery mechanism

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

Hi Declan,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
> Sent: Saturday, January 30, 2016 1:11 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] cryptodev: add capabilities discovery
> mechanism

[...]

> --- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> +++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> @@ -572,6 +572,7 @@ aesni_mb_pmd_dequeue_burst(void *queue_pair,
>  }
> 
> 
> +

Remove this blank line.

>  static int cryptodev_aesni_mb_uninit(const char *name);
> 
>  static int
> @@ -622,6 +623,24 @@ cryptodev_aesni_mb_create(const char *name,
> unsigned socket_id)

[...]

> diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
> b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
> index 96d22f6..368a803 100644
> --- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
> +++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
> @@ -38,6 +38,87 @@
> 
>  #include "rte_aesni_mb_pmd_private.h"
> 
> +
> +static const struct rte_cryptodev_capabilities aesni_mb_pmd_capabilities[]
> = {
> +	{	/* MD5 HMAC */
> +		.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
> +		.sym = {
> +			.type = RTE_CRYPTO_XFORM_AUTH,
> +			.auth = {
> +				RTE_CRYPTO_AUTH_MD5_HMAC, 64, 16, 12,
> { 0 }

I would use the variable names to set the values,
it will be longer but I find it more readable.

> +			}
> +		}

[...]

> diff --git a/lib/librte_cryptodev/rte_cryptodev.h
> b/lib/librte_cryptodev/rte_cryptodev.h
> index 892375d..61a162b 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -91,12 +91,131 @@ enum rte_cryptodev_type {
>  #define CDEV_PMD_TRACE(fmt, args...)
>  #endif
> 
> +
> +/**
> + * Symmetric Crypto Capability
> + */
> +struct rte_cryptodev_symmetric_capability {
> +	enum rte_crypto_xform_type type;
> +	/**< Transform type : Authentication / Cipher */
> +
> +	union {
> +		struct {
> +			enum rte_crypto_auth_algorithm algo;
> +			/**< authentication algorithm */
> +			uint16_t block;

Suggest to rename most variables to x_size (i.e. block_size).

> +			/**< algorithm block size */
> +			uint16_t key;
> +			/**< Key size supported */


In cipher structure, key size is a structure supporting a range of sizes,
authentication key does not need it?

> +			uint16_t digest;
> +			/**< Maximum digest size supported */
> +			struct {
> +				uint16_t min;	/**< minimum aad size */
> +				uint16_t max;	/**< maximum aad size */
> +				uint16_t increment;
> +				/**< if a range of sizes are supported,
> +				 * this parameter is used to indicate
> +				 * increments in byte size that are supported
> +				 * between the minimum and maximum */
> +			} aad;

[...]

> +/**
> + * Get the name of a crypto device feature flag
> + *
> + * @param	mask	The mask describing the flag.

Wrong parameter name.

> + *
> + * @return
> + *   The name of this flag, or NULL if it's not a valid feature flag.
> + */
> +
> +extern const char *
> +rte_cryptodev_get_feature_name(uint64_t flag);
> +

[...]

> diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map
> b/lib/librte_cryptodev/rte_cryptodev_version.map
> index ff8e93d..9cd12cf 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_version.map
> +++ b/lib/librte_cryptodev/rte_cryptodev_version.map
> @@ -10,6 +10,7 @@ DPDK_2.2 {
>  	rte_cryptodev_configure;
>  	rte_cryptodev_create_vdev;
>  	rte_cryptodev_get_dev_id;
> +	rte_cryptodev_get_feature_name;
>  	rte_cryptodev_info_get;
>  	rte_cryptodev_pmd_allocate;
>  	rte_cryptodev_pmd_callback_process;
> @@ -27,6 +28,5 @@ DPDK_2.2 {
>  	rte_cryptodev_queue_pair_setup;
>  	rte_cryptodev_queue_pair_start;
>  	rte_cryptodev_queue_pair_stop;
> -

No need to remove the blank line here.

>  	local: *;
>  };
> \ No newline at end of file
> --
> 2.5.0