DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v3 0/4] crypto: improve asym session usage
@ 2022-02-03 16:04 Ciara Power
  2022-02-03 16:04 ` [PATCH v3 1/4] crypto: use single buffer for asymmetric session Ciara Power
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Ciara Power @ 2022-02-03 16:04 UTC (permalink / raw)
  To: dev; +Cc: roy.fan.zhang, gakhil, anoobj, mdr, Ciara Power

This patchset includes improvements for the asymmetric session.
The main change is to the session structure, which is now a single buffer,
rather than having pointers to private data elsewhere.
This session structure is now hidden in an internal header,
so the app will never use it directly.

Some other changes include adding a user data API, and modifying
the return value for the create session function.

v3:
  - Added documentation in relevant patches.
  - Fixed setting user data size.
  - Fixed hiding structure, it should not be hidden from PMDs.
  - Fixed some other small formatting issues.
  - Increased size of max_priv_session_sz to uint16_t.
  - Removed trace for asym session init function that was
    previously removed.

Ciara Power (4):
  crypto: use single buffer for asymmetric session
  crypto: hide asym session structure
  crypto: add asym session user data API
  crypto: modify return value for asym session create

 app/test-crypto-perf/cperf_ops.c             |  22 +-
 app/test/test_cryptodev_asym.c               | 316 ++++++-------------
 doc/guides/prog_guide/cryptodev_lib.rst      |  83 ++---
 doc/guides/rel_notes/release_22_03.rst       |  13 +
 drivers/crypto/cnxk/cn10k_cryptodev_ops.c    |   6 +-
 drivers/crypto/cnxk/cn9k_cryptodev_ops.c     |   6 +-
 drivers/crypto/cnxk/cnxk_cryptodev_ops.c     |  11 +-
 drivers/crypto/octeontx/otx_cryptodev_ops.c  |  29 +-
 drivers/crypto/openssl/rte_openssl_pmd.c     |   5 +-
 drivers/crypto/openssl/rte_openssl_pmd_ops.c |  23 +-
 drivers/crypto/qat/qat_asym.c                |  53 +---
 lib/cryptodev/cryptodev_pmd.h                |  32 +-
 lib/cryptodev/cryptodev_trace_points.c       |   6 +-
 lib/cryptodev/rte_cryptodev.c                | 217 ++++++++-----
 lib/cryptodev/rte_cryptodev.h                | 114 ++++---
 lib/cryptodev/rte_cryptodev_trace.h          |  26 +-
 lib/cryptodev/version.map                    |   7 +-
 17 files changed, 451 insertions(+), 518 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/4] crypto: use single buffer for asymmetric session
  2022-02-03 16:04 [PATCH v3 0/4] crypto: improve asym session usage Ciara Power
@ 2022-02-03 16:04 ` Ciara Power
  2022-02-07  8:19   ` [EXT] " Akhil Goyal
  2022-02-03 16:04 ` [PATCH v3 2/4] crypto: hide asym session structure Ciara Power
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Ciara Power @ 2022-02-03 16:04 UTC (permalink / raw)
  To: dev
  Cc: roy.fan.zhang, gakhil, anoobj, mdr, Ciara Power, Declan Doherty,
	Ankur Dwivedi, Tejasree Kondoj, John Griffin, Fiona Trahe,
	Deepak Kumar Jain

Rather than using a session buffer that contains pointers to private
session data elsewhere, have a single session buffer.
This session is created for a driver ID, and the mempool element
contains space for the max session private data needed for any driver.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

---
v2:
  - Renamed function typedef from "free" to "clear" as session private
    data isn't being freed in that function.
  - Moved user data API to separate patch.
  - Minor fixes to comments, formatting, return values.
v3:
  - Corrected formatting of struct comments.
  - Increased size of max_priv_session_sz to uint16_t.
  - Removed trace for asym session init function that was
    previously removed.
  - Added documentation.
---
 app/test-crypto-perf/cperf_ops.c             |  14 +-
 app/test/test_cryptodev_asym.c               | 200 ++++---------------
 doc/guides/prog_guide/cryptodev_lib.rst      |  55 ++---
 doc/guides/rel_notes/release_22_03.rst       |   7 +
 drivers/crypto/cnxk/cn10k_cryptodev_ops.c    |   6 +-
 drivers/crypto/cnxk/cn9k_cryptodev_ops.c     |   6 +-
 drivers/crypto/cnxk/cnxk_cryptodev_ops.c     |  11 +-
 drivers/crypto/octeontx/otx_cryptodev_ops.c  |  29 +--
 drivers/crypto/openssl/rte_openssl_pmd.c     |   5 +-
 drivers/crypto/openssl/rte_openssl_pmd_ops.c |  23 +--
 drivers/crypto/qat/qat_asym.c                |  53 ++---
 lib/cryptodev/cryptodev_pmd.h                |  17 +-
 lib/cryptodev/cryptodev_trace_points.c       |   6 +-
 lib/cryptodev/rte_cryptodev.c                | 167 ++++++++++------
 lib/cryptodev/rte_cryptodev.h                |  72 ++++---
 lib/cryptodev/rte_cryptodev_trace.h          |  21 +-
 lib/cryptodev/version.map                    |   5 +-
 17 files changed, 258 insertions(+), 439 deletions(-)

diff --git a/app/test-crypto-perf/cperf_ops.c b/app/test-crypto-perf/cperf_ops.c
index d975ae1ab8..bdc5dc9544 100644
--- a/app/test-crypto-perf/cperf_ops.c
+++ b/app/test-crypto-perf/cperf_ops.c
@@ -735,7 +735,6 @@ cperf_create_session(struct rte_mempool *sess_mp,
 	struct rte_crypto_sym_xform aead_xform;
 	struct rte_cryptodev_sym_session *sess = NULL;
 	struct rte_crypto_asym_xform xform = {0};
-	int rc;
 
 	if (options->op_type == CPERF_ASYM_MODEX) {
 		xform.next = NULL;
@@ -745,19 +744,10 @@ cperf_create_session(struct rte_mempool *sess_mp,
 		xform.modex.exponent.data = perf_mod_e;
 		xform.modex.exponent.length = sizeof(perf_mod_e);
 
-		sess = (void *)rte_cryptodev_asym_session_create(sess_mp);
+		sess = (void *)rte_cryptodev_asym_session_create(sess_mp, dev_id, &xform);
 		if (sess == NULL)
 			return NULL;
-		rc = rte_cryptodev_asym_session_init(dev_id, (void *)sess,
-						     &xform, priv_mp);
-		if (rc < 0) {
-			if (sess != NULL) {
-				rte_cryptodev_asym_session_clear(dev_id,
-								 (void *)sess);
-				rte_cryptodev_asym_session_free((void *)sess);
-			}
-			return NULL;
-		}
+
 		return sess;
 	}
 #ifdef RTE_LIB_SECURITY
diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
index 68f4d8e7a6..f7c2fd2588 100644
--- a/app/test/test_cryptodev_asym.c
+++ b/app/test/test_cryptodev_asym.c
@@ -450,7 +450,8 @@ test_cryptodev_asym_op(struct crypto_testsuite_params_asym *ts_params,
 	}
 
 	if (!sessionless) {
-		sess = rte_cryptodev_asym_session_create(ts_params->session_mpool);
+		sess = rte_cryptodev_asym_session_create(ts_params->session_mpool,
+				dev_id, &xform_tc);
 		if (!sess) {
 			snprintf(test_msg, ASYM_TEST_MSG_LEN,
 					"line %u "
@@ -460,15 +461,6 @@ test_cryptodev_asym_op(struct crypto_testsuite_params_asym *ts_params,
 			goto error_exit;
 		}
 
-		if (rte_cryptodev_asym_session_init(dev_id, sess, &xform_tc,
-				ts_params->session_mpool) < 0) {
-			snprintf(test_msg, ASYM_TEST_MSG_LEN,
-					"line %u FAILED: %s",
-					__LINE__, "unabled to config sym session");
-			status = TEST_FAILED;
-			goto error_exit;
-		}
-
 		rte_crypto_op_attach_asym_session(op, sess);
 	} else {
 		asym_op->xform = &xform_tc;
@@ -667,18 +659,11 @@ test_rsa_sign_verify(void)
 		return TEST_SKIPPED;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &rsa_xform);
 
 	if (!sess) {
 		RTE_LOG(ERR, USER1, "Session creation failed for "
 			"sign_verify\n");
-		return TEST_FAILED;
-	}
-
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &rsa_xform,
-				sess_mpool) < 0) {
-		RTE_LOG(ERR, USER1, "Unable to config asym session for "
-			"sign_verify\n");
 		status = TEST_FAILED;
 		goto error_exit;
 	}
@@ -686,7 +671,6 @@ test_rsa_sign_verify(void)
 	status = queue_ops_rsa_sign_verify(sess);
 
 error_exit:
-
 	rte_cryptodev_asym_session_clear(dev_id, sess);
 	rte_cryptodev_asym_session_free(sess);
 
@@ -716,17 +700,10 @@ test_rsa_enc_dec(void)
 		return TEST_SKIPPED;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &rsa_xform);
 
 	if (!sess) {
 		RTE_LOG(ERR, USER1, "Session creation failed for enc_dec\n");
-		return TEST_FAILED;
-	}
-
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &rsa_xform,
-				sess_mpool) < 0) {
-		RTE_LOG(ERR, USER1, "Unable to config asym session for "
-			"enc_dec\n");
 		status = TEST_FAILED;
 		goto error_exit;
 	}
@@ -763,22 +740,15 @@ test_rsa_sign_verify_crt(void)
 		return TEST_SKIPPED;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &rsa_xform_crt);
 
 	if (!sess) {
 		RTE_LOG(ERR, USER1, "Session creation failed for "
 			"sign_verify_crt\n");
 		status = TEST_FAILED;
-		return status;
-	}
-
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &rsa_xform_crt,
-				sess_mpool) < 0) {
-		RTE_LOG(ERR, USER1, "Unable to config asym session for "
-			"sign_verify_crt\n");
-		status = TEST_FAILED;
 		goto error_exit;
 	}
+
 	status = queue_ops_rsa_sign_verify(sess);
 
 error_exit:
@@ -811,21 +781,15 @@ test_rsa_enc_dec_crt(void)
 		return TEST_SKIPPED;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &rsa_xform_crt);
 
 	if (!sess) {
 		RTE_LOG(ERR, USER1, "Session creation failed for "
 			"enc_dec_crt\n");
-		return TEST_FAILED;
-	}
-
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &rsa_xform_crt,
-				sess_mpool) < 0) {
-		RTE_LOG(ERR, USER1, "Unable to config asym session for "
-			"enc_dec_crt\n");
 		status = TEST_FAILED;
 		goto error_exit;
 	}
+
 	status = queue_ops_rsa_enc_dec(sess);
 
 error_exit:
@@ -924,7 +888,6 @@ testsuite_setup(void)
 	/* configure qp */
 	ts_params->qp_conf.nb_descriptors = DEFAULT_NUM_OPS_INFLIGHT;
 	ts_params->qp_conf.mp_session = ts_params->session_mpool;
-	ts_params->qp_conf.mp_session_private = ts_params->session_mpool;
 	for (qp_id = 0; qp_id < info.max_nb_queue_pairs; qp_id++) {
 		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
 			dev_id, qp_id, &ts_params->qp_conf,
@@ -933,21 +896,9 @@ testsuite_setup(void)
 			qp_id, dev_id);
 	}
 
-	/* setup asym session pool */
-	unsigned int session_size = RTE_MAX(
-		rte_cryptodev_asym_get_private_session_size(dev_id),
-		rte_cryptodev_asym_get_header_session_size());
-	/*
-	 * Create mempool with TEST_NUM_SESSIONS * 2,
-	 * to include the session headers
-	 */
-	ts_params->session_mpool = rte_mempool_create(
-				"test_asym_sess_mp",
-				TEST_NUM_SESSIONS * 2,
-				session_size,
-				0, 0, NULL, NULL, NULL,
-				NULL, SOCKET_ID_ANY,
-				0);
+	ts_params->session_mpool = rte_cryptodev_asym_session_pool_create(
+			"test_asym_sess_mp", TEST_NUM_SESSIONS * 2, 0,
+			SOCKET_ID_ANY);
 
 	TEST_ASSERT_NOT_NULL(ts_params->session_mpool,
 			"session mempool allocation failed");
@@ -1104,14 +1055,6 @@ test_dh_gen_shared_sec(struct rte_crypto_asym_xform *xfrm)
 	struct rte_crypto_asym_xform xform = *xfrm;
 	uint8_t peer[] = "01234567890123456789012345678901234567890123456789";
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
-	if (sess == NULL) {
-		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s", __LINE__,
-				"Session creation failed");
-		status = TEST_FAILED;
-		goto error_exit;
-	}
 	/* set up crypto op data structure */
 	op = rte_crypto_op_alloc(op_mpool, RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
 	if (!op) {
@@ -1134,11 +1077,11 @@ test_dh_gen_shared_sec(struct rte_crypto_asym_xform *xfrm)
 	asym_op->dh.shared_secret.data = output;
 	asym_op->dh.shared_secret.length = sizeof(output);
 
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
-			sess_mpool) < 0) {
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
+	if (sess == NULL) {
 		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s",
-				__LINE__, "unabled to config sym session");
+				"line %u FAILED: %s", __LINE__,
+				"Session creation failed");
 		status = TEST_FAILED;
 		goto error_exit;
 	}
@@ -1196,14 +1139,6 @@ test_dh_gen_priv_key(struct rte_crypto_asym_xform *xfrm)
 	uint8_t output[TEST_DH_MOD_LEN];
 	struct rte_crypto_asym_xform xform = *xfrm;
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
-	if (sess == NULL) {
-		RTE_LOG(ERR, USER1,
-				 "line %u FAILED: %s", __LINE__,
-				"Session creation failed");
-		status = TEST_FAILED;
-		goto error_exit;
-	}
 	/* set up crypto op data structure */
 	op = rte_crypto_op_alloc(op_mpool, RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
 	if (!op) {
@@ -1222,11 +1157,11 @@ test_dh_gen_priv_key(struct rte_crypto_asym_xform *xfrm)
 	asym_op->dh.priv_key.data = output;
 	asym_op->dh.priv_key.length = sizeof(output);
 
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
-			sess_mpool) < 0) {
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
+	if (sess == NULL) {
 		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s",
-				__LINE__, "unabled to config sym session");
+				"line %u FAILED: %s", __LINE__,
+				"Session creation failed");
 		status = TEST_FAILED;
 		goto error_exit;
 	}
@@ -1287,14 +1222,6 @@ test_dh_gen_pub_key(struct rte_crypto_asym_xform *xfrm)
 	uint8_t output[TEST_DH_MOD_LEN];
 	struct rte_crypto_asym_xform xform = *xfrm;
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
-	if (sess == NULL) {
-		RTE_LOG(ERR, USER1,
-				 "line %u FAILED: %s", __LINE__,
-				"Session creation failed");
-		status = TEST_FAILED;
-		goto error_exit;
-	}
 	/* set up crypto op data structure */
 	op = rte_crypto_op_alloc(op_mpool, RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
 	if (!op) {
@@ -1321,11 +1248,11 @@ test_dh_gen_pub_key(struct rte_crypto_asym_xform *xfrm)
 					0);
 	asym_op->dh.priv_key = dh_test_params.priv_key;
 
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
-			sess_mpool) < 0) {
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
+	if (sess == NULL) {
 		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s",
-				__LINE__, "unabled to config sym session");
+				"line %u FAILED: %s", __LINE__,
+				"Session creation failed");
 		status = TEST_FAILED;
 		goto error_exit;
 	}
@@ -1388,15 +1315,6 @@ test_dh_gen_kp(struct rte_crypto_asym_xform *xfrm)
 	struct rte_crypto_asym_xform pub_key_xform;
 	struct rte_crypto_asym_xform xform = *xfrm;
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
-	if (sess == NULL) {
-		RTE_LOG(ERR, USER1,
-				 "line %u FAILED: %s", __LINE__,
-				"Session creation failed");
-		status = TEST_FAILED;
-		goto error_exit;
-	}
-
 	/* set up crypto op data structure */
 	op = rte_crypto_op_alloc(op_mpool, RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
 	if (!op) {
@@ -1420,11 +1338,12 @@ test_dh_gen_kp(struct rte_crypto_asym_xform *xfrm)
 	asym_op->dh.pub_key.length = sizeof(out_pub_key);
 	asym_op->dh.priv_key.data = out_prv_key;
 	asym_op->dh.priv_key.length = sizeof(out_prv_key);
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
-			sess_mpool) < 0) {
+
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
+	if (sess == NULL) {
 		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s",
-				__LINE__, "unabled to config sym session");
+				"line %u FAILED: %s", __LINE__,
+				"Session creation failed");
 		status = TEST_FAILED;
 		goto error_exit;
 	}
@@ -1511,7 +1430,7 @@ test_mod_inv(void)
 				return TEST_SKIPPED;
 		}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &modinv_xform);
 	if (!sess) {
 		RTE_LOG(ERR, USER1, "line %u "
 				"FAILED: %s", __LINE__,
@@ -1520,15 +1439,6 @@ test_mod_inv(void)
 		goto error_exit;
 	}
 
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &modinv_xform,
-			sess_mpool) < 0) {
-		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s",
-				__LINE__, "unabled to config sym session");
-		status = TEST_FAILED;
-		goto error_exit;
-	}
-
 	/* generate crypto op data structure */
 	op = rte_crypto_op_alloc(op_mpool, RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
 	if (!op) {
@@ -1646,7 +1556,7 @@ test_mod_exp(void)
 		goto error_exit;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &modex_xform);
 	if (!sess) {
 		RTE_LOG(ERR, USER1,
 				 "line %u "
@@ -1656,15 +1566,6 @@ test_mod_exp(void)
 		goto error_exit;
 	}
 
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &modex_xform,
-			sess_mpool) < 0) {
-		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s",
-				__LINE__, "unabled to config sym session");
-		status = TEST_FAILED;
-		goto error_exit;
-	}
-
 	asym_op = op->asym;
 	memcpy(input, base, sizeof(base));
 	asym_op->modex.base.data = input;
@@ -1768,7 +1669,7 @@ test_dsa_sign(void)
 	uint8_t s[TEST_DH_MOD_LEN];
 	uint8_t dgst[] = "35d81554afaad2cf18f3a1770d5fedc4ea5be344";
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &dsa_xform);
 	if (sess == NULL) {
 		RTE_LOG(ERR, USER1,
 				 "line %u FAILED: %s", __LINE__,
@@ -1797,15 +1698,6 @@ test_dsa_sign(void)
 	debug_hexdump(stdout, "priv_key: ", dsa_xform.dsa.x.data,
 			dsa_xform.dsa.x.length);
 
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &dsa_xform,
-				sess_mpool) < 0) {
-		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s",
-				__LINE__, "unabled to config sym session");
-		status = TEST_FAILED;
-		goto error_exit;
-	}
-
 	/* attach asymmetric crypto session to crypto operations */
 	rte_crypto_op_attach_asym_session(op, sess);
 	asym_op->dsa.op_type = RTE_CRYPTO_ASYM_OP_SIGN;
@@ -1941,15 +1833,6 @@ test_ecdsa_sign_verify(enum curve curve_id)
 
 	rte_cryptodev_info_get(dev_id, &dev_info);
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
-	if (sess == NULL) {
-		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s", __LINE__,
-				"Session creation failed\n");
-		status = TEST_FAILED;
-		goto exit;
-	}
-
 	/* Setup crypto op data structure */
 	op = rte_crypto_op_alloc(op_mpool, RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
 	if (op == NULL) {
@@ -1967,11 +1850,11 @@ test_ecdsa_sign_verify(enum curve curve_id)
 	xform.xform_type = RTE_CRYPTO_ASYM_XFORM_ECDSA;
 	xform.ec.curve_id = input_params.curve;
 
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
-				sess_mpool) < 0) {
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
+	if (sess == NULL) {
 		RTE_LOG(ERR, USER1,
 				"line %u FAILED: %s", __LINE__,
-				"Unable to config asym session\n");
+				"Session creation failed\n");
 		status = TEST_FAILED;
 		goto exit;
 	}
