Hi Declan,
Some comments inline.
On Fri, Oct 20, 2017 at 10:21:11PM +0100, Declan Doherty wrote:
> 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.dohe...@intel.com>
> ---
> lib/librte_cryptodev/rte_cryptodev.h | 8 +-
> lib/librte_cryptodev/rte_cryptodev_pmd.c | 169
> +++++++++++++++++++++++++
> lib/librte_cryptodev/rte_cryptodev_pmd.h | 88 +++++++++++++
> lib/librte_cryptodev/rte_cryptodev_version.map | 3 +
> 4 files changed, 264 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_cryptodev/rte_cryptodev.h
> b/lib/librte_cryptodev/rte_cryptodev.h
> index fd0e3f1..86257b0 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -60,10 +60,10 @@ extern const char **rte_cyptodev_names;
> RTE_FMT("%s() line %u: " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \
> __func__, __LINE__, RTE_FMT_TAIL(__VA_ARGS__,)))
>
> -#define CDEV_PMD_LOG_ERR(dev, ...) \
> - RTE_LOG(ERR, CRYPTODEV, \
> - RTE_FMT("[%s] %s() line %u: " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \
> - dev, __func__, __LINE__, RTE_FMT_TAIL(__VA_ARGS__,)))
> +#define CDEV_LOG_INFO(...) \
> + RTE_LOG(INFO, CRYPTODEV, \
> + RTE_FMT("%s() line %u: " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \
> + __func__, __LINE__, RTE_FMT_TAIL(__VA_ARGS__,)))
>
> #ifdef RTE_LIBRTE_CRYPTODEV_DEBUG
> #define CDEV_LOG_DEBUG(...) \
> diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.c
> b/lib/librte_cryptodev/rte_cryptodev_pmd.c
> index a57faad..6cb4419 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_pmd.c
> +++ b/lib/librte_cryptodev/rte_cryptodev_pmd.c
> @@ -40,6 +40,175 @@
> * Parse name from argument
> */
> static int
> +rte_cryptodev_pmd_parse_name_arg(const char *key __rte_unused,
> + const char *value, void *extra_args)
> +{
> + struct rte_cryptodev_pmd_init_params *params = extra_args;
> +
> + if (strlen(value) >= RTE_CRYPTODEV_NAME_MAX_LEN - 1) {
> + CDEV_LOG_ERR("Invalid name %s, should be less than "
> + "%u bytes", value,
> + RTE_CRYPTODEV_NAME_MAX_LEN - 1);
> + return -1;
> + }
> +
> + strncpy(params->name, value, RTE_CRYPTODEV_NAME_MAX_LEN);
Would strcpy() do here? At this point we already know that name will
fit into params->name.
> +
> + return 0;
> +}
> +
> +/**
> + * Parse integer from argument
> + */
> +static int
> +rte_cryptodev_pmd_parse_integer_arg(const char *key __rte_unused,
> + const char *value, void *extra_args)
> +{
> + int *i = extra_args;
> +
> + *i = atoi(value);
> + if (*i < 0) {
> + CDEV_LOG_ERR("Argument has to be positive.");
Perhaps s/positive/non-negative ?
Number 0 looks like a valid argument.
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +int
> +rte_cryptodev_pmd_parse_input_args(
> + struct rte_cryptodev_pmd_init_params *params,
> + const char *args)
> +{
> + struct rte_kvargs *kvlist = NULL;
> + int ret = 0;
> +
> + if (params == NULL)
> + return -EINVAL;
> +
> + if (args) {
> + kvlist = rte_kvargs_parse(args, cryptodev_pmd_valid_params);
> + if (kvlist == NULL)
> + return -EINVAL;
> +
> + ret = rte_kvargs_process(kvlist,
> + RTE_CRYPTODEV_PMD_MAX_NB_QP_ARG,
> + &rte_cryptodev_pmd_parse_integer_arg,
> + ¶ms->max_nb_queue_pairs);
> + if (ret < 0)
> + goto free_kvlist;
> +
> + ret = rte_kvargs_process(kvlist,
> + RTE_CRYPTODEV_PMD_MAX_NB_SESS_ARG,
> + &rte_cryptodev_pmd_parse_integer_arg,
> + ¶ms->max_nb_sessions);
> + if (ret < 0)
> + goto free_kvlist;
> +
> + ret = rte_kvargs_process(kvlist,
> + RTE_CRYPTODEV_PMD_SOCKET_ID_ARG,
> + &rte_cryptodev_pmd_parse_integer_arg,
> + ¶ms->socket_id);
> + if (ret < 0)
> + goto free_kvlist;
> +
> + ret = rte_kvargs_process(kvlist,
> + RTE_CRYPTODEV_PMD_NAME_ARG,
> + &rte_cryptodev_pmd_parse_name_arg,
> + params);
> + if (ret < 0)
> + goto free_kvlist;
> + }
> +
> +free_kvlist:
> + rte_kvargs_free(kvlist);
> + return ret;
> +}
> +
> +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);
> +
> + /* allocate device structure */
> + cryptodev = rte_cryptodev_pmd_allocate(name, params->socket_id);
> + if (cryptodev == NULL) {
> + CDEV_LOG_ERR("[%s] Failed to allocate crypto device for %s",
> + device->driver->name, name);
> + return NULL;
> + }
> +
> + /* allocate private device structure */
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> + cryptodev->data->dev_private =
> + rte_zmalloc_socket("cryptodev device private",
> + params->private_data_size,
> + RTE_CACHE_LINE_SIZE,
> + params->socket_id);
> +
> + if (cryptodev->data->dev_private == NULL) {
> + CDEV_LOG_ERR("[%s] Cannot allocate memory for "
> + "cryptodev %s private data",
> + device->driver->name, name);
> +
> + rte_cryptodev_pmd_release_device(cryptodev);
> + return NULL;
> + }
> + }
> +
> + cryptodev->device = device;
> +
> + /* initialise user call-back tail queue */
> + TAILQ_INIT(&(cryptodev->link_intr_cbs));
> +
> + return cryptodev;
> +}
> +
> +
> +
> +int
> +rte_cryptodev_pmd_destroy(struct rte_cryptodev *cryptodev)
> +{
> + CDEV_LOG_INFO("Closing crypto device %s [%s]", cryptodev->data->name,
> + cryptodev->device->name);
> +
> + /* free crypto device */
> + rte_cryptodev_pmd_release_device(cryptodev);
> +
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> + rte_free(cryptodev->data->dev_private);
> +
> +
> + cryptodev->device = NULL;
> + cryptodev->data = NULL;
> +
> + return 0;
> +}
> +
> +/**
> + * Parse name from argument
> + */
> +static int
> rte_cryptodev_vdev_parse_name_arg(const char *key __rte_unused,
> const char *value, void *extra_args)
> {
> diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> index ba074e1..be9eb80 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> +++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> @@ -56,6 +56,35 @@ extern "C" {
> #include "rte_crypto.h"
> #include "rte_cryptodev.h"
>
> +
> +#define RTE_CRYPTODEV_PMD_DEFAULT_MAX_NB_QUEUE_PAIRS 8
> +#define RTE_CRYPTODEV_PMD_DEFAULT_MAX_NB_SESSIONS 2048
> +
> +#define RTE_CRYPTODEV_PMD_NAME_ARG ("name")
> +#define RTE_CRYPTODEV_PMD_MAX_NB_QP_ARG
> ("max_nb_queue_pairs")
> +#define RTE_CRYPTODEV_PMD_MAX_NB_SESS_ARG ("max_nb_sessions")
> +#define RTE_CRYPTODEV_PMD_SOCKET_ID_ARG ("socket_id")
> +
> +
> +static const char * const cryptodev_pmd_valid_params[] = {
> + RTE_CRYPTODEV_PMD_NAME_ARG,
> + RTE_CRYPTODEV_PMD_MAX_NB_QP_ARG,
> + RTE_CRYPTODEV_PMD_MAX_NB_SESS_ARG,
> + RTE_CRYPTODEV_PMD_SOCKET_ID_ARG
> +};
> +
> +/**
> + * @internal
> + * Initialisation parameters for crypto devices
> + */
> +struct rte_cryptodev_pmd_init_params {
> + char name[RTE_CRYPTODEV_NAME_MAX_LEN];
> + size_t private_data_size;
> + int socket_id;
> + unsigned int max_nb_queue_pairs;
> + unsigned int max_nb_sessions;
> +};
> +
> /** Global structure used for maintaining state of allocated crypto devices
> */
> struct rte_cryptodev_global {
> struct rte_cryptodev *devs; /**< Device information array */
> @@ -392,6 +421,65 @@ rte_cryptodev_pmd_allocate(const char *name, int
> socket_id);
> extern int
> rte_cryptodev_pmd_release_device(struct rte_cryptodev *cryptodev);
>
> +
> +/**
> + * @internal
> + *
> + * PMD assist function to parse initialisation arguments for crypto driver
> + * when creating a new crypto PMD device instance.
> + *
> + * PMD driver should set default values for that PMD before calling function,
> + * these default values will be over-written with successfully parsed values
> + * from args string.
> + *
> + * @param params parsed PMD initialisation parameters
> + * @param args input argument string to parse
> + *
> + * @return
> + * - 0 on success
> + * - errno on failure
> + */
> +int
> +rte_cryptodev_pmd_parse_input_args(
> + struct rte_cryptodev_pmd_init_params *params,
> + const char *args);
> +
> +/**
> + * @internal
> + *
> + * PMD assist function to provide boiler plate code for crypto driver to
> create
> + * and allocate resources for a new 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
> + */
> +struct rte_cryptodev *
> +rte_cryptodev_pmd_create(const char *name,
> + struct rte_device *device,
> + struct rte_cryptodev_pmd_init_params *params);
> +
> +/**
> + * @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);
> +
> /**
> * Executes all the user application registered callbacks for the specific
> * device.
> diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map
> b/lib/librte_cryptodev/rte_cryptodev_version.map
> index 919b6cc..a0ea7bf 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_version.map
> +++ b/lib/librte_cryptodev/rte_cryptodev_version.map
> @@ -84,5 +84,8 @@ DPDK_17.11 {
> global:
>
> rte_cryptodev_name_get;
> + rte_cryptodev_pmd_create;
> + rte_cryptodev_pmd_destroy;
> + rte_cryptodev_pmd_parse_input_args;
>
> } DPDK_17.08;
> --
> 2.9.4
>
--
- Tomasz Duszyński
