Re: [dpdk-dev] [PATCH 1/4] cryptodev: Initial DPDK Crypto APIs and device framework release

[Neil Horman <nhorman@tuxdriver.com>]

On Thu, Aug 20, 2015 at 03:07:20PM +0100, Declan Doherty wrote:
> Co-authored-by: Des O Dea <des.j.o.dea@intel.com>
> Co-authored-by: John Griffin <john.griffin@intel.com>
> Co-authored-by: Fiona Trahe <fiona.trahe@intel.com>
> 
> This patch contains the initial proposed APIs and device framework for
> integrating crypto packet processing into DPDK.
> 
> features include:
>  - Crypto device configuration / management APIs
>  - Definitions of supported cipher algorithms and operations.
>  - Definitions of supported hash/authentication algorithms and
>    operations.
>  - Crypto session management APIs
>  - Crypto operation data structures and APIs allocation of crypto
>    operation structure used to specify the crypto operations to
>    be performed  on a particular mbuf.
>  - Extension of mbuf to contain crypto operation data pointer and
>    extra flags.
>  - Burst enqueue / dequeue APIs for processing of crypto operations.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

Hey, only had a qick read so some of this might be off base, but a few comments
in line

><snip>
> index 0000000..b776609
> --- /dev/null
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -0,0 +1,649 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2015 Intel Corporation. All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _RTE_CRYPTO_H_
> +#define _RTE_CRYPTO_H_
> +
> +/**
> + * @file rte_crypto.h
> + *
> + * RTE Cryptographic Definitions
> + *
> + * Defines symmetric cipher and authentication algorithms and modes, as well
> + * as supported symmetric crypto operation combinations.
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <rte_mbuf.h>
> +#include <rte_memory.h>
> +#include <rte_mempool.h>
> +
> +/**
> + * This enumeration lists different types of crypto operations supported by rte
> + * crypto devices. The operation type is defined during session registration and
> + * cannot be changed for a session once it has been setup, or if using a
> + * session-less crypto operation it is defined within the crypto operation
> + * op_params.
> + */
> +enum rte_crypto_operation_chain {
> +	RTE_CRYPTO_SYM_OP_CIPHER_ONLY,
> +	/**< Cipher only operation on the data */
> +	RTE_CRYPTO_SYM_OP_HASH_ONLY,
> +	/**< Hash only operation on the data */
> +	RTE_CRYPTO_SYM_OPCHAIN_HASH_CIPHER,
> +	/**<
> +	 * Chain a hash followed by any cipher operation.
> +	 *
> +	 * If it is required that the result of the hash (i.e. the digest)
> +	 * is going to be included in the data to be ciphered, then:
> +	 *
> +	 * - The digest MUST be placed in the destination buffer at the
> +	 *   location corresponding to the end of the data region to be hashed
> +	 *   (hash_start_offset + message length to hash),  i.e. there must be
> +	 *   no gaps between the start of the digest and the end of the data
> +	 *   region to be hashed.
> +	 *
> +	 * - The message length to cipher member of the rte_crypto_op_data
> +	 *   structure must be equal to the overall length of the plain text,
> +	 *   the digest length and any (optional) trailing data that is to be
> +	 *   included.
> +	 *
> +	 * - The message length to cipher must be a multiple to the block
> +	 *   size if a block cipher is being used - the implementation does not
> +	 *   pad.
> +	 */
> +	RTE_CRYPTO_SYM_OPCHAIN_CIPHER_HASH,
> +	/**<
> +	 * Chain any cipher followed by any hash operation.The hash operation
> +	 * will be performed on the ciphertext resulting from the cipher
> +	 * operation.
> +	 */
> +};

So, this seems a bit...not sure what the word is..specific perhaps?  For an API.
That is to say, if a underlying device supports performing multiple operations
in a single transaction, I'm not sure that should be exposed in this way.  As
the number of devices and chain combinations grow, so too will this list, and if
there are multiple simmilar (but distinct) chain operations, an application will
have to know which chains are applicable to which devices which is sub-optimal

Instead, perhaps it would be better to simply ennumerate the list of crypto
primitives that a device supports (HASH/CIPHER/etc), and allow the application
to define the desired chain when creating a session.  That is to say, an
appilcation can create a session that requests a given chain of operations
(using an array of the primitive enum perhaps).  The implementing PMD is then
responsible for implementing that chain in hardware or software if need be.  If
you need to report on the disposition of the implementation, you can do so via
return code (i.e. SESSION_OK_HW_IMPL or SESSION_OK_SW_IMPL).  

Using this method, more complex operations can be implemented with chain macros
(e.g. AEAD is just an array of primitives [CIPHER, HASH])