@@ -2154,15 +2037,6 @@ test_ecpm(enum curve curve_id)
 
 	rte_cryptodev_info_get(dev_id, &dev_info);
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool);
-	if (sess == NULL) {
-		RTE_LOG(ERR, USER1,
-				"line %u FAILED: %s", __LINE__,
-				"Session creation failed\n");
-		status = TEST_FAILED;
-		goto exit;
-	}
-
 	/* Setup crypto op data structure */
 	op = rte_crypto_op_alloc(op_mpool, RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
 	if (op == NULL) {
@@ -2180,11 +2054,11 @@ test_ecpm(enum curve curve_id)
 	xform.xform_type = RTE_CRYPTO_ASYM_XFORM_ECPM;
 	xform.ec.curve_id = input_params.curve;
 
-	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
-				sess_mpool) < 0) {
+	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
+	if (sess == NULL) {
 		RTE_LOG(ERR, USER1,
 				"line %u FAILED: %s", __LINE__,
-				"Unable to config asym session\n");
+				"Session creation failed\n");
 		status = TEST_FAILED;
 		goto exit;
 	}
diff --git a/doc/guides/prog_guide/cryptodev_lib.rst b/doc/guides/prog_guide/cryptodev_lib.rst
index 8766bc34a9..f8f8562f4c 100644
--- a/doc/guides/prog_guide/cryptodev_lib.rst
+++ b/doc/guides/prog_guide/cryptodev_lib.rst
@@ -1038,20 +1038,17 @@ It is the application's responsibility to create and manage the session mempools
 Application using both symmetric and asymmetric sessions should allocate and maintain
 different sessions pools for each type.
 
-An application can use ``rte_cryptodev_get_asym_session_private_size()`` to
-get the private size of asymmetric session on a given crypto device. This
-function would allow an application to calculate the max device asymmetric
-session size of all crypto devices to create a single session mempool.
-If instead an application creates multiple asymmetric session mempools,
-the Crypto device framework also provides ``rte_cryptodev_asym_get_header_session_size()`` to get
-the size of an uninitialized session.
+An application can use ``rte_cryptodev_asym_session_pool_create()`` to create a mempool
+with a specified number of elements. The element size will allow for the session header,
+and the max private session size.
+The max private session size is chosen based on available crypto devices,
+the biggest private session size is used. This means any of those devices can be used,
+and the mempool element will have available space for its private session data.
 
 Once the session mempools have been created, ``rte_cryptodev_asym_session_create()``
-is used to allocate an uninitialized asymmetric session from the given mempool.
-The session then must be initialized using ``rte_cryptodev_asym_session_init()``
-for each of the required crypto devices. An asymmetric transform chain
-is used to specify the operation and its parameters. See the section below for
-details on transforms.
+is used to allocate and initialize an asymmetric session from the given mempool.
+An asymmetric transform chain is used to specify the operation and its parameters.
+See the section below for details on transforms.
 
 When a session is no longer used, user must call ``rte_cryptodev_asym_session_clear()``
 for each of the crypto devices that are using the session, to free all driver
@@ -1162,21 +1159,14 @@ crypto operations is similar except change to respective op and xform setup).
 
     uint8_t cdev_id = rte_cryptodev_get_dev_id(crypto_name);
 
-    /* Get private asym session data size. */
-    asym_session_size = rte_cryptodev_get_asym_private_session_size(cdev_id);
-
     /*
-     * Create session mempool, with two objects per session,
-     * one for the session header and another one for the
-     * private asym session data for the crypto device.
+     * Create session mempool, this will create elements big enough
+     * to hold the generic session header,
+     * and the max private session size of the available devices.
      */
-    asym_session_pool = rte_mempool_create("asym_session_pool",
-                                    MAX_ASYM_SESSIONS * 2,
-                                    asym_session_size,
-                                    0,
-                                    0, NULL, NULL, NULL,
-                                    NULL, socket_id,
-                                    0);
+    asym_session_pool = rte_cryptodev_asym_session_pool_create(
+                        "asym_session_pool", MAX_ASYM_SESSIONS, 0, 0,
+                        socket_id);
 
     /* Configure the crypto device. */
     struct rte_cryptodev_config conf = {
@@ -1190,8 +1180,7 @@ crypto operations is similar except change to respective op and xform setup).
     if (rte_cryptodev_configure(cdev_id, &conf) < 0)
         rte_exit(EXIT_FAILURE, "Failed to configure cryptodev %u", cdev_id);
 
-    if (rte_cryptodev_queue_pair_setup(cdev_id, 0, &qp_conf,
-                            socket_id, asym_session_pool) < 0)
+    if (rte_cryptodev_queue_pair_setup(cdev_id, 0, &qp_conf, socket_id) < 0)
         rte_exit(EXIT_FAILURE, "Failed to setup queue pair\n");
 
     if (rte_cryptodev_start(cdev_id) < 0)
@@ -1226,15 +1215,11 @@ crypto operations is similar except change to respective op and xform setup).
     };
     /* Create asym crypto session and initialize it for the crypto device. */
     struct rte_cryptodev_asym_session *asym_session;
-    asym_session = rte_cryptodev_asym_session_create(asym_session_pool);
+    asym_session = rte_cryptodev_asym_session_create(asym_session_pool,
+            cdev_id, &modex_xform);
     if (asym_session == NULL)
         rte_exit(EXIT_FAILURE, "Session could not be created\n");
 
