[v3,1/7] cryptodev: add APIs to get/set event metadata

Message ID 20220421143720.1583062-2-gakhil@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series Add new cryptodev op for event metadata |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Akhil Goyal April 21, 2022, 2:37 p.m. UTC
  From: Volodymyr Fialko <vfialko@marvell.com>

Currently, crypto session userdata is used to set event crypto
metadata from the application and the driver is dereferencing it
in driver which is not correct. User data is meant to be opaque
to the driver.
To support this, new API is added to get and set event crypto
metadata. The new API, rte_cryptodev_set_session_event_mdata,
allows setting event metadata in session private data which is
filled inside PMD using a new cryptodev op. This operation
can be performed on any of the PMD supported sessions
(sym/asym/security).
For SW abstraction of event crypto adapter to be used by
eventdev library, a new field is added in asymmetric crypto
session for now and for symmetric case, current implementation
of using userdata is used. Symmetric cases cannot be fixed now,
as it will be ABI breakage which will be resolved in DPDK 22.11.

Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
Signed-off-by: Akhil Goyal <gakhil@marvell.com>
---
 lib/cryptodev/cryptodev_pmd.c | 16 ++++++++++++++
 lib/cryptodev/cryptodev_pmd.h | 36 ++++++++++++++++++++++++++++++
 lib/cryptodev/rte_cryptodev.c | 41 +++++++++++++++++++++++++++++++++++
 lib/cryptodev/rte_cryptodev.h | 22 +++++++++++++++++++
 lib/cryptodev/version.map     |  4 ++++
 5 files changed, 119 insertions(+)
  

Comments

Fan Zhang April 27, 2022, 3:38 p.m. UTC | #1
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, April 21, 2022 3:37 PM
> To: dev@dpdk.org
> Cc: anoobj@marvell.com; jerinj@marvell.com; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>; vfialko@marvell.com; Akhil Goyal
> <gakhil@marvell.com>
> Subject: [PATCH v3 1/7] cryptodev: add APIs to get/set event metadata
> 
> From: Volodymyr Fialko <vfialko@marvell.com>
> 
> Currently, crypto session userdata is used to set event crypto
> metadata from the application and the driver is dereferencing it
> in driver which is not correct. User data is meant to be opaque
> to the driver.
> To support this, new API is added to get and set event crypto
> metadata. The new API, rte_cryptodev_set_session_event_mdata,
> allows setting event metadata in session private data which is
> filled inside PMD using a new cryptodev op. This operation
> can be performed on any of the PMD supported sessions
> (sym/asym/security).
> For SW abstraction of event crypto adapter to be used by
> eventdev library, a new field is added in asymmetric crypto
> session for now and for symmetric case, current implementation
> of using userdata is used. Symmetric cases cannot be fixed now,
> as it will be ABI breakage which will be resolved in DPDK 22.11.
> 
> Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> ---
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
  
