[v4,02/12] cryptodev: add sym session mempool create

Message ID 20190109225609.20590-3-roy.fan.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series cryptodev: change qp conf and sym session |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Fan Zhang Jan. 9, 2019, 10:55 p.m. UTC
  This patch adds a new API "rte_cryptodev_sym_session_pool_create()" to
cryptodev library. All applications are required to use this API to
create sym session mempool as it adds private data and nb_drivers
information to the mempool private data.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Fiona Trahe <fiona.trahe@intel.com>
---
 doc/guides/prog_guide/cryptodev_lib.rst        | 28 ++++++++++-----
 doc/guides/rel_notes/release_19_02.rst         |  5 +++
 lib/librte_cryptodev/rte_cryptodev.c           | 50 ++++++++++++++++++++++++++
 lib/librte_cryptodev/rte_cryptodev.h           | 31 ++++++++++++++++
 lib/librte_cryptodev/rte_cryptodev_version.map |  7 ++++
 5 files changed, 112 insertions(+), 9 deletions(-)
  

Comments

De Lara Guarch, Pablo Jan. 10, 2019, 11:22 a.m. UTC | #1
> -----Original Message-----
> From: Zhang, Roy Fan
> Sent: Wednesday, January 9, 2019 10:56 PM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>
> Subject: [PATCH v4 02/12] cryptodev: add sym session mempool create
> 
> This patch adds a new API "rte_cryptodev_sym_session_pool_create()" to
> cryptodev library. All applications are required to use this API to create sym
> session mempool as it adds private data and nb_drivers information to the
> mempool private data.
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
>  doc/guides/prog_guide/cryptodev_lib.rst        | 28 ++++++++++-----
>  doc/guides/rel_notes/release_19_02.rst         |  5 +++
>  lib/librte_cryptodev/rte_cryptodev.c           | 50
> ++++++++++++++++++++++++++
>  lib/librte_cryptodev/rte_cryptodev.h           | 31 ++++++++++++++++
>  lib/librte_cryptodev/rte_cryptodev_version.map |  7 ++++
>  5 files changed, 112 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/cryptodev_lib.rst
> b/doc/guides/prog_guide/cryptodev_lib.rst
> index 050a58dc4..366508618 100644
> --- a/doc/guides/prog_guide/cryptodev_lib.rst
> +++ b/doc/guides/prog_guide/cryptodev_lib.rst
> @@ -455,13 +455,15 @@ Crypto workloads.
> 
>  .. figure:: img/cryptodev_sym_sess.*
> 
> -The Crypto device framework provides APIs to allocate and initialize
> sessions -for crypto devices, where sessions are mempool objects.
> +The Crypto device framework provides APIs to create session mempool and
> +allocate and initialize sessions for crypto devices, where sessions are
> mempool objects.
>  It is the application's responsibility to create and manage the session
> mempools.
>  This approach allows for different scenarios such as having a single session
> mempool for all crypto devices (where the mempool object size is big
> enough to hold the private session of any crypto device), as well as having -
> multiple session mempools of different sizes for better memory usage.
> +multiple session mempools of different sizes for better memory usage.
> +However, the application is required to use
> +``rte_cryptodev_sym_session_pool_create()``
> +to create symmetric session mempool.
> 
>  An application can use ``rte_cryptodev_sym_get_private_session_size()`` to
> get the private session size of given crypto device. This function would allow
> @@ -623,7 +625,8 @@ using one of the crypto PMDs available in DPDK.
>      #define IV_OFFSET            (sizeof(struct rte_crypto_op) + \
>                                   sizeof(struct rte_crypto_sym_op))
> 
> -    struct rte_mempool *mbuf_pool, *crypto_op_pool, *session_pool;
> +    struct rte_mempool *mbuf_pool, *crypto_op_pool;
> +    struct *session_pool, *session_priv_pool;

Missing rte_mempool here.

>      unsigned int session_size;
>      int ret;
> 

...

> +++ b/doc/guides/rel_notes/release_19_02.rst
> @@ -167,6 +167,11 @@ API Changes
>  * cryptodev: as shown in the the 18.11 deprecation notice, the last
> parameter
>    of ``rte_cryptodev_queue_pair_setup()`` is removed.
> 
> +* cryptodev: a new function ``rte_cryptodev_sym_session_pool_create()``
> +is
> +  introduced. This function is used to create symmetric session mempool
> +safely
> +  and all crypto applications are required to use this function to
> +create
> +  symmetric session mempool from now on.