-    if (rte_cryptodev_asym_session_init(cdev_id, asym_session,
-                    &modex_xform, asym_session_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[1];
     if (rte_crypto_op_bulk_alloc(crypto_op_pool,
@@ -1245,11 +1230,11 @@ crypto operations is similar except change to respective op and xform setup).
     /* Set up the crypto operations. */
     struct rte_crypto_asym_op *asym_op = crypto_ops[0]->asym;
 
-	/* calculate mod exp of value 0xf8 */
+    /* calculate mod exp of value 0xf8 */
     static unsigned char base[] = {0xF8};
     asym_op->modex.base.data = base;
     asym_op->modex.base.length = sizeof(base);
-	asym_op->modex.base.iova = base;
+    asym_op->modex.base.iova = base;
 
     /* Attach the asym crypto session to the operation */
     rte_crypto_op_attach_asym_session(op, asym_session);
diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 3bc0630c7c..de8d8ce4e9 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -100,6 +100,13 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* cryptodev: The asym session handling was modified to use a single buffer.
+  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.
+  ``rte_cryptodev_asym_session_init`` was removed as this initialisation is
+  now done by ``rte_cryptodev_asym_session_create``.
+
 
 ABI Changes
 -----------
diff --git a/drivers/crypto/cnxk/cn10k_cryptodev_ops.c b/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
index d217bbf383..7390f976c6 100644
--- a/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
@@ -157,8 +157,7 @@ cn10k_cpt_fill_inst(struct cnxk_cpt_qp *qp, struct rte_crypto_op *ops[],
 
 		if (op->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
 			asym_op = op->asym;
-			ae_sess = get_asym_session_private_data(
-				asym_op->session, cn10k_cryptodev_driver_id);
+			ae_sess = get_asym_session_private_data(asym_op->session);
 			ret = cnxk_ae_enqueue(qp, op, infl_req, &inst[0],
 					      ae_sess);
 			if (unlikely(ret))
@@ -431,8 +430,7 @@ cn10k_cpt_dequeue_post_process(struct cnxk_cpt_qp *qp,
 			uintptr_t *mdata = infl_req->mdata;
 			struct cnxk_ae_sess *sess;
 
-			sess = get_asym_session_private_data(
-				op->session, cn10k_cryptodev_driver_id);
+			sess = get_asym_session_private_data(op->session);
 
 			cnxk_ae_post_process(cop, sess, (uint8_t *)mdata[0]);
 		}
diff --git a/drivers/crypto/cnxk/cn9k_cryptodev_ops.c b/drivers/crypto/cnxk/cn9k_cryptodev_ops.c
index ac1953b66d..59a06af30e 100644
--- a/drivers/crypto/cnxk/cn9k_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cn9k_cryptodev_ops.c
@@ -138,8 +138,7 @@ cn9k_cpt_inst_prep(struct cnxk_cpt_qp *qp, struct rte_crypto_op *op,
 
 		if (op->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
 			asym_op = op->asym;
-			sess = get_asym_session_private_data(
-				asym_op->session, cn9k_cryptodev_driver_id);
+			sess = get_asym_session_private_data(asym_op->session);
 			ret = cnxk_ae_enqueue(qp, op, infl_req, inst, sess);
 			inst->w7.u64 = sess->cpt_inst_w7;
 		} else {
@@ -453,8 +452,7 @@ cn9k_cpt_dequeue_post_process(struct cnxk_cpt_qp *qp, struct rte_crypto_op *cop,
 			uintptr_t *mdata = infl_req->mdata;
 			struct cnxk_ae_sess *sess;
 
-			sess = get_asym_session_private_data(
-				op->session, cn9k_cryptodev_driver_id);
+			sess = get_asym_session_private_data(op->session);
 
 			cnxk_ae_post_process(cop, sess, (uint8_t *)mdata[0]);
 		}
diff --git a/drivers/crypto/cnxk/cnxk_cryptodev_ops.c b/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
index 67a2d9b08e..0d561ba0f3 100644
--- a/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
@@ -657,7 +657,7 @@ cnxk_ae_session_clear(struct rte_cryptodev *dev,
 	struct rte_mempool *sess_mp;
 	struct cnxk_ae_sess *priv;
 
-	priv = get_asym_session_private_data(sess, dev->driver_id);
+	priv = get_asym_session_private_data(sess);
 	if (priv == NULL)
 		return;
 
@@ -667,7 +667,6 @@ cnxk_ae_session_clear(struct rte_cryptodev *dev,
 	/* Reset and free object back to pool */
 	memset(priv, 0, cnxk_ae_session_size_get(dev));
 	sess_mp = rte_mempool_from_obj(priv);
-	set_asym_session_private_data(sess, dev->driver_id, NULL);
 	rte_mempool_put(sess_mp, priv);
 }
 
@@ -679,15 +678,10 @@ cnxk_ae_session_cfg(struct rte_cryptodev *dev,
 {
 	struct cnxk_cpt_vf *vf = dev->data->dev_private;
 	struct roc_cpt *roc_cpt = &vf->cpt;
-	struct cnxk_ae_sess *priv;
+	struct cnxk_ae_sess *priv = get_asym_session_private_data(sess);
 	union cpt_inst_w7 w7;
 	int ret;
 
-	if (rte_mempool_get(pool, (void **)&priv))
-		return -ENOMEM;
-
-	memset(priv, 0, sizeof(struct cnxk_ae_sess));
-
 	ret = cnxk_ae_fill_session_parameters(priv, xform);
 	if (ret) {
 		rte_mempool_put(pool, priv);
@@ -699,7 +693,6 @@ cnxk_ae_session_cfg(struct rte_cryptodev *dev,
 	priv->cpt_inst_w7 = w7.u64;
 	priv->cnxk_fpm_iova = vf->cnxk_fpm_iova;
 	priv->ec_grp = vf->ec_grp;
-	set_asym_session_private_data(sess, dev->driver_id, priv);
 
 	return 0;
 }
diff --git a/drivers/crypto/octeontx/otx_cryptodev_ops.c b/drivers/crypto/octeontx/otx_cryptodev_ops.c
index f7ca8a8a8e..22c54dde68 100644
--- a/drivers/crypto/octeontx/otx_cryptodev_ops.c
+++ b/drivers/crypto/octeontx/otx_cryptodev_ops.c
@@ -375,35 +375,24 @@ otx_cpt_asym_session_size_get(struct rte_cryptodev *dev __rte_unused)
 }
 
 static int
-otx_cpt_asym_session_cfg(struct rte_cryptodev *dev,
+otx_cpt_asym_session_cfg(struct rte_cryptodev *dev __rte_unused,
 			 struct rte_crypto_asym_xform *xform __rte_unused,
 			 struct rte_cryptodev_asym_session *sess,
-			 struct rte_mempool *pool)
+			 struct rte_mempool *pool __rte_unused)
 {
-	struct cpt_asym_sess_misc *priv;
+	struct cpt_asym_sess_misc *priv = get_asym_session_private_data(sess);
 	int ret;
 
 	CPT_PMD_INIT_FUNC_TRACE();
 
-	if (rte_mempool_get(pool, (void **)&priv)) {
-		CPT_LOG_ERR("Could not allocate session private data");
-		return -ENOMEM;
-	}
-
-	memset(priv, 0, sizeof(struct cpt_asym_sess_misc));
-
 	ret = cpt_fill_asym_session_parameters(priv, xform);
 	if (ret) {
 		CPT_LOG_ERR("Could not configure session parameters");
-
-		/* Return session to mempool */
-		rte_mempool_put(pool, priv);
 		return ret;
 	}
 
 	priv->cpt_inst_w7 = 0;
 
-	set_asym_session_private_data(sess, dev->driver_id, priv);
 	return 0;
 }
 
@@ -412,11 +401,10 @@ otx_cpt_asym_session_clear(struct rte_cryptodev *dev,
 			   struct rte_cryptodev_asym_session *sess)
 {
 	struct cpt_asym_sess_misc *priv;
-	struct rte_mempool *sess_mp;
 
 	CPT_PMD_INIT_FUNC_TRACE();
 
-	priv = get_asym_session_private_data(sess, dev->driver_id);
+	priv = get_asym_session_private_data(sess);
 
 	if (priv == NULL)
 		return;
@@ -424,9 +412,6 @@ otx_cpt_asym_session_clear(struct rte_cryptodev *dev,
 	/* Free resources allocated during session configure */
 	cpt_free_asym_session_parameters(priv);
 	memset(priv, 0, otx_cpt_asym_session_size_get(dev));
-	sess_mp = rte_mempool_from_obj(priv);
-	set_asym_session_private_data(sess, dev->driver_id, NULL);
-	rte_mempool_put(sess_mp, priv);
 }
 
 static __rte_always_inline void * __rte_hot
@@ -471,8 +456,7 @@ otx_cpt_enq_single_asym(struct cpt_instance *instance,
 		return NULL;
 	}
 
-	sess = get_asym_session_private_data(asym_op->session,
-					     otx_cryptodev_driver_id);
+	sess = get_asym_session_private_data(asym_op->session);
 
 	/* Store phys_addr of the mdata to meta_buf */
 	params.meta_buf = rte_mempool_virt2iova(mdata);
@@ -852,8 +836,7 @@ otx_cpt_asym_post_process(struct rte_crypto_op *cop,
 	struct rte_crypto_asym_op *op = cop->asym;
 	struct cpt_asym_sess_misc *sess;
 
-	sess = get_asym_session_private_data(op->session,
-					     otx_cryptodev_driver_id);
+	sess = get_asym_session_private_data(op->session);
 
 	switch (sess->xfrm_type) {
 	case RTE_CRYPTO_ASYM_XFORM_RSA:
diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 5794ed8159..1e7e5f6849 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -747,10 +747,7 @@ get_session(struct openssl_qp *qp, struct rte_crypto_op *op)
 						cryptodev_driver_id);
 		} else {
 			if (likely(op->asym->session != NULL))
-				asym_sess = (struct openssl_asym_session *)
-						get_asym_session_private_data(
-						op->asym->session,
-						cryptodev_driver_id);
+				asym_sess = get_asym_session_private_data(op->asym->session);
 			if (asym_sess == NULL)
 				op->status =
 					RTE_CRYPTO_OP_STATUS_INVALID_SESSION;
diff --git a/drivers/crypto/openssl/rte_openssl_pmd_ops.c b/drivers/crypto/openssl/rte_openssl_pmd_ops.c
index 52715f86f8..061fdd2837 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd_ops.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd_ops.c
@@ -1120,7 +1120,7 @@ static int
 openssl_pmd_asym_session_configure(struct rte_cryptodev *dev __rte_unused,
 		struct rte_crypto_asym_xform *xform,
 		struct rte_cryptodev_asym_session *sess,
-		struct rte_mempool *mempool)
+		struct rte_mempool *mempool __rte_unused)
 {
 	void *asym_sess_private_data;
 	int ret;
@@ -1130,25 +1130,14 @@ openssl_pmd_asym_session_configure(struct rte_cryptodev *dev __rte_unused,
 		return -EINVAL;
 	}
 
-	if (rte_mempool_get(mempool, &asym_sess_private_data)) {
-		CDEV_LOG_ERR(
-			"Couldn't get object from session mempool");
-		return -ENOMEM;
-	}
-
+	asym_sess_private_data = get_asym_session_private_data(sess);
 	ret = openssl_set_asym_session_parameters(asym_sess_private_data,
 			xform);
 	if (ret != 0) {
 		OPENSSL_LOG(ERR, "failed configure session parameters");
-
-		/* Return session to mempool */
-		rte_mempool_put(mempool, asym_sess_private_data);
 		return ret;
 	}
 
-	set_asym_session_private_data(sess, dev->driver_id,
-			asym_sess_private_data);
-
 	return 0;
 }
 
@@ -1206,19 +1195,15 @@ static void openssl_reset_asym_session(struct openssl_asym_session *sess)
  * so it doesn't leave key material behind
  */
 static void
-openssl_pmd_asym_session_clear(struct rte_cryptodev *dev,
+openssl_pmd_asym_session_clear(struct rte_cryptodev *dev __rte_unused,
 		struct rte_cryptodev_asym_session *sess)
 {
-	uint8_t index = dev->driver_id;
-	void *sess_priv = get_asym_session_private_data(sess, index);
+	void *sess_priv = get_asym_session_private_data(sess);
 
 	/* Zero out the whole structure */
 	if (sess_priv) {
 		openssl_reset_asym_session(sess_priv);
 		memset(sess_priv, 0, sizeof(struct openssl_asym_session));
-		struct rte_mempool *sess_mp = rte_mempool_from_obj(sess_priv);
-		set_asym_session_private_data(sess, index, NULL);
-		rte_mempool_put(sess_mp, sess_priv);
 	}
 }
 
diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c
index 09d8761c5f..32d3f01a18 100644
--- a/drivers/crypto/qat/qat_asym.c
+++ b/drivers/crypto/qat/qat_asym.c
@@ -491,9 +491,7 @@ qat_asym_build_request(void *in_op,
 
 	op->status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
 	if (op->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
-		ctx = (struct qat_asym_session *)
-			get_asym_session_private_data(
-			op->asym->session, qat_asym_driver_id);
+		ctx = get_asym_session_private_data(op->asym->session);
 		if (unlikely(ctx == NULL)) {
 			QAT_LOG(ERR, "Session has not been created for this device");
 			goto error;
@@ -711,8 +709,7 @@ qat_asym_process_response(void **op, uint8_t *resp,
 	}
 
 	if (rx_op->sess_type == RTE_CRYPTO_OP_WITH_SESSION) {
-		ctx = (struct qat_asym_session *)get_asym_session_private_data(
-			rx_op->asym->session, qat_asym_driver_id);
+		ctx = get_asym_session_private_data(rx_op->asym->session);
 		qat_asym_collect_response(rx_op, cookie, ctx->xform);
 	} else if (rx_op->sess_type == RTE_CRYPTO_OP_SESSIONLESS) {
 		qat_asym_collect_response(rx_op, cookie, rx_op->asym->xform);
@@ -726,61 +723,43 @@ qat_asym_process_response(void **op, uint8_t *resp,
 }
 
 int
-qat_asym_session_configure(struct rte_cryptodev *dev,
+qat_asym_session_configure(struct rte_cryptodev *dev __rte_unused,
 		struct rte_crypto_asym_xform *xform,
 		struct rte_cryptodev_asym_session *sess,
-		struct rte_mempool *mempool)
+		struct rte_mempool *mempool __rte_unused)
 {
-	int err = 0;
-	void *sess_private_data;
 	struct qat_asym_session *session;
 
-	if (rte_mempool_get(mempool, &sess_private_data)) {
-		QAT_LOG(ERR,
-			"Couldn't get object from session mempool");
-		return -ENOMEM;
-	}
-
-	session = sess_private_data;
+	session = get_asym_session_private_data(sess);
 	if (xform->xform_type == RTE_CRYPTO_ASYM_XFORM_MODEX) {
 		if (xform->modex.exponent.length == 0 ||
 				xform->modex.modulus.length == 0) {
 			QAT_LOG(ERR, "Invalid mod exp input parameter");
-			err = -EINVAL;
-			goto error;
+			return -EINVAL;
 		}
 	} else if (xform->xform_type == RTE_CRYPTO_ASYM_XFORM_MODINV) {
 		if (xform->modinv.modulus.length == 0) {
 			QAT_LOG(ERR, "Invalid mod inv input parameter");
-			err = -EINVAL;
-			goto error;
+			return -EINVAL;
 		}
 	} else if (xform->xform_type == RTE_CRYPTO_ASYM_XFORM_RSA) {
 		if (xform->rsa.n.length == 0) {
 			QAT_LOG(ERR, "Invalid rsa input parameter");
-			err = -EINVAL;
-			goto error;
+			return -EINVAL;
 		}
 	} else if (xform->xform_type >= RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
 			|| xform->xform_type <= RTE_CRYPTO_ASYM_XFORM_NONE) {
 		QAT_LOG(ERR, "Invalid asymmetric crypto xform");
-		err = -EINVAL;
-		goto error;
+		return -EINVAL;
 	} else {
 		QAT_LOG(ERR, "Asymmetric crypto xform not implemented");
-		err = -EINVAL;
-		goto error;
+		return -EINVAL;
 	}
 
 	session->xform = xform;
-	qat_asym_build_req_tmpl(sess_private_data);
-	set_asym_session_private_data(sess, dev->driver_id,
-		sess_private_data);
+	qat_asym_build_req_tmpl(session);
 
 	return 0;
-error:
-	rte_mempool_put(mempool, sess_private_data);
-	return err;
 }
 
 unsigned int qat_asym_session_get_private_size(
@@ -793,15 +772,9 @@ void
 qat_asym_session_clear(struct rte_cryptodev *dev,
 		struct rte_cryptodev_asym_session *sess)
 {
-	uint8_t index = dev->driver_id;
-	void *sess_priv = get_asym_session_private_data(sess, index);
+	void *sess_priv = get_asym_session_private_data(sess);
 	struct qat_asym_session *s = (struct qat_asym_session *)sess_priv;
 
-	if (sess_priv) {
+	if (sess_priv)
 		memset(s, 0, qat_asym_session_get_private_size(dev));
-		struct rte_mempool *sess_mp = rte_mempool_from_obj(sess_priv);
-
-		set_asym_session_private_data(sess, index, NULL);
-		rte_mempool_put(sess_mp, sess_priv);
-	}
 }
diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
index b9146f652c..474d447496 100644
--- a/lib/cryptodev/cryptodev_pmd.h
+++ b/lib/cryptodev/cryptodev_pmd.h
@@ -340,12 +340,12 @@ typedef int (*cryptodev_asym_configure_session_t)(struct rte_cryptodev *dev,
 typedef void (*cryptodev_sym_free_session_t)(struct rte_cryptodev *dev,
 		struct rte_cryptodev_sym_session *sess);
 /**
- * Free asymmetric session private data.
+ * Clear asymmetric session private data.
  *
  * @param	dev		Crypto device pointer
  * @param	sess		Cryptodev session structure
  */
-typedef void (*cryptodev_asym_free_session_t)(struct rte_cryptodev *dev,
+typedef void (*cryptodev_asym_clear_session_t)(struct rte_cryptodev *dev,
 		struct rte_cryptodev_asym_session *sess);
 /**
  * Perform actual crypto processing (encrypt/digest or auth/decrypt)
@@ -429,7 +429,7 @@ struct rte_cryptodev_ops {
 	/**< Configure asymmetric Crypto session. */
 	cryptodev_sym_free_session_t sym_session_clear;
 	/**< Clear a Crypto sessions private data. */
-	cryptodev_asym_free_session_t asym_session_clear;
+	cryptodev_asym_clear_session_t asym_session_clear;
 	/**< Clear a Crypto sessions private data. */
 	union {
 		cryptodev_sym_cpu_crypto_process_t sym_cpu_process;
@@ -628,16 +628,9 @@ set_sym_session_private_data(struct rte_cryptodev_sym_session *sess,
 }
 
 static inline void *
-get_asym_session_private_data(const struct rte_cryptodev_asym_session *sess,
-		uint8_t driver_id) {
-	return sess->sess_private_data[driver_id];
-}
-
-static inline void
-set_asym_session_private_data(struct rte_cryptodev_asym_session *sess,
-		uint8_t driver_id, void *private_data)
+get_asym_session_private_data(struct rte_cryptodev_asym_session *sess)
 {
-	sess->sess_private_data[driver_id] = private_data;
+	return sess->sess_private_data;
 }
 
 #endif /* _CRYPTODEV_PMD_H_ */
diff --git a/lib/cryptodev/cryptodev_trace_points.c b/lib/cryptodev/cryptodev_trace_points.c
index 5d58951fd5..d23b30edd8 100644
--- a/lib/cryptodev/cryptodev_trace_points.c
+++ b/lib/cryptodev/cryptodev_trace_points.c
@@ -24,6 +24,9 @@ RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_setup,
 RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_pool_create,
 	lib.cryptodev.sym.pool.create)
 
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_pool_create,
+	lib.cryptodev.asym.pool.create)
+
 RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_create,
 	lib.cryptodev.sym.create)
 
@@ -39,9 +42,6 @@ RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_free,
 RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_init,
 	lib.cryptodev.sym.init)
 
-RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_init,
-	lib.cryptodev.asym.init)
-
 RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_clear,
 	lib.cryptodev.sym.clear)
 
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index a40536c5ea..d260f79bbc 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -195,7 +195,7 @@ const char *rte_crypto_asym_op_strings[] = {
 };
 
 /**
- * The private data structure stored in the session mempool private data.
+ * The private data structure stored in the sym session mempool private data.
  */
 struct rte_cryptodev_sym_session_pool_private_data {
 	uint16_t nb_drivers;
@@ -204,6 +204,14 @@ struct rte_cryptodev_sym_session_pool_private_data {
 	/**< session user data will be placed after sess_data */
 };
 
+/**
+ * The private data structure stored in the asym session mempool 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 */
+};
+
 int
 rte_cryptodev_get_cipher_algo_enum(enum rte_crypto_cipher_algorithm *algo_enum,
 		const char *algo_string)
@@ -1751,47 +1759,6 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
 	return 0;
 }
 
-int
-rte_cryptodev_asym_session_init(uint8_t dev_id,
-		struct rte_cryptodev_asym_session *sess,
-		struct rte_crypto_asym_xform *xforms,
-		struct rte_mempool *mp)
-{
-	struct rte_cryptodev *dev;
-	uint8_t index;
-	int ret;
-
-	if (!rte_cryptodev_is_valid_dev(dev_id)) {
-		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
-		return -EINVAL;
-	}
-
-	dev = rte_cryptodev_pmd_get_dev(dev_id);
-
-	if (sess == NULL || xforms == NULL || dev == NULL)
-		return -EINVAL;
-
-	index = dev->driver_id;
-
-	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->asym_session_configure,
-				-ENOTSUP);
-
-	if (sess->sess_private_data[index] == NULL) {
-		ret = dev->dev_ops->asym_session_configure(dev,
-							xforms,
-							sess, mp);
-		if (ret < 0) {
-			CDEV_LOG_ERR(
-				"dev_id %d failed to configure session details",
-				dev_id);
-			return ret;
-		}
-	}
-
-	rte_cryptodev_trace_asym_session_init(dev_id, sess, xforms, mp);
-	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,
@@ -1834,6 +1801,53 @@ rte_cryptodev_sym_session_pool_create(const char *name, uint32_t nb_elts,
 	return mp;
 }
 
+struct rte_mempool *
+rte_cryptodev_asym_session_pool_create(const char *name, uint32_t nb_elts,
+	uint32_t cache_size, int socket_id)
+{
+	struct rte_mempool *mp;
+	struct rte_cryptodev_asym_session_pool_private_data *pool_priv;
+	uint32_t obj_sz, obj_sz_aligned;
+	uint8_t dev_id, priv_sz, max_priv_sz = 0;
+
+	for (dev_id = 0; dev_id < RTE_CRYPTO_MAX_DEVS; dev_id++)
+		if (rte_cryptodev_is_valid_dev(dev_id)) {
+			priv_sz = rte_cryptodev_asym_get_private_session_size(dev_id);
+			if (priv_sz > max_priv_sz)
+				max_priv_sz = priv_sz;
+		}
+	if (max_priv_sz == 0) {
+		CDEV_LOG_INFO("Could not set max private session size\n");
+		return NULL;
+	}
+
+	obj_sz = rte_cryptodev_asym_get_header_session_size() + max_priv_sz;
+	obj_sz_aligned =  RTE_ALIGN_CEIL(obj_sz, RTE_CACHE_LINE_SIZE);
+
+	mp = rte_mempool_create(name, nb_elts, obj_sz_aligned, 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->max_priv_session_sz = max_priv_sz;
+
+	rte_cryptodev_trace_asym_session_pool_create(name, nb_elts,
+		cache_size, mp);
+	return mp;
+}
+
 static unsigned int
 rte_cryptodev_sym_session_data_size(struct rte_cryptodev_sym_session *sess)
 {
@@ -1895,19 +1909,43 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mp)
 }
 
 struct rte_cryptodev_asym_session *
-rte_cryptodev_asym_session_create(struct rte_mempool *mp)
+rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t dev_id,
+		struct rte_crypto_asym_xform *xforms)
 {
 	struct rte_cryptodev_asym_session *sess;
-	unsigned int session_size =
+	uint32_t session_priv_data_sz;
+	struct rte_cryptodev_asym_session_pool_private_data *pool_priv;
+	unsigned int session_header_size =
 			rte_cryptodev_asym_get_header_session_size();
+	struct rte_cryptodev *dev;
+	int ret;
+
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
+		return NULL;
+	}
+	session_priv_data_sz = rte_cryptodev_asym_get_private_session_size(
+			dev_id);
+	dev = rte_cryptodev_pmd_get_dev(dev_id);
+
+	if (dev == NULL)
+		return NULL;
 
 	if (!mp) {
 		CDEV_LOG_ERR("invalid mempool\n");
 		return NULL;
 	}
 
+	pool_priv = rte_mempool_get_priv(mp);
+
+	if (pool_priv->max_priv_session_sz < session_priv_data_sz) {
+		CDEV_LOG_DEBUG(
+			"The private session data size used when creating the mempool is smaller than this device's private session data.");
+		return NULL;
+	}
+
 	/* Verify if provided mempool can hold elements big enough. */
-	if (mp->elt_size < session_size) {
+	if (mp->elt_size < session_header_size + session_priv_data_sz) {
 		CDEV_LOG_ERR(
 			"mempool elements too small to hold session objects");
 		return NULL;
@@ -1919,10 +1957,27 @@ rte_cryptodev_asym_session_create(struct rte_mempool *mp)
 		return NULL;
 	}
 
+	sess->driver_id = dev->driver_id;
+	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, 0, session_size);
+	memset(sess->sess_private_data, 0, session_priv_data_sz);
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->asym_session_configure, NULL);
+
+	if (sess->sess_private_data[0] == 0) {
+		ret = dev->dev_ops->asym_session_configure(dev,
+							xforms,
+							sess, mp);
+		if (ret < 0) {
+			CDEV_LOG_ERR(
+				"dev_id %d failed to configure session details",
+				dev_id);
+			return NULL;
+		}
+	}
 
 	rte_cryptodev_trace_asym_session_create(mp, sess);
 	return sess;
@@ -2009,20 +2064,11 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
 int
 rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
 {
-	uint8_t i;
-	void *sess_priv;
 	struct rte_mempool *sess_mp;
 
 	if (sess == NULL)
 		return -EINVAL;
 
-	/* Check that all device private data has been freed */
-	for (i = 0; i < nb_drivers; i++) {
-		sess_priv = get_asym_session_private_data(sess, i);
-		if (sess_priv != NULL)
-			return -EBUSY;
-	}
-
 	/* Return session to mempool */
 	sess_mp = rte_mempool_from_obj(sess);
 	rte_mempool_put(sess_mp, sess);
@@ -2061,12 +2107,7 @@ rte_cryptodev_sym_get_existing_header_session_size(
 unsigned int
 rte_cryptodev_asym_get_header_session_size(void)
 {
-	/*
-	 * Header contains pointers to the private data
-	 * of all registered drivers, and a flag which
-	 * indicates presence of private data
-	 */
-	return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
+	return sizeof(struct rte_cryptodev_asym_session);
 }
 
 unsigned int
@@ -2092,7 +2133,6 @@ unsigned int
 rte_cryptodev_asym_get_private_session_size(uint8_t dev_id)
 {
 	struct rte_cryptodev *dev;
-	unsigned int header_size = sizeof(void *) * nb_drivers;
 	unsigned int priv_sess_size;
 
 	if (!rte_cryptodev_is_valid_dev(dev_id))
@@ -2104,11 +2144,8 @@ rte_cryptodev_asym_get_private_session_size(uint8_t dev_id)
 		return 0;
 
 	priv_sess_size = (*dev->dev_ops->asym_session_get_size)(dev);
-	if (priv_sess_size < header_size)
-		return header_size;
 
 	return priv_sess_size;
-
 }
 
 int
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index 59ea5a54df..8bd85f1575 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -919,9 +919,13 @@ struct rte_cryptodev_sym_session {
 };
 
 /** Cryptodev asymmetric crypto session */
-struct rte_cryptodev_asym_session {
-	__extension__ void *sess_private_data[0];
-	/**< Private asymmetric session material */
+__extension__ struct rte_cryptodev_asym_session {
+	uint8_t driver_id;
+	/**< Session driver ID. */
+	uint16_t max_priv_session_sz;
+	/**< Size of private session data used when creating mempool */
+	uint8_t padding[5];
+	uint8_t sess_private_data[0];
 };
 
 /**
@@ -956,6 +960,29 @@ 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 an asymmetric session mempool.
+ *
+ * @param name
+ *   The unique mempool name.
+ * @param nb_elts
+ *   The number of elements in the mempool.
+ * @param cache_size
+ *   The number of per-lcore cache elements
+ * @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 mempool
+ *  - On failure returns NULL
+ */
+__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);
+
 /**
  * Create symmetric crypto session header (generic with no private data)
  *
@@ -973,13 +1000,17 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mempool);
  *
  * @param   mempool    mempool to allocate asymmetric session
  *                     objects from
+ * @param   dev_id   ID of device that we want the session to be used on
+ * @param   xforms   Asymmetric crypto transform operations to apply on flow
+ *                   processed with this session
  * @return
  *  - On success return pointer to asym-session
  *  - On failure returns NULL
  */
 __rte_experimental
 struct rte_cryptodev_asym_session *
-rte_cryptodev_asym_session_create(struct rte_mempool *mempool);
+rte_cryptodev_asym_session_create(struct rte_mempool *mempool,
+		uint8_t dev_id, struct rte_crypto_asym_xform *xforms);
 
 /**
  * Frees symmetric crypto session header, after checking that all
@@ -997,8 +1028,7 @@ int
 rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess);
 
 /**
- * Frees asymmetric crypto session header, after checking that all
- * the device private data has been freed, returning it
+ * Frees asymmetric crypto session header, returning it
  * to its original mempool.
  *
  * @param   sess     Session header to be freed.
@@ -1006,7 +1036,6 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess);
  * @return
  *  - 0 if successful.
  *  - -EINVAL if session is NULL.
- *  - -EBUSY if not all device private data has been freed.
  */
 __rte_experimental
 int
@@ -1034,28 +1063,6 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
 			struct rte_crypto_sym_xform *xforms,
 			struct rte_mempool *mempool);
 
-/**
- * Initialize asymmetric session on a device with specific asymmetric xform
- *
- * @param   dev_id   ID of device that we want the session to be used on
- * @param   sess     Session to be set up on a device
- * @param   xforms   Asymmetric crypto transform operations to apply on flow
- *                   processed with this session
- * @param   mempool  Mempool to be used for internal allocation.
- *
- * @return
- *  - On success, zero.
- *  - -EINVAL if input parameters are invalid.
- *  - -ENOTSUP if crypto device does not support the crypto transform.
- *  - -ENOMEM if the private session could not be allocated.
- */
-__rte_experimental
-int
-rte_cryptodev_asym_session_init(uint8_t dev_id,
-			struct rte_cryptodev_asym_session *sess,
-			struct rte_crypto_asym_xform *xforms,
-			struct rte_mempool *mempool);
-
 /**
  * Frees private data for the device id, based on its device type,
  * returning it to its mempool. It is the application's responsibility
@@ -1075,11 +1082,10 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
 			struct rte_cryptodev_sym_session *sess);
 
 /**
- * Frees resources held by asymmetric session during rte_cryptodev_session_init
+ * Clear private data held by asymmetric session.
  *
  * @param   dev_id   ID of device that uses the asymmetric session.
- * @param   sess     Asymmetric session setup on device using
- *					 rte_cryptodev_session_init
+ * @param   sess     Asymmetric session setup on device.
  * @return
  *  - 0 if successful.
  *  - -EINVAL if device is invalid or session is NULL.
@@ -1116,7 +1122,7 @@ rte_cryptodev_sym_get_existing_header_session_size(
 		struct rte_cryptodev_sym_session *sess);
 
 /**
- * Get the size of the asymmetric session header, for all registered drivers.
+ * Get the size of the asymmetric session header.
  *
  * @return
  *   Size of the asymmetric header session.
diff --git a/lib/cryptodev/rte_cryptodev_trace.h b/lib/cryptodev/rte_cryptodev_trace.h
index d1f4f069a3..befbaf7f44 100644
--- a/lib/cryptodev/rte_cryptodev_trace.h
+++ b/lib/cryptodev/rte_cryptodev_trace.h
@@ -83,6 +83,16 @@ RTE_TRACE_POINT(
 	rte_trace_point_emit_u16(sess->user_data_sz);
 )
 
+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),
+	rte_trace_point_emit_string(name);
+	rte_trace_point_emit_u32(nb_elts);
+	rte_trace_point_emit_u32(cache_size);
+	rte_trace_point_emit_ptr(mempool);
+)
+
 RTE_TRACE_POINT(
 	rte_cryptodev_trace_asym_session_create,
 	RTE_TRACE_POINT_ARGS(void *mempool,
@@ -117,17 +127,6 @@ RTE_TRACE_POINT(
 	rte_trace_point_emit_ptr(mempool);
 )
 
-RTE_TRACE_POINT(
-	rte_cryptodev_trace_asym_session_init,
-	RTE_TRACE_POINT_ARGS(uint8_t dev_id,
-		struct rte_cryptodev_asym_session *sess, void *xforms,
-		void *mempool),
-	rte_trace_point_emit_u8(dev_id);
-	rte_trace_point_emit_ptr(sess);
-	rte_trace_point_emit_ptr(xforms);
-	rte_trace_point_emit_ptr(mempool);
-)
-
 RTE_TRACE_POINT(
 	rte_cryptodev_trace_sym_session_clear,
 	RTE_TRACE_POINT_ARGS(uint8_t dev_id, void *sess),
diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
index c50745fa8c..eaea976f21 100644
--- a/lib/cryptodev/version.map
+++ b/lib/cryptodev/version.map
@@ -58,7 +58,6 @@ EXPERIMENTAL {
 	rte_cryptodev_asym_session_clear;
 	rte_cryptodev_asym_session_create;
 	rte_cryptodev_asym_session_free;
-	rte_cryptodev_asym_session_init;
 	rte_cryptodev_asym_xform_capability_check_modlen;
 	rte_cryptodev_asym_xform_capability_check_optype;
 	rte_cryptodev_sym_cpu_crypto_process;
@@ -81,7 +80,6 @@ EXPERIMENTAL {
 	__rte_cryptodev_trace_sym_session_free;
 	__rte_cryptodev_trace_asym_session_free;
 	__rte_cryptodev_trace_sym_session_init;
-	__rte_cryptodev_trace_asym_session_init;
 	__rte_cryptodev_trace_sym_session_clear;
 	__rte_cryptodev_trace_asym_session_clear;
 	__rte_cryptodev_trace_dequeue_burst;
@@ -104,6 +102,9 @@ EXPERIMENTAL {
 	rte_cryptodev_remove_deq_callback;
 	rte_cryptodev_remove_enq_callback;
 
+	# added 22.03
+	rte_cryptodev_asym_session_pool_create;
+	__rte_cryptodev_trace_asym_session_pool_create;
 };
 
 INTERNAL {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 2/4] crypto: hide asym session structure
  2022-02-03 16:04 [PATCH v3 0/4] crypto: improve asym session usage Ciara Power
  2022-02-03 16:04 ` [PATCH v3 1/4] crypto: use single buffer for asymmetric session Ciara Power
@ 2022-02-03 16:04 ` Ciara Power
  2022-02-03 16:04 ` [PATCH v3 3/4] crypto: add asym session user data API Ciara Power
  2022-02-03 16:04 ` [PATCH v3 4/4] crypto: modify return value for asym session create Ciara Power
  3 siblings, 0 replies; 12+ messages in thread
From: Ciara Power @ 2022-02-03 16:04 UTC (permalink / raw)
  To: dev; +Cc: roy.fan.zhang, gakhil, anoobj, mdr, Ciara Power, Declan Doherty

The rte_cryptodev_asym_session structure is now moved to an internal
header. This will no longer be used directly by apps,
private session data can be accessed via get API.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

---
v3:
  - Removed the incorrect use of void * for asym session in drivers,
    these are internal and can use the struct directly as before.
  - Fixed other internal functions and comments that were incorrectly
    changed to use void *sess.
  - Added documentation.
---
 app/test-crypto-perf/cperf_ops.c        |  2 +-
 app/test/test_cryptodev_asym.c          | 34 ++++++++++++-------------
 doc/guides/prog_guide/cryptodev_lib.rst |  7 +++--
 doc/guides/rel_notes/release_22_03.rst  |  3 ++-
 lib/cryptodev/cryptodev_pmd.h           | 13 ++++++++++
 lib/cryptodev/rte_cryptodev.c           |  7 +++--
 lib/cryptodev/rte_cryptodev.h           | 17 +++----------
 lib/cryptodev/rte_cryptodev_trace.h     |  4 +--
 8 files changed, 46 insertions(+), 41 deletions(-)

diff --git a/app/test-crypto-perf/cperf_ops.c b/app/test-crypto-perf/cperf_ops.c
index bdc5dc9544..948dc0f608 100644
--- a/app/test-crypto-perf/cperf_ops.c
+++ b/app/test-crypto-perf/cperf_ops.c
@@ -21,7 +21,7 @@ cperf_set_ops_asym(struct rte_crypto_op **ops,
 		   uint64_t *tsc_start __rte_unused)
 {
 	uint16_t i;
-	struct rte_cryptodev_asym_session *asym_sess = (void *)sess;
+	void *asym_sess = (void *)sess;
 
 	for (i = 0; i < nb_ops; i++) {
 		struct rte_crypto_asym_op *asym_op = ops[i]->asym;
diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
index f7c2fd2588..f93f39af42 100644
--- a/app/test/test_cryptodev_asym.c
+++ b/app/test/test_cryptodev_asym.c
@@ -44,7 +44,7 @@ struct crypto_testsuite_params_asym {
 };
 
 struct crypto_unittest_params {
-	struct rte_cryptodev_asym_session *sess;
+	void *sess;
 	struct rte_crypto_op *op;
 };
 
@@ -65,7 +65,7 @@ static uint32_t test_index;
 static struct crypto_testsuite_params_asym testsuite_params = { NULL };
 
 static int
-queue_ops_rsa_sign_verify(struct rte_cryptodev_asym_session *sess)
+queue_ops_rsa_sign_verify(void *sess)
 {
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *op_mpool = ts_params->op_mpool;
@@ -156,7 +156,7 @@ queue_ops_rsa_sign_verify(struct rte_cryptodev_asym_session *sess)
 }
 
 static int
-queue_ops_rsa_enc_dec(struct rte_cryptodev_asym_session *sess)
+queue_ops_rsa_enc_dec(void *sess)
 {
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *op_mpool = ts_params->op_mpool;
@@ -308,7 +308,7 @@ test_cryptodev_asym_op(struct crypto_testsuite_params_asym *ts_params,
 	struct rte_crypto_op *op = NULL;
 	struct rte_crypto_op *result_op = NULL;
 	struct rte_crypto_asym_xform xform_tc;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	struct rte_cryptodev_asym_capability_idx cap_idx;
 	const struct rte_cryptodev_asymmetric_xform_capability *capability;
 	uint8_t dev_id = ts_params->valid_devs[0];
@@ -644,7 +644,7 @@ test_rsa_sign_verify(void)
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	uint8_t dev_id = ts_params->valid_devs[0];
-	struct rte_cryptodev_asym_session *sess;
+	void *sess;
 	struct rte_cryptodev_info dev_info;
 	int status = TEST_SUCCESS;
 
@@ -685,7 +685,7 @@ test_rsa_enc_dec(void)
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	uint8_t dev_id = ts_params->valid_devs[0];
-	struct rte_cryptodev_asym_session *sess;
+	void *sess;
 	struct rte_cryptodev_info dev_info;
 	int status = TEST_SUCCESS;
 
@@ -726,7 +726,7 @@ test_rsa_sign_verify_crt(void)
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	uint8_t dev_id = ts_params->valid_devs[0];
-	struct rte_cryptodev_asym_session *sess;
+	void *sess;
 	struct rte_cryptodev_info dev_info;
 	int status = TEST_SUCCESS;
 
@@ -767,7 +767,7 @@ test_rsa_enc_dec_crt(void)
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	uint8_t dev_id = ts_params->valid_devs[0];
-	struct rte_cryptodev_asym_session *sess;
+	void *sess;
 	struct rte_cryptodev_info dev_info;
 	int status = TEST_SUCCESS;
 
@@ -1049,7 +1049,7 @@ test_dh_gen_shared_sec(struct rte_crypto_asym_xform *xfrm)
 	uint8_t dev_id = ts_params->valid_devs[0];
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	int status = TEST_SUCCESS;
 	uint8_t output[TEST_DH_MOD_LEN];
 	struct rte_crypto_asym_xform xform = *xfrm;
@@ -1134,7 +1134,7 @@ test_dh_gen_priv_key(struct rte_crypto_asym_xform *xfrm)
 	uint8_t dev_id = ts_params->valid_devs[0];
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	int status = TEST_SUCCESS;
 	uint8_t output[TEST_DH_MOD_LEN];
 	struct rte_crypto_asym_xform xform = *xfrm;
@@ -1217,7 +1217,7 @@ test_dh_gen_pub_key(struct rte_crypto_asym_xform *xfrm)
 	uint8_t dev_id = ts_params->valid_devs[0];
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	int status = TEST_SUCCESS;
 	uint8_t output[TEST_DH_MOD_LEN];
 	struct rte_crypto_asym_xform xform = *xfrm;
@@ -1308,7 +1308,7 @@ test_dh_gen_kp(struct rte_crypto_asym_xform *xfrm)
 	uint8_t dev_id = ts_params->valid_devs[0];
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	int status = TEST_SUCCESS;
 	uint8_t out_pub_key[TEST_DH_MOD_LEN];
 	uint8_t out_prv_key[TEST_DH_MOD_LEN];
@@ -1397,7 +1397,7 @@ test_mod_inv(void)
 	uint8_t dev_id = ts_params->valid_devs[0];
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	int status = TEST_SUCCESS;
 	struct rte_cryptodev_asym_capability_idx cap_idx;
 	const struct rte_cryptodev_asymmetric_xform_capability *capability;
@@ -1512,7 +1512,7 @@ test_mod_exp(void)
 	uint8_t dev_id = ts_params->valid_devs[0];
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	int status = TEST_SUCCESS;
 	struct rte_cryptodev_asym_capability_idx cap_idx;
 	const struct rte_cryptodev_asymmetric_xform_capability *capability;
@@ -1663,7 +1663,7 @@ test_dsa_sign(void)
 	uint8_t dev_id = ts_params->valid_devs[0];
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	int status = TEST_SUCCESS;
 	uint8_t r[TEST_DH_MOD_LEN];
 	uint8_t s[TEST_DH_MOD_LEN];
@@ -1796,7 +1796,7 @@ test_ecdsa_sign_verify(enum curve curve_id)
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	struct rte_mempool *op_mpool = ts_params->op_mpool;
 	struct crypto_testsuite_ecdsa_params input_params;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	uint8_t dev_id = ts_params->valid_devs[0];
 	struct rte_crypto_op *result_op = NULL;
 	uint8_t output_buf_r[TEST_DATA_SIZE];
@@ -2000,7 +2000,7 @@ test_ecpm(enum curve curve_id)
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	struct rte_mempool *op_mpool = ts_params->op_mpool;
 	struct crypto_testsuite_ecpm_params input_params;
-	struct rte_cryptodev_asym_session *sess = NULL;
+	void *sess = NULL;
 	uint8_t dev_id = ts_params->valid_devs[0];
 	struct rte_crypto_op *result_op = NULL;
 	uint8_t output_buf_x[TEST_DATA_SIZE];
diff --git a/doc/guides/prog_guide/cryptodev_lib.rst b/doc/guides/prog_guide/cryptodev_lib.rst
index f8f8562f4c..56b19da3ae 100644
--- a/doc/guides/prog_guide/cryptodev_lib.rst
+++ b/doc/guides/prog_guide/cryptodev_lib.rst
@@ -1213,8 +1213,11 @@ crypto operations is similar except change to respective op and xform setup).
 			}
 		}
     };
-    /* Create asym crypto session and initialize it for the crypto device. */
-    struct rte_cryptodev_asym_session *asym_session;
+    /*
+     * Create asym crypto session and initialize it for the crypto device.
+     * The session structure is hidden from the app, so void * is used.
+     */
+    void *asym_session;
     asym_session = rte_cryptodev_asym_session_create(asym_session_pool,
             cdev_id, &modex_xform);
     if (asym_session == NULL)
diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index de8d8ce4e9..c4a25f54cd 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -100,7 +100,8 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
-* cryptodev: The asym session handling was modified to use a single buffer.
+* cryptodev: The asym session handling was modified to use a single buffer,
+  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.
diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
index 474d447496..d21db89837 100644
--- a/lib/cryptodev/cryptodev_pmd.h
+++ b/lib/cryptodev/cryptodev_pmd.h
@@ -627,6 +627,19 @@ set_sym_session_private_data(struct rte_cryptodev_sym_session *sess,
 	sess->sess_data[driver_id].data = private_data;
 }
 
+/**
+ * @internal
+ * Cryptodev asymmetric crypto session.
+ */
+__extension__ struct rte_cryptodev_asym_session {
+	uint8_t driver_id;
+	/**< Session driver ID. */
+	uint16_t max_priv_session_sz;
+	/**< Size of private session data used when creating mempool */
+	uint8_t padding[5];
+	uint8_t sess_private_data[0];
+};
+
 static inline void *
 get_asym_session_private_data(struct rte_cryptodev_asym_session *sess)
 {
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index d260f79bbc..a839abe34d 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -1908,7 +1908,7 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mp)
 	return sess;
 }
 
-struct rte_cryptodev_asym_session *
+void *
 rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t dev_id,
 		struct rte_crypto_asym_xform *xforms)
 {
@@ -2015,8 +2015,7 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
 }
 
 int
-rte_cryptodev_asym_session_clear(uint8_t dev_id,
-		struct rte_cryptodev_asym_session *sess)
+rte_cryptodev_asym_session_clear(uint8_t dev_id, void *sess)
 {
 	struct rte_cryptodev *dev;
 
@@ -2062,7 +2061,7 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
 }
 
 int
-rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
+rte_cryptodev_asym_session_free(void *sess)
 {
 	struct rte_mempool *sess_mp;
 
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index 8bd85f1575..af328de896 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -918,16 +918,6 @@ struct rte_cryptodev_sym_session {
 	/**< Driver specific session material, variable size */
 };
 
-/** Cryptodev asymmetric crypto session */
-__extension__ struct rte_cryptodev_asym_session {
-	uint8_t driver_id;
-	/**< Session driver ID. */
-	uint16_t max_priv_session_sz;
-	/**< Size of private session data used when creating mempool */
-	uint8_t padding[5];
-	uint8_t sess_private_data[0];
-};
-
 /**
  * Create a symmetric session mempool.
  *
@@ -1008,7 +998,7 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mempool);
  *  - On failure returns NULL
  */
 __rte_experimental
-struct rte_cryptodev_asym_session *
+void *
 rte_cryptodev_asym_session_create(struct rte_mempool *mempool,
 		uint8_t dev_id, struct rte_crypto_asym_xform *xforms);
 
@@ -1039,7 +1029,7 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess);
  */
 __rte_experimental
 int
-rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess);
+rte_cryptodev_asym_session_free(void *sess);
 
 /**
  * Fill out private data for the device id, based on its device type.
@@ -1092,8 +1082,7 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
  */
 __rte_experimental
 int
-rte_cryptodev_asym_session_clear(uint8_t dev_id,
-			struct rte_cryptodev_asym_session *sess);
+rte_cryptodev_asym_session_clear(uint8_t dev_id, void *sess);
 
 /**
  * Get the size of the header session, for all registered drivers excluding
diff --git a/lib/cryptodev/rte_cryptodev_trace.h b/lib/cryptodev/rte_cryptodev_trace.h
index befbaf7f44..82ef876c72 100644
--- a/lib/cryptodev/rte_cryptodev_trace.h
+++ b/lib/cryptodev/rte_cryptodev_trace.h
@@ -96,7 +96,7 @@ RTE_TRACE_POINT(
 RTE_TRACE_POINT(
 	rte_cryptodev_trace_asym_session_create,
 	RTE_TRACE_POINT_ARGS(void *mempool,
-		struct rte_cryptodev_asym_session *sess),
+		void *sess),
 	rte_trace_point_emit_ptr(mempool);
 	rte_trace_point_emit_ptr(sess);
 )
@@ -109,7 +109,7 @@ RTE_TRACE_POINT(
 
 RTE_TRACE_POINT(
 	rte_cryptodev_trace_asym_session_free,
-	RTE_TRACE_POINT_ARGS(struct rte_cryptodev_asym_session *sess),
+	RTE_TRACE_POINT_ARGS(void *sess),
 	rte_trace_point_emit_ptr(sess);
 )
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 3/4] crypto: add asym session user data API
  2022-02-03 16:04 [PATCH v3 0/4] crypto: improve asym session usage Ciara Power
  2022-02-03 16:04 ` [PATCH v3 1/4] crypto: use single buffer for asymmetric session Ciara Power
  2022-02-03 16:04 ` [PATCH v3 2/4] crypto: hide asym session structure Ciara Power
@ 2022-02-03 16:04 ` Ciara Power
  2022-02-07  8:41   ` [EXT] " Akhil Goyal
  2022-02-03 16:04 ` [PATCH v3 4/4] crypto: modify return value for asym session create Ciara Power
  3 siblings, 1 reply; 12+ messages in thread
From: Ciara Power @ 2022-02-03 16:04 UTC (permalink / raw)
  To: dev; +Cc: roy.fan.zhang, gakhil, anoobj, mdr, Ciara Power, Declan Doherty

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.
+
 * **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.
+ */
+__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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 4/4] crypto: modify return value for asym session create
  2022-02-03 16:04 [PATCH v3 0/4] crypto: improve asym session usage Ciara Power
                   ` (2 preceding siblings ...)
  2022-02-03 16:04 ` [PATCH v3 3/4] crypto: add asym session user data API Ciara Power
@ 2022-02-03 16:04 ` Ciara Power
  2022-02-07  9:04   ` [EXT] " Akhil Goyal
  3 siblings, 1 reply; 12+ messages in thread
From: Ciara Power @ 2022-02-03 16:04 UTC (permalink / raw)
  To: dev; +Cc: roy.fan.zhang, gakhil, anoobj, mdr, Ciara Power, Declan Doherty

Rather than the asym session create function returning a session on
success, and a NULL value on error, it is modified to now return int
values - 0 on success or -EINVAL/-ENOTSUP/-ENOMEM on failure.
The session to be used is passed as input.

This adds clarity on the failure of the create function, which enables
treating the -ENOTSUP return as TEST_SKIPPED in test apps.

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>

---
v3:
  - Fixed variable declarations, putting initialised variable last.
  - Made function comment for return value more generic.
  - Fixed log to include line break.
  - Added documentation.
---
 app/test-crypto-perf/cperf_ops.c        |  12 ++-
 app/test/test_cryptodev_asym.c          | 132 +++++++++++++-----------
 doc/guides/prog_guide/cryptodev_lib.rst |   6 +-
 doc/guides/rel_notes/release_22_03.rst  |   3 +-
 lib/cryptodev/rte_cryptodev.c           |  27 ++---
 lib/cryptodev/rte_cryptodev.h           |  11 +-
 6 files changed, 104 insertions(+), 87 deletions(-)

diff --git a/app/test-crypto-perf/cperf_ops.c b/app/test-crypto-perf/cperf_ops.c
index 948dc0f608..1486298931 100644
--- a/app/test-crypto-perf/cperf_ops.c
+++ b/app/test-crypto-perf/cperf_ops.c
@@ -734,7 +734,9 @@ cperf_create_session(struct rte_mempool *sess_mp,
 	struct rte_crypto_sym_xform auth_xform;
 	struct rte_crypto_sym_xform aead_xform;
 	struct rte_cryptodev_sym_session *sess = NULL;
+	void *asym_sess = NULL;
 	struct rte_crypto_asym_xform xform = {0};
+	int ret;
 
 	if (options->op_type == CPERF_ASYM_MODEX) {
 		xform.next = NULL;
@@ -744,11 +746,13 @@ cperf_create_session(struct rte_mempool *sess_mp,
 		xform.modex.exponent.data = perf_mod_e;
 		xform.modex.exponent.length = sizeof(perf_mod_e);
 
-		sess = (void *)rte_cryptodev_asym_session_create(sess_mp, dev_id, &xform);
-		if (sess == NULL)
+		ret = rte_cryptodev_asym_session_create(&asym_sess,
+				sess_mp, dev_id, &xform);
+		if (ret < 0) {
+			RTE_LOG(ERR, USER1, "Asym session create failed\n");
 			return NULL;
-
-		return sess;
+		}
+		return asym_sess;
 	}
 #ifdef RTE_LIB_SECURITY
 	/*
diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
index a81d6292f6..2edf8b5b42 100644
--- a/app/test/test_cryptodev_asym.c
+++ b/app/test/test_cryptodev_asym.c
@@ -315,7 +315,7 @@ test_cryptodev_asym_op(struct crypto_testsuite_params_asym *ts_params,
 	uint8_t input[TEST_DATA_SIZE] = {0};
 	uint8_t *result = NULL;
 
-	int status = TEST_SUCCESS;
+	int ret, status = TEST_SUCCESS;
 
 	xform_tc.next = NULL;
 	xform_tc.xform_type = data_tc->modex.xform_type;
@@ -450,14 +450,14 @@ test_cryptodev_asym_op(struct crypto_testsuite_params_asym *ts_params,
 	}
 
 	if (!sessionless) {
-		sess = rte_cryptodev_asym_session_create(ts_params->session_mpool,
-				dev_id, &xform_tc);
-		if (!sess) {
+		ret = rte_cryptodev_asym_session_create(&sess,
+				ts_params->session_mpool, dev_id, &xform_tc);
+		if (ret < 0) {
 			snprintf(test_msg, ASYM_TEST_MSG_LEN,
 					"line %u "
 					"FAILED: %s", __LINE__,
 					"Session creation failed");
-			status = TEST_FAILED;
+			status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 			goto error_exit;
 		}
 
@@ -644,9 +644,9 @@ test_rsa_sign_verify(void)
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	uint8_t dev_id = ts_params->valid_devs[0];
-	void *sess;
+	void *sess = NULL;
 	struct rte_cryptodev_info dev_info;
-	int status = TEST_SUCCESS;
+	int ret, status = TEST_SUCCESS;
 
 	/* Test case supports op with exponent key only,
 	 * Check in PMD feature flag for RSA exponent key type support.
@@ -659,12 +659,12 @@ test_rsa_sign_verify(void)
 		return TEST_SKIPPED;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &rsa_xform);
-
-	if (!sess) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &rsa_xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1, "Session creation failed for "
 			"sign_verify\n");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -685,9 +685,9 @@ test_rsa_enc_dec(void)
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	uint8_t dev_id = ts_params->valid_devs[0];
-	void *sess;
+	void *sess = NULL;
 	struct rte_cryptodev_info dev_info;
-	int status = TEST_SUCCESS;
+	int ret, status = TEST_SUCCESS;
 
 	/* Test case supports op with exponent key only,
 	 * Check in PMD feature flag for RSA exponent key type support.
@@ -700,11 +700,11 @@ test_rsa_enc_dec(void)
 		return TEST_SKIPPED;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &rsa_xform);
-
-	if (!sess) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &rsa_xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1, "Session creation failed for enc_dec\n");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -726,9 +726,9 @@ test_rsa_sign_verify_crt(void)
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	uint8_t dev_id = ts_params->valid_devs[0];
-	void *sess;
+	void *sess = NULL;
 	struct rte_cryptodev_info dev_info;
-	int status = TEST_SUCCESS;
+	int ret, status = TEST_SUCCESS;
 
 	/* Test case supports op with quintuple format key only,
 	 * Check im PMD feature flag for RSA quintuple key type support.
@@ -740,12 +740,12 @@ test_rsa_sign_verify_crt(void)
 		return TEST_SKIPPED;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &rsa_xform_crt);
-
-	if (!sess) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &rsa_xform_crt);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1, "Session creation failed for "
 			"sign_verify_crt\n");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -767,9 +767,9 @@ test_rsa_enc_dec_crt(void)
 	struct crypto_testsuite_params_asym *ts_params = &testsuite_params;
 	struct rte_mempool *sess_mpool = ts_params->session_mpool;
 	uint8_t dev_id = ts_params->valid_devs[0];
-	void *sess;
+	void *sess = NULL;
 	struct rte_cryptodev_info dev_info;
-	int status = TEST_SUCCESS;
+	int ret, status = TEST_SUCCESS;
 
 	/* Test case supports op with quintuple format key only,
 	 * Check in PMD feature flag for RSA quintuple key type support.
@@ -781,12 +781,12 @@ test_rsa_enc_dec_crt(void)
 		return TEST_SKIPPED;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &rsa_xform_crt);
-
-	if (!sess) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &rsa_xform_crt);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1, "Session creation failed for "
 			"enc_dec_crt\n");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -1050,7 +1050,7 @@ test_dh_gen_shared_sec(struct rte_crypto_asym_xform *xfrm)
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
 	void *sess = NULL;
-	int status = TEST_SUCCESS;
+	int ret, status = TEST_SUCCESS;
 	uint8_t output[TEST_DH_MOD_LEN];
 	struct rte_crypto_asym_xform xform = *xfrm;
 	uint8_t peer[] = "01234567890123456789012345678901234567890123456789";
@@ -1077,12 +1077,13 @@ test_dh_gen_shared_sec(struct rte_crypto_asym_xform *xfrm)
 	asym_op->dh.shared_secret.data = output;
 	asym_op->dh.shared_secret.length = sizeof(output);
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
-	if (sess == NULL) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1,
 				"line %u FAILED: %s", __LINE__,
 				"Session creation failed");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -1135,7 +1136,7 @@ test_dh_gen_priv_key(struct rte_crypto_asym_xform *xfrm)
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
 	void *sess = NULL;
-	int status = TEST_SUCCESS;
+	int ret, status = TEST_SUCCESS;
 	uint8_t output[TEST_DH_MOD_LEN];
 	struct rte_crypto_asym_xform xform = *xfrm;
 
@@ -1157,12 +1158,13 @@ test_dh_gen_priv_key(struct rte_crypto_asym_xform *xfrm)
 	asym_op->dh.priv_key.data = output;
 	asym_op->dh.priv_key.length = sizeof(output);
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
-	if (sess == NULL) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1,
 				"line %u FAILED: %s", __LINE__,
 				"Session creation failed");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -1218,7 +1220,7 @@ test_dh_gen_pub_key(struct rte_crypto_asym_xform *xfrm)
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
 	void *sess = NULL;
-	int status = TEST_SUCCESS;
+	int ret, status = TEST_SUCCESS;
 	uint8_t output[TEST_DH_MOD_LEN];
 	struct rte_crypto_asym_xform xform = *xfrm;
 
@@ -1248,12 +1250,13 @@ test_dh_gen_pub_key(struct rte_crypto_asym_xform *xfrm)
 					0);
 	asym_op->dh.priv_key = dh_test_params.priv_key;
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
-	if (sess == NULL) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1,
 				"line %u FAILED: %s", __LINE__,
 				"Session creation failed");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -1309,7 +1312,7 @@ test_dh_gen_kp(struct rte_crypto_asym_xform *xfrm)
 	struct rte_crypto_asym_op *asym_op = NULL;
 	struct rte_crypto_op *op = NULL, *result_op = NULL;
 	void *sess = NULL;
-	int status = TEST_SUCCESS;
+	int ret, status = TEST_SUCCESS;
 	uint8_t out_pub_key[TEST_DH_MOD_LEN];
 	uint8_t out_prv_key[TEST_DH_MOD_LEN];
 	struct rte_crypto_asym_xform pub_key_xform;
@@ -1339,12 +1342,13 @@ test_dh_gen_kp(struct rte_crypto_asym_xform *xfrm)
 	asym_op->dh.priv_key.data = out_prv_key;
 	asym_op->dh.priv_key.length = sizeof(out_prv_key);
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
-	if (sess == NULL) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1,
 				"line %u FAILED: %s", __LINE__,
 				"Session creation failed");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -1430,12 +1434,13 @@ test_mod_inv(void)
 				return TEST_SKIPPED;
 		}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &modinv_xform);
-	if (!sess) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &modinv_xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1, "line %u "
 				"FAILED: %s", __LINE__,
 				"Session creation failed");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -1556,13 +1561,14 @@ test_mod_exp(void)
 		goto error_exit;
 	}
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &modex_xform);
-	if (!sess) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &modex_xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1,
 				 "line %u "
 				"FAILED: %s", __LINE__,
 				"Session creation failed");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 
@@ -1668,13 +1674,14 @@ test_dsa_sign(void)
 	uint8_t r[TEST_DH_MOD_LEN];
 	uint8_t s[TEST_DH_MOD_LEN];
 	uint8_t dgst[] = "35d81554afaad2cf18f3a1770d5fedc4ea5be344";
+	int ret;
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &dsa_xform);
-	if (sess == NULL) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool, dev_id, &dsa_xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1,
 				 "line %u FAILED: %s", __LINE__,
 				"Session creation failed");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto error_exit;
 	}
 	/* set up crypto op data structure */
@@ -1805,7 +1812,7 @@ test_ecdsa_sign_verify(enum curve curve_id)
 	struct rte_crypto_asym_op *asym_op;
 	struct rte_cryptodev_info dev_info;
 	struct rte_crypto_op *op = NULL;
-	int status = TEST_SUCCESS, ret;
+	int ret, status = TEST_SUCCESS;
 
 	switch (curve_id) {
 	case SECP192R1:
@@ -1850,12 +1857,13 @@ test_ecdsa_sign_verify(enum curve curve_id)
 	xform.xform_type = RTE_CRYPTO_ASYM_XFORM_ECDSA;
 	xform.ec.curve_id = input_params.curve;
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
-	if (sess == NULL) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool,
+			dev_id, &xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1,
 				"line %u FAILED: %s", __LINE__,
 				"Session creation failed\n");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto exit;
 	}
 
@@ -2009,7 +2017,7 @@ test_ecpm(enum curve curve_id)
 	struct rte_crypto_asym_op *asym_op;
 	struct rte_cryptodev_info dev_info;
 	struct rte_crypto_op *op = NULL;
-	int status = TEST_SUCCESS, ret;
+	int ret, status = TEST_SUCCESS;
 
 	switch (curve_id) {
 	case SECP192R1:
@@ -2054,12 +2062,12 @@ test_ecpm(enum curve curve_id)
 	xform.xform_type = RTE_CRYPTO_ASYM_XFORM_ECPM;
 	xform.ec.curve_id = input_params.curve;
 
-	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id, &xform);
-	if (sess == NULL) {
+	ret = rte_cryptodev_asym_session_create(&sess, sess_mpool, dev_id, &xform);
+	if (ret < 0) {
 		RTE_LOG(ERR, USER1,
 				"line %u FAILED: %s", __LINE__,
 				"Session creation failed\n");
-		status = TEST_FAILED;
+		status = (ret == -ENOTSUP) ? TEST_SKIPPED : TEST_FAILED;
 		goto exit;
 	}
 
diff --git a/doc/guides/prog_guide/cryptodev_lib.rst b/doc/guides/prog_guide/cryptodev_lib.rst
index 62bd3577f5..8e16461dc6 100644
--- a/doc/guides/prog_guide/cryptodev_lib.rst
+++ b/doc/guides/prog_guide/cryptodev_lib.rst
@@ -1236,10 +1236,10 @@ crypto operations is similar except change to respective op and xform setup).
      * Create asym crypto session and initialize it for the crypto device.
      * The session structure is hidden from the app, so void * is used.
      */
-    void *asym_session;
-    asym_session = rte_cryptodev_asym_session_create(asym_session_pool,
+    void *asym_session = NULL;
+    ret = rte_cryptodev_asym_session_create(&asym_session, asym_session_pool,
             cdev_id, &modex_xform);
-    if (asym_session == NULL)
+    if (ret < 0)
         rte_exit(EXIT_FAILURE, "Session could not be created\n");
 
     /* Get a burst of crypto operations. */
diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 1022f77828..195a7efdd5 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -110,7 +110,8 @@ API Changes
   create a mempool with element size to hold the generic asym session header,
   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``.
+  now done by ``rte_cryptodev_asym_session_create``, which was updated to
+  return an integer value to indicate initialisation errors.
 
 
 ABI Changes
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index 0d816ed4a9..005f0e7952 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -1912,9 +1912,9 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mp)
 	return sess;
 }
 
-void *
-rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t dev_id,
-		struct rte_crypto_asym_xform *xforms)
+int
+rte_cryptodev_asym_session_create(void **session, struct rte_mempool *mp,
+		uint8_t dev_id, struct rte_crypto_asym_xform *xforms)
 {
 	struct rte_cryptodev_asym_session *sess;
 	uint32_t session_priv_data_sz;
@@ -1926,18 +1926,18 @@ rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t dev_id,
 
 	if (!rte_cryptodev_is_valid_dev(dev_id)) {
 		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
-		return NULL;
+		return -EINVAL;
 	}
 	session_priv_data_sz = rte_cryptodev_asym_get_private_session_size(
 			dev_id);
 	dev = rte_cryptodev_pmd_get_dev(dev_id);
 
 	if (dev == NULL)
-		return NULL;
+		return -EINVAL;
 
 	if (!mp) {
 		CDEV_LOG_ERR("invalid mempool\n");
-		return NULL;
+		return -EINVAL;
 	}
 
 	pool_priv = rte_mempool_get_priv(mp);
@@ -1945,22 +1945,23 @@ rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t dev_id,
 	if (pool_priv->max_priv_session_sz < session_priv_data_sz) {
 		CDEV_LOG_DEBUG(
 			"The private session data size used when creating the mempool is smaller than this device's private session data.");
-		return NULL;
+		return -EINVAL;
 	}
 
 	/* Verify if provided mempool can hold elements big enough. */
 	if (mp->elt_size < session_header_size + session_priv_data_sz) {
 		CDEV_LOG_ERR(
 			"mempool elements too small to hold session objects");
-		return NULL;
+		return -EINVAL;
 	}
 
 	/* Allocate a session structure from the session pool */
-	if (rte_mempool_get(mp, (void **)&sess)) {
+	if (rte_mempool_get(mp, session)) {
 		CDEV_LOG_ERR("couldn't get object from session mempool");
-		return NULL;
+		return -ENOMEM;
 	}
 
+	sess = *session;
 	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;
@@ -1970,7 +1971,7 @@ rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t dev_id,
 	 */
 	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);
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->asym_session_configure, -ENOTSUP);
 
 	if (sess->sess_private_data[0] == 0) {
 		ret = dev->dev_ops->asym_session_configure(dev,
@@ -1980,12 +1981,12 @@ rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t dev_id,
 			CDEV_LOG_ERR(
 				"dev_id %d failed to configure session details",
 				dev_id);
-			return NULL;
+			return ret;
 		}
 	}
 
 	rte_cryptodev_trace_asym_session_create(mp, sess);
-	return sess;
+	return 0;
 }
 
 int
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index 6a4d6d9934..9a75936963 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -990,18 +990,21 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mempool);
 /**
  * Create asymmetric crypto session header (generic with no private data)
  *
+ * @param   session    void ** for session to be used
  * @param   mempool    mempool to allocate asymmetric session
  *                     objects from
  * @param   dev_id   ID of device that we want the session to be used on
  * @param   xforms   Asymmetric crypto transform operations to apply on flow
  *                   processed with this session
  * @return
- *  - On success return pointer to asym-session
- *  - On failure returns NULL
+ *  - 0 on success.
+ *  - -EINVAL on invalid arguments.
+ *  - -ENOMEM on memory error for session allocation.
+ *  - -ENOTSUP if device doesn't support session configuration.
  */
 __rte_experimental
-void *
-rte_cryptodev_asym_session_create(struct rte_mempool *mempool,
+int
+rte_cryptodev_asym_session_create(void **session, struct rte_mempool *mempool,
 		uint8_t dev_id, struct rte_crypto_asym_xform *xforms);
 
 /**
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [EXT] [PATCH v3 1/4] crypto: use single buffer for asymmetric session
  2022-02-03 16:04 ` [PATCH v3 1/4] crypto: use single buffer for asymmetric session Ciara Power
@ 2022-02-07  8:19   ` Akhil Goyal
  2022-02-07 14:22     ` Power, Ciara
  0 siblings, 1 reply; 12+ messages in thread
From: Akhil Goyal @ 2022-02-07  8:19 UTC (permalink / raw)
  To: Ciara Power, dev
  Cc: roy.fan.zhang, Anoob Joseph, mdr, Declan Doherty, Ankur Dwivedi,
	Tejasree Kondoj, John Griffin, Fiona Trahe, Deepak Kumar Jain

> Rather than using a session buffer that contains pointers to private
> session data elsewhere, have a single session buffer.
> This session is created for a driver ID, and the mempool element
> contains space for the max session private data needed for any driver.

This means asymmetric ops are not allowed with scheduler PMD.

Also since session_create() and session_init() are merged to a single function,
Why not merge session_clear() and session_free()?

> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
> 
> ---
> v2:
>   - Renamed function typedef from "free" to "clear" as session private
>     data isn't being freed in that function.
>   - Moved user data API to separate patch.
>   - Minor fixes to comments, formatting, return values.
> v3:
>   - Corrected formatting of struct comments.
>   - Increased size of max_priv_session_sz to uint16_t.
>   - Removed trace for asym session init function that was
>     previously removed.
>   - Added documentation.
> ---
>  app/test-crypto-perf/cperf_ops.c             |  14 +-
>  app/test/test_cryptodev_asym.c               | 200 ++++---------------
>  doc/guides/prog_guide/cryptodev_lib.rst      |  55 ++---
>  doc/guides/rel_notes/release_22_03.rst       |   7 +
>  drivers/crypto/cnxk/cn10k_cryptodev_ops.c    |   6 +-
>  drivers/crypto/cnxk/cn9k_cryptodev_ops.c     |   6 +-
>  drivers/crypto/cnxk/cnxk_cryptodev_ops.c     |  11 +-
>  drivers/crypto/octeontx/otx_cryptodev_ops.c  |  29 +--
>  drivers/crypto/openssl/rte_openssl_pmd.c     |   5 +-
>  drivers/crypto/openssl/rte_openssl_pmd_ops.c |  23 +--
>  drivers/crypto/qat/qat_asym.c                |  53 ++---
>  lib/cryptodev/cryptodev_pmd.h                |  17 +-
>  lib/cryptodev/cryptodev_trace_points.c       |   6 +-
>  lib/cryptodev/rte_cryptodev.c                | 167 ++++++++++------
>  lib/cryptodev/rte_cryptodev.h                |  72 ++++---
>  lib/cryptodev/rte_cryptodev_trace.h          |  21 +-
>  lib/cryptodev/version.map                    |   5 +-
>  17 files changed, 258 insertions(+), 439 deletions(-)
> 
> diff --git a/app/test-crypto-perf/cperf_ops.c b/app/test-crypto-perf/cperf_ops.c
> index d975ae1ab8..bdc5dc9544 100644
> --- a/app/test-crypto-perf/cperf_ops.c
> +++ b/app/test-crypto-perf/cperf_ops.c
> @@ -735,7 +735,6 @@ cperf_create_session(struct rte_mempool *sess_mp,
>  	struct rte_crypto_sym_xform aead_xform;
>  	struct rte_cryptodev_sym_session *sess = NULL;
>  	struct rte_crypto_asym_xform xform = {0};
> -	int rc;
> 
>  	if (options->op_type == CPERF_ASYM_MODEX) {
>  		xform.next = NULL;
> @@ -745,19 +744,10 @@ cperf_create_session(struct rte_mempool *sess_mp,
>  		xform.modex.exponent.data = perf_mod_e;
>  		xform.modex.exponent.length = sizeof(perf_mod_e);
> 
> -		sess = (void *)rte_cryptodev_asym_session_create(sess_mp);
> +		sess = (void *)rte_cryptodev_asym_session_create(sess_mp,
> dev_id, &xform);
>  		if (sess == NULL)
>  			return NULL;
> -		rc = rte_cryptodev_asym_session_init(dev_id, (void *)sess,
> -						     &xform, priv_mp);
> -		if (rc < 0) {
> -			if (sess != NULL) {
> -				rte_cryptodev_asym_session_clear(dev_id,
> -								 (void *)sess);
> -				rte_cryptodev_asym_session_free((void
> *)sess);
> -			}
> -			return NULL;
> -		}
> +
>  		return sess;
>  	}
>  #ifdef RTE_LIB_SECURITY
> diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
> index 68f4d8e7a6..f7c2fd2588 100644
> --- a/app/test/test_cryptodev_asym.c
> +++ b/app/test/test_cryptodev_asym.c
> @@ -450,7 +450,8 @@ test_cryptodev_asym_op(struct
> crypto_testsuite_params_asym *ts_params,
>  	}
> 
>  	if (!sessionless) {
> -		sess = rte_cryptodev_asym_session_create(ts_params-
> >session_mpool);
> +		sess = rte_cryptodev_asym_session_create(ts_params-
> >session_mpool,
> +				dev_id, &xform_tc);
>  		if (!sess) {
>  			snprintf(test_msg, ASYM_TEST_MSG_LEN,
>  					"line %u "
> @@ -460,15 +461,6 @@ test_cryptodev_asym_op(struct
> crypto_testsuite_params_asym *ts_params,
>  			goto error_exit;
>  		}
> 
> -		if (rte_cryptodev_asym_session_init(dev_id, sess, &xform_tc,
> -				ts_params->session_mpool) < 0) {
> -			snprintf(test_msg, ASYM_TEST_MSG_LEN,
> -					"line %u FAILED: %s",
> -					__LINE__, "unabled to config sym
> session");
> -			status = TEST_FAILED;
> -			goto error_exit;
> -		}
> -
>  		rte_crypto_op_attach_asym_session(op, sess);
>  	} else {
>  		asym_op->xform = &xform_tc;
> @@ -667,18 +659,11 @@ test_rsa_sign_verify(void)
>  		return TEST_SKIPPED;
>  	}
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &rsa_xform);
> 
>  	if (!sess) {
>  		RTE_LOG(ERR, USER1, "Session creation failed for "
>  			"sign_verify\n");
> -		return TEST_FAILED;
> -	}
> -
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &rsa_xform,
> -				sess_mpool) < 0) {
> -		RTE_LOG(ERR, USER1, "Unable to config asym session for "
> -			"sign_verify\n");
>  		status = TEST_FAILED;
>  		goto error_exit;
>  	}
> @@ -686,7 +671,6 @@ test_rsa_sign_verify(void)
>  	status = queue_ops_rsa_sign_verify(sess);
> 
>  error_exit:
> -
>  	rte_cryptodev_asym_session_clear(dev_id, sess);
>  	rte_cryptodev_asym_session_free(sess);
> 
> @@ -716,17 +700,10 @@ test_rsa_enc_dec(void)
>  		return TEST_SKIPPED;
>  	}
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &rsa_xform);
> 
>  	if (!sess) {
>  		RTE_LOG(ERR, USER1, "Session creation failed for enc_dec\n");
> -		return TEST_FAILED;
> -	}
> -
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &rsa_xform,
> -				sess_mpool) < 0) {
> -		RTE_LOG(ERR, USER1, "Unable to config asym session for "
> -			"enc_dec\n");
>  		status = TEST_FAILED;
>  		goto error_exit;
>  	}
> @@ -763,22 +740,15 @@ test_rsa_sign_verify_crt(void)
>  		return TEST_SKIPPED;
>  	}
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &rsa_xform_crt);
> 
>  	if (!sess) {
>  		RTE_LOG(ERR, USER1, "Session creation failed for "
>  			"sign_verify_crt\n");
>  		status = TEST_FAILED;
> -		return status;
> -	}
> -
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &rsa_xform_crt,
> -				sess_mpool) < 0) {
> -		RTE_LOG(ERR, USER1, "Unable to config asym session for "
> -			"sign_verify_crt\n");
> -		status = TEST_FAILED;
>  		goto error_exit;
>  	}
> +
>  	status = queue_ops_rsa_sign_verify(sess);
> 
>  error_exit:
> @@ -811,21 +781,15 @@ test_rsa_enc_dec_crt(void)
>  		return TEST_SKIPPED;
>  	}
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &rsa_xform_crt);
> 
>  	if (!sess) {
>  		RTE_LOG(ERR, USER1, "Session creation failed for "
>  			"enc_dec_crt\n");
> -		return TEST_FAILED;
> -	}
> -
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &rsa_xform_crt,
> -				sess_mpool) < 0) {
> -		RTE_LOG(ERR, USER1, "Unable to config asym session for "
> -			"enc_dec_crt\n");
>  		status = TEST_FAILED;
>  		goto error_exit;
>  	}
> +
>  	status = queue_ops_rsa_enc_dec(sess);
> 
>  error_exit:
> @@ -924,7 +888,6 @@ testsuite_setup(void)
>  	/* configure qp */
>  	ts_params->qp_conf.nb_descriptors = DEFAULT_NUM_OPS_INFLIGHT;
>  	ts_params->qp_conf.mp_session = ts_params->session_mpool;
> -	ts_params->qp_conf.mp_session_private = ts_params->session_mpool;
>  	for (qp_id = 0; qp_id < info.max_nb_queue_pairs; qp_id++) {
>  		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
>  			dev_id, qp_id, &ts_params->qp_conf,
> @@ -933,21 +896,9 @@ testsuite_setup(void)
>  			qp_id, dev_id);
>  	}
> 
> -	/* setup asym session pool */
> -	unsigned int session_size = RTE_MAX(
> -		rte_cryptodev_asym_get_private_session_size(dev_id),
> -		rte_cryptodev_asym_get_header_session_size());
> -	/*
> -	 * Create mempool with TEST_NUM_SESSIONS * 2,
> -	 * to include the session headers
> -	 */
> -	ts_params->session_mpool = rte_mempool_create(
> -				"test_asym_sess_mp",
> -				TEST_NUM_SESSIONS * 2,
> -				session_size,
> -				0, 0, NULL, NULL, NULL,
> -				NULL, SOCKET_ID_ANY,
> -				0);
> +	ts_params->session_mpool =
> rte_cryptodev_asym_session_pool_create(
> +			"test_asym_sess_mp", TEST_NUM_SESSIONS * 2, 0,
> +			SOCKET_ID_ANY);
> 
>  	TEST_ASSERT_NOT_NULL(ts_params->session_mpool,
>  			"session mempool allocation failed");
> @@ -1104,14 +1055,6 @@ test_dh_gen_shared_sec(struct
> rte_crypto_asym_xform *xfrm)
>  	struct rte_crypto_asym_xform xform = *xfrm;
>  	uint8_t peer[] =
> "01234567890123456789012345678901234567890123456789";
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> -	if (sess == NULL) {
> -		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s", __LINE__,
> -				"Session creation failed");
> -		status = TEST_FAILED;
> -		goto error_exit;
> -	}
>  	/* set up crypto op data structure */
>  	op = rte_crypto_op_alloc(op_mpool,
> RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
>  	if (!op) {
> @@ -1134,11 +1077,11 @@ test_dh_gen_shared_sec(struct
> rte_crypto_asym_xform *xfrm)
>  	asym_op->dh.shared_secret.data = output;
>  	asym_op->dh.shared_secret.length = sizeof(output);
> 
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
> -			sess_mpool) < 0) {
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &xform);
> +	if (sess == NULL) {
>  		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s",
> -				__LINE__, "unabled to config sym session");
> +				"line %u FAILED: %s", __LINE__,
> +				"Session creation failed");
>  		status = TEST_FAILED;
>  		goto error_exit;
>  	}
> @@ -1196,14 +1139,6 @@ test_dh_gen_priv_key(struct
> rte_crypto_asym_xform *xfrm)
>  	uint8_t output[TEST_DH_MOD_LEN];
>  	struct rte_crypto_asym_xform xform = *xfrm;
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> -	if (sess == NULL) {
> -		RTE_LOG(ERR, USER1,
> -				 "line %u FAILED: %s", __LINE__,
> -				"Session creation failed");
> -		status = TEST_FAILED;
> -		goto error_exit;
> -	}
>  	/* set up crypto op data structure */
>  	op = rte_crypto_op_alloc(op_mpool,
> RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
>  	if (!op) {
> @@ -1222,11 +1157,11 @@ test_dh_gen_priv_key(struct
> rte_crypto_asym_xform *xfrm)
>  	asym_op->dh.priv_key.data = output;
>  	asym_op->dh.priv_key.length = sizeof(output);
> 
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
> -			sess_mpool) < 0) {
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &xform);
> +	if (sess == NULL) {
>  		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s",
> -				__LINE__, "unabled to config sym session");
> +				"line %u FAILED: %s", __LINE__,
> +				"Session creation failed");
>  		status = TEST_FAILED;
>  		goto error_exit;
>  	}
> @@ -1287,14 +1222,6 @@ test_dh_gen_pub_key(struct
> rte_crypto_asym_xform *xfrm)
>  	uint8_t output[TEST_DH_MOD_LEN];
>  	struct rte_crypto_asym_xform xform = *xfrm;
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> -	if (sess == NULL) {
> -		RTE_LOG(ERR, USER1,
> -				 "line %u FAILED: %s", __LINE__,
> -				"Session creation failed");
> -		status = TEST_FAILED;
> -		goto error_exit;
> -	}
>  	/* set up crypto op data structure */
>  	op = rte_crypto_op_alloc(op_mpool,
> RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
>  	if (!op) {
> @@ -1321,11 +1248,11 @@ test_dh_gen_pub_key(struct
> rte_crypto_asym_xform *xfrm)
>  					0);
>  	asym_op->dh.priv_key = dh_test_params.priv_key;
> 
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
> -			sess_mpool) < 0) {
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &xform);
> +	if (sess == NULL) {
>  		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s",
> -				__LINE__, "unabled to config sym session");
> +				"line %u FAILED: %s", __LINE__,
> +				"Session creation failed");
>  		status = TEST_FAILED;
>  		goto error_exit;
>  	}
> @@ -1388,15 +1315,6 @@ test_dh_gen_kp(struct rte_crypto_asym_xform
> *xfrm)
>  	struct rte_crypto_asym_xform pub_key_xform;
>  	struct rte_crypto_asym_xform xform = *xfrm;
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> -	if (sess == NULL) {
> -		RTE_LOG(ERR, USER1,
> -				 "line %u FAILED: %s", __LINE__,
> -				"Session creation failed");
> -		status = TEST_FAILED;
> -		goto error_exit;
> -	}
> -
>  	/* set up crypto op data structure */
>  	op = rte_crypto_op_alloc(op_mpool,
> RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
>  	if (!op) {
> @@ -1420,11 +1338,12 @@ test_dh_gen_kp(struct rte_crypto_asym_xform
> *xfrm)
>  	asym_op->dh.pub_key.length = sizeof(out_pub_key);
>  	asym_op->dh.priv_key.data = out_prv_key;
>  	asym_op->dh.priv_key.length = sizeof(out_prv_key);
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
> -			sess_mpool) < 0) {
> +
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &xform);
> +	if (sess == NULL) {
>  		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s",
> -				__LINE__, "unabled to config sym session");
> +				"line %u FAILED: %s", __LINE__,
> +				"Session creation failed");
>  		status = TEST_FAILED;
>  		goto error_exit;
>  	}
> @@ -1511,7 +1430,7 @@ test_mod_inv(void)
>  				return TEST_SKIPPED;
>  		}
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &modinv_xform);
>  	if (!sess) {
>  		RTE_LOG(ERR, USER1, "line %u "
>  				"FAILED: %s", __LINE__,
> @@ -1520,15 +1439,6 @@ test_mod_inv(void)
>  		goto error_exit;
>  	}
> 
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &modinv_xform,
> -			sess_mpool) < 0) {
> -		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s",
> -				__LINE__, "unabled to config sym session");
> -		status = TEST_FAILED;
> -		goto error_exit;
> -	}
> -
>  	/* generate crypto op data structure */
>  	op = rte_crypto_op_alloc(op_mpool,
> RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
>  	if (!op) {
> @@ -1646,7 +1556,7 @@ test_mod_exp(void)
>  		goto error_exit;
>  	}
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &modex_xform);
>  	if (!sess) {
>  		RTE_LOG(ERR, USER1,
>  				 "line %u "
> @@ -1656,15 +1566,6 @@ test_mod_exp(void)
>  		goto error_exit;
>  	}
> 
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &modex_xform,
> -			sess_mpool) < 0) {
> -		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s",
> -				__LINE__, "unabled to config sym session");
> -		status = TEST_FAILED;
> -		goto error_exit;
> -	}
> -
>  	asym_op = op->asym;
>  	memcpy(input, base, sizeof(base));
>  	asym_op->modex.base.data = input;
> @@ -1768,7 +1669,7 @@ test_dsa_sign(void)
>  	uint8_t s[TEST_DH_MOD_LEN];
>  	uint8_t dgst[] = "35d81554afaad2cf18f3a1770d5fedc4ea5be344";
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &dsa_xform);
>  	if (sess == NULL) {
>  		RTE_LOG(ERR, USER1,
>  				 "line %u FAILED: %s", __LINE__,
> @@ -1797,15 +1698,6 @@ test_dsa_sign(void)
>  	debug_hexdump(stdout, "priv_key: ", dsa_xform.dsa.x.data,
>  			dsa_xform.dsa.x.length);
> 
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &dsa_xform,
> -				sess_mpool) < 0) {
> -		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s",
> -				__LINE__, "unabled to config sym session");
> -		status = TEST_FAILED;
> -		goto error_exit;
> -	}
> -
>  	/* attach asymmetric crypto session to crypto operations */
>  	rte_crypto_op_attach_asym_session(op, sess);
>  	asym_op->dsa.op_type = RTE_CRYPTO_ASYM_OP_SIGN;
> @@ -1941,15 +1833,6 @@ test_ecdsa_sign_verify(enum curve curve_id)
> 
>  	rte_cryptodev_info_get(dev_id, &dev_info);
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> -	if (sess == NULL) {
> -		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s", __LINE__,
> -				"Session creation failed\n");
> -		status = TEST_FAILED;
> -		goto exit;
> -	}
> -
>  	/* Setup crypto op data structure */
>  	op = rte_crypto_op_alloc(op_mpool,
> RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
>  	if (op == NULL) {
> @@ -1967,11 +1850,11 @@ test_ecdsa_sign_verify(enum curve curve_id)
>  	xform.xform_type = RTE_CRYPTO_ASYM_XFORM_ECDSA;
>  	xform.ec.curve_id = input_params.curve;
> 
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
> -				sess_mpool) < 0) {
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &xform);
> +	if (sess == NULL) {
>  		RTE_LOG(ERR, USER1,
>  				"line %u FAILED: %s", __LINE__,
> -				"Unable to config asym session\n");
> +				"Session creation failed\n");
>  		status = TEST_FAILED;
>  		goto exit;
>  	}
> @@ -2154,15 +2037,6 @@ test_ecpm(enum curve curve_id)
> 
>  	rte_cryptodev_info_get(dev_id, &dev_info);
> 
> -	sess = rte_cryptodev_asym_session_create(sess_mpool);
> -	if (sess == NULL) {
> -		RTE_LOG(ERR, USER1,
> -				"line %u FAILED: %s", __LINE__,
> -				"Session creation failed\n");
> -		status = TEST_FAILED;
> -		goto exit;
> -	}
> -
>  	/* Setup crypto op data structure */
>  	op = rte_crypto_op_alloc(op_mpool,
> RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
>  	if (op == NULL) {
> @@ -2180,11 +2054,11 @@ test_ecpm(enum curve curve_id)
>  	xform.xform_type = RTE_CRYPTO_ASYM_XFORM_ECPM;
>  	xform.ec.curve_id = input_params.curve;
> 
> -	if (rte_cryptodev_asym_session_init(dev_id, sess, &xform,
> -				sess_mpool) < 0) {
> +	sess = rte_cryptodev_asym_session_create(sess_mpool, dev_id,
> &xform);
> +	if (sess == NULL) {
>  		RTE_LOG(ERR, USER1,
>  				"line %u FAILED: %s", __LINE__,
> -				"Unable to config asym session\n");
> +				"Session creation failed\n");
>  		status = TEST_FAILED;
>  		goto exit;
>  	}
> diff --git a/doc/guides/prog_guide/cryptodev_lib.rst
> b/doc/guides/prog_guide/cryptodev_lib.rst
> index 8766bc34a9..f8f8562f4c 100644
> --- a/doc/guides/prog_guide/cryptodev_lib.rst
> +++ b/doc/guides/prog_guide/cryptodev_lib.rst
> @@ -1038,20 +1038,17 @@ It is the application's responsibility to create and
> manage the session mempools
>  Application using both symmetric and asymmetric sessions should allocate and
> maintain
>  different sessions pools for each type.
> 
> -An application can use ``rte_cryptodev_get_asym_session_private_size()`` to
> -get the private size of asymmetric session on a given crypto device. This
> -function would allow an application to calculate the max device asymmetric
> -session size of all crypto devices to create a single session mempool.
> -If instead an application creates multiple asymmetric session mempools,
> -the Crypto device framework also provides
> ``rte_cryptodev_asym_get_header_session_size()`` to get
> -the size of an uninitialized session.
> +An application can use ``rte_cryptodev_asym_session_pool_create()`` to create
> a mempool
> +with a specified number of elements. The element size will allow for the
> session header,
> +and the max private session size.
> +The max private session size is chosen based on available crypto devices,
> +the biggest private session size is used. This means any of those devices can be
> used,
> +and the mempool element will have available space for its private session data.
> 
>  Once the session mempools have been created,
> ``rte_cryptodev_asym_session_create()``
> -is used to allocate an uninitialized asymmetric session from the given mempool.
> -The session then must be initialized using ``rte_cryptodev_asym_session_init()``
> -for each of the required crypto devices. An asymmetric transform chain
> -is used to specify the operation and its parameters. See the section below for
> -details on transforms.
> +is used to allocate and initialize an asymmetric session from the given
> mempool.
> +An asymmetric transform chain is used to specify the operation and its
> parameters.
> +See the section below for details on transforms.
> 
>  When a session is no longer used, user must call
> ``rte_cryptodev_asym_session_clear()``
>  for each of the crypto devices that are using the session, to free all driver
> @@ -1162,21 +1159,14 @@ crypto operations is similar except change to
> respective op and xform setup).
> 
>      uint8_t cdev_id = rte_cryptodev_get_dev_id(crypto_name);
> 
> -    /* Get private asym session data size. */
> -    asym_session_size = rte_cryptodev_get_asym_private_session_size(cdev_id);
> -
>      /*
> -     * Create session mempool, with two objects per session,
> -     * one for the session header and another one for the
> -     * private asym session data for the crypto device.
> +     * Create session mempool, this will create elements big enough
> +     * to hold the generic session header,
> +     * and the max private session size of the available devices.
>       */
> -    asym_session_pool = rte_mempool_create("asym_session_pool",
> -                                    MAX_ASYM_SESSIONS * 2,
> -                                    asym_session_size,
> -                                    0,
> -                                    0, NULL, NULL, NULL,
> -                                    NULL, socket_id,
> -                                    0);
> +    asym_session_pool = rte_cryptodev_asym_session_pool_create(
> +                        "asym_session_pool", MAX_ASYM_SESSIONS, 0, 0,
> +                        socket_id);
> 
>      /* Configure the crypto device. */
>      struct rte_cryptodev_config conf = {
> @@ -1190,8 +1180,7 @@ crypto operations is similar except change to
> respective op and xform setup).
>      if (rte_cryptodev_configure(cdev_id, &conf) < 0)
>          rte_exit(EXIT_FAILURE, "Failed to configure cryptodev %u", cdev_id);
> 
> -    if (rte_cryptodev_queue_pair_setup(cdev_id, 0, &qp_conf,
> -                            socket_id, asym_session_pool) < 0)
> +    if (rte_cryptodev_queue_pair_setup(cdev_id, 0, &qp_conf, socket_id) < 0)
>          rte_exit(EXIT_FAILURE, "Failed to setup queue pair\n");
> 
>      if (rte_cryptodev_start(cdev_id) < 0)
> @@ -1226,15 +1215,11 @@ crypto operations is similar except change to
> respective op and xform setup).
>      };
>      /* Create asym crypto session and initialize it for the crypto device. */
>      struct rte_cryptodev_asym_session *asym_session;
> -    asym_session = rte_cryptodev_asym_session_create(asym_session_pool);
> +    asym_session = rte_cryptodev_asym_session_create(asym_session_pool,
> +            cdev_id, &modex_xform);
>      if (asym_session == NULL)
>          rte_exit(EXIT_FAILURE, "Session could not be created\n");
> 
> -    if (rte_cryptodev_asym_session_init(cdev_id, asym_session,
> -                    &modex_xform, asym_session_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[1];
>      if (rte_crypto_op_bulk_alloc(crypto_op_pool,
> @@ -1245,11 +1230,11 @@ crypto operations is similar except change to
> respective op and xform setup).
>      /* Set up the crypto operations. */
>      struct rte_crypto_asym_op *asym_op = crypto_ops[0]->asym;
> 
> -	/* calculate mod exp of value 0xf8 */
> +    /* calculate mod exp of value 0xf8 */
>      static unsigned char base[] = {0xF8};
>      asym_op->modex.base.data = base;
>      asym_op->modex.base.length = sizeof(base);
> -	asym_op->modex.base.iova = base;
> +    asym_op->modex.base.iova = base;
> 
>      /* Attach the asym crypto session to the operation */
>      rte_crypto_op_attach_asym_session(op, asym_session);
> diff --git a/doc/guides/rel_notes/release_22_03.rst
> b/doc/guides/rel_notes/release_22_03.rst
> index 3bc0630c7c..de8d8ce4e9 100644
> --- a/doc/guides/rel_notes/release_22_03.rst
> +++ b/doc/guides/rel_notes/release_22_03.rst
> @@ -100,6 +100,13 @@ API Changes
>     Also, make sure to start the actual text at the margin.
>     =======================================================
> 
> +* cryptodev: The asym session handling was modified to use a single buffer.
> +  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.
> +  ``rte_cryptodev_asym_session_init`` was removed as this initialisation is
> +  now done by ``rte_cryptodev_asym_session_create``.
> +
cryptodev: The asymmetric session handling was modified to use a single mempool object.
An API ``rte_cryptodev_asym_session_pool_create`` was added to create a mempool with
element size big enough to hold the generic asymmetric session header and max size for a
device private session data. The API ``rte_cryptodev_asym_session_init`` was removed as
the initialization is now moved to ``rte_cryptodev_asym_session_create``.



> diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
> index b9146f652c..474d447496 100644
> --- a/lib/cryptodev/cryptodev_pmd.h
> +++ b/lib/cryptodev/cryptodev_pmd.h
> @@ -340,12 +340,12 @@ typedef int
> (*cryptodev_asym_configure_session_t)(struct rte_cryptodev *dev,
>  typedef void (*cryptodev_sym_free_session_t)(struct rte_cryptodev *dev,
>  		struct rte_cryptodev_sym_session *sess);
>  /**
> - * Free asymmetric session private data.
> + * Clear asymmetric session private data.
>   *
>   * @param	dev		Crypto device pointer
>   * @param	sess		Cryptodev session structure
>   */
> -typedef void (*cryptodev_asym_free_session_t)(struct rte_cryptodev *dev,
> +typedef void (*cryptodev_asym_clear_session_t)(struct rte_cryptodev *dev,
>  		struct rte_cryptodev_asym_session *sess);
>  /**
>   * Perform actual crypto processing (encrypt/digest or auth/decrypt)
> @@ -429,7 +429,7 @@ struct rte_cryptodev_ops {
>  	/**< Configure asymmetric Crypto session. */
>  	cryptodev_sym_free_session_t sym_session_clear;
>  	/**< Clear a Crypto sessions private data. */
> -	cryptodev_asym_free_session_t asym_session_clear;
> +	cryptodev_asym_clear_session_t asym_session_clear;
>  	/**< Clear a Crypto sessions private data. */
>  	union {
>  		cryptodev_sym_cpu_crypto_process_t sym_cpu_process;
> @@ -628,16 +628,9 @@ set_sym_session_private_data(struct
> rte_cryptodev_sym_session *sess,
>  }
> 
>  static inline void *
> -get_asym_session_private_data(const struct rte_cryptodev_asym_session
> *sess,
> -		uint8_t driver_id) {
> -	return sess->sess_private_data[driver_id];
> -}
> -
> -static inline void
> -set_asym_session_private_data(struct rte_cryptodev_asym_session *sess,
> -		uint8_t driver_id, void *private_data)
> +get_asym_session_private_data(struct rte_cryptodev_asym_session *sess)
>  {
> -	sess->sess_private_data[driver_id] = private_data;
> +	return sess->sess_private_data;
>  }
> 
>  #endif /* _CRYPTODEV_PMD_H_ */
> diff --git a/lib/cryptodev/cryptodev_trace_points.c
> b/lib/cryptodev/cryptodev_trace_points.c
> index 5d58951fd5..d23b30edd8 100644
> --- a/lib/cryptodev/cryptodev_trace_points.c
> +++ b/lib/cryptodev/cryptodev_trace_points.c
> @@ -24,6 +24,9 @@
> RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_setup,
>  RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_pool_create,
>  	lib.cryptodev.sym.pool.create)
> 
> +RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_pool_creat
> e,
> +	lib.cryptodev.asym.pool.create)
> +
>  RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_create,
>  	lib.cryptodev.sym.create)
> 
> @@ -39,9 +42,6 @@
> RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_free,
>  RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_init,
>  	lib.cryptodev.sym.init)
> 
> -RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_init,
> -	lib.cryptodev.asym.init)
> -
>  RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_clear,
>  	lib.cryptodev.sym.clear)
> 
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index a40536c5ea..d260f79bbc 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -195,7 +195,7 @@ const char *rte_crypto_asym_op_strings[] = {
>  };
> 
>  /**
> - * The private data structure stored in the session mempool private data.
> + * The private data structure stored in the sym session mempool private data.
>   */
>  struct rte_cryptodev_sym_session_pool_private_data {
>  	uint16_t nb_drivers;
> @@ -204,6 +204,14 @@ struct rte_cryptodev_sym_session_pool_private_data
> {
>  	/**< session user data will be placed after sess_data */
>  };
> 
> +/**
> + * The private data structure stored in the asym session mempool 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 */
> +};
> +
>  int
>  rte_cryptodev_get_cipher_algo_enum(enum rte_crypto_cipher_algorithm
> *algo_enum,
>  		const char *algo_string)
> @@ -1751,47 +1759,6 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
>  	return 0;
>  }
> 
> -int
> -rte_cryptodev_asym_session_init(uint8_t dev_id,
> -		struct rte_cryptodev_asym_session *sess,
> -		struct rte_crypto_asym_xform *xforms,
> -		struct rte_mempool *mp)
> -{
> -	struct rte_cryptodev *dev;
> -	uint8_t index;
> -	int ret;
> -
> -	if (!rte_cryptodev_is_valid_dev(dev_id)) {
> -		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> -		return -EINVAL;
> -	}
> -
> -	dev = rte_cryptodev_pmd_get_dev(dev_id);
> -
> -	if (sess == NULL || xforms == NULL || dev == NULL)
> -		return -EINVAL;
> -
> -	index = dev->driver_id;
> -
> -	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >asym_session_configure,
> -				-ENOTSUP);
> -
> -	if (sess->sess_private_data[index] == NULL) {
> -		ret = dev->dev_ops->asym_session_configure(dev,
> -							xforms,
> -							sess, mp);
> -		if (ret < 0) {
> -			CDEV_LOG_ERR(
> -				"dev_id %d failed to configure session details",
> -				dev_id);
> -			return ret;
> -		}
> -	}
> -
> -	rte_cryptodev_trace_asym_session_init(dev_id, sess, xforms, mp);
> -	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,
> @@ -1834,6 +1801,53 @@ rte_cryptodev_sym_session_pool_create(const char
> *name, uint32_t nb_elts,
>  	return mp;
>  }
> 
> +struct rte_mempool *
> +rte_cryptodev_asym_session_pool_create(const char *name, uint32_t nb_elts,
> +	uint32_t cache_size, int socket_id)
> +{
> +	struct rte_mempool *mp;
> +	struct rte_cryptodev_asym_session_pool_private_data *pool_priv;
> +	uint32_t obj_sz, obj_sz_aligned;
> +	uint8_t dev_id, priv_sz, max_priv_sz = 0;
> +
> +	for (dev_id = 0; dev_id < RTE_CRYPTO_MAX_DEVS; dev_id++)
> +		if (rte_cryptodev_is_valid_dev(dev_id)) {
> +			priv_sz =
> rte_cryptodev_asym_get_private_session_size(dev_id);
> +			if (priv_sz > max_priv_sz)
> +				max_priv_sz = priv_sz;
> +		}
> +	if (max_priv_sz == 0) {
> +		CDEV_LOG_INFO("Could not set max private session size\n");
> +		return NULL;
> +	}
> +
> +	obj_sz = rte_cryptodev_asym_get_header_session_size() + max_priv_sz;
> +	obj_sz_aligned =  RTE_ALIGN_CEIL(obj_sz, RTE_CACHE_LINE_SIZE);
> +
> +	mp = rte_mempool_create(name, nb_elts, obj_sz_aligned, 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->max_priv_session_sz = max_priv_sz;
> +
> +	rte_cryptodev_trace_asym_session_pool_create(name, nb_elts,
> +		cache_size, mp);
> +	return mp;
> +}
> +
>  static unsigned int
>  rte_cryptodev_sym_session_data_size(struct rte_cryptodev_sym_session *sess)
>  {
> @@ -1895,19 +1909,43 @@ rte_cryptodev_sym_session_create(struct
> rte_mempool *mp)
>  }
> 
>  struct rte_cryptodev_asym_session *
> -rte_cryptodev_asym_session_create(struct rte_mempool *mp)
> +rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t dev_id,
> +		struct rte_crypto_asym_xform *xforms)