Gujjar, Abhinandan S April 28, 2022, 2:42 p.m. UTC | #2
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, April 21, 2022 8:07 PM
> To: dev@dpdk.org
> Cc: anoobj@marvell.com; jerinj@marvell.com; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>; vfialko@marvell.com; Akhil Goyal
> <gakhil@marvell.com>
> Subject: [PATCH v3 1/7] cryptodev: add APIs to get/set event metadata
> 
> From: Volodymyr Fialko <vfialko@marvell.com>
> 
> Currently, crypto session userdata is used to set event crypto metadata from
> the application and the driver is dereferencing it in driver which is not correct.
> User data is meant to be opaque to the driver.
> To support this, new API is added to get and set event crypto metadata. The
> new API, rte_cryptodev_set_session_event_mdata,
> allows setting event metadata in session private data which is filled inside PMD
> using a new cryptodev op. This operation can be performed on any of the PMD
> supported sessions (sym/asym/security).
> For SW abstraction of event crypto adapter to be used by eventdev library, a
> new field is added in asymmetric crypto session for now and for symmetric
> case, current implementation of using userdata is used. Symmetric cases cannot
> be fixed now, as it will be ABI breakage which will be resolved in DPDK 22.11.
> 
> Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> ---
>  lib/cryptodev/cryptodev_pmd.c | 16 ++++++++++++++
> lib/cryptodev/cryptodev_pmd.h | 36 ++++++++++++++++++++++++++++++
> lib/cryptodev/rte_cryptodev.c | 41 +++++++++++++++++++++++++++++++++++
>  lib/cryptodev/rte_cryptodev.h | 22 +++++++++++++++++++
>  lib/cryptodev/version.map     |  4 ++++
>  5 files changed, 119 insertions(+)
> 
> diff --git a/lib/cryptodev/cryptodev_pmd.c b/lib/cryptodev/cryptodev_pmd.c
> index 739a0b3f34..1903ade388 100644
> --- a/lib/cryptodev/cryptodev_pmd.c
> +++ b/lib/cryptodev/cryptodev_pmd.c
> @@ -227,3 +227,19 @@ cryptodev_fp_ops_set(struct rte_crypto_fp_ops
> *fp_ops,
>  	fp_ops->qp.enq_cb = dev->enq_cbs;
>  	fp_ops->qp.deq_cb = dev->deq_cbs;
>  }
> +
> +void *
> +rte_cryptodev_session_event_mdata_get(struct rte_crypto_op *op) {
Null check for op?
> +	if (op->type == RTE_CRYPTO_OP_TYPE_SYMMETRIC &&
> +			op->sess_type == RTE_CRYPTO_OP_WITH_SESSION)
> +		return rte_cryptodev_sym_session_get_user_data(op->sym-
> >session);
> +	else if (op->type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC &&
> +			op->sess_type == RTE_CRYPTO_OP_WITH_SESSION)
> +		return op->asym->session->event_mdata;
> +	else if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
> +			op->private_data_offset)
> +		return ((uint8_t *)op + op->private_data_offset);
> +	else
> +		return NULL;
> +}
> diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
> index 2b1ce2da2d..7969944b66 100644
> --- a/lib/cryptodev/cryptodev_pmd.h
> +++ b/lib/cryptodev/cryptodev_pmd.h
> @@ -398,6 +398,25 @@ typedef int
> (*cryptodev_sym_configure_raw_dp_ctx_t)(
>  	enum rte_crypto_op_sess_type sess_type,
>  	union rte_cryptodev_session_ctx session_ctx, uint8_t is_update);
> 
> +/**
> + * Typedef that the driver provided to set event crypto meta data.
> + *
> + * @param	dev		Crypto device pointer.
> + * @param	sess		Crypto or security session.
> + * @param	op_type		Operation type.
> + * @param	sess_type	Session type.
> + * @param	ev_mdata	Pointer to the event crypto meta data
> + *				(aka *union rte_event_crypto_metadata*)
> + * @return
> + *   - On success return 0.
> + *   - On failure return negative integer.
> + */
> +typedef int (*cryptodev_session_event_mdata_set_t)(
> +	struct rte_cryptodev *dev, void *sess,
> +	enum rte_crypto_op_type op_type,
> +	enum rte_crypto_op_sess_type sess_type,
> +	void *ev_mdata);
> +
>  /** Crypto device operations function pointer table */  struct
> rte_cryptodev_ops {
>  	cryptodev_configure_t dev_configure;	/**< Configure device. */
> @@ -442,6 +461,8 @@ struct rte_cryptodev_ops {
>  			/**< Initialize raw data path context data. */
>  		};
>  	};
> +	cryptodev_session_event_mdata_set_t session_ev_mdata_set;
> +	/**< Set a Crypto or Security session even meta data. */
>  };
> 
> 
> @@ -603,6 +624,19 @@ void
>  cryptodev_fp_ops_set(struct rte_crypto_fp_ops *fp_ops,
>  		     const struct rte_cryptodev *dev);
> 
> +/**
> + * Get session event meta data (aka *union rte_event_crypto_metadata*)
> + *
> + * @param	op            pointer to *rte_crypto_op* structure.
> + *
> + * @return
> + *  - On success, pointer to event crypto metadata
> + *  - On failure, a negative value.
> + */
> +__rte_internal
> +void *
> +rte_cryptodev_session_event_mdata_get(struct rte_crypto_op *op);
> +
>  static inline void *
>  get_sym_session_private_data(const struct rte_cryptodev_sym_session *sess,
>  		uint8_t driver_id) {
> @@ -636,6 +670,8 @@ RTE_STD_C11 struct rte_cryptodev_asym_session {
>  	/**< Size of private data used when creating mempool */
>  	uint16_t user_data_sz;
>  	/**< Session user data will be placed after sess_data */
> +	void *event_mdata;
> +	/**< Event crypto adapter metadata */
Add reference to rte_event_crypto_metadata for clarity?
>  	uint8_t padding[3];
>  	uint8_t sess_private_data[0];
>  };
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c index
> 3500a2d470..a070cb2a00 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -2051,6 +2051,9 @@ rte_cryptodev_asym_session_free(uint8_t dev_id,
> void *sess)
> 
>  	dev->dev_ops->asym_session_clear(dev, sess);
> 
> +	if (((struct rte_cryptodev_asym_session *)sess)->event_mdata)
> +		rte_free(((struct rte_cryptodev_asym_session *)sess)-
> >event_mdata);
> +
Who allocates memory for event_mdata? If this done by application before calling
rte_cryptodev_session_event_mdata_set(), please document it.
>  	/* Return session to mempool */
>  	sess_mp = rte_mempool_from_obj(sess);
>  	rte_mempool_put(sess_mp, sess);
> @@ -2259,6 +2262,44 @@ rte_cryptodev_configure_raw_dp_ctx(uint8_t
> dev_id, uint16_t qp_id,
>  			sess_type, session_ctx, is_update);
>  }
> 
> +int
> +rte_cryptodev_session_event_mdata_set(uint8_t dev_id, void *sess,
> +	enum rte_crypto_op_type op_type,
> +	enum rte_crypto_op_sess_type sess_type,
> +	void *ev_mdata,
> +	uint16_t size)
> +{
> +	struct rte_cryptodev *dev;
> +
> +	if (!rte_cryptodev_is_valid_dev(dev_id))
> +		goto skip_pmd_op;
> +
> +	dev = rte_cryptodev_pmd_get_dev(dev_id);
> +	if (dev->dev_ops->session_ev_mdata_set == NULL)
> +		goto skip_pmd_op;
> +
> +	return (*dev->dev_ops->session_ev_mdata_set)(dev, sess, op_type,
> +			sess_type, ev_mdata);
> +
> +skip_pmd_op:
> +	if (op_type == RTE_CRYPTO_OP_TYPE_SYMMETRIC)
> +		return rte_cryptodev_sym_session_set_user_data(sess,
> ev_mdata,
> +				size);
> +	else if (op_type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
> +		struct rte_cryptodev_asym_session *s = sess;
Null check for sess?
> +
> +		if (s->event_mdata == NULL) {
> +			s->event_mdata = rte_malloc(NULL, size, 0);
> +			if (s->event_mdata == NULL)
> +				return -ENOMEM;
> +		}
> +		rte_memcpy(s->event_mdata, ev_mdata, size);
> +
> +		return 0;
> +	} else
> +		return -ENOTSUP;
> +}
> +
>  uint32_t
>  rte_cryptodev_raw_enqueue_burst(struct rte_crypto_raw_dp_ctx *ctx,
>  	struct rte_crypto_sym_vec *vec, union rte_crypto_sym_ofs ofs, diff --
> git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h index
> 45d33f4a50..2c2c2edeb7 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -1269,6 +1269,28 @@ __rte_experimental  int
> rte_cryptodev_get_raw_dp_ctx_size(uint8_t dev_id);
> 
> +/**
> + * Set session event meta data
> + *
> + * @param	dev_id		The device identifier.
> + * @param	sess            Crypto or security session.
> + * @param	op_type         Operation type.
> + * @param	sess_type       Session type.
> + * @param	ev_mdata	Pointer to the event crypto meta data
> + *				(aka *union rte_event_crypto_metadata*)
> + * @param	size            Size of ev_mdata.
> + *
> + * @return
> + *  - On success, zero.
> + *  - On failure, a negative value.
> + */
> +__rte_experimental
> +int
> +rte_cryptodev_session_event_mdata_set(uint8_t dev_id, void *sess,
> +	enum rte_crypto_op_type op_type,
> +	enum rte_crypto_op_sess_type sess_type,
> +	void *ev_mdata, uint16_t size);
> +
>  /**
>   * Union of different crypto session types, including session-less xform
>   * pointer.
> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map index
> c7c5aefceb..f0abfaa47d 100644
> --- a/lib/cryptodev/version.map
> +++ b/lib/cryptodev/version.map
> @@ -105,6 +105,9 @@ EXPERIMENTAL {
>  	rte_cryptodev_asym_session_pool_create;
>  	rte_cryptodev_asym_session_set_user_data;
>  	__rte_cryptodev_trace_asym_session_pool_create;
> +
> +	#added in 22.07
> +	rte_cryptodev_session_event_mdata_set;
>  };
> 
>  INTERNAL {
> @@ -123,5 +126,6 @@ INTERNAL {
>  	rte_cryptodev_pmd_parse_input_args;
>  	rte_cryptodev_pmd_probing_finish;
>  	rte_cryptodev_pmd_release_device;
> +	rte_cryptodev_session_event_mdata_get;
>  	rte_cryptodevs;
>  };
> --
> 2.25.1
  
Akhil Goyal April 29, 2022, 12:16 p.m. UTC | #3
Hi Abhinandan,

Please see inline.
> > +
> > +void *
> > +rte_cryptodev_session_event_mdata_get(struct rte_crypto_op *op) {
> Null check for op?

Null check can be added, but this a datapath dpdk internal API.
We do not normally add checks in datapath.
If you insist, I can add, but before calling this API, PMD/lib would have already
checked for null op.

> > +	if (op->type == RTE_CRYPTO_OP_TYPE_SYMMETRIC &&
> > +			op->sess_type == RTE_CRYPTO_OP_WITH_SESSION)
> > +		return rte_cryptodev_sym_session_get_user_data(op->sym-
> > >session);
> > +	else if (op->type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC &&
> > +			op->sess_type == RTE_CRYPTO_OP_WITH_SESSION)
> > +		return op->asym->session->event_mdata;
> > +	else if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
> > +			op->private_data_offset)
> > +		return ((uint8_t *)op + op->private_data_offset);
> > +	else
> > +		return NULL;
> > +}
> > diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
> > index 2b1ce2da2d..7969944b66 100644
> > --- a/lib/cryptodev/cryptodev_pmd.h
> > +++ b/lib/cryptodev/cryptodev_pmd.h
> > @@ -398,6 +398,25 @@ typedef int
> > (*cryptodev_sym_configure_raw_dp_ctx_t)(
> >  	enum rte_crypto_op_sess_type sess_type,
> >  	union rte_cryptodev_session_ctx session_ctx, uint8_t is_update);
> >
> > +/**
> > + * Typedef that the driver provided to set event crypto meta data.
> > + *
> > + * @param	dev		Crypto device pointer.
> > + * @param	sess		Crypto or security session.
> > + * @param	op_type		Operation type.
> > + * @param	sess_type	Session type.
> > + * @param	ev_mdata	Pointer to the event crypto meta data
> > + *				(aka *union rte_event_crypto_metadata*)
> > + * @return
> > + *   - On success return 0.
> > + *   - On failure return negative integer.
> > + */
> > +typedef int (*cryptodev_session_event_mdata_set_t)(
> > +	struct rte_cryptodev *dev, void *sess,
> > +	enum rte_crypto_op_type op_type,
> > +	enum rte_crypto_op_sess_type sess_type,
> > +	void *ev_mdata);
> > +
> >  /** Crypto device operations function pointer table */  struct
> > rte_cryptodev_ops {
> >  	cryptodev_configure_t dev_configure;	/**< Configure device. */
> > @@ -442,6 +461,8 @@ struct rte_cryptodev_ops {
> >  			/**< Initialize raw data path context data. */
> >  		};
> >  	};
> > +	cryptodev_session_event_mdata_set_t session_ev_mdata_set;
> > +	/**< Set a Crypto or Security session even meta data. */
> >  };
> >
> >
> > @@ -603,6 +624,19 @@ void
> >  cryptodev_fp_ops_set(struct rte_crypto_fp_ops *fp_ops,
> >  		     const struct rte_cryptodev *dev);
> >
> > +/**
> > + * Get session event meta data (aka *union rte_event_crypto_metadata*)
> > + *
> > + * @param	op            pointer to *rte_crypto_op* structure.
> > + *
> > + * @return
> > + *  - On success, pointer to event crypto metadata
> > + *  - On failure, a negative value.
> > + */
> > +__rte_internal
> > +void *
> > +rte_cryptodev_session_event_mdata_get(struct rte_crypto_op *op);
> > +
> >  static inline void *
> >  get_sym_session_private_data(const struct rte_cryptodev_sym_session
> *sess,
> >  		uint8_t driver_id) {
> > @@ -636,6 +670,8 @@ RTE_STD_C11 struct rte_cryptodev_asym_session {
> >  	/**< Size of private data used when creating mempool */
> >  	uint16_t user_data_sz;
> >  	/**< Session user data will be placed after sess_data */
> > +	void *event_mdata;
> > +	/**< Event crypto adapter metadata */
> Add reference to rte_event_crypto_metadata for clarity?

Ok will add the comment.

> >  	uint8_t padding[3];
> >  	uint8_t sess_private_data[0];
> >  };
> > diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index
> > 3500a2d470..a070cb2a00 100644
> > --- a/lib/cryptodev/rte_cryptodev.c
> > +++ b/lib/cryptodev/rte_cryptodev.c
> > @@ -2051,6 +2051,9 @@ rte_cryptodev_asym_session_free(uint8_t dev_id,
> > void *sess)
> >
> >  	dev->dev_ops->asym_session_clear(dev, sess);
> >
> > +	if (((struct rte_cryptodev_asym_session *)sess)->event_mdata)
> > +		rte_free(((struct rte_cryptodev_asym_session *)sess)-
> > >event_mdata);
> > +
> Who allocates memory for event_mdata? If this done by application before
> calling
> rte_cryptodev_session_event_mdata_set(), please document it.

It is same as was done before this patch.
The rte_cryptodev_session_event_mdata_set is allocating and making the copy
as it was copied in userdata before. Hence, no update is required.

> >  	/* Return session to mempool */
> >  	sess_mp = rte_mempool_from_obj(sess);
> >  	rte_mempool_put(sess_mp, sess);
> > @@ -2259,6 +2262,44 @@ rte_cryptodev_configure_raw_dp_ctx(uint8_t
> > dev_id, uint16_t qp_id,
> >  			sess_type, session_ctx, is_update);
> >  }
> >
> > +int
> > +rte_cryptodev_session_event_mdata_set(uint8_t dev_id, void *sess,
> > +	enum rte_crypto_op_type op_type,
> > +	enum rte_crypto_op_sess_type sess_type,
> > +	void *ev_mdata,
> > +	uint16_t size)
> > +{
> > +	struct rte_cryptodev *dev;
> > +
> > +	if (!rte_cryptodev_is_valid_dev(dev_id))
> > +		goto skip_pmd_op;
> > +
> > +	dev = rte_cryptodev_pmd_get_dev(dev_id);
> > +	if (dev->dev_ops->session_ev_mdata_set == NULL)
> > +		goto skip_pmd_op;
> > +
> > +	return (*dev->dev_ops->session_ev_mdata_set)(dev, sess, op_type,
> > +			sess_type, ev_mdata);
> > +
> > +skip_pmd_op:
> > +	if (op_type == RTE_CRYPTO_OP_TYPE_SYMMETRIC)
> > +		return rte_cryptodev_sym_session_set_user_data(sess,
> > ev_mdata,
> > +				size);
> > +	else if (op_type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
> > +		struct rte_cryptodev_asym_session *s = sess;
> Null check for sess?

Again, it is a datapath, avoided extra checks.

> > +
> > +		if (s->event_mdata == NULL) {
> > +			s->event_mdata = rte_malloc(NULL, size, 0);
> > +			if (s->event_mdata == NULL)
> > +				return -ENOMEM;
> > +		}
> > +		rte_memcpy(s->event_mdata, ev_mdata, size);
> > +
> > +		return 0;
> > +	} else
> > +		return -ENOTSUP;
> > +}
  
