[PATCH v1 00/13] test/bbdev: changes for 23.03

[PATCH v1 00/13] test/bbdev: changes for 23.03

[Hernan Vargas]

Upstreaming bbdev-test changes for 23.03.

Hernan Vargas (13):
  test/bbdev: fix seg fault for non supported HARQ len
  test/bbdev: refactor TB throughput report
  test/bbdev: add timeout for latency tests
  test/bbdev: early termination not explicit set
  test/bbdev: report device status in bbdev-test
  test/bbdev: log capture from queue stop
  test/bbdev: add support for BLER for 4G
  test/bbdev: extend support for large TB
  test/bbdev: bbdev-test cannot compare some scenarios
  test/bbdev: adjustment for soft output
  test/bbdev: expose warning counters
  test/bbdev: remove check for invalid opaque data
  test/bbdev: remove iteration count check

 app/test-bbdev/test_bbdev_perf.c | 319 +++++++++++++++++++++++++------
 1 file changed, 265 insertions(+), 54 deletions(-)

-- 
2.37.1

[PATCH v1 01/13] test/bbdev: fix seg fault for non supported HARQ len

[Hernan Vargas]

Catching a corner in bbdev-test (not in the actual PMD) when running
some specific vectors which size are not supported by the PMD.

Fixes: 335c11fd276 ("app/bbdev: support HARQ validation")
Cc: stable@dpdk.org

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

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 3818b74c79..cc7b5481d6 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -84,7 +84,7 @@
 /* Increment for next code block in external HARQ memory */
 #define HARQ_INCR 32768
 /* Headroom for filler LLRs insertion in HARQ buffer */
-#define FILLER_HEADROOM 1024
+#define FILLER_HEADROOM 2048
 /* Constants from K0 computation from 3GPP 38.212 Table 5.4.2.1-2 */
 #define N_ZC_1 66 /* N = 66 Zc for BG 1 */
 #define N_ZC_2 50 /* N = 50 Zc for BG 2 */
@@ -2111,9 +2111,9 @@ validate_op_harq_chain(struct rte_bbdev_op_data *op,
 					ops_ld->n_filler;
 			if (data_len > deRmOutSize)
 				data_len = deRmOutSize;
-			if (data_len > orig_op->segments[i].length)
-				data_len = orig_op->segments[i].length;
 		}