Please rearrange the function arguments similar to other APIs.
Order should be rte_cryptodev_asym_session_create(dev_id, xforms, mp)


>  {
>  	struct rte_cryptodev_asym_session *sess;
> -	unsigned int session_size =
> +	uint32_t session_priv_data_sz;
> +	struct rte_cryptodev_asym_session_pool_private_data *pool_priv;
> +	unsigned int session_header_size =
>  			rte_cryptodev_asym_get_header_session_size();
> +	struct rte_cryptodev *dev;
> +	int ret;
> +
> +	if (!rte_cryptodev_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return NULL;
> +	}
> +	session_priv_data_sz = rte_cryptodev_asym_get_private_session_size(
> +			dev_id);
> +	dev = rte_cryptodev_pmd_get_dev(dev_id);
> +
> +	if (dev == NULL)
> +		return NULL;
> 
>  	if (!mp) {
>  		CDEV_LOG_ERR("invalid mempool\n");
>  		return NULL;
>  	}

Above checks should be before calling rte_cryptodev_asym_get_private_session_size(dev_id)

> 
> +	pool_priv = rte_mempool_get_priv(mp);
> +
> +	if (pool_priv->max_priv_session_sz < session_priv_data_sz) {
> +		CDEV_LOG_DEBUG(
> +			"The private session data size used when creating the
> mempool is smaller than this device's private session data.");
> +		return NULL;
> +	}
> +
>  	/* Verify if provided mempool can hold elements big enough. */
> -	if (mp->elt_size < session_size) {
> +	if (mp->elt_size < session_header_size + session_priv_data_sz) {
>  		CDEV_LOG_ERR(
>  			"mempool elements too small to hold session objects");
>  		return NULL;
> @@ -1919,10 +1957,27 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp)
>  		return NULL;
>  	}
> 
> +	sess->driver_id = dev->driver_id;
> +	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, 0, session_size);
> +	memset(sess->sess_private_data, 0, session_priv_data_sz);
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >asym_session_configure, NULL);
> +
> +	if (sess->sess_private_data[0] == 0) {
> +		ret = dev->dev_ops->asym_session_configure(dev,
> +							xforms,
> +							sess, mp);

The mempool object is allocated in the library layer,
so why is it need to be passed to PMD? PMD cannot get mempool object. Right?

> +		if (ret < 0) {
> +			CDEV_LOG_ERR(
> +				"dev_id %d failed to configure session details",
> +				dev_id);
> +			return NULL;
> +		}
> +	}
> 
>  	rte_cryptodev_trace_asym_session_create(mp, sess);
>  	return sess;
> @@ -2009,20 +2064,11 @@ rte_cryptodev_sym_session_free(struct
> rte_cryptodev_sym_session *sess)
>  int
>  rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
>  {
> -	uint8_t i;
> -	void *sess_priv;
>  	struct rte_mempool *sess_mp;
> 
>  	if (sess == NULL)
>  		return -EINVAL;
> 
> -	/* Check that all device private data has been freed */
> -	for (i = 0; i < nb_drivers; i++) {
> -		sess_priv = get_asym_session_private_data(sess, i);
> -		if (sess_priv != NULL)
> -			return -EBUSY;
> -	}
> -
>  	/* Return session to mempool */
>  	sess_mp = rte_mempool_from_obj(sess);
>  	rte_mempool_put(sess_mp, sess);
As commented earlier, free and clear can be squashed to a single API.

> @@ -2061,12 +2107,7 @@
> rte_cryptodev_sym_get_existing_header_session_size(
>  unsigned int
>  rte_cryptodev_asym_get_header_session_size(void)
>  {
> -	/*
> -	 * Header contains pointers to the private data
> -	 * of all registered drivers, and a flag which
> -	 * indicates presence of private data
> -	 */
> -	return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
> +	return sizeof(struct rte_cryptodev_asym_session);
>  }
> 
>  unsigned int
> @@ -2092,7 +2133,6 @@ unsigned int
>  rte_cryptodev_asym_get_private_session_size(uint8_t dev_id)
>  {
>  	struct rte_cryptodev *dev;
> -	unsigned int header_size = sizeof(void *) * nb_drivers;
>  	unsigned int priv_sess_size;
> 
>  	if (!rte_cryptodev_is_valid_dev(dev_id))
> @@ -2104,11 +2144,8 @@
> rte_cryptodev_asym_get_private_session_size(uint8_t dev_id)
>  		return 0;
> 
>  	priv_sess_size = (*dev->dev_ops->asym_session_get_size)(dev);
> -	if (priv_sess_size < header_size)
> -		return header_size;
> 
>  	return priv_sess_size;
> -
>  }
> 
>  int
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index 59ea5a54df..8bd85f1575 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -919,9 +919,13 @@ struct rte_cryptodev_sym_session {
>  };
> 
>  /** Cryptodev asymmetric crypto session */
> -struct rte_cryptodev_asym_session {
> -	__extension__ void *sess_private_data[0];
> -	/**< Private asymmetric session material */
> +__extension__ struct rte_cryptodev_asym_session {
> +	uint8_t driver_id;
> +	/**< Session driver ID. */
> +	uint16_t max_priv_session_sz;
> +	/**< Size of private session data used when creating mempool */
> +	uint8_t padding[5];
> +	uint8_t sess_private_data[0];
>  };
RTE_STD_C11 can be used instead of __extension__.

Max_priv_session_sz can be renamed to max_priv_data_sz.
Session word is redundant as it is inside session struct.

> 
>  /**
> @@ -956,6 +960,29 @@ 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 an asymmetric session mempool.
> + *
> + * @param name
> + *   The unique mempool name.
> + * @param nb_elts
> + *   The number of elements in the mempool.
> + * @param cache_size
> + *   The number of per-lcore cache elements
> + * @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 mempool
> + *  - On failure returns NULL
> + */
> +__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);
> +
>  /**
>   * Create symmetric crypto session header (generic with no private data)
>   *
> @@ -973,13 +1000,17 @@ rte_cryptodev_sym_session_create(struct
> rte_mempool *mempool);
>   *
>   * @param   mempool    mempool to allocate asymmetric session
>   *                     objects from
> + * @param   dev_id   ID of device that we want the session to be used on
> + * @param   xforms   Asymmetric crypto transform operations to apply on flow
> + *                   processed with this session

Change the order of arguments as commented above.

>   * @return
>   *  - On success return pointer to asym-session
>   *  - On failure returns NULL
>   */
>  __rte_experimental
>  struct rte_cryptodev_asym_session *
> -rte_cryptodev_asym_session_create(struct rte_mempool *mempool);
> +rte_cryptodev_asym_session_create(struct rte_mempool *mempool,
> +		uint8_t dev_id, struct rte_crypto_asym_xform *xforms);
> 
>  /**
>   * Frees symmetric crypto session header, after checking that all
> @@ -997,8 +1028,7 @@ int
>  rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess);
> 
>  /**
> - * Frees asymmetric crypto session header, after checking that all
> - * the device private data has been freed, returning it
> + * Frees asymmetric crypto session header, returning it
>   * to its original mempool.
>   *
>   * @param   sess     Session header to be freed.
> @@ -1006,7 +1036,6 @@ rte_cryptodev_sym_session_free(struct
> rte_cryptodev_sym_session *sess);
>   * @return
>   *  - 0 if successful.
>   *  - -EINVAL if session is NULL.
> - *  - -EBUSY if not all device private data has been freed.
>   */
>  __rte_experimental
>  int
> @@ -1034,28 +1063,6 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
>  			struct rte_crypto_sym_xform *xforms,
>  			struct rte_mempool *mempool);
> 
> -/**
> - * Initialize asymmetric session on a device with specific asymmetric xform
> - *
> - * @param   dev_id   ID of device that we want the session to be used on
> - * @param   sess     Session to be set up on a device
> - * @param   xforms   Asymmetric crypto transform operations to apply on flow
> - *                   processed with this session
> - * @param   mempool  Mempool to be used for internal allocation.
> - *
> - * @return
> - *  - On success, zero.
> - *  - -EINVAL if input parameters are invalid.
> - *  - -ENOTSUP if crypto device does not support the crypto transform.
> - *  - -ENOMEM if the private session could not be allocated.
> - */

These error numbers should be added in the create() API.
I guess your subsequent patch is doing that.

> -__rte_experimental
> -int
> -rte_cryptodev_asym_session_init(uint8_t dev_id,
> -			struct rte_cryptodev_asym_session *sess,
> -			struct rte_crypto_asym_xform *xforms,
> -			struct rte_mempool *mempool);
> -
>  /**
>   * Frees private data for the device id, based on its device type,
>   * returning it to its mempool. It is the application's responsibility
> @@ -1075,11 +1082,10 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
>  			struct rte_cryptodev_sym_session *sess);
> 
>  /**
> - * Frees resources held by asymmetric session during
> rte_cryptodev_session_init
> + * Clear private data held by asymmetric session.
>   *
>   * @param   dev_id   ID of device that uses the asymmetric session.
> - * @param   sess     Asymmetric session setup on device using
> - *					 rte_cryptodev_session_init
> + * @param   sess     Asymmetric session setup on device.
>   * @return
>   *  - 0 if successful.
>   *  - -EINVAL if device is invalid or session is NULL.
> @@ -1116,7 +1122,7 @@
> rte_cryptodev_sym_get_existing_header_session_size(
>  		struct rte_cryptodev_sym_session *sess);
> 
>  /**
> - * Get the size of the asymmetric session header, for all registered drivers.
> + * Get the size of the asymmetric session header.
>   *
>   * @return
>   *   Size of the asymmetric header session.
> diff --git a/lib/cryptodev/rte_cryptodev_trace.h
> b/lib/cryptodev/rte_cryptodev_trace.h
> index d1f4f069a3..befbaf7f44 100644
> --- a/lib/cryptodev/rte_cryptodev_trace.h
> +++ b/lib/cryptodev/rte_cryptodev_trace.h
> @@ -83,6 +83,16 @@ RTE_TRACE_POINT(
>  	rte_trace_point_emit_u16(sess->user_data_sz);
>  )
> 
> +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),
> +	rte_trace_point_emit_string(name);
> +	rte_trace_point_emit_u32(nb_elts);
> +	rte_trace_point_emit_u32(cache_size);
> +	rte_trace_point_emit_ptr(mempool);
> +)
> +
>  RTE_TRACE_POINT(
>  	rte_cryptodev_trace_asym_session_create,
>  	RTE_TRACE_POINT_ARGS(void *mempool,
> @@ -117,17 +127,6 @@ RTE_TRACE_POINT(
>  	rte_trace_point_emit_ptr(mempool);
>  )
> 
> -RTE_TRACE_POINT(
> -	rte_cryptodev_trace_asym_session_init,
> -	RTE_TRACE_POINT_ARGS(uint8_t dev_id,
> -		struct rte_cryptodev_asym_session *sess, void *xforms,
> -		void *mempool),
> -	rte_trace_point_emit_u8(dev_id);
> -	rte_trace_point_emit_ptr(sess);
> -	rte_trace_point_emit_ptr(xforms);
> -	rte_trace_point_emit_ptr(mempool);
> -)

rte_cryptodev_trace_asym_session_create() would need an update due to these changes.

> -
>  RTE_TRACE_POINT(
>  	rte_cryptodev_trace_sym_session_clear,
>  	RTE_TRACE_POINT_ARGS(uint8_t dev_id, void *sess),
> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> index c50745fa8c..eaea976f21 100644
> --- a/lib/cryptodev/version.map
> +++ b/lib/cryptodev/version.map
> @@ -58,7 +58,6 @@ EXPERIMENTAL {
>  	rte_cryptodev_asym_session_clear;
>  	rte_cryptodev_asym_session_create;
>  	rte_cryptodev_asym_session_free;
> -	rte_cryptodev_asym_session_init;
>  	rte_cryptodev_asym_xform_capability_check_modlen;
>  	rte_cryptodev_asym_xform_capability_check_optype;
>  	rte_cryptodev_sym_cpu_crypto_process;
> @@ -81,7 +80,6 @@ EXPERIMENTAL {
>  	__rte_cryptodev_trace_sym_session_free;
>  	__rte_cryptodev_trace_asym_session_free;
>  	__rte_cryptodev_trace_sym_session_init;
> -	__rte_cryptodev_trace_asym_session_init;
>  	__rte_cryptodev_trace_sym_session_clear;
>  	__rte_cryptodev_trace_asym_session_clear;
>  	__rte_cryptodev_trace_dequeue_burst;
> @@ -104,6 +102,9 @@ EXPERIMENTAL {
>  	rte_cryptodev_remove_deq_callback;
>  	rte_cryptodev_remove_enq_callback;
> 
> +	# added 22.03
> +	rte_cryptodev_asym_session_pool_create;
> +	__rte_cryptodev_trace_asym_session_pool_create;
>  };
> 
>  INTERNAL {
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [EXT] [PATCH v3 3/4] crypto: add asym session user data API
  2022-02-03 16:04 ` [PATCH v3 3/4] crypto: add asym session user data API Ciara Power
@ 2022-02-07  8:41   ` Akhil Goyal
  0 siblings, 0 replies; 12+ messages in thread
From: Akhil Goyal @ 2022-02-07  8:41 UTC (permalink / raw)
  To: Ciara Power, dev; +Cc: roy.fan.zhang, Anoob Joseph, mdr, Declan Doherty



> -----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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [EXT] [PATCH v3 4/4] crypto: modify return value for asym session create
  2022-02-03 16:04 ` [PATCH v3 4/4] crypto: modify return value for asym session create Ciara Power
@ 2022-02-07  9:04   ` Akhil Goyal
  2022-02-07 13:02     ` Thomas Monjalon
  2022-02-07 14:50     ` Power, Ciara
  0 siblings, 2 replies; 12+ messages in thread
From: Akhil Goyal @ 2022-02-07  9:04 UTC (permalink / raw)
  To: Ciara Power, dev, thomas; +Cc: roy.fan.zhang, Anoob Joseph, mdr, Declan Doherty

> diff --git a/doc/guides/prog_guide/cryptodev_lib.rst
> b/doc/guides/prog_guide/cryptodev_lib.rst
> index 62bd3577f5..8e16461dc6 100644
> --- a/doc/guides/prog_guide/cryptodev_lib.rst
> +++ b/doc/guides/prog_guide/cryptodev_lib.rst
> @@ -1236,10 +1236,10 @@ crypto operations is similar except change to
> respective op and xform setup).
>       * Create asym crypto session and initialize it for the crypto device.
>       * The session structure is hidden from the app, so void * is used.
>       */
> -    void *asym_session;
> -    asym_session =
> rte_cryptodev_asym_session_create(asym_session_pool,
> +    void *asym_session = NULL;
> +    ret = rte_cryptodev_asym_session_create(&asym_session,
> asym_session_pool,
>              cdev_id, &modex_xform);
> -    if (asym_session == NULL)
> +    if (ret < 0)
>          rte_exit(EXIT_FAILURE, "Session could not be created\n");

Sample Code in the rst files is no more added. @Thomas: Could you please confirm?
Probably a separate patch is required to clean this up.

> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index 0d816ed4a9..005f0e7952 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -1912,9 +1912,9 @@ rte_cryptodev_sym_session_create(struct
> rte_mempool *mp)
>  	return sess;
>  }
> 
> -void *
> -rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t
> dev_id,
> -		struct rte_crypto_asym_xform *xforms)
> +int
> +rte_cryptodev_asym_session_create(void **session, struct rte_mempool
> *mp,
> +		uint8_t dev_id, struct rte_crypto_asym_xform *xforms)