><snip>
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
> new file mode 100644
> index 0000000..a1797ce
> --- /dev/null
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -0,0 +1,966 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2015 Intel Corporation. All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <sys/types.h>
> +#include <sys/queue.h>
> +#include <ctype.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdarg.h>
> +#include <errno.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +#include <netinet/in.h>
> +
> +#include <rte_byteorder.h>
> +#include <rte_log.h>
> +#include <rte_debug.h>
> +#include <rte_dev.h>
> +#include <rte_interrupts.h>
> +#include <rte_pci.h>
> +#include <rte_memory.h>
> +#include <rte_memcpy.h>
> +#include <rte_memzone.h>
> +#include <rte_launch.h>
> +#include <rte_tailq.h>
> +#include <rte_eal.h>
> +#include <rte_per_lcore.h>
> +#include <rte_lcore.h>
> +#include <rte_atomic.h>
> +#include <rte_branch_prediction.h>
> +#include <rte_common.h>
> +#include <rte_ring.h>
> +#include <rte_mempool.h>
> +#include <rte_malloc.h>
> +#include <rte_mbuf.h>
> +#include <rte_errno.h>
> +#include <rte_spinlock.h>
> +#include <rte_string_fns.h>
> +
> +#include "rte_crypto.h"
> +#include "rte_cryptodev.h"
> +
> +
> +
> +/* Macros to check for invalid function pointers in dev_ops structure */
> +#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
> +	if ((func) == NULL) { \
> +		CDEV_LOG_ERR("Function not supported"); \
> +		return retval; \
> +	} \
> +} while (0)
> +
> +#define PROC_PRIMARY_OR_RET() do { \
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> +		CDEV_LOG_ERR("Cannot run in secondary processes"); \
> +		return; \
> +	} \
> +} while (0)
> +
> +#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> +		CDEV_LOG_ERR("Cannot run in secondary processes"); \
> +		return retval; \
> +	} \
> +} while (0)
> +
> +#define FUNC_PTR_OR_RET(func) do { \
> +	if ((func) == NULL) { \
> +		CDEV_LOG_ERR("Function not supported"); \
> +		return; \
> +	} \
> +} while (0)
> +
These are all defined in rte_ethdev.  You should just move those to a public
place rather then re-creating them

