DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Roy Fan" <roy.fan.zhang@intel.com>
To: Akhil Goyal <akhil.goyal@nxp.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Trahe, Fiona" <fiona.trahe@intel.com>,
	"Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>,
	"Dybkowski, AdamX" <adamx.dybkowski@intel.com>,
	"Bronowski, PiotrX" <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: Mon, 21 Sep 2020 10:40:41 +0000
Message-ID: <BL0PR11MB30437EB24D98BEF232144AB8B83A0@BL0PR11MB3043.namprd11.prod.outlook.com> (raw)
In-Reply-To: <VI1PR04MB31685AE73AF4DC80B371C8EFE63F0@VI1PR04MB3168.eurprd04.prod.outlook.com>

Hi Akhil,

Thanks for the review!

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Friday, September 18, 2020 10:50 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>; Bronowski, PiotrX
> <piotrx.bronowski@intel.com>; Anoob Joseph <anoobj@marvell.com>
> Subject: RE: [dpdk-dev v9 1/4] cryptodev: add crypto data-path service APIs
> 
> 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?
> 
Will add comments to this enum.
Unless the driver will store xform data in certain way (in fact QAT has it) the driver may not know which data-path to choose from.
The purpose of having this enum is that the driver knows to attach the correct handler into the service data structure fast.

> > +
> >  #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
Thanks will change.
> 
> > +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
This field is used for cipher only, auth only, or chain use-cases so I believe this is a better name for it.
> 
> > +	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.
[Then this is an API break right?] 
> 
> > +
> > +		/* 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.
We have 2 main different uses cases, 1 for cpu crypto and 1 for data path APIs. Within each main uses case there are 4 types of algo (cipher only/auth only/aead/chain), one requiring HW address and virtual address, the other doesn't.
It seems to causing too much confusion to include these many union into the structure that initially was designed for cpu crypto only. 
I suggest better to use different structure than squeeze all into a big union.

> 
> >  	/**
> >  	 * 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.
> 
Will update. Thanks a lot!
> > +
> > +	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.
Will change.
> 
> > +
> > +	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.
Will do.
> 
> > +	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);
> 
Will change.
> 
> > +}
> > +
> > +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.
Will update.
> 
> >
> >  /**
> >   * 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?
For a device - sorry for the typo.
> 
> > + *
> > + * @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 ..
Sorry, will change.
> 
> > + */
> > +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?
User provided data that wants to retrieve when dequeue. Will do.
> 
> > + * @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.
Will remove. 
> 
> > + * @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?
Same as crypto session, the user need to query the driver specific service data
Size and allocate the buffer accordingly. The difference is it does not have to
Be from mempool as it can be reused.
> 
> > +
> > +/**
> > + * 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.
> 
Will do.
> 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-21 10:40 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
2020-09-21 10:40           ` Zhang, Roy Fan [this message]
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=BL0PR11MB30437EB24D98BEF232144AB8B83A0@BL0PR11MB3043.namprd11.prod.outlook.com \
    --to=roy.fan.zhang@intel.com \
    --cc=adamx.dybkowski@intel.com \
    --cc=akhil.goyal@nxp.com \
    --cc=anoobj@marvell.com \
    --cc=arkadiuszx.kusztal@intel.com \
    --cc=dev@dpdk.org \
    --cc=fiona.trahe@intel.com \
    --cc=piotrx.bronowski@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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git