+		if (data_len > orig_op->segments[i].length)
+			data_len = orig_op->segments[i].length;
 		/*
 		 * HARQ output can have minor differences
 		 * due to integer representation and related scaling
-- 
2.37.1

Re: [PATCH v1 01/13] test/bbdev: fix seg fault for non supported HARQ len

[Maxime Coquelin]

Hi Hernan,

On 1/17/23 17:50, Hernan Vargas wrote:
> Catching a corner in bbdev-test (not in the actual PMD) when running
> some specific vectors which size are not supported by the PMD.

Could you please reword the commit message, the title is clearer than
the commit message itself.

Also, the app name is test-bbdev.


> 
> Fixes: 335c11fd276 ("app/bbdev: support HARQ validation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 3818b74c79..cc7b5481d6 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -84,7 +84,7 @@
>   /* Increment for next code block in external HARQ memory */
>   #define HARQ_INCR 32768
>   /* Headroom for filler LLRs insertion in HARQ buffer */
> -#define FILLER_HEADROOM 1024
> +#define FILLER_HEADROOM 2048
>   /* Constants from K0 computation from 3GPP 38.212 Table 5.4.2.1-2 */
>   #define N_ZC_1 66 /* N = 66 Zc for BG 1 */
>   #define N_ZC_2 50 /* N = 50 Zc for BG 2 */
> @@ -2111,9 +2111,9 @@ validate_op_harq_chain(struct rte_bbdev_op_data *op,
>   					ops_ld->n_filler;
>   			if (data_len > deRmOutSize)
>   				data_len = deRmOutSize;
> -			if (data_len > orig_op->segments[i].length)
> -				data_len = orig_op->segments[i].length;
>   		}
> +		if (data_len > orig_op->segments[i].length)
> +			data_len = orig_op->segments[i].length;
>   		/*
>   		 * HARQ output can have minor differences
>   		 * due to integer representation and related scaling

[PATCH v1 02/13] test/bbdev: refactor TB throughput report

[Hernan Vargas]

Refactor calculation for tb_size.
No functional impact.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 app/test-bbdev/test_bbdev_perf.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index cc7b5481d6..1a8a6b9f82 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -2613,18 +2613,15 @@ calc_enc_TB_size(struct rte_bbdev_enc_op *op)
 static uint32_t
 calc_ldpc_enc_TB_size(struct rte_bbdev_enc_op *op)
 {
-	uint8_t i;
-	uint32_t c, r, tb_size = 0;
+	uint32_t tb_size = 0;
 	uint16_t sys_cols = (op->ldpc_enc.basegraph == 1) ? 22 : 10;
 
 	if (op->ldpc_enc.code_block_mode == RTE_BBDEV_CODE_BLOCK) {
 		tb_size = sys_cols * op->ldpc_enc.z_c - op->ldpc_enc.n_filler;
 	} else {
-		c = op->turbo_enc.tb_params.c;
-		r = op->turbo_enc.tb_params.r;
-		for (i = 0; i < c-r; i++)
-			tb_size += sys_cols * op->ldpc_enc.z_c
-					- op->ldpc_enc.n_filler;
+		tb_size = (sys_cols * op->ldpc_enc.z_c - op->ldpc_enc.n_filler)
+				* (op->ldpc_enc.tb_params.c -
+				op->ldpc_enc.tb_params.r);
 	}
 	return tb_size;
 }
-- 
2.37.1

Re: [PATCH v1 02/13] test/bbdev: refactor TB throughput report

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> Refactor calculation for tb_size.
> No functional impact.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index cc7b5481d6..1a8a6b9f82 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -2613,18 +2613,15 @@ calc_enc_TB_size(struct rte_bbdev_enc_op *op)
>   static uint32_t
>   calc_ldpc_enc_TB_size(struct rte_bbdev_enc_op *op)
>   {
> -	uint8_t i;
> -	uint32_t c, r, tb_size = 0;
> +	uint32_t tb_size = 0;
>   	uint16_t sys_cols = (op->ldpc_enc.basegraph == 1) ? 22 : 10;
>   
>   	if (op->ldpc_enc.code_block_mode == RTE_BBDEV_CODE_BLOCK) {
>   		tb_size = sys_cols * op->ldpc_enc.z_c - op->ldpc_enc.n_filler;
>   	} else {
> -		c = op->turbo_enc.tb_params.c;
> -		r = op->turbo_enc.tb_params.r;
> -		for (i = 0; i < c-r; i++)
> -			tb_size += sys_cols * op->ldpc_enc.z_c
> -					- op->ldpc_enc.n_filler;
> +		tb_size = (sys_cols * op->ldpc_enc.z_c - op->ldpc_enc.n_filler)
> +				* (op->ldpc_enc.tb_params.c -
> +				op->ldpc_enc.tb_params.r);

It could be made simpler.

static uint32_t
calc_ldpc_dec_TB_size(struct rte_bbdev_dec_op *op)
{
	uint8_t i;
	uint32_t tb_size = 0;
	uint16_t sys_cols = (op->ldpc_dec.basegraph == 1) ? 22 : 10;

	if (op->ldpc_dec.code_block_mode == RTE_BBDEV_CODE_BLOCK)
		i = 1
	else
		i = op->ldpc_dec.tb_params.c - op->ldpc_dec.tb_params.r;

	tb_size = (sys_cols * op->ldpc_dec.z_c - op->ldpc_dec.n_filler) * i;

	return tb_size;
}

What do you think?

Thanks,
Maxime

>   	}
>   	return tb_size;
>   }

[PATCH v1 03/13] test/bbdev: add timeout for latency tests

[Hernan Vargas]

Add a timeout to force exit the latency tests in case dequeue never
happens.

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

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 1a8a6b9f82..14fe76eb64 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -26,6 +26,7 @@
 
 #define MAX_QUEUES RTE_MAX_LCORE
 #define TEST_REPETITIONS 100
+#define TIME_OUT_POLL 1e8
 #define WAIT_OFFLOAD_US 1000
 
 #ifdef RTE_BASEBAND_FPGA_LTE_FEC
@@ -4544,6 +4545,7 @@ latency_test_ldpc_dec(struct rte_mempool *mempool,
 
 	for (i = 0, dequeued = 0; dequeued < num_to_process; ++i) {
 		uint16_t enq = 0, deq = 0;
+		uint32_t time_out = 0;
 		bool first_time = true;
 		last_time = 0;
 
@@ -4595,7 +4597,8 @@ latency_test_ldpc_dec(struct rte_mempool *mempool,
 				last_time = rte_rdtsc_precise() - start_time;
 				first_time = false;
 			}
-		} while (unlikely(burst_sz != deq));
+			time_out++;
+		} while ((burst_sz != deq) && (time_out < TIME_OUT_POLL));
 
 		*max_time = RTE_MAX(*max_time, last_time);
 		*min_time = RTE_MIN(*min_time, last_time);
@@ -4604,7 +4607,13 @@ latency_test_ldpc_dec(struct rte_mempool *mempool,
 		if (extDdr)
 			retrieve_harq_ddr(dev_id, queue_id, ops_enq, burst_sz);
 
-		if (test_vector.op_type != RTE_BBDEV_OP_NONE) {
+		if (burst_sz != deq) {
+			ret = TEST_FAILED;
+			dequeued = num_to_process; /* Force exit */
+			struct rte_bbdev_info info;
+			rte_bbdev_info_get(dev_id, &info);
+			TEST_ASSERT_SUCCESS(ret, "Dequeue timeout!");
+		} else if (test_vector.op_type != RTE_BBDEV_OP_NONE) {
 			ret = validate_ldpc_dec_op(ops_deq, burst_sz, ref_op,
 					vector_mask);
 			TEST_ASSERT_SUCCESS(ret, "Validation failed!");
@@ -4630,6 +4639,7 @@ latency_test_enc(struct rte_mempool *mempool,
 
 	for (i = 0, dequeued = 0; dequeued < num_to_process; ++i) {
 		uint16_t enq = 0, deq = 0;
+		uint32_t time_out = 0;
 		bool first_time = true;
 		last_time = 0;
 
@@ -4665,13 +4675,19 @@ latency_test_enc(struct rte_mempool *mempool,
 				last_time += rte_rdtsc_precise() - start_time;
 				first_time = false;
 			}
-		} while (unlikely(burst_sz != deq));
+			time_out++;
+		} while ((burst_sz != deq) && (time_out < TIME_OUT_POLL));
 
 		*max_time = RTE_MAX(*max_time, last_time);
 		*min_time = RTE_MIN(*min_time, last_time);
 		*total_time += last_time;
-
-		if (test_vector.op_type != RTE_BBDEV_OP_NONE) {
+		if (burst_sz != deq) {
+			ret = TEST_FAILED;
+			dequeued = num_to_process; /* Force exit */
+			struct rte_bbdev_info info;
+			rte_bbdev_info_get(dev_id, &info);
+			TEST_ASSERT_SUCCESS(ret, "Dequeue timeout!");
+		} else if (test_vector.op_type != RTE_BBDEV_OP_NONE) {
 			ret = validate_enc_op(ops_deq, burst_sz, ref_op);
 			TEST_ASSERT_SUCCESS(ret, "Validation failed!");
 		}
-- 
2.37.1

Re: [PATCH v1 03/13] test/bbdev: add timeout for latency tests

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> Add a timeout to force exit the latency tests in case dequeue never
> happens.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 26 +++++++++++++++++++++-----
>   1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 1a8a6b9f82..14fe76eb64 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -26,6 +26,7 @@
>   
>   #define MAX_QUEUES RTE_MAX_LCORE
>   #define TEST_REPETITIONS 100
> +#define TIME_OUT_POLL 1e8
>   #define WAIT_OFFLOAD_US 1000
>   
>   #ifdef RTE_BASEBAND_FPGA_LTE_FEC
> @@ -4544,6 +4545,7 @@ latency_test_ldpc_dec(struct rte_mempool *mempool,
>   
>   	for (i = 0, dequeued = 0; dequeued < num_to_process; ++i) {
>   		uint16_t enq = 0, deq = 0;
> +		uint32_t time_out = 0;
>   		bool first_time = true;
>   		last_time = 0;
>   
> @@ -4595,7 +4597,8 @@ latency_test_ldpc_dec(struct rte_mempool *mempool,
>   				last_time = rte_rdtsc_precise() - start_time;
>   				first_time = false;
>   			}
> -		} while (unlikely(burst_sz != deq));
> +			time_out++;
> +		} while ((burst_sz != deq) && (time_out < TIME_OUT_POLL));
>   
>   		*max_time = RTE_MAX(*max_time, last_time);
>   		*min_time = RTE_MIN(*min_time, last_time);
> @@ -4604,7 +4607,13 @@ latency_test_ldpc_dec(struct rte_mempool *mempool,
>   		if (extDdr)
>   			retrieve_harq_ddr(dev_id, queue_id, ops_enq, burst_sz);
>   
> -		if (test_vector.op_type != RTE_BBDEV_OP_NONE) {
> +		if (burst_sz != deq) {
> +			ret = TEST_FAILED;
> +			dequeued = num_to_process; /* Force exit */

This should not be necessary, as TEST_ASSERT_SUCCESS() hides a return
statement.

> +			struct rte_bbdev_info info;

Don't mix declaration & code.

> +			rte_bbdev_info_get(dev_id, &info);
> +			TEST_ASSERT_SUCCESS(ret, "Dequeue timeout!");
> +		} else if (test_vector.op_type != RTE_BBDEV_OP_NONE) {
>   			ret = validate_ldpc_dec_op(ops_deq, burst_sz, ref_op,
>   					vector_mask);
>   			TEST_ASSERT_SUCCESS(ret, "Validation failed!");
> @@ -4630,6 +4639,7 @@ latency_test_enc(struct rte_mempool *mempool,
>   
>   	for (i = 0, dequeued = 0; dequeued < num_to_process; ++i) {
>   		uint16_t enq = 0, deq = 0;
> +		uint32_t time_out = 0;
>   		bool first_time = true;
>   		last_time = 0;
>   
> @@ -4665,13 +4675,19 @@ latency_test_enc(struct rte_mempool *mempool,
>   				last_time += rte_rdtsc_precise() - start_time;
>   				first_time = false;
>   			}
> -		} while (unlikely(burst_sz != deq));
> +			time_out++;
> +		} while ((burst_sz != deq) && (time_out < TIME_OUT_POLL));
>   
>   		*max_time = RTE_MAX(*max_time, last_time);
>   		*min_time = RTE_MIN(*min_time, last_time);
>   		*total_time += last_time;
> -
> -		if (test_vector.op_type != RTE_BBDEV_OP_NONE) {
> +		if (burst_sz != deq) {
> +			ret = TEST_FAILED;
> +			dequeued = num_to_process; /* Force exit */
> +			struct rte_bbdev_info info;
> +			rte_bbdev_info_get(dev_id, &info);
> +			TEST_ASSERT_SUCCESS(ret, "Dequeue timeout!");

Same comments here.

> +		} else if (test_vector.op_type != RTE_BBDEV_OP_NONE) {
>   			ret = validate_enc_op(ops_deq, burst_sz, ref_op);
>   			TEST_ASSERT_SUCCESS(ret, "Validation failed!");
>   		}

[PATCH v1 04/13] test/bbdev: early termination not explicit set

[Hernan Vargas]

Early termination needs to be explicitly enabled.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 app/test-bbdev/test_bbdev_perf.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 14fe76eb64..12fa988da6 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -4463,7 +4463,7 @@ latency_test_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;
@@ -4481,6 +4481,13 @@ latency_test_dec(struct rte_mempool *mempool,
 		ret = rte_bbdev_dec_op_alloc_bulk(mempool, ops_enq, burst_sz);
 		TEST_ASSERT_SUCCESS(ret,
 				"rte_bbdev_dec_op_alloc_bulk() failed");
+		ref_op->turbo_dec.iter_max = get_iter_max();
+		/* For validation tests we want to enable early termination */
+		if (!disable_et && !check_bit(ref_op->turbo_dec.op_flags,
+				RTE_BBDEV_TURBO_EARLY_TERMINATION))
+			ref_op->turbo_dec.op_flags |=
+					RTE_BBDEV_TURBO_EARLY_TERMINATION;
+
 		if (test_vector.op_type != RTE_BBDEV_OP_NONE)
 			copy_reference_dec_op(ops_enq, burst_sz, dequeued,
 					bufs->inputs,
@@ -4873,7 +4880,7 @@ validation_latency_test(struct active_device *ad,
 		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);
+				burst_sz, &total_time, &min_time, &max_time, latency_flag);
 	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,
-- 
2.37.1

Re: [PATCH v1 04/13] test/bbdev: early termination not explicit set

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> Early termination needs to be explicitly enabled.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 14fe76eb64..12fa988da6 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -4463,7 +4463,7 @@ latency_test_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;
> @@ -4481,6 +4481,13 @@ latency_test_dec(struct rte_mempool *mempool,
>   		ret = rte_bbdev_dec_op_alloc_bulk(mempool, ops_enq, burst_sz);
>   		TEST_ASSERT_SUCCESS(ret,
>   				"rte_bbdev_dec_op_alloc_bulk() failed");
> +		ref_op->turbo_dec.iter_max = get_iter_max();
> +		/* For validation tests we want to enable early termination */
> +		if (!disable_et && !check_bit(ref_op->turbo_dec.op_flags,
> +				RTE_BBDEV_TURBO_EARLY_TERMINATION))
> +			ref_op->turbo_dec.op_flags |=
> +					RTE_BBDEV_TURBO_EARLY_TERMINATION;
> +
>   		if (test_vector.op_type != RTE_BBDEV_OP_NONE)
>   			copy_reference_dec_op(ops_enq, burst_sz, dequeued,
>   					bufs->inputs,
> @@ -4873,7 +4880,7 @@ validation_latency_test(struct active_device *ad,
>   		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);
> +				burst_sz, &total_time, &min_time, &max_time, latency_flag);
>   	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,

Not clear to me whether it should be a fix. Any thoughts?

Thanks,
Maxime

RE: [PATCH v1 04/13] test/bbdev: early termination not explicit set

[Vargas, Hernan]

Hi Maxime,

-----Original Message-----
From: Maxime Coquelin <maxime.coquelin@redhat.com> 
Sent: Tuesday, January 31, 2023 4:04 AM
To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom <trix@redhat.com>
Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Subject: Re: [PATCH v1 04/13] test/bbdev: early termination not explicit set



On 1/17/23 17:50, Hernan Vargas wrote:
> Early termination needs to be explicitly enabled.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 14fe76eb64..12fa988da6 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -4463,7 +4463,7 @@ latency_test_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;
> @@ -4481,6 +4481,13 @@ latency_test_dec(struct rte_mempool *mempool,
>   		ret = rte_bbdev_dec_op_alloc_bulk(mempool, ops_enq, burst_sz);
>   		TEST_ASSERT_SUCCESS(ret,
>   				"rte_bbdev_dec_op_alloc_bulk() failed");
> +		ref_op->turbo_dec.iter_max = get_iter_max();
> +		/* For validation tests we want to enable early termination */
> +		if (!disable_et && !check_bit(ref_op->turbo_dec.op_flags,
> +				RTE_BBDEV_TURBO_EARLY_TERMINATION))
> +			ref_op->turbo_dec.op_flags |=
> +					RTE_BBDEV_TURBO_EARLY_TERMINATION;
> +
>   		if (test_vector.op_type != RTE_BBDEV_OP_NONE)
>   			copy_reference_dec_op(ops_enq, burst_sz, dequeued,
>   					bufs->inputs,
> @@ -4873,7 +4880,7 @@ validation_latency_test(struct active_device *ad,
>   		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);
> +				burst_sz, &total_time, &min_time, &max_time, latency_flag);
>   	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,

Not clear to me whether it should be a fix. Any thoughts?

Thanks,
Maxime

This is not a fix, it's just an enhancement to enable early termination for validation tests if the device supports it.
The validation tests currently run iter_max number of iterations for decoding, with this enhancement the decoding could terminate early.

Thanks,
Hernan

Re: [PATCH v1 04/13] test/bbdev: early termination not explicit set

[Maxime Coquelin]

On 2/10/23 18:15, Vargas, Hernan wrote:
> Hi Maxime,
> 
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, January 31, 2023 4:04 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v1 04/13] test/bbdev: early termination not explicit set
> 
> 
> 
> On 1/17/23 17:50, Hernan Vargas wrote:
>> Early termination needs to be explicitly enabled.
>>
>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>> ---
>>    app/test-bbdev/test_bbdev_perf.c | 11 +++++++++--
>>    1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
>> index 14fe76eb64..12fa988da6 100644
>> --- a/app/test-bbdev/test_bbdev_perf.c
>> +++ b/app/test-bbdev/test_bbdev_perf.c
>> @@ -4463,7 +4463,7 @@ latency_test_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;
>> @@ -4481,6 +4481,13 @@ latency_test_dec(struct rte_mempool *mempool,
>>    		ret = rte_bbdev_dec_op_alloc_bulk(mempool, ops_enq, burst_sz);
>>    		TEST_ASSERT_SUCCESS(ret,
>>    				"rte_bbdev_dec_op_alloc_bulk() failed");
>> +		ref_op->turbo_dec.iter_max = get_iter_max();
>> +		/* For validation tests we want to enable early termination */
>> +		if (!disable_et && !check_bit(ref_op->turbo_dec.op_flags,
>> +				RTE_BBDEV_TURBO_EARLY_TERMINATION))
>> +			ref_op->turbo_dec.op_flags |=
>> +					RTE_BBDEV_TURBO_EARLY_TERMINATION;
>> +
>>    		if (test_vector.op_type != RTE_BBDEV_OP_NONE)
>>    			copy_reference_dec_op(ops_enq, burst_sz, dequeued,
>>    					bufs->inputs,
>> @@ -4873,7 +4880,7 @@ validation_latency_test(struct active_device *ad,
>>    		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);
>> +				burst_sz, &total_time, &min_time, &max_time, latency_flag);
>>    	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,
> 
> Not clear to me whether it should be a fix. Any thoughts?
> 
> Thanks,
> Maxime
> 
> This is not a fix, it's just an enhancement to enable early termination for validation tests if the device supports it.
> The validation tests currently run iter_max number of iterations for decoding, with this enhancement the decoding could terminate early.
> 
> Thanks,
> Hernan

Could you please fix your email client?
The message you are replying to is not quoted.

Thanks,
Maxime

[PATCH v1 05/13] test/bbdev: report device status in bbdev-test

[Hernan Vargas]

No functional impact.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 app/test-bbdev/test_bbdev_perf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 12fa988da6..36ea399938 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -849,6 +849,8 @@ add_bbdev_dev(uint8_t dev_id, struct rte_bbdev_info *info,
 #endif
 	/* Let's refresh this now this is configured */
 	rte_bbdev_info_get(dev_id, info);
+	if (info->drv.device_status == RTE_BBDEV_DEV_FATAL_ERR)
+		printf("Device Status %s\n", rte_bbdev_device_status_str(info->drv.device_status));
 	nb_queues = RTE_MIN(rte_lcore_count(), info->drv.max_num_queues);
 	nb_queues = RTE_MIN(nb_queues, (unsigned int) MAX_QUEUES);
 
-- 
2.37.1

Re: [PATCH v1 05/13] test/bbdev: report device status in bbdev-test

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> No functional impact.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 12fa988da6..36ea399938 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -849,6 +849,8 @@ add_bbdev_dev(uint8_t dev_id, struct rte_bbdev_info *info,
>   #endif
>   	/* Let's refresh this now this is configured */
>   	rte_bbdev_info_get(dev_id, info);
> +	if (info->drv.device_status == RTE_BBDEV_DEV_FATAL_ERR)
> +		printf("Device Status %s\n", rte_bbdev_device_status_str(info->drv.device_status));
>   	nb_queues = RTE_MIN(rte_lcore_count(), info->drv.max_num_queues);
>   	nb_queues = RTE_MIN(nb_queues, (unsigned int) MAX_QUEUES);
>   

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH v1 06/13] test/bbdev: log capture from queue stop

[Hernan Vargas]

Improve log capture in bbdev not requiring standard output.

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

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 36ea399938..ffb2de0604 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -889,6 +889,7 @@ add_bbdev_dev(uint8_t dev_id, struct rte_bbdev_info *info,
 			ret = rte_bbdev_queue_configure(ad->dev_id, queue_id,
 					&qconf);
 		}
+		rte_bbdev_queue_start(ad->dev_id, queue_id);
 		if (ret != 0) {
 			printf("All queues on dev %u allocated: %u\n",
 					dev_id, queue_id);
@@ -897,8 +898,8 @@ add_bbdev_dev(uint8_t dev_id, struct rte_bbdev_info *info,
 		ad->queue_ids[queue_id] = queue_id;
 	}
 	TEST_ASSERT(queue_id != 0,
-			"ERROR Failed to configure any queues on dev %u",
-			dev_id);
+			"ERROR Failed to configure any queues on dev %u\n"
+			"\tthe device may not support or have been configured", dev_id);
 	ad->nb_queues = queue_id;
 
 	set_avail_op(ad, op_type);
@@ -3844,6 +3845,7 @@ throughput_pmd_lcore_ldpc_dec(void *arg)
 		TEST_ASSERT_SUCCESS(ret, "Validation failed!");
 	}
 
+	rte_bbdev_queue_stop(tp->dev_id, queue_id);
 	rte_bbdev_dec_op_free_bulk(ops_enq, num_ops);
 
 	double tb_len_bits = calc_ldpc_dec_TB_size(ref_op);
-- 
2.37.1

Re: [PATCH v1 06/13] test/bbdev: log capture from queue stop

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> Improve log capture in bbdev not requiring standard output.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 36ea399938..ffb2de0604 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -889,6 +889,7 @@ add_bbdev_dev(uint8_t dev_id, struct rte_bbdev_info *info,
>   			ret = rte_bbdev_queue_configure(ad->dev_id, queue_id,
>   					&qconf);
>   		}
> +		rte_bbdev_queue_start(ad->dev_id, queue_id);
>   		if (ret != 0) {
>   			printf("All queues on dev %u allocated: %u\n",
>   					dev_id, queue_id);
> @@ -897,8 +898,8 @@ add_bbdev_dev(uint8_t dev_id, struct rte_bbdev_info *info,
>   		ad->queue_ids[queue_id] = queue_id;
>   	}
>   	TEST_ASSERT(queue_id != 0,
> -			"ERROR Failed to configure any queues on dev %u",
> -			dev_id);
> +			"ERROR Failed to configure any queues on dev %u\n"
> +			"\tthe device may not support or have been configured", dev_id);
>   	ad->nb_queues = queue_id;
>   
>   	set_avail_op(ad, op_type);
> @@ -3844,6 +3845,7 @@ throughput_pmd_lcore_ldpc_dec(void *arg)
>   		TEST_ASSERT_SUCCESS(ret, "Validation failed!");
>   	}
>   
> +	rte_bbdev_queue_stop(tp->dev_id, queue_id);

The patch is doing more than advertised.

>   	rte_bbdev_dec_op_free_bulk(ops_enq, num_ops);
>   
>   	double tb_len_bits = calc_ldpc_dec_TB_size(ref_op);

Maxime

[PATCH v1 07/13] test/bbdev: add support for BLER for 4G

[Hernan Vargas]

New feature to add BLER support for 4G in bbdev-test.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 app/test-bbdev/test_bbdev_perf.c | 184 ++++++++++++++++++++++++++++++-
 1 file changed, 183 insertions(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index ffb2de0604..69b86cdeb1 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -1756,6 +1756,30 @@ gen_qm2_llr(int8_t *llrs, uint32_t j, double N0, double llr_max)
 	llrs[j] = (int8_t) b;
 }
 
+/* Simple LLR generation assuming AWGN and QPSK */
+static void
+gen_turbo_llr(int8_t *llrs, uint32_t j, double N0, double llr_max)
+{
+	double b, b1, n;
+	double coeff = 2.0 * sqrt(N0);
+
+	/* Ignore in vectors null LLRs not to be saturated */
+	if (llrs[j] == 0)
+		return;
+
+	/* Note don't change sign here */
+	n = randn(j % 2);
+	b1 = ((llrs[j] > 0 ? 2.0 : -2.0)
+			+ coeff * n) / N0;
+	b = b1 * (1 << 4);
+	b = round(b);
+	if (b > llr_max)
+		b = llr_max;
+	if (b < -llr_max)
+		b = -llr_max;
+	llrs[j] = (int8_t) b;
+}
+
 /* Generate LLR for a given SNR */
 static void
 generate_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
@@ -1791,6 +1815,27 @@ generate_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
 	}
 }
 
+/* Generate LLR for turbo decoder for a given SNR */
+static void
+generate_turbo_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
+		struct rte_bbdev_dec_op *ref_op)
+{
+	struct rte_mbuf *m;
+	uint32_t i, j, range;
+	double N0, llr_max;
+
+	llr_max = 127;
+	range = ref_op->turbo_dec.input.length;
+	N0 = 1.0 / pow(10.0, get_snr() / 10.0);
+
+	for (i = 0; i < n; ++i) {
+		m = inputs[i].data;
+		int8_t *llrs = rte_pktmbuf_mtod_offset(m, int8_t *, 0);
+		for (j = 0; j < range; ++j)
+			gen_turbo_llr(llrs, j, N0, llr_max);
+	}
+}
+
 static void
 copy_reference_ldpc_dec_op(struct rte_bbdev_dec_op **ops, unsigned int n,
 		unsigned int start_idx,
@@ -2301,6 +2346,31 @@ validate_ldpc_bler(struct rte_bbdev_dec_op **ops, const uint16_t n)
 	return errors;
 }
 
+/* Check Number of code blocks errors */
+static int
+validate_turbo_bler(struct rte_bbdev_dec_op **ops, const uint16_t n)
+{
+	unsigned int i;
+	struct op_data_entries *hard_data_orig =
+			&test_vector.entries[DATA_HARD_OUTPUT];
+	struct rte_bbdev_op_turbo_dec *ops_td;
+	struct rte_bbdev_op_data *hard_output;
+	int errors = 0;
+	struct rte_mbuf *m;
+
+	for (i = 0; i < n; ++i) {
+		ops_td = &ops[i]->turbo_dec;
+		hard_output = &ops_td->hard_output;
+		m = hard_output->data;
+		if (memcmp(rte_pktmbuf_mtod_offset(m, uint32_t *, 0),
+				hard_data_orig->segments[0].addr,
+				hard_data_orig->segments[0].length))
+			errors++;
+	}
+	return errors;
+}
+
+
 static int
 validate_ldpc_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
 		struct rte_bbdev_dec_op *ref_op, const int vector_mask)
@@ -3736,6 +3806,114 @@ bler_pmd_lcore_ldpc_dec(void *arg)
 	return TEST_SUCCESS;
 }
 
+
+static int
+bler_pmd_lcore_turbo_dec(void *arg)
+{
+	struct thread_params *tp = arg;
+	uint16_t enq, deq;
+	uint64_t total_time = 0, start_time;
+	const uint16_t queue_id = tp->queue_id;
+	const uint16_t burst_sz = tp->op_params->burst_sz;
+	const uint16_t num_ops = tp->op_params->num_to_process;
+	struct rte_bbdev_dec_op *ops_enq[num_ops];
+	struct rte_bbdev_dec_op *ops_deq[num_ops];
+	struct rte_bbdev_dec_op *ref_op = tp->op_params->ref_dec_op;
+	struct test_buffers *bufs = NULL;
+	int i, j, ret;
+	struct rte_bbdev_info info;
+	uint16_t num_to_enq;
+
+	TEST_ASSERT_SUCCESS((burst_sz > MAX_BURST),
+			"BURST_SIZE should be <= %u", MAX_BURST);
+
+	rte_bbdev_info_get(tp->dev_id, &info);
+
+	TEST_ASSERT_SUCCESS((num_ops > info.drv.queue_size_lim),
+			"NUM_OPS cannot exceed %u for this device",
+			info.drv.queue_size_lim);
+
+	bufs = &tp->op_params->q_bufs[GET_SOCKET(info.socket_id)][queue_id];
+
+	rte_wait_until_equal_16(&tp->op_params->sync, SYNC_START, __ATOMIC_RELAXED);
+
+	ret = rte_bbdev_dec_op_alloc_bulk(tp->op_params->mp, ops_enq, num_ops);
+	TEST_ASSERT_SUCCESS(ret, "Allocation failed for %d ops", num_ops);
+
+	/* For BLER tests we need to enable early termination */
+	if (!check_bit(ref_op->turbo_dec.op_flags,
+			RTE_BBDEV_TURBO_EARLY_TERMINATION))
+		ref_op->turbo_dec.op_flags +=
+				RTE_BBDEV_TURBO_EARLY_TERMINATION;
+	ref_op->turbo_dec.iter_max = get_iter_max();
+	ref_op->turbo_dec.iter_count = ref_op->turbo_dec.iter_max;
+
+	if (test_vector.op_type != RTE_BBDEV_OP_NONE)
+		copy_reference_dec_op(ops_enq, num_ops, 0, bufs->inputs,
+				bufs->hard_outputs, bufs->soft_outputs,
+				ref_op);
+	generate_turbo_llr_input(num_ops, bufs->inputs, ref_op);
+
+	/* Set counter to validate the ordering */
+	for (j = 0; j < num_ops; ++j)
+		ops_enq[j]->opaque_data = (void *)(uintptr_t)j;
+
+	for (i = 0; i < 1; ++i) { /* Could add more iterations */
+		for (j = 0; j < num_ops; ++j) {
+			mbuf_reset(
+			ops_enq[j]->turbo_dec.hard_output.data);
+		}
+
+		start_time = rte_rdtsc_precise();
+
+		for (enq = 0, deq = 0; enq < num_ops;) {
+			num_to_enq = burst_sz;
+
+			if (unlikely(num_ops - enq < num_to_enq))
+				num_to_enq = num_ops - enq;
+
+			enq += rte_bbdev_enqueue_dec_ops(tp->dev_id,
+					queue_id, &ops_enq[enq], num_to_enq);
+
+			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
+					queue_id, &ops_deq[deq], enq - deq);
+		}
+
+		/* dequeue the remaining */
+		while (deq < enq) {
+			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
+					queue_id, &ops_deq[deq], enq - deq);
+		}
+
+		total_time += rte_rdtsc_precise() - start_time;
+	}
+
+	tp->iter_count = 0;
+	tp->iter_average = 0;
+	/* get the max of iter_count for all dequeued ops */
+	for (i = 0; i < num_ops; ++i) {
+		tp->iter_count = RTE_MAX(ops_enq[i]->turbo_dec.iter_count,
+				tp->iter_count);
+		tp->iter_average += (double) ops_enq[i]->turbo_dec.iter_count;
+	}
+
+	tp->iter_average /= num_ops;
+	tp->bler = (double) validate_turbo_bler(ops_deq, num_ops) / num_ops;
+
+	rte_bbdev_dec_op_free_bulk(ops_enq, num_ops);
+
+	double tb_len_bits = calc_dec_TB_size(ref_op);
+	tp->ops_per_sec = ((double)num_ops * 1) /
+			((double)total_time / (double)rte_get_tsc_hz());
+	tp->mbps = (((double)(num_ops * 1 * tb_len_bits)) /
+			1000000.0) / ((double)total_time /
+			(double)rte_get_tsc_hz());
+	printf("TBS %.0f Time %.0f\n", tb_len_bits, 1000000.0 *
+			((double)total_time / (double)rte_get_tsc_hz()));
+
+	return TEST_SUCCESS;
+}
+
 static int
 throughput_pmd_lcore_ldpc_dec(void *arg)
 {
@@ -4193,7 +4371,7 @@ print_dec_bler(struct thread_params *t_params, unsigned int used_cores)
 	total_bler /= used_cores;
 	total_iter /= used_cores;
 
-	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.1f Mbps %s\n",
+	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.3f Mbps %s\n",
 			snr, total_bler * 100, total_iter, get_iter_max(),
 			total_mbps, get_vector_filename());
 }
