Hi Brian, Since Ciara is no longer available and you are the new maintainer, can you investigate this patch? There were some concerns which Ciara highlighted. Can you check?
Regards, Akhil > > 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]: > > https://urldefense.proofpoint.com/v2/url?u=http- > 3A__mails.dpdk.org_archives_dev_2023- > 2DJuly_273328.html&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_P > RwpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m=eTj0O7iYH- > xiTQ6dNUZpsOXPqnyC1O_- > _IKt0j_yQ_N__vy0wIBLb_QyMQtodUrr&s=eDz_NLjqkUH2cYMilKEtdWImOPj5f- > CVKV5UW8P9frk&e= > > > > 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