Even though this is new API, it is replacing a necessary call to
rte_mempool_create() which was used to create the session pools.
I think we should also include that it needs to be used instead of mempool_create (being more explicit).

> +
> 
>  ABI Changes
>  -----------

...

> +++ b/lib/librte_cryptodev/rte_cryptodev_version.map
> @@ -88,6 +88,13 @@ DPDK_18.05 {
> 
>  } DPDK_17.11;
> 
> +DPDK_19.02 {
> +	global:
> +
> +	rte_cryptodev_sym_session_pool_create;

Just saw that check-symbol-change is complaining about this new API being stable.
Apologies for having you changing from experimental to stable, when the API needs to be experimental.

> +
> +} DPDK_18.05;
> +
>  EXPERIMENTAL {
>  	global:
> 
> --
> 2.13.6

Apart from this, patch looks ok to me:

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
  

Patch

diff --git a/doc/guides/prog_guide/cryptodev_lib.rst b/doc/guides/prog_guide/cryptodev_lib.rst
index 050a58dc4..366508618 100644
--- a/doc/guides/prog_guide/cryptodev_lib.rst
+++ b/doc/guides/prog_guide/cryptodev_lib.rst
@@ -455,13 +455,15 @@  Crypto workloads.
 
 .. figure:: img/cryptodev_sym_sess.*
 
-The Crypto device framework provides APIs to allocate and initialize sessions
-for crypto devices, where sessions are mempool objects.
+The Crypto device framework provides APIs to create session mempool and allocate
+and initialize sessions for crypto devices, where sessions are mempool objects.
 It is the application's responsibility to create and manage the session mempools.
 This approach allows for different scenarios such as having a single session
 mempool for all crypto devices (where the mempool object size is big
 enough to hold the private session of any crypto device), as well as having
-multiple session mempools of different sizes for better memory usage.
+multiple session mempools of different sizes for better memory usage. However,
+the application is required to use ``rte_cryptodev_sym_session_pool_create()``
+to create symmetric session mempool.
 
 An application can use ``rte_cryptodev_sym_get_private_session_size()`` to
 get the private session size of given crypto device. This function would allow
@@ -623,7 +625,8 @@  using one of the crypto PMDs available in DPDK.
     #define IV_OFFSET            (sizeof(struct rte_crypto_op) + \
                                  sizeof(struct rte_crypto_sym_op))
 
-    struct rte_mempool *mbuf_pool, *crypto_op_pool, *session_pool;
+    struct rte_mempool *mbuf_pool, *crypto_op_pool;
+    struct *session_pool, *session_priv_pool;
     unsigned int session_size;
     int ret;
 
@@ -673,13 +676,20 @@  using one of the crypto PMDs available in DPDK.
     /* Get private session data size. */
     session_size = rte_cryptodev_sym_get_private_session_size(cdev_id);
 
+    /* Create session mempool for the session header. */
+    session_pool = rte_cryptodev_sym_session_pool_create("session_pool",
+                                    MAX_SESSIONS,
+                                    0,
+                                    POOL_CACHE_SIZE,
+                                    0,
+                                    socket_id);
+
     /*
-     * Create session mempool, with two objects per session,
-     * one for the session header and another one for the
+     * Create session private data mempool for the
      * private session data for the crypto device.
      */
-    session_pool = rte_mempool_create("session_pool",
-                                    MAX_SESSIONS * 2,
+    session_priv_pool = rte_mempool_create("session_pool",
+                                    MAX_SESSIONS,
                                     session_size,
                                     POOL_CACHE_SIZE,
                                     0, NULL, NULL, NULL,
@@ -731,7 +741,7 @@  using one of the crypto PMDs available in DPDK.
         rte_exit(EXIT_FAILURE, "Session could not be created\n");
 
     if (rte_cryptodev_sym_session_init(cdev_id, session,
-                    &cipher_xform, session_pool) < 0)
+                    &cipher_xform, session_priv_pool) < 0)
         rte_exit(EXIT_FAILURE, "Session could not be initialized "
                     "for the crypto device\n");
 
diff --git a/doc/guides/rel_notes/release_19_02.rst b/doc/guides/rel_notes/release_19_02.rst
index 75128f8f2..0b3c30c80 100644
--- a/doc/guides/rel_notes/release_19_02.rst
+++ b/doc/guides/rel_notes/release_19_02.rst
@@ -167,6 +167,11 @@  API Changes
 * cryptodev: as shown in the the 18.11 deprecation notice, the last parameter
   of ``rte_cryptodev_queue_pair_setup()`` is removed.
 