@@ -4245,6 +4423,10 @@ bler_test(struct active_device *ad,
 			&& !check_bit(test_vector.ldpc_dec.op_flags,
 			RTE_BBDEV_LDPC_LLR_COMPRESSION))
 		bler_function = bler_pmd_lcore_ldpc_dec;
+	else if ((test_vector.op_type == RTE_BBDEV_OP_TURBO_DEC) &&
+			!check_bit(test_vector.turbo_dec.op_flags,
+			RTE_BBDEV_TURBO_SOFT_OUTPUT))
+		bler_function = bler_pmd_lcore_turbo_dec;
 	else
 		return TEST_SKIPPED;
 
-- 
2.37.1

Re: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> New feature to add BLER support for 4G in bbdev-test.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 184 ++++++++++++++++++++++++++++++-
>   1 file changed, 183 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index ffb2de0604..69b86cdeb1 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -1756,6 +1756,30 @@ gen_qm2_llr(int8_t *llrs, uint32_t j, double N0, double llr_max)
>   	llrs[j] = (int8_t) b;
>   }
>   
> +/* Simple LLR generation assuming AWGN and QPSK */
> +static void
> +gen_turbo_llr(int8_t *llrs, uint32_t j, double N0, double llr_max)
> +{
> +	double b, b1, n;
> +	double coeff = 2.0 * sqrt(N0);
> +
> +	/* Ignore in vectors null LLRs not to be saturated */
> +	if (llrs[j] == 0)
> +		return;
> +
> +	/* Note don't change sign here */
> +	n = randn(j % 2);
> +	b1 = ((llrs[j] > 0 ? 2.0 : -2.0)
> +			+ coeff * n) / N0;
> +	b = b1 * (1 << 4);
> +	b = round(b);
> +	if (b > llr_max)
> +		b = llr_max;
> +	if (b < -llr_max)
> +		b = -llr_max;
> +	llrs[j] = (int8_t) b;
> +}
> +
>   /* Generate LLR for a given SNR */
>   static void
>   generate_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
> @@ -1791,6 +1815,27 @@ generate_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
>   	}
>   }
>   
> +/* Generate LLR for turbo decoder for a given SNR */
> +static void
> +generate_turbo_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
> +		struct rte_bbdev_dec_op *ref_op)
> +{
> +	struct rte_mbuf *m;
> +	uint32_t i, j, range;
> +	double N0, llr_max;
> +
> +	llr_max = 127;
> +	range = ref_op->turbo_dec.input.length;
> +	N0 = 1.0 / pow(10.0, get_snr() / 10.0);
> +
> +	for (i = 0; i < n; ++i) {
> +		m = inputs[i].data;
> +		int8_t *llrs = rte_pktmbuf_mtod_offset(m, int8_t *, 0);
> +		for (j = 0; j < range; ++j)
> +			gen_turbo_llr(llrs, j, N0, llr_max);
> +	}
> +}
> +
>   static void
>   copy_reference_ldpc_dec_op(struct rte_bbdev_dec_op **ops, unsigned int n,
>   		unsigned int start_idx,
> @@ -2301,6 +2346,31 @@ validate_ldpc_bler(struct rte_bbdev_dec_op **ops, const uint16_t n)
>   	return errors;
>   }
>   
> +/* Check Number of code blocks errors */
> +static int
> +validate_turbo_bler(struct rte_bbdev_dec_op **ops, const uint16_t n)
> +{
> +	unsigned int i;
> +	struct op_data_entries *hard_data_orig =
> +			&test_vector.entries[DATA_HARD_OUTPUT];
> +	struct rte_bbdev_op_turbo_dec *ops_td;
> +	struct rte_bbdev_op_data *hard_output;
> +	int errors = 0;
> +	struct rte_mbuf *m;
> +
> +	for (i = 0; i < n; ++i) {
> +		ops_td = &ops[i]->turbo_dec;
> +		hard_output = &ops_td->hard_output;
> +		m = hard_output->data;
> +		if (memcmp(rte_pktmbuf_mtod_offset(m, uint32_t *, 0),
> +				hard_data_orig->segments[0].addr,
> +				hard_data_orig->segments[0].length))

No need to check mbuf is at least as large as segment 0 length?

> +			errors++;
> +	}
> +	return errors;
> +}
> +
> +
>   static int
>   validate_ldpc_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
>   		struct rte_bbdev_dec_op *ref_op, const int vector_mask)
> @@ -3736,6 +3806,114 @@ bler_pmd_lcore_ldpc_dec(void *arg)
>   	return TEST_SUCCESS;
>   }
>   
> +
> +static int
> +bler_pmd_lcore_turbo_dec(void *arg)
> +{
> +	struct thread_params *tp = arg;
> +	uint16_t enq, deq;
> +	uint64_t total_time = 0, start_time;
> +	const uint16_t queue_id = tp->queue_id;
> +	const uint16_t burst_sz = tp->op_params->burst_sz;
> +	const uint16_t num_ops = tp->op_params->num_to_process;
> +	struct rte_bbdev_dec_op *ops_enq[num_ops];
> +	struct rte_bbdev_dec_op *ops_deq[num_ops];
> +	struct rte_bbdev_dec_op *ref_op = tp->op_params->ref_dec_op;
> +	struct test_buffers *bufs = NULL;
> +	int i, j, ret;
> +	struct rte_bbdev_info info;
> +	uint16_t num_to_enq;
> +
> +	TEST_ASSERT_SUCCESS((burst_sz > MAX_BURST),
> +			"BURST_SIZE should be <= %u", MAX_BURST);
> +
> +	rte_bbdev_info_get(tp->dev_id, &info);
> +
> +	TEST_ASSERT_SUCCESS((num_ops > info.drv.queue_size_lim),
> +			"NUM_OPS cannot exceed %u for this device",
> +			info.drv.queue_size_lim);
> +
> +	bufs = &tp->op_params->q_bufs[GET_SOCKET(info.socket_id)][queue_id];
> +
> +	rte_wait_until_equal_16(&tp->op_params->sync, SYNC_START, __ATOMIC_RELAXED);
> +
> +	ret = rte_bbdev_dec_op_alloc_bulk(tp->op_params->mp, ops_enq, num_ops);
> +	TEST_ASSERT_SUCCESS(ret, "Allocation failed for %d ops", num_ops);
> +
> +	/* For BLER tests we need to enable early termination */
> +	if (!check_bit(ref_op->turbo_dec.op_flags,
> +			RTE_BBDEV_TURBO_EARLY_TERMINATION))
> +		ref_op->turbo_dec.op_flags +=
> +				RTE_BBDEV_TURBO_EARLY_TERMINATION;
> +	ref_op->turbo_dec.iter_max = get_iter_max();
> +	ref_op->turbo_dec.iter_count = ref_op->turbo_dec.iter_max;
> +
> +	if (test_vector.op_type != RTE_BBDEV_OP_NONE)
> +		copy_reference_dec_op(ops_enq, num_ops, 0, bufs->inputs,
> +				bufs->hard_outputs, bufs->soft_outputs,
> +				ref_op);
> +	generate_turbo_llr_input(num_ops, bufs->inputs, ref_op);
> +
> +	/* Set counter to validate the ordering */
> +	for (j = 0; j < num_ops; ++j)
> +		ops_enq[j]->opaque_data = (void *)(uintptr_t)j;
> +
> +	for (i = 0; i < 1; ++i) { /* Could add more iterations */
> +		for (j = 0; j < num_ops; ++j) {
> +			mbuf_reset(
> +			ops_enq[j]->turbo_dec.hard_output.data);
> +		}
> +
> +		start_time = rte_rdtsc_precise();
> +
> +		for (enq = 0, deq = 0; enq < num_ops;) {
> +			num_to_enq = burst_sz;
> +
> +			if (unlikely(num_ops - enq < num_to_enq))
> +				num_to_enq = num_ops - enq;
> +
> +			enq += rte_bbdev_enqueue_dec_ops(tp->dev_id,
> +					queue_id, &ops_enq[enq], num_to_enq);

If for some reason the operation fails (looks like it can fail in at
least ACC200 I checked), it ends up doing an endless loop as enc won't
get incremented.

> +
> +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
> +					queue_id, &ops_deq[deq], enq - deq);
> +		}
> +
> +		/* dequeue the remaining */
> +		while (deq < enq) {
> +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
> +					queue_id, &ops_deq[deq], enq - deq);
> +		}
> +
> +		total_time += rte_rdtsc_precise() - start_time;
> +	}
> +
> +	tp->iter_count = 0;
> +	tp->iter_average = 0;
> +	/* get the max of iter_count for all dequeued ops */
> +	for (i = 0; i < num_ops; ++i) {
> +		tp->iter_count = RTE_MAX(ops_enq[i]->turbo_dec.iter_count,
> +				tp->iter_count);
> +		tp->iter_average += (double) ops_enq[i]->turbo_dec.iter_count;
> +	}
> +
> +	tp->iter_average /= num_ops;
> +	tp->bler = (double) validate_turbo_bler(ops_deq, num_ops) / num_ops;
> +
> +	rte_bbdev_dec_op_free_bulk(ops_enq, num_ops);
> +
> +	double tb_len_bits = calc_dec_TB_size(ref_op);
> +	tp->ops_per_sec = ((double)num_ops * 1) /
> +			((double)total_time / (double)rte_get_tsc_hz());
> +	tp->mbps = (((double)(num_ops * 1 * tb_len_bits)) /
> +			1000000.0) / ((double)total_time /
> +			(double)rte_get_tsc_hz());
> +	printf("TBS %.0f Time %.0f\n", tb_len_bits, 1000000.0 *
> +			((double)total_time / (double)rte_get_tsc_hz()));
> +
> +	return TEST_SUCCESS;
> +}
> +
>   static int
>   throughput_pmd_lcore_ldpc_dec(void *arg)
>   {
> @@ -4193,7 +4371,7 @@ print_dec_bler(struct thread_params *t_params, unsigned int used_cores)
>   	total_bler /= used_cores;
>   	total_iter /= used_cores;
>   
> -	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.1f Mbps %s\n",
> +	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.3f Mbps %s\n",
>   			snr, total_bler * 100, total_iter, get_iter_max(),
>   			total_mbps, get_vector_filename());
>   }
> @@ -4245,6 +4423,10 @@ bler_test(struct active_device *ad,
>   			&& !check_bit(test_vector.ldpc_dec.op_flags,
>   			RTE_BBDEV_LDPC_LLR_COMPRESSION))
>   		bler_function = bler_pmd_lcore_ldpc_dec;
> +	else if ((test_vector.op_type == RTE_BBDEV_OP_TURBO_DEC) &&
> +			!check_bit(test_vector.turbo_dec.op_flags,
> +			RTE_BBDEV_TURBO_SOFT_OUTPUT))
> +		bler_function = bler_pmd_lcore_turbo_dec;
>   	else
>   		return TEST_SKIPPED;
>   

RE: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G

