* [PATCH] app/test-crypto-perf: add throughput OOP decryption @ 2024-01-05 10:01 Suanming Mou 2024-03-14 18:44 ` [EXT] " Akhil Goyal 2024-03-19 11:46 ` [PATCH v2] " Suanming Mou 0 siblings, 2 replies; 16+ messages in thread From: Suanming Mou @ 2024-01-05 10:01 UTC (permalink / raw) To: anoobj, ciara.power; +Cc: dev 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 <suanmingm@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; + } + 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); + +} + 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption 2024-01-05 10:01 [PATCH] app/test-crypto-perf: add throughput OOP decryption Suanming Mou @ 2024-03-14 18:44 ` Akhil Goyal 2024-03-19 1:57 ` Suanming Mou 2024-03-19 11:46 ` [PATCH v2] " Suanming Mou 1 sibling, 1 reply; 16+ messages in thread From: Akhil Goyal @ 2024-03-14 18:44 UTC (permalink / raw) To: Suanming Mou, Anoob Joseph, ciara.power; +Cc: dev > 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 <suanmingm@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. > + > 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 > +} > + > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption 2024-03-14 18:44 ` [EXT] " Akhil Goyal @ 2024-03-19 1:57 ` Suanming Mou 2024-03-19 8:23 ` Akhil Goyal 0 siblings, 1 reply; 16+ messages in thread From: Suanming Mou @ 2024-03-19 1:57 UTC (permalink / raw) To: Akhil Goyal, Anoob Joseph, ciara.power; +Cc: dev Hi Akhil Sorry for the late reply. > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Friday, March 15, 2024 2:45 AM > To: Suanming Mou <suanmingm@nvidia.com>; Anoob Joseph > <anoobj@marvell.com>; ciara.power@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 <suanmingm@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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption 2024-03-19 1:57 ` Suanming Mou @ 2024-03-19 8:23 ` Akhil Goyal 2024-03-19 9:06 ` Suanming Mou 0 siblings, 1 reply; 16+ messages in thread From: Akhil Goyal @ 2024-03-19 8:23 UTC (permalink / raw) To: Suanming Mou, Anoob Joseph, ciara.power; +Cc: dev > > > + 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? I would suggest to check for "auth_op == RTE_CRYPTO_AUTH_OP_VERIFY" Instead of cipher_op. Ciara, What do you suggest? You were also seeing some issues in this patch. ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption 2024-03-19 8:23 ` Akhil Goyal @ 2024-03-19 9:06 ` Suanming Mou 2024-03-19 9:32 ` Akhil Goyal 0 siblings, 1 reply; 16+ messages in thread From: Suanming Mou @ 2024-03-19 9:06 UTC (permalink / raw) To: Akhil Goyal, Anoob Joseph, ciara.power; +Cc: dev > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Tuesday, March 19, 2024 4:23 PM > To: Suanming Mou <suanmingm@nvidia.com>; Anoob Joseph > <anoobj@marvell.com>; ciara.power@intel.com > Cc: dev@dpdk.org > Subject: RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption > > > > > + 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? > > I would suggest to check for "auth_op == RTE_CRYPTO_AUTH_OP_VERIFY" > Instead of cipher_op. I'm not sure. Since in AEAD OP, auth_op will always be RTE_CRYPTO_AUTH_OP_VERIFY, in that case even in place encrypt will be rejected. If the combination here is too complicated, what about just remove that limits and let user to decide? If the input is not correct, PMD will reject it as well. > > Ciara, What do you suggest? You were also seeing some issues in this patch. ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption 2024-03-19 9:06 ` Suanming Mou @ 2024-03-19 9:32 ` Akhil Goyal 2024-03-19 11:43 ` Suanming Mou 0 siblings, 1 reply; 16+ messages in thread From: Akhil Goyal @ 2024-03-19 9:32 UTC (permalink / raw) To: Suanming Mou, Anoob Joseph, ciara.power; +Cc: dev > > Subject: RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP > decryption > > > > > > > + 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? > > > > I would suggest to check for "auth_op == RTE_CRYPTO_AUTH_OP_VERIFY" > > Instead of cipher_op. > > I'm not sure. Since in AEAD OP, auth_op will always be > RTE_CRYPTO_AUTH_OP_VERIFY, in that case even in place encrypt will be > rejected. > If the combination here is too complicated, what about just remove that limits and > let user to decide? If the input is not correct, PMD will reject it as well. The problematic cases are where auth data (ICV) is not correct. i.e. AEAD, AUTH_ONLY and CIPHER_AUTH. Hence following check should be ok. 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) { Yes PMD will report error if the input data is not correct, but we cannot just fail in that case just because the app is intentionally not filling the data. It should report unsupported case. > > > > > Ciara, What do you suggest? You were also seeing some issues in this patch. ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption 2024-03-19 9:32 ` Akhil Goyal @ 2024-03-19 11:43 ` Suanming Mou 0 siblings, 0 replies; 16+ messages in thread From: Suanming Mou @ 2024-03-19 11:43 UTC (permalink / raw) To: Akhil Goyal, Anoob Joseph, ciara.power; +Cc: dev > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Tuesday, March 19, 2024 5:32 PM > To: Suanming Mou <suanmingm@nvidia.com>; Anoob Joseph > <anoobj@marvell.com>; ciara.power@intel.com > Cc: dev@dpdk.org > Subject: RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP decryption > > > > Subject: RE: [EXT] [PATCH] app/test-crypto-perf: add throughput OOP > > decryption > > > > > > > > > + 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? > > > > > > I would suggest to check for "auth_op == RTE_CRYPTO_AUTH_OP_VERIFY" > > > Instead of cipher_op. > > > > I'm not sure. Since in AEAD OP, auth_op will always be > > RTE_CRYPTO_AUTH_OP_VERIFY, in that case even in place encrypt will be > > rejected. > > If the combination here is too complicated, what about just remove > > that limits and let user to decide? If the input is not correct, PMD will reject it as > well. > > The problematic cases are where auth data (ICV) is not correct. > i.e. AEAD, AUTH_ONLY and CIPHER_AUTH. > > Hence following check should be ok. > 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) { OK, that make sense. Will update, thanks. > > Yes PMD will report error if the input data is not correct, but we cannot just fail in > that case just because the app is intentionally not filling the data. > It should report unsupported case. > > > > > > > > Ciara, What do you suggest? You were also seeing some issues in this patch. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] app/test-crypto-perf: add throughput OOP decryption 2024-01-05 10:01 [PATCH] app/test-crypto-perf: add throughput OOP decryption Suanming Mou 2024-03-14 18:44 ` [EXT] " Akhil Goyal @ 2024-03-19 11:46 ` Suanming Mou 2024-03-19 15:14 ` Power, Ciara 1 sibling, 1 reply; 16+ messages in thread From: Suanming Mou @ 2024-03-19 11:46 UTC (permalink / raw) To: gakhil, Ciara Power; +Cc: dev 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 <suanmingm@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; + } + 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption 2024-03-19 11:46 ` [PATCH v2] " Suanming Mou @ 2024-03-19 15:14 ` Power, Ciara 2024-03-20 0:14 ` Suanming Mou 2024-06-20 6:44 ` Akhil Goyal 0 siblings, 2 replies; 16+ messages in thread From: Power, Ciara @ 2024-03-19 15:14 UTC (permalink / raw) To: Suanming Mou, gakhil; +Cc: dev > -----Original Message----- > From: Suanming Mou <suanmingm@nvidia.com> > Sent: Tuesday, March 19, 2024 11:46 AM > To: gakhil@marvell.com; Power, Ciara <ciara.power@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 <suanmingm@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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption 2024-03-19 15:14 ` Power, Ciara @ 2024-03-20 0:14 ` Suanming Mou 2024-04-01 0:30 ` Suanming Mou 2024-06-20 6:44 ` Akhil Goyal 1 sibling, 1 reply; 16+ messages in thread From: Suanming Mou @ 2024-03-20 0:14 UTC (permalink / raw) To: Power, Ciara, gakhil; +Cc: dev > -----Original Message----- > From: Power, Ciara <ciara.power@intel.com> > Sent: Tuesday, March 19, 2024 11:15 PM > To: Suanming Mou <suanmingm@nvidia.com>; gakhil@marvell.com > Cc: dev@dpdk.org > Subject: RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption > > > > > -----Original Message----- > > From: Suanming Mou <suanmingm@nvidia.com> > > Sent: Tuesday, March 19, 2024 11:46 AM > > To: gakhil@marvell.com; Power, Ciara <ciara.power@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 <suanmingm@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. Yes, think about that, in throughput mode, we will not fill the test data time to time, otherwise the testing is useless. So that means the test data should not be overwritten, otherwise decryption will be with invalid data after the first round of decryption. Since the 1st round decryption overwritten the data to the original buf. In that case, test decryption throughput in non-oop mode is meaningless. That's the reason we add that limit to avoid the invalid data issue. > > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption 2024-03-20 0:14 ` Suanming Mou @ 2024-04-01 0:30 ` Suanming Mou 2024-06-14 9:38 ` Suanming Mou 0 siblings, 1 reply; 16+ messages in thread From: Suanming Mou @ 2024-04-01 0:30 UTC (permalink / raw) To: Suanming Mou, Power, Ciara, gakhil; +Cc: dev Hi guys, Just want to make sure if anything still need to be checked with that patch? > -----Original Message----- > From: Suanming Mou <suanmingm@nvidia.com> > Sent: Wednesday, March 20, 2024 8:15 AM > To: Power, Ciara <ciara.power@intel.com>; gakhil@marvell.com > Cc: dev@dpdk.org > Subject: RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption > > > > 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. > > Yes, think about that, in throughput mode, we will not fill the test data time to > time, otherwise the testing is useless. > So that means the test data should not be overwritten, otherwise decryption will > be with invalid data after the first round of decryption. Since the 1st round > decryption overwritten the data to the original buf. In that case, test decryption > throughput in non-oop mode is meaningless. > That's the reason we add that limit to avoid the invalid data issue. > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption 2024-04-01 0:30 ` Suanming Mou @ 2024-06-14 9:38 ` Suanming Mou 0 siblings, 0 replies; 16+ messages in thread From: Suanming Mou @ 2024-06-14 9:38 UTC (permalink / raw) To: Power, Ciara, gakhil; +Cc: dev Hi Akhil, As we mentioned the application in another thread, do you know if we can take the patch for now? BR, Suanming > -----Original Message----- > From: Suanming Mou > Sent: Monday, April 1, 2024 8:30 AM > To: Suanming Mou <suanmingm@nvidia.com>; Power, Ciara > <ciara.power@intel.com>; gakhil@marvell.com > Cc: dev@dpdk.org > Subject: RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption > > Hi guys, > > Just want to make sure if anything still need to be checked with that patch? > > > -----Original Message----- > > From: Suanming Mou <suanmingm@nvidia.com> > > Sent: Wednesday, March 20, 2024 8:15 AM > > To: Power, Ciara <ciara.power@intel.com>; gakhil@marvell.com > > Cc: dev@dpdk.org > > Subject: RE: [PATCH v2] app/test-crypto-perf: add throughput OOP > > decryption > > > > > > 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. > > > > Yes, think about that, in throughput mode, we will not fill the test > > data time to time, otherwise the testing is useless. > > So that means the test data should not be overwritten, otherwise > > decryption will be with invalid data after the first round of > > decryption. Since the 1st round decryption overwritten the data to the > > original buf. In that case, test decryption throughput in non-oop mode is > meaningless. > > That's the reason we add that limit to avoid the invalid data issue. > > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption 2024-03-19 15:14 ` Power, Ciara 2024-03-20 0:14 ` Suanming Mou @ 2024-06-20 6:44 ` Akhil Goyal 2024-07-18 19:57 ` Akhil Goyal 1 sibling, 1 reply; 16+ messages in thread From: Akhil Goyal @ 2024-06-20 6:44 UTC (permalink / raw) To: Power, Ciara, Suanming Mou, Brian Dooley; +Cc: dev 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 <suanmingm@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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption 2024-06-20 6:44 ` Akhil Goyal @ 2024-07-18 19:57 ` Akhil Goyal 2024-07-19 15:13 ` Dooley, Brian 0 siblings, 1 reply; 16+ messages in thread From: Akhil Goyal @ 2024-07-18 19:57 UTC (permalink / raw) To: Suanming Mou, Brian Dooley, john.mcnamara, Kai Ji, Kusztal, ArkadiuszX Cc: dev > 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 <suanmingm@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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption 2024-07-18 19:57 ` Akhil Goyal @ 2024-07-19 15:13 ` Dooley, Brian 2024-07-19 15:43 ` Akhil Goyal 0 siblings, 1 reply; 16+ messages in thread From: Dooley, Brian @ 2024-07-19 15:13 UTC (permalink / raw) To: Akhil Goyal, Suanming Mou, Mcnamara, John, Ji, Kai, Kusztal, ArkadiuszX Cc: dev Hi Suanming, This change looks ok to me. > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Thursday, July 18, 2024 8:57 PM > To: Suanming Mou <suanmingm@nvidia.com>; Dooley, Brian > <brian.dooley@intel.com>; Mcnamara, John <john.mcnamara@intel.com>; Ji, > Kai <kai.ji@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com> > Cc: dev@dpdk.org > Subject: RE: [PATCH v2] app/test-crypto-perf: add throughput OOP > decryption > > > 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=DnL7Si2 > wl_P > > > RwpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m=eTj0O7iYH- > > > xiTQ6dNUZpsOXPqnyC1O_- > > > > _IKt0j_yQ_N__vy0wIBLb_QyMQtodUrr&s=eDz_NLjqkUH2cYMilKEtdWImOPj > 5f- > > > CVKV5UW8P9frk&e= > > > > > > > > Signed-off-by: Suanming Mou <suanmingm@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 Acked-by: Brian Dooley <brian.dooley@intel.com> ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] app/test-crypto-perf: add throughput OOP decryption 2024-07-19 15:13 ` Dooley, Brian @ 2024-07-19 15:43 ` Akhil Goyal 0 siblings, 0 replies; 16+ messages in thread From: Akhil Goyal @ 2024-07-19 15:43 UTC (permalink / raw) To: Dooley, Brian, Suanming Mou, Mcnamara, John, Ji, Kai, Kusztal, ArkadiuszX Cc: dev > > > > > 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. > > > > > > > > > > Signed-off-by: Suanming Mou <suanmingm@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(-) > > > > > > Acked-by: Brian Dooley <brian.dooley@intel.com> Applied to dpdk-next-crypto ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-07-19 15:43 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-05 10:01 [PATCH] app/test-crypto-perf: add throughput OOP decryption Suanming Mou 2024-03-14 18:44 ` [EXT] " Akhil Goyal 2024-03-19 1:57 ` Suanming Mou 2024-03-19 8:23 ` Akhil Goyal 2024-03-19 9:06 ` Suanming Mou 2024-03-19 9:32 ` Akhil Goyal 2024-03-19 11:43 ` Suanming Mou 2024-03-19 11:46 ` [PATCH v2] " Suanming Mou 2024-03-19 15:14 ` Power, Ciara 2024-03-20 0:14 ` Suanming Mou 2024-04-01 0:30 ` Suanming Mou 2024-06-14 9:38 ` Suanming Mou 2024-06-20 6:44 ` Akhil Goyal 2024-07-18 19:57 ` Akhil Goyal 2024-07-19 15:13 ` Dooley, Brian 2024-07-19 15:43 ` Akhil Goyal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).