[PATCH v2] app/test-crypto-perf: add shared session option

[Jack Bond-Preston <jack.bond-preston@foss.arm.com>]

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