[Vargas, Hernan]

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, January 31, 2023 4:20 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G
> 
> 
> 
> On 1/17/23 17:50, Hernan Vargas wrote:
> > New feature to add BLER support for 4G in bbdev-test.
> >
> > Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> > ---
> >   app/test-bbdev/test_bbdev_perf.c | 184
> ++++++++++++++++++++++++++++++-
> >   1 file changed, 183 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-bbdev/test_bbdev_perf.c
> > b/app/test-bbdev/test_bbdev_perf.c
> > index ffb2de0604..69b86cdeb1 100644
> > --- a/app/test-bbdev/test_bbdev_perf.c
> > +++ b/app/test-bbdev/test_bbdev_perf.c
> > @@ -1756,6 +1756,30 @@ gen_qm2_llr(int8_t *llrs, uint32_t j, double N0,
> double llr_max)
> >   	llrs[j] = (int8_t) b;
> >   }
> >
> > +/* Simple LLR generation assuming AWGN and QPSK */ static void
> > +gen_turbo_llr(int8_t *llrs, uint32_t j, double N0, double llr_max) {
> > +	double b, b1, n;
> > +	double coeff = 2.0 * sqrt(N0);
> > +
> > +	/* Ignore in vectors null LLRs not to be saturated */
> > +	if (llrs[j] == 0)
> > +		return;
> > +
> > +	/* Note don't change sign here */
> > +	n = randn(j % 2);
> > +	b1 = ((llrs[j] > 0 ? 2.0 : -2.0)
> > +			+ coeff * n) / N0;
> > +	b = b1 * (1 << 4);
> > +	b = round(b);
> > +	if (b > llr_max)
> > +		b = llr_max;
> > +	if (b < -llr_max)
> > +		b = -llr_max;
> > +	llrs[j] = (int8_t) b;
> > +}
> > +
> >   /* Generate LLR for a given SNR */
> >   static void
> >   generate_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs, @@
> > -1791,6 +1815,27 @@ generate_llr_input(uint16_t n, struct
> rte_bbdev_op_data *inputs,
> >   	}
> >   }
> >
> > +/* Generate LLR for turbo decoder for a given SNR */ static void
> > +generate_turbo_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
> > +		struct rte_bbdev_dec_op *ref_op)
> > +{
> > +	struct rte_mbuf *m;
> > +	uint32_t i, j, range;
> > +	double N0, llr_max;
> > +
> > +	llr_max = 127;
> > +	range = ref_op->turbo_dec.input.length;
> > +	N0 = 1.0 / pow(10.0, get_snr() / 10.0);
> > +
> > +	for (i = 0; i < n; ++i) {
> > +		m = inputs[i].data;
> > +		int8_t *llrs = rte_pktmbuf_mtod_offset(m, int8_t *, 0);
> > +		for (j = 0; j < range; ++j)
> > +			gen_turbo_llr(llrs, j, N0, llr_max);
> > +	}
> > +}
> > +
> >   static void
> >   copy_reference_ldpc_dec_op(struct rte_bbdev_dec_op **ops, unsigned
> int n,
> >   		unsigned int start_idx,
> > @@ -2301,6 +2346,31 @@ validate_ldpc_bler(struct rte_bbdev_dec_op
> **ops, const uint16_t n)
> >   	return errors;
> >   }
> >
> > +/* Check Number of code blocks errors */ static int
> > +validate_turbo_bler(struct rte_bbdev_dec_op **ops, const uint16_t n)
> > +{
> > +	unsigned int i;
> > +	struct op_data_entries *hard_data_orig =
> > +			&test_vector.entries[DATA_HARD_OUTPUT];
> > +	struct rte_bbdev_op_turbo_dec *ops_td;
> > +	struct rte_bbdev_op_data *hard_output;
> > +	int errors = 0;
> > +	struct rte_mbuf *m;
> > +
> > +	for (i = 0; i < n; ++i) {
> > +		ops_td = &ops[i]->turbo_dec;
> > +		hard_output = &ops_td->hard_output;
> > +		m = hard_output->data;
> > +		if (memcmp(rte_pktmbuf_mtod_offset(m, uint32_t *, 0),
> > +				hard_data_orig->segments[0].addr,
> > +				hard_data_orig->segments[0].length))
> 
> No need to check mbuf is at least as large as segment 0 length?
>
> > +			errors++;
> > +	}
> > +	return errors;
> > +}
> > +
> > +
> >   static int
> >   validate_ldpc_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
> >   		struct rte_bbdev_dec_op *ref_op, const int vector_mask)
> @@ -3736,6
> > +3806,114 @@ bler_pmd_lcore_ldpc_dec(void *arg)
> >   	return TEST_SUCCESS;
> >   }
> >
> > +
> > +static int
> > +bler_pmd_lcore_turbo_dec(void *arg)
> > +{
> > +	struct thread_params *tp = arg;
> > +	uint16_t enq, deq;
> > +	uint64_t total_time = 0, start_time;
> > +	const uint16_t queue_id = tp->queue_id;
> > +	const uint16_t burst_sz = tp->op_params->burst_sz;
> > +	const uint16_t num_ops = tp->op_params->num_to_process;
> > +	struct rte_bbdev_dec_op *ops_enq[num_ops];
> > +	struct rte_bbdev_dec_op *ops_deq[num_ops];
> > +	struct rte_bbdev_dec_op *ref_op = tp->op_params->ref_dec_op;
> > +	struct test_buffers *bufs = NULL;
> > +	int i, j, ret;
> > +	struct rte_bbdev_info info;
> > +	uint16_t num_to_enq;
> > +
> > +	TEST_ASSERT_SUCCESS((burst_sz > MAX_BURST),
> > +			"BURST_SIZE should be <= %u", MAX_BURST);
> > +
> > +	rte_bbdev_info_get(tp->dev_id, &info);
> > +
> > +	TEST_ASSERT_SUCCESS((num_ops > info.drv.queue_size_lim),
> > +			"NUM_OPS cannot exceed %u for this device",
> > +			info.drv.queue_size_lim);
> > +
> > +	bufs = &tp->op_params-
> >q_bufs[GET_SOCKET(info.socket_id)][queue_id];
> > +
> > +	rte_wait_until_equal_16(&tp->op_params->sync, SYNC_START,
> > +__ATOMIC_RELAXED);
> > +
> > +	ret = rte_bbdev_dec_op_alloc_bulk(tp->op_params->mp, ops_enq,
> num_ops);
> > +	TEST_ASSERT_SUCCESS(ret, "Allocation failed for %d ops",
> num_ops);
> > +
> > +	/* For BLER tests we need to enable early termination */
> > +	if (!check_bit(ref_op->turbo_dec.op_flags,
> > +			RTE_BBDEV_TURBO_EARLY_TERMINATION))
> > +		ref_op->turbo_dec.op_flags +=
> > +				RTE_BBDEV_TURBO_EARLY_TERMINATION;
> > +	ref_op->turbo_dec.iter_max = get_iter_max();
> > +	ref_op->turbo_dec.iter_count = ref_op->turbo_dec.iter_max;
> > +
> > +	if (test_vector.op_type != RTE_BBDEV_OP_NONE)
> > +		copy_reference_dec_op(ops_enq, num_ops, 0, bufs-
> >inputs,
> > +				bufs->hard_outputs, bufs->soft_outputs,
> > +				ref_op);
> > +	generate_turbo_llr_input(num_ops, bufs->inputs, ref_op);
> > +
> > +	/* Set counter to validate the ordering */
> > +	for (j = 0; j < num_ops; ++j)
> > +		ops_enq[j]->opaque_data = (void *)(uintptr_t)j;
> > +
> > +	for (i = 0; i < 1; ++i) { /* Could add more iterations */
> > +		for (j = 0; j < num_ops; ++j) {
> > +			mbuf_reset(
> > +			ops_enq[j]->turbo_dec.hard_output.data);
> > +		}
> > +
> > +		start_time = rte_rdtsc_precise();
> > +
> > +		for (enq = 0, deq = 0; enq < num_ops;) {
> > +			num_to_enq = burst_sz;
> > +
> > +			if (unlikely(num_ops - enq < num_to_enq))
> > +				num_to_enq = num_ops - enq;
> > +
> > +			enq += rte_bbdev_enqueue_dec_ops(tp->dev_id,
> > +					queue_id, &ops_enq[enq],
> num_to_enq);
> 
> If for some reason the operation fails (looks like it can fail in at least ACC200 I
> checked), it ends up doing an endless loop as enc won't get incremented.
The python wrapper script used to invoke test-bbdev has a configurable timeout (default 600 seconds) that would stop the test in case it ends up in an infinite loop.

> > +
> > +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
> > +					queue_id, &ops_deq[deq], enq -
> deq);
> > +		}
> > +
> > +		/* dequeue the remaining */
> > +		while (deq < enq) {
> > +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
> > +					queue_id, &ops_deq[deq], enq -
> deq);
> > +		}
> > +
> > +		total_time += rte_rdtsc_precise() - start_time;
> > +	}
> > +
> > +	tp->iter_count = 0;
> > +	tp->iter_average = 0;
> > +	/* get the max of iter_count for all dequeued ops */
> > +	for (i = 0; i < num_ops; ++i) {
> > +		tp->iter_count = RTE_MAX(ops_enq[i]-
> >turbo_dec.iter_count,
> > +				tp->iter_count);
> > +		tp->iter_average += (double) ops_enq[i]-
> >turbo_dec.iter_count;
> > +	}
> > +
> > +	tp->iter_average /= num_ops;
> > +	tp->bler = (double) validate_turbo_bler(ops_deq, num_ops) /
> num_ops;
> > +
> > +	rte_bbdev_dec_op_free_bulk(ops_enq, num_ops);
> > +
> > +	double tb_len_bits = calc_dec_TB_size(ref_op);
> > +	tp->ops_per_sec = ((double)num_ops * 1) /
> > +			((double)total_time / (double)rte_get_tsc_hz());
> > +	tp->mbps = (((double)(num_ops * 1 * tb_len_bits)) /
> > +			1000000.0) / ((double)total_time /
> > +			(double)rte_get_tsc_hz());
> > +	printf("TBS %.0f Time %.0f\n", tb_len_bits, 1000000.0 *
> > +			((double)total_time / (double)rte_get_tsc_hz()));
> > +
> > +	return TEST_SUCCESS;
> > +}
> > +
> >   static int
> >   throughput_pmd_lcore_ldpc_dec(void *arg)
> >   {
> > @@ -4193,7 +4371,7 @@ print_dec_bler(struct thread_params *t_params,
> unsigned int used_cores)
> >   	total_bler /= used_cores;
> >   	total_iter /= used_cores;
> >
> > -	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.1f Mbps
> %s\n",
> > +	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.3f Mbps
> > +%s\n",
> >   			snr, total_bler * 100, total_iter, get_iter_max(),
> >   			total_mbps, get_vector_filename());
> >   }
> > @@ -4245,6 +4423,10 @@ bler_test(struct active_device *ad,
> >   			&& !check_bit(test_vector.ldpc_dec.op_flags,
> >   			RTE_BBDEV_LDPC_LLR_COMPRESSION))
> >   		bler_function = bler_pmd_lcore_ldpc_dec;
> > +	else if ((test_vector.op_type == RTE_BBDEV_OP_TURBO_DEC) &&
> > +			!check_bit(test_vector.turbo_dec.op_flags,
> > +			RTE_BBDEV_TURBO_SOFT_OUTPUT))
> > +		bler_function = bler_pmd_lcore_turbo_dec;
> >   	else
> >   		return TEST_SKIPPED;
> >

Re: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G

[Maxime Coquelin]

On 2/13/23 21:59, Vargas, Hernan wrote:
> 
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Tuesday, January 31, 2023 4:20 AM
>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>
>> Subject: Re: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G
>>
>>
>>
>> On 1/17/23 17:50, Hernan Vargas wrote:
>>> New feature to add BLER support for 4G in bbdev-test.
>>>
>>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>>> ---
>>>    app/test-bbdev/test_bbdev_perf.c | 184
>> ++++++++++++++++++++++++++++++-
>>>    1 file changed, 183 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>> b/app/test-bbdev/test_bbdev_perf.c
>>> index ffb2de0604..69b86cdeb1 100644
>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>> @@ -1756,6 +1756,30 @@ gen_qm2_llr(int8_t *llrs, uint32_t j, double N0,
>> double llr_max)
>>>    	llrs[j] = (int8_t) b;
>>>    }
>>>
>>> +/* Simple LLR generation assuming AWGN and QPSK */ static void
>>> +gen_turbo_llr(int8_t *llrs, uint32_t j, double N0, double llr_max) {
>>> +	double b, b1, n;
>>> +	double coeff = 2.0 * sqrt(N0);
>>> +
>>> +	/* Ignore in vectors null LLRs not to be saturated */
>>> +	if (llrs[j] == 0)
>>> +		return;
>>> +
>>> +	/* Note don't change sign here */
>>> +	n = randn(j % 2);
>>> +	b1 = ((llrs[j] > 0 ? 2.0 : -2.0)
>>> +			+ coeff * n) / N0;
>>> +	b = b1 * (1 << 4);
>>> +	b = round(b);
>>> +	if (b > llr_max)
>>> +		b = llr_max;
>>> +	if (b < -llr_max)
>>> +		b = -llr_max;
>>> +	llrs[j] = (int8_t) b;
>>> +}
>>> +
>>>    /* Generate LLR for a given SNR */
>>>    static void
>>>    generate_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs, @@
>>> -1791,6 +1815,27 @@ generate_llr_input(uint16_t n, struct
>> rte_bbdev_op_data *inputs,
>>>    	}
>>>    }
>>>
>>> +/* Generate LLR for turbo decoder for a given SNR */ static void
>>> +generate_turbo_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
>>> +		struct rte_bbdev_dec_op *ref_op)
>>> +{
>>> +	struct rte_mbuf *m;
>>> +	uint32_t i, j, range;
>>> +	double N0, llr_max;
>>> +
>>> +	llr_max = 127;
>>> +	range = ref_op->turbo_dec.input.length;
>>> +	N0 = 1.0 / pow(10.0, get_snr() / 10.0);
>>> +
>>> +	for (i = 0; i < n; ++i) {
>>> +		m = inputs[i].data;
>>> +		int8_t *llrs = rte_pktmbuf_mtod_offset(m, int8_t *, 0);
>>> +		for (j = 0; j < range; ++j)
>>> +			gen_turbo_llr(llrs, j, N0, llr_max);
>>> +	}
>>> +}
>>> +
>>>    static void
>>>    copy_reference_ldpc_dec_op(struct rte_bbdev_dec_op **ops, unsigned
>> int n,
>>>    		unsigned int start_idx,
>>> @@ -2301,6 +2346,31 @@ validate_ldpc_bler(struct rte_bbdev_dec_op
>> **ops, const uint16_t n)
>>>    	return errors;
>>>    }
>>>
>>> +/* Check Number of code blocks errors */ static int
>>> +validate_turbo_bler(struct rte_bbdev_dec_op **ops, const uint16_t n)
>>> +{
>>> +	unsigned int i;
>>> +	struct op_data_entries *hard_data_orig =
>>> +			&test_vector.entries[DATA_HARD_OUTPUT];
>>> +	struct rte_bbdev_op_turbo_dec *ops_td;
>>> +	struct rte_bbdev_op_data *hard_output;
>>> +	int errors = 0;
>>> +	struct rte_mbuf *m;
>>> +
>>> +	for (i = 0; i < n; ++i) {
>>> +		ops_td = &ops[i]->turbo_dec;
>>> +		hard_output = &ops_td->hard_output;
>>> +		m = hard_output->data;
>>> +		if (memcmp(rte_pktmbuf_mtod_offset(m, uint32_t *, 0),
>>> +				hard_data_orig->segments[0].addr,
>>> +				hard_data_orig->segments[0].length))
>>
>> No need to check mbuf is at least as large as segment 0 length?
>>
>>> +			errors++;
>>> +	}
>>> +	return errors;
>>> +}
>>> +
>>> +
>>>    static int
>>>    validate_ldpc_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
>>>    		struct rte_bbdev_dec_op *ref_op, const int vector_mask)
>> @@ -3736,6
>>> +3806,114 @@ bler_pmd_lcore_ldpc_dec(void *arg)
>>>    	return TEST_SUCCESS;
>>>    }
>>>
>>> +
>>> +static int
>>> +bler_pmd_lcore_turbo_dec(void *arg)
>>> +{
>>> +	struct thread_params *tp = arg;
>>> +	uint16_t enq, deq;
>>> +	uint64_t total_time = 0, start_time;
>>> +	const uint16_t queue_id = tp->queue_id;
>>> +	const uint16_t burst_sz = tp->op_params->burst_sz;
>>> +	const uint16_t num_ops = tp->op_params->num_to_process;
>>> +	struct rte_bbdev_dec_op *ops_enq[num_ops];
>>> +	struct rte_bbdev_dec_op *ops_deq[num_ops];
>>> +	struct rte_bbdev_dec_op *ref_op = tp->op_params->ref_dec_op;
>>> +	struct test_buffers *bufs = NULL;
>>> +	int i, j, ret;
>>> +	struct rte_bbdev_info info;
>>> +	uint16_t num_to_enq;
>>> +
>>> +	TEST_ASSERT_SUCCESS((burst_sz > MAX_BURST),
>>> +			"BURST_SIZE should be <= %u", MAX_BURST);
>>> +
>>> +	rte_bbdev_info_get(tp->dev_id, &info);
>>> +
>>> +	TEST_ASSERT_SUCCESS((num_ops > info.drv.queue_size_lim),
>>> +			"NUM_OPS cannot exceed %u for this device",
>>> +			info.drv.queue_size_lim);
>>> +
>>> +	bufs = &tp->op_params-
>>> q_bufs[GET_SOCKET(info.socket_id)][queue_id];
>>> +
>>> +	rte_wait_until_equal_16(&tp->op_params->sync, SYNC_START,
>>> +__ATOMIC_RELAXED);
>>> +
>>> +	ret = rte_bbdev_dec_op_alloc_bulk(tp->op_params->mp, ops_enq,
>> num_ops);
>>> +	TEST_ASSERT_SUCCESS(ret, "Allocation failed for %d ops",
>> num_ops);
>>> +
>>> +	/* For BLER tests we need to enable early termination */
>>> +	if (!check_bit(ref_op->turbo_dec.op_flags,
>>> +			RTE_BBDEV_TURBO_EARLY_TERMINATION))
>>> +		ref_op->turbo_dec.op_flags +=
>>> +				RTE_BBDEV_TURBO_EARLY_TERMINATION;
>>> +	ref_op->turbo_dec.iter_max = get_iter_max();
>>> +	ref_op->turbo_dec.iter_count = ref_op->turbo_dec.iter_max;
>>> +
>>> +	if (test_vector.op_type != RTE_BBDEV_OP_NONE)
>>> +		copy_reference_dec_op(ops_enq, num_ops, 0, bufs-
>>> inputs,
>>> +				bufs->hard_outputs, bufs->soft_outputs,
>>> +				ref_op);
>>> +	generate_turbo_llr_input(num_ops, bufs->inputs, ref_op);
>>> +
>>> +	/* Set counter to validate the ordering */
>>> +	for (j = 0; j < num_ops; ++j)
>>> +		ops_enq[j]->opaque_data = (void *)(uintptr_t)j;
>>> +
>>> +	for (i = 0; i < 1; ++i) { /* Could add more iterations */
>>> +		for (j = 0; j < num_ops; ++j) {
>>> +			mbuf_reset(
>>> +			ops_enq[j]->turbo_dec.hard_output.data);
>>> +		}
>>> +
>>> +		start_time = rte_rdtsc_precise();
>>> +
>>> +		for (enq = 0, deq = 0; enq < num_ops;) {
>>> +			num_to_enq = burst_sz;
>>> +
>>> +			if (unlikely(num_ops - enq < num_to_enq))
>>> +				num_to_enq = num_ops - enq;
>>> +
>>> +			enq += rte_bbdev_enqueue_dec_ops(tp->dev_id,
>>> +					queue_id, &ops_enq[enq],
>> num_to_enq);
>>
>> If for some reason the operation fails (looks like it can fail in at least ACC200 I
>> checked), it ends up doing an endless loop as enc won't get incremented.
> The python wrapper script used to invoke test-bbdev has a configurable timeout (default 600 seconds) that would stop the test in case it ends up in an infinite loop.

Ok, but having a timeout in the test script does not prevent to do a
proprer error propagation/handling in the application.

It is happens in when running in a CI, having errors properly checked
would provide useful information to investigate without having to
reproduce, or when failing to reproduce.

Regards,
Maxime

> 
>>> +
>>> +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
>>> +					queue_id, &ops_deq[deq], enq -
>> deq);
>>> +		}
>>> +
>>> +		/* dequeue the remaining */
>>> +		while (deq < enq) {
>>> +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
>>> +					queue_id, &ops_deq[deq], enq -
>> deq);
>>> +		}
>>> +
>>> +		total_time += rte_rdtsc_precise() - start_time;
>>> +	}
>>> +
>>> +	tp->iter_count = 0;
>>> +	tp->iter_average = 0;
>>> +	/* get the max of iter_count for all dequeued ops */
>>> +	for (i = 0; i < num_ops; ++i) {
>>> +		tp->iter_count = RTE_MAX(ops_enq[i]-
>>> turbo_dec.iter_count,
>>> +				tp->iter_count);
>>> +		tp->iter_average += (double) ops_enq[i]-
>>> turbo_dec.iter_count;
>>> +	}
>>> +
>>> +	tp->iter_average /= num_ops;
>>> +	tp->bler = (double) validate_turbo_bler(ops_deq, num_ops) /
>> num_ops;
>>> +
>>> +	rte_bbdev_dec_op_free_bulk(ops_enq, num_ops);
>>> +
>>> +	double tb_len_bits = calc_dec_TB_size(ref_op);
>>> +	tp->ops_per_sec = ((double)num_ops * 1) /
>>> +			((double)total_time / (double)rte_get_tsc_hz());
>>> +	tp->mbps = (((double)(num_ops * 1 * tb_len_bits)) /
>>> +			1000000.0) / ((double)total_time /
>>> +			(double)rte_get_tsc_hz());
>>> +	printf("TBS %.0f Time %.0f\n", tb_len_bits, 1000000.0 *
>>> +			((double)total_time / (double)rte_get_tsc_hz()));
>>> +
>>> +	return TEST_SUCCESS;
>>> +}
>>> +
>>>    static int
>>>    throughput_pmd_lcore_ldpc_dec(void *arg)
>>>    {
>>> @@ -4193,7 +4371,7 @@ print_dec_bler(struct thread_params *t_params,
>> unsigned int used_cores)
>>>    	total_bler /= used_cores;
>>>    	total_iter /= used_cores;
>>>
>>> -	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.1f Mbps
>> %s\n",
>>> +	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.3f Mbps
>>> +%s\n",
>>>    			snr, total_bler * 100, total_iter, get_iter_max(),
>>>    			total_mbps, get_vector_filename());
>>>    }
>>> @@ -4245,6 +4423,10 @@ bler_test(struct active_device *ad,
>>>    			&& !check_bit(test_vector.ldpc_dec.op_flags,
>>>    			RTE_BBDEV_LDPC_LLR_COMPRESSION))
>>>    		bler_function = bler_pmd_lcore_ldpc_dec;
>>> +	else if ((test_vector.op_type == RTE_BBDEV_OP_TURBO_DEC) &&
>>> +			!check_bit(test_vector.turbo_dec.op_flags,
>>> +			RTE_BBDEV_TURBO_SOFT_OUTPUT))
>>> +		bler_function = bler_pmd_lcore_turbo_dec;
>>>    	else
>>>    		return TEST_SKIPPED;
>>>
> 

