Re: [dpdk-dev] [PATCH v2 3/6] app/test: add basic dmadev copy tests

[Kevin Laatz <kevin.laatz@intel.com>]

On 01/09/2021 17:32, Bruce Richardson wrote:
> For each dmadev instance, perform some basic copy tests to validate that
> functionality.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   app/test/test_dmadev.c | 174 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 174 insertions(+)

<snip>

> +
> +static int
> +test_enqueue_copies(int dev_id, uint16_t vchan)
> +{
> +	unsigned int i;
> +	uint16_t id;
> +
> +	/* test doing a single copy */
> +	do {
> +		struct rte_mbuf *src, *dst;
> +		char *src_data, *dst_data;
> +
> +		src = rte_pktmbuf_alloc(pool);
> +		dst = rte_pktmbuf_alloc(pool);
> +		src_data = rte_pktmbuf_mtod(src, char *);
> +		dst_data = rte_pktmbuf_mtod(dst, char *);
> +
> +		for (i = 0; i < COPY_LEN; i++)
> +			src_data[i] = rte_rand() & 0xFF;
> +
> +		id = rte_dmadev_copy(dev_id, vchan, src->buf_iova + src->data_off,
> +				dst->buf_iova + dst->data_off, COPY_LEN, RTE_DMA_OP_FLAG_SUBMIT);

Could use the rte_mbuf APIs to get the struct members here and 
throughout the other tests in this set.

No strong opinion on this either way.


> +		if (id != id_count) {
> +			PRINT_ERR("Error with rte_dmadev_copy, got %u, expected %u\n",
> +					id, id_count);
> +			return -1;
> +		}
> +
> +		/* give time for copy to finish, then check it was done */
> +		await_hw(dev_id, vchan);
> +
> +		for (i = 0; i < COPY_LEN; i++) {
> +			if (dst_data[i] != src_data[i]) {
> +				PRINT_ERR("Data mismatch at char %u [Got %02x not %02x]\n", i,
> +						dst_data[i], src_data[i]);
> +				rte_dmadev_dump(dev_id, stderr);
> +				return -1;
> +			}
> +		}
> +
> +		/* now check completion works */
> +		if (rte_dmadev_completed(dev_id, vchan, 1, &id, NULL) != 1) {
> +			PRINT_ERR("Error with rte_dmadev_completed\n");
> +			return -1;
> +		}
> +		if (id != id_count) {
> +			PRINT_ERR("Error:incorrect job id received, %u [expected %u]\n",
> +					id, id_count);
> +			return -1;
> +		}
> +
> +		rte_pktmbuf_free(src);
> +		rte_pktmbuf_free(dst);
> +
> +		/* now check completion works */

This comment doesn't match with the check being done.


> +		if (rte_dmadev_completed(dev_id, 0, 1, NULL, NULL) != 0) {
> +			PRINT_ERR("Error with rte_dmadev_completed in empty check\n");
> +			return -1;
> +		}
> +		id_count++;
> +
> +	} while (0);
> +

<snip>

Apart from minor comments above, LGTM.

Reviewed-by: Kevin Laatz <kevin.laatz@intel.com>