test/dma: fix for buffer auto free
Checks
Commit Message
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(-)
Comments
> 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;
[Hemant] should't it be per device id?
> +
> 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;
> --
> 2.25.1
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;
>
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>
@@ -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;