Gujjar, Abhinandan S May 1, 2022, 12:24 p.m. UTC | #4
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Friday, April 29, 2022 5:47 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org
> Cc: Anoob Joseph <anoobj@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>;
> Vangati, Narender <narender.vangati@intel.com>; Volodymyr Fialko
> <vfialko@marvell.com>
> Subject: RE: [PATCH v3 1/7] cryptodev: add APIs to get/set event metadata
> 
> Hi Abhinandan,
> 
> Please see inline.
> > > +
> > > +void *
> > > +rte_cryptodev_session_event_mdata_get(struct rte_crypto_op *op) {
> > Null check for op?
> 
> Null check can be added, but this a datapath dpdk internal API.
> We do not normally add checks in datapath.
> If you insist, I can add, but before calling this API, PMD/lib would have already
> checked for null op.
It is ok for get API. It is better to add the check for set API as it exposed to user application.
Currently, the set API is validating dev_id. It is better to add a null check only for set API.
> 
> > > +	if (op->type == RTE_CRYPTO_OP_TYPE_SYMMETRIC &&
> > > +			op->sess_type == RTE_CRYPTO_OP_WITH_SESSION)
> > > +		return rte_cryptodev_sym_session_get_user_data(op->sym-
> > > >session);
> > > +	else if (op->type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC &&
> > > +			op->sess_type == RTE_CRYPTO_OP_WITH_SESSION)
> > > +		return op->asym->session->event_mdata;
> > > +	else if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
> > > +			op->private_data_offset)
> > > +		return ((uint8_t *)op + op->private_data_offset);
> > > +	else
> > > +		return NULL;
> > > +}
> > > diff --git a/lib/cryptodev/cryptodev_pmd.h
> > > b/lib/cryptodev/cryptodev_pmd.h index 2b1ce2da2d..7969944b66 100644
> > > --- a/lib/cryptodev/cryptodev_pmd.h
> > > +++ b/lib/cryptodev/cryptodev_pmd.h
> > > @@ -398,6 +398,25 @@ typedef int
> > > (*cryptodev_sym_configure_raw_dp_ctx_t)(
> > >  	enum rte_crypto_op_sess_type sess_type,
> > >  	union rte_cryptodev_session_ctx session_ctx, uint8_t is_update);
> > >
> > > +/**
> > > + * Typedef that the driver provided to set event crypto meta data.
> > > + *
> > > + * @param	dev		Crypto device pointer.
> > > + * @param	sess		Crypto or security session.
> > > + * @param	op_type		Operation type.
> > > + * @param	sess_type	Session type.
> > > + * @param	ev_mdata	Pointer to the event crypto meta data
> > > + *				(aka *union rte_event_crypto_metadata*)
> > > + * @return
> > > + *   - On success return 0.
> > > + *   - On failure return negative integer.
> > > + */
> > > +typedef int (*cryptodev_session_event_mdata_set_t)(
> > > +	struct rte_cryptodev *dev, void *sess,
> > > +	enum rte_crypto_op_type op_type,
> > > +	enum rte_crypto_op_sess_type sess_type,
> > > +	void *ev_mdata);
> > > +
> > >  /** Crypto device operations function pointer table */  struct
> > > rte_cryptodev_ops {
> > >  	cryptodev_configure_t dev_configure;	/**< Configure device. */
> > > @@ -442,6 +461,8 @@ struct rte_cryptodev_ops {
> > >  			/**< Initialize raw data path context data. */
> > >  		};
> > >  	};
> > > +	cryptodev_session_event_mdata_set_t session_ev_mdata_set;
> > > +	/**< Set a Crypto or Security session even meta data. */
> > >  };
> > >
> > >
> > > @@ -603,6 +624,19 @@ void
> > >  cryptodev_fp_ops_set(struct rte_crypto_fp_ops *fp_ops,
> > >  		     const struct rte_cryptodev *dev);
> > >
> > > +/**
> > > + * Get session event meta data (aka *union
> > > +rte_event_crypto_metadata*)
> > > + *
> > > + * @param	op            pointer to *rte_crypto_op* structure.
> > > + *
> > > + * @return
> > > + *  - On success, pointer to event crypto metadata
> > > + *  - On failure, a negative value.
> > > + */
> > > +__rte_internal
> > > +void *
> > > +rte_cryptodev_session_event_mdata_get(struct rte_crypto_op *op);
> > > +
> > >  static inline void *
> > >  get_sym_session_private_data(const struct rte_cryptodev_sym_session
> > *sess,
> > >  		uint8_t driver_id) {
> > > @@ -636,6 +670,8 @@ RTE_STD_C11 struct rte_cryptodev_asym_session {
> > >  	/**< Size of private data used when creating mempool */
> > >  	uint16_t user_data_sz;
> > >  	/**< Session user data will be placed after sess_data */
> > > +	void *event_mdata;
> > > +	/**< Event crypto adapter metadata */
> > Add reference to rte_event_crypto_metadata for clarity?
> 
> Ok will add the comment.
> 
> > >  	uint8_t padding[3];
> > >  	uint8_t sess_private_data[0];
> > >  };
> > > diff --git a/lib/cryptodev/rte_cryptodev.c
> > > b/lib/cryptodev/rte_cryptodev.c
> > index
> > > 3500a2d470..a070cb2a00 100644
> > > --- a/lib/cryptodev/rte_cryptodev.c
> > > +++ b/lib/cryptodev/rte_cryptodev.c
> > > @@ -2051,6 +2051,9 @@ rte_cryptodev_asym_session_free(uint8_t
> > > dev_id, void *sess)
> > >
> > >  	dev->dev_ops->asym_session_clear(dev, sess);
> > >
> > > +	if (((struct rte_cryptodev_asym_session *)sess)->event_mdata)
> > > +		rte_free(((struct rte_cryptodev_asym_session *)sess)-
> > > >event_mdata);
> > > +
> > Who allocates memory for event_mdata? If this done by application
> > before calling rte_cryptodev_session_event_mdata_set(), please
> > document it.
> 
> It is same as was done before this patch.
> The rte_cryptodev_session_event_mdata_set is allocating and making the copy
> as it was copied in userdata before. Hence, no update is required.
ok
> 
> > >  	/* Return session to mempool */
> > >  	sess_mp = rte_mempool_from_obj(sess);
> > >  	rte_mempool_put(sess_mp, sess);
> > > @@ -2259,6 +2262,44 @@ rte_cryptodev_configure_raw_dp_ctx(uint8_t
> > > dev_id, uint16_t qp_id,
> > >  			sess_type, session_ctx, is_update);  }
> > >
> > > +int
> > > +rte_cryptodev_session_event_mdata_set(uint8_t dev_id, void *sess,
> > > +	enum rte_crypto_op_type op_type,
> > > +	enum rte_crypto_op_sess_type sess_type,
> > > +	void *ev_mdata,
> > > +	uint16_t size)
> > > +{
> > > +	struct rte_cryptodev *dev;
> > > +
> > > +	if (!rte_cryptodev_is_valid_dev(dev_id))
> > > +		goto skip_pmd_op;
> > > +
> > > +	dev = rte_cryptodev_pmd_get_dev(dev_id);
> > > +	if (dev->dev_ops->session_ev_mdata_set == NULL)
> > > +		goto skip_pmd_op;
> > > +
> > > +	return (*dev->dev_ops->session_ev_mdata_set)(dev, sess, op_type,
> > > +			sess_type, ev_mdata);
> > > +
> > > +skip_pmd_op:
> > > +	if (op_type == RTE_CRYPTO_OP_TYPE_SYMMETRIC)
> > > +		return rte_cryptodev_sym_session_set_user_data(sess,
> > > ev_mdata,
> > > +				size);
> > > +	else if (op_type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
> > > +		struct rte_cryptodev_asym_session *s = sess;
> > Null check for sess?
> 
> Again, it is a datapath, avoided extra checks.
> 
> > > +
> > > +		if (s->event_mdata == NULL) {
> > > +			s->event_mdata = rte_malloc(NULL, size, 0);
> > > +			if (s->event_mdata == NULL)
> > > +				return -ENOMEM;
> > > +		}
> > > +		rte_memcpy(s->event_mdata, ev_mdata, size);
> > > +
> > > +		return 0;
> > > +	} else
> > > +		return -ENOTSUP;
> > > +}
  
