Re: [PATCH] test/dma: fix for buffer auto free

[fengchengwen <fengchengwen@huawei.com>]

Hi Amit,

  I prefer not use static variable to control it because it introduce many coupling.

  Suggest add one function which prepare the test_m2d_auto_free, like prepare_m2d_auto_free

	if ((info.dev_capa & RTE_DMA_CAPA_M2D_AUTO_FREE) &&
	    dma_add_test[TEST_M2D_AUTO_FREE].enabled == true) {
                if (prepare_m2d_auto_free(dev_id) != 0)
                    goto err;
		if (runtest("m2d_auto_free", test_m2d_auto_free, 128, dev_id, vchan,
			    CHECK_ERRS) < 0)
			goto err;
	}	

In the new function, could do reinit vchan just like the beginging test_m2d_auto_free.
static int prepare_m2d_auto_free(int dev_id) {
	const struct rte_dma_vchan_conf qconf = {
		.direction = RTE_DMA_DIR_MEM_TO_DEV,
		.nb_desc = TEST_RINGSIZE,
		.auto_free.m2d.pool = pool,
		.dst_port.port_type = RTE_DMA_PORT_PCIE,
		.dst_port.pcie.coreid = 0,
	};
	/* Stop the device to reconfigure vchan because should use Mem-to-Dev mode. */
	if (rte_dma_stop(dev_id) < 0)
		ERR_RETURN("Error stopping device %u\n", dev_id);
	if (rte_dma_vchan_setup(dev_id, vchan, &qconf) < 0)
		ERR_RETURN("Error with queue configuration\n");
	if (rte_dma_start(dev_id) != 0)
		ERR_RETURN("Error with rte_dma_start()\n");
	return 0;
}



Also I found a bug in test_m2d_auto_free function, if above path taken:
	if (rte_pktmbuf_alloc_bulk(pool, src, NR_MBUF) != 0) {
		printf("alloc src mbufs failed.\n");
		ret = -1;
		goto done;
	}

done:
	rte_pktmbuf_free_bulk(dst, NR_MBUF);
	/* If the test passes source buffer will be freed in hardware. */
	if (ret < 0)
		rte_pktmbuf_free_bulk(&src[nb_done], (NR_MBUF - nb_done));
		----- then it will free invalid mbuf to pool because src was not success init


On 2023/11/1 18:18, Amit Prakash Shukla wrote:
> Buffer auto free test failed for more than 1 dma device as the device
> initialization for the test was been done only for the first dma device.
> This changeset fixes the same.
> 
> Fixes: 877cb3e37426 ("dmadev: add buffer auto free offload")
> 
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> ---
>  app/test/test_dmadev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
> index 216f84b6bb..3d4cb37ee6 100644
> --- a/app/test/test_dmadev.c
> +++ b/app/test/test_dmadev.c
> @@ -49,6 +49,8 @@ struct dma_add_test dma_add_test[] = {
>  	[TEST_M2D_AUTO_FREE] = {.name = "m2d_auto_free", .enabled = false},
>  };
>  
> +static bool dev_init;
> +
>  static void
>  __rte_format_printf(3, 4)
>  print_err(const char *func, int lineno, const char *format, ...)
> @@ -837,7 +839,6 @@ test_m2d_auto_free(int16_t dev_id, uint16_t vchan)
>  	};
>  	uint32_t buf_cnt1, buf_cnt2;
>  	struct rte_mempool_ops *ops;
> -	static bool dev_init;
>  	uint16_t nb_done = 0;
>  	bool dma_err = false;
>  	int retry = 100;
> @@ -1011,6 +1012,7 @@ test_dmadev_instance(int16_t dev_id)
>  
>  	if ((info.dev_capa & RTE_DMA_CAPA_M2D_AUTO_FREE) &&
>  	    dma_add_test[TEST_M2D_AUTO_FREE].enabled == true) {
> +		dev_init = false;
>  		if (runtest("m2d_auto_free", test_m2d_auto_free, 128, dev_id, vchan,
>  			    CHECK_ERRS) < 0)
>  			goto err;
>