* [PATCH] crypto: use single buffer for asymmetric session
@ 2021-12-13 15:04 Ciara Power
2021-12-13 16:33 ` [EXT] " Anoob Joseph
2021-12-19 23:12 ` Ray Kinsella
0 siblings, 2 replies; 4+ messages in thread
From: Ciara Power @ 2021-12-13 15:04 UTC (permalink / raw)
To: dev
Cc: roy.fan.zhang, gakhil, Ciara Power, Declan Doherty,
Ankur Dwivedi, Anoob Joseph, Tejasree Kondoj, John Griffin,
Fiona Trahe, Deepak Kumar Jain, Ray Kinsella
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>
---
Hiding the asym session structure by moving it to an internal header
will be implemented in a later version of this patch.
---
app/test-crypto-perf/cperf_ops.c | 14 +-
app/test/test_cryptodev_asym.c | 204 ++++--------------
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/octeontx2/otx2_cryptodev_ops.c | 25 +--
drivers/crypto/openssl/rte_openssl_pmd.c | 5 +-
drivers/crypto/openssl/rte_openssl_pmd_ops.c | 23 +-
drivers/crypto/qat/qat_asym.c | 35 +--
lib/cryptodev/cryptodev_pmd.h | 11 +-
lib/cryptodev/cryptodev_trace_points.c | 3 +
lib/cryptodev/rte_cryptodev.c | 199 +++++++++++------
lib/cryptodev/rte_cryptodev.h | 107 ++++++---
lib/cryptodev/rte_cryptodev_trace.h | 12 ++
lib/cryptodev/version.map | 6 +-
16 files changed, 302 insertions(+), 394 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 9d19a6d6d9..672260fb78 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, 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;
}
@@ -2079,10 +1962,8 @@ test_ecdsa_sign_verify(enum curve curve_id)
}
exit:
- if (sess != NULL) {
- rte_cryptodev_asym_session_clear(dev_id, sess);
+ if (sess != NULL)
rte_cryptodev_asym_session_free(sess);
- }
if (op != NULL)
rte_crypto_op_free(op);
return status;
@@ -2154,15 +2035,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 +2052,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/drivers/crypto/cnxk/cn10k_cryptodev_ops.c b/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
index d25a17cbf3..ea658dc1df 100644
--- a/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
@@ -158,8 +158,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))
@@ -451,8 +450,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 449208da8f..30a970120f 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 {
@@ -447,8 +446,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 a2281fb8de..c37d47d63b 100644
--- a/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
@@ -645,7 +645,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;
@@ -655,7 +655,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);
}
@@ -667,15 +666,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);
@@ -687,7 +681,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 9e8fd495cf..f0cd0b5799 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/octeontx2/otx2_cryptodev_ops.c b/drivers/crypto/octeontx2/otx2_cryptodev_ops.c
index 339b82f33e..912b768585 100644
--- a/drivers/crypto/octeontx2/otx2_cryptodev_ops.c
+++ b/drivers/crypto/octeontx2/otx2_cryptodev_ops.c
@@ -587,8 +587,7 @@ otx2_cpt_enqueue_asym(struct otx2_cpt_qp *qp,
return -ENOMEM;
}
- sess = get_asym_session_private_data(asym_op->session,
- otx2_cryptodev_driver_id);
+ sess = get_asym_session_private_data(asym_op->session);
/* Store IO address of the mdata to meta_buf */
params.meta_buf = rte_mempool_virt2iova(mdata);
@@ -941,8 +940,7 @@ otx2_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,
- otx2_cryptodev_driver_id);
+ sess = get_asym_session_private_data(op->session);
switch (sess->xfrm_type) {
case RTE_CRYPTO_ASYM_XFORM_RSA:
@@ -1353,24 +1351,17 @@ otx2_cpt_asym_session_size_get(struct rte_cryptodev *dev __rte_unused)
}
static int
-otx2_cpt_asym_session_cfg(struct rte_cryptodev *dev,
+otx2_cpt_asym_session_cfg(struct rte_cryptodev *dev __rte_unused,
struct rte_crypto_asym_xform *xform,
struct rte_cryptodev_asym_session *sess,
struct rte_mempool *pool)
{
- struct cpt_asym_sess_misc *priv;
+ struct cpt_asym_sess_misc *priv = get_asym_session_private_data(sess);
vq_cmd_word3_t vq_cmd_w3;
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");
@@ -1384,8 +1375,6 @@ otx2_cpt_asym_session_cfg(struct rte_cryptodev *dev,
vq_cmd_w3.s.grp = OTX2_CPT_EGRP_AE;
priv->cpt_inst_w7 = vq_cmd_w3.u64;
- set_asym_session_private_data(sess, dev->driver_id, priv);
-
return 0;
}
@@ -1394,11 +1383,10 @@ otx2_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;
@@ -1407,9 +1395,6 @@ otx2_cpt_asym_session_clear(struct rte_cryptodev *dev,
/* Reset and free object back to pool */
memset(priv, 0, otx2_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);
}
struct rte_cryptodev_ops otx2_cpt_ops = {
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 f893508030..f3c1fb2a69 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,22 +723,15 @@ 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) {
@@ -773,13 +763,10 @@ qat_asym_session_configure(struct rte_cryptodev *dev,
}
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(sess->sess_private_data);
return 0;
error:
- rte_mempool_put(mempool, sess_private_data);
return err;
}
@@ -793,15 +780,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..833f8044fb 100644
--- a/lib/cryptodev/cryptodev_pmd.h
+++ b/lib/cryptodev/cryptodev_pmd.h
@@ -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..df166e2bc4 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)
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index a40536c5ea..ffd8733fcb 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,16 @@ 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 {
+ uint8_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
rte_cryptodev_get_cipher_algo_enum(enum rte_crypto_cipher_algorithm *algo_enum,
const char *algo_string)
@@ -1751,47 +1761,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 +1803,54 @@ 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, uint16_t user_data_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 +
+ 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,
+ (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,
+ user_data_size, cache_size, mp);
+ return mp;
+}
+
static unsigned int
rte_cryptodev_sym_session_data_size(struct rte_cryptodev_sym_session *sess)
{
@@ -1895,19 +1912,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 +1960,28 @@ rte_cryptodev_asym_session_create(struct rte_mempool *mp)
return NULL;
}
+ 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, 0, session_size);
+ 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);
+
+ 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 +2068,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 +2111,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 +2137,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 +2148,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
@@ -2137,6 +2178,34 @@ rte_cryptodev_sym_session_get_user_data(
return (void *)(sess->sess_data + sess->nb_drivers);
}
+int
+rte_cryptodev_asym_session_set_user_data(
+ struct rte_cryptodev_asym_session *sess,
+ void *data, uint16_t size)
+{
+ 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(
+ struct rte_cryptodev_asym_session *sess)
+{
+ 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 59ea5a54df..11a62bb555 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -919,9 +919,15 @@ 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. */
+ uint8_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_data */
+ uint8_t padding[4];
+ uint8_t sess_private_data[0];
};
/**
@@ -956,6 +962,31 @@ 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 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
+ * constraint for the reserved zone.
+ *
+ * @return
+ * - On success return size of the session
+ * - On failure returns 0
+ */
+__rte_experimental
+struct rte_mempool *
+rte_cryptodev_asym_session_pool_create(const char *name, uint32_t nb_elts,
+ uint32_t cache_size, uint16_t user_data_size, int socket_id);
+
/**
* Create symmetric crypto session header (generic with no private data)
*
@@ -973,13 +1004,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
@@ -1034,28 +1069,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,14 +1088,13 @@ 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
+ * Frees resources 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.
+- * - -EINVAL if device is invalid or session is NULL.
*/
__rte_experimental
int
@@ -1116,7 +1128,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.
@@ -1218,6 +1230,39 @@ 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(
+ struct rte_cryptodev_asym_session *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(
+ struct rte_cryptodev_asym_session *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 d1f4f069a3..a8ee8c901c 100644
--- a/lib/cryptodev/rte_cryptodev_trace.h
+++ b/lib/cryptodev/rte_cryptodev_trace.h
@@ -83,6 +83,18 @@ 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,
+ 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);
+)
+
RTE_TRACE_POINT(
rte_cryptodev_trace_asym_session_create,
RTE_TRACE_POINT_ARGS(void *mempool,
diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
index c50745fa8c..00b1c9ae35 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;
@@ -104,6 +103,11 @@ EXPERIMENTAL {
rte_cryptodev_remove_deq_callback;
rte_cryptodev_remove_enq_callback;
+ # added 22.03
+ rte_cryptodev_asym_session_pool_create;
+ rte_cryptodev_asym_session_get_user_data;
+ rte_cryptodev_asym_session_set_user_data;
+ __rte_cryptodev_trace_asym_session_pool_create;
};
INTERNAL {
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [EXT] [PATCH] crypto: use single buffer for asymmetric session
2021-12-13 15:04 [PATCH] crypto: use single buffer for asymmetric session Ciara Power
@ 2021-12-13 16:33 ` Anoob Joseph
2022-01-20 10:51 ` Power, Ciara
2021-12-19 23:12 ` Ray Kinsella
1 sibling, 1 reply; 4+ messages in thread
From: Anoob Joseph @ 2021-12-13 16:33 UTC (permalink / raw)
To: Ciara Power, dev
Cc: roy.fan.zhang, Akhil Goyal, Declan Doherty, Ankur Dwivedi,
Tejasree Kondoj, John Griffin, Fiona Trahe, Deepak Kumar Jain,
Ray Kinsella
Hi Ciara,
+1 to the overall approach. Few comments inline.
Thanks,
Anoob
> -----Original Message-----
> From: Ciara Power <ciara.power@intel.com>
> Sent: Monday, December 13, 2021 8:34 PM
> To: dev@dpdk.org
> Cc: roy.fan.zhang@intel.com; Akhil Goyal <gakhil@marvell.com>; Ciara
> Power <ciara.power@intel.com>; Declan Doherty
> <declan.doherty@intel.com>; Ankur Dwivedi <adwivedi@marvell.com>;
> Anoob Joseph <anoobj@marvell.com>; Tejasree Kondoj
> <ktejasree@marvell.com>; John Griffin <john.griffin@intel.com>; Fiona
> Trahe <fiona.trahe@intel.com>; Deepak Kumar Jain
> <deepak.k.jain@intel.com>; Ray Kinsella <mdr@ashroe.eu>
> Subject: [EXT] [PATCH] crypto: use single buffer for asymmetric session
>
> External Email
>
> ----------------------------------------------------------------------
> 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>
>
> ---
> Hiding the asym session structure by moving it to an internal header will be
> implemented in a later version of this patch.
> ---
> app/test-crypto-perf/cperf_ops.c | 14 +-
> app/test/test_cryptodev_asym.c | 204 ++++--------------
> 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/octeontx2/otx2_cryptodev_ops.c | 25 +--
> drivers/crypto/openssl/rte_openssl_pmd.c | 5 +-
> drivers/crypto/openssl/rte_openssl_pmd_ops.c | 23 +-
> drivers/crypto/qat/qat_asym.c | 35 +--
> lib/cryptodev/cryptodev_pmd.h | 11 +-
> lib/cryptodev/cryptodev_trace_points.c | 3 +
> lib/cryptodev/rte_cryptodev.c | 199 +++++++++++------
> lib/cryptodev/rte_cryptodev.h | 107 ++++++---
> lib/cryptodev/rte_cryptodev_trace.h | 12 ++
> lib/cryptodev/version.map | 6 +-
> 16 files changed, 302 insertions(+), 394 deletions(-)
>
[snip]
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index 59ea5a54df..11a62bb555 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -919,9 +919,15 @@ 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. */
> + uint8_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_data */
> + uint8_t padding[4];
> + uint8_t sess_private_data[0];
> };
[Anoob] Should we add a uint64_t member to hold IOVA address of, may be, rte_cryptodev_asym_session()? IOVA address could be required for hardware PMDs. And typically rte_mempool_virt2iova() used to help in that. Also, did you consider whether this layout of crypto session can be kept uniform across sym, asym & security? There is no asym specific field in this struct, right?
>
> /**
> @@ -956,6 +962,31 @@ 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 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
> + * constraint for the reserved zone.
> + *
> + * @return
> + * - On success return size of the session
> + * - On failure returns 0
> + */
> +__rte_experimental
> +struct rte_mempool *
> +rte_cryptodev_asym_session_pool_create(const char *name, uint32_t
> nb_elts,
> + uint32_t cache_size, uint16_t user_data_size, int socket_id);
> +
> /**
> * Create symmetric crypto session header (generic with no private data)
> *
> @@ -973,13 +1004,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 @@ -
> 1034,28 +1069,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.
[Anoob] API rte_cryptodev_asym_session_create() returning NULL is treated as an error. But error can be either due to -EINVAL/-ENOMEM/-ENOTSUP, in which -ENOTSUP is typically used by PMD to declare unsupported combinations of xforms. Should we clarify this in the API description?
Also, none of rte_cryptodev_asym_session_create() calls in validation tests consider the API returning NULL due to -ENOTSUP. For sym crypto test cases, API returning -ENOTSUP was used to skip the test. Can you update the tests such that returning NULL would mean test is skipped? Agreed that current code also doesn't handle -ENOTSUP case returned by init API.
> - * - -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,14 +1088,13 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
> struct rte_cryptodev_sym_session *sess);
>
[snip]
> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map index
> c50745fa8c..00b1c9ae35 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;
> @@ -104,6 +103,11 @@ EXPERIMENTAL {
> rte_cryptodev_remove_deq_callback;
> rte_cryptodev_remove_enq_callback;
>
> + # added 22.03
+1 for get & set user_data API. Ideally it should have been a separate series but I agree that it's better getting addressed along with the session rework.
> + rte_cryptodev_asym_session_pool_create;
> + rte_cryptodev_asym_session_get_user_data;
> + rte_cryptodev_asym_session_set_user_data;
> + __rte_cryptodev_trace_asym_session_pool_create;
> };
>
> INTERNAL {
> --
> 2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] crypto: use single buffer for asymmetric session
2021-12-13 15:04 [PATCH] crypto: use single buffer for asymmetric session Ciara Power
2021-12-13 16:33 ` [EXT] " Anoob Joseph
@ 2021-12-19 23:12 ` Ray Kinsella
1 sibling, 0 replies; 4+ messages in thread
From: Ray Kinsella @ 2021-12-19 23:12 UTC (permalink / raw)
To: Ciara Power
Cc: dev, roy.fan.zhang, gakhil, Declan Doherty, Ankur Dwivedi,
Anoob Joseph, Tejasree Kondoj, John Griffin, Fiona Trahe,
Deepak Kumar Jain
Ciara,
One minor niggle.
Ciara Power <ciara.power@intel.com> writes:
> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> index c50745fa8c..00b1c9ae35 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;
> @@ -104,6 +103,11 @@ EXPERIMENTAL {
> rte_cryptodev_remove_deq_callback;
> rte_cryptodev_remove_enq_callback;
>
> + # added 22.03
<niggle>
rte_cryptodev_asym_session_get_user_data should be before
rte_cryptodev_asym_session_pool_create, right?
</niggle>
> + rte_cryptodev_asym_session_pool_create;
> + rte_cryptodev_asym_session_get_user_data;
> + rte_cryptodev_asym_session_set_user_data;
> + __rte_cryptodev_trace_asym_session_pool_create;
> };
>
> INTERNAL {
--
Regards, Ray K
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [EXT] [PATCH] crypto: use single buffer for asymmetric session
2021-12-13 16:33 ` [EXT] " Anoob Joseph
@ 2022-01-20 10:51 ` Power, Ciara
0 siblings, 0 replies; 4+ messages in thread
From: Power, Ciara @ 2022-01-20 10:51 UTC (permalink / raw)
To: Anoob Joseph, dev
Cc: Zhang, Roy Fan, Akhil Goyal, Doherty, Declan, Ankur Dwivedi,
Tejasree Kondoj, Griffin, John, Trahe, Fiona, Jain, Deepak K,
Ray Kinsella
Hi Anoob,
Thanks for the review, apologies for the delay in reply.
Comments inline.
Thanks,
Ciara
>-----Original Message-----
>From: Anoob Joseph <anoobj@marvell.com>
>Sent: Monday 13 December 2021 16:34
>To: Power, Ciara <ciara.power@intel.com>; dev@dpdk.org
>Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Akhil Goyal
><gakhil@marvell.com>; 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>;
>Ray Kinsella <mdr@ashroe.eu>
>Subject: RE: [EXT] [PATCH] crypto: use single buffer for asymmetric session
>
>Hi Ciara,
>
>+1 to the overall approach. Few comments inline.
>
>Thanks,
>Anoob
>
>> -----Original Message-----
>> From: Ciara Power <ciara.power@intel.com>
>> Sent: Monday, December 13, 2021 8:34 PM
>> To: dev@dpdk.org
>> Cc: roy.fan.zhang@intel.com; Akhil Goyal <gakhil@marvell.com>; Ciara
>> Power <ciara.power@intel.com>; Declan Doherty
>> <declan.doherty@intel.com>; Ankur Dwivedi <adwivedi@marvell.com>;
>> Anoob Joseph <anoobj@marvell.com>; Tejasree Kondoj
>> <ktejasree@marvell.com>; John Griffin <john.griffin@intel.com>; Fiona
>> Trahe <fiona.trahe@intel.com>; Deepak Kumar Jain
>> <deepak.k.jain@intel.com>; Ray Kinsella <mdr@ashroe.eu>
>> Subject: [EXT] [PATCH] crypto: use single buffer for asymmetric
>> session
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> 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>
>>
>> ---
>> Hiding the asym session structure by moving it to an internal header
>> will be implemented in a later version of this patch.
>> ---
>> app/test-crypto-perf/cperf_ops.c | 14 +-
>> app/test/test_cryptodev_asym.c | 204 ++++--------------
>> 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/octeontx2/otx2_cryptodev_ops.c | 25 +--
>> drivers/crypto/openssl/rte_openssl_pmd.c | 5 +-
>> drivers/crypto/openssl/rte_openssl_pmd_ops.c | 23 +-
>> drivers/crypto/qat/qat_asym.c | 35 +--
>> lib/cryptodev/cryptodev_pmd.h | 11 +-
>> lib/cryptodev/cryptodev_trace_points.c | 3 +
>> lib/cryptodev/rte_cryptodev.c | 199 +++++++++++------
>> lib/cryptodev/rte_cryptodev.h | 107 ++++++---
>> lib/cryptodev/rte_cryptodev_trace.h | 12 ++
>> lib/cryptodev/version.map | 6 +-
>> 16 files changed, 302 insertions(+), 394 deletions(-)
>>
>
>[snip]
>
>> diff --git a/lib/cryptodev/rte_cryptodev.h
>> b/lib/cryptodev/rte_cryptodev.h index 59ea5a54df..11a62bb555 100644
>> --- a/lib/cryptodev/rte_cryptodev.h
>> +++ b/lib/cryptodev/rte_cryptodev.h
>> @@ -919,9 +919,15 @@ 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. */
>> + uint8_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_data */
>> + uint8_t padding[4];
>> + uint8_t sess_private_data[0];
>> };
>
>[Anoob] Should we add a uint64_t member to hold IOVA address of, may be,
>rte_cryptodev_asym_session()? IOVA address could be required for
>hardware PMDs. And typically rte_mempool_virt2iova() used to help in that.
>Also, did you consider whether this layout of crypto session can be kept
>uniform across sym, asym & security? There is no asym specific field in this
>struct, right?
>
[CP]
Do you think mempool would not be used for session in some cases? I guess the IOVA address would be needed in that case.
Yes, I believe this approach for session could be applicable to the sym and security sessions also, but for now we are only doing the asym session change, but I think they should all be aligned in future releases.
<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.
>
>[Anoob] API rte_cryptodev_asym_session_create() returning NULL is treated
>as an error. But error can be either due to -EINVAL/-ENOMEM/-ENOTSUP, in
>which -ENOTSUP is typically used by PMD to declare unsupported
>combinations of xforms. Should we clarify this in the API description?
>
>Also, none of rte_cryptodev_asym_session_create() calls in validation tests
>consider the API returning NULL due to -ENOTSUP. For sym crypto test cases,
>API returning -ENOTSUP was used to skip the test. Can you update the tests
>such that returning NULL would mean test is skipped? Agreed that current
>code also doesn't handle -ENOTSUP case returned by init API.
>
[CP]
Skipping for NULL return would mean it would also count as skipped for error cases other than -ENOTSUP, which isn't ideal.
In v2 I will change rte_cryptodev_asym_session_create() to return an int value (and just pass in session to be used as input),
this will allow the error returns to be treated correctly.
>> - * - -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,14 +1088,13 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
>> struct rte_cryptodev_sym_session *sess);
>>
>
>[snip]
>
>> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
>> index
>> c50745fa8c..00b1c9ae35 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;
>> @@ -104,6 +103,11 @@ EXPERIMENTAL {
>> rte_cryptodev_remove_deq_callback;
>> rte_cryptodev_remove_enq_callback;
>>
>> + # added 22.03
>
>+1 for get & set user_data API. Ideally it should have been a separate series
>but I agree that it's better getting addressed along with the session rework.
>
[CP]
Agreed, for v2 I have split it into its own patch in this patchset rather than squashing it with other session changes.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-20 10:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-13 15:04 [PATCH] crypto: use single buffer for asymmetric session Ciara Power
2021-12-13 16:33 ` [EXT] " Anoob Joseph
2022-01-20 10:51 ` Power, Ciara
2021-12-19 23:12 ` Ray Kinsella
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).