DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] app/test-crypto-perf: add shared session option
@ 2024-06-04 14:54 Jack Bond-Preston
  2024-06-12 10:38 ` Dooley, Brian
  2024-06-18 15:48 ` [PATCH v2] " Jack Bond-Preston
  0 siblings, 2 replies; 4+ messages in thread
From: Jack Bond-Preston @ 2024-06-04 14:54 UTC (permalink / raw)
  To: Ciara Power; +Cc: dev, Wathsala Vithanage, Paul Szczepanek

Add the option to create one session for the PMD, and share it across
all of the queue pairs. This may help to discover/debug concurrency
issues (both correctness and performance) that can occur when using this
configuration.

Signed-off-by: Jack Bond-Preston <jack.bond-preston@foss.arm.com>
Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
 app/test-crypto-perf/cperf.h                  |  3 ++-
 app/test-crypto-perf/cperf_options.h          |  2 ++
 app/test-crypto-perf/cperf_options_parsing.c  | 12 ++++++++++
 app/test-crypto-perf/cperf_test_latency.c     | 23 +++++++++++++------
 app/test-crypto-perf/cperf_test_latency.h     |  3 ++-
 .../cperf_test_pmd_cyclecount.c               | 21 ++++++++++++-----
 .../cperf_test_pmd_cyclecount.h               |  3 ++-
 app/test-crypto-perf/cperf_test_throughput.c  | 21 ++++++++++++-----
 app/test-crypto-perf/cperf_test_throughput.h  |  3 ++-
 app/test-crypto-perf/cperf_test_verify.c      | 21 ++++++++++++-----
 app/test-crypto-perf/cperf_test_verify.h      |  3 ++-
 app/test-crypto-perf/main.c                   | 23 +++++++++++++++++--
 12 files changed, 106 insertions(+), 32 deletions(-)

diff --git a/app/test-crypto-perf/cperf.h b/app/test-crypto-perf/cperf.h
index db58228dce..06df5d88ad 100644
--- a/app/test-crypto-perf/cperf.h
+++ b/app/test-crypto-perf/cperf.h
@@ -19,7 +19,8 @@ typedef void  *(*cperf_constructor_t)(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *t_vec,
-		const struct cperf_op_fns *op_fns);
+		const struct cperf_op_fns *op_fns,
+		void **sess);
 
 typedef int (*cperf_runner_t)(void *test_ctx);
 typedef void (*cperf_destructor_t)(void *test_ctx);
diff --git a/app/test-crypto-perf/cperf_options.h b/app/test-crypto-perf/cperf_options.h
index be36c70be1..69312a8f7f 100644
--- a/app/test-crypto-perf/cperf_options.h
+++ b/app/test-crypto-perf/cperf_options.h
@@ -27,6 +27,7 @@
 #define CPERF_DEVTYPE		("devtype")
 #define CPERF_OPTYPE		("optype")
 #define CPERF_SESSIONLESS	("sessionless")
+#define CPERF_SHARED_SESSION	("shared-session")
 #define CPERF_OUT_OF_PLACE	("out-of-place")
 #define CPERF_TEST_FILE		("test-file")
 #define CPERF_TEST_NAME		("test-name")
