Re: [dpdk-dev] [PATCH v3 06/14] test-bbdev: support HARQ validation

[Akhil Goyal <akhil.goyal@nxp.com>]

rte_bbdev_dec_op *op,
>  		unsigned int order_idx, const int expected_status)
>  {
> -	TEST_ASSERT(op->status == expected_status,
> +	int status = op->status;
> +	/* ignore parity mismatch false alarms for long iterations */
> +	{
> +		if (!(expected_status & (1 << RTE_BBDEV_SYNDROME_ERROR))
> &&
> +				(status & (1 <<
> RTE_BBDEV_SYNDROME_ERROR))) {
> +			printf("WARNING: Ignore Syndrome Check
> mismatch\n");
> +			status -= (1 << RTE_BBDEV_SYNDROME_ERROR);
> +		}
> +		if ((expected_status & (1 << RTE_BBDEV_SYNDROME_ERROR))
> &&
> +				!(status & (1 <<
> RTE_BBDEV_SYNDROME_ERROR))) {
> +			printf("WARNING: Ignore Syndrome Check
> mismatch\n");
> +			status += (1 << RTE_BBDEV_SYNDROME_ERROR);
> +		}
> +	}

What is the need of these extra braces.

> +
> +	TEST_ASSERT(status == expected_status,
>  			"op_status (%d) != expected_status (%d)",
>  			op->status, expected_status);
> 
> @@ -1236,9 +1333,10 @@ typedef int (test_case_function)(struct active_device
> *ad,
>  			"op_status (%d) != expected_status (%d)",
>  			op->status, expected_status);
> 
> -	TEST_ASSERT((void *)(uintptr_t)order_idx == op->opaque_data,
> -			"Ordering error, expected %p, got %p",
> -			(void *)(uintptr_t)order_idx, op->opaque_data);
> +	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);
> 
>  	return TEST_SUCCESS;
>  }
> @@ -1282,6 +1380,170 @@ typedef int (test_case_function)(struct
> active_device *ad,
>  	return TEST_SUCCESS;
>  }
> 
> +
> +/* Compute K0 for a given configuration for HARQ output length computation
> */
> +static inline uint16_t
> +get_k0(uint16_t n_cb, uint16_t z_c, uint8_t basegraph, uint8_t rv_index)
> +{
> +	if (rv_index == 0)
> +		return 0;
> +	uint16_t n = (basegraph == 1 ? 66 : 50) * z_c;
> +	if (n_cb == n) {
> +		if (rv_index == 1)
> +			return (basegraph == 1 ? 17 : 13) * z_c;
> +		else if (rv_index == 2)
> +			return (basegraph == 1 ? 33 : 25) * z_c;
> +		else
> +			return (basegraph == 1 ? 56 : 43) * z_c;
> +	}
> +	/* LBRM case - includes a division by N */
> +	if (rv_index == 1)
> +		return (((basegraph == 1 ? 17 : 13) * n_cb)
> +				/ n) * z_c;
> +	else if (rv_index == 2)
> +		return (((basegraph == 1 ? 33 : 25) * n_cb)
> +				/ n) * z_c;
> +	else
> +		return (((basegraph == 1 ? 56 : 43) * n_cb)
> +				/ n) * z_c;
> +}

Please add comments for the logic behind these calculations. 
It would be better to remove these hard codings and define some macros.
There are some other hard codings in the patch. Please fix those as well.
> +
> +/* HARQ output length including the Filler bits */
> +static inline uint16_t
> +compute_Harq_Len(struct rte_bbdev_op_ldpc_dec *ops_ld) {

Camel Casing?



One generic comment on the patches. I can see that the name of MACROS
Is quite big. It would be better to rename them with a shorter logical name.