RE: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G

[Vargas, Hernan]

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Monday, February 20, 2023 9:44 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G
> 
> 
> 
> On 2/13/23 21:59, Vargas, Hernan wrote:
> >
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Tuesday, January 31, 2023 4:20 AM
> >> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> >> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> >> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>
> >> Subject: Re: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G
> >>
> >>
> >>
> >> On 1/17/23 17:50, Hernan Vargas wrote:
> >>> New feature to add BLER support for 4G in bbdev-test.
> >>>
> >>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> >>> ---
> >>>    app/test-bbdev/test_bbdev_perf.c | 184
> >> ++++++++++++++++++++++++++++++-
> >>>    1 file changed, 183 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/app/test-bbdev/test_bbdev_perf.c
> >>> b/app/test-bbdev/test_bbdev_perf.c
> >>> index ffb2de0604..69b86cdeb1 100644
> >>> --- a/app/test-bbdev/test_bbdev_perf.c
> >>> +++ b/app/test-bbdev/test_bbdev_perf.c
> >>> @@ -1756,6 +1756,30 @@ gen_qm2_llr(int8_t *llrs, uint32_t j, double
> >>> N0,
> >> double llr_max)
> >>>    	llrs[j] = (int8_t) b;
> >>>    }
> >>>
> >>> +/* Simple LLR generation assuming AWGN and QPSK */ static void
> >>> +gen_turbo_llr(int8_t *llrs, uint32_t j, double N0, double llr_max) {
> >>> +	double b, b1, n;
> >>> +	double coeff = 2.0 * sqrt(N0);
> >>> +
> >>> +	/* Ignore in vectors null LLRs not to be saturated */
> >>> +	if (llrs[j] == 0)
> >>> +		return;
> >>> +
> >>> +	/* Note don't change sign here */
> >>> +	n = randn(j % 2);
> >>> +	b1 = ((llrs[j] > 0 ? 2.0 : -2.0)
> >>> +			+ coeff * n) / N0;
> >>> +	b = b1 * (1 << 4);
> >>> +	b = round(b);
> >>> +	if (b > llr_max)
> >>> +		b = llr_max;
> >>> +	if (b < -llr_max)
> >>> +		b = -llr_max;
> >>> +	llrs[j] = (int8_t) b;
> >>> +}
> >>> +
> >>>    /* Generate LLR for a given SNR */
> >>>    static void
> >>>    generate_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
> >>> @@
> >>> -1791,6 +1815,27 @@ generate_llr_input(uint16_t n, struct
> >> rte_bbdev_op_data *inputs,
> >>>    	}
> >>>    }
> >>>
> >>> +/* Generate LLR for turbo decoder for a given SNR */ static void
> >>> +generate_turbo_llr_input(uint16_t n, struct rte_bbdev_op_data
> *inputs,
> >>> +		struct rte_bbdev_dec_op *ref_op)
> >>> +{
> >>> +	struct rte_mbuf *m;
> >>> +	uint32_t i, j, range;
> >>> +	double N0, llr_max;
> >>> +
> >>> +	llr_max = 127;
> >>> +	range = ref_op->turbo_dec.input.length;
> >>> +	N0 = 1.0 / pow(10.0, get_snr() / 10.0);
> >>> +
> >>> +	for (i = 0; i < n; ++i) {
> >>> +		m = inputs[i].data;
> >>> +		int8_t *llrs = rte_pktmbuf_mtod_offset(m, int8_t *, 0);
> >>> +		for (j = 0; j < range; ++j)
> >>> +			gen_turbo_llr(llrs, j, N0, llr_max);
> >>> +	}
> >>> +}
> >>> +
> >>>    static void
> >>>    copy_reference_ldpc_dec_op(struct rte_bbdev_dec_op **ops,
> >>> unsigned
> >> int n,
> >>>    		unsigned int start_idx,
> >>> @@ -2301,6 +2346,31 @@ validate_ldpc_bler(struct rte_bbdev_dec_op
> >> **ops, const uint16_t n)
> >>>    	return errors;
> >>>    }
> >>>
> >>> +/* Check Number of code blocks errors */ static int
> >>> +validate_turbo_bler(struct rte_bbdev_dec_op **ops, const uint16_t
> >>> +n) {
> >>> +	unsigned int i;
> >>> +	struct op_data_entries *hard_data_orig =
> >>> +			&test_vector.entries[DATA_HARD_OUTPUT];
> >>> +	struct rte_bbdev_op_turbo_dec *ops_td;
> >>> +	struct rte_bbdev_op_data *hard_output;
> >>> +	int errors = 0;
> >>> +	struct rte_mbuf *m;
> >>> +
> >>> +	for (i = 0; i < n; ++i) {
> >>> +		ops_td = &ops[i]->turbo_dec;
> >>> +		hard_output = &ops_td->hard_output;
> >>> +		m = hard_output->data;
> >>> +		if (memcmp(rte_pktmbuf_mtod_offset(m, uint32_t *, 0),
> >>> +				hard_data_orig->segments[0].addr,
> >>> +				hard_data_orig->segments[0].length))
> >>
> >> No need to check mbuf is at least as large as segment 0 length?
> >>
> >>> +			errors++;
> >>> +	}
> >>> +	return errors;
> >>> +}
> >>> +
> >>> +
> >>>    static int
> >>>    validate_ldpc_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t
> n,
> >>>    		struct rte_bbdev_dec_op *ref_op, const int vector_mask)
> >> @@ -3736,6
> >>> +3806,114 @@ bler_pmd_lcore_ldpc_dec(void *arg)
> >>>    	return TEST_SUCCESS;
> >>>    }
> >>>
> >>> +
> >>> +static int
> >>> +bler_pmd_lcore_turbo_dec(void *arg) {
> >>> +	struct thread_params *tp = arg;
> >>> +	uint16_t enq, deq;
> >>> +	uint64_t total_time = 0, start_time;
> >>> +	const uint16_t queue_id = tp->queue_id;
> >>> +	const uint16_t burst_sz = tp->op_params->burst_sz;
> >>> +	const uint16_t num_ops = tp->op_params->num_to_process;
> >>> +	struct rte_bbdev_dec_op *ops_enq[num_ops];
> >>> +	struct rte_bbdev_dec_op *ops_deq[num_ops];
> >>> +	struct rte_bbdev_dec_op *ref_op = tp->op_params->ref_dec_op;
> >>> +	struct test_buffers *bufs = NULL;
> >>> +	int i, j, ret;
> >>> +	struct rte_bbdev_info info;
> >>> +	uint16_t num_to_enq;
> >>> +
> >>> +	TEST_ASSERT_SUCCESS((burst_sz > MAX_BURST),
> >>> +			"BURST_SIZE should be <= %u", MAX_BURST);
> >>> +
> >>> +	rte_bbdev_info_get(tp->dev_id, &info);
> >>> +
> >>> +	TEST_ASSERT_SUCCESS((num_ops > info.drv.queue_size_lim),
> >>> +			"NUM_OPS cannot exceed %u for this device",
> >>> +			info.drv.queue_size_lim);
> >>> +
> >>> +	bufs = &tp->op_params-
> >>> q_bufs[GET_SOCKET(info.socket_id)][queue_id];
> >>> +
> >>> +	rte_wait_until_equal_16(&tp->op_params->sync, SYNC_START,
> >>> +__ATOMIC_RELAXED);
> >>> +
> >>> +	ret = rte_bbdev_dec_op_alloc_bulk(tp->op_params->mp, ops_enq,
> >> num_ops);
> >>> +	TEST_ASSERT_SUCCESS(ret, "Allocation failed for %d ops",
> >> num_ops);
> >>> +
> >>> +	/* For BLER tests we need to enable early termination */
> >>> +	if (!check_bit(ref_op->turbo_dec.op_flags,
> >>> +			RTE_BBDEV_TURBO_EARLY_TERMINATION))
> >>> +		ref_op->turbo_dec.op_flags +=
> >>> +				RTE_BBDEV_TURBO_EARLY_TERMINATION;
> >>> +	ref_op->turbo_dec.iter_max = get_iter_max();
> >>> +	ref_op->turbo_dec.iter_count = ref_op->turbo_dec.iter_max;
> >>> +
> >>> +	if (test_vector.op_type != RTE_BBDEV_OP_NONE)
> >>> +		copy_reference_dec_op(ops_enq, num_ops, 0, bufs-
> >>> inputs,
> >>> +				bufs->hard_outputs, bufs->soft_outputs,
> >>> +				ref_op);
> >>> +	generate_turbo_llr_input(num_ops, bufs->inputs, ref_op);
> >>> +
> >>> +	/* Set counter to validate the ordering */
> >>> +	for (j = 0; j < num_ops; ++j)
> >>> +		ops_enq[j]->opaque_data = (void *)(uintptr_t)j;
> >>> +
> >>> +	for (i = 0; i < 1; ++i) { /* Could add more iterations */
> >>> +		for (j = 0; j < num_ops; ++j) {
> >>> +			mbuf_reset(
> >>> +			ops_enq[j]->turbo_dec.hard_output.data);
> >>> +		}
> >>> +
> >>> +		start_time = rte_rdtsc_precise();
> >>> +
> >>> +		for (enq = 0, deq = 0; enq < num_ops;) {
> >>> +			num_to_enq = burst_sz;
> >>> +
> >>> +			if (unlikely(num_ops - enq < num_to_enq))
> >>> +				num_to_enq = num_ops - enq;
> >>> +
> >>> +			enq += rte_bbdev_enqueue_dec_ops(tp->dev_id,
> >>> +					queue_id, &ops_enq[enq],
> >> num_to_enq);
> >>
> >> If for some reason the operation fails (looks like it can fail in at
> >> least ACC200 I checked), it ends up doing an endless loop as enc won't get
> incremented.
> > The python wrapper script used to invoke test-bbdev has a configurable
> timeout (default 600 seconds) that would stop the test in case it ends up in
> an infinite loop.
> 
> Ok, but having a timeout in the test script does not prevent to do a proprer
> error propagation/handling in the application.
> 
> It is happens in when running in a CI, having errors properly checked would
> provide useful information to investigate without having to reproduce, or
> when failing to reproduce.
> 
> Regards,
> Maxime
> 
This is a good point for future improvement but not related to this series.
We can implement it in the future. What do you think?

Thanks,
Hernan
> >
> >>> +
> >>> +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
> >>> +					queue_id, &ops_deq[deq], enq -
> >> deq);
> >>> +		}
> >>> +
> >>> +		/* dequeue the remaining */
> >>> +		while (deq < enq) {
> >>> +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
> >>> +					queue_id, &ops_deq[deq], enq -
> >> deq);
> >>> +		}
> >>> +
> >>> +		total_time += rte_rdtsc_precise() - start_time;
> >>> +	}
> >>> +
> >>> +	tp->iter_count = 0;
> >>> +	tp->iter_average = 0;
> >>> +	/* get the max of iter_count for all dequeued ops */
> >>> +	for (i = 0; i < num_ops; ++i) {
> >>> +		tp->iter_count = RTE_MAX(ops_enq[i]-
> >>> turbo_dec.iter_count,
> >>> +				tp->iter_count);
> >>> +		tp->iter_average += (double) ops_enq[i]-
> >>> turbo_dec.iter_count;
> >>> +	}
> >>> +
> >>> +	tp->iter_average /= num_ops;
> >>> +	tp->bler = (double) validate_turbo_bler(ops_deq, num_ops) /
> >> num_ops;
> >>> +
> >>> +	rte_bbdev_dec_op_free_bulk(ops_enq, num_ops);
> >>> +
> >>> +	double tb_len_bits = calc_dec_TB_size(ref_op);
> >>> +	tp->ops_per_sec = ((double)num_ops * 1) /
> >>> +			((double)total_time / (double)rte_get_tsc_hz());
> >>> +	tp->mbps = (((double)(num_ops * 1 * tb_len_bits)) /
> >>> +			1000000.0) / ((double)total_time /
> >>> +			(double)rte_get_tsc_hz());
> >>> +	printf("TBS %.0f Time %.0f\n", tb_len_bits, 1000000.0 *
> >>> +			((double)total_time / (double)rte_get_tsc_hz()));
> >>> +
> >>> +	return TEST_SUCCESS;
> >>> +}
> >>> +
> >>>    static int
> >>>    throughput_pmd_lcore_ldpc_dec(void *arg)
> >>>    {
> >>> @@ -4193,7 +4371,7 @@ print_dec_bler(struct thread_params
> *t_params,
> >> unsigned int used_cores)
> >>>    	total_bler /= used_cores;
> >>>    	total_iter /= used_cores;
> >>>
> >>> -	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.1f Mbps
> >> %s\n",
> >>> +	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.3f Mbps
> >>> +%s\n",
> >>>    			snr, total_bler * 100, total_iter, get_iter_max(),
> >>>    			total_mbps, get_vector_filename());
> >>>    }
> >>> @@ -4245,6 +4423,10 @@ bler_test(struct active_device *ad,
> >>>    			&& !check_bit(test_vector.ldpc_dec.op_flags,
> >>>    			RTE_BBDEV_LDPC_LLR_COMPRESSION))
> >>>    		bler_function = bler_pmd_lcore_ldpc_dec;
> >>> +	else if ((test_vector.op_type == RTE_BBDEV_OP_TURBO_DEC) &&
> >>> +			!check_bit(test_vector.turbo_dec.op_flags,
> >>> +			RTE_BBDEV_TURBO_SOFT_OUTPUT))
> >>> +		bler_function = bler_pmd_lcore_turbo_dec;
> >>>    	else
> >>>    		return TEST_SKIPPED;
> >>>
> >