@@ -104,6 +105,7 @@ struct cperf_options {
 	uint16_t nb_qps;
 
 	uint32_t sessionless:1;
+	uint32_t shared_session:1;
 	uint32_t out_of_place:1;
 	uint32_t silent:1;
 	uint32_t csv:1;
diff --git a/app/test-crypto-perf/cperf_options_parsing.c b/app/test-crypto-perf/cperf_options_parsing.c
index 8c20974273..55f858fa1b 100644
--- a/app/test-crypto-perf/cperf_options_parsing.c
+++ b/app/test-crypto-perf/cperf_options_parsing.c
@@ -39,6 +39,7 @@ usage(char *progname)
 		" --optype cipher-only / auth-only / cipher-then-auth / auth-then-cipher /\n"
 		"        aead / pdcp / docsis / ipsec / modex / tls-record : set operation type\n"
 		" --sessionless: enable session-less crypto operations\n"
+		" --shared-session: share 1 session across all queue pairs on crypto device\n"
 		" --out-of-place: enable out-of-place crypto operations\n"
 		" --test-file NAME: set the test vector file path\n"
 		" --test-name NAME: set specific test name section in test file\n"
@@ -508,6 +509,14 @@ parse_sessionless(struct cperf_options *opts,
 	return 0;
 }
 
+static int
+parse_shared_session(struct cperf_options *opts,
+		const char *arg __rte_unused)
+{
+	opts->shared_session = 1;
+	return 0;
+}
+
 static int
 parse_out_of_place(struct cperf_options *opts,
 		const char *arg __rte_unused)
@@ -910,6 +919,7 @@ static struct option lgopts[] = {
 
 	{ CPERF_SILENT, no_argument, 0, 0 },
 	{ CPERF_SESSIONLESS, no_argument, 0, 0 },
+	{ CPERF_SHARED_SESSION, no_argument, 0, 0 },
 	{ CPERF_OUT_OF_PLACE, no_argument, 0, 0 },
 	{ CPERF_TEST_FILE, required_argument, 0, 0 },
 	{ CPERF_TEST_NAME, required_argument, 0, 0 },
@@ -1035,6 +1045,7 @@ cperf_opts_parse_long(int opt_idx, struct cperf_options *opts)
 		{ CPERF_DEVTYPE,	parse_device_type },
 		{ CPERF_OPTYPE,		parse_op_type },
 		{ CPERF_SESSIONLESS,	parse_sessionless },
+		{ CPERF_SHARED_SESSION,	parse_shared_session },
 		{ CPERF_OUT_OF_PLACE,	parse_out_of_place },
 		{ CPERF_IMIX,		parse_imix },
 		{ CPERF_TEST_FILE,	parse_test_file },
@@ -1439,6 +1450,7 @@ cperf_options_dump(struct cperf_options *opts)
 	printf("# number of queue pairs per device: %u\n", opts->nb_qps);
 	printf("# crypto operation: %s\n", cperf_op_type_strs[opts->op_type]);
 	printf("# sessionless: %s\n", opts->sessionless ? "yes" : "no");
+	printf("# shared session: %s\n", opts->shared_session ? "yes" : "no");
 	printf("# out of place: %s\n", opts->out_of_place ? "yes" : "no");
 	if (opts->test == CPERF_TEST_TYPE_PMDCC)
 		printf("# inter-burst delay: %u ms\n", opts->pmdcc_delay);
diff --git a/app/test-crypto-perf/cperf_test_latency.c b/app/test-crypto-perf/cperf_test_latency.c
index b8ad6bf4d4..bbf33fa03d 100644
--- a/app/test-crypto-perf/cperf_test_latency.c
+++ b/app/test-crypto-perf/cperf_test_latency.c
@@ -1,7 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2016-2017 Intel Corporation
  */
-
 #include <rte_malloc.h>
 #include <rte_cycles.h>
 #include <rte_crypto.h>
@@ -25,6 +24,7 @@ struct cperf_latency_ctx {
 	struct rte_mempool *pool;
 
 	void *sess;
+	uint8_t sess_owner;
 
 	cperf_populate_ops_t populate_ops;
 
@@ -46,7 +46,7 @@ cperf_latency_test_free(struct cperf_latency_ctx *ctx)
 	if (ctx == NULL)
 		return;
 
-	if (ctx->sess != NULL) {
+	if (ctx->sess != NULL && ctx->sess_owner) {
 		if (ctx->options->op_type == CPERF_ASYM_MODEX)
 			rte_cryptodev_asym_session_free(ctx->dev_id, ctx->sess);
 #ifdef RTE_LIB_SECURITY
@@ -72,7 +72,8 @@ cperf_latency_test_constructor(struct rte_mempool *sess_mp,
 		uint8_t dev_id, uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *op_fns)
+		const struct cperf_op_fns *op_fns,
+		void **sess)
 {
 	struct cperf_latency_ctx *ctx = NULL;
 	size_t extra_op_priv_size = sizeof(struct priv_op_data);
@@ -93,10 +94,18 @@ cperf_latency_test_constructor(struct rte_mempool *sess_mp,
 		sizeof(struct rte_crypto_sym_op) +
 		sizeof(struct cperf_op_result *);
 
-	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
-			test_vector, iv_offset);
-	if (ctx->sess == NULL)
-		goto err;
+
+	if (*sess != NULL) {
+		ctx->sess = *sess;
+		ctx->sess_owner = false;
+	} else {
+		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options, test_vector,
+			iv_offset);
+		if (ctx->sess == NULL)
+			goto err;
+		*sess = ctx->sess;
+		ctx->sess_owner = true;
+	}
 
 	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id,
 			extra_op_priv_size,
diff --git a/app/test-crypto-perf/cperf_test_latency.h b/app/test-crypto-perf/cperf_test_latency.h
index d3fc3218d7..0f2b61e21f 100644
--- a/app/test-crypto-perf/cperf_test_latency.h
+++ b/app/test-crypto-perf/cperf_test_latency.h
@@ -21,7 +21,8 @@ cperf_latency_test_constructor(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *ops_fn);
+		const struct cperf_op_fns *ops_fn,
+		void **sess);
 
 int
 cperf_latency_test_runner(void *test_ctx);
diff --git a/app/test-crypto-perf/cperf_test_pmd_cyclecount.c b/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
index 7191d99ea4..07a842f40a 100644
--- a/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
+++ b/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
@@ -29,6 +29,7 @@ struct cperf_pmd_cyclecount_ctx {
 	struct rte_crypto_op **ops_processed;
 
 	void *sess;
+	uint8_t sess_owner;
 
 	cperf_populate_ops_t populate_ops;
 
@@ -63,7 +64,7 @@ cperf_pmd_cyclecount_test_free(struct cperf_pmd_cyclecount_ctx *ctx)
 	if (!ctx)
 		return;
 
-	if (ctx->sess) {
+	if (ctx->sess != NULL && ctx->sess_owner) {
 #ifdef RTE_LIB_SECURITY
 		if (ctx->options->op_type == CPERF_PDCP ||
 				ctx->options->op_type == CPERF_DOCSIS) {
@@ -89,7 +90,8 @@ cperf_pmd_cyclecount_test_constructor(struct rte_mempool *sess_mp,
 		uint8_t dev_id, uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *op_fns)
+		const struct cperf_op_fns *op_fns,
+		void **sess)
 {
 	struct cperf_pmd_cyclecount_ctx *ctx = NULL;
 
@@ -112,10 +114,17 @@ cperf_pmd_cyclecount_test_constructor(struct rte_mempool *sess_mp,
 	uint16_t iv_offset = sizeof(struct rte_crypto_op) +
 			sizeof(struct rte_crypto_sym_op);
 
-	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
-			test_vector, iv_offset);
-	if (ctx->sess == NULL)
-		goto err;
+	if (*sess != NULL) {
+		ctx->sess = *sess;
+		ctx->sess_owner = false;
+	} else {
+		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options, test_vector,
+			iv_offset);
+		if (ctx->sess == NULL)
+			goto err;
+		*sess = ctx->sess;
+		ctx->sess_owner = true;
+	}
 
 	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id, 0,
 			&ctx->src_buf_offset, &ctx->dst_buf_offset,
diff --git a/app/test-crypto-perf/cperf_test_pmd_cyclecount.h b/app/test-crypto-perf/cperf_test_pmd_cyclecount.h
index beb4419910..417fdbbed4 100644
--- a/app/test-crypto-perf/cperf_test_pmd_cyclecount.h
+++ b/app/test-crypto-perf/cperf_test_pmd_cyclecount.h
@@ -22,7 +22,8 @@ cperf_pmd_cyclecount_test_constructor(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *ops_fn);
+		const struct cperf_op_fns *ops_fn,
+		void **sess);
 
 int
 cperf_pmd_cyclecount_test_runner(void *test_ctx);
diff --git a/app/test-crypto-perf/cperf_test_throughput.c b/app/test-crypto-perf/cperf_test_throughput.c
index c0891e7c99..54f2f6a060 100644
--- a/app/test-crypto-perf/cperf_test_throughput.c
+++ b/app/test-crypto-perf/cperf_test_throughput.c
@@ -21,6 +21,7 @@ struct cperf_throughput_ctx {
 	struct rte_mempool *pool;
 
 	void *sess;
+	uint8_t sess_owner;
 
 	cperf_populate_ops_t populate_ops;
 
@@ -36,7 +37,7 @@ cperf_throughput_test_free(struct cperf_throughput_ctx *ctx)
 {
 	if (!ctx)
 		return;
-	if (ctx->sess) {
+	if (ctx->sess != NULL && ctx->sess_owner) {
 		if (ctx->options->op_type == CPERF_ASYM_MODEX)
 			rte_cryptodev_asym_session_free(ctx->dev_id,
 					(void *)ctx->sess);
@@ -63,7 +64,8 @@ cperf_throughput_test_constructor(struct rte_mempool *sess_mp,
 		uint8_t dev_id, uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *op_fns)
+		const struct cperf_op_fns *op_fns,
+		void **sess)
 {
 	struct cperf_throughput_ctx *ctx = NULL;
 
@@ -82,10 +84,17 @@ cperf_throughput_test_constructor(struct rte_mempool *sess_mp,
 	uint16_t iv_offset = sizeof(struct rte_crypto_op) +
 		sizeof(struct rte_crypto_sym_op);
 
-	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
-			test_vector, iv_offset);
-	if (ctx->sess == NULL)
-		goto err;
+	if (*sess != NULL) {
+		ctx->sess = *sess;
+		ctx->sess_owner = false;
+	} else {
+		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options, test_vector,
+			iv_offset);
+		if (ctx->sess == NULL)
+			goto err;
+		*sess = ctx->sess;
+		ctx->sess_owner = true;
+	}
 
 	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id, 0,
 			&ctx->src_buf_offset, &ctx->dst_buf_offset,
diff --git a/app/test-crypto-perf/cperf_test_throughput.h b/app/test-crypto-perf/cperf_test_throughput.h
index 439ec8e559..d9011b6a51 100644
--- a/app/test-crypto-perf/cperf_test_throughput.h
+++ b/app/test-crypto-perf/cperf_test_throughput.h
@@ -22,7 +22,8 @@ cperf_throughput_test_constructor(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *ops_fn);
+		const struct cperf_op_fns *ops_fn,
+		void **sess);
 
 int
 cperf_throughput_test_runner(void *test_ctx);
diff --git a/app/test-crypto-perf/cperf_test_verify.c b/app/test-crypto-perf/cperf_test_verify.c
index 222c7a1cd8..7f6a244794 100644
--- a/app/test-crypto-perf/cperf_test_verify.c
+++ b/app/test-crypto-perf/cperf_test_verify.c
@@ -21,6 +21,7 @@ struct cperf_verify_ctx {
 	struct rte_mempool *pool;
 
 	void *sess;
+	uint8_t sess_owner;
 
 	cperf_populate_ops_t populate_ops;
 
@@ -41,7 +42,7 @@ cperf_verify_test_free(struct cperf_verify_ctx *ctx)
 	if (ctx == NULL)
 		return;
 
-	if (ctx->sess != NULL) {
+	if (ctx->sess != NULL && ctx->sess_owner) {
 		if (ctx->options->op_type == CPERF_ASYM_MODEX)
 			rte_cryptodev_asym_session_free(ctx->dev_id, ctx->sess);
 #ifdef RTE_LIB_SECURITY
@@ -67,7 +68,8 @@ cperf_verify_test_constructor(struct rte_mempool *sess_mp,
 		uint8_t dev_id, uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *op_fns)
+		const struct cperf_op_fns *op_fns,
+		void **sess)
 {
 	struct cperf_verify_ctx *ctx = NULL;
 
@@ -86,10 +88,17 @@ cperf_verify_test_constructor(struct rte_mempool *sess_mp,
 	uint16_t iv_offset = sizeof(struct rte_crypto_op) +
 		sizeof(struct rte_crypto_sym_op);
 
-	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
-			test_vector, iv_offset);
-	if (ctx->sess == NULL)
-		goto err;
+	if (*sess != NULL) {
+		ctx->sess = *sess;
+		ctx->sess_owner = false;
+	} else {
+		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
+				test_vector, iv_offset);
+		if (ctx->sess == NULL)
+			goto err;
+		*sess = ctx->sess;
+		ctx->sess_owner = true;
+	}
 
 	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id, 0,
 			&ctx->src_buf_offset, &ctx->dst_buf_offset,
diff --git a/app/test-crypto-perf/cperf_test_verify.h b/app/test-crypto-perf/cperf_test_verify.h
index 9f70ad87ba..dcc10e6e98 100644
--- a/app/test-crypto-perf/cperf_test_verify.h
+++ b/app/test-crypto-perf/cperf_test_verify.h
@@ -22,7 +22,8 @@ cperf_verify_test_constructor(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *ops_fn);
+		const struct cperf_op_fns *ops_fn,
+		void **sess);
 
 int
 cperf_verify_test_runner(void *test_ctx);
diff --git a/app/test-crypto-perf/main.c b/app/test-crypto-perf/main.c
index 40c0b4b54f..312c9dcc96 100644
--- a/app/test-crypto-perf/main.c
+++ b/app/test-crypto-perf/main.c
@@ -647,6 +647,9 @@ main(int argc, char **argv)
 
 	i = 0;
 	uint8_t qp_id = 0, cdev_index = 0;
+
+	void *sess = NULL;
+
 	RTE_LCORE_FOREACH_WORKER(lcore_id) {
 
 		if (i == total_nb_qps)
@@ -663,14 +666,30 @@ main(int argc, char **argv)
 		ctx[i] = cperf_testmap[opts.test].constructor(
 				session_pool_socket[socket_id].sess_mp,
 				cdev_id, qp_id,
-				&opts, t_vec, &op_fns);
+				&opts, t_vec, &op_fns, &sess);
+
+		/*
+		 * If sess was NULL, the constructor will have set it to a newly
+		 * created session. This means future calls to constructors will
+		 * provide this session, sharing it across all qps. If session
+		 * sharing is not enabled, re-set sess to NULL, to prevent this.
+		 */
+		if (!opts.shared_session)
+			sess = NULL;
+
 		if (ctx[i] == NULL) {
 			RTE_LOG(ERR, USER1, "Test run constructor failed\n");
 			goto err;
 		}
+
 		qp_id = (qp_id + 1) % opts.nb_qps;
-		if (qp_id == 0)
+		if (qp_id == 0) {
 			cdev_index++;
+			/* If next qp is on a new cdev, don't share the session
+			 * - it shouldn't be shared across different cdevs.
+			 */
+			sess = NULL;
+		}
 		i++;
 	}
 
-- 
2.34.1


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

* RE: [PATCH] app/test-crypto-perf: add shared session option
  2024-06-04 14:54 [PATCH] app/test-crypto-perf: add shared session option Jack Bond-Preston
@ 2024-06-12 10:38 ` Dooley, Brian
  2024-06-18 15:50   ` Jack Bond-Preston
  2024-06-18 15:48 ` [PATCH v2] " Jack Bond-Preston
  1 sibling, 1 reply; 4+ messages in thread
From: Dooley, Brian @ 2024-06-12 10:38 UTC (permalink / raw)
  To: Jack Bond-Preston, Ciara Power; +Cc: dev, Wathsala Vithanage, Paul Szczepanek

Hi Jack,

> -----Original Message-----
> From: Jack Bond-Preston <jack.bond-preston@foss.arm.com>
> Sent: Tuesday, June 4, 2024 3:55 PM
> To: Ciara Power <ciara.power@intel.com>
> Cc: dev@dpdk.org; Wathsala Vithanage <wathsala.vithanage@arm.com>; Paul
> Szczepanek <paul.szczepanek@arm.com>
> Subject: [PATCH] app/test-crypto-perf: add shared session option
> 
> Add the option to create one session for the PMD, and share it across all of the
> queue pairs. This may help to discover/debug concurrency issues (both
> correctness and performance) that can occur when using this configuration.
> 
> Signed-off-by: Jack Bond-Preston <jack.bond-preston@foss.arm.com>
> Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
>  app/test-crypto-perf/cperf.h                  |  3 ++-
>  app/test-crypto-perf/cperf_options.h          |  2 ++
>  app/test-crypto-perf/cperf_options_parsing.c  | 12 ++++++++++
>  app/test-crypto-perf/cperf_test_latency.c     | 23 +++++++++++++------
>  app/test-crypto-perf/cperf_test_latency.h     |  3 ++-
>  .../cperf_test_pmd_cyclecount.c               | 21 ++++++++++++-----
>  .../cperf_test_pmd_cyclecount.h               |  3 ++-
>  app/test-crypto-perf/cperf_test_throughput.c  | 21 ++++++++++++-----
> app/test-crypto-perf/cperf_test_throughput.h  |  3 ++-
>  app/test-crypto-perf/cperf_test_verify.c      | 21 ++++++++++++-----
>  app/test-crypto-perf/cperf_test_verify.h      |  3 ++-
>  app/test-crypto-perf/main.c                   | 23 +++++++++++++++++--
>  12 files changed, 106 insertions(+), 32 deletions(-)
> 
> diff --git a/app/test-crypto-perf/cperf.h b/app/test-crypto-perf/cperf.h index
> db58228dce..06df5d88ad 100644
> --- a/app/test-crypto-perf/cperf.h
> +++ b/app/test-crypto-perf/cperf.h
> @@ -19,7 +19,8 @@ typedef void  *(*cperf_constructor_t)(
>  		uint16_t qp_id,
>  		const struct cperf_options *options,
>  		const struct cperf_test_vector *t_vec,
> -		const struct cperf_op_fns *op_fns);
> +		const struct cperf_op_fns *op_fns,
> +		void **sess);
> 
>  typedef int (*cperf_runner_t)(void *test_ctx);  typedef void
> (*cperf_destructor_t)(void *test_ctx); diff --git a/app/test-crypto-
> perf/cperf_options.h b/app/test-crypto-perf/cperf_options.h
> index be36c70be1..69312a8f7f 100644
> --- a/app/test-crypto-perf/cperf_options.h
> +++ b/app/test-crypto-perf/cperf_options.h
> @@ -27,6 +27,7 @@
>  #define CPERF_DEVTYPE		("devtype")
>  #define CPERF_OPTYPE		("optype")
>  #define CPERF_SESSIONLESS	("sessionless")
> +#define CPERF_SHARED_SESSION	("shared-session")
>  #define CPERF_OUT_OF_PLACE	("out-of-place")
>  #define CPERF_TEST_FILE		("test-file")
>  #define CPERF_TEST_NAME		("test-name")
> @@ -104,6 +105,7 @@ struct cperf_options {
>  	uint16_t nb_qps;
> 
>  	uint32_t sessionless:1;
> +	uint32_t shared_session:1;
>  	uint32_t out_of_place:1;
>  	uint32_t silent:1;
>  	uint32_t csv:1;
> diff --git a/app/test-crypto-perf/cperf_options_parsing.c b/app/test-crypto-
> perf/cperf_options_parsing.c
> index 8c20974273..55f858fa1b 100644
> --- a/app/test-crypto-perf/cperf_options_parsing.c
> +++ b/app/test-crypto-perf/cperf_options_parsing.c
> @@ -39,6 +39,7 @@ usage(char *progname)
>  		" --optype cipher-only / auth-only / cipher-then-auth / auth-
> then-cipher /\n"
>  		"        aead / pdcp / docsis / ipsec / modex / tls-record : set
> operation type\n"
>  		" --sessionless: enable session-less crypto operations\n"
> +		" --shared-session: share 1 session across all queue pairs on
> crypto device\n"
>  		" --out-of-place: enable out-of-place crypto operations\n"
>  		" --test-file NAME: set the test vector file path\n"
>  		" --test-name NAME: set specific test name section in test
> file\n"
> @@ -508,6 +509,14 @@ parse_sessionless(struct cperf_options *opts,
>  	return 0;
>  }
> 
> +static int
> +parse_shared_session(struct cperf_options *opts,
> +		const char *arg __rte_unused)
> +{
> +	opts->shared_session = 1;
> +	return 0;
> +}
> +
>  static int
>  parse_out_of_place(struct cperf_options *opts,
>  		const char *arg __rte_unused)
> @@ -910,6 +919,7 @@ static struct option lgopts[] = {
> 
>  	{ CPERF_SILENT, no_argument, 0, 0 },
>  	{ CPERF_SESSIONLESS, no_argument, 0, 0 },
> +	{ CPERF_SHARED_SESSION, no_argument, 0, 0 },
>  	{ CPERF_OUT_OF_PLACE, no_argument, 0, 0 },
>  	{ CPERF_TEST_FILE, required_argument, 0, 0 },
>  	{ CPERF_TEST_NAME, required_argument, 0, 0 }, @@ -1035,6
> +1045,7 @@ cperf_opts_parse_long(int opt_idx, struct cperf_options *opts)
>  		{ CPERF_DEVTYPE,	parse_device_type },
>  		{ CPERF_OPTYPE,		parse_op_type },
>  		{ CPERF_SESSIONLESS,	parse_sessionless },
> +		{ CPERF_SHARED_SESSION,	parse_shared_session },
>  		{ CPERF_OUT_OF_PLACE,	parse_out_of_place },
>  		{ CPERF_IMIX,		parse_imix },
>  		{ CPERF_TEST_FILE,	parse_test_file },
> @@ -1439,6 +1450,7 @@ cperf_options_dump(struct cperf_options *opts)
>  	printf("# number of queue pairs per device: %u\n", opts->nb_qps);
>  	printf("# crypto operation: %s\n", cperf_op_type_strs[opts-
> >op_type]);
>  	printf("# sessionless: %s\n", opts->sessionless ? "yes" : "no");
> +	printf("# shared session: %s\n", opts->shared_session ? "yes" : "no");
>  	printf("# out of place: %s\n", opts->out_of_place ? "yes" : "no");
>  	if (opts->test == CPERF_TEST_TYPE_PMDCC)
>  		printf("# inter-burst delay: %u ms\n", opts->pmdcc_delay);
> diff --git a/app/test-crypto-perf/cperf_test_latency.c b/app/test-crypto-
> perf/cperf_test_latency.c
> index b8ad6bf4d4..bbf33fa03d 100644
> --- a/app/test-crypto-perf/cperf_test_latency.c
> +++ b/app/test-crypto-perf/cperf_test_latency.c
> @@ -1,7 +1,6 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
>   * Copyright(c) 2016-2017 Intel Corporation
>   */
> -
>  #include <rte_malloc.h>
>  #include <rte_cycles.h>
>  #include <rte_crypto.h>
> @@ -25,6 +24,7 @@ struct cperf_latency_ctx {
>  	struct rte_mempool *pool;
> 
>  	void *sess;
> +	uint8_t sess_owner;
> 
>  	cperf_populate_ops_t populate_ops;
> 
> @@ -46,7 +46,7 @@ cperf_latency_test_free(struct cperf_latency_ctx *ctx)
>  	if (ctx == NULL)
>  		return;
> 
> -	if (ctx->sess != NULL) {
> +	if (ctx->sess != NULL && ctx->sess_owner) {
>  		if (ctx->options->op_type == CPERF_ASYM_MODEX)
>  			rte_cryptodev_asym_session_free(ctx->dev_id, ctx-
> >sess);  #ifdef RTE_LIB_SECURITY @@ -72,7 +72,8 @@
> cperf_latency_test_constructor(struct rte_mempool *sess_mp,
>  		uint8_t dev_id, uint16_t qp_id,
>  		const struct cperf_options *options,
>  		const struct cperf_test_vector *test_vector,
> -		const struct cperf_op_fns *op_fns)
> +		const struct cperf_op_fns *op_fns,
> +		void **sess)
>  {
>  	struct cperf_latency_ctx *ctx = NULL;
>  	size_t extra_op_priv_size = sizeof(struct priv_op_data); @@ -93,10
> +94,18 @@ cperf_latency_test_constructor(struct rte_mempool *sess_mp,
>  		sizeof(struct rte_crypto_sym_op) +
>  		sizeof(struct cperf_op_result *);
> 
> -	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
> -			test_vector, iv_offset);
> -	if (ctx->sess == NULL)
> -		goto err;
> +
> +	if (*sess != NULL) {
> +		ctx->sess = *sess;
> +		ctx->sess_owner = false;
> +	} else {
> +		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
> test_vector,
> +			iv_offset);
> +		if (ctx->sess == NULL)
> +			goto err;
> +		*sess = ctx->sess;
> +		ctx->sess_owner = true;
> +	}
> 
>  	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id,
>  			extra_op_priv_size,
> diff --git a/app/test-crypto-perf/cperf_test_latency.h b/app/test-crypto-
> perf/cperf_test_latency.h
> index d3fc3218d7..0f2b61e21f 100644
> --- a/app/test-crypto-perf/cperf_test_latency.h
> +++ b/app/test-crypto-perf/cperf_test_latency.h
> @@ -21,7 +21,8 @@ cperf_latency_test_constructor(
>  		uint16_t qp_id,
>  		const struct cperf_options *options,
>  		const struct cperf_test_vector *test_vector,
> -		const struct cperf_op_fns *ops_fn);
> +		const struct cperf_op_fns *ops_fn,
> +		void **sess);
> 
>  int
>  cperf_latency_test_runner(void *test_ctx); diff --git a/app/test-crypto-
> perf/cperf_test_pmd_cyclecount.c b/app/test-crypto-
> perf/cperf_test_pmd_cyclecount.c
> index 7191d99ea4..07a842f40a 100644
> --- a/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
> +++ b/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
> @@ -29,6 +29,7 @@ struct cperf_pmd_cyclecount_ctx {
>  	struct rte_crypto_op **ops_processed;
> 
>  	void *sess;
> +	uint8_t sess_owner;
> 
>  	cperf_populate_ops_t populate_ops;
> 
> @@ -63,7 +64,7 @@ cperf_pmd_cyclecount_test_free(struct
> cperf_pmd_cyclecount_ctx *ctx)
>  	if (!ctx)
>  		return;
> 
> -	if (ctx->sess) {
> +	if (ctx->sess != NULL && ctx->sess_owner) {
>  #ifdef RTE_LIB_SECURITY
>  		if (ctx->options->op_type == CPERF_PDCP ||
>  				ctx->options->op_type == CPERF_DOCSIS) {
> @@ -89,7 +90,8 @@ cperf_pmd_cyclecount_test_constructor(struct
> rte_mempool *sess_mp,
>  		uint8_t dev_id, uint16_t qp_id,
>  		const struct cperf_options *options,
>  		const struct cperf_test_vector *test_vector,
> -		const struct cperf_op_fns *op_fns)
> +		const struct cperf_op_fns *op_fns,
> +		void **sess)
>  {
>  	struct cperf_pmd_cyclecount_ctx *ctx = NULL;
> 
> @@ -112,10 +114,17 @@ cperf_pmd_cyclecount_test_constructor(struct
> rte_mempool *sess_mp,
>  	uint16_t iv_offset = sizeof(struct rte_crypto_op) +
>  			sizeof(struct rte_crypto_sym_op);
> 
> -	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
> -			test_vector, iv_offset);
> -	if (ctx->sess == NULL)
> -		goto err;
> +	if (*sess != NULL) {
> +		ctx->sess = *sess;
> +		ctx->sess_owner = false;
> +	} else {
> +		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
> test_vector,
> +			iv_offset);
> +		if (ctx->sess == NULL)
> +			goto err;
> +		*sess = ctx->sess;
> +		ctx->sess_owner = true;
> +	}
> 
>  	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id,
> 0,
>  			&ctx->src_buf_offset, &ctx->dst_buf_offset, diff --git
> a/app/test-crypto-perf/cperf_test_pmd_cyclecount.h b/app/test-crypto-
> perf/cperf_test_pmd_cyclecount.h
> index beb4419910..417fdbbed4 100644
> --- a/app/test-crypto-perf/cperf_test_pmd_cyclecount.h
> +++ b/app/test-crypto-perf/cperf_test_pmd_cyclecount.h
> @@ -22,7 +22,8 @@ cperf_pmd_cyclecount_test_constructor(
>  		uint16_t qp_id,
>  		const struct cperf_options *options,
>  		const struct cperf_test_vector *test_vector,
> -		const struct cperf_op_fns *ops_fn);
> +		const struct cperf_op_fns *ops_fn,
> +		void **sess);
> 
>  int
>  cperf_pmd_cyclecount_test_runner(void *test_ctx); diff --git a/app/test-
> crypto-perf/cperf_test_throughput.c b/app/test-crypto-
> perf/cperf_test_throughput.c
> index c0891e7c99..54f2f6a060 100644
> --- a/app/test-crypto-perf/cperf_test_throughput.c
> +++ b/app/test-crypto-perf/cperf_test_throughput.c
> @@ -21,6 +21,7 @@ struct cperf_throughput_ctx {
>  	struct rte_mempool *pool;
> 
>  	void *sess;
> +	uint8_t sess_owner;
> 
>  	cperf_populate_ops_t populate_ops;
> 
> @@ -36,7 +37,7 @@ cperf_throughput_test_free(struct
> cperf_throughput_ctx *ctx)  {
>  	if (!ctx)
>  		return;
> -	if (ctx->sess) {
> +	if (ctx->sess != NULL && ctx->sess_owner) {
>  		if (ctx->options->op_type == CPERF_ASYM_MODEX)
>  			rte_cryptodev_asym_session_free(ctx->dev_id,
>  					(void *)ctx->sess);
> @@ -63,7 +64,8 @@ cperf_throughput_test_constructor(struct
> rte_mempool *sess_mp,
>  		uint8_t dev_id, uint16_t qp_id,
>  		const struct cperf_options *options,
>  		const struct cperf_test_vector *test_vector,
> -		const struct cperf_op_fns *op_fns)
> +		const struct cperf_op_fns *op_fns,
> +		void **sess)
>  {
>  	struct cperf_throughput_ctx *ctx = NULL;
> 
> @@ -82,10 +84,17 @@ cperf_throughput_test_constructor(struct
> rte_mempool *sess_mp,
>  	uint16_t iv_offset = sizeof(struct rte_crypto_op) +
>  		sizeof(struct rte_crypto_sym_op);
> 
> -	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
> -			test_vector, iv_offset);
> -	if (ctx->sess == NULL)
> -		goto err;
> +	if (*sess != NULL) {
> +		ctx->sess = *sess;
> +		ctx->sess_owner = false;
> +	} else {
> +		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
> test_vector,
> +			iv_offset);
> +		if (ctx->sess == NULL)
> +			goto err;
> +		*sess = ctx->sess;
> +		ctx->sess_owner = true;
> +	}
> 
>  	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id,
> 0,
>  			&ctx->src_buf_offset, &ctx->dst_buf_offset, diff --git
> a/app/test-crypto-perf/cperf_test_throughput.h b/app/test-crypto-
> perf/cperf_test_throughput.h
> index 439ec8e559..d9011b6a51 100644
> --- a/app/test-crypto-perf/cperf_test_throughput.h
> +++ b/app/test-crypto-perf/cperf_test_throughput.h
> @@ -22,7 +22,8 @@ cperf_throughput_test_constructor(
>  		uint16_t qp_id,
>  		const struct cperf_options *options,
>  		const struct cperf_test_vector *test_vector,
> -		const struct cperf_op_fns *ops_fn);
> +		const struct cperf_op_fns *ops_fn,
> +		void **sess);
> 
>  int
>  cperf_throughput_test_runner(void *test_ctx); diff --git a/app/test-crypto-
> perf/cperf_test_verify.c b/app/test-crypto-perf/cperf_test_verify.c
> index 222c7a1cd8..7f6a244794 100644
> --- a/app/test-crypto-perf/cperf_test_verify.c
> +++ b/app/test-crypto-perf/cperf_test_verify.c
> @@ -21,6 +21,7 @@ struct cperf_verify_ctx {
>  	struct rte_mempool *pool;
> 
>  	void *sess;
> +	uint8_t sess_owner;
> 
>  	cperf_populate_ops_t populate_ops;
> 
> @@ -41,7 +42,7 @@ cperf_verify_test_free(struct cperf_verify_ctx *ctx)
>  	if (ctx == NULL)
>  		return;
> 
> -	if (ctx->sess != NULL) {
> +	if (ctx->sess != NULL && ctx->sess_owner) {
>  		if (ctx->options->op_type == CPERF_ASYM_MODEX)
>  			rte_cryptodev_asym_session_free(ctx->dev_id, ctx-
> >sess);  #ifdef RTE_LIB_SECURITY @@ -67,7 +68,8 @@
> cperf_verify_test_constructor(struct rte_mempool *sess_mp,
>  		uint8_t dev_id, uint16_t qp_id,
>  		const struct cperf_options *options,
>  		const struct cperf_test_vector *test_vector,
> -		const struct cperf_op_fns *op_fns)
> +		const struct cperf_op_fns *op_fns,
> +		void **sess)
>  {
>  	struct cperf_verify_ctx *ctx = NULL;
> 
> @@ -86,10 +88,17 @@ cperf_verify_test_constructor(struct rte_mempool
> *sess_mp,
>  	uint16_t iv_offset = sizeof(struct rte_crypto_op) +
>  		sizeof(struct rte_crypto_sym_op);
> 
> -	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
> -			test_vector, iv_offset);
> -	if (ctx->sess == NULL)
> -		goto err;
> +	if (*sess != NULL) {
> +		ctx->sess = *sess;
> +		ctx->sess_owner = false;
> +	} else {
> +		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
> +				test_vector, iv_offset);
> +		if (ctx->sess == NULL)
> +			goto err;
> +		*sess = ctx->sess;
> +		ctx->sess_owner = true;
> +	}
> 
>  	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id,
> 0,
>  			&ctx->src_buf_offset, &ctx->dst_buf_offset, diff --git
> a/app/test-crypto-perf/cperf_test_verify.h b/app/test-crypto-
> perf/cperf_test_verify.h
> index 9f70ad87ba..dcc10e6e98 100644
> --- a/app/test-crypto-perf/cperf_test_verify.h
> +++ b/app/test-crypto-perf/cperf_test_verify.h
> @@ -22,7 +22,8 @@ cperf_verify_test_constructor(
>  		uint16_t qp_id,
>  		const struct cperf_options *options,
>  		const struct cperf_test_vector *test_vector,
> -		const struct cperf_op_fns *ops_fn);
> +		const struct cperf_op_fns *ops_fn,
> +		void **sess);
> 
>  int
>  cperf_verify_test_runner(void *test_ctx); diff --git a/app/test-crypto-
> perf/main.c b/app/test-crypto-perf/main.c index 40c0b4b54f..312c9dcc96
> 100644
> --- a/app/test-crypto-perf/main.c
> +++ b/app/test-crypto-perf/main.c
> @@ -647,6 +647,9 @@ main(int argc, char **argv)
> 
>  	i = 0;
>  	uint8_t qp_id = 0, cdev_index = 0;
> +
> +	void *sess = NULL;
> +
>  	RTE_LCORE_FOREACH_WORKER(lcore_id) {
> 
>  		if (i == total_nb_qps)
> @@ -663,14 +666,30 @@ main(int argc, char **argv)
>  		ctx[i] = cperf_testmap[opts.test].constructor(
>  				session_pool_socket[socket_id].sess_mp,
>  				cdev_id, qp_id,
> -				&opts, t_vec, &op_fns);
> +				&opts, t_vec, &op_fns, &sess);
> +
> +		/*
> +		 * If sess was NULL, the constructor will have set it to a newly
> +		 * created session. This means future calls to constructors will
> +		 * provide this session, sharing it across all qps. If session
> +		 * sharing is not enabled, re-set sess to NULL, to prevent this.
> +		 */
> +		if (!opts.shared_session)
> +			sess = NULL;
> +
>  		if (ctx[i] == NULL) {
>  			RTE_LOG(ERR, USER1, "Test run constructor
> failed\n");
>  			goto err;
>  		}
> +
>  		qp_id = (qp_id + 1) % opts.nb_qps;
> -		if (qp_id == 0)
> +		if (qp_id == 0) {
>  			cdev_index++;
> +			/* If next qp is on a new cdev, don't share the session
> +			 * - it shouldn't be shared across different cdevs.
> +			 */
> +			sess = NULL;
> +		}
>  		i++;
>  	}
> 
> --
> 2.34.1

Would be good to add a doc update too. Thanks.

Acked-by: Brian Dooley <brian.dooley@intel.com>


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

* [PATCH v2] app/test-crypto-perf: add shared session option
  2024-06-04 14:54 [PATCH] app/test-crypto-perf: add shared session option Jack Bond-Preston
  2024-06-12 10:38 ` Dooley, Brian
@ 2024-06-18 15:48 ` Jack Bond-Preston
  1 sibling, 0 replies; 4+ messages in thread
From: Jack Bond-Preston @ 2024-06-18 15:48 UTC (permalink / raw)
  To: Ciara Power; +Cc: dev, Brian Dooley, Wathsala Vithanage, Paul Szczepanek

Add the option to create one session for the PMD, and share it across
all of the queue pairs. This may help to discover/debug concurrency
issues (both correctness and performance) that can occur when using this
configuration.

Signed-off-by: Jack Bond-Preston <jack.bond-preston@foss.arm.com>
Acked-by: Brian Dooley <brian.dooley@intel.com>
Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
v2:
 * Added docs entry for the new option.

 app/test-crypto-perf/cperf.h                  |  3 ++-
 app/test-crypto-perf/cperf_options.h          |  2 ++
 app/test-crypto-perf/cperf_options_parsing.c  | 12 ++++++++++
 app/test-crypto-perf/cperf_test_latency.c     | 23 +++++++++++++------
 app/test-crypto-perf/cperf_test_latency.h     |  3 ++-
 .../cperf_test_pmd_cyclecount.c               | 21 ++++++++++++-----
 .../cperf_test_pmd_cyclecount.h               |  3 ++-
 app/test-crypto-perf/cperf_test_throughput.c  | 21 ++++++++++++-----
 app/test-crypto-perf/cperf_test_throughput.h  |  3 ++-
 app/test-crypto-perf/cperf_test_verify.c      | 21 ++++++++++++-----
 app/test-crypto-perf/cperf_test_verify.h      |  3 ++-
 app/test-crypto-perf/main.c                   | 23 +++++++++++++++++--
 doc/guides/tools/cryptoperf.rst               |  9 +++++++-
 13 files changed, 114 insertions(+), 33 deletions(-)

diff --git a/app/test-crypto-perf/cperf.h b/app/test-crypto-perf/cperf.h
index db58228dce..06df5d88ad 100644
--- a/app/test-crypto-perf/cperf.h
+++ b/app/test-crypto-perf/cperf.h
@@ -19,7 +19,8 @@ typedef void  *(*cperf_constructor_t)(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *t_vec,
-		const struct cperf_op_fns *op_fns);
+		const struct cperf_op_fns *op_fns,
+		void **sess);
 
 typedef int (*cperf_runner_t)(void *test_ctx);
 typedef void (*cperf_destructor_t)(void *test_ctx);
diff --git a/app/test-crypto-perf/cperf_options.h b/app/test-crypto-perf/cperf_options.h
index be36c70be1..69312a8f7f 100644
--- a/app/test-crypto-perf/cperf_options.h
+++ b/app/test-crypto-perf/cperf_options.h
@@ -27,6 +27,7 @@
 #define CPERF_DEVTYPE		("devtype")
 #define CPERF_OPTYPE		("optype")
 #define CPERF_SESSIONLESS	("sessionless")
+#define CPERF_SHARED_SESSION	("shared-session")
 #define CPERF_OUT_OF_PLACE	("out-of-place")
 #define CPERF_TEST_FILE		("test-file")
 #define CPERF_TEST_NAME		("test-name")
@@ -104,6 +105,7 @@ struct cperf_options {
 	uint16_t nb_qps;
 
 	uint32_t sessionless:1;
+	uint32_t shared_session:1;
 	uint32_t out_of_place:1;
 	uint32_t silent:1;
 	uint32_t csv:1;
diff --git a/app/test-crypto-perf/cperf_options_parsing.c b/app/test-crypto-perf/cperf_options_parsing.c
index 8c20974273..55f858fa1b 100644
--- a/app/test-crypto-perf/cperf_options_parsing.c
+++ b/app/test-crypto-perf/cperf_options_parsing.c
@@ -39,6 +39,7 @@ usage(char *progname)
 		" --optype cipher-only / auth-only / cipher-then-auth / auth-then-cipher /\n"
 		"        aead / pdcp / docsis / ipsec / modex / tls-record : set operation type\n"
 		" --sessionless: enable session-less crypto operations\n"
+		" --shared-session: share 1 session across all queue pairs on crypto device\n"
 		" --out-of-place: enable out-of-place crypto operations\n"
 		" --test-file NAME: set the test vector file path\n"
 		" --test-name NAME: set specific test name section in test file\n"
@@ -508,6 +509,14 @@ parse_sessionless(struct cperf_options *opts,
 	return 0;
 }
 
+static int
+parse_shared_session(struct cperf_options *opts,
+		const char *arg __rte_unused)
+{
+	opts->shared_session = 1;
+	return 0;
+}
+
 static int
 parse_out_of_place(struct cperf_options *opts,
 		const char *arg __rte_unused)
@@ -910,6 +919,7 @@ static struct option lgopts[] = {
 
 	{ CPERF_SILENT, no_argument, 0, 0 },
 	{ CPERF_SESSIONLESS, no_argument, 0, 0 },
+	{ CPERF_SHARED_SESSION, no_argument, 0, 0 },
 	{ CPERF_OUT_OF_PLACE, no_argument, 0, 0 },
 	{ CPERF_TEST_FILE, required_argument, 0, 0 },
 	{ CPERF_TEST_NAME, required_argument, 0, 0 },
@@ -1035,6 +1045,7 @@ cperf_opts_parse_long(int opt_idx, struct cperf_options *opts)
 		{ CPERF_DEVTYPE,	parse_device_type },
 		{ CPERF_OPTYPE,		parse_op_type },
 		{ CPERF_SESSIONLESS,	parse_sessionless },
+		{ CPERF_SHARED_SESSION,	parse_shared_session },
 		{ CPERF_OUT_OF_PLACE,	parse_out_of_place },
 		{ CPERF_IMIX,		parse_imix },
 		{ CPERF_TEST_FILE,	parse_test_file },
@@ -1439,6 +1450,7 @@ cperf_options_dump(struct cperf_options *opts)
 	printf("# number of queue pairs per device: %u\n", opts->nb_qps);
 	printf("# crypto operation: %s\n", cperf_op_type_strs[opts->op_type]);
 	printf("# sessionless: %s\n", opts->sessionless ? "yes" : "no");
+	printf("# shared session: %s\n", opts->shared_session ? "yes" : "no");
 	printf("# out of place: %s\n", opts->out_of_place ? "yes" : "no");
 	if (opts->test == CPERF_TEST_TYPE_PMDCC)
 		printf("# inter-burst delay: %u ms\n", opts->pmdcc_delay);
diff --git a/app/test-crypto-perf/cperf_test_latency.c b/app/test-crypto-perf/cperf_test_latency.c
index b8ad6bf4d4..bbf33fa03d 100644
--- a/app/test-crypto-perf/cperf_test_latency.c
+++ b/app/test-crypto-perf/cperf_test_latency.c
@@ -1,7 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2016-2017 Intel Corporation
  */
-
 #include <rte_malloc.h>
 #include <rte_cycles.h>
 #include <rte_crypto.h>
@@ -25,6 +24,7 @@ struct cperf_latency_ctx {
 	struct rte_mempool *pool;
 
 	void *sess;
+	uint8_t sess_owner;
 
 	cperf_populate_ops_t populate_ops;
 
@@ -46,7 +46,7 @@ cperf_latency_test_free(struct cperf_latency_ctx *ctx)
 	if (ctx == NULL)
 		return;
 
-	if (ctx->sess != NULL) {
+	if (ctx->sess != NULL && ctx->sess_owner) {
 		if (ctx->options->op_type == CPERF_ASYM_MODEX)
 			rte_cryptodev_asym_session_free(ctx->dev_id, ctx->sess);
 #ifdef RTE_LIB_SECURITY
@@ -72,7 +72,8 @@ cperf_latency_test_constructor(struct rte_mempool *sess_mp,
 		uint8_t dev_id, uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *op_fns)
+		const struct cperf_op_fns *op_fns,
+		void **sess)
 {
 	struct cperf_latency_ctx *ctx = NULL;
 	size_t extra_op_priv_size = sizeof(struct priv_op_data);
@@ -93,10 +94,18 @@ cperf_latency_test_constructor(struct rte_mempool *sess_mp,
 		sizeof(struct rte_crypto_sym_op) +
 		sizeof(struct cperf_op_result *);
 
-	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
-			test_vector, iv_offset);
-	if (ctx->sess == NULL)
-		goto err;
+
+	if (*sess != NULL) {
+		ctx->sess = *sess;
+		ctx->sess_owner = false;
+	} else {
+		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options, test_vector,
+			iv_offset);
+		if (ctx->sess == NULL)
+			goto err;
+		*sess = ctx->sess;
+		ctx->sess_owner = true;
+	}
 
 	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id,
 			extra_op_priv_size,
diff --git a/app/test-crypto-perf/cperf_test_latency.h b/app/test-crypto-perf/cperf_test_latency.h
index d3fc3218d7..0f2b61e21f 100644
--- a/app/test-crypto-perf/cperf_test_latency.h
+++ b/app/test-crypto-perf/cperf_test_latency.h
@@ -21,7 +21,8 @@ cperf_latency_test_constructor(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *ops_fn);
+		const struct cperf_op_fns *ops_fn,
+		void **sess);
 
 int
 cperf_latency_test_runner(void *test_ctx);
diff --git a/app/test-crypto-perf/cperf_test_pmd_cyclecount.c b/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
index 7191d99ea4..07a842f40a 100644
--- a/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
+++ b/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
@@ -29,6 +29,7 @@ struct cperf_pmd_cyclecount_ctx {
 	struct rte_crypto_op **ops_processed;
 
 	void *sess;
+	uint8_t sess_owner;
 
 	cperf_populate_ops_t populate_ops;
 
@@ -63,7 +64,7 @@ cperf_pmd_cyclecount_test_free(struct cperf_pmd_cyclecount_ctx *ctx)
 	if (!ctx)
 		return;
 
-	if (ctx->sess) {
+	if (ctx->sess != NULL && ctx->sess_owner) {
 #ifdef RTE_LIB_SECURITY
 		if (ctx->options->op_type == CPERF_PDCP ||
 				ctx->options->op_type == CPERF_DOCSIS) {
@@ -89,7 +90,8 @@ cperf_pmd_cyclecount_test_constructor(struct rte_mempool *sess_mp,
 		uint8_t dev_id, uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *op_fns)
+		const struct cperf_op_fns *op_fns,
+		void **sess)
 {
 	struct cperf_pmd_cyclecount_ctx *ctx = NULL;
 
@@ -112,10 +114,17 @@ cperf_pmd_cyclecount_test_constructor(struct rte_mempool *sess_mp,
 	uint16_t iv_offset = sizeof(struct rte_crypto_op) +
 			sizeof(struct rte_crypto_sym_op);
 
-	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
-			test_vector, iv_offset);
-	if (ctx->sess == NULL)
-		goto err;
+	if (*sess != NULL) {
+		ctx->sess = *sess;
+		ctx->sess_owner = false;
+	} else {
+		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options, test_vector,
+			iv_offset);
+		if (ctx->sess == NULL)
+			goto err;
+		*sess = ctx->sess;
+		ctx->sess_owner = true;
+	}
 
 	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id, 0,
 			&ctx->src_buf_offset, &ctx->dst_buf_offset,
diff --git a/app/test-crypto-perf/cperf_test_pmd_cyclecount.h b/app/test-crypto-perf/cperf_test_pmd_cyclecount.h
index beb4419910..417fdbbed4 100644
--- a/app/test-crypto-perf/cperf_test_pmd_cyclecount.h
+++ b/app/test-crypto-perf/cperf_test_pmd_cyclecount.h
@@ -22,7 +22,8 @@ cperf_pmd_cyclecount_test_constructor(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *ops_fn);
+		const struct cperf_op_fns *ops_fn,
+		void **sess);
 
 int
 cperf_pmd_cyclecount_test_runner(void *test_ctx);
diff --git a/app/test-crypto-perf/cperf_test_throughput.c b/app/test-crypto-perf/cperf_test_throughput.c
index c0891e7c99..54f2f6a060 100644
--- a/app/test-crypto-perf/cperf_test_throughput.c
+++ b/app/test-crypto-perf/cperf_test_throughput.c
@@ -21,6 +21,7 @@ struct cperf_throughput_ctx {
 	struct rte_mempool *pool;
 
 	void *sess;
+	uint8_t sess_owner;
 
 	cperf_populate_ops_t populate_ops;
 
@@ -36,7 +37,7 @@ cperf_throughput_test_free(struct cperf_throughput_ctx *ctx)
 {
 	if (!ctx)
 		return;
-	if (ctx->sess) {
+	if (ctx->sess != NULL && ctx->sess_owner) {
 		if (ctx->options->op_type == CPERF_ASYM_MODEX)
 			rte_cryptodev_asym_session_free(ctx->dev_id,
 					(void *)ctx->sess);
@@ -63,7 +64,8 @@ cperf_throughput_test_constructor(struct rte_mempool *sess_mp,
 		uint8_t dev_id, uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *op_fns)
+		const struct cperf_op_fns *op_fns,
+		void **sess)
 {
 	struct cperf_throughput_ctx *ctx = NULL;
 
@@ -82,10 +84,17 @@ cperf_throughput_test_constructor(struct rte_mempool *sess_mp,
 	uint16_t iv_offset = sizeof(struct rte_crypto_op) +
 		sizeof(struct rte_crypto_sym_op);
 
-	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
-			test_vector, iv_offset);
-	if (ctx->sess == NULL)
-		goto err;
+	if (*sess != NULL) {
+		ctx->sess = *sess;
+		ctx->sess_owner = false;
+	} else {
+		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options, test_vector,
+			iv_offset);
+		if (ctx->sess == NULL)
+			goto err;
+		*sess = ctx->sess;
+		ctx->sess_owner = true;
+	}
 
 	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id, 0,
 			&ctx->src_buf_offset, &ctx->dst_buf_offset,
diff --git a/app/test-crypto-perf/cperf_test_throughput.h b/app/test-crypto-perf/cperf_test_throughput.h
index 439ec8e559..d9011b6a51 100644
--- a/app/test-crypto-perf/cperf_test_throughput.h
+++ b/app/test-crypto-perf/cperf_test_throughput.h
@@ -22,7 +22,8 @@ cperf_throughput_test_constructor(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *ops_fn);
+		const struct cperf_op_fns *ops_fn,
+		void **sess);
 
 int
 cperf_throughput_test_runner(void *test_ctx);
diff --git a/app/test-crypto-perf/cperf_test_verify.c b/app/test-crypto-perf/cperf_test_verify.c
index 222c7a1cd8..7f6a244794 100644
--- a/app/test-crypto-perf/cperf_test_verify.c
+++ b/app/test-crypto-perf/cperf_test_verify.c
@@ -21,6 +21,7 @@ struct cperf_verify_ctx {
 	struct rte_mempool *pool;
 
 	void *sess;
+	uint8_t sess_owner;
 
 	cperf_populate_ops_t populate_ops;
 
@@ -41,7 +42,7 @@ cperf_verify_test_free(struct cperf_verify_ctx *ctx)
 	if (ctx == NULL)
 		return;
 
-	if (ctx->sess != NULL) {
+	if (ctx->sess != NULL && ctx->sess_owner) {
 		if (ctx->options->op_type == CPERF_ASYM_MODEX)
 			rte_cryptodev_asym_session_free(ctx->dev_id, ctx->sess);
 #ifdef RTE_LIB_SECURITY
@@ -67,7 +68,8 @@ cperf_verify_test_constructor(struct rte_mempool *sess_mp,
 		uint8_t dev_id, uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *op_fns)
+		const struct cperf_op_fns *op_fns,
+		void **sess)
 {
 	struct cperf_verify_ctx *ctx = NULL;
 
@@ -86,10 +88,17 @@ cperf_verify_test_constructor(struct rte_mempool *sess_mp,
 	uint16_t iv_offset = sizeof(struct rte_crypto_op) +
 		sizeof(struct rte_crypto_sym_op);
 
-	ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
-			test_vector, iv_offset);
-	if (ctx->sess == NULL)
-		goto err;
+	if (*sess != NULL) {
+		ctx->sess = *sess;
+		ctx->sess_owner = false;
+	} else {
+		ctx->sess = op_fns->sess_create(sess_mp, dev_id, options,
+				test_vector, iv_offset);
+		if (ctx->sess == NULL)
+			goto err;
+		*sess = ctx->sess;
+		ctx->sess_owner = true;
+	}
 
 	if (cperf_alloc_common_memory(options, test_vector, dev_id, qp_id, 0,
 			&ctx->src_buf_offset, &ctx->dst_buf_offset,
diff --git a/app/test-crypto-perf/cperf_test_verify.h b/app/test-crypto-perf/cperf_test_verify.h
index 9f70ad87ba..dcc10e6e98 100644
--- a/app/test-crypto-perf/cperf_test_verify.h
+++ b/app/test-crypto-perf/cperf_test_verify.h
@@ -22,7 +22,8 @@ cperf_verify_test_constructor(
 		uint16_t qp_id,
 		const struct cperf_options *options,
 		const struct cperf_test_vector *test_vector,
-		const struct cperf_op_fns *ops_fn);
+		const struct cperf_op_fns *ops_fn,
+		void **sess);
 
 int
 cperf_verify_test_runner(void *test_ctx);
diff --git a/app/test-crypto-perf/main.c b/app/test-crypto-perf/main.c
index 40c0b4b54f..312c9dcc96 100644
--- a/app/test-crypto-perf/main.c
+++ b/app/test-crypto-perf/main.c
@@ -647,6 +647,9 @@ main(int argc, char **argv)
 
 	i = 0;
 	uint8_t qp_id = 0, cdev_index = 0;
+
+	void *sess = NULL;
+
 	RTE_LCORE_FOREACH_WORKER(lcore_id) {
 
 		if (i == total_nb_qps)
@@ -663,14 +666,30 @@ main(int argc, char **argv)
 		ctx[i] = cperf_testmap[opts.test].constructor(
 				session_pool_socket[socket_id].sess_mp,
 				cdev_id, qp_id,
-				&opts, t_vec, &op_fns);
+				&opts, t_vec, &op_fns, &sess);
+
+		/*
+		 * If sess was NULL, the constructor will have set it to a newly
+		 * created session. This means future calls to constructors will
+		 * provide this session, sharing it across all qps. If session
+		 * sharing is not enabled, re-set sess to NULL, to prevent this.
+		 */
+		if (!opts.shared_session)
+			sess = NULL;
+
 		if (ctx[i] == NULL) {
 			RTE_LOG(ERR, USER1, "Test run constructor failed\n");
 			goto err;
 		}
+
 		qp_id = (qp_id + 1) % opts.nb_qps;
-		if (qp_id == 0)
+		if (qp_id == 0) {
 			cdev_index++;
+			/* If next qp is on a new cdev, don't share the session
+			 * - it shouldn't be shared across different cdevs.
+			 */
+			sess = NULL;
+		}
 		i++;
 	}
 
diff --git a/doc/guides/tools/cryptoperf.rst b/doc/guides/tools/cryptoperf.rst
index facf412799..c2fb60806c 100644
--- a/doc/guides/tools/cryptoperf.rst
+++ b/doc/guides/tools/cryptoperf.rst
@@ -7,7 +7,7 @@ dpdk-test-crypto-perf Application
 The ``dpdk-test-crypto-perf`` tool is a Data Plane Development Kit (DPDK)
 utility that allows measuring performance parameters of PMDs available in the
 crypto tree. There are available two measurement types: throughput and latency.
-User can use multiply cores to run tests on but only
+User can use multiple cores to run tests on but only
 one type of crypto PMD can be measured during single application
 execution. Cipher parameters, type of device, type of operation and
 chain mode have to be specified in the command line as application
@@ -184,6 +184,13 @@ The following are the application command-line options:
 
         Enable session-less crypto operations mode.
 
+* ``--shared-session``
+
+        Enable sharing sessions between all queue pairs on a single crypto PMD.
+        This can be useful for benchmarking this setup, or finding and debugging
+        concurrency errors that can occur while using sessions on multiple
+        lcores simultaneously.
+
 * ``--out-of-place``
 
         Enable out-of-place crypto operations mode.
-- 
2.34.1


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

* Re: [PATCH] app/test-crypto-perf: add shared session option
  2024-06-12 10:38 ` Dooley, Brian
@ 2024-06-18 15:50   ` Jack Bond-Preston
  0 siblings, 0 replies; 4+ messages in thread
From: Jack Bond-Preston @ 2024-06-18 15:50 UTC (permalink / raw)
  To: Dooley, Brian; +Cc: dev

Hi Brian,

> Would be good to add a doc update too. Thanks.
> 
> Acked-by: Brian Dooley <brian.dooley@intel.com>

Thanks for the suggestion! I totally missed that.
I've uploaded v2 with a doc update.

Thanks,
Jack



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

end of thread, other threads:[~2024-06-18 15:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-04 14:54 [PATCH] app/test-crypto-perf: add shared session option Jack Bond-Preston
2024-06-12 10:38 ` Dooley, Brian
2024-06-18 15:50   ` Jack Bond-Preston
2024-06-18 15:48 ` [PATCH v2] " Jack Bond-Preston

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