Re: [PATCH v1 13/13] test/bbdev: remove iteration count check

[Maxime Coquelin]

Hi Nicolas,

On 2/9/23 17:59, Chautru, Nicolas wrote:

Hi Maxime,

-----Original Message-----
From: Maxime Coquelin <maxime.coque...@redhat.com>
Sent: Thursday, February 9, 2023 1:11 AM
To: Vargas, Hernan <hernan.var...@intel.com>; dev@dpdk.org;
gak...@marvell.com; Rix, Tom <t...@redhat.com>
Cc: Chautru, Nicolas <nicolas.chau...@intel.com>; Zhang, Qi Z
<qi.z.zh...@intel.com>
Subject: Re: [PATCH v1 13/13] test/bbdev: remove iteration count check



On 2/8/23 21:38, Vargas, Hernan wrote:

Hi Maxime,

We would like to keep the same signature for validate_dec_op because there

are functions such as latency_test_dec that have vector_mask on their
signatures and they pass it to validate_dec_op.

Let me know if you'd like to discuss more.

I think this is not a valid reason, just simplify latency_test_dec too.

The principle is that all these functions may or may not use that generic 
operation masks, but we still use a stable (future proof) and consistent 
prototype for these
test functions.

I would agree that it would be necessary if these were callbacks, but
that's not the case.

I believe this is valid and better practice for the test functions, but again 
if you really want to push back, this could be changed.

I prefer we do not bloat the code with things that could be useful in an
hypothetical future.

Thanks,
Maxime

Thanks!!
Nic

Thanks,
Maxime

Thanks,
Hernan

-----Original Message-----
From: Maxime Coquelin <maxime.coque...@redhat.com>
Sent: Tuesday, January 31, 2023 6:36 AM
To: Vargas, Hernan <hernan.var...@intel.com>; dev@dpdk.org;
gak...@marvell.com; Rix, Tom <t...@redhat.com>
Cc: Chautru, Nicolas <nicolas.chau...@intel.com>; Zhang, Qi Z
<qi.z.zh...@intel.com>
Subject: Re: [PATCH v1 13/13] test/bbdev: remove iteration count check



On 1/17/23 17:50, Hernan Vargas wrote:

To make the test compatible with devices that do not support early
termination, the iteration count assert can be removed.

Signed-off-by: Hernan Vargas <hernan.var...@intel.com>
---
    app/test-bbdev/test_bbdev_perf.c | 6 +-----
    1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_perf.c
b/app/test-bbdev/test_bbdev_perf.c
index 81bf2c8b60..c68d79cf29 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -2290,6 +2290,7 @@ static int
    validate_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
                struct rte_bbdev_dec_op *ref_op, const int vector_mask)
    {
+       RTE_SET_USED(vector_mask);

Why not just remove vector_mask if it isn't of any use instead of hiding the

warning?

        unsigned int i;
        int ret;
        struct op_data_entries *hard_data_orig = @@ -2299,17 +2300,12

@@

validate_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
        struct rte_bbdev_op_turbo_dec *ops_td;
        struct rte_bbdev_op_data *hard_output;
        struct rte_bbdev_op_data *soft_output;
-       struct rte_bbdev_op_turbo_dec *ref_td = &ref_op->turbo_dec;

        for (i = 0; i < n; ++i) {
                ops_td = &ops[i]->turbo_dec;
                hard_output = &ops_td->hard_output;
                soft_output = &ops_td->soft_output;

-               if (vector_mask & TEST_BBDEV_VF_EXPECTED_ITER_COUNT)
-                       TEST_ASSERT(ops_td->iter_count <= ref_td-

iter_count,

-                                       "Returned iter_count (%d) > expected

iter_count (%d)",

-                                       ops_td->iter_count, ref_td-

iter_count);

                ret = check_dec_status_and_ordering(ops[i], i, ref_op-

status);

                TEST_ASSERT_SUCCESS(ret,
                                "Checking status and ordering for decoder

failed");

Maxime