> +struct rte_cryptodev rte_crypto_devices[RTE_MAX_CRYPTODEVS];
> +
> +static struct rte_cryptodev_global cryptodev_globals = {
> +		.devs			= &rte_crypto_devices[0],
> +		.data			= NULL,
> +		.nb_devs		= 0,
> +		.max_devs		= RTE_MAX_CRYPTODEVS
> +};
> +
> +struct rte_cryptodev_global *rte_cryptodev_globals = &cryptodev_globals;
> +
> +/* spinlock for crypto device callbacks */
> +static rte_spinlock_t rte_cryptodev_cb_lock = RTE_SPINLOCK_INITIALIZER;
> +
> +
> +/**
> + * The user application callback description.
> + *
> + * It contains callback address to be registered by user application,
> + * the pointer to the parameters for callback, and the event type.
> + */
> +struct rte_cryptodev_callback {
> +	TAILQ_ENTRY(rte_cryptodev_callback) next; /**< Callbacks list */
> +	rte_cryptodev_cb_fn cb_fn;                /**< Callback address */
> +	void *cb_arg;                           /**< Parameter for callback */
> +	enum rte_cryptodev_event_type event;          /**< Interrupt event type */
> +	uint32_t active;                        /**< Callback is executing */
> +};
> +
> +int
> +rte_cryptodev_create_vdev(const char *name, const char *args)
> +{
> +	return rte_eal_vdev_init(name, args);
> +}
> +
> +
> +static inline void
> +rte_cryptodev_data_alloc(int socket_id)
> +{
> +	const unsigned flags = 0;
> +	const struct rte_memzone *mz;
> +
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +		mz = rte_memzone_reserve("rte_cryptodev_data",
> +				cryptodev_globals.max_devs * sizeof(struct rte_cryptodev_data),
> +				socket_id, flags);
> +	} else
> +		mz = rte_memzone_lookup("rte_cryptodev_data");
> +	if (mz == NULL)
> +		rte_panic("Cannot allocate memzone for the crypto device data");
> +
> +	cryptodev_globals.data = mz->addr;
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +		memset(cryptodev_globals.data, 0,
> +				cryptodev_globals.max_devs * sizeof(struct rte_cryptodev_data));
> +}
> +
> +
> +static uint8_t
> +rte_cryptodev_find_free_device_index(void)
> +{
> +	uint8_t dev_id;
> +
> +	for (dev_id = 0; dev_id < RTE_MAX_CRYPTODEVS; dev_id++) {
> +		if (rte_crypto_devices[dev_id].attached == RTE_CRYPTODEV_DETACHED)
> +			return dev_id;
> +	}
> +	return RTE_MAX_CRYPTODEVS;
> +}
> +
> +struct rte_cryptodev *
> +rte_cryptodev_pmd_allocate(const char *name, enum pmd_type type, int socket_id)
> +{
> +	uint8_t dev_id;
> +	struct rte_cryptodev *cryptodev;
> +
> +	dev_id = rte_cryptodev_find_free_device_index();
> +	if (dev_id == RTE_MAX_CRYPTODEVS) {
> +		CDEV_LOG_ERR("Reached maximum number of crypto devices");
> +		return NULL;
> +	}
> +
> +	if (cryptodev_globals.data == NULL)
> +		rte_cryptodev_data_alloc(socket_id);
> +
> +	if (rte_cryptodev_pmd_get_named_dev(name) != NULL) {
> +		CDEV_LOG_ERR("Crypto device with name %s already "
> +				"allocated!", name);
> +		return NULL;
> +	}
> +
> +	cryptodev = rte_cryptodev_pmd_get_dev(dev_id);
> +	cryptodev->data = &cryptodev_globals.data[dev_id];
> +	snprintf(cryptodev->data->name, RTE_CRYPTODEV_NAME_MAX_LEN, "%s", name);
> +	cryptodev->data->dev_id = dev_id;
> +	cryptodev->attached = RTE_CRYPTODEV_ATTACHED;
> +	cryptodev->pmd_type = type;
> +	cryptodev_globals.nb_devs++;
> +
> +	return cryptodev;
> +}
> +
> +static inline int
> +rte_cryptodev_create_unique_device_name(char *name, size_t size,
> +		struct rte_pci_device *pci_dev)
> +{
> +	int ret;
> +
> +	if ((name == NULL) || (pci_dev == NULL))
> +		return -EINVAL;
> +
> +	ret = snprintf(name, size, "%d:%d.%d",
> +			pci_dev->addr.bus, pci_dev->addr.devid,
> +			pci_dev->addr.function);
> +	if (ret < 0)
> +		return ret;
> +	return 0;
> +}
> +
> +int
> +rte_cryptodev_pmd_release_device(struct rte_cryptodev *cryptodev)
> +{
> +	if (cryptodev == NULL)
> +		return -EINVAL;
> +
> +	cryptodev->attached = RTE_CRYPTODEV_DETACHED;
> +	cryptodev_globals.nb_devs--;
> +	return 0;
> +}
> +
> +struct rte_cryptodev *
> +rte_cryptodev_pmd_virtual_dev_init(const char *name, size_t dev_private_size,
> +		int socket_id)
> +{
> +	struct rte_cryptodev *cryptodev;
> +
> +	/* allocate device structure */
> +	cryptodev = rte_cryptodev_pmd_allocate(name, PMD_VDEV, socket_id);
> +	if (cryptodev == NULL)
> +		return NULL;
> +
> +	/* allocate private device structure */
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +		cryptodev->data->dev_private =
> +				rte_zmalloc("%s private structure",
> +						dev_private_size,
> +						RTE_CACHE_LINE_SIZE);
> +
> +		if (cryptodev->data->dev_private == NULL)
> +			rte_panic("Cannot allocate memzone for private device"
> +					" data");
> +	}
> +
> +	/* initialise user call-back tail queue */
> +	TAILQ_INIT(&(cryptodev->link_intr_cbs));
> +
> +	return cryptodev;
> +}
> +
> +static int
> +rte_cryptodev_init(struct rte_pci_driver *pci_drv,
> +		struct rte_pci_device *pci_dev)
> +{
> +	struct rte_cryptodev_driver *cryptodrv;
> +	struct rte_cryptodev *cryptodev;
> +
> +	char cryptodev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
> +
> +	int retval;
> +
> +	cryptodrv = (struct rte_cryptodev_driver *)pci_drv;
> +	if (cryptodrv == NULL)
> +			return -ENODEV;
> +
> +	/* Create unique Crypto device name using PCI address */
> +	rte_cryptodev_create_unique_device_name(cryptodev_name,
> +			sizeof(cryptodev_name), pci_dev);
> +
> +	cryptodev = rte_cryptodev_pmd_allocate(cryptodev_name, PMD_PDEV, rte_socket_id());
> +	if (cryptodev == NULL)
> +		return -ENOMEM;
> +
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +		cryptodev->data->dev_private =
> +				rte_zmalloc_socket("cryptodev private structure",
> +						cryptodrv->dev_private_size,
> +						RTE_CACHE_LINE_SIZE, rte_socket_id());
> +
> +		if (cryptodev->data->dev_private == NULL)
> +			rte_panic("Cannot allocate memzone for private device data");
> +	}
> +
> +	cryptodev->pci_dev = pci_dev;
> +	cryptodev->driver = cryptodrv;
> +
> +	/* init user callbacks */
> +	TAILQ_INIT(&(cryptodev->link_intr_cbs));
> +
> +	/* Invoke PMD device initialization function */
> +	retval = (*cryptodrv->cryptodev_init)(cryptodrv, cryptodev);
> +	if (retval == 0)
> +		return 0;
> +
> +	CDEV_LOG_ERR("driver %s: crypto_dev_init(vendor_id=0x%u device_id=0x%x)"
> +			" failed", pci_drv->name,
> +			(unsigned) pci_dev->id.vendor_id,
> +			(unsigned) pci_dev->id.device_id);
> +
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +		rte_free(cryptodev->data->dev_private);
> +
> +	cryptodev->attached = RTE_CRYPTODEV_DETACHED;
> +	cryptodev_globals.nb_devs--;
> +
> +	return -ENXIO;
> +}
> +
> +static int
> +rte_cryptodev_uninit(struct rte_pci_device *pci_dev)
> +{
> +	const struct rte_cryptodev_driver *cryptodrv;
> +	struct rte_cryptodev *cryptodev;
> +	char cryptodev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
> +	int ret;
> +
> +	if (pci_dev == NULL)
> +		return -EINVAL;
> +
> +	/* Create unique device name using PCI address */
> +	rte_cryptodev_create_unique_device_name(cryptodev_name,
> +			sizeof(cryptodev_name), pci_dev);
> +
> +	cryptodev = rte_cryptodev_pmd_get_named_dev(cryptodev_name);
> +	if (cryptodev == NULL)
> +		return -ENODEV;
> +
> +	cryptodrv = (const struct rte_cryptodev_driver *)pci_dev->driver;
> +	if (cryptodrv == NULL)
> +			return -ENODEV;
> +
> +	/* Invoke PMD device uninit function */
> +	if (*cryptodrv->cryptodev_uninit) {
> +		ret = (*cryptodrv->cryptodev_uninit)(cryptodrv, cryptodev);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* free ether device */
> +	rte_cryptodev_pmd_release_device(cryptodev);
> +
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +		rte_free(cryptodev->data->dev_private);
> +
> +	cryptodev->pci_dev = NULL;
> +	cryptodev->driver = NULL;
> +	cryptodev->data = NULL;
> +
> +	return 0;
> +}
> +
Shouldn't there be some interlock here if a device is being removed to block on
closure of all the sessions that may be open against it, and serializtion
againsnt any list modifications for tracking of these devices?

> +int
> +rte_cryptodev_pmd_driver_register(struct rte_cryptodev_driver *cryptodrv,
> +		enum pmd_type type)
> +{
> +	/* Call crypto device initialization directly if device is virtual */
> +	if (type == PMD_VDEV)
> +		return rte_cryptodev_init((struct rte_pci_driver *)cryptodrv,
> +				NULL);
> +
> +	/* Register PCI driver for physical device intialisation during
> +	 * PCI probing */
> +	cryptodrv->pci_drv.devinit = rte_cryptodev_init;
> +	cryptodrv->pci_drv.devuninit = rte_cryptodev_uninit;
> +	rte_eal_pci_register(&cryptodrv->pci_drv);
> +	return 0;
> +}
> +
> +int
> +rte_cryptodev_pmd_attach(const char *devargs __rte_unused,
> +			uint8_t *dev_id __rte_unused)
> +{
> +	RTE_LOG(ERR, EAL, "Hotplug support isn't enabled");
> +	return -1;
> +}
> +
> +/* detach the device, then store the name of the device */
> +int
> +rte_cryptodev_pmd_detach(uint8_t dev_id __rte_unused,
> +			char *name __rte_unused)
> +{
> +	RTE_LOG(ERR, EAL, "Hotplug support isn't enabled");
> +	return -1;
> +}
> +
> +uint16_t
> +rte_cryptodev_queue_pair_count(uint8_t dev_id)
> +{
> +	struct rte_cryptodev *dev;
> +
> +	dev = &rte_crypto_devices[dev_id];
> +	return dev->data->nb_queue_pairs;
> +}
> +
> +static int
> +rte_cryptodev_queue_pairs_config(struct rte_cryptodev *dev, uint16_t nb_qpairs, int socket_id)
> +{
> +	struct rte_cryptodev_info dev_info;
> +	uint16_t old_nb_queues = dev->data->nb_queue_pairs;
> +	void **qp;
> +	unsigned i;
> +
> +	if ((dev == NULL) || (nb_qpairs < 1)) {
> +		CDEV_LOG_ERR("invalid param: dev %p, nb_queues %u",
> +							dev, nb_qpairs);
> +		return -EINVAL;
> +	}
> +
> +	CDEV_LOG_DEBUG("Setup %d queues pairs on device %u",
> +			nb_qpairs, dev->data->dev_id);
> +
> +
> +	memset(&dev_info, 0, sizeof(struct rte_cryptodev_info));
> +
> +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
> +	(*dev->dev_ops->dev_infos_get)(dev, &dev_info);
> +
> +	if (nb_qpairs > (dev_info.max_queue_pairs)) {
> +		CDEV_LOG_ERR("Invalid num queue_pairs (%u) for dev %u",
> +				nb_qpairs, dev->data->dev_id);
> +	    return (-EINVAL);
> +	}
> +
> +	if (dev->data->queue_pairs == NULL) { /* first time configuration */
> +		dev->data->queue_pairs = rte_zmalloc_socket(
> +				"cryptodev->queue_pairs",
> +				sizeof(dev->data->queue_pairs[0]) * nb_qpairs,
> +				RTE_CACHE_LINE_SIZE, socket_id);
> +
> +		if (dev->data->queue_pairs == NULL) {
> +			dev->data->nb_queue_pairs = 0;
> +			CDEV_LOG_ERR("failed to get memory for qp meta data, "
> +							"nb_queues %u", nb_qpairs);
> +			return -(ENOMEM);
> +		}
> +	} else { /* re-configure */
> +		FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_pair_release, -ENOTSUP);
> +
> +		qp = dev->data->queue_pairs;
> +
> +		for (i = nb_qpairs; i < old_nb_queues; i++)
> +			(*dev->dev_ops->queue_pair_release)(dev, i);
> +		qp = rte_realloc(qp, sizeof(qp[0]) * nb_qpairs,
> +				RTE_CACHE_LINE_SIZE);
> +		if (qp == NULL) {
> +			CDEV_LOG_ERR("failed to realloc qp meta data,"
> +						" nb_queues %u", nb_qpairs);
> +			return -(ENOMEM);
> +		}
> +		if (nb_qpairs > old_nb_queues) {
> +			uint16_t new_qs = nb_qpairs - old_nb_queues;
> +
> +			memset(qp + old_nb_queues, 0,
> +				sizeof(qp[0]) * new_qs);
> +		}
> +
> +		dev->data->queue_pairs = qp;
> +
> +	}
> +	dev->data->nb_queue_pairs = nb_qpairs;
> +	return 0;
> +}
> +
> +int
> +rte_cryptodev_queue_pair_start(uint8_t dev_id, uint16_t queue_pair_id)
> +{
> +	struct rte_cryptodev *dev;
> +
> +	/* This function is only safe when called from the primary process
> +	 * in a multi-process setup*/
> +	PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> +
> +	if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_crypto_devices[dev_id];
> +	if (queue_pair_id >= dev->data->nb_queue_pairs) {
> +		CDEV_LOG_ERR("Invalid queue_pair_id=%d", queue_pair_id);
> +		return -EINVAL;
> +	}
> +
> +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_pair_start, -ENOTSUP);
> +
> +	return dev->dev_ops->queue_pair_start(dev, queue_pair_id);
> +
> +}
> +
> +int
> +rte_cryptodev_queue_pair_stop(uint8_t dev_id, uint16_t queue_pair_id)
> +{
> +	struct rte_cryptodev *dev;
> +
> +	/* This function is only safe when called from the primary process
> +	 * in a multi-process setup*/
> +	PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> +
> +	if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_crypto_devices[dev_id];
> +	if (queue_pair_id >= dev->data->nb_queue_pairs) {
> +		CDEV_LOG_ERR("Invalid queue_pair_id=%d", queue_pair_id);
> +		return -EINVAL;
> +	}
> +
> +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_pair_stop, -ENOTSUP);
> +
> +	return dev->dev_ops->queue_pair_stop(dev, queue_pair_id);
> +
> +}
> +
So, I'm a bit confused here.  How do you communicate with a cryptodev.  I see
you're creating queue pairs here, which I think are intended for input/output,
but you also allow the creation of sessions.  The former seems to have no
linkage to the latter, so you have sessionless queue pairs and sessions without
method to perform operations on?  I'm clearly missing something, but I can't see
the relationship.