Re: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G

[Maxime Coquelin]

Hi Hernan,

On 2/22/23 22:55, Vargas, Hernan wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Monday, February 20, 2023 9:44 AM
>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>
>> Subject: Re: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G
>>
>>
>>
>> On 2/13/23 21:59, Vargas, Hernan wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Sent: Tuesday, January 31, 2023 4:20 AM
>>>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>>>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>>>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>>>> <qi.z.zhang@intel.com>
>>>> Subject: Re: [PATCH v1 07/13] test/bbdev: add support for BLER for 4G
>>>>
>>>>
>>>>
>>>> On 1/17/23 17:50, Hernan Vargas wrote:
>>>>> New feature to add BLER support for 4G in bbdev-test.
>>>>>
>>>>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>>>>> ---
>>>>>     app/test-bbdev/test_bbdev_perf.c | 184
>>>> ++++++++++++++++++++++++++++++-
>>>>>     1 file changed, 183 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>>>> b/app/test-bbdev/test_bbdev_perf.c
>>>>> index ffb2de0604..69b86cdeb1 100644
>>>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>>>> @@ -1756,6 +1756,30 @@ gen_qm2_llr(int8_t *llrs, uint32_t j, double
>>>>> N0,
>>>> double llr_max)
>>>>>     	llrs[j] = (int8_t) b;
>>>>>     }
>>>>>
>>>>> +/* Simple LLR generation assuming AWGN and QPSK */ static void
>>>>> +gen_turbo_llr(int8_t *llrs, uint32_t j, double N0, double llr_max) {
>>>>> +	double b, b1, n;
>>>>> +	double coeff = 2.0 * sqrt(N0);
>>>>> +
>>>>> +	/* Ignore in vectors null LLRs not to be saturated */
>>>>> +	if (llrs[j] == 0)
>>>>> +		return;
>>>>> +
>>>>> +	/* Note don't change sign here */
>>>>> +	n = randn(j % 2);
>>>>> +	b1 = ((llrs[j] > 0 ? 2.0 : -2.0)
>>>>> +			+ coeff * n) / N0;
>>>>> +	b = b1 * (1 << 4);
>>>>> +	b = round(b);
>>>>> +	if (b > llr_max)
>>>>> +		b = llr_max;
>>>>> +	if (b < -llr_max)
>>>>> +		b = -llr_max;
>>>>> +	llrs[j] = (int8_t) b;
>>>>> +}
>>>>> +
>>>>>     /* Generate LLR for a given SNR */
>>>>>     static void
>>>>>     generate_llr_input(uint16_t n, struct rte_bbdev_op_data *inputs,
>>>>> @@
>>>>> -1791,6 +1815,27 @@ generate_llr_input(uint16_t n, struct
>>>> rte_bbdev_op_data *inputs,
>>>>>     	}
>>>>>     }
>>>>>
>>>>> +/* Generate LLR for turbo decoder for a given SNR */ static void
>>>>> +generate_turbo_llr_input(uint16_t n, struct rte_bbdev_op_data
>> *inputs,
>>>>> +		struct rte_bbdev_dec_op *ref_op)
>>>>> +{
>>>>> +	struct rte_mbuf *m;
>>>>> +	uint32_t i, j, range;
>>>>> +	double N0, llr_max;
>>>>> +
>>>>> +	llr_max = 127;
>>>>> +	range = ref_op->turbo_dec.input.length;
>>>>> +	N0 = 1.0 / pow(10.0, get_snr() / 10.0);
>>>>> +
>>>>> +	for (i = 0; i < n; ++i) {
>>>>> +		m = inputs[i].data;
>>>>> +		int8_t *llrs = rte_pktmbuf_mtod_offset(m, int8_t *, 0);
>>>>> +		for (j = 0; j < range; ++j)
>>>>> +			gen_turbo_llr(llrs, j, N0, llr_max);
>>>>> +	}
>>>>> +}
>>>>> +
>>>>>     static void
>>>>>     copy_reference_ldpc_dec_op(struct rte_bbdev_dec_op **ops,
>>>>> unsigned
>>>> int n,
>>>>>     		unsigned int start_idx,
>>>>> @@ -2301,6 +2346,31 @@ validate_ldpc_bler(struct rte_bbdev_dec_op
>>>> **ops, const uint16_t n)
>>>>>     	return errors;
>>>>>     }
>>>>>
>>>>> +/* Check Number of code blocks errors */ static int
>>>>> +validate_turbo_bler(struct rte_bbdev_dec_op **ops, const uint16_t
>>>>> +n) {
>>>>> +	unsigned int i;
>>>>> +	struct op_data_entries *hard_data_orig =
>>>>> +			&test_vector.entries[DATA_HARD_OUTPUT];
>>>>> +	struct rte_bbdev_op_turbo_dec *ops_td;
>>>>> +	struct rte_bbdev_op_data *hard_output;
>>>>> +	int errors = 0;
>>>>> +	struct rte_mbuf *m;
>>>>> +
>>>>> +	for (i = 0; i < n; ++i) {
>>>>> +		ops_td = &ops[i]->turbo_dec;
>>>>> +		hard_output = &ops_td->hard_output;
>>>>> +		m = hard_output->data;
>>>>> +		if (memcmp(rte_pktmbuf_mtod_offset(m, uint32_t *, 0),
>>>>> +				hard_data_orig->segments[0].addr,
>>>>> +				hard_data_orig->segments[0].length))
>>>>
>>>> No need to check mbuf is at least as large as segment 0 length?
>>>>
>>>>> +			errors++;
>>>>> +	}
>>>>> +	return errors;
>>>>> +}
>>>>> +
>>>>> +
>>>>>     static int
>>>>>     validate_ldpc_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t
>> n,
>>>>>     		struct rte_bbdev_dec_op *ref_op, const int vector_mask)
>>>> @@ -3736,6
>>>>> +3806,114 @@ bler_pmd_lcore_ldpc_dec(void *arg)
>>>>>     	return TEST_SUCCESS;
>>>>>     }
>>>>>
>>>>> +
>>>>> +static int
>>>>> +bler_pmd_lcore_turbo_dec(void *arg) {
>>>>> +	struct thread_params *tp = arg;
>>>>> +	uint16_t enq, deq;
>>>>> +	uint64_t total_time = 0, start_time;
>>>>> +	const uint16_t queue_id = tp->queue_id;
>>>>> +	const uint16_t burst_sz = tp->op_params->burst_sz;
>>>>> +	const uint16_t num_ops = tp->op_params->num_to_process;
>>>>> +	struct rte_bbdev_dec_op *ops_enq[num_ops];
>>>>> +	struct rte_bbdev_dec_op *ops_deq[num_ops];
>>>>> +	struct rte_bbdev_dec_op *ref_op = tp->op_params->ref_dec_op;
>>>>> +	struct test_buffers *bufs = NULL;
>>>>> +	int i, j, ret;
>>>>> +	struct rte_bbdev_info info;
>>>>> +	uint16_t num_to_enq;
>>>>> +
>>>>> +	TEST_ASSERT_SUCCESS((burst_sz > MAX_BURST),
>>>>> +			"BURST_SIZE should be <= %u", MAX_BURST);
>>>>> +
>>>>> +	rte_bbdev_info_get(tp->dev_id, &info);
>>>>> +
>>>>> +	TEST_ASSERT_SUCCESS((num_ops > info.drv.queue_size_lim),
>>>>> +			"NUM_OPS cannot exceed %u for this device",
>>>>> +			info.drv.queue_size_lim);
>>>>> +
>>>>> +	bufs = &tp->op_params-
>>>>> q_bufs[GET_SOCKET(info.socket_id)][queue_id];
>>>>> +
>>>>> +	rte_wait_until_equal_16(&tp->op_params->sync, SYNC_START,
>>>>> +__ATOMIC_RELAXED);
>>>>> +
>>>>> +	ret = rte_bbdev_dec_op_alloc_bulk(tp->op_params->mp, ops_enq,
>>>> num_ops);
>>>>> +	TEST_ASSERT_SUCCESS(ret, "Allocation failed for %d ops",
>>>> num_ops);
>>>>> +
>>>>> +	/* For BLER tests we need to enable early termination */
>>>>> +	if (!check_bit(ref_op->turbo_dec.op_flags,
>>>>> +			RTE_BBDEV_TURBO_EARLY_TERMINATION))
>>>>> +		ref_op->turbo_dec.op_flags +=
>>>>> +				RTE_BBDEV_TURBO_EARLY_TERMINATION;
>>>>> +	ref_op->turbo_dec.iter_max = get_iter_max();
>>>>> +	ref_op->turbo_dec.iter_count = ref_op->turbo_dec.iter_max;
>>>>> +
>>>>> +	if (test_vector.op_type != RTE_BBDEV_OP_NONE)
>>>>> +		copy_reference_dec_op(ops_enq, num_ops, 0, bufs-
>>>>> inputs,
>>>>> +				bufs->hard_outputs, bufs->soft_outputs,
>>>>> +				ref_op);
>>>>> +	generate_turbo_llr_input(num_ops, bufs->inputs, ref_op);
>>>>> +
>>>>> +	/* Set counter to validate the ordering */
>>>>> +	for (j = 0; j < num_ops; ++j)
>>>>> +		ops_enq[j]->opaque_data = (void *)(uintptr_t)j;
>>>>> +
>>>>> +	for (i = 0; i < 1; ++i) { /* Could add more iterations */
>>>>> +		for (j = 0; j < num_ops; ++j) {
>>>>> +			mbuf_reset(
>>>>> +			ops_enq[j]->turbo_dec.hard_output.data);
>>>>> +		}
>>>>> +
>>>>> +		start_time = rte_rdtsc_precise();
>>>>> +
>>>>> +		for (enq = 0, deq = 0; enq < num_ops;) {
>>>>> +			num_to_enq = burst_sz;
>>>>> +
>>>>> +			if (unlikely(num_ops - enq < num_to_enq))
>>>>> +				num_to_enq = num_ops - enq;
>>>>> +
>>>>> +			enq += rte_bbdev_enqueue_dec_ops(tp->dev_id,
>>>>> +					queue_id, &ops_enq[enq],
>>>> num_to_enq);
>>>>
>>>> If for some reason the operation fails (looks like it can fail in at
>>>> least ACC200 I checked), it ends up doing an endless loop as enc won't get
>> incremented.
>>> The python wrapper script used to invoke test-bbdev has a configurable
>> timeout (default 600 seconds) that would stop the test in case it ends up in
>> an infinite loop.
>>
>> Ok, but having a timeout in the test script does not prevent to do a proprer
>> error propagation/handling in the application.
>>
>> It is happens in when running in a CI, having errors properly checked would
>> provide useful information to investigate without having to reproduce, or
>> when failing to reproduce.
>>
>> Regards,
>> Maxime
>>
> This is a good point for future improvement but not related to this series.
> We can implement it in the future. What do you think?

This patch is introducing a new feature, there is a bug in this exact
feature. So this should be fixed in this patch, not in a future release.

Thanks,
Maxime

> Thanks,
> Hernan
>>>
>>>>> +
>>>>> +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
>>>>> +					queue_id, &ops_deq[deq], enq -
>>>> deq);
>>>>> +		}
>>>>> +
>>>>> +		/* dequeue the remaining */
>>>>> +		while (deq < enq) {
>>>>> +			deq += rte_bbdev_dequeue_dec_ops(tp->dev_id,
>>>>> +					queue_id, &ops_deq[deq], enq -
>>>> deq);
>>>>> +		}
>>>>> +
>>>>> +		total_time += rte_rdtsc_precise() - start_time;
>>>>> +	}
>>>>> +
>>>>> +	tp->iter_count = 0;
>>>>> +	tp->iter_average = 0;
>>>>> +	/* get the max of iter_count for all dequeued ops */
>>>>> +	for (i = 0; i < num_ops; ++i) {
>>>>> +		tp->iter_count = RTE_MAX(ops_enq[i]-
>>>>> turbo_dec.iter_count,
>>>>> +				tp->iter_count);
>>>>> +		tp->iter_average += (double) ops_enq[i]-
>>>>> turbo_dec.iter_count;
>>>>> +	}
>>>>> +
>>>>> +	tp->iter_average /= num_ops;
>>>>> +	tp->bler = (double) validate_turbo_bler(ops_deq, num_ops) /
>>>> num_ops;
>>>>> +
>>>>> +	rte_bbdev_dec_op_free_bulk(ops_enq, num_ops);
>>>>> +
>>>>> +	double tb_len_bits = calc_dec_TB_size(ref_op);
>>>>> +	tp->ops_per_sec = ((double)num_ops * 1) /
>>>>> +			((double)total_time / (double)rte_get_tsc_hz());
>>>>> +	tp->mbps = (((double)(num_ops * 1 * tb_len_bits)) /
>>>>> +			1000000.0) / ((double)total_time /
>>>>> +			(double)rte_get_tsc_hz());
>>>>> +	printf("TBS %.0f Time %.0f\n", tb_len_bits, 1000000.0 *
>>>>> +			((double)total_time / (double)rte_get_tsc_hz()));
>>>>> +
>>>>> +	return TEST_SUCCESS;
>>>>> +}
>>>>> +
>>>>>     static int
>>>>>     throughput_pmd_lcore_ldpc_dec(void *arg)
>>>>>     {
>>>>> @@ -4193,7 +4371,7 @@ print_dec_bler(struct thread_params
>> *t_params,
>>>> unsigned int used_cores)
>>>>>     	total_bler /= used_cores;
>>>>>     	total_iter /= used_cores;
>>>>>
>>>>> -	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.1f Mbps
>>>> %s\n",
>>>>> +	printf("SNR %.2f BLER %.1f %% - Iterations %.1f %d - Tp %.3f Mbps
>>>>> +%s\n",
>>>>>     			snr, total_bler * 100, total_iter, get_iter_max(),
>>>>>     			total_mbps, get_vector_filename());
>>>>>     }
>>>>> @@ -4245,6 +4423,10 @@ bler_test(struct active_device *ad,
>>>>>     			&& !check_bit(test_vector.ldpc_dec.op_flags,
>>>>>     			RTE_BBDEV_LDPC_LLR_COMPRESSION))
>>>>>     		bler_function = bler_pmd_lcore_ldpc_dec;
>>>>> +	else if ((test_vector.op_type == RTE_BBDEV_OP_TURBO_DEC) &&
>>>>> +			!check_bit(test_vector.turbo_dec.op_flags,
>>>>> +			RTE_BBDEV_TURBO_SOFT_OUTPUT))
>>>>> +		bler_function = bler_pmd_lcore_turbo_dec;
>>>>>     	else
>>>>>     		return TEST_SKIPPED;
>>>>>
>>>
> 

[PATCH v1 08/13] test/bbdev: extend support for large TB

[Hernan Vargas]

Add support for large TB when it cannot fit into a true mbuf.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 app/test-bbdev/test_bbdev_perf.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 69b86cdeb1..fdf7a28ba2 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -1072,8 +1072,6 @@ init_op_data_objs(struct rte_bbdev_op_data *bufs,
 			 * Special case when DPDK mbuf cannot handle
 			 * the required input size
 			 */
-			printf("Warning: Larger input size than DPDK mbuf %d\n",
-					seg->length);
 			large_input = true;
 		}
 		bufs[i].data = m_head;