Do you really need a double pointer for the session handle?

>  {
>  	struct rte_cryptodev_asym_session *sess;
>  	uint32_t session_priv_data_sz;
> @@ -1926,18 +1926,18 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp, uint8_t dev_id,
> 
>  	if (!rte_cryptodev_is_valid_dev(dev_id)) {
>  		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> -		return NULL;
> +		return -EINVAL;
>  	}
>  	session_priv_data_sz =
> rte_cryptodev_asym_get_private_session_size(
>  			dev_id);
>  	dev = rte_cryptodev_pmd_get_dev(dev_id);
> 
>  	if (dev == NULL)
> -		return NULL;
> +		return -EINVAL;
> 
>  	if (!mp) {
>  		CDEV_LOG_ERR("invalid mempool\n");
> -		return NULL;
> +		return -EINVAL;
>  	}
> 
>  	pool_priv = rte_mempool_get_priv(mp);
> @@ -1945,22 +1945,23 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp, uint8_t dev_id,
>  	if (pool_priv->max_priv_session_sz < session_priv_data_sz) {
>  		CDEV_LOG_DEBUG(
>  			"The private session data size used when creating the
> mempool is smaller than this device's private session data.");
> -		return NULL;
> +		return -EINVAL;
>  	}
> 
>  	/* Verify if provided mempool can hold elements big enough. */
>  	if (mp->elt_size < session_header_size + session_priv_data_sz) {
>  		CDEV_LOG_ERR(
>  			"mempool elements too small to hold session
> objects");
> -		return NULL;
> +		return -EINVAL;
>  	}
> 
>  	/* Allocate a session structure from the session pool */
> -	if (rte_mempool_get(mp, (void **)&sess)) {
> +	if (rte_mempool_get(mp, session)) {
>  		CDEV_LOG_ERR("couldn't get object from session
> mempool");
> -		return NULL;
> +		return -ENOMEM;
>  	}
> 
> +	sess = *session;
>  	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;
> @@ -1970,7 +1971,7 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp, uint8_t dev_id,
>  	 */
>  	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);
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >asym_session_configure, -ENOTSUP);
> 
>  	if (sess->sess_private_data[0] == 0) {
>  		ret = dev->dev_ops->asym_session_configure(dev,
> @@ -1980,12 +1981,12 @@ rte_cryptodev_asym_session_create(struct
> rte_mempool *mp, uint8_t dev_id,
>  			CDEV_LOG_ERR(
>  				"dev_id %d failed to configure session
> details",
>  				dev_id);
> -			return NULL;
> +			return ret;
>  		}
>  	}
> 
>  	rte_cryptodev_trace_asym_session_create(mp, sess);
> -	return sess;
> +	return 0;
>  }
> 
>  int
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index 6a4d6d9934..9a75936963 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -990,18 +990,21 @@ rte_cryptodev_sym_session_create(struct
> rte_mempool *mempool);
>  /**
>   * Create asymmetric crypto session header (generic with no private data)
>   *
> + * @param   session    void ** for session to be used
>   * @param   mempool    mempool to allocate asymmetric session
>   *                     objects from
>   * @param   dev_id   ID of device that we want the session to be used on
>   * @param   xforms   Asymmetric crypto transform operations to apply on
> flow
>   *                   processed with this session
>   * @return
> - *  - On success return pointer to asym-session
> - *  - On failure returns NULL
> + *  - 0 on success.
> + *  - -EINVAL on invalid arguments.
> + *  - -ENOMEM on memory error for session allocation.
> + *  - -ENOTSUP if device doesn't support session configuration.
>   */
>  __rte_experimental
> -void *
> -rte_cryptodev_asym_session_create(struct rte_mempool *mempool,
> +int
> +rte_cryptodev_asym_session_create(void **session, struct rte_mempool
> *mempool,
>  		uint8_t dev_id, struct rte_crypto_asym_xform *xforms);
> 
Session should be the last parameter.
First all in params and then out params.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [EXT] [PATCH v3 4/4] crypto: modify return value for asym session create
  2022-02-07  9:04   ` [EXT] " Akhil Goyal
@ 2022-02-07 13:02     ` Thomas Monjalon
  2022-02-07 14:50     ` Power, Ciara
  1 sibling, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2022-02-07 13:02 UTC (permalink / raw)
  To: Ciara Power, Akhil Goyal
  Cc: dev, roy.fan.zhang, Anoob Joseph, mdr, Declan Doherty

07/02/2022 10:04, Akhil Goyal:
> > diff --git a/doc/guides/prog_guide/cryptodev_lib.rst
> > b/doc/guides/prog_guide/cryptodev_lib.rst
> > index 62bd3577f5..8e16461dc6 100644
> > --- a/doc/guides/prog_guide/cryptodev_lib.rst
> > +++ b/doc/guides/prog_guide/cryptodev_lib.rst
> > @@ -1236,10 +1236,10 @@ crypto operations is similar except change to
> > respective op and xform setup).
> >       * Create asym crypto session and initialize it for the crypto device.
> >       * The session structure is hidden from the app, so void * is used.
> >       */
> > -    void *asym_session;
> > -    asym_session =
> > rte_cryptodev_asym_session_create(asym_session_pool,
> > +    void *asym_session = NULL;
> > +    ret = rte_cryptodev_asym_session_create(&asym_session,
> > asym_session_pool,
> >              cdev_id, &modex_xform);
> > -    if (asym_session == NULL)
> > +    if (ret < 0)
> >          rte_exit(EXIT_FAILURE, "Session could not be created\n");
> 
> Sample Code in the rst files is no more added. @Thomas: Could you please confirm?
> Probably a separate patch is required to clean this up.

Yes the target is to remove them.
Instead we want to include parts of examples with a specific syntax.
See literalinclude here:
https://doc.dpdk.org/guides/contributing/documentation.html#code-and-literal-block-sections




^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [EXT] [PATCH v3 1/4] crypto: use single buffer for asymmetric session
  2022-02-07  8:19   ` [EXT] " Akhil Goyal
@ 2022-02-07 14:22     ` Power, Ciara
  0 siblings, 0 replies; 12+ messages in thread
From: Power, Ciara @ 2022-02-07 14:22 UTC (permalink / raw)
  To: Akhil Goyal, dev
  Cc: Zhang, Roy Fan, Anoob Joseph, mdr, Doherty, Declan,
	Ankur Dwivedi, Tejasree Kondoj, Griffin, John, Trahe, Fiona,
	Jain, Deepak K

Hi Akhil,

Left some replies inline. I will address all other comments in v4.

Thanks,
Ciara

>-----Original Message-----
>From: Akhil Goyal <gakhil@marvell.com>
>Sent: Monday 7 February 2022 08:20
>To: Power, Ciara <ciara.power@intel.com>; dev@dpdk.org
>Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Anoob Joseph
><anoobj@marvell.com>; mdr@ashroe.eu; Doherty, Declan
><declan.doherty@intel.com>; Ankur Dwivedi <adwivedi@marvell.com>;
>Tejasree Kondoj <ktejasree@marvell.com>; Griffin, John
><john.griffin@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>; Jain,
>Deepak K <deepak.k.jain@intel.com>
>Subject: RE: [EXT] [PATCH v3 1/4] crypto: use single buffer for asymmetric
>session
>
>> Rather than using a session buffer that contains pointers to private
>> session data elsewhere, have a single session buffer.
>> This session is created for a driver ID, and the mempool element
>> contains space for the max session private data needed for any driver.
>
>This means asymmetric ops are not allowed with scheduler PMD.
>
[CP]  Yes, currently asymmetric isn't supported for scheduler PMD anyway so this shouldn't be an issue for this patchset.
For the approach to be applied to symmetric crypto also in future release, the scheduler PMD would need to be reworked to align with the new session usage.

<snip>
		return NULL;
>> @@ -1919,10 +1957,27 @@ rte_cryptodev_asym_session_create(struct
>> rte_mempool *mp)
>>  		return NULL;
>>  	}
>>
>> +	sess->driver_id = dev->driver_id;
>> +	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, 0, session_size);
>> +	memset(sess->sess_private_data, 0, session_priv_data_sz);
>> +
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
>> >asym_session_configure, NULL);
>> +
>> +	if (sess->sess_private_data[0] == 0) {
>> +		ret = dev->dev_ops->asym_session_configure(dev,
>> +							xforms,
>> +							sess, mp);
>
>The mempool object is allocated in the library layer, so why is it need to be
>passed to PMD? PMD cannot get mempool object. Right?

[CP] Yes true, I don't think the mempool needs to be passed to the configure function anymore, same with dev,
these were just used to allocate private session data before I believe. Will remove these parameters altogether instead of leaving as rte_unused.

<snip>
>> -/**
>> - * Initialize asymmetric session on a device with specific asymmetric
>> xform
>> - *
>> - * @param   dev_id   ID of device that we want the session to be used on
>> - * @param   sess     Session to be set up on a device
>> - * @param   xforms   Asymmetric crypto transform operations to apply on
>flow
>> - *                   processed with this session
>> - * @param   mempool  Mempool to be used for internal allocation.
>> - *
>> - * @return
>> - *  - On success, zero.
>> - *  - -EINVAL if input parameters are invalid.
>> - *  - -ENOTSUP if crypto device does not support the crypto transform.
>> - *  - -ENOMEM if the private session could not be allocated.
>> - */
>
>These error numbers should be added in the create() API.
>I guess your subsequent patch is doing that.

[CP] Correct, the 4th patch changes the return values of the create() function and adds these errors numbers. 

<snip>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [EXT] [PATCH v3 4/4] crypto: modify return value for asym session create
  2022-02-07  9:04   ` [EXT] " Akhil Goyal
  2022-02-07 13:02     ` Thomas Monjalon
@ 2022-02-07 14:50     ` Power, Ciara
  2022-02-08 20:21       ` Akhil Goyal
  1 sibling, 1 reply; 12+ messages in thread
From: Power, Ciara @ 2022-02-07 14:50 UTC (permalink / raw)
  To: Akhil Goyal, dev, thomas
  Cc: Zhang, Roy Fan, Anoob Joseph, mdr, Doherty, Declan

Hi Akhil,

Some replies inline.

Thanks,
Ciara


>-----Original Message-----
>From: Akhil Goyal <gakhil@marvell.com>
>Sent: Monday 7 February 2022 09:05
>To: Power, Ciara <ciara.power@intel.com>; dev@dpdk.org;
>thomas@monjalon.net
>Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Anoob Joseph
><anoobj@marvell.com>; mdr@ashroe.eu; Doherty, Declan
><declan.doherty@intel.com>
>Subject: RE: [EXT] [PATCH v3 4/4] crypto: modify return value for asym session
>create
>
>> diff --git a/doc/guides/prog_guide/cryptodev_lib.rst
>> b/doc/guides/prog_guide/cryptodev_lib.rst
>> index 62bd3577f5..8e16461dc6 100644
>> --- a/doc/guides/prog_guide/cryptodev_lib.rst
>> +++ b/doc/guides/prog_guide/cryptodev_lib.rst
>> @@ -1236,10 +1236,10 @@ crypto operations is similar except change to
>> respective op and xform setup).
>>       * Create asym crypto session and initialize it for the crypto device.
>>       * The session structure is hidden from the app, so void * is used.
>>       */
>> -    void *asym_session;
>> -    asym_session =
>> rte_cryptodev_asym_session_create(asym_session_pool,
>> +    void *asym_session = NULL;
>> +    ret = rte_cryptodev_asym_session_create(&asym_session,
>> asym_session_pool,
>>              cdev_id, &modex_xform);
>> -    if (asym_session == NULL)
>> +    if (ret < 0)
>>          rte_exit(EXIT_FAILURE, "Session could not be created\n");
>
>Sample Code in the rst files is no more added. @Thomas: Could you please
>confirm?
>Probably a separate patch is required to clean this up.
>

[CP] I see Thomas replied on this - thanks. Will try find a section of example/test code that does the same thing as being shown here.


>> diff --git a/lib/cryptodev/rte_cryptodev.c
>> b/lib/cryptodev/rte_cryptodev.c index 0d816ed4a9..005f0e7952 100644
>> --- a/lib/cryptodev/rte_cryptodev.c
>> +++ b/lib/cryptodev/rte_cryptodev.c
>> @@ -1912,9 +1912,9 @@ rte_cryptodev_sym_session_create(struct
>> rte_mempool *mp)
>>  	return sess;
>>  }
>>
>> -void *
>> -rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t
>> dev_id,
>> -		struct rte_crypto_asym_xform *xforms)
>> +int
>> +rte_cryptodev_asym_session_create(void **session, struct rte_mempool
>> *mp,
>> +		uint8_t dev_id, struct rte_crypto_asym_xform *xforms)
>
>Do you really need a double pointer for the session handle?
>

[CP] Yes I believe so, the return value used to be session, but now that we have an int return value, the session needs to be passed in as a parameter somehow.
We need the double pointer because we need the call to rte_mempool_get() to set the original session pointer that can be accessed outside of this function,
rather than just the local copy if it were a singular session pointer passed in as a parameter.


<snip>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [EXT] [PATCH v3 4/4] crypto: modify return value for asym session create
  2022-02-07 14:50     ` Power, Ciara
@ 2022-02-08 20:21       ` Akhil Goyal
  0 siblings, 0 replies; 12+ messages in thread
From: Akhil Goyal @ 2022-02-08 20:21 UTC (permalink / raw)
  To: Power, Ciara, dev
  Cc: Zhang, Roy Fan, Anoob Joseph, mdr, Doherty, Declan, thomas

> >> diff --git a/lib/cryptodev/rte_cryptodev.c
> >> b/lib/cryptodev/rte_cryptodev.c index 0d816ed4a9..005f0e7952 100644
> >> --- a/lib/cryptodev/rte_cryptodev.c
> >> +++ b/lib/cryptodev/rte_cryptodev.c
> >> @@ -1912,9 +1912,9 @@ rte_cryptodev_sym_session_create(struct
> >> rte_mempool *mp)
> >>  	return sess;
> >>  }
> >>
> >> -void *
> >> -rte_cryptodev_asym_session_create(struct rte_mempool *mp, uint8_t
> >> dev_id,
> >> -		struct rte_crypto_asym_xform *xforms)
> >> +int
> >> +rte_cryptodev_asym_session_create(void **session, struct rte_mempool
> >> *mp,
> >> +		uint8_t dev_id, struct rte_crypto_asym_xform *xforms)
> >
> >Do you really need a double pointer for the session handle?
> >
> 
> [CP] Yes I believe so, the return value used to be session, but now that we have
> an int return value, the session needs to be passed in as a parameter somehow.
> We need the double pointer because we need the call to rte_mempool_get() to
> set the original session pointer that can be accessed outside of this function,
> rather than just the local copy if it were a singular session pointer passed in as a
> parameter.
> 
Ok

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-02-08 20:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03 16:04 [PATCH v3 0/4] crypto: improve asym session usage Ciara Power
2022-02-03 16:04 ` [PATCH v3 1/4] crypto: use single buffer for asymmetric session Ciara Power
2022-02-07  8:19   ` [EXT] " Akhil Goyal
2022-02-07 14:22     ` Power, Ciara
2022-02-03 16:04 ` [PATCH v3 2/4] crypto: hide asym session structure Ciara Power
2022-02-03 16:04 ` [PATCH v3 3/4] crypto: add asym session user data API Ciara Power
2022-02-07  8:41   ` [EXT] " Akhil Goyal
2022-02-03 16:04 ` [PATCH v3 4/4] crypto: modify return value for asym session create Ciara Power
2022-02-07  9:04   ` [EXT] " Akhil Goyal
2022-02-07 13:02     ` Thomas Monjalon
2022-02-07 14:50     ` Power, Ciara
2022-02-08 20:21       ` Akhil Goyal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).