> -----Original Message-----
> From: Suanming Mou <suanmi...@nvidia.com>
> Sent: Tuesday, March 19, 2024 11:46 AM
> To: gak...@marvell.com; Power, Ciara <ciara.po...@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption
> 
> During throughput running, re-filling the test data will impact the 
> performance
> test result. So for now, to run decrypt throughput testing is not supported 
> since
> the test data is not filled.
> 
> But if user requires OOP(out-of-place) mode, the test data from source mbuf 
> will
> never be modified, and if the test data can be prepared out of the running 
> loop,
> the decryption test should be fine.
> 
> This commit adds the support of out-of-place decryption testing for 
> throughput.
> 
> [1]:
> http://mails.dpdk.org/archives/dev/2023-July/273328.html
> 
> Signed-off-by: Suanming Mou <suanmi...@nvidia.com>
> ---
>  app/test-crypto-perf/cperf_ops.c             |  5 ++-
>  app/test-crypto-perf/cperf_options_parsing.c |  8 +++++  app/test-crypto-
> perf/cperf_test_throughput.c | 34 +++++++++++++++++---
>  3 files changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test-crypto-perf/cperf_ops.c 
> b/app/test-crypto-perf/cperf_ops.c
> index d3fd115bc0..714616c697 100644
> --- a/app/test-crypto-perf/cperf_ops.c
> +++ b/app/test-crypto-perf/cperf_ops.c
> @@ -644,7 +644,10 @@ cperf_set_ops_aead(struct rte_crypto_op **ops,
>       }
> 
>       if ((options->test == CPERF_TEST_TYPE_VERIFY) ||
> -                     (options->test == CPERF_TEST_TYPE_LATENCY)) {
> +         (options->test == CPERF_TEST_TYPE_LATENCY) ||
> +         (options->test == CPERF_TEST_TYPE_THROUGHPUT &&
> +          (options->aead_op == RTE_CRYPTO_AEAD_OP_DECRYPT ||
> +           options->cipher_op == RTE_CRYPTO_CIPHER_OP_DECRYPT))) {
>               for (i = 0; i < nb_ops; i++) {
>                       uint8_t *iv_ptr = rte_crypto_op_ctod_offset(ops[i],
>                                       uint8_t *, iv_offset);
> diff --git a/app/test-crypto-perf/cperf_options_parsing.c b/app/test-crypto-
> perf/cperf_options_parsing.c
> index 8c20974273..90526e676f 100644
> --- a/app/test-crypto-perf/cperf_options_parsing.c
> +++ b/app/test-crypto-perf/cperf_options_parsing.c
> @@ -1341,6 +1341,14 @@ cperf_options_check(struct cperf_options
> *options)
>               }
>       }
> 
> +     if (options->test == CPERF_TEST_TYPE_THROUGHPUT &&
> +         (options->aead_op == RTE_CRYPTO_AEAD_OP_DECRYPT ||
> +          options->auth_op == RTE_CRYPTO_AUTH_OP_VERIFY) &&
> +         !options->out_of_place) {
> +             RTE_LOG(ERR, USER1, "Only out-of-place is allowed in
> throughput decryption.\n");
> +             return -EINVAL;
> +     }

Not totally following some of this, why do we only want to add this for OOP 
mode?

For example an inplace command I can use before this patch but not after:
./build/app/dpdk-test-crypto-perf -l 2,3 -- --ptest throughput --optype aead 
--aead-algo aes-gcm --aead-op decrypt --devtype crypto_qat --aead-key-sz 16

I get an error;
USER1: Only out-of-place is allowed in throughput decryption.
USER1: Checking one or more user options failed

Do we want to always force the user to use OOP + test vector file for these 
throughput decryption tests?
Or should we just add a warning that the throughput may not be reflecting the 
"success" verify path in PMD if using inplace and the dummy data.

I am not sure.
If we do want to add the limitation on the throughput tests, these changes I 
think are ok for that.

Thanks,
Ciara

> +
>       if (options->op_type == CPERF_CIPHER_ONLY ||
>                       options->op_type == CPERF_CIPHER_THEN_AUTH ||
>                       options->op_type == CPERF_AUTH_THEN_CIPHER) { diff
> --git a/app/test-crypto-perf/cperf_test_throughput.c b/app/test-crypto-
> perf/cperf_test_throughput.c
> index e3d266d7a4..b347baa913 100644
> --- a/app/test-crypto-perf/cperf_test_throughput.c
> +++ b/app/test-crypto-perf/cperf_test_throughput.c
> @@ -99,6 +99,26 @@ cperf_throughput_test_constructor(struct rte_mempool
> *sess_mp,
>       return NULL;
>  }
> 
> +static void
> +cperf_verify_init_ops(struct rte_mempool *mp __rte_unused,
> +                   void *opaque_arg,
> +                   void *obj,
> +                   __rte_unused unsigned int i)
> +{
> +     uint16_t iv_offset = sizeof(struct rte_crypto_op) +
> +             sizeof(struct rte_crypto_sym_op);
> +     uint32_t imix_idx = 0;
> +     struct cperf_throughput_ctx *ctx = opaque_arg;
> +     struct rte_crypto_op *op = obj;
> +
> +     (ctx->populate_ops)(&op, ctx->src_buf_offset,
> +                     ctx->dst_buf_offset,
> +                     1, ctx->sess, ctx->options,
> +                     ctx->test_vector, iv_offset, &imix_idx, NULL);
> +
> +     cperf_mbuf_set(op->sym->m_src, ctx->options, ctx->test_vector); }
> +
>  int
>  cperf_throughput_test_runner(void *test_ctx)  { @@ -144,6 +164,9 @@
> cperf_throughput_test_runner(void *test_ctx)
>       uint16_t iv_offset = sizeof(struct rte_crypto_op) +
>               sizeof(struct rte_crypto_sym_op);
> 
> +     if (ctx->options->out_of_place)
> +             rte_mempool_obj_iter(ctx->pool, cperf_verify_init_ops, (void
> *)ctx);
> +
>       while (test_burst_size <= ctx->options->max_burst_size) {
>               uint64_t ops_enqd = 0, ops_enqd_total = 0, ops_enqd_failed =
> 0;
>               uint64_t ops_deqd = 0, ops_deqd_total = 0, ops_deqd_failed =
> 0; @@ -176,11 +199,12 @@ cperf_throughput_test_runner(void *test_ctx)
>                       }
> 
>                       /* Setup crypto op, attach mbuf etc */
> -                     (ctx->populate_ops)(ops, ctx->src_buf_offset,
> -                                     ctx->dst_buf_offset,
> -                                     ops_needed, ctx->sess,
> -                                     ctx->options, ctx->test_vector,
> -                                     iv_offset, &imix_idx, &tsc_start);
> +                     if (!ctx->options->out_of_place)
> +                             (ctx->populate_ops)(ops, ctx->src_buf_offset,
> +                                             ctx->dst_buf_offset,
> +                                             ops_needed, ctx->sess,
> +                                             ctx->options, ctx->test_vector,
> +                                             iv_offset, &imix_idx,
> &tsc_start);
> 
>                       /**
>                        * When ops_needed is smaller than ops_enqd, the
> --
> 2.34.1

Reply via email to