Akhil Goyal May 1, 2022, 6:37 p.m. UTC | #5
> > > > +
> > > > +void *
> > > > +rte_cryptodev_session_event_mdata_get(struct rte_crypto_op *op) {
> > > Null check for op?
> >
> > Null check can be added, but this a datapath dpdk internal API.
> > We do not normally add checks in datapath.
> > If you insist, I can add, but before calling this API, PMD/lib would have already
> > checked for null op.
> It is ok for get API. It is better to add the check for set API as it exposed to user
> application.
> Currently, the set API is validating dev_id. It is better to add a null check only for
> set API.
Ok. Will do that.
  

Patch

diff --git a/lib/cryptodev/cryptodev_pmd.c b/lib/cryptodev/cryptodev_pmd.c
index 739a0b3f34..1903ade388 100644
--- a/lib/cryptodev/cryptodev_pmd.c
+++ b/lib/cryptodev/cryptodev_pmd.c
@@ -227,3 +227,19 @@  cryptodev_fp_ops_set(struct rte_crypto_fp_ops *fp_ops,
 	fp_ops->qp.enq_cb = dev->enq_cbs;
 	fp_ops->qp.deq_cb = dev->deq_cbs;
 }
+
+void *
+rte_cryptodev_session_event_mdata_get(struct rte_crypto_op *op)
+{
+	if (op->type == RTE_CRYPTO_OP_TYPE_SYMMETRIC &&
+			op->sess_type == RTE_CRYPTO_OP_WITH_SESSION)
+		return rte_cryptodev_sym_session_get_user_data(op->sym->session);
+	else if (op->type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC &&
+			op->sess_type == RTE_CRYPTO_OP_WITH_SESSION)
+		return op->asym->session->event_mdata;
+	else if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
+			op->private_data_offset)
+		return ((uint8_t *)op + op->private_data_offset);
+	else
+		return NULL;
+}
diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
index 2b1ce2da2d..7969944b66 100644
--- a/lib/cryptodev/cryptodev_pmd.h
+++ b/lib/cryptodev/cryptodev_pmd.h
@@ -398,6 +398,25 @@  typedef int (*cryptodev_sym_configure_raw_dp_ctx_t)(
 	enum rte_crypto_op_sess_type sess_type,
 	union rte_cryptodev_session_ctx session_ctx, uint8_t is_update);
 
