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

[Amit Prakash Shukla <amitprakashs@marvell.com>]

Hi Chengwen,

Thanks for the review and feedback. I will send v2 with suggested changes.

Thanks,
Amit Shukla

> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Thursday, November 2, 2023 7:37 AM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Kevin Laatz
> <kevin.laatz@intel.com>; Bruce Richardson <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> conor.walsh@intel.com; Vamsi Krishna Attunuru <vattunuru@marvell.com>;
> g.singh@nxp.com; sachin.saxena@oss.nxp.com; hemant.agrawal@nxp.com;
> cheng1.jiang@intel.com; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> mb@smartsharesystems.com
> Subject: [EXT] Re: [PATCH] test/dma: fix for buffer auto free
> 
> External Email
> 
> ----------------------------------------------------------------------
> 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
> 
> 

<snip>