[dpdk-dev] [PATCH 1/3] cryptodev: add new APIs to assist PMD initialisation

[De Lara Guarch, Pablo]

Hi Declan,

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Declan Doherty
> Sent: Friday, October 20, 2017 10:21 PM
> To: dev at dpdk.org
> Cc: Doherty, Declan <declan.doherty at intel.com>
> Subject: [dpdk-dev] [PATCH 1/3] cryptodev: add new APIs to assist PMD
> initialisation
> 
> Adds new PMD assist functions which are bus independent for driver to
> create and destroy new device instances.
> 
> Also includes function to parse parameters which can be passed to driver
> on device initialisation.
> 
> Signed-off-by: Declan Doherty <declan.doherty at intel.com>

...

> +struct rte_cryptodev *
> +rte_cryptodev_pmd_create(const char *name,
> +		struct rte_device *device,
> +		struct rte_cryptodev_pmd_init_params *params) {
> +	struct rte_cryptodev *cryptodev;
> +
> +	if (params->name[0] != '\0') {
> +		CDEV_LOG_INFO("[%s] User specified device name = %s\n",
> +				device->driver->name, params->name);
> +		name = params->name;
> +	}
> +
> +	CDEV_LOG_INFO("[%s] Creating cryptodev %s\n",
> +			device->driver->name, name);
> +
> +	CDEV_LOG_INFO("[%s] Initialisation parameters [ name: %s, "
> +			"private data size:  %zu, "
> +			"socket id: %d,"
> +			"max queue pairs: %u, "
> +			"max sessions: %u ]\n",
> +			device->driver->name, name, params-
> >private_data_size,
> +			params->socket_id, params->max_nb_queue_pairs,
> +			params->max_nb_sessions);

Testing this, I see the following:

CRYPTODEV: rte_cryptodev_pmd_create() line 140: [crypto_aesni_mb]
Creating cryptodev crypto_aesni_mb

CRYPTODEV: rte_cryptodev_pmd_create() line 149: [crypto_aesni_mb]
Initialisation parameters [ name: crypto_aesni_mb, private data size:  12,
socket id: 1,max queue pairs: 8, max sessions: 2048 ]

It looks too long to me. I would remove the first line (containing the line number),
and also the private data size in the initialization parameters,
as I think they are not giving any useful info for a user.

...

> diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h
...

> +/**
> + * @internal
> + *
> + * PMD assist function to provide boiler plate code for crypto driver
> +to
> + * destroy and free resources associated with a crypto PMD device
> instance.
> + *
> + * @param	name	crypto device name.
> + * @param	device	base device instance
> + * @param	params	PMD initialisation parameters
> + *
> + * @return
> + *  - crypto device instance on success
> + *  - NULL on creation failure
> + */
> +int
> +rte_cryptodev_pmd_destroy(struct rte_cryptodev *cryptodev);

Parameters and return value are wrong here.