[3/3] cryptodev: hide sym session structure

Message ID 20220829160645.378406-4-roy.fan.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series cryptodev: sym session framework rework |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation warning apply issues

Commit Message

Fan Zhang Aug. 29, 2022, 4:06 p.m. UTC
  Structure rte_cryptodev_sym_session is moved to internal
headers which are not visible to applications.
The only field which should be used by app is opaque_data.
This field can now be accessed via set/get APIs added in this
patch.
Subsequent changes in app and lib are made to compile the code.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Signed-off-by: Akhil Goyal <gakhil@marvell.com>
---
 app/test/test_ipsec_perf.c                  |  4 +-
 doc/guides/prog_guide/cryptodev_lib.rst     | 16 ++----
 doc/guides/rel_notes/deprecation.rst        |  9 ++++
 doc/guides/rel_notes/release_22_11.rst      |  7 +++
 drivers/crypto/dpaa2_sec/dpaa2_sec_raw_dp.c |  2 +-
 drivers/crypto/dpaa_sec/dpaa_sec_raw_dp.c   |  2 +-
 drivers/crypto/qat/qat_sym.c                |  2 +-
 lib/cryptodev/cryptodev_pmd.h               | 31 ++++++++++++
 lib/cryptodev/rte_cryptodev.c               | 16 +++---
 lib/cryptodev/rte_cryptodev.h               | 56 ++++++++++-----------
 lib/cryptodev/rte_cryptodev_trace.h         |  8 +--
 lib/ipsec/rte_ipsec_group.h                 |  5 +-
 lib/ipsec/ses.c                             |  3 +-
 13 files changed, 100 insertions(+), 61 deletions(-)
  

Comments

Akhil Goyal Sept. 18, 2022, 1:19 p.m. UTC | #1
> +#define CRYPTO_SESS_OPAQUE_DATA_OFF 0

CRYPTO_SESS_OPAQUE_DATA_OFF cannot be 0 as you have added a driver_id at start of struct.


> +/**
> + * Get opaque data from session handle
> + */
> +static inline uint64_t
> +rte_cryptodev_sym_session_opaque_data_get(void *sess)
> +{
> +	return *((uint64_t *)sess - CRYPTO_SESS_OPAQUE_DATA_OFF);
> +}
> +
> +/**
> + * Set opaque data in session handle
> + */
> +static inline void
> +rte_cryptodev_sym_session_opaque_data_set(void *sess, uint64_t opaque)
> +{
> +	uint64_t *data;
> +	data = (((uint64_t *)sess) - CRYPTO_SESS_OPAQUE_DATA_OFF);
> +	*data = opaque;
> +}
  

Patch

diff --git a/app/test/test_ipsec_perf.c b/app/test/test_ipsec_perf.c
index b5d0c2e036..b221b7fc32 100644
--- a/app/test/test_ipsec_perf.c
+++ b/app/test/test_ipsec_perf.c
@@ -227,7 +227,7 @@  static int
 create_sa(enum rte_security_session_action_type action_type,
 	  struct ipsec_sa *sa)
 {
-	static struct rte_cryptodev_sym_session dummy_ses;
+	void *dummy_ses = NULL;
 	size_t sz;
 	int rc;
 
@@ -247,7 +247,7 @@  create_sa(enum rte_security_session_action_type action_type,
 		"failed to allocate memory for rte_ipsec_sa\n");
 
 	sa->ss[0].type = action_type;
-	sa->ss[0].crypto.ses = &dummy_ses;
+	sa->ss[0].crypto.ses = dummy_ses;
 
 	rc = rte_ipsec_sa_init(sa->ss[0].sa, &sa->sa_prm, sz);
 	rc = (rc > 0 && (uint32_t)rc <= sz) ? 0 : -EINVAL;
diff --git a/doc/guides/prog_guide/cryptodev_lib.rst b/doc/guides/prog_guide/cryptodev_lib.rst
index 9e54683aa1..01aad842a9 100644
--- a/doc/guides/prog_guide/cryptodev_lib.rst
+++ b/doc/guides/prog_guide/cryptodev_lib.rst
@@ -125,13 +125,11 @@  Each queue pairs resources may be allocated on a specified socket.
         uint32_t nb_descriptors; /**< Number of descriptors per queue pair */
         struct rte_mempool *mp_session;
         /**< The mempool for creating session in sessionless mode */
-        struct rte_mempool *mp_session_private;
-        /**< The mempool for creating sess private data in sessionless mode */
     };
 
 