+/**
+ * Typedef that the driver provided to set event crypto meta data.
+ *
+ * @param	dev		Crypto device pointer.
+ * @param	sess		Crypto or security session.
+ * @param	op_type		Operation type.
+ * @param	sess_type	Session type.
+ * @param	ev_mdata	Pointer to the event crypto meta data
+ *				(aka *union rte_event_crypto_metadata*)
+ * @return
+ *   - On success return 0.
+ *   - On failure return negative integer.
+ */
+typedef int (*cryptodev_session_event_mdata_set_t)(
+	struct rte_cryptodev *dev, void *sess,
+	enum rte_crypto_op_type op_type,
+	enum rte_crypto_op_sess_type sess_type,
+	void *ev_mdata);
+
 /** Crypto device operations function pointer table */
 struct rte_cryptodev_ops {
 	cryptodev_configure_t dev_configure;	/**< Configure device. */
@@ -442,6 +461,8 @@  struct rte_cryptodev_ops {
 			/**< Initialize raw data path context data. */
 		};
 	};
+	cryptodev_session_event_mdata_set_t session_ev_mdata_set;
+	/**< Set a Crypto or Security session even meta data. */
 };
 
 
@@ -603,6 +624,19 @@  void
 cryptodev_fp_ops_set(struct rte_crypto_fp_ops *fp_ops,
 		     const struct rte_cryptodev *dev);
 
