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

["Chautru, Nicolas" <nicolas.chautru@intel.com>]

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);
> >