From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Duszynski Subject: Re: [PATCH 1/3] cryptodev: add new APIs to assist PMD initialisation Date: Tue, 24 Oct 2017 16:09:19 +0200 Message-ID: <20171024140919.GA15441@tdu> References: <20171020212113.4543-1-declan.doherty@intel.com> <20171020212113.4543-2-declan.doherty@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org To: Declan Doherty Return-path: Received: from mail-lf0-f68.google.com (mail-lf0-f68.google.com [209.85.215.68]) by dpdk.org (Postfix) with ESMTP id 804EF1B7FB for ; Tue, 24 Oct 2017 16:09:21 +0200 (CEST) Received: by mail-lf0-f68.google.com with SMTP id w21so24206799lfc.6 for ; Tue, 24 Oct 2017 07:09:21 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20171020212113.4543-2-declan.doherty@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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_crypto= dev/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 =3D extra_args; > + > + if (strlen(value) >=3D 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 =3D extra_args; > + > + *i =3D 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 =3D NULL; > + int ret =3D 0; > + > + if (params =3D=3D NULL) > + return -EINVAL; > + > + if (args) { > + kvlist =3D rte_kvargs_parse(args, cryptodev_pmd_valid_params); > + if (kvlist =3D=3D NULL) > + return -EINVAL; > + > + ret =3D 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 =3D 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 =3D 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 =3D 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] !=3D '\0') { > + CDEV_LOG_INFO("[%s] User specified device name =3D %s\n", > + device->driver->name, params->name); > + name =3D 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 =3D rte_cryptodev_pmd_allocate(name, params->socket_id); > + if (cryptodev =3D=3D 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() =3D=3D RTE_PROC_PRIMARY) { > + cryptodev->data->dev_private =3D > + rte_zmalloc_socket("cryptodev device private", > + params->private_data_size, > + RTE_CACHE_LINE_SIZE, > + params->socket_id); > + > + if (cryptodev->data->dev_private =3D=3D 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 =3D 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() =3D=3D RTE_PROC_PRIMARY) > + rte_free(cryptodev->data->dev_private); > + > + > + cryptodev->device =3D NULL; > + cryptodev->data =3D 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_crypto= dev/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[] =3D { > + 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 devi= ces */ > struct rte_cryptodev_global { > struct rte_cryptodev *devs; /**< Device information array */ > @@ -392,6 +421,65 @@ rte_cryptodev_pmd_allocate(const char *name, int soc= ket_id); > extern int > rte_cryptodev_pmd_release_device(struct rte_cryptodev *cryptodev); > > + > +/** > + * @internal > + * > + * PMD assist function to parse initialisation arguments for crypto driv= er > + * when creating a new crypto PMD device instance. > + * > + * PMD driver should set default values for that PMD before calling func= tion, > + * these default values will be over-written with successfully parsed va= lues > + * 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 instan= ce. > + * > + * @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 specif= ic > * 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=C5=84ski