-The fields ``mp_session`` and ``mp_session_private`` are used for creating
-temporary session to process the crypto operations in the session-less mode.
+The field ``mp_session`` is used for creating temporary session to process
+the crypto operations in the session-less mode.
 They can be the same other different mempools. Please note not all Cryptodev
 PMDs supports session-less mode.
 
@@ -595,7 +593,7 @@  chain.
         struct rte_mbuf *m_dst;
 
         union {
-            struct rte_cryptodev_sym_session *session;
+            void *session;
             /**< Handle for the initialised session context */
             struct rte_crypto_sym_xform *xform;
             /**< Session-less API Crypto operation parameters */
@@ -943,15 +941,11 @@  using one of the crypto PMDs available in DPDK.
 
     /* Create crypto session and initialize it for the crypto device. */
     struct rte_cryptodev_sym_session *session;
-    session = rte_cryptodev_sym_session_create(session_pool);
+    session = rte_cryptodev_sym_session_create(cdev_id, &cipher_xform,
+                    session_pool);
     if (session == NULL)
         rte_exit(EXIT_FAILURE, "Session could not be created\n");
 
-    if (rte_cryptodev_sym_session_init(cdev_id, session,
-                    &cipher_xform, session_priv_pool) < 0)
-        rte_exit(EXIT_FAILURE, "Session could not be initialized "
-                    "for the crypto device\n");
-
     /* Get a burst of crypto operations. */
     struct rte_crypto_op *crypto_ops[BURST_SIZE];
     if (rte_crypto_op_bulk_alloc(crypto_op_pool,
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index e7583cae4c..ba46b6930f 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -237,3 +237,12 @@  Deprecation Notices
   applications should be updated to use the ``dmadev`` library instead,
   with the underlying HW-functionality being provided by the ``ioat`` or
   ``idxd`` dma drivers
+
+* cryptodev: Hide structure ``rte_cryptodev_sym_session`` to remove unnecessary
+  indirection between session and the private data of session. An opaque pointer
+  can be exposed directly to application which can be attached to the
+  ``rte_crypto_op``.
+
+* cryptodev: The functions ``rte_cryptodev_sym_session_init`` and
+  ``rte_cryptodev_sym_session_clear`` are deprecated. The sym crypto session
+  opaque pointer can no longer be used by different crypto device drivers.
diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
index 7fab9d6550..d6f199573f 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -89,6 +89,13 @@  API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* cryptodev: The structure ``rte_cryptodev_sym_session`` was moved to
+  cryptodev_pmd.h and was hidden from the application. The APIs to create/init and
+  destroy sym crypto session were updated to take a single mempool with element size
+  enough to hold session data and session private data. Inline APIs was created to
+  get and set the session data. All sample applications were updated to attach an
+  opaque pointer for the session to the ``rte_crypto_op`` while enqueuing.
+
 
 ABI Changes
 -----------
diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_raw_dp.c b/drivers/crypto/dpaa2_sec/dpaa2_sec_raw_dp.c
index 795be3acc3..8c1e0abb95 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_raw_dp.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_raw_dp.c
@@ -1012,7 +1012,7 @@  dpaa2_sec_configure_raw_dp_ctx(struct rte_cryptodev *dev, uint16_t qp_id,
 		sess = (dpaa2_sec_session *)get_sec_session_private_data(
 				session_ctx.sec_sess);
 	else if (sess_type == RTE_CRYPTO_OP_WITH_SESSION)
-		sess = (void *)session_ctx.crypto_sess->driver_priv_data;
+		sess = CRYPTODEV_GET_SYM_SESS_PRIV(session_ctx.crypto_sess);
 	else
 		return -ENOTSUP;
 	raw_dp_ctx->dequeue_burst = dpaa2_sec_raw_dequeue_burst;
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec_raw_dp.c b/drivers/crypto/dpaa_sec/dpaa_sec_raw_dp.c
index d2e4d9d787..93129e7614 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec_raw_dp.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec_raw_dp.c
@@ -1018,7 +1018,7 @@  dpaa_sec_configure_raw_dp_ctx(struct rte_cryptodev *dev, uint16_t qp_id,
 				session_ctx.sec_sess);
 	else if (sess_type == RTE_CRYPTO_OP_WITH_SESSION)
 		sess = (dpaa_sec_session *)
-			session_ctx.crypto_sess->driver_priv_data;
+			CRYPTODEV_GET_SYM_SESS_PRIV(session_ctx.crypto_sess);
 	else
 		return -ENOTSUP;
 	raw_dp_ctx->dequeue_burst = dpaa_sec_raw_dequeue_burst;
diff --git a/drivers/crypto/qat/qat_sym.c b/drivers/crypto/qat/qat_sym.c
index 408ae9e42b..e94a199f4b 100644
--- a/drivers/crypto/qat/qat_sym.c
+++ b/drivers/crypto/qat/qat_sym.c
@@ -386,7 +386,7 @@  qat_sym_configure_dp_ctx(struct rte_cryptodev *dev, uint16_t qp_id,
 	if (sess_type != RTE_CRYPTO_OP_WITH_SESSION)
 		return -EINVAL;
 
-	ctx = (void *)session_ctx.crypto_sess->driver_priv_data;
+	ctx = CRYPTODEV_GET_SYM_SESS_PRIV(session_ctx.crypto_sess);
 
 	dp_ctx->session = ctx;
 
diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
index f518a0f89b..7bd39e02a3 100644
--- a/lib/cryptodev/cryptodev_pmd.h
+++ b/lib/cryptodev/cryptodev_pmd.h
@@ -132,6 +132,37 @@  struct cryptodev_driver {
 	uint8_t id;
 };
 
+/** Cryptodev symmetric crypto session
+ * Each session is derived from a fixed xform chain. Therefore each session
+ * has a fixed algo, key, op-type, digest_len etc.
+ */
+struct rte_cryptodev_sym_session {
+	RTE_MARKER cacheline0;
+	uint8_t driver_id;
+	uint64_t opaque_data;
+	/**< Can be used for external metadata */
+	uint32_t sess_data_sz;
+	/**< Pointer to the user data stored after sess data */
+	uint16_t user_data_sz;
+	/**< session user data will be placed after sess data */
+	rte_iova_t driver_priv_data_iova;
+	/**< session driver data IOVA address */
+
+	RTE_MARKER cacheline1 __rte_cache_min_aligned;
+	/**< second cache line - start of the driver session data */
+	uint8_t driver_priv_data[0];
+	/**< Driver specific session data, variable size */
+};
+
+/**
+ * Helper macro to get driver private data
+ */
+#define CRYPTODEV_GET_SYM_SESS_PRIV(s) \
+	(void *)(((struct rte_cryptodev_sym_session *)s)->driver_priv_data)
+#define CRYPTODEV_GET_SYM_SESS_PRIV_IOVA(s) \
+	(((struct rte_cryptodev_sym_session *)s)->driver_priv_data_iova)
+
+
 /**
  * Get the rte_cryptodev structure device pointer for the device. Assumes a
  * valid device index.
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index 63b8255a45..88dbe71c51 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -1941,11 +1941,11 @@  rte_cryptodev_asym_session_create(uint8_t dev_id,
 }
 
 int
-rte_cryptodev_sym_session_free(uint8_t dev_id,
-	struct rte_cryptodev_sym_session *sess)
+rte_cryptodev_sym_session_free(uint8_t dev_id, void *_sess)
 {
 	struct rte_cryptodev *dev;
 	struct rte_mempool *sess_mp;
+	struct rte_cryptodev_sym_session *sess = _sess;
 	struct rte_cryptodev_sym_session_pool_private_data *pool_priv;
 
 	if (sess == NULL)
@@ -2060,10 +2060,11 @@  rte_cryptodev_asym_get_private_session_size(uint8_t dev_id)
 }
 
 int
-rte_cryptodev_sym_session_set_user_data(
-		struct rte_cryptodev_sym_session *sess, void *data,
+rte_cryptodev_sym_session_set_user_data(void *_sess, void *data,
 		uint16_t size)
 {
+	struct rte_cryptodev_sym_session *sess = _sess;
+
 	if (sess == NULL)
 		return -EINVAL;
 
@@ -2075,8 +2076,10 @@  rte_cryptodev_sym_session_set_user_data(
 }
 
 void *
-rte_cryptodev_sym_session_get_user_data(struct rte_cryptodev_sym_session *sess)
+rte_cryptodev_sym_session_get_user_data(void *_sess)
 {
+	struct rte_cryptodev_sym_session *sess = _sess;
+
 	if (sess == NULL || sess->user_data_sz == 0)
 		return NULL;
 
@@ -2120,10 +2123,11 @@  sym_crypto_fill_status(struct rte_crypto_sym_vec *vec, int32_t errnum)
 
 uint32_t
 rte_cryptodev_sym_cpu_crypto_process(uint8_t dev_id,
-	struct rte_cryptodev_sym_session *sess, union rte_crypto_sym_ofs ofs,
+	void *_sess, union rte_crypto_sym_ofs ofs,
 	struct rte_crypto_sym_vec *vec)
 {
 	struct rte_cryptodev *dev;
+	struct rte_cryptodev_sym_session *sess = _sess;
 
 	if (!rte_cryptodev_is_valid_dev(dev_id)) {
 		sym_crypto_fill_status(vec, EINVAL);
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index 0c65958f25..0cbdccc61a 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -902,28 +902,6 @@  struct rte_cryptodev_cb_rcu {
 void *
 rte_cryptodev_get_sec_ctx(uint8_t dev_id);
 
-/** Cryptodev symmetric crypto session
- * Each session is derived from a fixed xform chain. Therefore each session
- * has a fixed algo, key, op-type, digest_len etc.
- */
-struct rte_cryptodev_sym_session {
-	RTE_MARKER cacheline0;
-	uint8_t driver_id;
-	uint64_t opaque_data;
-	/**< Can be used for external metadata */
-	uint32_t sess_data_sz;
-	/**< Pointer to the user data stored after sess data */
-	uint16_t user_data_sz;
-	/**< session user data will be placed after sess data */
-	rte_iova_t driver_priv_data_iova;
-	/**< session driver data IOVA address */
-
-	RTE_MARKER cacheline1 __rte_cache_min_aligned;
-	/**< second cache line - start of the driver session data */
-	uint8_t driver_priv_data[0];
-	/**< Driver specific session data, variable size */
-};
-
 /**
  * Create a symmetric session mempool.
  *
@@ -1036,7 +1014,7 @@  rte_cryptodev_asym_session_create(uint8_t dev_id,
  */
 int
 rte_cryptodev_sym_session_free(uint8_t dev_id,
-	struct rte_cryptodev_sym_session *sess);
+	void *sess);
 
 /**
  * Clears and frees asymmetric crypto session header and private data,
@@ -1136,11 +1114,31 @@  const char *rte_cryptodev_driver_name_get(uint8_t driver_id);
  */
 __rte_experimental
 int
-rte_cryptodev_sym_session_set_user_data(
-					struct rte_cryptodev_sym_session *sess,
+rte_cryptodev_sym_session_set_user_data(void *sess,
 					void *data,
 					uint16_t size);
 
+#define CRYPTO_SESS_OPAQUE_DATA_OFF 0
+/**
+ * Get opaque data from session handle
+ */
+static inline uint64_t
+rte_cryptodev_sym_session_opaque_data_get(void *sess)
+{
+	return *((uint64_t *)sess - CRYPTO_SESS_OPAQUE_DATA_OFF);
+}
+
+/**
+ * Set opaque data in session handle
+ */
+static inline void
+rte_cryptodev_sym_session_opaque_data_set(void *sess, uint64_t opaque)
+{
+	uint64_t *data;
+	data = (((uint64_t *)sess) - CRYPTO_SESS_OPAQUE_DATA_OFF);
+	*data = opaque;
+}
+
 /**
  * Get user data stored in a session.
  *
@@ -1153,8 +1151,7 @@  rte_cryptodev_sym_session_set_user_data(
  */
 __rte_experimental
 void *
-rte_cryptodev_sym_session_get_user_data(
-					struct rte_cryptodev_sym_session *sess);
+rte_cryptodev_sym_session_get_user_data(void *sess);
 
 /**
  * Store user data in an asymmetric session.
@@ -1202,7 +1199,7 @@  rte_cryptodev_asym_session_get_user_data(void *sess);
 __rte_experimental
 uint32_t
 rte_cryptodev_sym_cpu_crypto_process(uint8_t dev_id,
-	struct rte_cryptodev_sym_session *sess, union rte_crypto_sym_ofs ofs,
+	void *sess, union rte_crypto_sym_ofs ofs,
 	struct rte_crypto_sym_vec *vec);
 
 /**
@@ -1244,8 +1241,7 @@  rte_cryptodev_session_event_mdata_set(uint8_t dev_id, void *sess,
  * Union of different crypto session types, including session-less xform
  * pointer.
  */
-union rte_cryptodev_session_ctx {
-	struct rte_cryptodev_sym_session *crypto_sess;
+union rte_cryptodev_session_ctx {void *crypto_sess;
 	struct rte_crypto_sym_xform *xform;
 	struct rte_security_session *sec_sess;
 };
diff --git a/lib/cryptodev/rte_cryptodev_trace.h b/lib/cryptodev/rte_cryptodev_trace.h
index 055c44fb22..7b4ec5c389 100644
--- a/lib/cryptodev/rte_cryptodev_trace.h
+++ b/lib/cryptodev/rte_cryptodev_trace.h
@@ -73,14 +73,10 @@  RTE_TRACE_POINT(
 
 RTE_TRACE_POINT(
 	rte_cryptodev_trace_sym_session_create,
-	RTE_TRACE_POINT_ARGS(uint8_t dev_id,
-		struct rte_cryptodev_sym_session *sess, void *xforms,
+	RTE_TRACE_POINT_ARGS(uint8_t dev_id, void *sess, void *xforms,
 		void *mempool),
 	rte_trace_point_emit_u8(dev_id);
 	rte_trace_point_emit_ptr(sess);
-	rte_trace_point_emit_u64(sess->opaque_data);
-	rte_trace_point_emit_u8(sess->driver_id);
-	rte_trace_point_emit_u16(sess->user_data_sz);
 	rte_trace_point_emit_ptr(xforms);
 	rte_trace_point_emit_ptr(mempool);
 )
@@ -108,7 +104,7 @@  RTE_TRACE_POINT(
 
 RTE_TRACE_POINT(
 	rte_cryptodev_trace_sym_session_free,
-	RTE_TRACE_POINT_ARGS(struct rte_cryptodev_sym_session *sess),
+	RTE_TRACE_POINT_ARGS(void *sess),
 	rte_trace_point_emit_ptr(sess);
 )
 
diff --git a/lib/ipsec/rte_ipsec_group.h b/lib/ipsec/rte_ipsec_group.h
index 62c2bd7217..a4e0e128f8 100644
--- a/lib/ipsec/rte_ipsec_group.h
+++ b/lib/ipsec/rte_ipsec_group.h
@@ -45,14 +45,15 @@  static inline struct rte_ipsec_session *
 rte_ipsec_ses_from_crypto(const struct rte_crypto_op *cop)
 {
 	const struct rte_security_session *ss;
-	const struct rte_cryptodev_sym_session *cs;
+	void *cs;
 
 	if (cop->sess_type == RTE_CRYPTO_OP_SECURITY_SESSION) {
 		ss = cop->sym[0].sec_session;
 		return (struct rte_ipsec_session *)(uintptr_t)ss->opaque_data;
 	} else if (cop->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
 		cs = cop->sym[0].session;
-		return (struct rte_ipsec_session *)(uintptr_t)cs->opaque_data;
+		return (struct rte_ipsec_session *)(uintptr_t)
+			rte_cryptodev_sym_session_opaque_data_get(cs);
 	}
 	return NULL;
 }
diff --git a/lib/ipsec/ses.c b/lib/ipsec/ses.c
index 3d51ac4986..0d3c932302 100644
--- a/lib/ipsec/ses.c
+++ b/lib/ipsec/ses.c
@@ -45,7 +45,8 @@  rte_ipsec_session_prepare(struct rte_ipsec_session *ss)
 	ss->pkt_func = fp;
 
 	if (ss->type == RTE_SECURITY_ACTION_TYPE_NONE)
-		ss->crypto.ses->opaque_data = (uintptr_t)ss;
+		rte_cryptodev_sym_session_opaque_data_set(ss->crypto.ses,
+			(uintptr_t)ss);
 	else
 		ss->security.ses->opaque_data = (uintptr_t)ss;