RE: [EXT] [PATCH v3 3/4] crypto: add asym session user data API

[Akhil Goyal <gakhil@marvell.com>]

> -----Original Message-----
> From: Ciara Power <ciara.power@intel.com>
> Sent: Thursday, February 3, 2022 9:35 PM
> To: dev@dpdk.org
> Cc: roy.fan.zhang@intel.com; Akhil Goyal <gakhil@marvell.com>; Anoob Joseph
> <anoobj@marvell.com>; mdr@ashroe.eu; Ciara Power
> <ciara.power@intel.com>; Declan Doherty <declan.doherty@intel.com>
> Subject: [EXT] [PATCH v3 3/4] crypto: add asym session user data API
> 
> External Email
> 
> ----------------------------------------------------------------------
> A user data field is added to the asymmetric session structure.
> Relevant API added to get/set the field.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
> Acked-by: Anoob Joseph <anoobj@marvell.com>
> 
> ---
> v2: Corrected order of version map entries.
> v3:
>   - Corrected formatting of struct comments.
>   - Added setting user data size in pool creation.
>   - Added documentation.
> ---
>  app/test/test_cryptodev_asym.c          |  2 +-
>  doc/guides/prog_guide/cryptodev_lib.rst | 21 ++++++++++++-
>  doc/guides/rel_notes/release_22_03.rst  |  6 +++-
>  lib/cryptodev/cryptodev_pmd.h           |  4 ++-
>  lib/cryptodev/rte_cryptodev.c           | 40 ++++++++++++++++++++++---
>  lib/cryptodev/rte_cryptodev.h           | 34 ++++++++++++++++++++-
>  lib/cryptodev/rte_cryptodev_trace.h     |  3 +-
>  lib/cryptodev/version.map               |  2 ++
>  8 files changed, 102 insertions(+), 10 deletions(-)
> 
> diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
> index f93f39af42..a81d6292f6 100644
> --- a/app/test/test_cryptodev_asym.c
> +++ b/app/test/test_cryptodev_asym.c
> @@ -897,7 +897,7 @@ testsuite_setup(void)
>  	}
> 
>  	ts_params->session_mpool =
> rte_cryptodev_asym_session_pool_create(
> -			"test_asym_sess_mp", TEST_NUM_SESSIONS * 2, 0,
> +			"test_asym_sess_mp", TEST_NUM_SESSIONS * 2, 0, 0,
>  			SOCKET_ID_ANY);
> 
>  	TEST_ASSERT_NOT_NULL(ts_params->session_mpool,
> diff --git a/doc/guides/prog_guide/cryptodev_lib.rst
> b/doc/guides/prog_guide/cryptodev_lib.rst
> index 56b19da3ae..62bd3577f5 100644
> --- a/doc/guides/prog_guide/cryptodev_lib.rst
> +++ b/doc/guides/prog_guide/cryptodev_lib.rst
> @@ -1110,6 +1110,25 @@ They operate on data buffer of type
> ``rte_crypto_param``.
> 
>  See *DPDK API Reference* for details on each rte_crypto_xxx_op_param struct
> 
> +Private user data
> +~~~~~~~~~~~~~~~~~
> +
> +Similar to symmetric above, asymmetric also has a set and get API that
> provides a
> +mechanism for an application to store and retrieve the private user data
> information
> +stored along with the crypto session.
> +
> +.. code-block:: c
> +
> +	int rte_cryptodev_asym_session_set_user_data(void *sess,
> +		void *data, uint16_t size);
> +
> +	void * rte_cryptodev_asym_session_get_user_data(void *sess);
> +
> +Please note the ``size`` passed to set API cannot be bigger than the predefined
> +``user_data_sz`` when creating the session mempool, otherwise the function
> will
> +return an error. Also when ``user_data_sz`` was defined as ``0`` when
> +creating the session mempool, the get API will always return ``NULL``.
> +
>  Asymmetric crypto Sample code
>  -----------------------------
> 
> @@ -1166,7 +1185,7 @@ crypto operations is similar except change to
> respective op and xform setup).
>       */
>      asym_session_pool = rte_cryptodev_asym_session_pool_create(
>                          "asym_session_pool", MAX_ASYM_SESSIONS, 0, 0,
> -                        socket_id);
> +                        0, socket_id);
> 
>      /* Configure the crypto device. */
>      struct rte_cryptodev_config conf = {
> diff --git a/doc/guides/rel_notes/release_22_03.rst
> b/doc/guides/rel_notes/release_22_03.rst
> index c4a25f54cd..1022f77828 100644
> --- a/doc/guides/rel_notes/release_22_03.rst
> +++ b/doc/guides/rel_notes/release_22_03.rst
> @@ -65,6 +65,10 @@ New Features
>    * Added AES-XCBC support in lookaside protocol (IPsec) for CN9K & CN10K.
>    * Added AES-CMAC support in CN9K & CN10K.
> 
> +* **Added an API for private user data in asymmetric session.**
> +
> +  An API was added for getting and setting an asymmetric session's user data.
> +

Asymmetric crypto session

>  * **Added an API to retrieve event port id of ethdev Rx adapter.**
> 
>    The new API ``rte_event_eth_rx_adapter_event_port_get()`` was added.
> @@ -104,7 +108,7 @@ API Changes
>    and moved to ``cryptodev_pmd.h``, hiding it from applications.
>    A ``rte_cryptodev_asym_session_pool_create`` function was added to
>    create a mempool with element size to hold the generic asym session header,
> -  along with the max size for a device private session data.
> +  along with the max size for a device private session data, and user data size.
>    ``rte_cryptodev_asym_session_init`` was removed as this initialisation is
>    now done by ``rte_cryptodev_asym_session_create``.
> 
> diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
> index d21db89837..a866440b0b 100644
> --- a/lib/cryptodev/cryptodev_pmd.h
> +++ b/lib/cryptodev/cryptodev_pmd.h
> @@ -636,7 +636,9 @@ __extension__ struct rte_cryptodev_asym_session {
>  	/**< Session driver ID. */
>  	uint16_t max_priv_session_sz;
>  	/**< Size of private session data used when creating mempool */
> -	uint8_t padding[5];
> +	uint16_t user_data_sz;
> +	/**< Session user data will be placed after sess_data */
> +	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 a839abe34d..0d816ed4a9 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -210,6 +210,8 @@ struct rte_cryptodev_sym_session_pool_private_data {
>  struct rte_cryptodev_asym_session_pool_private_data {
>  	uint16_t max_priv_session_sz;
>  	/**< Size of private session data used when creating mempool */
> +	uint16_t user_data_sz;
> +	/**< Session user data will be placed after sess_private_data */
>  };
> 
>  int
> @@ -1803,7 +1805,7 @@ rte_cryptodev_sym_session_pool_create(const char
> *name, uint32_t nb_elts,
> 
>  struct rte_mempool *
>  rte_cryptodev_asym_session_pool_create(const char *name, uint32_t nb_elts,
> -	uint32_t cache_size, int socket_id)
> +	uint32_t cache_size, uint16_t user_data_size, int socket_id)
>  {
>  	struct rte_mempool *mp;
>  	struct rte_cryptodev_asym_session_pool_private_data *pool_priv;
> @@ -1821,7 +1823,8 @@ rte_cryptodev_asym_session_pool_create(const char
> *name, uint32_t nb_elts,
>  		return NULL;
>  	}
> 
> -	obj_sz = rte_cryptodev_asym_get_header_session_size() + max_priv_sz;
> +	obj_sz = rte_cryptodev_asym_get_header_session_size() + max_priv_sz
> +
> +			user_data_size;
>  	obj_sz_aligned =  RTE_ALIGN_CEIL(obj_sz, RTE_CACHE_LINE_SIZE);
> 
>  	mp = rte_mempool_create(name, nb_elts, obj_sz_aligned, cache_size,
> @@ -1842,9 +1845,10 @@ rte_cryptodev_asym_session_pool_create(const
> char *name, uint32_t nb_elts,
>  		return NULL;
>  	}
>  	pool_priv->max_priv_session_sz = max_priv_sz;
> +	pool_priv->user_data_sz = user_data_size;
> 
>  	rte_cryptodev_trace_asym_session_pool_create(name, nb_elts,
> -		cache_size, mp);
> +		user_data_size, cache_size, mp);
>  	return mp;
>  }
> 
> @@ -1958,12 +1962,13 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp, uint8_t dev_id,
>  	}
> 
>  	sess->driver_id = dev->driver_id;
> +	sess->user_data_sz = pool_priv->user_data_sz;
>  	sess->max_priv_session_sz = pool_priv->max_priv_session_sz;
> 
>  	/* Clear device session pointer.
>  	 * Include the flag indicating presence of private data
>  	 */
> -	memset(sess->sess_private_data, 0, session_priv_data_sz);
> +	memset(sess->sess_private_data, 0, session_priv_data_sz + sess-
> >user_data_sz);
> 
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >asym_session_configure, NULL);
> 
> @@ -2173,6 +2178,33 @@ rte_cryptodev_sym_session_get_user_data(
>  	return (void *)(sess->sess_data + sess->nb_drivers);
>  }
> 
> +int
> +rte_cryptodev_asym_session_set_user_data(void *session, void *data,
> uint16_t size)
> +{
> +	struct rte_cryptodev_asym_session *sess = session;
> +	if (sess == NULL)
> +		return -EINVAL;
> +
> +	if (sess->user_data_sz < size)
> +		return -ENOMEM;
> +
> +	rte_memcpy(sess->sess_private_data +
> +			sess->max_priv_session_sz,
> +			data, size);
> +	return 0;
> +}
> +
> +void *
> +rte_cryptodev_asym_session_get_user_data(void *session)
> +{
> +	struct rte_cryptodev_asym_session *sess = session;
> +	if (sess == NULL || sess->user_data_sz == 0)
> +		return NULL;
> +
> +	return (void *)(sess->sess_private_data +
> +			sess->max_priv_session_sz);
> +}
> +
>  static inline void
>  sym_crypto_fill_status(struct rte_crypto_sym_vec *vec, int32_t errnum)
>  {
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index af328de896..6a4d6d9934 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -959,6 +959,8 @@ rte_cryptodev_sym_session_pool_create(const char
> *name, uint32_t nb_elts,
>   *   The number of elements in the mempool.
>   * @param cache_size
>   *   The number of per-lcore cache elements
> + * @param user_data_size
> + *   The size of user data to be placed after session private data.
>   * @param socket_id
>   *   The *socket_id* argument is the socket identifier in the case of
>   *   NUMA. The value can be *SOCKET_ID_ANY* if there is no NUMA
> @@ -971,7 +973,7 @@ rte_cryptodev_sym_session_pool_create(const char
> *name, uint32_t nb_elts,
>  __rte_experimental
>  struct rte_mempool *
>  rte_cryptodev_asym_session_pool_create(const char *name, uint32_t nb_elts,
> -	uint32_t cache_size, int socket_id);
> +	uint32_t cache_size, uint16_t user_data_size, int socket_id);
> 
>  /**
>   * Create symmetric crypto session header (generic with no private data)
> @@ -1213,6 +1215,36 @@ void *
>  rte_cryptodev_sym_session_get_user_data(
>  					struct rte_cryptodev_sym_session
> *sess);
> 
> +/**
> + * Store user data in an asymmetric session.
> + *
> + * @param	sess		Session pointer allocated by
> + *				*rte_cryptodev_asym_session_create*.
> + * @param	data		Pointer to the user data.
> + * @param	size		Size of the user data.
> + *
> + * @return
> + *  - On success, zero.
> + *  - On failure, a negative value.

Can we specify expected error numbers here.

> + */
> +__rte_experimental
> +int
> +rte_cryptodev_asym_session_set_user_data(void *sess, void *data, uint16_t
> size);
> +
> +/**
> + * Get user data stored in an asymmetric session.
> + *
> + * @param	sess		Session pointer allocated by
> + *				*rte_cryptodev_asym_session_create*.
> + *
> + * @return
> + *  - On success return pointer to user data.
> + *  - On failure returns NULL.
> + */
> +__rte_experimental
> +void *
> +rte_cryptodev_asym_session_get_user_data(void *sess);
> +
>  /**
>   * Perform actual crypto processing (encrypt/digest or auth/decrypt)
>   * on user provided data.
> diff --git a/lib/cryptodev/rte_cryptodev_trace.h
> b/lib/cryptodev/rte_cryptodev_trace.h
> index 82ef876c72..0980c7d7af 100644
> --- a/lib/cryptodev/rte_cryptodev_trace.h
> +++ b/lib/cryptodev/rte_cryptodev_trace.h
> @@ -86,9 +86,10 @@ RTE_TRACE_POINT(
>  RTE_TRACE_POINT(
>  	rte_cryptodev_trace_asym_session_pool_create,
>  	RTE_TRACE_POINT_ARGS(const char *name, uint32_t nb_elts,
> -		uint32_t cache_size, void *mempool),
> +		uint16_t user_data_size, uint32_t cache_size, void *mempool),
>  	rte_trace_point_emit_string(name);
>  	rte_trace_point_emit_u32(nb_elts);
> +	rte_trace_point_emit_u16(user_data_size);
>  	rte_trace_point_emit_u32(cache_size);
>  	rte_trace_point_emit_ptr(mempool);
>  )
> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> index eaea976f21..3528aa44b1 100644
> --- a/lib/cryptodev/version.map
> +++ b/lib/cryptodev/version.map
> @@ -103,7 +103,9 @@ EXPERIMENTAL {
>  	rte_cryptodev_remove_enq_callback;
> 
>  	# added 22.03
> +	rte_cryptodev_asym_session_get_user_data;
>  	rte_cryptodev_asym_session_pool_create;
> +	rte_cryptodev_asym_session_set_user_data;
>  	__rte_cryptodev_trace_asym_session_pool_create;
>  };
> 
> --
> 2.25.1