+* cryptodev: a new function ``rte_cryptodev_sym_session_pool_create()`` is
+  introduced. This function is used to create symmetric session mempool safely
+  and all crypto applications are required to use this function to create
+  symmetric session mempool from now on.
+
 
 ABI Changes
 -----------
diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 4929d0451..5b1449d14 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -189,6 +189,16 @@  const char *rte_crypto_asym_op_strings[] = {
 	[RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE] = "sharedsecret_compute",
 };
 
+/**
+ * The private data structure stored in the session mempool private data.
+ */
+struct rte_cryptodev_sym_session_pool_private_data {
+	uint16_t nb_drivers;
+	/**< number of elements in sess_data array */
+	uint16_t user_data_sz;
+	/**< session user data will be placed after sess_data */
+};
+
 int
 rte_cryptodev_get_cipher_algo_enum(enum rte_crypto_cipher_algorithm *algo_enum,
 		const char *algo_string)
@@ -1223,6 +1233,46 @@  rte_cryptodev_asym_session_init(uint8_t dev_id,
 	return 0;
 }
 
+struct rte_mempool *
+rte_cryptodev_sym_session_pool_create(const char *name, uint32_t nb_elts,
+	uint32_t elt_size, uint32_t cache_size, uint16_t user_data_size,
+	int socket_id)
+{
+	struct rte_mempool *mp;
+	struct rte_cryptodev_sym_session_pool_private_data *pool_priv;
+	uint32_t obj_sz;
+
+	obj_sz = rte_cryptodev_sym_get_header_session_size() + user_data_size;
+	if (obj_sz > elt_size)
+		CDEV_LOG_INFO("elt_size %u is expanded to %u\n", elt_size,
+				obj_sz);
+	else
+		obj_sz = elt_size;
+
+	mp = rte_mempool_create(name, nb_elts, obj_sz, cache_size,
+			(uint32_t)(sizeof(*pool_priv)),
+			NULL, NULL, NULL, NULL,
+			socket_id, 0);
+	if (mp == NULL) {
+		CDEV_LOG_ERR("%s(name=%s) failed, rte_errno=%d\n",
+			__func__, name, rte_errno);
+		return NULL;
+	}
+
+	pool_priv = rte_mempool_get_priv(mp);
+	if (!pool_priv) {
+		CDEV_LOG_ERR("%s(name=%s) failed to get private data\n",
+			__func__, name);
+		rte_mempool_free(mp);
+		return NULL;
+	}
+
+	pool_priv->nb_drivers = nb_drivers;
+	pool_priv->user_data_sz = user_data_size;
+
+	return mp;
+}
+
 struct rte_cryptodev_sym_session *
 rte_cryptodev_sym_session_create(struct rte_mempool *mp)
 {
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index f9e7507da..265f02cbe 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -968,6 +968,37 @@  struct rte_cryptodev_asym_session {
 };
 
 /**
+ * Create a symmetric session mempool.
+ *
+ * @param name
+ *   The unique mempool name.
+ * @param nb_elts
+ *   The number of elements in the mempool.
+ * @param elt_size
+ *   The size of the element. This value will be ignored if it is smaller than
+ *   the minimum session header size required for the system. For the user who
+ *   want to use the same mempool for sym session and session private data it
+ *   can be the maximum value of all existing devices' private data and session
+ *   header sizes.
+ * @param cache_size
+ *   The number of per-lcore cache elements
+ * @param priv_size
+ *   The private data size of each session.
+ * @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
+ *   constraint for the reserved zone.
+ *
+ * @return
+ *  - On success return size of the session
+ *  - On failure returns 0
+ */
+struct rte_mempool *
+rte_cryptodev_sym_session_pool_create(const char *name, uint32_t nb_elts,
+	uint32_t elt_size, uint32_t cache_size, uint16_t priv_size,
+	int socket_id);
+
+/**
  * Create symmetric crypto session header (generic with no private data)
  *
  * @param   mempool    Symmetric session mempool to allocate session
diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map
index a695b61dc..5b6b679f5 100644
--- a/lib/librte_cryptodev/rte_cryptodev_version.map
+++ b/lib/librte_cryptodev/rte_cryptodev_version.map
@@ -88,6 +88,13 @@  DPDK_18.05 {
 
 } DPDK_17.11;
 
+DPDK_19.02 {
+	global:
+
+	rte_cryptodev_sym_session_pool_create;
+
+} DPDK_18.05;
+
 EXPERIMENTAL {
 	global: