Hi Akhil
Sorry for the late reply.
> -----Original Message-----
> From: Akhil Goyal <gak...@marvell.com>
> Sent: Friday, March 15, 2024 2:45 AM
> To: Suanming Mou <suanmi...@nvidia.com>; Anoob Joseph
> <ano...@marvell.com>; ciara.po...@intel.com
> Cc: dev@dpdk.org
> Subject: RE: [EXT] [PATCH] 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.
> >
>
>
> > 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 | 37 +++++++++++++++++---
> > 3 files changed, 44 insertions(+), 6 deletions(-)
> >
> > diff --git a/app/test-crypto-perf/cperf_ops.c
> > b/app/test-crypto-perf/cperf_ops.c
> > index 84945d1313..1d57b78c2b 100644
> > --- a/app/test-crypto-perf/cperf_ops.c
> > +++ b/app/test-crypto-perf/cperf_ops.c
> > @@ -608,7 +608,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
> > 75afedc7fd..6caca44371 100644
> > --- a/app/test-crypto-perf/cperf_options_parsing.c
> > +++ b/app/test-crypto-perf/cperf_options_parsing.c
> > @@ -1291,6 +1291,14 @@ cperf_options_check(struct cperf_options *options)
> > }
> > }
> >
> > + if (options->test == CPERF_TEST_TYPE_THROUGHPUT &&
> > + (options->aead_op == RTE_CRYPTO_AEAD_OP_DECRYPT ||
> > + options->cipher_op == RTE_CRYPTO_CIPHER_OP_DECRYPT) &&
> > + !options->out_of_place) {
> > + RTE_LOG(ERR, USER1, "Only out-of-place is allowed in
> > throughput decryption.\n");
> > + return -EINVAL;
> > + }
>
> This check is blocking cipher_only decryption which should pass irrespective
> of
> inplace/oop and Data correct/incorrect.
Sorry, in that case I will remove "options->cipher_op ==
RTE_CRYPTO_CIPHER_OP_DECRYPT" and only kept " options->aead_op ==
RTE_CRYPTO_AEAD_OP_DECRYPT ", what do you think?
>
> > +
> > 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 f8f8bd717f..eab25ec863 100644
> > --- a/app/test-crypto-perf/cperf_test_throughput.c
> > +++ b/app/test-crypto-perf/cperf_test_throughput.c
> > @@ -98,6 +98,29 @@ 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);
> Unnecessary line break.
>
> > +
> Extra line
Will update.
>
> > +}
> > +
> > int
> > cperf_throughput_test_runner(void *test_ctx) { @@ -143,6 +166,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;
> @@
> > -175,11 +201,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
