From: Tomasz Duszynski <tdu@semihalf.com>
To: Declan Doherty <declan.doherty@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/3] cryptodev: add new APIs to assist PMD initialisation
Date: Tue, 24 Oct 2017 16:09:19 +0200 [thread overview]
Message-ID: <20171024140919.GA15441@tdu> (raw)
In-Reply-To: <20171020212113.4543-2-declan.doherty@intel.com>
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.doherty@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
next prev parent reply other threads:[~2017-10-24 14:09 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-20 21:21 [dpdk-dev] [PATCH 0/3] Break dependency on bus infrastructure* Declan Doherty
2017-10-20 21:21 ` [dpdk-dev] [PATCH 1/3] cryptodev: add new APIs to assist PMD initialisation Declan Doherty
2017-10-24 11:03 ` De Lara Guarch, Pablo
2017-10-24 14:09 ` Tomasz Duszynski [this message]
2017-10-25 0:59 ` Gaëtan Rivet
2017-10-20 21:21 ` [dpdk-dev] [PATCH 2/3] cryptodev: break dependency on virtual device bus Declan Doherty
2017-10-24 11:18 ` De Lara Guarch, Pablo
2017-10-24 11:32 ` Akhil Goyal
2017-10-25 6:18 ` Tomasz Duszynski
2017-10-25 7:45 ` De Lara Guarch, Pablo
2017-10-20 21:21 ` [dpdk-dev] [PATCH 3/3] cryptodev: break dependency on rte_pci.h Declan Doherty
2017-10-24 11:23 ` De Lara Guarch, Pablo
2017-10-23 9:21 ` [dpdk-dev] [PATCH 0/3] Break dependency on bus infrastructure* Doherty, Declan
2017-10-25 0:50 ` Gaëtan Rivet
2017-10-25 12:00 ` [dpdk-dev] [PATCH v2 0/3] Break cryptodev dependency on bus infrastructure Declan Doherty
2017-10-25 12:00 ` [dpdk-dev] [PATCH v2 1/3] cryptodev: add new APIs to assist PMD initialisation Declan Doherty
2017-10-25 15:56 ` Trahe, Fiona
2017-10-25 12:00 ` [dpdk-dev] [PATCH v2 2/3] cryptodev: break dependency on virtual device bus Declan Doherty
2017-10-25 12:00 ` [dpdk-dev] [PATCH v2 3/3] cryptodev: break dependency on PCI " Declan Doherty
2017-10-25 15:55 ` Trahe, Fiona
2017-10-25 14:36 ` [dpdk-dev] [PATCH v2 0/3] Break cryptodev dependency on bus infrastructure De Lara Guarch, Pablo
2017-10-25 16:06 ` De Lara Guarch, Pablo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171024140919.GA15441@tdu \
--to=tdu@semihalf.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).