+/**
+ * Get session event meta data (aka *union rte_event_crypto_metadata*)
+ *
+ * @param	op            pointer to *rte_crypto_op* structure.
+ *
+ * @return
+ *  - On success, pointer to event crypto metadata
+ *  - On failure, a negative value.
+ */
+__rte_internal
+void *
+rte_cryptodev_session_event_mdata_get(struct rte_crypto_op *op);
+
 static inline void *
 get_sym_session_private_data(const struct rte_cryptodev_sym_session *sess,
 		uint8_t driver_id) {
@@ -636,6 +670,8 @@  RTE_STD_C11 struct rte_cryptodev_asym_session {
 	/**< Size of private data used when creating mempool */
 	uint16_t user_data_sz;
 	/**< Session user data will be placed after sess_data */
+	void *event_mdata;
+	/**< Event crypto adapter metadata */
 	uint8_t padding[3];
 	uint8_t sess_private_data[0];
 };
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index 3500a2d470..a070cb2a00 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -2051,6 +2051,9 @@  rte_cryptodev_asym_session_free(uint8_t dev_id, void *sess)
 
 	dev->dev_ops->asym_session_clear(dev, sess);
 
+	if (((struct rte_cryptodev_asym_session *)sess)->event_mdata)
+		rte_free(((struct rte_cryptodev_asym_session *)sess)->event_mdata);
+
 	/* Return session to mempool */
 	sess_mp = rte_mempool_from_obj(sess);
 	rte_mempool_put(sess_mp, sess);
@@ -2259,6 +2262,44 @@  rte_cryptodev_configure_raw_dp_ctx(uint8_t dev_id, uint16_t qp_id,
 			sess_type, session_ctx, is_update);
 }
 
