From: "Chautru, Nicolas" <nicolas.chautru@intel.com> To: Tom Rix <trix@redhat.com>, "dev@dpdk.org" <dev@dpdk.org>, "akhil.goyal@nxp.com" <akhil.goyal@nxp.com> Cc: "david.marchand@redhat.com" <david.marchand@redhat.com> Subject: Re: [dpdk-dev] [PATCH v5 1/7] app/bbdev: add explicit ut for latency vs validation Date: Mon, 26 Oct 2020 17:30:11 +0000 Message-ID: <BY5PR11MB44512C9323AE732F47147278F8190@BY5PR11MB4451.namprd11.prod.outlook.com> (raw) In-Reply-To: <33fb2ef0-5609-fd5e-4bc2-b21350946a41@redhat.com> > -----Original Message----- > From: Tom Rix <trix@redhat.com> > Sent: Monday, October 26, 2020 5:56 AM > To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org; > akhil.goyal@nxp.com > Cc: david.marchand@redhat.com > Subject: Re: [PATCH v5 1/7] app/bbdev: add explicit ut for latency vs > validation > > > On 10/23/20 4:42 PM, Nicolas Chautru wrote: > > Adding explicit different ut when testing for validation or latency > > (early termination enabled or not). > > > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com> > > Acked-by: Aidan Goddard <aidan.goddard@accelercomm.com> > > Acked-by: Dave Burley <dave.burley@accelercomm.com> > > --- > > app/test-bbdev/test_bbdev_perf.c | 92 > > ++++++++++++++++++++++++++++++++++++++-- > Should update the copyright. > > 1 file changed, 88 insertions(+), 4 deletions(-) > > > > diff --git a/app/test-bbdev/test_bbdev_perf.c > > b/app/test-bbdev/test_bbdev_perf.c > > index 6e5535d..3554a77 100644 > > --- a/app/test-bbdev/test_bbdev_perf.c > > +++ b/app/test-bbdev/test_bbdev_perf.c > > @@ -3999,12 +3999,14 @@ typedef int (test_case_function)(struct > active_device *ad, > > return i; > > } > > > > +/* Test case for latency/validation for LDPC Decoder */ > > static int > > latency_test_ldpc_dec(struct rte_mempool *mempool, > > struct test_buffers *bufs, struct rte_bbdev_dec_op *ref_op, > > int vector_mask, uint16_t dev_id, uint16_t queue_id, > > const uint16_t num_to_process, uint16_t burst_sz, > > - uint64_t *total_time, uint64_t *min_time, uint64_t > *max_time) > > + uint64_t *total_time, uint64_t *min_time, uint64_t > *max_time, > > + bool disable_et) > > { > > int ret = TEST_SUCCESS; > > uint16_t i, j, dequeued; > > @@ -4026,7 +4028,7 @@ typedef int (test_case_function)(struct > active_device *ad, > > "rte_bbdev_dec_op_alloc_bulk() failed"); > > > > /* For latency tests we need to disable early termination */ > > - if (check_bit(ref_op->ldpc_dec.op_flags, > > + if (disable_et && check_bit(ref_op->ldpc_dec.op_flags, > > > RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE)) > > ref_op->ldpc_dec.op_flags -= > > > RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE; > Bit clearing is usually done with &= ~() This is the coding style for rest of the file hence sticking to it. > > @@ -4248,7 +4250,7 @@ typedef int (test_case_function)(struct > active_device *ad, > > TEST_ASSERT_NOT_NULL(op_type_str, "Invalid op type: %u", > op_type); > > > > printf("+ ------------------------------------------------------- +\n"); > > - printf("== test: validation/latency\ndev: %s, burst size: %u, num ops: > %u, op type: %s\n", > > + printf("== test: latency\ndev: %s, burst size: %u, num ops: %u, op > > +type: %s\n", > > info.dev_name, burst_sz, num_to_process, > op_type_str); > > > > if (op_type == RTE_BBDEV_OP_TURBO_DEC) @@ -4270,7 +4272,83 > @@ > > typedef int (test_case_function)(struct active_device *ad, > > iter = latency_test_ldpc_dec(op_params->mp, bufs, > > op_params->ref_dec_op, op_params- > >vector_mask, > > ad->dev_id, queue_id, num_to_process, > > + burst_sz, &total_time, &min_time, > &max_time, > > + true); > > + else > > + iter = latency_test_enc(op_params->mp, bufs, > > + op_params->ref_enc_op, > > + ad->dev_id, queue_id, > > + num_to_process, burst_sz, > &total_time, > > + &min_time, &max_time); > > This is a repeat of RTE_BBDEV_OP_TURBO_ENC. > > Do not need both. Fair enough. That is part of previous code but can simplify. > > If the point is to have a else and not fail when the op_type is unknown, then > > remove the earlier all and comment the else something like > > else /* RTE_BBDEC_OP_TURBO_ENC */ > > > + > > + if (iter <= 0) > > + return TEST_FAILED; > > + > > + printf("Operation latency:\n" > > + "\tavg: %lg cycles, %lg us\n" > > + "\tmin: %lg cycles, %lg us\n" > > + "\tmax: %lg cycles, %lg us\n", > > + (double)total_time / (double)iter, > > + (double)(total_time * 1000000) / (double)iter / > > + (double)rte_get_tsc_hz(), (double)min_time, > > + (double)(min_time * 1000000) / > (double)rte_get_tsc_hz(), > > + (double)max_time, (double)(max_time * 1000000) / > > + (double)rte_get_tsc_hz()); > Could remove a tab from the last 9 lines for better alignment with printf I am unsure I follow. The recommended spacing is 2 tabs for continuation and unsure how the alignment would be better. I typically only reduce to 1 tab only if I have to (80 chars limit becoming cumbersome with nested statements). > > + > > + return TEST_SUCCESS; > > +} > > + > > +static int > > +validation_test(struct active_device *ad, > > + struct test_op_params *op_params) > > +{ > > + int iter; > > + uint16_t burst_sz = op_params->burst_sz; > > + const uint16_t num_to_process = op_params->num_to_process; > > + const enum rte_bbdev_op_type op_type = test_vector.op_type; > > + const uint16_t queue_id = ad->queue_ids[0]; > > + struct test_buffers *bufs = NULL; > > + struct rte_bbdev_info info; > > + uint64_t total_time, min_time, max_time; > > + const char *op_type_str; > > + > > + total_time = max_time = 0; > > + min_time = UINT64_MAX; > > + > > + TEST_ASSERT_SUCCESS((burst_sz > MAX_BURST), > > + "BURST_SIZE should be <= %u", MAX_BURST); > > + > > + rte_bbdev_info_get(ad->dev_id, &info); > > + bufs = &op_params- > >q_bufs[GET_SOCKET(info.socket_id)][queue_id]; > > + > > + op_type_str = rte_bbdev_op_type_str(op_type); > > + TEST_ASSERT_NOT_NULL(op_type_str, "Invalid op type: %u", > op_type); > > + > > + printf("+ ------------------------------------------------------- +\n"); > > + printf("== test: validation\ndev: %s, burst size: %u, num ops: %u, op > type: %s\n", > > + info.dev_name, burst_sz, num_to_process, > op_type_str); > > + > > + if (op_type == RTE_BBDEV_OP_TURBO_DEC) > > + iter = latency_test_dec(op_params->mp, bufs, > > + op_params->ref_dec_op, op_params- > >vector_mask, > > + ad->dev_id, queue_id, num_to_process, > > burst_sz, &total_time, &min_time, > &max_time); > > + else if (op_type == RTE_BBDEV_OP_TURBO_ENC) > > + iter = latency_test_enc(op_params->mp, bufs, > > + op_params->ref_enc_op, ad->dev_id, > queue_id, > > + num_to_process, burst_sz, &total_time, > > + &min_time, &max_time); > > + else if (op_type == RTE_BBDEV_OP_LDPC_ENC) > > + iter = latency_test_ldpc_enc(op_params->mp, bufs, > > + op_params->ref_enc_op, ad->dev_id, > queue_id, > > + num_to_process, burst_sz, &total_time, > > + &min_time, &max_time); > > + else if (op_type == RTE_BBDEV_OP_LDPC_DEC) > > + iter = latency_test_ldpc_dec(op_params->mp, bufs, > > + op_params->ref_dec_op, op_params- > >vector_mask, > > + ad->dev_id, queue_id, num_to_process, > > + burst_sz, &total_time, &min_time, > &max_time, > > + false); > > This 'false' is the only change from f latency_test. > > These should be refactored to a common function. Then use a #define or > similar wrapper for calling with/without this flag. Fair enough. Thanks. I will push an update later today. > > Tom > > > else > > iter = latency_test_enc(op_params->mp, bufs, > > op_params->ref_enc_op, > > @@ -4930,6 +5008,12 @@ typedef int (test_case_function)(struct > > active_device *ad, } > > > > static int > > +validation_tc(void) > > +{ > > + return run_test_case(validation_test); } > > + > > +static int > > interrupt_tc(void) > > { > > return run_test_case(throughput_test); @@ -4960,7 +5044,7 @@ > typedef > > int (test_case_function)(struct active_device *ad, > > .setup = testsuite_setup, > > .teardown = testsuite_teardown, > > .unit_test_cases = { > > - TEST_CASE_ST(ut_setup, ut_teardown, latency_tc), > > + TEST_CASE_ST(ut_setup, ut_teardown, validation_tc), > > TEST_CASES_END() /**< NULL terminate unit test array */ > > } > > };
next prev parent reply other threads:[~2020-10-26 17:30 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-23 23:42 [dpdk-dev] [PATCH v5 0/7] BBDEV test updates Nicolas Chautru 2020-10-23 23:42 ` [dpdk-dev] [PATCH v5 1/7] app/bbdev: add explicit ut for latency vs validation Nicolas Chautru 2020-10-26 12:55 ` Tom Rix 2020-10-26 17:30 ` Chautru, Nicolas [this message] 2020-10-28 20:37 ` Tom Rix 2020-10-23 23:42 ` [dpdk-dev] [PATCH v5 2/7] app/bbdev: add explicit check for counters Nicolas Chautru 2020-10-26 13:05 ` Tom Rix 2020-10-26 16:29 ` Chautru, Nicolas 2020-10-28 20:31 ` Tom Rix 2020-10-23 23:42 ` [dpdk-dev] [PATCH v5 3/7] app/bbdev: include explicit HARQ preloading Nicolas Chautru 2020-10-26 13:31 ` Tom Rix 2020-10-26 16:50 ` Chautru, Nicolas 2020-10-28 20:33 ` Tom Rix 2020-10-23 23:42 ` [dpdk-dev] [PATCH v5 4/7] app/bbdev: define wait for offload Nicolas Chautru 2020-10-26 13:33 ` Tom Rix 2020-10-26 16:04 ` Chautru, Nicolas 2020-10-28 20:24 ` Tom Rix 2020-10-23 23:42 ` [dpdk-dev] [PATCH v5 5/7] app/bbdev: skip bler ut when compression is used Nicolas Chautru 2020-10-26 13:35 ` Tom Rix 2020-10-23 23:43 ` [dpdk-dev] [PATCH v5 6/7] app/bbdev: reduce duration of throughput test Nicolas Chautru 2020-10-26 13:39 ` Tom Rix 2020-10-23 23:43 ` [dpdk-dev] [PATCH v5 7/7] app/bbdev: update offload test to dequeue full ring Nicolas Chautru 2020-10-26 13:55 ` Tom Rix 2020-10-26 16:27 ` Chautru, Nicolas 2020-10-28 20:28 ` Tom Rix 2020-10-24 7:47 ` [dpdk-dev] [PATCH v5 0/7] BBDEV test updates David Marchand
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=BY5PR11MB44512C9323AE732F47147278F8190@BY5PR11MB4451.namprd11.prod.outlook.com \ --to=nicolas.chautru@intel.com \ --cc=akhil.goyal@nxp.com \ --cc=david.marchand@redhat.com \ --cc=dev@dpdk.org \ --cc=trix@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
DPDK patches and discussions This inbox may be cloned and mirrored by anyone: git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \ dev@dpdk.org public-inbox-index dev Example config snippet for mirrors. Newsgroup available over NNTP: nntp://inbox.dpdk.org/inbox.dpdk.dev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git