DPDK patches and discussions
 help / color / mirror / Atom feed
From: Akhil Goyal <akhil.goyal@nxp.com>
To: Fan Zhang <roy.fan.zhang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "fiona.trahe@intel.com" <fiona.trahe@intel.com>,
	"arkadiuszx.kusztal@intel.com" <arkadiuszx.kusztal@intel.com>,
	"adamx.dybkowski@intel.com" <adamx.dybkowski@intel.com>,
	Piotr Bronowski <piotrx.bronowski@intel.com>,
	Anoob Joseph <anoobj@marvell.com>
Subject: Re: [dpdk-dev] [dpdk-dev v9 1/4] cryptodev: add crypto data-path service APIs
Date: Fri, 18 Sep 2020 21:50:02 +0000	[thread overview]
Message-ID: <VI1PR04MB31685AE73AF4DC80B371C8EFE63F0@VI1PR04MB3168.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20200908084253.81022-2-roy.fan.zhang@intel.com>

Hi Fan,

> Subject: [dpdk-dev v9 1/4] cryptodev: add crypto data-path service APIs
> 
> This patch adds data-path service APIs for enqueue and dequeue
> operations to cryptodev. The APIs support flexible user-define
> enqueue and dequeue behaviors and operation mode.
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto.h             |   9 +
>  lib/librte_cryptodev/rte_crypto_sym.h         |  49 ++-
>  lib/librte_cryptodev/rte_cryptodev.c          |  98 +++++
>  lib/librte_cryptodev/rte_cryptodev.h          | 335 +++++++++++++++++-
>  lib/librte_cryptodev/rte_cryptodev_pmd.h      |  48 ++-
>  .../rte_cryptodev_version.map                 |  10 +
>  6 files changed, 540 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto.h b/lib/librte_cryptodev/rte_crypto.h
> index fd5ef3a87..f009be9af 100644
> --- a/lib/librte_cryptodev/rte_crypto.h
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -438,6 +438,15 @@ rte_crypto_op_attach_asym_session(struct
> rte_crypto_op *op,
>  	return 0;
>  }
> 
> +/** Crypto data-path service types */
> +enum rte_crypto_dp_service {
> +	RTE_CRYPTO_DP_SYM_CIPHER_ONLY = 0,
> +	RTE_CRYPTO_DP_SYM_AUTH_ONLY,
> +	RTE_CRYPTO_DP_SYM_CHAIN,
> +	RTE_CRYPTO_DP_SYM_AEAD,
> +	RTE_CRYPTO_DP_N_SERVICE
> +};

Comments missing for this enum.
Do we really need this enum?
Can we not have this info in the driver from the xform list?
And if we really want to add this, why to have it specific to raw data path APIs?

> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/librte_cryptodev/rte_crypto_sym.h
> b/lib/librte_cryptodev/rte_crypto_sym.h
> index f29c98051..376412e94 100644
> --- a/lib/librte_cryptodev/rte_crypto_sym.h
> +++ b/lib/librte_cryptodev/rte_crypto_sym.h
> @@ -50,6 +50,30 @@ struct rte_crypto_sgl {
>  	uint32_t num;
>  };
> 
> +/**
> + * Symmetri Crypto Addtional Data other than src and destination data.
> + * Supposed to be used to pass IV/digest/aad data buffers with lengths
> + * defined when creating crypto session.
> + */

Fix typo

> +union rte_crypto_sym_additional_data {
> +	struct {
> +		void *cipher_iv_ptr;
> +		rte_iova_t cipher_iv_iova;
> +		void *auth_iv_ptr;
> +		rte_iova_t auth_iv_iova;
> +		void *digest_ptr;
> +		rte_iova_t digest_iova;
> +	} cipher_auth;

Should be chain instead of cipher_auth

> +	struct {
> +		void *iv_ptr;
> +		rte_iova_t iv_iova;
> +		void *digest_ptr;
> +		rte_iova_t digest_iova;
> +		void *aad_ptr;
> +		rte_iova_t aad_iova;
> +	} aead;
> +};
> +
>  /**
>   * Synchronous operation descriptor.
>   * Supposed to be used with CPU crypto API call.
> @@ -57,12 +81,25 @@ struct rte_crypto_sgl {
>  struct rte_crypto_sym_vec {
>  	/** array of SGL vectors */
>  	struct rte_crypto_sgl *sgl;
> -	/** array of pointers to IV */
> -	void **iv;
> -	/** array of pointers to AAD */
> -	void **aad;
> -	/** array of pointers to digest */
> -	void **digest;
> +
> +	union {
> +
> +		/* Supposed to be used with CPU crypto API call. */
> +		struct {
> +			/** array of pointers to IV */
> +			void **iv;
> +			/** array of pointers to AAD */
> +			void **aad;
> +			/** array of pointers to digest */
> +			void **digest;
> +		};

Can we also name this struct?
Probably we should split this as a separate patch.

> +
> +		/* Supposed to be used with
> rte_cryptodev_dp_sym_submit_vec()
> +		 * call.
> +		 */
> +		union rte_crypto_sym_additional_data *additional_data;
> +	};
> +