+int
+rte_cryptodev_session_event_mdata_set(uint8_t dev_id, void *sess,
+	enum rte_crypto_op_type op_type,
+	enum rte_crypto_op_sess_type sess_type,
+	void *ev_mdata,
+	uint16_t size)
+{
+	struct rte_cryptodev *dev;
+
+	if (!rte_cryptodev_is_valid_dev(dev_id))
+		goto skip_pmd_op;
+
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+	if (dev->dev_ops->session_ev_mdata_set == NULL)
+		goto skip_pmd_op;
+
+	return (*dev->dev_ops->session_ev_mdata_set)(dev, sess, op_type,
+			sess_type, ev_mdata);
+
+skip_pmd_op:
+	if (op_type == RTE_CRYPTO_OP_TYPE_SYMMETRIC)
+		return rte_cryptodev_sym_session_set_user_data(sess, ev_mdata,
+				size);
+	else if (op_type == RTE_CRYPTO_OP_TYPE_ASYMMETRIC) {
+		struct rte_cryptodev_asym_session *s = sess;
+
+		if (s->event_mdata == NULL) {
+			s->event_mdata = rte_malloc(NULL, size, 0);
+			if (s->event_mdata == NULL)
+				return -ENOMEM;
+		}
+		rte_memcpy(s->event_mdata, ev_mdata, size);
+
+		return 0;
+	} else
+		return -ENOTSUP;
+}
+
 uint32_t
 rte_cryptodev_raw_enqueue_burst(struct rte_crypto_raw_dp_ctx *ctx,
 	struct rte_crypto_sym_vec *vec, union rte_crypto_sym_ofs ofs,
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index 45d33f4a50..2c2c2edeb7 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -1269,6 +1269,28 @@  __rte_experimental
 int
 rte_cryptodev_get_raw_dp_ctx_size(uint8_t dev_id);
 
+/**
+ * Set session event meta data
+ *
+ * @param	dev_id		The device identifier.
+ * @param	sess            Crypto or security session.
+ * @param	op_type         Operation type.
+ * @param	sess_type       Session type.
+ * @param	ev_mdata	Pointer to the event crypto meta data
+ *				(aka *union rte_event_crypto_metadata*)
+ * @param	size            Size of ev_mdata.
+ *
+ * @return
+ *  - On success, zero.
+ *  - On failure, a negative value.
+ */
+__rte_experimental
+int
+rte_cryptodev_session_event_mdata_set(uint8_t dev_id, void *sess,
+	enum rte_crypto_op_type op_type,
+	enum rte_crypto_op_sess_type sess_type,
+	void *ev_mdata, uint16_t size);
+
 /**
  * Union of different crypto session types, including session-less xform
  * pointer.
diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
index c7c5aefceb..f0abfaa47d 100644
--- a/lib/cryptodev/version.map
+++ b/lib/cryptodev/version.map
@@ -105,6 +105,9 @@  EXPERIMENTAL {
 	rte_cryptodev_asym_session_pool_create;
 	rte_cryptodev_asym_session_set_user_data;
 	__rte_cryptodev_trace_asym_session_pool_create;
+
+	#added in 22.07
+	rte_cryptodev_session_event_mdata_set;
 };
 
 INTERNAL {
@@ -123,5 +126,6 @@  INTERNAL {
 	rte_cryptodev_pmd_parse_input_args;
 	rte_cryptodev_pmd_probing_finish;
 	rte_cryptodev_pmd_release_device;
+	rte_cryptodev_session_event_mdata_get;
 	rte_cryptodevs;
 };