@@ -2030,6 +2028,7 @@ validate_op_chain(struct rte_bbdev_op_data *op,
 	struct rte_mbuf *m = op->data;
 	uint8_t nb_dst_segments = orig_op->nb_segments;
 	uint32_t total_data_size = 0;
+	bool ignore_mbuf = false; /* ignore mbuf limitations */
 
 	TEST_ASSERT(nb_dst_segments == m->nb_segs,
 			"Number of segments differ in original (%u) and filled (%u) op",
@@ -2042,21 +2041,25 @@ validate_op_chain(struct rte_bbdev_op_data *op,
 		uint16_t data_len = rte_pktmbuf_data_len(m) - offset;
 		total_data_size += orig_op->segments[i].length;
 
-		TEST_ASSERT(orig_op->segments[i].length == data_len,
-				"Length of segment differ in original (%u) and filled (%u) op",
-				orig_op->segments[i].length, data_len);
+		if (orig_op->segments[i].length > RTE_BBDEV_LDPC_E_MAX_MBUF)
+			ignore_mbuf = true;
+		if (!ignore_mbuf)
+			TEST_ASSERT(orig_op->segments[i].length == data_len,
+					"Length of segment differ in original (%u) and filled (%u) op",
+					orig_op->segments[i].length, data_len);
 		TEST_ASSERT_BUFFERS_ARE_EQUAL(orig_op->segments[i].addr,
 				rte_pktmbuf_mtod_offset(m, uint32_t *, offset),
-				data_len,
+				orig_op->segments[i].length,
 				"Output buffers (CB=%u) are not equal", i);
 		m = m->next;
 	}
 
 	/* Validate total mbuf pkt length */
 	uint32_t pkt_len = rte_pktmbuf_pkt_len(op->data) - op->offset;
-	TEST_ASSERT(total_data_size == pkt_len,
-			"Length of data differ in original (%u) and filled (%u) op",
-			total_data_size, pkt_len);
+	if (!ignore_mbuf)
+		TEST_ASSERT(total_data_size == pkt_len,
+				"Length of data differ in original (%u) and filled (%u) op",
+				total_data_size, pkt_len);
 
 	return TEST_SUCCESS;
 }
-- 
2.37.1

Re: [PATCH v1 08/13] test/bbdev: extend support for large TB

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> Add support for large TB when it cannot fit into a true mbuf.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 69b86cdeb1..fdf7a28ba2 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -1072,8 +1072,6 @@ init_op_data_objs(struct rte_bbdev_op_data *bufs,
>   			 * Special case when DPDK mbuf cannot handle
>   			 * the required input size
>   			 */
> -			printf("Warning: Larger input size than DPDK mbuf %d\n",
> -					seg->length);
>   			large_input = true;
>   		}
>   		bufs[i].data = m_head;
> @@ -2030,6 +2028,7 @@ validate_op_chain(struct rte_bbdev_op_data *op,
>   	struct rte_mbuf *m = op->data;
>   	uint8_t nb_dst_segments = orig_op->nb_segments;
>   	uint32_t total_data_size = 0;
> +	bool ignore_mbuf = false; /* ignore mbuf limitations */
>   
>   	TEST_ASSERT(nb_dst_segments == m->nb_segs,
>   			"Number of segments differ in original (%u) and filled (%u) op",
> @@ -2042,21 +2041,25 @@ validate_op_chain(struct rte_bbdev_op_data *op,
>   		uint16_t data_len = rte_pktmbuf_data_len(m) - offset;
>   		total_data_size += orig_op->segments[i].length;
>   
> -		TEST_ASSERT(orig_op->segments[i].length == data_len,
> -				"Length of segment differ in original (%u) and filled (%u) op",
> -				orig_op->segments[i].length, data_len);
> +		if (orig_op->segments[i].length > RTE_BBDEV_LDPC_E_MAX_MBUF)
> +			ignore_mbuf = true;
> +		if (!ignore_mbuf)
> +			TEST_ASSERT(orig_op->segments[i].length == data_len,
> +					"Length of segment differ in original (%u) and filled (%u) op",
> +					orig_op->segments[i].length, data_len);
>   		TEST_ASSERT_BUFFERS_ARE_EQUAL(orig_op->segments[i].addr,
>   				rte_pktmbuf_mtod_offset(m, uint32_t *, offset),
> -				data_len,
> +				orig_op->segments[i].length,

Isn't it ending up in performing out of bounds access in the mbuf?

>   				"Output buffers (CB=%u) are not equal", i);
>   		m = m->next;
>   	}
>   
>   	/* Validate total mbuf pkt length */
>   	uint32_t pkt_len = rte_pktmbuf_pkt_len(op->data) - op->offset;
> -	TEST_ASSERT(total_data_size == pkt_len,
> -			"Length of data differ in original (%u) and filled (%u) op",
> -			total_data_size, pkt_len);
> +	if (!ignore_mbuf)
> +		TEST_ASSERT(total_data_size == pkt_len,
> +				"Length of data differ in original (%u) and filled (%u) op",
> +				total_data_size, pkt_len);
>   
>   	return TEST_SUCCESS;
>   }

RE: [PATCH v1 08/13] test/bbdev: extend support for large TB

[Vargas, Hernan]

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, January 31, 2023 5:29 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v1 08/13] test/bbdev: extend support for large TB
> 
> 
> 
> On 1/17/23 17:50, Hernan Vargas wrote:
> > Add support for large TB when it cannot fit into a true mbuf.
> >
> > Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> > ---
> >   app/test-bbdev/test_bbdev_perf.c | 21 ++++++++++++---------
> >   1 file changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/app/test-bbdev/test_bbdev_perf.c
> > b/app/test-bbdev/test_bbdev_perf.c
> > index 69b86cdeb1..fdf7a28ba2 100644
> > --- a/app/test-bbdev/test_bbdev_perf.c
> > +++ b/app/test-bbdev/test_bbdev_perf.c
> > @@ -1072,8 +1072,6 @@ init_op_data_objs(struct rte_bbdev_op_data
> *bufs,
> >   			 * Special case when DPDK mbuf cannot handle
> >   			 * the required input size
> >   			 */
> > -			printf("Warning: Larger input size than DPDK mbuf
> %d\n",
> > -					seg->length);
> >   			large_input = true;
> >   		}
> >   		bufs[i].data = m_head;
> > @@ -2030,6 +2028,7 @@ validate_op_chain(struct rte_bbdev_op_data
> *op,
> >   	struct rte_mbuf *m = op->data;
> >   	uint8_t nb_dst_segments = orig_op->nb_segments;
> >   	uint32_t total_data_size = 0;
> > +	bool ignore_mbuf = false; /* ignore mbuf limitations */
> >
> >   	TEST_ASSERT(nb_dst_segments == m->nb_segs,
> >   			"Number of segments differ in original (%u) and filled
> (%u) op",
> > @@ -2042,21 +2041,25 @@ validate_op_chain(struct rte_bbdev_op_data
> *op,
> >   		uint16_t data_len = rte_pktmbuf_data_len(m) - offset;
> >   		total_data_size += orig_op->segments[i].length;
> >
> > -		TEST_ASSERT(orig_op->segments[i].length == data_len,
> > -				"Length of segment differ in original (%u) and
> filled (%u) op",
> > -				orig_op->segments[i].length, data_len);
> > +		if (orig_op->segments[i].length >
> RTE_BBDEV_LDPC_E_MAX_MBUF)
> > +			ignore_mbuf = true;
> > +		if (!ignore_mbuf)
> > +			TEST_ASSERT(orig_op->segments[i].length ==
> data_len,
> > +					"Length of segment differ in original
> (%u) and filled (%u) op",
> > +					orig_op->segments[i].length,
> data_len);
> >   		TEST_ASSERT_BUFFERS_ARE_EQUAL(orig_op-
> >segments[i].addr,
> >   				rte_pktmbuf_mtod_offset(m, uint32_t *,
> offset),
> > -				data_len,
> > +				orig_op->segments[i].length,
> 
> Isn't it ending up in performing out of bounds access in the mbuf?

No, in the case when ignore_mbuf is set to true, we use a "fake" mbuf allocated in memory with rte_malloc.
The size allocated is segments[i].length.
Thanks

> >   				"Output buffers (CB=%u) are not equal", i);
> >   		m = m->next;
> >   	}
> >
> >   	/* Validate total mbuf pkt length */
> >   	uint32_t pkt_len = rte_pktmbuf_pkt_len(op->data) - op->offset;
> > -	TEST_ASSERT(total_data_size == pkt_len,
> > -			"Length of data differ in original (%u) and filled (%u)
> op",
> > -			total_data_size, pkt_len);
> > +	if (!ignore_mbuf)
> > +		TEST_ASSERT(total_data_size == pkt_len,
> > +				"Length of data differ in original (%u) and
> filled (%u) op",
> > +				total_data_size, pkt_len);
> >
> >   	return TEST_SUCCESS;
> >   }

Re: [PATCH v1 08/13] test/bbdev: extend support for large TB

[Maxime Coquelin]

On 2/13/23 21:20, Vargas, Hernan wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Tuesday, January 31, 2023 5:29 AM
>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>
>> Subject: Re: [PATCH v1 08/13] test/bbdev: extend support for large TB
>>
>>
>>
>> On 1/17/23 17:50, Hernan Vargas wrote:
>>> Add support for large TB when it cannot fit into a true mbuf.
>>>
>>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>>> ---
>>>    app/test-bbdev/test_bbdev_perf.c | 21 ++++++++++++---------
>>>    1 file changed, 12 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>> b/app/test-bbdev/test_bbdev_perf.c
>>> index 69b86cdeb1..fdf7a28ba2 100644
>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>> @@ -1072,8 +1072,6 @@ init_op_data_objs(struct rte_bbdev_op_data
>> *bufs,
>>>    			 * Special case when DPDK mbuf cannot handle
>>>    			 * the required input size
>>>    			 */
>>> -			printf("Warning: Larger input size than DPDK mbuf
>> %d\n",
>>> -					seg->length);
>>>    			large_input = true;
>>>    		}
>>>    		bufs[i].data = m_head;
>>> @@ -2030,6 +2028,7 @@ validate_op_chain(struct rte_bbdev_op_data
>> *op,
>>>    	struct rte_mbuf *m = op->data;
>>>    	uint8_t nb_dst_segments = orig_op->nb_segments;
>>>    	uint32_t total_data_size = 0;
>>> +	bool ignore_mbuf = false; /* ignore mbuf limitations */
>>>
>>>    	TEST_ASSERT(nb_dst_segments == m->nb_segs,
>>>    			"Number of segments differ in original (%u) and filled
>> (%u) op",
>>> @@ -2042,21 +2041,25 @@ validate_op_chain(struct rte_bbdev_op_data
>> *op,
>>>    		uint16_t data_len = rte_pktmbuf_data_len(m) - offset;
>>>    		total_data_size += orig_op->segments[i].length;
>>>
>>> -		TEST_ASSERT(orig_op->segments[i].length == data_len,
>>> -				"Length of segment differ in original (%u) and
>> filled (%u) op",
>>> -				orig_op->segments[i].length, data_len);
>>> +		if (orig_op->segments[i].length >
>> RTE_BBDEV_LDPC_E_MAX_MBUF)
>>> +			ignore_mbuf = true;
>>> +		if (!ignore_mbuf)
>>> +			TEST_ASSERT(orig_op->segments[i].length ==
>> data_len,
>>> +					"Length of segment differ in original
>> (%u) and filled (%u) op",
>>> +					orig_op->segments[i].length,
>> data_len);
>>>    		TEST_ASSERT_BUFFERS_ARE_EQUAL(orig_op-
>>> segments[i].addr,
>>>    				rte_pktmbuf_mtod_offset(m, uint32_t *,
>> offset),
>>> -				data_len,
>>> +				orig_op->segments[i].length,
>>
>> Isn't it ending up in performing out of bounds access in the mbuf?
> 
> No, in the case when ignore_mbuf is set to true, we use a "fake" mbuf allocated in memory with rte_malloc.
> The size allocated is segments[i].length.

Ok.

Thanks,
Maxime

> Thanks
> 
>>>    				"Output buffers (CB=%u) are not equal", i);
>>>    		m = m->next;
>>>    	}
>>>
>>>    	/* Validate total mbuf pkt length */
>>>    	uint32_t pkt_len = rte_pktmbuf_pkt_len(op->data) - op->offset;
>>> -	TEST_ASSERT(total_data_size == pkt_len,
>>> -			"Length of data differ in original (%u) and filled (%u)
>> op",
>>> -			total_data_size, pkt_len);
>>> +	if (!ignore_mbuf)
>>> +		TEST_ASSERT(total_data_size == pkt_len,
>>> +				"Length of data differ in original (%u) and
>> filled (%u) op",
>>> +				total_data_size, pkt_len);
>>>
>>>    	return TEST_SUCCESS;
>>>    }
> 

[PATCH v1 09/13] test/bbdev: bbdev-test cannot compare some scenarios

[Hernan Vargas]

Updating logic for compression usecases.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 app/test-bbdev/test_bbdev_perf.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index fdf7a28ba2..3b2578baf6 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -2143,12 +2143,17 @@ validate_op_harq_chain(struct rte_bbdev_op_data *op,
 		total_data_size += orig_op->segments[i].length;
 
 		TEST_ASSERT(orig_op->segments[i].length <
-				(uint32_t)(data_len + 64),
+				(uint32_t)(data_len + 256),
 				"Length of segment differ in original (%u) and filled (%u) op",
 				orig_op->segments[i].length, data_len);
 		harq_orig = (int8_t *) orig_op->segments[i].addr;
 		harq_out = rte_pktmbuf_mtod_offset(m, int8_t *, offset);
 
+		/* Cannot compare HARQ output data for such cases */
+		if ((ldpc_llr_decimals > 1) && ((ops_ld->op_flags & RTE_BBDEV_LDPC_LLR_COMPRESSION)
+				|| (ops_ld->op_flags & RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION)))
+			break;
+
 		if (!(ldpc_cap_flags &
 				RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_FILLERS
 				) || (ops_ld->op_flags &
@@ -2224,7 +2229,7 @@ validate_op_harq_chain(struct rte_bbdev_op_data *op,
 
 	/* Validate total mbuf pkt length */
 	uint32_t pkt_len = rte_pktmbuf_pkt_len(op->data) - op->offset;
-	TEST_ASSERT(total_data_size < pkt_len + 64,
+	TEST_ASSERT(total_data_size < pkt_len + 256,
 			"Length of data differ in original (%u) and filled (%u) op",
 			total_data_size, pkt_len);
 
-- 
2.37.1

Re: [PATCH v1 09/13] test/bbdev: bbdev-test cannot compare some scenarios

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> Updating logic for compression usecases.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index fdf7a28ba2..3b2578baf6 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -2143,12 +2143,17 @@ validate_op_harq_chain(struct rte_bbdev_op_data *op,
>   		total_data_size += orig_op->segments[i].length;
>   
>   		TEST_ASSERT(orig_op->segments[i].length <
> -				(uint32_t)(data_len + 64),
> +				(uint32_t)(data_len + 256),

Where is that value coming from?
It lacks explanations in my opinion, and the patch looks like a fix but
is not tagged as one.

>   				"Length of segment differ in original (%u) and filled (%u) op",
>   				orig_op->segments[i].length, data_len);
>   		harq_orig = (int8_t *) orig_op->segments[i].addr;
>   		harq_out = rte_pktmbuf_mtod_offset(m, int8_t *, offset);
>   
> +		/* Cannot compare HARQ output data for such cases */
> +		if ((ldpc_llr_decimals > 1) && ((ops_ld->op_flags & RTE_BBDEV_LDPC_LLR_COMPRESSION)
> +				|| (ops_ld->op_flags & RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION)))
> +			break;
> +
>   		if (!(ldpc_cap_flags &
>   				RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_FILLERS
>   				) || (ops_ld->op_flags &
> @@ -2224,7 +2229,7 @@ validate_op_harq_chain(struct rte_bbdev_op_data *op,
>   
>   	/* Validate total mbuf pkt length */
>   	uint32_t pkt_len = rte_pktmbuf_pkt_len(op->data) - op->offset;
> -	TEST_ASSERT(total_data_size < pkt_len + 64,
> +	TEST_ASSERT(total_data_size < pkt_len + 256,
>   			"Length of data differ in original (%u) and filled (%u) op",
>   			total_data_size, pkt_len);
>   

RE: [PATCH v1 09/13] test/bbdev: bbdev-test cannot compare some scenarios

[Chautru, Nicolas]

Hi Maxime, 


> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, January 31, 2023 4:16 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v1 09/13] test/bbdev: bbdev-test cannot compare some
> scenarios
> 
> 
> 
> On 1/17/23 17:50, Hernan Vargas wrote:
> > Updating logic for compression usecases.
> >
> > Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> > ---
> >   app/test-bbdev/test_bbdev_perf.c | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test-bbdev/test_bbdev_perf.c
> > b/app/test-bbdev/test_bbdev_perf.c
> > index fdf7a28ba2..3b2578baf6 100644
> > --- a/app/test-bbdev/test_bbdev_perf.c
> > +++ b/app/test-bbdev/test_bbdev_perf.c
> > @@ -2143,12 +2143,17 @@ validate_op_harq_chain(struct
> rte_bbdev_op_data *op,
> >   		total_data_size += orig_op->segments[i].length;
> >
> >   		TEST_ASSERT(orig_op->segments[i].length <
> > -				(uint32_t)(data_len + 64),
> > +				(uint32_t)(data_len + 256),
> 
> Where is that value coming from?
> It lacks explanations in my opinion, and the patch looks like a fix but is not
> tagged as one.

This is a tolerance as different implementations have different assumptions with regards to the HARQ size explicitly stored in memory (some data may be optimized out or having different alignment assumptions).
Hence the check includes some tolerance specific to that buffer. That tolerance is extended to cover different implementations. 
An #define will be used now instead of a magic number HARQ_MEM_TOLERANCE

I don’t believe it is critical to be backported. Still it would not harm to add the fix tag.  

Thanks

> 
> >   				"Length of segment differ in original (%u) and
> filled (%u) op",
> >   				orig_op->segments[i].length, data_len);
> >   		harq_orig = (int8_t *) orig_op->segments[i].addr;
> >   		harq_out = rte_pktmbuf_mtod_offset(m, int8_t *, offset);
> >
> > +		/* Cannot compare HARQ output data for such cases */
> > +		if ((ldpc_llr_decimals > 1) && ((ops_ld->op_flags &
> RTE_BBDEV_LDPC_LLR_COMPRESSION)
> > +				|| (ops_ld->op_flags &
> RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION)))
> > +			break;
> > +
> >   		if (!(ldpc_cap_flags &
> >
> 	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_FILLERS
> >   				) || (ops_ld->op_flags &
> > @@ -2224,7 +2229,7 @@ validate_op_harq_chain(struct
> rte_bbdev_op_data
> > *op,
> >
> >   	/* Validate total mbuf pkt length */
> >   	uint32_t pkt_len = rte_pktmbuf_pkt_len(op->data) - op->offset;
> > -	TEST_ASSERT(total_data_size < pkt_len + 64,
> > +	TEST_ASSERT(total_data_size < pkt_len + 256,
> >   			"Length of data differ in original (%u) and filled (%u)
> op",
> >   			total_data_size, pkt_len);
> >

[PATCH v1 10/13] test/bbdev: adjustment for soft output

[Hernan Vargas]

Extend bbdev-test for soft output check, notably due to the logic in
bbdev-test to enable termination changing.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 app/test-bbdev/test_bbdev_perf.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 3b2578baf6..1d18d9e054 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -1348,6 +1348,7 @@ fill_queue_buffers(struct test_op_params *op_params,
 				RTE_BBDEV_LDPC_LLR_COMPRESSION;
 		bool harq_comp = op_params->ref_dec_op->ldpc_dec.op_flags &
 				RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION;
+
 		ldpc_llr_decimals = capabilities->cap.ldpc_dec.llr_decimals;
 		ldpc_llr_size = capabilities->cap.ldpc_dec.llr_size;
 		ldpc_cap_flags = capabilities->cap.ldpc_dec.capability_flags;
@@ -2425,7 +2426,7 @@ validate_ldpc_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
 					i);
 
 		if (ref_op->ldpc_dec.op_flags & RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)
-			TEST_ASSERT_SUCCESS(validate_op_chain(soft_output,
+			TEST_ASSERT_SUCCESS(validate_op_so_chain(soft_output,
 					soft_data_orig),
 					"Soft output buffers (CB=%u) are not equal",
 					i);
@@ -2495,7 +2496,6 @@ validate_ldpc_enc_op(struct rte_bbdev_enc_op **ops, const uint16_t n,
 	return TEST_SUCCESS;
 }
 
-
 static inline int
 validate_op_fft_chain(struct rte_bbdev_op_data *op, struct op_data_entries *orig_op)
 {
@@ -3185,11 +3185,11 @@ throughput_intr_lcore_ldpc_dec(void *arg)
 	for (j = 0; j < TEST_REPETITIONS; ++j) {
 		for (i = 0; i < num_to_process; ++i) {
 			if (!loopback)
-				rte_pktmbuf_reset(
-					ops[i]->ldpc_dec.hard_output.data);
+				rte_pktmbuf_reset(ops[i]->ldpc_dec.hard_output.data);
 			if (hc_out || loopback)
-				mbuf_reset(
-				ops[i]->ldpc_dec.harq_combined_output.data);
+				mbuf_reset(ops[i]->ldpc_dec.harq_combined_output.data);
+			if (ops[i]->ldpc_dec.soft_output.data != NULL)
+				rte_pktmbuf_reset(ops[i]->ldpc_dec.soft_output.data);
 		}
 
 		tp->start_time = rte_rdtsc_precise();
@@ -3284,7 +3284,6 @@ throughput_intr_lcore_dec(void *arg)
 				rte_pktmbuf_reset(ops[i]->turbo_dec.soft_output.data);
 		}
 
-
 		tp->start_time = rte_rdtsc_precise();
 		for (enqueued = 0; enqueued < num_to_process;) {
 			num_to_enq = burst_sz;
@@ -3746,10 +3745,11 @@ bler_pmd_lcore_ldpc_dec(void *arg)
 	for (i = 0; i < 1; ++i) { /* Could add more iterations */
 		for (j = 0; j < num_ops; ++j) {
 			if (!loopback)
-				mbuf_reset(
-				ops_enq[j]->ldpc_dec.hard_output.data);
+				mbuf_reset(ops_enq[j]->ldpc_dec.hard_output.data);
 			if (hc_out || loopback)
 				mbuf_reset(ops_enq[j]->ldpc_dec.harq_combined_output.data);
+			if (ops_enq[j]->ldpc_dec.soft_output.data != NULL)
+				mbuf_reset(ops_enq[j]->ldpc_dec.soft_output.data);
 		}
 		if (extDdr)
 			preload_harq_ddr(tp->dev_id, queue_id, ops_enq,
@@ -3981,11 +3981,11 @@ throughput_pmd_lcore_ldpc_dec(void *arg)
 	for (i = 0; i < TEST_REPETITIONS; ++i) {
 		for (j = 0; j < num_ops; ++j) {
 			if (!loopback)
-				mbuf_reset(
-				ops_enq[j]->ldpc_dec.hard_output.data);
+				mbuf_reset(ops_enq[j]->ldpc_dec.hard_output.data);
 			if (hc_out || loopback)
-				mbuf_reset(
-				ops_enq[j]->ldpc_dec.harq_combined_output.data);
+				mbuf_reset(ops_enq[j]->ldpc_dec.harq_combined_output.data);
+			if (ops_enq[j]->ldpc_dec.soft_output.data != NULL)
+				mbuf_reset(ops_enq[j]->ldpc_dec.soft_output.data);
 		}
 		if (extDdr)
 			preload_harq_ddr(tp->dev_id, queue_id, ops_enq,
-- 
2.37.1

Re: [PATCH v1 10/13] test/bbdev: adjustment for soft output

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> Extend bbdev-test for soft output check, notably due to the logic in

test-bbdev

> bbdev-test to enable termination changing.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 26 +++++++++++++-------------
>   1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 3b2578baf6..1d18d9e054 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -1348,6 +1348,7 @@ fill_queue_buffers(struct test_op_params *op_params,
>   				RTE_BBDEV_LDPC_LLR_COMPRESSION;
>   		bool harq_comp = op_params->ref_dec_op->ldpc_dec.op_flags &
>   				RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION;
> +
>   		ldpc_llr_decimals = capabilities->cap.ldpc_dec.llr_decimals;
>   		ldpc_llr_size = capabilities->cap.ldpc_dec.llr_size;
>   		ldpc_cap_flags = capabilities->cap.ldpc_dec.capability_flags;
> @@ -2425,7 +2426,7 @@ validate_ldpc_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
>   					i);
>   
>   		if (ref_op->ldpc_dec.op_flags & RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)
> -			TEST_ASSERT_SUCCESS(validate_op_chain(soft_output,
> +			TEST_ASSERT_SUCCESS(validate_op_so_chain(soft_output,
>   					soft_data_orig),
>   					"Soft output buffers (CB=%u) are not equal",
>   					i);
> @@ -2495,7 +2496,6 @@ validate_ldpc_enc_op(struct rte_bbdev_enc_op **ops, const uint16_t n,
>   	return TEST_SUCCESS;
>   }
>   
> -
>   static inline int
>   validate_op_fft_chain(struct rte_bbdev_op_data *op, struct op_data_entries *orig_op)
>   {
> @@ -3185,11 +3185,11 @@ throughput_intr_lcore_ldpc_dec(void *arg)
>   	for (j = 0; j < TEST_REPETITIONS; ++j) {
>   		for (i = 0; i < num_to_process; ++i) {
>   			if (!loopback)
> -				rte_pktmbuf_reset(
> -					ops[i]->ldpc_dec.hard_output.data);
> +				rte_pktmbuf_reset(ops[i]->ldpc_dec.hard_output.data);

This is not introduced by this patch, but I wonder why not using
mbuf_reset() or rte_pktmbuf_reset() everywhere?

Looking at the changes below, it seems mbuf_reset() is used for the
!loopback case.

>   			if (hc_out || loopback)
> -				mbuf_reset(
> -				ops[i]->ldpc_dec.harq_combined_output.data);
> +				mbuf_reset(ops[i]->ldpc_dec.harq_combined_output.data);
> +			if (ops[i]->ldpc_dec.soft_output.data != NULL)
> +				rte_pktmbuf_reset(ops[i]->ldpc_dec.soft_output.data);
>   		}
>   
>   		tp->start_time = rte_rdtsc_precise();
> @@ -3284,7 +3284,6 @@ throughput_intr_lcore_dec(void *arg)
>   				rte_pktmbuf_reset(ops[i]->turbo_dec.soft_output.data);
>   		}
>   
> -
>   		tp->start_time = rte_rdtsc_precise();
>   		for (enqueued = 0; enqueued < num_to_process;) {
>   			num_to_enq = burst_sz;
> @@ -3746,10 +3745,11 @@ bler_pmd_lcore_ldpc_dec(void *arg)
>   	for (i = 0; i < 1; ++i) { /* Could add more iterations */
>   		for (j = 0; j < num_ops; ++j) {
>   			if (!loopback)
> -				mbuf_reset(
> -				ops_enq[j]->ldpc_dec.hard_output.data);
> +				mbuf_reset(ops_enq[j]->ldpc_dec.hard_output.data);
>   			if (hc_out || loopback)
>   				mbuf_reset(ops_enq[j]->ldpc_dec.harq_combined_output.data);
> +			if (ops_enq[j]->ldpc_dec.soft_output.data != NULL)
> +				mbuf_reset(ops_enq[j]->ldpc_dec.soft_output.data);
>   		}
>   		if (extDdr)
>   			preload_harq_ddr(tp->dev_id, queue_id, ops_enq,
> @@ -3981,11 +3981,11 @@ throughput_pmd_lcore_ldpc_dec(void *arg)
>   	for (i = 0; i < TEST_REPETITIONS; ++i) {
>   		for (j = 0; j < num_ops; ++j) {
>   			if (!loopback)
> -				mbuf_reset(
> -				ops_enq[j]->ldpc_dec.hard_output.data);
> +				mbuf_reset(ops_enq[j]->ldpc_dec.hard_output.data);
>   			if (hc_out || loopback)
> -				mbuf_reset(
> -				ops_enq[j]->ldpc_dec.harq_combined_output.data);
> +				mbuf_reset(ops_enq[j]->ldpc_dec.harq_combined_output.data);
> +			if (ops_enq[j]->ldpc_dec.soft_output.data != NULL)
> +				mbuf_reset(ops_enq[j]->ldpc_dec.soft_output.data);
>   		}
>   		if (extDdr)
>   			preload_harq_ddr(tp->dev_id, queue_id, ops_enq,

[PATCH v1 11/13] test/bbdev: expose warning counters

[Hernan Vargas]

Print warnings reported on queues for offload test.
No functional impact.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 app/test-bbdev/test_bbdev_perf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 1d18d9e054..0930786fda 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -5679,6 +5679,9 @@ offload_cost_test(struct active_device *ad,
 
 	struct rte_bbdev_stats stats = {0};
 	get_bbdev_queue_stats(ad->dev_id, queue_id, &stats);
+	if (stats.enqueue_warn_count > 0)
+		printf("Warning reported on the queue : %10"PRIu64"\n",
+			stats.enqueue_warn_count);
 	if (op_type != RTE_BBDEV_OP_LDPC_DEC) {
 		TEST_ASSERT_SUCCESS(stats.enqueued_count != num_to_process,
 				"Mismatch in enqueue count %10"PRIu64" %d",
-- 
2.37.1

Re: [PATCH v1 11/13] test/bbdev: expose warning counters

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> Print warnings reported on queues for offload test.
> No functional impact.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 1d18d9e054..0930786fda 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -5679,6 +5679,9 @@ offload_cost_test(struct active_device *ad,
>   
>   	struct rte_bbdev_stats stats = {0};
>   	get_bbdev_queue_stats(ad->dev_id, queue_id, &stats);
> +	if (stats.enqueue_warn_count > 0)
> +		printf("Warning reported on the queue : %10"PRIu64"\n",
> +			stats.enqueue_warn_count);
>   	if (op_type != RTE_BBDEV_OP_LDPC_DEC) {
>   		TEST_ASSERT_SUCCESS(stats.enqueued_count != num_to_process,
>   				"Mismatch in enqueue count %10"PRIu64" %d",

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH v1 12/13] test/bbdev: remove check for invalid opaque data

[Hernan Vargas]

Assert also if the opaque date is invalid.

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

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 0930786fda..81bf2c8b60 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -79,7 +79,6 @@
 
 #define SYNC_WAIT 0
 #define SYNC_START 1
-#define INVALID_OPAQUE -1
 
 #define INVALID_QUEUE_ID -1
 /* Increment for next code block in external HARQ memory */
@@ -1998,10 +1997,9 @@ check_enc_status_and_ordering(struct rte_bbdev_enc_op *op,
 			"op_status (%d) != expected_status (%d)",
 			op->status, expected_status);
 
-	if (op->opaque_data != (void *)(uintptr_t)INVALID_OPAQUE)
-		TEST_ASSERT((void *)(uintptr_t)order_idx == op->opaque_data,
-				"Ordering error, expected %p, got %p",
-				(void *)(uintptr_t)order_idx, op->opaque_data);
+	TEST_ASSERT((void *)(uintptr_t)order_idx == op->opaque_data,
+			"Ordering error, expected %p, got %p",
+			(void *)(uintptr_t)order_idx, op->opaque_data);
 
 	return TEST_SUCCESS;
 }
-- 
2.37.1

Re: [PATCH v1 12/13] test/bbdev: remove check for invalid opaque data

[Maxime Coquelin]

On 1/17/23 17:50, Hernan Vargas wrote:
> Assert also if the opaque date is invalid.

data*

> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test_bbdev_perf.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 0930786fda..81bf2c8b60 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -79,7 +79,6 @@
>   
>   #define SYNC_WAIT 0
>   #define SYNC_START 1
> -#define INVALID_OPAQUE -1
>   
>   #define INVALID_QUEUE_ID -1
>   /* Increment for next code block in external HARQ memory */
> @@ -1998,10 +1997,9 @@ check_enc_status_and_ordering(struct rte_bbdev_enc_op *op,
>   			"op_status (%d) != expected_status (%d)",
>   			op->status, expected_status);
>   
> -	if (op->opaque_data != (void *)(uintptr_t)INVALID_OPAQUE)
> -		TEST_ASSERT((void *)(uintptr_t)order_idx == op->opaque_data,
> -				"Ordering error, expected %p, got %p",
> -				(void *)(uintptr_t)order_idx, op->opaque_data);
> +	TEST_ASSERT((void *)(uintptr_t)order_idx == op->opaque_data,
> +			"Ordering error, expected %p, got %p",
> +			(void *)(uintptr_t)order_idx, op->opaque_data);
>   
>   	return TEST_SUCCESS;
>   }

It is a fix, isn't it?

Maxime

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

[Hernan Vargas]

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.vargas@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);
 	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");
-- 
2.37.1

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

[Maxime Coquelin]

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.vargas@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

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

[Vargas, Hernan]

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.

Thanks,
Hernan

-----Original Message-----
From: Maxime Coquelin <maxime.coquelin@redhat.com> 
Sent: Tuesday, January 31, 2023 6:36 AM
To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom <trix@redhat.com>
Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z <qi.z.zhang@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.vargas@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

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

[Maxime Coquelin]

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.

Thanks,
Maxime

> Thanks,
> Hernan
> 
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, January 31, 2023 6:36 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z <qi.z.zhang@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.vargas@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
> 

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

[Chautru, Nicolas]

Hi Maxime, 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, February 9, 2023 1:11 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> <qi.z.zhang@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 believe this is valid and better practice for the test functions, but again if you really want to push back, this could be changed.

Thanks!!
Nic

> 
> Thanks,
> Maxime
> 
> > Thanks,
> > Hernan
> >
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: Tuesday, January 31, 2023 6:36 AM
> > To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> > gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> > Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@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.vargas@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
> >

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.coquelin@redhat.com>
>> Sent: Thursday, February 9, 2023 1:11 AM
>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@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.coquelin@redhat.com>
>>> Sent: Tuesday, January 31, 2023 6:36 AM
>>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>>> <qi.z.zhang@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.vargas@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
>>>
> 

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

[Chautru, Nicolas]

Hi Maxime, 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, February 10, 2023 6:01 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan
> <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom
> <trix@redhat.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v1 13/13] test/bbdev: remove iteration count check
> 
> Hi Nicolas,
> 
> On 2/9/23 17:59, Chautru, Nicolas wrote:
> > Hi Maxime,
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Thursday, February 9, 2023 1:11 AM
> >> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> >> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> >> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@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.

OK, no big deal. Thanks. 

> 
> Thanks,
> Maxime
> 
> > Thanks!!
> > Nic
> >
> >>
> >> Thanks,
> >> Maxime
> >>
> >>> Thanks,
> >>> Hernan
> >>>
> >>> -----Original Message-----
> >>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>> Sent: Tuesday, January 31, 2023 6:36 AM
> >>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> >>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> >>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> >>> <qi.z.zhang@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.vargas@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
> >>>
> >