DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jack Bond-Preston <jack.bond-preston@foss.arm.com>
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
Date: Tue,  4 Jun 2024 14:54:46 +0000	[thread overview]
Message-ID: <20240604145446.2216337-1-jack.bond-preston@foss.arm.com> (raw)

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


             reply	other threads:[~2024-06-04 14:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-04 14:54 Jack Bond-Preston [this message]
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
2024-07-19 13:01   ` [EXTERNAL] " Akhil Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240604145446.2216337-1-jack.bond-preston@foss.arm.com \
    --to=jack.bond-preston@foss.arm.com \
    --cc=ciara.power@intel.com \
    --cc=dev@dpdk.org \
    --cc=paul.szczepanek@arm.com \
    --cc=wathsala.vithanage@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).