app/test-crypto-perf: add shared session option

Message ID 20240604145446.2216337-1-jack.bond-preston@foss.arm.com (mailing list archive)
State Superseded
Delegated to: akhil goyal
Headers
Series app/test-crypto-perf: add shared session option |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS

Commit Message

Jack Bond-Preston June 4, 2024, 2:54 p.m. UTC
  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(-)
  

Comments

Dooley, Brian June 12, 2024, 10:38 a.m. UTC | #1
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>
  
Jack Bond-Preston June 18, 2024, 3:50 p.m. UTC | #2
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
  

Patch

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++;
 	}