From mboxrd@z Thu Jan 1 00:00:00 1970 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) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=qAJjVJT85gPEQnslFn0aH47mtAdPbiHKalglbmZq5pE=; b=VZSpMiBgQzIHuUU+XchrTcQKLxnJzTqHs5f8/XFzzc4M6x6mex2vGOCkCoO4BnEYUz lxmyQ4LNEoic+hYsTliAzcl2yYXAEruNdkT6WSWqPb9IAOh7kI1oNvlnhF9ugTtjOgm+ Q9BFsExjoC0aoHg5eil1CnNsEKDHcsQKZUyen4P5tEW9YXp2ulEruZHw7ih0ABw8WAl3 62BHSmHQ5YAXQzydjBYQB5b6PZbjk6pTDcsLXmsMNs4BYLWDZWbTPxNtRB9Rikde9Vhx 1Yz4zNyHmKLrLjph0s9+x79VVfw62QBT3yzn8M36Pi4u/2yTraM/wTtqI0FhE/55Nzka 0Cjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=qAJjVJT85gPEQnslFn0aH47mtAdPbiHKalglbmZq5pE=; b=EjaisEoF2z7ERrG64hz8xQ08Eo47dgN/UjMsTOk3gfB5KBn/IT0zUvxArrnrA1zGQO grDQN6ggtBPAlZWGe3LEuy2Ry8Bzvj3P3fWe2PUCVK8tx3SADxGOWhCPmhWBqGcIB6wV +3GPhhpOCukwFAtcx6Pssrwk+FIGqd/ET7QzqDv6hGSHzBqEOzmDqdK5AgLDyrOiQyTM XOhnqbc42AHMppDcpNetndPIiWFMsarNHRlsA/QtOknem49cKv4t/cNawCYpOoy273x4 OwkV4OKD4liU96p3ZRCjrGQ/Jl/7y8UTg80E9p5GJyboYmZzJpEWUc/th2xRDgHMAita +4Dg== X-Gm-Message-State: AMCzsaVErQo1cVC+yjYHdjVFfIRgF5f8VrmL7ATNkvzY0Gs4gI6Zjv6z dpJ0SJimMiEFPomN9K1VFXMug+1o6V4= X-Google-Smtp-Source: ABhQp+Q+mf/iwaCIU+E4l9EOomkdRq56JXsn/YXo/tMqLKGZa8xc8zdYWjQLU9FgOv4ZXLaP1ef3VA== X-Received: by 10.25.202.25 with SMTP id a25mr6173983lfg.83.1508854160754; Tue, 24 Oct 2017 07:09:20 -0700 (PDT) Received: from localhost (31-172-191-173.noc.fibertech.net.pl. [31.172.191.173]) by smtp.gmail.com with ESMTPSA id p75sm75249lfg.56.2017.10.24.07.09.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 24 Oct 2017 07:09:20 -0700 (PDT) Date: Tue, 24 Oct 2017 16:09:19 +0200 From: Tomasz Duszynski To: Declan Doherty Cc: dev@dpdk.org 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-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20171020212113.4543-2-declan.doherty@intel.com> User-Agent: Mutt/1.5.23.1 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH 1/3] cryptodev: add new APIs to assist PMD initialisation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Oct 2017 14:09:21 -0000 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