RE: [PATCH v1 1/3] cryptodev: add queue pair reset API

[Akhil Goyal <gakhil@marvell.com>]

> Subject: [PATCH v1 1/3] cryptodev: add queue pair reset API
> 
> The API will reset the specific queue pair of a cryptodev.
> The current API, cryptodev_queue_pair_setup(), requires the cryptodev
> to be stopped before reconfiguring any queue pair. Stopping the
> cryptodev in one thread can result in a segmentation fault when
> multiple queues are used for enqueue and dequeue operations.
> 
> On supported PMDs, the cryptodev_queue_pair_reset() will
> reconfigure/reset the queue pair without affecting other queues or
> the cryptodev state.
> 
> The caller should ensure that there are no enqueue or dequeue operations
> ongoing on that queue and that there are no inflight packets before
> calling this API.
> 
> Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>

Acked-by: Akhil Goyal <gakhil@marvell.com>

Please update release notes.

> 
> diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
> index 6c114f7181..311ae63abb 100644
> --- a/lib/cryptodev/cryptodev_pmd.h
> +++ b/lib/cryptodev/cryptodev_pmd.h
> @@ -290,6 +290,22 @@ typedef int (*cryptodev_queue_pair_setup_t)(struct
> rte_cryptodev *dev,
>  typedef int (*cryptodev_queue_pair_release_t)(struct rte_cryptodev *dev,
>  		uint16_t qp_id);
> 
> +/**
> + * Reset or reconfigure a queue pair for a device.
> + *
> + * @param	dev		Crypto device pointer
> + * @param	qp_id		Queue pair index
> + * @param	qp_conf		Queue configuration structure
> + * @param	socket_id	Socket index
> + *
> + * @return
> + *  - 0: on success.
> + *  - ENOTSUP: if crypto device does not support the operation.
> + */
> +typedef int (*cryptodev_queue_pair_reset_t)(struct rte_cryptodev *dev,
> +		uint16_t qp_id,	const struct rte_cryptodev_qp_conf *qp_conf,
> +		int socket_id);
> +
>  /**
>   * Create a session mempool to allocate sessions from
>   *
> @@ -476,6 +492,8 @@ struct rte_cryptodev_ops {
>  	/**< Set up a device queue pair. */
>  	cryptodev_queue_pair_release_t queue_pair_release;
>  	/**< Release a queue pair. */
> +	cryptodev_queue_pair_reset_t queue_pair_reset;
> +	/**< Reset a queue pair. */
> 
>  	cryptodev_sym_get_session_private_size_t sym_session_get_size;
>  	/**< Return private session. */
> diff --git a/lib/cryptodev/cryptodev_trace.h b/lib/cryptodev/cryptodev_trace.h
> index 935f0d564b..633f17afe1 100644
> --- a/lib/cryptodev/cryptodev_trace.h
> +++ b/lib/cryptodev/cryptodev_trace.h
> @@ -58,6 +58,16 @@ RTE_TRACE_POINT(
>  	rte_trace_point_emit_ptr(conf->mp_session);
>  )
> 
> +RTE_TRACE_POINT(
> +	rte_cryptodev_trace_queue_pair_reset,
> +	RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t queue_pair_id,
> +		const struct rte_cryptodev_qp_conf *conf, int socket_id),
> +	rte_trace_point_emit_u8(dev_id);
> +	rte_trace_point_emit_u16(queue_pair_id);
> +	rte_trace_point_emit_u32(conf->nb_descriptors);
> +	rte_trace_point_emit_int(socket_id);
> +)
> +
>  RTE_TRACE_POINT(
>  	rte_cryptodev_trace_sym_session_pool_create,
>  	RTE_TRACE_POINT_ARGS(const char *name, uint32_t nb_elts,
> diff --git a/lib/cryptodev/cryptodev_trace_points.c
> b/lib/cryptodev/cryptodev_trace_points.c
> index 7403412553..6f37780595 100644
> --- a/lib/cryptodev/cryptodev_trace_points.c
> +++ b/lib/cryptodev/cryptodev_trace_points.c
> @@ -21,6 +21,9 @@ RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_close,
>  RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_setup,
>  	lib.cryptodev.queue.pair.setup)
> 
> +RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_reset,
> +	lib.cryptodev.queue.pair.reset)
> +
>  RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_pool_create,
>  	lib.cryptodev.sym.pool.create)
> 
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index 682c9f49d0..281ca51cf3 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -1222,6 +1222,30 @@ rte_cryptodev_queue_pairs_config(struct
> rte_cryptodev *dev, uint16_t nb_qpairs,
>  	return 0;
>  }
> 
> +int
> +rte_cryptodev_queue_pair_reset(uint8_t dev_id, uint16_t queue_pair_id,
> +		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id)
> +{
> +	struct rte_cryptodev *dev;
> +
> +	if (!rte_cryptodev_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;
> +	}
> +
> +	if (*dev->dev_ops->queue_pair_reset == NULL)
> +		return -ENOTSUP;
> +
> +	rte_cryptodev_trace_queue_pair_reset(dev_id, queue_pair_id, qp_conf,
> socket_id);
> +	return (*dev->dev_ops->queue_pair_reset)(dev, queue_pair_id, qp_conf,
> socket_id);
> +}
> +
>  int
>  rte_cryptodev_configure(uint8_t dev_id, struct rte_cryptodev_config *config)
>  {
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index bec947f6d5..e0fc35db2a 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -840,6 +840,35 @@ int
>  rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
>  		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Reset a queue pair for a device.
> + * The caller of this API must ensure that, there are no enqueues to the queue
> and there are no
> + * pending/inflight packets in the queue when the API is called.
> + * The API can reconfigure the queue pair when the queue pair configuration
> data is provided.
> + *
> + * @param	dev_id		The identifier of the device.
> + * @param	queue_pair_id	The index of the queue pairs to set up. The
> value must be in the
> + *				range [0, nb_queue_pair - 1] previously supplied
> to
> + *				rte_cryptodev_configure().
> + * @param	qp_conf		The pointer to configuration data to be used for
> the queue pair.
> + *				It should be NULL, if the API is called from an
> interrupt context.
> + * @param	socket_id	The *socket_id* argument is the socket
> identifier in case of NUMA.
> + *				The value can be *SOCKET_ID_ANY* if there is
> no NUMA constraint
> + *				for the DMA memory allocated for the queue
> pair.
> + *
> + * @return
> + *   - 0:  Queue pair is reset successfully.
> + *   - ENOTSUP: If the operation is not supported by the PMD.
> + *   - <0: Queue pair reset failed
> + */
> +__rte_experimental
> +int
> +rte_cryptodev_queue_pair_reset(uint8_t dev_id, uint16_t queue_pair_id,
> +		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
> +
>  /**
>   * Get the status of queue pairs setup on a specific crypto device
>   *
> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> index fdac0d876e..eec06d9939 100644
> --- a/lib/cryptodev/version.map
> +++ b/lib/cryptodev/version.map
> @@ -87,6 +87,9 @@ EXPERIMENTAL {
> 
>  	# added in 24.03
>  	__rte_cryptodev_trace_qp_depth_used;
> +
> +	# added in 24.07
> +	rte_cryptodev_queue_pair_reset;
>  };
> 
>  INTERNAL {
> --
> 2.25.1