Can we get rid of this unnecessary union rte_crypto_sym_additional_data
And place chain and aead directly in the union? At any point, only one of the three
would be used.

>  	/**
>  	 * array of statuses for each operation:
>  	 *  - 0 on success
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> b/lib/librte_cryptodev/rte_cryptodev.c
> index 1dd795bcb..4f59cf800 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -1914,6 +1914,104 @@ rte_cryptodev_sym_cpu_crypto_process(uint8_t
> dev_id,
>  	return dev->dev_ops->sym_cpu_process(dev, sess, ofs, vec);
>  }
> 
> +int
> +rte_cryptodev_dp_get_service_ctx_data_size(uint8_t dev_id)
> +{
> +	struct rte_cryptodev *dev;
> +	int32_t size = sizeof(struct rte_crypto_dp_service_ctx);
> +	int32_t priv_size;
> +
> +	if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
> +		return -1;
> +
> +	dev = rte_cryptodev_pmd_get_dev(dev_id);
> +
> +	if (*dev->dev_ops->get_drv_ctx_size == NULL ||
> +		!(dev->feature_flags &
> RTE_CRYPTODEV_FF_DATA_PATH_SERVICE)) {
> +		return -1;
> +	}

I have some suggestions for the naming of the APIs / flags in the doc patch, 
Please check that and make changes in this patch.
Also, you have missed adding this feature flag in the
doc/guides/cryptodevs/features/default.ini file.
And Subsequently in the doc/guides/cryptodevs/features/qat.ini file.

> +
> +	priv_size = (*dev->dev_ops->get_drv_ctx_size)(dev);
> +	if (priv_size < 0)
> +		return -1;
> +
> +	return RTE_ALIGN_CEIL((size + priv_size), 8);
> +}
> +
> +int
> +rte_cryptodev_dp_configure_service(uint8_t dev_id, uint16_t qp_id,
> +	enum rte_crypto_dp_service service_type,
> +	enum rte_crypto_op_sess_type sess_type,
> +	union rte_cryptodev_session_ctx session_ctx,
> +	struct rte_crypto_dp_service_ctx *ctx, uint8_t is_update)
> +{
> +	struct rte_cryptodev *dev;
> +
> +	if (!rte_cryptodev_get_qp_status(dev_id, qp_id))
> +		return -1;
> +
> +	dev = rte_cryptodev_pmd_get_dev(dev_id);
> +	if (!(dev->feature_flags & RTE_CRYPTODEV_FF_DATA_PATH_SERVICE)
> +			|| dev->dev_ops->configure_service == NULL)
> +		return -1;
It would be better to return actual error number like ENOTSUP/EINVAL.
It would be helpful in debugging.

> +
> +	return (*dev->dev_ops->configure_service)(dev, qp_id, service_type,
> +			sess_type, session_ctx, ctx, is_update);
> +}
> +
> +int
> +rte_cryptodev_dp_sym_submit_single_job(struct rte_crypto_dp_service_ctx
> *ctx,
> +		struct rte_crypto_vec *data, uint16_t n_data_vecs,
> +		union rte_crypto_sym_ofs ofs,
> +		union rte_crypto_sym_additional_data *additional_data,
> +		void *opaque)
> +{

Can we have some debug checks for NULL checking.

> +	return _cryptodev_dp_submit_single_job(ctx, data, n_data_vecs, ofs,
> +			additional_data, opaque);

Unnecessary function call _cryptodev_dp_submit_single_job.
You can directly call
return (*ctx->submit_single_job)(ctx->qp_data, ctx->drv_service_data,
		data, n_data_vecs, ofs, additional_data, opaque);


> +}
> +
> +uint32_t
> +rte_cryptodev_dp_sym_submit_vec(struct rte_crypto_dp_service_ctx *ctx,
> +	struct rte_crypto_sym_vec *vec, union rte_crypto_sym_ofs ofs,
> +	void **opaque)
> +{
> +	return (*ctx->submit_vec)(ctx->qp_data, ctx->drv_service_data, vec,
> +			ofs, opaque);
> +}
> +
> +int
> +rte_cryptodev_dp_sym_dequeue_single_job(struct rte_crypto_dp_service_ctx
> *ctx,
> +		void **out_opaque)
> +{
> +	return _cryptodev_dp_sym_dequeue_single_job(ctx, out_opaque);

Same here.
> +}
> +
> +int
> +rte_cryptodev_dp_sym_submit_done(struct rte_crypto_dp_service_ctx *ctx,
> +		uint32_t n)
> +{
> +	return (*ctx->submit_done)(ctx->qp_data, ctx->drv_service_data, n);
> +}
> +
> +int
> +rte_cryptodev_dp_sym_dequeue_done(struct rte_crypto_dp_service_ctx *ctx,
> +		uint32_t n)
> +{
> +	return (*ctx->dequeue_done)(ctx->qp_data, ctx->drv_service_data, n);
> +}
> +
> +uint32_t
> +rte_cryptodev_dp_sym_dequeue(struct rte_crypto_dp_service_ctx *ctx,
> +	rte_cryptodev_get_dequeue_count_t get_dequeue_count,
> +	rte_cryptodev_post_dequeue_t post_dequeue,
> +	void **out_opaque, uint8_t is_opaque_array,
> +	uint32_t *n_success_jobs)
> +{
> +	return (*ctx->dequeue_opaque)(ctx->qp_data, ctx->drv_service_data,
> +		get_dequeue_count, post_dequeue, out_opaque,
> is_opaque_array,
> +		n_success_jobs);
> +}
> +
>  /** Initialise rte_crypto_op mempool element */
>  static void
>  rte_crypto_op_init(struct rte_mempool *mempool,
> diff --git a/lib/librte_cryptodev/rte_cryptodev.h
> b/lib/librte_cryptodev/rte_cryptodev.h
> index 7b3ebc20f..4da0389d1 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -466,7 +466,8 @@ rte_cryptodev_asym_get_xform_enum(enum
> rte_crypto_asym_xform_type *xform_enum,
>  /**< Support symmetric session-less operations */
>  #define RTE_CRYPTODEV_FF_NON_BYTE_ALIGNED_DATA		(1ULL
> << 23)
>  /**< Support operations on data which is not byte aligned */
> -
> +#define RTE_CRYPTODEV_FF_DATA_PATH_SERVICE		(1ULL << 24)
> +/**< Support accelerated specific raw data as input */

Support data path APIs for raw data as input.

> 
>  /**
>   * Get the name of a crypto device feature flag
> @@ -1351,6 +1352,338 @@ rte_cryptodev_sym_cpu_crypto_process(uint8_t
> dev_id,
>  	struct rte_cryptodev_sym_session *sess, union rte_crypto_sym_ofs ofs,
>  	struct rte_crypto_sym_vec *vec);
> 
> +/**
> + * Get the size of the data-path service context for all registered drivers.

For all drivers ? or for a device?

> + *
> + * @param	dev_id		The device identifier.
> + *
> + * @return
> + *   - If the device supports data-path service, return the context size.
> + *   - If the device does not support the data-dath service, return -1.
> + */
> +__rte_experimental
> +int
> +rte_cryptodev_dp_get_service_ctx_data_size(uint8_t dev_id);
> +
> +/**
> + * Union of different crypto session types, including session-less xform
> + * pointer.

Union of different symmetric crypto session types ..

> + */
> +union rte_cryptodev_session_ctx {
> +	struct rte_cryptodev_sym_session *crypto_sess;
> +	struct rte_crypto_sym_xform *xform;
> +	struct rte_security_session *sec_sess;
> +};
> +
> +/**
> + * Submit a data vector into device queue but the driver will not start
> + * processing until rte_cryptodev_dp_sym_submit_vec() is called.
> + *
> + * @param	qp		Driver specific queue pair data.
> + * @param	service_data	Driver specific service data.
> + * @param	vec		The array of job vectors.
> + * @param	ofs		Start and stop offsets for auth and cipher
> + *				operations.
> + * @param	opaque		The array of opaque data for dequeue.

Can you elaborate the usage of opaque here?

> + * @return
> + *   - The number of jobs successfully submitted.
> + */
> +typedef uint32_t (*cryptodev_dp_sym_submit_vec_t)(
> +	void *qp, uint8_t *service_data, struct rte_crypto_sym_vec *vec,
> +	union rte_crypto_sym_ofs ofs, void **opaque);
> +
> +/**
> + * Submit single job into device queue but the driver will not start
> + * processing until rte_cryptodev_dp_sym_submit_vec() is called.
> + *
> + * @param	qp		Driver specific queue pair data.
> + * @param	service_data	Driver specific service data.
> + * @param	data		The buffer vector.
> + * @param	n_data_vecs	Number of buffer vectors.
> + * @param	ofs		Start and stop offsets for auth and cipher
> + *				operations.
> + * @param	additional_data	IV, digest, and aad data.
> + * @param	opaque		The opaque data for dequeue.
> + * @return
> + *   - On success return 0.
> + *   - On failure return negative integer.
> + */
> +typedef int (*cryptodev_dp_submit_single_job_t)(
> +	void *qp, uint8_t *service_data, struct rte_crypto_vec *data,
> +	uint16_t n_data_vecs, union rte_crypto_sym_ofs ofs,
> +	union rte_crypto_sym_additional_data *additional_data,
> +	void *opaque);
> +
> +/**
> + * Inform the queue pair to start processing or finish dequeuing all
> + * submitted/dequeued jobs.
> + *
> + * @param	qp		Driver specific queue pair data.
> + * @param	service_data	Driver specific service data.
> + * @param	n		The total number of submitted jobs.
> + * @return
> + *   - On success return 0.
> + *   - On failure return negative integer.
> + */
> +typedef int (*cryptodev_dp_sym_operation_done_t)(void *qp,
> +		uint8_t *service_data, uint32_t n);
> +
> +/**
> + * Typedef that the user provided for the driver to get the dequeue count.
> + * The function may return a fixed number or the number parsed from the
> opaque
> + * data stored in the first processed job.
> + *
> + * @param	opaque		Dequeued opaque data.
> + **/
> +typedef uint32_t (*rte_cryptodev_get_dequeue_count_t)(void *opaque);
> +
> +/**
> + * Typedef that the user provided to deal with post dequeue operation, such
> + * as filling status.
> + *
> + * @param	opaque		Dequeued opaque data. In case
> + *
> 	RTE_CRYPTO_HW_DP_FF_GET_OPAQUE_ARRAY bit is
> + *				set, this value will be the opaque data stored
> + *				in the specific processed jobs referenced by
> + *				index, otherwise it will be the opaque data
> + *				stored in the first processed job in the burst.

What is RTE_CRYPTO_HW_DP_FF_GET_OPAQUE_ARRAY, I did not find this in the series.

> + * @param	index		Index number of the processed job.
> + * @param	is_op_success	Driver filled operation status.
> + **/
> +typedef void (*rte_cryptodev_post_dequeue_t)(void *opaque, uint32_t index,
> +		uint8_t is_op_success);
> +
> +/**
> + * Dequeue symmetric crypto processing of user provided data.
> + *
> + * @param	qp			Driver specific queue pair data.
> + * @param	service_data		Driver specific service data.
> + * @param	get_dequeue_count	User provided callback function to
> + *					obtain dequeue count.
> + * @param	post_dequeue		User provided callback function to
> + *					post-process a dequeued operation.
> + * @param	out_opaque		Opaque pointer array to be retrieve
> from
> + *					device queue. In case of
> + *					*is_opaque_array* is set there should
> + *					be enough room to store all opaque
> data.
> + * @param	is_opaque_array		Set 1 if every dequeued job will
> be
> + *					written the opaque data into
> + *					*out_opaque* array.
> + * @param	n_success_jobs		Driver written value to specific the
> + *					total successful operations count.
> + *
> + * @return
> + *  - Returns number of dequeued packets.
> + */
> +typedef uint32_t (*cryptodev_dp_sym_dequeue_t)(void *qp, uint8_t
> *service_data,
> +	rte_cryptodev_get_dequeue_count_t get_dequeue_count,
> +	rte_cryptodev_post_dequeue_t post_dequeue,
> +	void **out_opaque, uint8_t is_opaque_array,
> +	uint32_t *n_success_jobs);
> +
> +/**
> + * Dequeue symmetric crypto processing of user provided data.
> + *
> + * @param	qp			Driver specific queue pair data.
> + * @param	service_data		Driver specific service data.
> + * @param	out_opaque		Opaque pointer to be retrieve from
> + *					device queue.
> + *
> + * @return
> + *   - 1 if the job is dequeued and the operation is a success.
> + *   - 0 if the job is dequeued but the operation is failed.
> + *   - -1 if no job is dequeued.
> + */
> +typedef int (*cryptodev_dp_sym_dequeue_single_job_t)(
> +		void *qp, uint8_t *service_data, void **out_opaque);
> +
> +/**
> + * Context data for asynchronous crypto process.
> + */
> +struct rte_crypto_dp_service_ctx {
> +	void *qp_data;
> +
> +	struct {
> +		cryptodev_dp_submit_single_job_t submit_single_job;
> +		cryptodev_dp_sym_submit_vec_t submit_vec;
> +		cryptodev_dp_sym_operation_done_t submit_done;
> +		cryptodev_dp_sym_dequeue_t dequeue_opaque;
> +		cryptodev_dp_sym_dequeue_single_job_t dequeue_single;
> +		cryptodev_dp_sym_operation_done_t dequeue_done;
> +	};
> +
> +	/* Driver specific service data */
> +	__extension__ uint8_t drv_service_data[];
> +};

Comments missing for structure params.
Struct name can be rte_crypto_raw_dp_ctx.

Who allocate and free this structure?

> +
> +/**
> + * Configure one DP service context data. Calling this function for the first
> + * time the user should unset the *is_update* parameter and the driver will
> + * fill necessary operation data into ctx buffer. Only when
> + * rte_cryptodev_dp_submit_done() is called the data stored in the ctx buffer
> + * will not be effective.
> + *
> + * @param	dev_id		The device identifier.
> + * @param	qp_id		The index of the queue pair from which to
> + *				retrieve processed packets. The value must be
> + *				in the range [0, nb_queue_pair - 1] previously
> + *				supplied to rte_cryptodev_configure().
> + * @param	service_type	Type of the service requested.
> + * @param	sess_type	session type.
> + * @param	session_ctx	Session context data.
> + * @param	ctx		The data-path service context data.
> + * @param	is_update	Set 1 if ctx is pre-initialized but need
> + *				update to different service type or session,
> + *				but the rest driver data remains the same.
> + *				Since service context data buffer is provided
> + *				by user, the driver will not check the
> + *				validity of the buffer nor its content. It is
> + *				the user's obligation to initialize and
> + *				uses the buffer properly by setting this field.
> + * @return
> + *   - On success return 0.
> + *   - On failure return negative integer.
> + */
> +__rte_experimental
> +int
> +rte_cryptodev_dp_configure_service(uint8_t dev_id, uint16_t qp_id,
> +	enum rte_crypto_dp_service service_type,
> +	enum rte_crypto_op_sess_type sess_type,
> +	union rte_cryptodev_session_ctx session_ctx,
> +	struct rte_crypto_dp_service_ctx *ctx, uint8_t is_update);
> +
> +static __rte_always_inline int
> +_cryptodev_dp_submit_single_job(struct rte_crypto_dp_service_ctx *ctx,
> +		struct rte_crypto_vec *data, uint16_t n_data_vecs,
> +		union rte_crypto_sym_ofs ofs,
> +		union rte_crypto_sym_additional_data *additional_data,
> +		void *opaque)
> +{
> +	return (*ctx->submit_single_job)(ctx->qp_data, ctx->drv_service_data,
> +		data, n_data_vecs, ofs, additional_data, opaque);
> +}
> +
> +static __rte_always_inline int
> +_cryptodev_dp_sym_dequeue_single_job(struct rte_crypto_dp_service_ctx
> *ctx,
> +		void **out_opaque)
> +{
> +	return (*ctx->dequeue_single)(ctx->qp_data, ctx->drv_service_data,
> +		out_opaque);
> +}
> +
> +/**
> + * Submit single job into device queue but the driver will not start
> + * processing until rte_cryptodev_dp_submit_done() is called. This is a
> + * simplified
> + *
> + * @param	ctx		The initialized data-path service context data.
> + * @param	data		The buffer vector.
> + * @param	n_data_vecs	Number of buffer vectors.
> + * @param	ofs		Start and stop offsets for auth and cipher
> + *				operations.
> + * @param	additional_data	IV, digest, and aad
> + * @param	opaque		The array of opaque data for dequeue.
> + * @return
> + *   - On success return 0.
> + *   - On failure return negative integer.
> + */
> +__rte_experimental
> +int
> +rte_cryptodev_dp_sym_submit_single_job(struct rte_crypto_dp_service_ctx
> *ctx,
> +		struct rte_crypto_vec *data, uint16_t n_data_vecs,
> +		union rte_crypto_sym_ofs ofs,
> +		union rte_crypto_sym_additional_data *additional_data,
> +		void *opaque);
> +
> +/**
> + * Submit a data vector into device queue but the driver will not start
> + * processing until rte_cryptodev_dp_submit_done() is called.
> + *
> + * @param	ctx	The initialized data-path service context data.
> + * @param	vec	The array of job vectors.
> + * @param	ofs	Start and stop offsets for auth and cipher operations.
> + * @param	opaque	The array of opaque data for dequeue.
> + * @return
> + *   - The number of jobs successfully submitted.
> + */
> +__rte_experimental
> +uint32_t
> +rte_cryptodev_dp_sym_submit_vec(struct rte_crypto_dp_service_ctx *ctx,
> +	struct rte_crypto_sym_vec *vec, union rte_crypto_sym_ofs ofs,
> +	void **opaque);
> +
> +/**
> + * Command the queue pair to start processing all submitted jobs from last
> + * rte_cryptodev_init_dp_service() call.
> + *
> + * @param	ctx	The initialized data-path service context data.
> + * @param	n		The total number of submitted jobs.
> + */
> +__rte_experimental
> +int
> +rte_cryptodev_dp_sym_submit_done(struct rte_crypto_dp_service_ctx *ctx,
> +		uint32_t n);
> +
> +/**
> + * Dequeue symmetric crypto processing of user provided data.
> + *
> + * @param	ctx			The initialized data-path service
> + *					context data.
> + * @param	get_dequeue_count	User provided callback function to
> + *					obtain dequeue count.
> + * @param	post_dequeue		User provided callback function to
> + *					post-process a dequeued operation.
> + * @param	out_opaque		Opaque pointer array to be retrieve
> from
> + *					device queue. In case of
> + *					*is_opaque_array* is set there should
> + *					be enough room to store all opaque
> data.
> + * @param	is_opaque_array		Set 1 if every dequeued job will
> be
> + *					written the opaque data into
> + *					*out_opaque* array.
> + * @param	n_success_jobs		Driver written value to specific the
> + *					total successful operations count.
> + *
> + * @return
> + *   - Returns number of dequeued packets.
> + */
> +__rte_experimental
> +uint32_t
> +rte_cryptodev_dp_sym_dequeue(struct rte_crypto_dp_service_ctx *ctx,
> +	rte_cryptodev_get_dequeue_count_t get_dequeue_count,
> +	rte_cryptodev_post_dequeue_t post_dequeue,
> +	void **out_opaque, uint8_t is_opaque_array,
> +	uint32_t *n_success_jobs);
> +
> +/**
> + * Dequeue Single symmetric crypto processing of user provided data.
> + *
> + * @param	ctx			The initialized data-path service
> + *					context data.
> + * @param	out_opaque		Opaque pointer to be retrieve from
> + *					device queue. The driver shall support
> + *					NULL input of this parameter.
> + *
> + * @return
> + *   - 1 if the job is dequeued and the operation is a success.
> + *   - 0 if the job is dequeued but the operation is failed.
> + *   - -1 if no job is dequeued.
> + */
> +__rte_experimental
> +int
> +rte_cryptodev_dp_sym_dequeue_single_job(struct rte_crypto_dp_service_ctx
> *ctx,
> +		void **out_opaque);
> +
> +/**
> + * Inform the queue pair dequeue jobs finished.
> + *
> + * @param	ctx	The initialized data-path service context data.
> + * @param	n	The total number of jobs already dequeued.
> + */
> +__rte_experimental
> +int
> +rte_cryptodev_dp_sym_dequeue_done(struct rte_crypto_dp_service_ctx *ctx,
> +		uint32_t n);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> index 81975d72b..e19de458c 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> +++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> @@ -316,6 +316,42 @@ typedef uint32_t
> (*cryptodev_sym_cpu_crypto_process_t)
>  	(struct rte_cryptodev *dev, struct rte_cryptodev_sym_session *sess,
>  	union rte_crypto_sym_ofs ofs, struct rte_crypto_sym_vec *vec);
> 
> +/**
> + * Typedef that the driver provided to get service context private date size.
> + *
> + * @param	dev	Crypto device pointer.
> + *
> + * @return
> + *   - On success return the size of the device's service context private data.
> + *   - On failure return negative integer.
> + */
> +typedef int (*cryptodev_dp_get_service_ctx_size_t)(
> +	struct rte_cryptodev *dev);
> +
> +/**
> + * Typedef that the driver provided to configure data-path service.
> + *
> + * @param	dev		Crypto device pointer.
> + * @param	qp_id		Crypto device queue pair index.
> + * @param	service_type	Type of the service requested.
> + * @param	sess_type	session type.
> + * @param	session_ctx	Session context data.
> + * @param	ctx		The data-path service context data.
> + * @param	is_update	Set 1 if ctx is pre-initialized but need
> + *				update to different service type or session,
> + *				but the rest driver data remains the same.
> + *				buffer will always be one.
> + * @return
> + *   - On success return 0.
> + *   - On failure return negative integer.
> + */
> +typedef int (*cryptodev_dp_configure_service_t)(
> +	struct rte_cryptodev *dev, uint16_t qp_id,
> +	enum rte_crypto_dp_service service_type,
> +	enum rte_crypto_op_sess_type sess_type,
> +	union rte_cryptodev_session_ctx session_ctx,
> +	struct rte_crypto_dp_service_ctx *ctx,
> +	uint8_t is_update);
> 
>  /** Crypto device operations function pointer table */
>  struct rte_cryptodev_ops {
> @@ -348,8 +384,16 @@ struct rte_cryptodev_ops {
>  	/**< Clear a Crypto sessions private data. */
>  	cryptodev_asym_free_session_t asym_session_clear;
>  	/**< Clear a Crypto sessions private data. */
> -	cryptodev_sym_cpu_crypto_process_t sym_cpu_process;
> -	/**< process input data synchronously (cpu-crypto). */
> +	union {
> +		cryptodev_sym_cpu_crypto_process_t sym_cpu_process;
> +		/**< process input data synchronously (cpu-crypto). */
> +		struct {
> +			cryptodev_dp_get_service_ctx_size_t get_drv_ctx_size;
> +			/**< Get data path service context data size. */
> +			cryptodev_dp_configure_service_t configure_service;
> +			/**< Initialize crypto service ctx data. */
> +		};
> +	};
>  };
> 
> 
> diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map
> b/lib/librte_cryptodev/rte_cryptodev_version.map
> index 02f6dcf72..10388ae90 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_version.map
> +++ b/lib/librte_cryptodev/rte_cryptodev_version.map
> @@ -105,4 +105,14 @@ EXPERIMENTAL {
> 
>  	# added in 20.08
>  	rte_cryptodev_get_qp_status;
> +
> +	# added in 20.11
> +	rte_cryptodev_dp_configure_service;

Rte_cryptodev_configure_raw_dp

> +	rte_cryptodev_dp_get_service_ctx_data_size;

Rte_cryptodev_get_raw_dp_ctx_size

> +	rte_cryptodev_dp_sym_dequeue;

rte_cryptodev_raw_dequeue_burst

> +	rte_cryptodev_dp_sym_dequeue_done;
rte_cryptodev_raw_dequeue_done

> +	rte_cryptodev_dp_sym_dequeue_single_job;
rte_cryptodev_raw_dequeue

> +	rte_cryptodev_dp_sym_submit_done;

rte_cryptodev_raw_enqueue_done

> +	rte_cryptodev_dp_sym_submit_single_job;

rte_cryptodev_raw_enqueue

> +	rte_cryptodev_dp_sym_submit_vec;

rte_cryptodev_raw_enqueue_burst

>  };

Please use above names for the APIs.
No need for keyword dp in enq/deq APIs as it is implicit that enq/deq APIs are data path APIs.

I could not complete the review of this patch specifically as I see a lot of issues in the current version
Please send reply to my queries so that review can be completed.

Regards,
Akhil



  reply	other threads:[~2020-09-18 21:50 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-18 16:28 [dpdk-dev] [dpdk-dev v6 0/4] cryptodev: add " Fan Zhang
2020-08-18 16:28 ` [dpdk-dev] [dpdk-dev v6 1/4] cryptodev: add crypto " Fan Zhang
2020-08-18 16:28 ` [dpdk-dev] [dpdk-dev v6 2/4] crypto/qat: add crypto data-path service API support Fan Zhang
2020-08-18 16:28 ` [dpdk-dev] [dpdk-dev v6 3/4] test/crypto: add unit-test for cryptodev direct APIs Fan Zhang
2020-08-18 16:28 ` [dpdk-dev] [dpdk-dev v6 4/4] doc: add cryptodev service APIs guide Fan Zhang
2020-08-28 12:58 ` [dpdk-dev] [dpdk-dev v7 0/4] cryptodev: add data-path service APIs Fan Zhang
2020-08-28 12:58   ` [dpdk-dev] [dpdk-dev v7 1/4] cryptodev: add crypto " Fan Zhang
2020-08-31  6:23     ` Kusztal, ArkadiuszX
2020-08-31 12:21       ` Zhang, Roy Fan
2020-08-31 15:15       ` Zhang, Roy Fan
2020-08-28 12:58   ` [dpdk-dev] [dpdk-dev v7 2/4] crypto/qat: add crypto data-path service API support Fan Zhang
2020-08-28 12:58   ` [dpdk-dev] [dpdk-dev v7 3/4] test/crypto: add unit-test for cryptodev direct APIs Fan Zhang
2020-08-28 12:58   ` [dpdk-dev] [dpdk-dev v7 4/4] doc: add cryptodev service APIs guide Fan Zhang
2020-09-04 15:25   ` [dpdk-dev] [dpdk-dev v8 0/4] cryptodev: add data-path service APIs Fan Zhang
2020-09-04 15:25     ` [dpdk-dev] [dpdk-dev v8 1/4] cryptodev: add crypto " Fan Zhang
2020-09-07 12:36       ` Dybkowski, AdamX
2020-09-04 15:25     ` [dpdk-dev] [dpdk-dev v8 2/4] crypto/qat: add crypto data-path service API support Fan Zhang
2020-09-04 15:25     ` [dpdk-dev] [dpdk-dev v8 3/4] test/crypto: add unit-test for cryptodev direct APIs Fan Zhang
2020-09-04 15:25     ` [dpdk-dev] [dpdk-dev v8 4/4] doc: add cryptodev service APIs guide Fan Zhang
2020-09-08  8:42     ` [dpdk-dev] [dpdk-dev v9 0/4] cryptodev: add data-path service APIs Fan Zhang
2020-09-08  8:42       ` [dpdk-dev] [dpdk-dev v9 1/4] cryptodev: add crypto " Fan Zhang
2020-09-18 21:50         ` Akhil Goyal [this message]
2020-09-21 10:40           ` Zhang, Roy Fan
2020-09-21 11:59             ` Akhil Goyal
2020-09-21 15:26               ` Zhang, Roy Fan
2020-09-21 15:41               ` Zhang, Roy Fan
2020-09-21 15:49                 ` Akhil Goyal
2020-09-22  8:08                   ` Zhang, Roy Fan
2020-09-22  8:21                   ` Zhang, Roy Fan
2020-09-22  8:48                     ` Ananyev, Konstantin
2020-09-22  9:05                       ` Akhil Goyal
2020-09-22  9:28                         ` Zhang, Roy Fan
2020-09-22 10:18                           ` Ananyev, Konstantin
2020-09-22 12:15                             ` Zhang, Roy Fan
2020-09-22 12:50                             ` Zhang, Roy Fan
2020-09-22 12:52                               ` Akhil Goyal
2020-09-08  8:42       ` [dpdk-dev] [dpdk-dev v9 2/4] crypto/qat: add crypto data-path service API support Fan Zhang
2020-09-08  8:42       ` [dpdk-dev] [dpdk-dev v9 3/4] test/crypto: add unit-test for cryptodev direct APIs Fan Zhang
2020-09-18 20:03         ` Akhil Goyal
2020-09-21 12:41           ` Zhang, Roy Fan
2020-09-08  8:42       ` [dpdk-dev] [dpdk-dev v9 4/4] doc: add cryptodev service APIs guide Fan Zhang
2020-09-18 20:39         ` Akhil Goyal
2020-09-21 12:28           ` Zhang, Roy Fan
2020-09-23 13:37           ` Zhang, Roy Fan
2020-09-24 16:34       ` [dpdk-dev] [dpdk-dev v10 0/4] cryptodev: add raw data-path APIs Fan Zhang
2020-09-24 16:34         ` [dpdk-dev] [dpdk-dev v10 1/4] cryptodev: change crypto symmetric vector structure Fan Zhang
2020-09-25  8:03           ` Dybkowski, AdamX
2020-09-28 17:01           ` Ananyev, Konstantin
2020-09-24 16:34         ` [dpdk-dev] [dpdk-dev v10 2/4] cryptodev: add raw crypto data-path APIs Fan Zhang
2020-09-25  8:04           ` Dybkowski, AdamX
2020-10-08 14:26           ` Akhil Goyal
2020-10-08 15:29             ` Zhang, Roy Fan
2020-10-08 16:07               ` Akhil Goyal
2020-10-08 16:24                 ` Zhang, Roy Fan
2020-10-09  8:32                 ` Zhang, Roy Fan
2020-10-08 14:37           ` Akhil Goyal
2020-09-24 16:34         ` [dpdk-dev] [dpdk-dev v10 3/4] crypto/qat: add raw crypto data-path API support Fan Zhang
2020-09-25  8:04           ` Dybkowski, AdamX
2020-09-24 16:34         ` [dpdk-dev] [dpdk-dev v10 4/4] test/crypto: add unit-test for cryptodev raw API test Fan Zhang
2020-09-25  8:05           ` Dybkowski, AdamX
2020-10-08 15:01           ` Akhil Goyal
2020-10-08 15:04         ` [dpdk-dev] [dpdk-dev v10 0/4] cryptodev: add raw data-path APIs Akhil Goyal
2020-10-08 15:30           ` Zhang, Roy Fan
2020-10-09 21:11         ` [dpdk-dev] [dpdk-dev v11 " Fan Zhang
2020-10-09 21:11           ` [dpdk-dev] [dpdk-dev v11 1/4] cryptodev: change crypto symmetric vector structure Fan Zhang
2020-10-09 21:11           ` [dpdk-dev] [dpdk-dev v11 2/4] cryptodev: add raw crypto data-path APIs Fan Zhang
2020-10-10 19:38             ` Akhil Goyal
2020-10-10 20:40               ` Zhang, Roy Fan
2020-10-09 21:11           ` [dpdk-dev] [dpdk-dev v11 3/4] crypto/qat: add raw crypto data-path API support Fan Zhang
2020-10-09 21:11           ` [dpdk-dev] [dpdk-dev v11 4/4] test/crypto: add unit-test for cryptodev raw API test Fan Zhang
2020-10-10 19:55             ` Akhil Goyal
2020-10-10 20:50               ` Zhang, Roy Fan
2020-10-10 21:03                 ` Akhil Goyal
2020-10-11  0:32           ` [dpdk-dev] [dpdk-dev v12 0/4] cryptodev: add raw data-path APIs Fan Zhang
2020-10-11  0:32             ` [dpdk-dev] [dpdk-dev v12 1/4] cryptodev: change crypto symmetric vector structure Fan Zhang
2020-10-11  0:32             ` [dpdk-dev] [dpdk-dev v12 2/4] cryptodev: add raw crypto data-path APIs Fan Zhang
2020-10-11  0:32             ` [dpdk-dev] [dpdk-dev v12 3/4] crypto/qat: add raw crypto data-path API support Fan Zhang
2020-10-11  0:32             ` [dpdk-dev] [dpdk-dev v12 4/4] test/crypto: add unit-test for cryptodev raw API test Fan Zhang
2020-10-11  0:38             ` [dpdk-dev] [dpdk-dev v13 0/4] cryptodev: add raw data-path APIs Fan Zhang
2020-10-11  0:38               ` [dpdk-dev] [dpdk-dev v13 1/4] cryptodev: change crypto symmetric vector structure Fan Zhang
2020-10-11  0:38               ` [dpdk-dev] [dpdk-dev v13 2/4] cryptodev: add raw crypto data-path APIs Fan Zhang
2020-10-11  0:38               ` [dpdk-dev] [dpdk-dev v13 3/4] crypto/qat: add raw crypto data-path API support Fan Zhang
2020-10-11  0:38               ` [dpdk-dev] [dpdk-dev v13 4/4] test/crypto: add unit-test for cryptodev raw API test Fan Zhang
2020-10-12 16:15               ` [dpdk-dev] [dpdk-dev v13 0/4] cryptodev: add raw data-path APIs Akhil Goyal

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=VI1PR04MB31685AE73AF4DC80B371C8EFE63F0@VI1PR04MB3168.eurprd04.prod.outlook.com \
    --to=akhil.goyal@nxp.com \
    --cc=adamx.dybkowski@intel.com \
    --cc=anoobj@marvell.com \
    --cc=arkadiuszx.kusztal@intel.com \
    --cc=dev@dpdk.org \
    --cc=fiona.trahe@intel.com \
    --cc=piotrx.bronowski@intel.com \
    --cc=roy.fan.zhang@intel.com \
    /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).