> 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? >
Any update on this patch? > > > 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
