[v9,1/4] cryptodev: add crypto data-path service APIs
Checks
Commit Message
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(-)
Comments
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
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
>
Hi Fan,
> > >
> > > +/** 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.
>
I believe all drivers are storing that information already in some way in the session private data.
This enum is maintained inside driver as of current implementation. This is not specific to raw
Data path APIs. If you are introducing this enum in library, then it should be generic for the legacy
Case as well.
> >
> > > +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.
Agreed that this struct will be used for all 3 cases, that is what is happening in
Other crypto cases. We use chain for all these three cases in legacy codepath.
Chain can be of one or two xforms and ordering can be anything -
Cipher only, auth only, cipher auth and auth cipher.
> >
> > > + 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?]
Since this an LTS release, I am ok to take this change.
But others can comment on this.
@Ananyev, Konstantin, @Thomas Monjalon
Can you comment on this?
> >
> > > +
> > > + /* 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.
>
IMO, the following union can clarify all doubts.
@Ananyev, Konstantin: Any suggestions from your side?
/** IV and aad information for various use cases. */
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;
} cpu_crypto; < or any other useful name>
/* Supposed to be used with HW raw crypto API call. */
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;
} hw_chain;
/* Supposed to be used with HW raw crypto API call. */
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;
} hw_aead;
};
> > > +/**
> > > + * 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.
So this structure is saved and filled by the lib/driver and not the application. Right?
This struct is opaque to application and will be part of session private data. Right?
Assignment and calling appropriate driver's call backs will be hidden inside library
and will be opaque to the application. In other words, the structure is not exposed
to the application.
Please add relevant comments on top of this structure.
> > > +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
Comment not complete.
Regards,
Akhil
Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Monday, September 21, 2020 1:00 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> 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,
>
> > > >
> > > > +/** 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.
> >
> I believe all drivers are storing that information already in some way in the
> session private data.
> This enum is maintained inside driver as of current implementation. This is
> not specific to raw
> Data path APIs. If you are introducing this enum in library, then it should be
> generic for the legacy
> Case as well.
>
[Fan: I am not sure other drivers (that's part of the reason why it is here), but indeed QAT does, Ok]
>
> > >
> > > > +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.
>
> Agreed that this struct will be used for all 3 cases, that is what is happening in
> Other crypto cases. We use chain for all these three cases in legacy codepath.
> Chain can be of one or two xforms and ordering can be anything -
> Cipher only, auth only, cipher auth and auth cipher.
>
>
> > >
> > > > + 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?]
>
> Since this an LTS release, I am ok to take this change.
> But others can comment on this.
> @Ananyev, Konstantin, @Thomas Monjalon
> Can you comment on this?
>
> > >
> > > > +
> > > > + /* 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.
> >
>
> IMO, the following union can clarify all doubts.
> @Ananyev, Konstantin: Any suggestions from your side?
>
> /** IV and aad information for various use cases. */
> 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;
> } cpu_crypto; < or any other useful name>
> /* Supposed to be used with HW raw crypto API call. */
> 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;
> } hw_chain;
> /* Supposed to be used with HW raw crypto API call. */
> 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;
> } hw_aead;
> };
>
>
[Structure looks good to me thanks!]
>
> > > > +/**
> > > > + * 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.
>
> So this structure is saved and filled by the lib/driver and not the application.
> Right?
> This struct is opaque to application and will be part of session private data.
> Right?
> Assignment and calling appropriate driver's call backs will be hidden inside
> library
> and will be opaque to the application. In other words, the structure is not
> exposed
> to the application.
> Please add relevant comments on top of this structure.
>
[Fan: will do]
>
> > > > +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
>
> Comment not complete.
>
> Regards,
> Akhil
Hi AKhil
> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Monday, September 21, 2020 1:00 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> 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
>
...
> IMO, the following union can clarify all doubts.
> @Ananyev, Konstantin: Any suggestions from your side?
>
> /** IV and aad information for various use cases. */
> 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;
> } cpu_crypto; < or any other useful name>
> /* Supposed to be used with HW raw crypto API call. */
> 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;
> } hw_chain;
> /* Supposed to be used with HW raw crypto API call. */
> 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;
> } hw_aead;
> };
>
>
The above structure cannot support the array of multiple jobs but a single job.
So we have to use something like
struct {
void **cipher_iv_ptr;
rtei_iova_t *cipher_iv_iova;
...
} hw_chain;
struct {
void **iv_ptr;
rte_iova_t *iv_iova;
...
} hw_aead;
Is it ok?
Regards,
Fan
Hi Fan,
> Hi AKhil
>
> ...
> > IMO, the following union can clarify all doubts.
> > @Ananyev, Konstantin: Any suggestions from your side?
> >
> > /** IV and aad information for various use cases. */
> > 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;
> > } cpu_crypto; < or any other useful name>
> > /* Supposed to be used with HW raw crypto API call. */
> > 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;
> > } hw_chain;
> > /* Supposed to be used with HW raw crypto API call. */
> > 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;
> > } hw_aead;
> > };
> >
> >
>
> The above structure cannot support the array of multiple jobs but a single job.
So was your previous structure. Was it not tested before?
> So we have to use something like
>
> struct {
> void **cipher_iv_ptr;
You can even drop _ptr from the name of each of them.
> rtei_iova_t *cipher_iv_iova;
> ...
> } hw_chain;
> struct {
> void **iv_ptr;
> rte_iova_t *iv_iova;
> ...
> } hw_aead;
>
> Is it ok?
>
> Regards,
> Fan
Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Monday, September 21, 2020 4:49 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> 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,
> > Hi AKhil
> >
> > ...
> > > IMO, the following union can clarify all doubts.
> > > @Ananyev, Konstantin: Any suggestions from your side?
> > >
> > > /** IV and aad information for various use cases. */
> > > 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;
> > > } cpu_crypto; < or any other useful name>
> > > /* Supposed to be used with HW raw crypto API call. */
> > > 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;
> > > } hw_chain;
> > > /* Supposed to be used with HW raw crypto API call. */
> > > 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;
> > > } hw_aead;
> > > };
> > >
> > >
> >
> > The above structure cannot support the array of multiple jobs but a single
> job.
>
> So was your previous structure. Was it not tested before?
Of course it was tested in both DPDK and VPP. The previous structure is an array to the union additional_data
" union rte_crypto_sym_additional_data *additional_data;". That's why the union was declared in the first place.
I am ok with the way you propose with "*" in the beginning of every member.
>
> > So we have to use something like
> >
> > struct {
> > void **cipher_iv_ptr;
>
> You can even drop _ptr from the name of each of them.
>
> > rtei_iova_t *cipher_iv_iova;
> > ...
> > } hw_chain;
> > struct {
> > void **iv_ptr;
> > rte_iova_t *iv_iova;
> > ...
> > } hw_aead;
> >
> > Is it ok?
> >
> > Regards,
> > Fan
Regards,
Fan
Hi Akhil,
Thanks again for the review!
To summarize, the following places to be changed for v10.
1. Documentation update and reviewed internally in Intel first.
2. Add the missing comments to the structure.
3. Change the name "dp_service" to "raw_dp" to all APIs and documentation.
4. Change the structure
struct rte_crypto_sym_vec {
/** array of SGL vectors */
struct rte_crypto_sgl *sgl;
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;
} cpu_crypto;
/** Supposed to be used with HW raw crypto API call. */
struct {
/** array of pointers to cipher IV */
void **cipher_iv_ptr;
/** array of IOVA addresses to cipher IV */
rte_iova_t *cipher_iv_iova;
/** array of pointers to auth IV */
void **auth_iv_ptr;
/** array of IOVA addresses to auth IV */
rte_iova_t *auth_iv_iova;
/** array of pointers to digest */
void **digest_ptr;
/** array of IOVA addresses to digest */
rte_iova_t *digest_iova;
} hw_chain;
/** Supposed to be used with HW raw crypto API call. */
struct {
/** array of pointers to AEAD IV */
void **iv_ptr;
/** array of IOVA addresses to AEAD IV */
rte_iova_t *iv_iova;
/** array of pointers to AAD */
void **aad_ptr;
/** array of IOVA addresses to AAD */
rte_iova_t *aad_iova;
/** array of pointers to digest */
void **digest_ptr;
/** array of IOVA addresses to digest */
rte_iova_t *digest_iova;
} hw_aead;
};
/**
* array of statuses for each operation:
* - 0 on success
* - errno on error
*/
int32_t *status;
/** number of operations to perform */
uint32_t num;
};
5. Remove enum rte_crypto_dp_service, let the PMDs using the session private data to decide function handler.
6. Remove is_update parameter.
The main point that is uncertain is the existance of "submit_single".
I am ok to remove "submit_single" function. In VPP we can use rte_cryptodev_dp_sym_submit_vec() with vec.num=1 each time to avoid double looping.
But we have to put the rte_cryptodev_dp_sym_submit_vec() as an inline function - this will cause the API not traced in version map.
Any ideas?
Regards,
Fan
> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Monday, September 21, 2020 4:49 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> 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,
> > Hi AKhil
> >
> > ...
> > > IMO, the following union can clarify all doubts.
> > > @Ananyev, Konstantin: Any suggestions from your side?
> > >
> > > /** IV and aad information for various use cases. */
> > > 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;
> > > } cpu_crypto; < or any other useful name>
> > > /* Supposed to be used with HW raw crypto API call. */
> > > 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;
> > > } hw_chain;
> > > /* Supposed to be used with HW raw crypto API call. */
> > > 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;
> > > } hw_aead;
> > > };
> > >
> > >
> >
> > The above structure cannot support the array of multiple jobs but a single
> job.
>
> So was your previous structure. Was it not tested before?
>
> > So we have to use something like
> >
> > struct {
> > void **cipher_iv_ptr;
>
> You can even drop _ptr from the name of each of them.
>
> > rtei_iova_t *cipher_iv_iova;
> > ...
> > } hw_chain;
> > struct {
> > void **iv_ptr;
> > rte_iova_t *iv_iova;
> > ...
> > } hw_aead;
> >
> > Is it ok?
> >
> > Regards,
> > Fan
Hi lads,
>
> Hi Akhil,
>
> Thanks again for the review!
> To summarize, the following places to be changed for v10.
>
> 1. Documentation update and reviewed internally in Intel first.
> 2. Add the missing comments to the structure.
> 3. Change the name "dp_service" to "raw_dp" to all APIs and documentation.
> 4. Change the structure
> struct rte_crypto_sym_vec {
> /** array of SGL vectors */
> struct rte_crypto_sgl *sgl;
>
> 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;
> } cpu_crypto;
> /** Supposed to be used with HW raw crypto API call. */
> struct {
> /** array of pointers to cipher IV */
> void **cipher_iv_ptr;
> /** array of IOVA addresses to cipher IV */
> rte_iova_t *cipher_iv_iova;
> /** array of pointers to auth IV */
> void **auth_iv_ptr;
> /** array of IOVA addresses to auth IV */
> rte_iova_t *auth_iv_iova;
> /** array of pointers to digest */
> void **digest_ptr;
> /** array of IOVA addresses to digest */
> rte_iova_t *digest_iova;
> } hw_chain;
> /** Supposed to be used with HW raw crypto API call. */
> struct {
> /** array of pointers to AEAD IV */
> void **iv_ptr;
> /** array of IOVA addresses to AEAD IV */
> rte_iova_t *iv_iova;
> /** array of pointers to AAD */
> void **aad_ptr;
> /** array of IOVA addresses to AAD */
> rte_iova_t *aad_iova;
> /** array of pointers to digest */
> void **digest_ptr;
> /** array of IOVA addresses to digest */
> rte_iova_t *digest_iova;
> } hw_aead;
> };
>
> /**
> * array of statuses for each operation:
> * - 0 on success
> * - errno on error
> */
> int32_t *status;
> /** number of operations to perform */
> uint32_t num;
> };
As I understand you just need to add pointers to iova[] for iv, aad and digest, correct?
If so, why not simply:
struct rte_va_iova_ptr {
void *va;
rte_iova_t *iova;
};
struct rte_crypto_sym_vec {
/** array of SGL vectors */
struct rte_crypto_sgl *sgl;
/** array of pointers to IV */
struct rte_va_iova_ptr iv;
/** array of pointers to AAD */
struct rte_va_iova_ptr aad;
/** array of pointers to digest */
struct rte_va_iova_ptr digest;
/**
* array of statuses for each operation:
* - 0 on success
* - errno on error
*/
int32_t *status;
/** number of operations to perform */
uint32_t num;
};
BTW, it would be both ABI and API breakage,
though all functions using this struct are marked as experimental,
plus it is an LTS release, so it seems to be ok.
Though I think it needs to be flagged in RN.
Another option obviously - introduce completely new structure for it
and leave existing one unaffected.
>
> 5. Remove enum rte_crypto_dp_service, let the PMDs using the session private data to decide function handler.
> 6. Remove is_update parameter.
>
> The main point that is uncertain is the existance of "submit_single".
> I am ok to remove "submit_single" function. In VPP we can use rte_cryptodev_dp_sym_submit_vec() with vec.num=1 each time to avoid
> double looping.
> But we have to put the rte_cryptodev_dp_sym_submit_vec() as an inline function - this will cause the API not traced in version map.
>
> Any ideas?
>
> Regards,
> Fan
>
> > -----Original Message-----
> > From: Akhil Goyal <akhil.goyal@nxp.com>
> > Sent: Monday, September 21, 2020 4:49 PM
> > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org; Ananyev,
> > Konstantin <konstantin.ananyev@intel.com>; Thomas Monjalon
> > <thomas@monjalon.net>
> > 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,
> > > Hi AKhil
> > >
> > > ...
> > > > IMO, the following union can clarify all doubts.
> > > > @Ananyev, Konstantin: Any suggestions from your side?
> > > >
> > > > /** IV and aad information for various use cases. */
> > > > 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;
> > > > } cpu_crypto; < or any other useful name>
> > > > /* Supposed to be used with HW raw crypto API call. */
> > > > 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;
> > > > } hw_chain;
> > > > /* Supposed to be used with HW raw crypto API call. */
> > > > 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;
> > > > } hw_aead;
> > > > };
> > > >
> > > >
> > >
> > > The above structure cannot support the array of multiple jobs but a single
> > job.
> >
> > So was your previous structure. Was it not tested before?
> >
> > > So we have to use something like
> > >
> > > struct {
> > > void **cipher_iv_ptr;
> >
> > You can even drop _ptr from the name of each of them.
> >
> > > rtei_iova_t *cipher_iv_iova;
> > > ...
> > > } hw_chain;
> > > struct {
> > > void **iv_ptr;
> > > rte_iova_t *iv_iova;
> > > ...
> > > } hw_aead;
> > >
> > > Is it ok?
> > >
> > > Regards,
> > > Fan
Hi Konstantin,
> Hi lads,
>
> >
> > Hi Akhil,
> >
> > Thanks again for the review!
> > To summarize, the following places to be changed for v10.
> >
> > 1. Documentation update and reviewed internally in Intel first.
> > 2. Add the missing comments to the structure.
> > 3. Change the name "dp_service" to "raw_dp" to all APIs and documentation.
> > 4. Change the structure
> > struct rte_crypto_sym_vec {
> > /** array of SGL vectors */
> > struct rte_crypto_sgl *sgl;
> >
> > 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;
> > } cpu_crypto;
> > /** Supposed to be used with HW raw crypto API call. */
> > struct {
> > /** array of pointers to cipher IV */
> > void **cipher_iv_ptr;
> > /** array of IOVA addresses to cipher IV */
> > rte_iova_t *cipher_iv_iova;
> > /** array of pointers to auth IV */
> > void **auth_iv_ptr;
> > /** array of IOVA addresses to auth IV */
> > rte_iova_t *auth_iv_iova;
> > /** array of pointers to digest */
> > void **digest_ptr;
> > /** array of IOVA addresses to digest */
> > rte_iova_t *digest_iova;
> > } hw_chain;
> > /** Supposed to be used with HW raw crypto API call. */
> > struct {
> > /** array of pointers to AEAD IV */
> > void **iv_ptr;
> > /** array of IOVA addresses to AEAD IV */
> > rte_iova_t *iv_iova;
> > /** array of pointers to AAD */
> > void **aad_ptr;
> > /** array of IOVA addresses to AAD */
> > rte_iova_t *aad_iova;
> > /** array of pointers to digest */
> > void **digest_ptr;
> > /** array of IOVA addresses to digest */
> > rte_iova_t *digest_iova;
> > } hw_aead;
> > };
> >
> > /**
> > * array of statuses for each operation:
> > * - 0 on success
> > * - errno on error
> > */
> > int32_t *status;
> > /** number of operations to perform */
> > uint32_t num;
> > };
>
>
> As I understand you just need to add pointers to iova[] for iv, aad and digest,
> correct?
> If so, why not simply:
>
> struct rte_va_iova_ptr {
> void *va;
> rte_iova_t *iova;
> };
>
> struct rte_crypto_sym_vec {
> /** array of SGL vectors */
> struct rte_crypto_sgl *sgl;
> /** array of pointers to IV */
> struct rte_va_iova_ptr iv;
> /** array of pointers to AAD */
> struct rte_va_iova_ptr aad;
> /** array of pointers to digest */
> struct rte_va_iova_ptr digest;
> /**
> * array of statuses for each operation:
> * - 0 on success
> * - errno on error
> */
> int32_t *status;
> /** number of operations to perform */
> uint32_t num;
> };
>
> BTW, it would be both ABI and API breakage,
> though all functions using this struct are marked as experimental,
> plus it is an LTS release, so it seems to be ok.
> Though I think it needs to be flagged in RN.
This is a good suggestion. This will make some changes in the cpu-crypto support as well
And should be a separate patch.
We can take the API and ABI breakage in this release. That is not an issue.
>
> Another option obviously - introduce completely new structure for it
> and leave existing one unaffected.
>
This will create some duplicate code. Would not prefer that.
> >
> > 5. Remove enum rte_crypto_dp_service, let the PMDs using the session private
> data to decide function handler.
> > 6. Remove is_update parameter.
> >
> > The main point that is uncertain is the existance of "submit_single".
> > I am ok to remove "submit_single" function. In VPP we can use
> rte_cryptodev_dp_sym_submit_vec() with vec.num=1 each time to avoid
> > double looping.
> > But we have to put the rte_cryptodev_dp_sym_submit_vec() as an inline
> function - this will cause the API not traced in version map.
> >
> > Any ideas?
> >
> > Regards,
> > Fan
> >
Hi Akhil and Konstantin,
> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Tuesday, September 22, 2020 10:06 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; dev@dpdk.org; Thomas Monjalon
> <thomas@monjalon.net>
> 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 Konstantin,
> > Hi lads,
> >
> > >
> > > Hi Akhil,
> > >
> > > Thanks again for the review!
> > > To summarize, the following places to be changed for v10.
> > >
> > > 1. Documentation update and reviewed internally in Intel first.
> > > 2. Add the missing comments to the structure.
> > > 3. Change the name "dp_service" to "raw_dp" to all APIs and
> documentation.
> > > 4. Change the structure
> > > struct rte_crypto_sym_vec {
> > > /** array of SGL vectors */
> > > struct rte_crypto_sgl *sgl;
> > >
> > > 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;
> > > } cpu_crypto;
> > > /** Supposed to be used with HW raw crypto API call. */
> > > struct {
> > > /** array of pointers to cipher IV */
> > > void **cipher_iv_ptr;
> > > /** array of IOVA addresses to cipher IV */
> > > rte_iova_t *cipher_iv_iova;
> > > /** array of pointers to auth IV */
> > > void **auth_iv_ptr;
> > > /** array of IOVA addresses to auth IV */
> > > rte_iova_t *auth_iv_iova;
> > > /** array of pointers to digest */
> > > void **digest_ptr;
> > > /** array of IOVA addresses to digest */
> > > rte_iova_t *digest_iova;
> > > } hw_chain;
> > > /** Supposed to be used with HW raw crypto API call. */
> > > struct {
> > > /** array of pointers to AEAD IV */
> > > void **iv_ptr;
> > > /** array of IOVA addresses to AEAD IV */
> > > rte_iova_t *iv_iova;
> > > /** array of pointers to AAD */
> > > void **aad_ptr;
> > > /** array of IOVA addresses to AAD */
> > > rte_iova_t *aad_iova;
> > > /** array of pointers to digest */
> > > void **digest_ptr;
> > > /** array of IOVA addresses to digest */
> > > rte_iova_t *digest_iova;
> > > } hw_aead;
> > > };
> > >
> > > /**
> > > * array of statuses for each operation:
> > > * - 0 on success
> > > * - errno on error
> > > */
> > > int32_t *status;
> > > /** number of operations to perform */
> > > uint32_t num;
> > > };
> >
> >
> > As I understand you just need to add pointers to iova[] for iv, aad and
> digest,
> > correct?
> > If so, why not simply:
> >
> > struct rte_va_iova_ptr {
> > void *va;
> > rte_iova_t *iova;
> > };
> >
> > struct rte_crypto_sym_vec {
> > /** array of SGL vectors */
> > struct rte_crypto_sgl *sgl;
> > /** array of pointers to IV */
> > struct rte_va_iova_ptr iv;
We will need 2 IV here, one for cipher and one for auth (GMAC for example).
> > /** array of pointers to AAD */
> > struct rte_va_iova_ptr aad;
> > /** array of pointers to digest */
> > struct rte_va_iova_ptr digest;
> > /**
> > * array of statuses for each operation:
> > * - 0 on success
> > * - errno on error
> > */
> > int32_t *status;
> > /** number of operations to perform */
> > uint32_t num;
> > };
> >
> > BTW, it would be both ABI and API breakage,
> > though all functions using this struct are marked as experimental,
> > plus it is an LTS release, so it seems to be ok.
> > Though I think it needs to be flagged in RN.
>
> This is a good suggestion. This will make some changes in the cpu-crypto
> support as well
> And should be a separate patch.
> We can take the API and ABI breakage in this release. That is not an issue.
>
>
> >
> > Another option obviously - introduce completely new structure for it
> > and leave existing one unaffected.
> >
> This will create some duplicate code. Would not prefer that.
>
> > >
> > > 5. Remove enum rte_crypto_dp_service, let the PMDs using the session
> private
> > data to decide function handler.
> > > 6. Remove is_update parameter.
> > >
> > > The main point that is uncertain is the existance of "submit_single".
> > > I am ok to remove "submit_single" function. In VPP we can use
> > rte_cryptodev_dp_sym_submit_vec() with vec.num=1 each time to avoid
> > > double looping.
> > > But we have to put the rte_cryptodev_dp_sym_submit_vec() as an inline
> > function - this will cause the API not traced in version map.
> > >
> > > Any ideas?
> > >
> > > Regards,
> > > Fan
> > >
>
> Hi Akhil and Konstantin,
>
> > -----Original Message-----
> > From: Akhil Goyal <akhil.goyal@nxp.com>
> > Sent: Tuesday, September 22, 2020 10:06 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Roy Fan
> > <roy.fan.zhang@intel.com>; dev@dpdk.org; Thomas Monjalon
> > <thomas@monjalon.net>
> > 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 Konstantin,
> > > Hi lads,
> > >
> > > >
> > > > Hi Akhil,
> > > >
> > > > Thanks again for the review!
> > > > To summarize, the following places to be changed for v10.
> > > >
> > > > 1. Documentation update and reviewed internally in Intel first.
> > > > 2. Add the missing comments to the structure.
> > > > 3. Change the name "dp_service" to "raw_dp" to all APIs and
> > documentation.
> > > > 4. Change the structure
> > > > struct rte_crypto_sym_vec {
> > > > /** array of SGL vectors */
> > > > struct rte_crypto_sgl *sgl;
> > > >
> > > > 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;
> > > > } cpu_crypto;
> > > > /** Supposed to be used with HW raw crypto API call. */
> > > > struct {
> > > > /** array of pointers to cipher IV */
> > > > void **cipher_iv_ptr;
> > > > /** array of IOVA addresses to cipher IV */
> > > > rte_iova_t *cipher_iv_iova;
> > > > /** array of pointers to auth IV */
> > > > void **auth_iv_ptr;
> > > > /** array of IOVA addresses to auth IV */
> > > > rte_iova_t *auth_iv_iova;
> > > > /** array of pointers to digest */
> > > > void **digest_ptr;
> > > > /** array of IOVA addresses to digest */
> > > > rte_iova_t *digest_iova;
> > > > } hw_chain;
> > > > /** Supposed to be used with HW raw crypto API call. */
> > > > struct {
> > > > /** array of pointers to AEAD IV */
> > > > void **iv_ptr;
> > > > /** array of IOVA addresses to AEAD IV */
> > > > rte_iova_t *iv_iova;
> > > > /** array of pointers to AAD */
> > > > void **aad_ptr;
> > > > /** array of IOVA addresses to AAD */
> > > > rte_iova_t *aad_iova;
> > > > /** array of pointers to digest */
> > > > void **digest_ptr;
> > > > /** array of IOVA addresses to digest */
> > > > rte_iova_t *digest_iova;
> > > > } hw_aead;
> > > > };
> > > >
> > > > /**
> > > > * array of statuses for each operation:
> > > > * - 0 on success
> > > > * - errno on error
> > > > */
> > > > int32_t *status;
> > > > /** number of operations to perform */
> > > > uint32_t num;
> > > > };
> > >
> > >
> > > As I understand you just need to add pointers to iova[] for iv, aad and
> > digest,
> > > correct?
> > > If so, why not simply:
> > >
> > > struct rte_va_iova_ptr {
> > > void *va;
> > > rte_iova_t *iova;
> > > };
> > >
> > > struct rte_crypto_sym_vec {
> > > /** array of SGL vectors */
> > > struct rte_crypto_sgl *sgl;
> > > /** array of pointers to IV */
> > > struct rte_va_iova_ptr iv;
>
> We will need 2 IV here, one for cipher and one for auth (GMAC for example).
Hmm, why do we need to different IVs for GMAC?
And if so how does it work now with either rte_crypto_op or with rte_crypto_sym_vec?
>
> > > /** array of pointers to AAD */
> > > struct rte_va_iova_ptr aad;
> > > /** array of pointers to digest */
> > > struct rte_va_iova_ptr digest;
> > > /**
> > > * array of statuses for each operation:
> > > * - 0 on success
> > > * - errno on error
> > > */
> > > int32_t *status;
> > > /** number of operations to perform */
> > > uint32_t num;
> > > };
> > >
> > > BTW, it would be both ABI and API breakage,
> > > though all functions using this struct are marked as experimental,
> > > plus it is an LTS release, so it seems to be ok.
> > > Though I think it needs to be flagged in RN.
> >
> > This is a good suggestion. This will make some changes in the cpu-crypto
> > support as well
> > And should be a separate patch.
> > We can take the API and ABI breakage in this release. That is not an issue.
> >
> >
> > >
> > > Another option obviously - introduce completely new structure for it
> > > and leave existing one unaffected.
> > >
> > This will create some duplicate code. Would not prefer that.
> >
> > > >
> > > > 5. Remove enum rte_crypto_dp_service, let the PMDs using the session
> > private
> > > data to decide function handler.
> > > > 6. Remove is_update parameter.
> > > >
> > > > The main point that is uncertain is the existance of "submit_single".
> > > > I am ok to remove "submit_single" function. In VPP we can use
> > > rte_cryptodev_dp_sym_submit_vec() with vec.num=1 each time to avoid
> > > > double looping.
> > > > But we have to put the rte_cryptodev_dp_sym_submit_vec() as an inline
> > > function - this will cause the API not traced in version map.
> > > >
> > > > Any ideas?
> > > >
> > > > Regards,
> > > > Fan
> > > >
Hi Konstantin,
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Tuesday, September 22, 2020 11:18 AM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; dev@dpdk.org; Thomas Monjalon
> <thomas@monjalon.net>
> 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 Akhil and Konstantin,
> >
> > > -----Original Message-----
> > > From: Akhil Goyal <akhil.goyal@nxp.com>
> > > Sent: Tuesday, September 22, 2020 10:06 AM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Roy
> Fan
> > > <roy.fan.zhang@intel.com>; dev@dpdk.org; Thomas Monjalon
> > > <thomas@monjalon.net>
> > > 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 Konstantin,
> > > > Hi lads,
> > > >
> > > > >
> > > > > Hi Akhil,
> > > > >
> > > > > Thanks again for the review!
> > > > > To summarize, the following places to be changed for v10.
> > > > >
> > > > > 1. Documentation update and reviewed internally in Intel first.
> > > > > 2. Add the missing comments to the structure.
> > > > > 3. Change the name "dp_service" to "raw_dp" to all APIs and
> > > documentation.
> > > > > 4. Change the structure
> > > > > struct rte_crypto_sym_vec {
> > > > > /** array of SGL vectors */
> > > > > struct rte_crypto_sgl *sgl;
> > > > >
> > > > > 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;
> > > > > } cpu_crypto;
> > > > > /** Supposed to be used with HW raw crypto API call. */
> > > > > struct {
> > > > > /** array of pointers to cipher IV */
> > > > > void **cipher_iv_ptr;
> > > > > /** array of IOVA addresses to cipher IV */
> > > > > rte_iova_t *cipher_iv_iova;
> > > > > /** array of pointers to auth IV */
> > > > > void **auth_iv_ptr;
> > > > > /** array of IOVA addresses to auth IV */
> > > > > rte_iova_t *auth_iv_iova;
> > > > > /** array of pointers to digest */
> > > > > void **digest_ptr;
> > > > > /** array of IOVA addresses to digest */
> > > > > rte_iova_t *digest_iova;
> > > > > } hw_chain;
> > > > > /** Supposed to be used with HW raw crypto API call. */
> > > > > struct {
> > > > > /** array of pointers to AEAD IV */
> > > > > void **iv_ptr;
> > > > > /** array of IOVA addresses to AEAD IV */
> > > > > rte_iova_t *iv_iova;
> > > > > /** array of pointers to AAD */
> > > > > void **aad_ptr;
> > > > > /** array of IOVA addresses to AAD */
> > > > > rte_iova_t *aad_iova;
> > > > > /** array of pointers to digest */
> > > > > void **digest_ptr;
> > > > > /** array of IOVA addresses to digest */
> > > > > rte_iova_t *digest_iova;
> > > > > } hw_aead;
> > > > > };
> > > > >
> > > > > /**
> > > > > * array of statuses for each operation:
> > > > > * - 0 on success
> > > > > * - errno on error
> > > > > */
> > > > > int32_t *status;
> > > > > /** number of operations to perform */
> > > > > uint32_t num;
> > > > > };
> > > >
> > > >
> > > > As I understand you just need to add pointers to iova[] for iv, aad and
> > > digest,
> > > > correct?
> > > > If so, why not simply:
> > > >
> > > > struct rte_va_iova_ptr {
> > > > void *va;
> > > > rte_iova_t *iova;
> > > > };
> > > >
> > > > struct rte_crypto_sym_vec {
> > > > /** array of SGL vectors */
> > > > struct rte_crypto_sgl *sgl;
> > > > /** array of pointers to IV */
> > > > struct rte_va_iova_ptr iv;
> >
> > We will need 2 IV here, one for cipher and one for auth (GMAC for
> example).
>
> Hmm, why do we need to different IVs for GMAC?
> And if so how does it work now with either rte_crypto_op or with
> rte_crypto_sym_vec?
>
Not only GMAC, the wireless chain algos such as SNOW3G requires IV in auth field (test_snow3g_cipher_auth() in unit test).
Rte_crypto_sym_op has auth_iv.offset to indicate where the iv is stored in crypto op, so the virtual and physical addresses of it can be deducted through the offset, but not yet for rte_crypto_sym_vec.
> >
> > > > /** array of pointers to AAD */
> > > > struct rte_va_iova_ptr aad;
> > > > /** array of pointers to digest */
> > > > struct rte_va_iova_ptr digest;
> > > > /**
> > > > * array of statuses for each operation:
> > > > * - 0 on success
> > > > * - errno on error
> > > > */
> > > > int32_t *status;
> > > > /** number of operations to perform */
> > > > uint32_t num;
> > > > };
> > > >
> > > > BTW, it would be both ABI and API breakage,
> > > > though all functions using this struct are marked as experimental,
> > > > plus it is an LTS release, so it seems to be ok.
> > > > Though I think it needs to be flagged in RN.
> > >
> > > This is a good suggestion. This will make some changes in the cpu-crypto
> > > support as well
> > > And should be a separate patch.
> > > We can take the API and ABI breakage in this release. That is not an issue.
> > >
> > >
> > > >
> > > > Another option obviously - introduce completely new structure for it
> > > > and leave existing one unaffected.
> > > >
> > > This will create some duplicate code. Would not prefer that.
> > >
> > > > >
> > > > > 5. Remove enum rte_crypto_dp_service, let the PMDs using the
> session
> > > private
> > > > data to decide function handler.
> > > > > 6. Remove is_update parameter.
> > > > >
> > > > > The main point that is uncertain is the existance of "submit_single".
> > > > > I am ok to remove "submit_single" function. In VPP we can use
> > > > rte_cryptodev_dp_sym_submit_vec() with vec.num=1 each time to
> avoid
> > > > > double looping.
> > > > > But we have to put the rte_cryptodev_dp_sym_submit_vec() as an
> inline
> > > > function - this will cause the API not traced in version map.
> > > > >
> > > > > Any ideas?
> > > > >
> > > > > Regards,
> > > > > Fan
> > > > >
Hi Akhil,
Konstantin and I had an off-line discussion. Is this structure ok for you?
/**
* Crypto virtual and IOVA address descriptor. Used to describe cryptographic
* data without
*
*/
struct rte_crypto_va_iova_ptr {
void *va;
rte_iova_t *iova;
};
/**
* Raw data operation descriptor.
* Supposed to be used with synchronous CPU crypto API call or asynchronous
* RAW data path API call.
*/
struct rte_crypto_sym_vec {
/** array of SGL vectors */
struct rte_crypto_sgl *sgl;
/** array of pointers to cipher IV */
struct rte_crypto_va_iova_ptr *iv;
/** array of pointers to digest */
struct rte_crypto_va_iova_ptr *digest;
__extension__
union {
/** array of pointers to auth IV, used for chain operation */
struct rte_crypto_va_iova_ptr *auth_iv;
/** array of pointers to AAD, used for AEAD operation */
struct rte_crypto_va_iova_ptr *aad;
};
/**
* array of statuses for each operation:
* - 0 on success
* - errno on error
*/
int32_t *status;
/** number of operations to perform */
uint32_t num;
};
Regards,
Fan
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Tuesday, September 22, 2020 11:18 AM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; dev@dpdk.org; Thomas Monjalon
> <thomas@monjalon.net>
> 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 Akhil and Konstantin,
> >
> > > -----Original Message-----
> > > From: Akhil Goyal <akhil.goyal@nxp.com>
> > > Sent: Tuesday, September 22, 2020 10:06 AM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Roy
> Fan
> > > <roy.fan.zhang@intel.com>; dev@dpdk.org; Thomas Monjalon
> > > <thomas@monjalon.net>
> > > 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 Konstantin,
> > > > Hi lads,
> > > >
> > > > >
> > > > > Hi Akhil,
> > > > >
> > > > > Thanks again for the review!
> > > > > To summarize, the following places to be changed for v10.
> > > > >
> > > > > 1. Documentation update and reviewed internally in Intel first.
> > > > > 2. Add the missing comments to the structure.
> > > > > 3. Change the name "dp_service" to "raw_dp" to all APIs and
> > > documentation.
> > > > > 4. Change the structure
> > > > > struct rte_crypto_sym_vec {
> > > > > /** array of SGL vectors */
> > > > > struct rte_crypto_sgl *sgl;
> > > > >
> > > > > 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;
> > > > > } cpu_crypto;
> > > > > /** Supposed to be used with HW raw crypto API call. */
> > > > > struct {
> > > > > /** array of pointers to cipher IV */
> > > > > void **cipher_iv_ptr;
> > > > > /** array of IOVA addresses to cipher IV */
> > > > > rte_iova_t *cipher_iv_iova;
> > > > > /** array of pointers to auth IV */
> > > > > void **auth_iv_ptr;
> > > > > /** array of IOVA addresses to auth IV */
> > > > > rte_iova_t *auth_iv_iova;
> > > > > /** array of pointers to digest */
> > > > > void **digest_ptr;
> > > > > /** array of IOVA addresses to digest */
> > > > > rte_iova_t *digest_iova;
> > > > > } hw_chain;
> > > > > /** Supposed to be used with HW raw crypto API call. */
> > > > > struct {
> > > > > /** array of pointers to AEAD IV */
> > > > > void **iv_ptr;
> > > > > /** array of IOVA addresses to AEAD IV */
> > > > > rte_iova_t *iv_iova;
> > > > > /** array of pointers to AAD */
> > > > > void **aad_ptr;
> > > > > /** array of IOVA addresses to AAD */
> > > > > rte_iova_t *aad_iova;
> > > > > /** array of pointers to digest */
> > > > > void **digest_ptr;
> > > > > /** array of IOVA addresses to digest */
> > > > > rte_iova_t *digest_iova;
> > > > > } hw_aead;
> > > > > };
> > > > >
> > > > > /**
> > > > > * array of statuses for each operation:
> > > > > * - 0 on success
> > > > > * - errno on error
> > > > > */
> > > > > int32_t *status;
> > > > > /** number of operations to perform */
> > > > > uint32_t num;
> > > > > };
> > > >
> > > >
> > > > As I understand you just need to add pointers to iova[] for iv, aad and
> > > digest,
> > > > correct?
> > > > If so, why not simply:
> > > >
> > > > struct rte_va_iova_ptr {
> > > > void *va;
> > > > rte_iova_t *iova;
> > > > };
> > > >
> > > > struct rte_crypto_sym_vec {
> > > > /** array of SGL vectors */
> > > > struct rte_crypto_sgl *sgl;
> > > > /** array of pointers to IV */
> > > > struct rte_va_iova_ptr iv;
> >
> > We will need 2 IV here, one for cipher and one for auth (GMAC for
> example).
>
> Hmm, why do we need to different IVs for GMAC?
> And if so how does it work now with either rte_crypto_op or with
> rte_crypto_sym_vec?
>
> >
> > > > /** array of pointers to AAD */
> > > > struct rte_va_iova_ptr aad;
> > > > /** array of pointers to digest */
> > > > struct rte_va_iova_ptr digest;
> > > > /**
> > > > * array of statuses for each operation:
> > > > * - 0 on success
> > > > * - errno on error
> > > > */
> > > > int32_t *status;
> > > > /** number of operations to perform */
> > > > uint32_t num;
> > > > };
> > > >
> > > > BTW, it would be both ABI and API breakage,
> > > > though all functions using this struct are marked as experimental,
> > > > plus it is an LTS release, so it seems to be ok.
> > > > Though I think it needs to be flagged in RN.
> > >
> > > This is a good suggestion. This will make some changes in the cpu-crypto
> > > support as well
> > > And should be a separate patch.
> > > We can take the API and ABI breakage in this release. That is not an issue.
> > >
> > >
> > > >
> > > > Another option obviously - introduce completely new structure for it
> > > > and leave existing one unaffected.
> > > >
> > > This will create some duplicate code. Would not prefer that.
> > >
> > > > >
> > > > > 5. Remove enum rte_crypto_dp_service, let the PMDs using the
> session
> > > private
> > > > data to decide function handler.
> > > > > 6. Remove is_update parameter.
> > > > >
> > > > > The main point that is uncertain is the existance of "submit_single".
> > > > > I am ok to remove "submit_single" function. In VPP we can use
> > > > rte_cryptodev_dp_sym_submit_vec() with vec.num=1 each time to
> avoid
> > > > > double looping.
> > > > > But we have to put the rte_cryptodev_dp_sym_submit_vec() as an
> inline
> > > > function - this will cause the API not traced in version map.
> > > > >
> > > > > Any ideas?
> > > > >
> > > > > Regards,
> > > > > Fan
> > > > >
Hi Fan,
> Hi Akhil,
>
> Konstantin and I had an off-line discussion. Is this structure ok for you?
>
> /**
> * Crypto virtual and IOVA address descriptor. Used to describe cryptographic
> * data without
The comment is incomplete, however the structure is fine.
> *
> */
> struct rte_crypto_va_iova_ptr {
> void *va;
> rte_iova_t *iova;
> };
>
> /**
> * Raw data operation descriptor.
> * Supposed to be used with synchronous CPU crypto API call or asynchronous
> * RAW data path API call.
> */
> struct rte_crypto_sym_vec {
> /** array of SGL vectors */
> struct rte_crypto_sgl *sgl;
> /** array of pointers to cipher IV */
> struct rte_crypto_va_iova_ptr *iv;
> /** array of pointers to digest */
> struct rte_crypto_va_iova_ptr *digest;
>
> __extension__
> union {
> /** array of pointers to auth IV, used for chain operation */
> struct rte_crypto_va_iova_ptr *auth_iv;
> /** array of pointers to AAD, used for AEAD operation */
> struct rte_crypto_va_iova_ptr *aad;
> };
>
> /**
> * array of statuses for each operation:
> * - 0 on success
> * - errno on error
> */
> int32_t *status;
> /** number of operations to perform */
> uint32_t num;
> };
>
> Regards,
> Fan
>
> > -----Original Message-----
> > From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Sent: Tuesday, September 22, 2020 11:18 AM
> > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Akhil Goyal
> > <akhil.goyal@nxp.com>; dev@dpdk.org; Thomas Monjalon
> > <thomas@monjalon.net>
> > 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 Akhil and Konstantin,
> > >
> > > > -----Original Message-----
> > > > From: Akhil Goyal <akhil.goyal@nxp.com>
> > > > Sent: Tuesday, September 22, 2020 10:06 AM
> > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Roy
> > Fan
> > > > <roy.fan.zhang@intel.com>; dev@dpdk.org; Thomas Monjalon
> > > > <thomas@monjalon.net>
> > > > 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 Konstantin,
> > > > > Hi lads,
> > > > >
> > > > > >
> > > > > > Hi Akhil,
> > > > > >
> > > > > > Thanks again for the review!
> > > > > > To summarize, the following places to be changed for v10.
> > > > > >
> > > > > > 1. Documentation update and reviewed internally in Intel first.
> > > > > > 2. Add the missing comments to the structure.
> > > > > > 3. Change the name "dp_service" to "raw_dp" to all APIs and
> > > > documentation.
> > > > > > 4. Change the structure
> > > > > > struct rte_crypto_sym_vec {
> > > > > > /** array of SGL vectors */
> > > > > > struct rte_crypto_sgl *sgl;
> > > > > >
> > > > > > 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;
> > > > > > } cpu_crypto;
> > > > > > /** Supposed to be used with HW raw crypto API call.
> */
> > > > > > struct {
> > > > > > /** array of pointers to cipher IV */
> > > > > > void **cipher_iv_ptr;
> > > > > > /** array of IOVA addresses to cipher IV */
> > > > > > rte_iova_t *cipher_iv_iova;
> > > > > > /** array of pointers to auth IV */
> > > > > > void **auth_iv_ptr;
> > > > > > /** array of IOVA addresses to auth IV */
> > > > > > rte_iova_t *auth_iv_iova;
> > > > > > /** array of pointers to digest */
> > > > > > void **digest_ptr;
> > > > > > /** array of IOVA addresses to digest */
> > > > > > rte_iova_t *digest_iova;
> > > > > > } hw_chain;
> > > > > > /** Supposed to be used with HW raw crypto API call.
> */
> > > > > > struct {
> > > > > > /** array of pointers to AEAD IV */
> > > > > > void **iv_ptr;
> > > > > > /** array of IOVA addresses to AEAD IV */
> > > > > > rte_iova_t *iv_iova;
> > > > > > /** array of pointers to AAD */
> > > > > > void **aad_ptr;
> > > > > > /** array of IOVA addresses to AAD */
> > > > > > rte_iova_t *aad_iova;
> > > > > > /** array of pointers to digest */
> > > > > > void **digest_ptr;
> > > > > > /** array of IOVA addresses to digest */
> > > > > > rte_iova_t *digest_iova;
> > > > > > } hw_aead;
> > > > > > };
> > > > > >
> > > > > > /**
> > > > > > * array of statuses for each operation:
> > > > > > * - 0 on success
> > > > > > * - errno on error
> > > > > > */
> > > > > > int32_t *status;
> > > > > > /** number of operations to perform */
> > > > > > uint32_t num;
> > > > > > };
> > > > >
> > > > >
> > > > > As I understand you just need to add pointers to iova[] for iv, aad and
> > > > digest,
> > > > > correct?
> > > > > If so, why not simply:
> > > > >
> > > > > struct rte_va_iova_ptr {
> > > > > void *va;
> > > > > rte_iova_t *iova;
> > > > > };
> > > > >
> > > > > struct rte_crypto_sym_vec {
> > > > > /** array of SGL vectors */
> > > > > struct rte_crypto_sgl *sgl;
> > > > > /** array of pointers to IV */
> > > > > struct rte_va_iova_ptr iv;
> > >
> > > We will need 2 IV here, one for cipher and one for auth (GMAC for
> > example).
> >
> > Hmm, why do we need to different IVs for GMAC?
> > And if so how does it work now with either rte_crypto_op or with
> > rte_crypto_sym_vec?
> >
> > >
> > > > > /** array of pointers to AAD */
> > > > > struct rte_va_iova_ptr aad;
> > > > > /** array of pointers to digest */
> > > > > struct rte_va_iova_ptr digest;
> > > > > /**
> > > > > * array of statuses for each operation:
> > > > > * - 0 on success
> > > > > * - errno on error
> > > > > */
> > > > > int32_t *status;
> > > > > /** number of operations to perform */
> > > > > uint32_t num;
> > > > > };
> > > > >
> > > > > BTW, it would be both ABI and API breakage,
> > > > > though all functions using this struct are marked as experimental,
> > > > > plus it is an LTS release, so it seems to be ok.
> > > > > Though I think it needs to be flagged in RN.
> > > >
> > > > This is a good suggestion. This will make some changes in the cpu-crypto
> > > > support as well
> > > > And should be a separate patch.
> > > > We can take the API and ABI breakage in this release. That is not an issue.
> > > >
> > > >
> > > > >
> > > > > Another option obviously - introduce completely new structure for it
> > > > > and leave existing one unaffected.
> > > > >
> > > > This will create some duplicate code. Would not prefer that.
> > > >
> > > > > >
> > > > > > 5. Remove enum rte_crypto_dp_service, let the PMDs using the
> > session
> > > > private
> > > > > data to decide function handler.
> > > > > > 6. Remove is_update parameter.
> > > > > >
> > > > > > The main point that is uncertain is the existance of "submit_single".
> > > > > > I am ok to remove "submit_single" function. In VPP we can use
> > > > > rte_cryptodev_dp_sym_submit_vec() with vec.num=1 each time to
> > avoid
> > > > > > double looping.
> > > > > > But we have to put the rte_cryptodev_dp_sym_submit_vec() as an
> > inline
> > > > > function - this will cause the API not traced in version map.
> > > > > >
> > > > > > Any ideas?
> > > > > >
> > > > > > Regards,
> > > > > > Fan
> > > > > >
@@ -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
+};
+
#ifdef __cplusplus
}
#endif
@@ -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.
+ */
+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;
+ 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;
+ };
+
+ /* Supposed to be used with rte_cryptodev_dp_sym_submit_vec()
+ * call.
+ */
+ union rte_crypto_sym_additional_data *additional_data;
+ };
+
/**
* array of statuses for each operation:
* - 0 on success
@@ -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;
+ }
+
+ 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;
+
+ 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)
+{
+ return _cryptodev_dp_submit_single_job(ctx, 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);
+}
+
+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,
@@ -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 */
/**
* 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.
+ *
+ * @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 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.
+ * @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.
+ * @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[];
+};
+
+/**
+ * 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
@@ -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. */
+ };
+ };
};
@@ -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_dp_get_service_ctx_data_size;
+ rte_cryptodev_dp_sym_dequeue;
+ rte_cryptodev_dp_sym_dequeue_done;
+ rte_cryptodev_dp_sym_dequeue_single_job;
+ rte_cryptodev_dp_sym_submit_done;
+ rte_cryptodev_dp_sym_submit_single_job;
+ rte_cryptodev_dp_sym_submit_vec;
};