[PATCH v1 0/2] offload support to free dma source buffer

[PATCH v1 0/2] offload support to free dma source buffer

[Amit Prakash Shukla]

This series adds offload support to free source buffer in dma library
and adds a test support in dmadev_autotest to validate the
functionality.

v1:
- Implementation from RFC.
- Add test support to validate functionality.

Amit Prakash Shukla (2):
  dmadev: offload to free source buffer
  test/dma: add source buffer offload free test

 app/test/test_dmadev.c  | 132 +++++++++++++++++++++++++++++++++++++++-
 lib/dmadev/rte_dmadev.h |  27 ++++++++
 2 files changed, 158 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v1 1/2] dmadev: offload to free source buffer

[Amit Prakash Shukla]

This changeset adds support in DMA library to free source DMA buffer by
hardware. On a supported hardware, application can pass on the mempool
information as part of vchan config when the DMA transfer direction is
configured as RTE_DMA_DIR_MEM_TO_DEV.

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h
index b157ab7600..d6a685907f 100644
--- a/lib/dmadev/rte_dmadev.h
+++ b/lib/dmadev/rte_dmadev.h
@@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
 #define RTE_DMA_CAPA_OPS_COPY_SG	RTE_BIT64(33)
 /** Support fill operation. */
 #define RTE_DMA_CAPA_OPS_FILL		RTE_BIT64(34)
+/** Support for source buffer free for mem to dev transfer.
+ *
+ * @note Even though the DMA driver has this capability, it may not support all
+ * mempool drivers. If the mempool is not supported by the DMA driver,
+ * rte_dma_vchan_setup() will fail.
+ **/
+#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE	RTE_BIT64(35)
 /**@}*/
 
 /**
@@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
 	 * @see struct rte_dma_port_param
 	 */
 	struct rte_dma_port_param dst_port;
+	/** mempool from which source buffer is allocated. mempool info is used
+	 * for freeing source buffer by hardware when configured direction is
+	 * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by hardware,
+	 * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling rte_dma_copy and
+	 * rte_dma_copy_sg().
+	 *
+	 * @note If the mempool is not supported by the DMA driver,
+	 * rte_dma_vchan_setup() will fail.
+	 *
+	 * @see RTE_DMA_OP_FLAG_FREE_SBUF
+	 */
+	struct rte_mempool *mem_to_dev_src_buf_pool;
+
 };
 
 /**
@@ -818,6 +838,13 @@ struct rte_dma_sge {
  * capability bit for this, driver should not return error if this flag was set.
  */
 #define RTE_DMA_OP_FLAG_LLC     RTE_BIT64(2)
+/** Mem to dev source buffer free flag.
+ * Used for freeing source DMA buffer by hardware when the transfer direction is
+ * configured as RTE_DMA_DIR_MEM_TO_DEV.
+ *
+ * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
+ */
+#define RTE_DMA_OP_FLAG_FREE_SBUF	RTE_BIT64(3)
 /**@}*/
 
 /**
-- 
2.25.1

RE: [PATCH v1 1/2] dmadev: offload to free source buffer

[Amit Prakash Shukla]

Driver implementation of the spec: http://patches.dpdk.org/project/dpdk/patch/20230907082443.1002665-1-amitprakashs@marvell.com/


> -----Original Message-----
> From: Amit Prakash Shukla <amitprakashs@marvell.com>
> Sent: Thursday, September 7, 2023 1:41 PM
> To: Chengwen Feng <fengchengwen@huawei.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>; Amit
> Prakash Shukla <amitprakashs@marvell.com>; Morten Brørup
> <mb@smartsharesystems.com>
> Subject: [PATCH v1 1/2] dmadev: offload to free source buffer
> 
> This changeset adds support in DMA library to free source DMA buffer by
> hardware. On a supported hardware, application can pass on the mempool
> information as part of vchan config when the DMA transfer direction is
> configured as RTE_DMA_DIR_MEM_TO_DEV.
> 
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index
> b157ab7600..d6a685907f 100644
> --- a/lib/dmadev/rte_dmadev.h
> +++ b/lib/dmadev/rte_dmadev.h
> @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
>  #define RTE_DMA_CAPA_OPS_COPY_SG	RTE_BIT64(33)
>  /** Support fill operation. */
>  #define RTE_DMA_CAPA_OPS_FILL		RTE_BIT64(34)
> +/** Support for source buffer free for mem to dev transfer.
> + *
> + * @note Even though the DMA driver has this capability, it may not
> +support all
> + * mempool drivers. If the mempool is not supported by the DMA driver,
> + * rte_dma_vchan_setup() will fail.
> + **/
> +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE
> 	RTE_BIT64(35)
>  /**@}*/
> 
>  /**
> @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
>  	 * @see struct rte_dma_port_param
>  	 */
>  	struct rte_dma_port_param dst_port;
> +	/** mempool from which source buffer is allocated. mempool info is
> used
> +	 * for freeing source buffer by hardware when configured direction is
> +	 * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by
> hardware,
> +	 * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling
> rte_dma_copy and
> +	 * rte_dma_copy_sg().
> +	 *
> +	 * @note If the mempool is not supported by the DMA driver,
> +	 * rte_dma_vchan_setup() will fail.
> +	 *
> +	 * @see RTE_DMA_OP_FLAG_FREE_SBUF
> +	 */
> +	struct rte_mempool *mem_to_dev_src_buf_pool;
> +
>  };
> 
>  /**
> @@ -818,6 +838,13 @@ struct rte_dma_sge {
>   * capability bit for this, driver should not return error if this flag was set.
>   */
>  #define RTE_DMA_OP_FLAG_LLC     RTE_BIT64(2)
> +/** Mem to dev source buffer free flag.
> + * Used for freeing source DMA buffer by hardware when the transfer
> +direction is
> + * configured as RTE_DMA_DIR_MEM_TO_DEV.
> + *
> + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
> + */
> +#define RTE_DMA_OP_FLAG_FREE_SBUF	RTE_BIT64(3)
>  /**@}*/
> 
>  /**
> --
> 2.25.1

RE: [PATCH v1 1/2] dmadev: offload to free source buffer

[Anoob Joseph]

Hi Amit,

Thanks for adding the feature. Please see inline.

With the mentioned fixes,

Acked-by: Anoob Joseph <anoobj@marvell.com>

Thanks,
Anoob

> -----Original Message-----
> From: Amit Prakash Shukla <amitprakashs@marvell.com>
> Sent: Thursday, September 7, 2023 1:41 PM
> To: Chengwen Feng <fengchengwen@huawei.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>; Amit
> Prakash Shukla <amitprakashs@marvell.com>; Morten Brørup
> <mb@smartsharesystems.com>
> Subject: [PATCH v1 1/2] dmadev: offload to free source buffer
> 
> This changeset adds support in DMA library to free source DMA buffer by
> hardware. On a supported hardware, application can pass on the mempool
> information as part of vchan config when the DMA transfer direction is
> configured as RTE_DMA_DIR_MEM_TO_DEV.
> 
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index
> b157ab7600..d6a685907f 100644
> --- a/lib/dmadev/rte_dmadev.h
> +++ b/lib/dmadev/rte_dmadev.h
> @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
>  #define RTE_DMA_CAPA_OPS_COPY_SG	RTE_BIT64(33)
>  /** Support fill operation. */
>  #define RTE_DMA_CAPA_OPS_FILL		RTE_BIT64(34)
> +/** Support for source buffer free for mem to dev transfer.
> + *
> + * @note Even though the DMA driver has this capability, it may not
> +support all
> + * mempool drivers. If the mempool is not supported by the DMA driver,
> + * rte_dma_vchan_setup() will fail.
> + **/

[Anoob] I think you can remove the note. It is mentioned below also, right? That should be sufficient I guess. Not a strong comment. You can decide.

> +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE
> 	RTE_BIT64(35)
>  /**@}*/
> 
>  /**
> @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
>  	 * @see struct rte_dma_port_param
>  	 */
>  	struct rte_dma_port_param dst_port;
> +	/** mempool from which source buffer is allocated. mempool info is

[Anoob] Mempool -> Mempool (in both places)

> used
> +	 * for freeing source buffer by hardware when configured direction is
> +	 * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by
> hardware,
> +	 * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling
> rte_dma_copy and
> +	 * rte_dma_copy_sg().
> +	 *
> +	 * @note If the mempool is not supported by the DMA driver,
> +	 * rte_dma_vchan_setup() will fail.

[Anoob] "If the mempool is not supported by the DMA device, ..."

driver -> device.

> +	 *
> +	 * @see RTE_DMA_OP_FLAG_FREE_SBUF
> +	 */
> +	struct rte_mempool *mem_to_dev_src_buf_pool;
> +
>  };
> 
>  /**
> @@ -818,6 +838,13 @@ struct rte_dma_sge {
>   * capability bit for this, driver should not return error if this flag was set.
>   */
>  #define RTE_DMA_OP_FLAG_LLC     RTE_BIT64(2)
> +/** Mem to dev source buffer free flag.
> + * Used for freeing source DMA buffer by hardware when the transfer
> +direction is
> + * configured as RTE_DMA_DIR_MEM_TO_DEV.
> + *
> + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
> + */
> +#define RTE_DMA_OP_FLAG_FREE_SBUF	RTE_BIT64(3)
>  /**@}*/
> 
>  /**
> --
> 2.25.1

[PATCH v1 2/2] test/dma: add source buffer offload free test

[Amit Prakash Shukla]

Add a test case to validate the functionality of drivers' dma
source buffer offload free. As part of dmadev_autotest, test case
will be executed only if the driver supports source buffer offload
free and if the test is exported by env variable DPDK_ADD_DMA_TEST.

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
---
 app/test/test_dmadev.c | 132 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 131 insertions(+), 1 deletion(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index 6ef875e545..48da4664ae 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -18,11 +18,26 @@
 
 #define ERR_RETURN(...) do { print_err(__func__, __LINE__, __VA_ARGS__); return -1; } while (0)
 
+#define TEST_RINGSIZE 512
 #define COPY_LEN 1024
 
 static struct rte_mempool *pool;
 static uint16_t id_count;
 
+enum {
+	TEST_SRC_BUF_FREE = 0,
+	TEST_MAX,
+};
+
+struct dma_add_test {
+	const char *name;
+	bool enabled;
+};
+
+struct dma_add_test dma_add_test[] = {
+	[TEST_SRC_BUF_FREE] = {.name = "sbuf_free", .enabled = false},
+};
+
 static void
 __rte_format_printf(3, 4)
 print_err(const char *func, int lineno, const char *format, ...)
@@ -797,10 +812,93 @@ test_burst_capacity(int16_t dev_id, uint16_t vchan)
 	return 0;
 }
 
+static int
+test_sbuf_free(int16_t dev_id, uint16_t vchan)
+{
+#define NR_MBUF 256
+	int i, ret = 0;
+	int retry = 100;
+	uint16_t nb_done = 0;
+	bool dma_err = false;
+	uint32_t buf_cnt1, buf_cnt2;
+	struct rte_mempool_ops *ops;
+	uint64_t remote_addr = 0x40000000ull;
+	struct rte_mbuf *src[NR_MBUF], *dst[NR_MBUF];
+	const struct rte_dma_vchan_conf qconf = {
+		.direction = RTE_DMA_DIR_MEM_TO_DEV,
+		.nb_desc = TEST_RINGSIZE,
+		.mem_to_dev_src_buf_pool = pool,
+		.dst_port.port_type = RTE_DMA_PORT_PCIE,
+		/* Assuming pemid as 0. */
+		.dst_port.pcie.coreid = 0,
+	};
+	static int dev_init;
+
+	if (!dev_init) {
+		/* Stop the device to reconfigure vchan. */
+		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");
+
+		dev_init++;
+	}
+
+	if (rte_pktmbuf_alloc_bulk(pool, dst, NR_MBUF) != 0)
+		ERR_RETURN("alloc dst mbufs failed.\n");
+
+	for (i = 0; i < NR_MBUF; i++) {
+		/* Using mbuf structure to hold remote iova address. */
+		rte_mbuf_iova_set(dst[i], (rte_iova_t)remote_addr);
+		dst[i]->data_off = 0;
+	}
+
+	/* Capture buffer count before allocating source buffer. */
+	ops = rte_mempool_get_ops(pool->ops_index);
+	buf_cnt1 = ops->get_count(pool);
+
+	if (rte_pktmbuf_alloc_bulk(pool, src, NR_MBUF) != 0)
+		ERR_RETURN("alloc src mbufs failed.\n");
+
+	if ((buf_cnt1 - NR_MBUF) != ops->get_count(pool))
+		ERR_RETURN("Buffer count check failed.\n");
+
+	for (i = 0; i < NR_MBUF; i++) {
+		ret = rte_dma_copy(dev_id, vchan, rte_mbuf_data_iova(src[i]),
+				rte_mbuf_data_iova(dst[i]), COPY_LEN,
+				RTE_DMA_OP_FLAG_FREE_SBUF);
+
+		if (ret < 0)
+			ERR_RETURN("rte_dma_copy returned error.\n");
+	}
+
+	rte_dma_submit(dev_id, vchan);
+	nb_done = 0;
+	do {
+		nb_done += rte_dma_completed(dev_id, vchan, (NR_MBUF - nb_done), NULL, &dma_err);
+		if (dma_err)
+			break;
+		/* Sleep for 1 millisecond */
+		rte_delay_us_sleep(1000);
+	} while (retry-- && (nb_done < NR_MBUF));
+
+	buf_cnt2 = ops->get_count(pool);
+	if ((buf_cnt1 != buf_cnt2) || dma_err)
+		ERR_RETURN("Free source buffer test failed.\n");
+
+	/* If the test passes source buffer will be freed in hardware. */
+	rte_pktmbuf_free_bulk(dst, NR_MBUF);
+
+	return 0;
+}
+
 static int
 test_dmadev_instance(int16_t dev_id)
 {
-#define TEST_RINGSIZE 512
 #define CHECK_ERRS    true
 	struct rte_dma_stats stats;
 	struct rte_dma_info info;
@@ -890,6 +988,12 @@ test_dmadev_instance(int16_t dev_id)
 	else if (runtest("fill", test_enqueue_fill, 1, dev_id, vchan, CHECK_ERRS) < 0)
 		goto err;
 
+	if ((info.dev_capa & RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE) &&
+	    dma_add_test[TEST_SRC_BUF_FREE].enabled == true) {
+		if (runtest("sbuf_free", test_sbuf_free, 128, dev_id, vchan, CHECK_ERRS) < 0)
+			goto err;
+	}
+
 	rte_mempool_free(pool);
 
 	if (rte_dma_stop(dev_id) < 0)
@@ -922,11 +1026,37 @@ test_apis(void)
 	return ret;
 }
 
+static void
+parse_dma_env_var(void)
+{
+	char *dma_env_str = getenv("DPDK_ADD_DMA_TEST");
+	char *tests[32] = {0};
+	int n_tests = 0;
+	int i, j;
+
+	if (dma_env_str && strlen(dma_env_str) > 0) {
+		char *additional_test = strdup(dma_env_str);
+		if (additional_test) {
+			n_tests = rte_strsplit(additional_test, strlen(additional_test), tests,
+				       RTE_DIM(tests), ',');
+			for (i = 0; i < n_tests; i++) {
+				for (j = 0; j < TEST_MAX; j++) {
+					if (!strcmp(tests[i], dma_add_test[j].name))
+						dma_add_test[j].enabled = true;
+				}
+			}
+		}
+		free(additional_test);
+	}
+}
+
 static int
 test_dma(void)
 {
 	int i;
 
+	parse_dma_env_var();
+
 	/* basic sanity on dmadev infrastructure */
 	if (test_apis() < 0)
 		ERR_RETURN("Error performing API tests\n");
-- 
2.25.1

RE: [PATCH v1 2/2] test/dma: add source buffer offload free test

[Anoob Joseph]

Hi Amit,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Amit Prakash Shukla <amitprakashs@marvell.com>
> Sent: Thursday, September 7, 2023 1:41 PM
> To: Chengwen Feng <fengchengwen@huawei.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>; Amit
> Prakash Shukla <amitprakashs@marvell.com>
> Subject: [PATCH v1 2/2] test/dma: add source buffer offload free test
> 
> Add a test case to validate the functionality of drivers' dma source buffer
> offload free. As part of dmadev_autotest, test case will be executed only if
> the driver supports source buffer offload free and if the test is exported by
> env variable DPDK_ADD_DMA_TEST.
> 
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> ---
>  app/test/test_dmadev.c | 132
> ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 131 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c index
> 6ef875e545..48da4664ae 100644
> --- a/app/test/test_dmadev.c
> +++ b/app/test/test_dmadev.c
> @@ -18,11 +18,26 @@
> 
>  #define ERR_RETURN(...) do { print_err(__func__, __LINE__,
> __VA_ARGS__); return -1; } while (0)
> 
> +#define TEST_RINGSIZE 512
>  #define COPY_LEN 1024
> 
>  static struct rte_mempool *pool;
>  static uint16_t id_count;
> 
> +enum {
> +	TEST_SRC_BUF_FREE = 0,
> +	TEST_MAX,
> +};
> +
> +struct dma_add_test {
> +	const char *name;
> +	bool enabled;
> +};
> +
> +struct dma_add_test dma_add_test[] = {
> +	[TEST_SRC_BUF_FREE] = {.name = "sbuf_free", .enabled = false}, };
> +
>  static void
>  __rte_format_printf(3, 4)
>  print_err(const char *func, int lineno, const char *format, ...) @@ -797,10
> +812,93 @@ test_burst_capacity(int16_t dev_id, uint16_t vchan)
>  	return 0;
>  }
> 
> +static int
> +test_sbuf_free(int16_t dev_id, uint16_t vchan) { #define NR_MBUF 256
> +	int i, ret = 0;
> +	int retry = 100;
> +	uint16_t nb_done = 0;
> +	bool dma_err = false;
> +	uint32_t buf_cnt1, buf_cnt2;
> +	struct rte_mempool_ops *ops;
> +	uint64_t remote_addr = 0x40000000ull;

[Anoob] Can you make remote_addr as a variable read from env variable? Hard coding may not be the right approach.

> +	struct rte_mbuf *src[NR_MBUF], *dst[NR_MBUF];
> +	const struct rte_dma_vchan_conf qconf = {
> +		.direction = RTE_DMA_DIR_MEM_TO_DEV,
> +		.nb_desc = TEST_RINGSIZE,
> +		.mem_to_dev_src_buf_pool = pool,
> +		.dst_port.port_type = RTE_DMA_PORT_PCIE,
> +		/* Assuming pemid as 0. */
> +		.dst_port.pcie.coreid = 0,
> +	};
> +	static int dev_init;

[Anoob] Can you use bool instead of int for dev_init?

> +
> +	if (!dev_init) {
> +		/* Stop the device to reconfigure vchan. */
> +		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");
> +
> +		dev_init++;
> +	}
> +
> +	if (rte_pktmbuf_alloc_bulk(pool, dst, NR_MBUF) != 0)
> +		ERR_RETURN("alloc dst mbufs failed.\n");
> +
> +	for (i = 0; i < NR_MBUF; i++) {
> +		/* Using mbuf structure to hold remote iova address. */
> +		rte_mbuf_iova_set(dst[i], (rte_iova_t)remote_addr);
> +		dst[i]->data_off = 0;
> +	}
> +
> +	/* Capture buffer count before allocating source buffer. */
> +	ops = rte_mempool_get_ops(pool->ops_index);
> +	buf_cnt1 = ops->get_count(pool);
> +
> +	if (rte_pktmbuf_alloc_bulk(pool, src, NR_MBUF) != 0)
> +		ERR_RETURN("alloc src mbufs failed.\n");

[Anoob] The memory is not freed in case of errors. May be you can free them in the end and use goto as required.

> +
> +	if ((buf_cnt1 - NR_MBUF) != ops->get_count(pool))
> +		ERR_RETURN("Buffer count check failed.\n");
> +
> +	for (i = 0; i < NR_MBUF; i++) {
> +		ret = rte_dma_copy(dev_id, vchan,
> rte_mbuf_data_iova(src[i]),
> +				rte_mbuf_data_iova(dst[i]), COPY_LEN,
> +				RTE_DMA_OP_FLAG_FREE_SBUF);
> +
> +		if (ret < 0)
> +			ERR_RETURN("rte_dma_copy returned error.\n");
> +	}
> +
> +	rte_dma_submit(dev_id, vchan);
> +	nb_done = 0;
> +	do {
> +		nb_done += rte_dma_completed(dev_id, vchan, (NR_MBUF
> - nb_done), NULL, &dma_err);
> +		if (dma_err)
> +			break;
> +		/* Sleep for 1 millisecond */
> +		rte_delay_us_sleep(1000);
> +	} while (retry-- && (nb_done < NR_MBUF));
> +
> +	buf_cnt2 = ops->get_count(pool);
> +	if ((buf_cnt1 != buf_cnt2) || dma_err)
> +		ERR_RETURN("Free source buffer test failed.\n");
> +
> +	/* If the test passes source buffer will be freed in hardware. */
> +	rte_pktmbuf_free_bulk(dst, NR_MBUF);
> +
> +	return 0;
> +}
> +
>  static int
>  test_dmadev_instance(int16_t dev_id)
>  {
> -#define TEST_RINGSIZE 512
>  #define CHECK_ERRS    true
>  	struct rte_dma_stats stats;
>  	struct rte_dma_info info;
> @@ -890,6 +988,12 @@ test_dmadev_instance(int16_t dev_id)
>  	else if (runtest("fill", test_enqueue_fill, 1, dev_id, vchan,
> CHECK_ERRS) < 0)
>  		goto err;
> 
> +	if ((info.dev_capa &
> RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE) &&
> +	    dma_add_test[TEST_SRC_BUF_FREE].enabled == true) {
> +		if (runtest("sbuf_free", test_sbuf_free, 128, dev_id, vchan,
> CHECK_ERRS) < 0)
> +			goto err;
> +	}
> +
>  	rte_mempool_free(pool);
> 
>  	if (rte_dma_stop(dev_id) < 0)
> @@ -922,11 +1026,37 @@ test_apis(void)
>  	return ret;
>  }
> 
> +static void
> +parse_dma_env_var(void)
> +{
> +	char *dma_env_str = getenv("DPDK_ADD_DMA_TEST");
> +	char *tests[32] = {0};
> +	int n_tests = 0;
> +	int i, j;
> +
> +	if (dma_env_str && strlen(dma_env_str) > 0) {
> +		char *additional_test = strdup(dma_env_str);
> +		if (additional_test) {
> +			n_tests = rte_strsplit(additional_test,
> strlen(additional_test), tests,
> +				       RTE_DIM(tests), ',');
> +			for (i = 0; i < n_tests; i++) {
> +				for (j = 0; j < TEST_MAX; j++) {
> +					if (!strcmp(tests[i],
> dma_add_test[j].name))
> +						dma_add_test[j].enabled =
> true;
> +				}
> +			}
> +		}
> +		free(additional_test);
> +	}
> +}
> +
>  static int
>  test_dma(void)
>  {
>  	int i;
> 
> +	parse_dma_env_var();
> +
>  	/* basic sanity on dmadev infrastructure */
>  	if (test_apis() < 0)
>  		ERR_RETURN("Error performing API tests\n");
> --
> 2.25.1

RE: [PATCH v1 2/2] test/dma: add source buffer offload free test

[Amit Prakash Shukla]

Hi Anoob,

Thanks for the review and feedback. I will make the suggested changes in next version of the patch.

Thanks,
Amit Shukla

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Tuesday, September 19, 2023 5:19 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Chengwen Feng
> <fengchengwen@huawei.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>; Amit Prakash Shukla
> <amitprakashs@marvell.com>
> Subject: RE: [PATCH v1 2/2] test/dma: add source buffer offload free test
> 
> Hi Amit,
> 
> Please see inline.
> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: Amit Prakash Shukla <amitprakashs@marvell.com>
> > Sent: Thursday, September 7, 2023 1:41 PM
> > To: Chengwen Feng <fengchengwen@huawei.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>;
> Amit
> > Prakash Shukla <amitprakashs@marvell.com>
> > Subject: [PATCH v1 2/2] test/dma: add source buffer offload free test
> >
> > Add a test case to validate the functionality of drivers' dma source
> > buffer offload free. As part of dmadev_autotest, test case will be
> > executed only if the driver supports source buffer offload free and if
> > the test is exported by env variable DPDK_ADD_DMA_TEST.
> >
> > Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> > ---
> >  app/test/test_dmadev.c | 132
> > ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 131 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c index
> > 6ef875e545..48da4664ae 100644
> > --- a/app/test/test_dmadev.c
> > +++ b/app/test/test_dmadev.c
> > @@ -18,11 +18,26 @@
> >
> >  #define ERR_RETURN(...) do { print_err(__func__, __LINE__,
> > __VA_ARGS__); return -1; } while (0)
> >
> > +#define TEST_RINGSIZE 512
> >  #define COPY_LEN 1024
> >
> >  static struct rte_mempool *pool;
> >  static uint16_t id_count;
> >
> > +enum {
> > +	TEST_SRC_BUF_FREE = 0,
> > +	TEST_MAX,
> > +};
> > +
> > +struct dma_add_test {
> > +	const char *name;
> > +	bool enabled;
> > +};
> > +
> > +struct dma_add_test dma_add_test[] = {
> > +	[TEST_SRC_BUF_FREE] = {.name = "sbuf_free", .enabled = false}, };
> > +
> >  static void
> >  __rte_format_printf(3, 4)
> >  print_err(const char *func, int lineno, const char *format, ...) @@
> > -797,10
> > +812,93 @@ test_burst_capacity(int16_t dev_id, uint16_t vchan)
> >  	return 0;
> >  }
> >
> > +static int
> > +test_sbuf_free(int16_t dev_id, uint16_t vchan) { #define NR_MBUF 256
> > +	int i, ret = 0;
> > +	int retry = 100;
> > +	uint16_t nb_done = 0;
> > +	bool dma_err = false;
> > +	uint32_t buf_cnt1, buf_cnt2;
> > +	struct rte_mempool_ops *ops;
> > +	uint64_t remote_addr = 0x40000000ull;
> 
> [Anoob] Can you make remote_addr as a variable read from env variable?
> Hard coding may not be the right approach.
> 
> > +	struct rte_mbuf *src[NR_MBUF], *dst[NR_MBUF];
> > +	const struct rte_dma_vchan_conf qconf = {
> > +		.direction = RTE_DMA_DIR_MEM_TO_DEV,
> > +		.nb_desc = TEST_RINGSIZE,
> > +		.mem_to_dev_src_buf_pool = pool,
> > +		.dst_port.port_type = RTE_DMA_PORT_PCIE,
> > +		/* Assuming pemid as 0. */
> > +		.dst_port.pcie.coreid = 0,
> > +	};
> > +	static int dev_init;
> 
> [Anoob] Can you use bool instead of int for dev_init?
> 
> > +
> > +	if (!dev_init) {
> > +		/* Stop the device to reconfigure vchan. */
> > +		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");
> > +
> > +		dev_init++;
> > +	}
> > +
> > +	if (rte_pktmbuf_alloc_bulk(pool, dst, NR_MBUF) != 0)
> > +		ERR_RETURN("alloc dst mbufs failed.\n");
> > +
> > +	for (i = 0; i < NR_MBUF; i++) {
> > +		/* Using mbuf structure to hold remote iova address. */
> > +		rte_mbuf_iova_set(dst[i], (rte_iova_t)remote_addr);
> > +		dst[i]->data_off = 0;
> > +	}
> > +
> > +	/* Capture buffer count before allocating source buffer. */
> > +	ops = rte_mempool_get_ops(pool->ops_index);
> > +	buf_cnt1 = ops->get_count(pool);
> > +
> > +	if (rte_pktmbuf_alloc_bulk(pool, src, NR_MBUF) != 0)
> > +		ERR_RETURN("alloc src mbufs failed.\n");
> 
> [Anoob] The memory is not freed in case of errors. May be you can free
> them in the end and use goto as required.
> 
> > +
> > +	if ((buf_cnt1 - NR_MBUF) != ops->get_count(pool))
> > +		ERR_RETURN("Buffer count check failed.\n");
> > +
> > +	for (i = 0; i < NR_MBUF; i++) {
> > +		ret = rte_dma_copy(dev_id, vchan,
> > rte_mbuf_data_iova(src[i]),
> > +				rte_mbuf_data_iova(dst[i]), COPY_LEN,
> > +				RTE_DMA_OP_FLAG_FREE_SBUF);
> > +
> > +		if (ret < 0)
> > +			ERR_RETURN("rte_dma_copy returned error.\n");
> > +	}
> > +
> > +	rte_dma_submit(dev_id, vchan);
> > +	nb_done = 0;
> > +	do {
> > +		nb_done += rte_dma_completed(dev_id, vchan, (NR_MBUF
> > - nb_done), NULL, &dma_err);
> > +		if (dma_err)
> > +			break;
> > +		/* Sleep for 1 millisecond */
> > +		rte_delay_us_sleep(1000);
> > +	} while (retry-- && (nb_done < NR_MBUF));
> > +
> > +	buf_cnt2 = ops->get_count(pool);
> > +	if ((buf_cnt1 != buf_cnt2) || dma_err)
> > +		ERR_RETURN("Free source buffer test failed.\n");
> > +
> > +	/* If the test passes source buffer will be freed in hardware. */
> > +	rte_pktmbuf_free_bulk(dst, NR_MBUF);
> > +
> > +	return 0;
> > +}
> > +
> >  static int
> >  test_dmadev_instance(int16_t dev_id)
> >  {
> > -#define TEST_RINGSIZE 512
> >  #define CHECK_ERRS    true
> >  	struct rte_dma_stats stats;
> >  	struct rte_dma_info info;
> > @@ -890,6 +988,12 @@ test_dmadev_instance(int16_t dev_id)
> >  	else if (runtest("fill", test_enqueue_fill, 1, dev_id, vchan,
> > CHECK_ERRS) < 0)
> >  		goto err;
> >
> > +	if ((info.dev_capa &
> > RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE) &&
> > +	    dma_add_test[TEST_SRC_BUF_FREE].enabled == true) {
> > +		if (runtest("sbuf_free", test_sbuf_free, 128, dev_id, vchan,
> > CHECK_ERRS) < 0)
> > +			goto err;
> > +	}
> > +
> >  	rte_mempool_free(pool);
> >
> >  	if (rte_dma_stop(dev_id) < 0)
> > @@ -922,11 +1026,37 @@ test_apis(void)
> >  	return ret;
> >  }
> >
> > +static void
> > +parse_dma_env_var(void)
> > +{
> > +	char *dma_env_str = getenv("DPDK_ADD_DMA_TEST");
> > +	char *tests[32] = {0};
> > +	int n_tests = 0;
> > +	int i, j;
> > +
> > +	if (dma_env_str && strlen(dma_env_str) > 0) {
> > +		char *additional_test = strdup(dma_env_str);
> > +		if (additional_test) {
> > +			n_tests = rte_strsplit(additional_test,
> > strlen(additional_test), tests,
> > +				       RTE_DIM(tests), ',');
> > +			for (i = 0; i < n_tests; i++) {
> > +				for (j = 0; j < TEST_MAX; j++) {
> > +					if (!strcmp(tests[i],
> > dma_add_test[j].name))
> > +						dma_add_test[j].enabled =
> > true;
> > +				}
> > +			}
> > +		}
> > +		free(additional_test);
> > +	}
> > +}
> > +
> >  static int
> >  test_dma(void)
> >  {
> >  	int i;
> >
> > +	parse_dma_env_var();
> > +
> >  	/* basic sanity on dmadev infrastructure */
> >  	if (test_apis() < 0)
> >  		ERR_RETURN("Error performing API tests\n");
> > --
> > 2.25.1

[PATCH v2 0/2] offload support to free dma source buffer

[Amit Prakash Shukla]

This series adds offload support to free source buffer in dma library
and adds a test support in dmadev_autotest to validate the
functionality.

v2:
- Resolved review comments.
- Fixed compilation issue.

v1:
- Implementation from RFC.
- Add test support to validate functionality.

Amit Prakash Shukla (2):
  dmadev: offload to free source buffer
  test/dma: add source buffer offload free test

 app/test/test_dmadev.c  | 167 +++++++++++++++++++++++++++++++++++++++-
 lib/dmadev/rte_dmadev.h |  27 +++++++
 2 files changed, 193 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v2 1/2] dmadev: offload to free source buffer

[Amit Prakash Shukla]

This changeset adds support in DMA library to free source DMA buffer by
hardware. On a supported hardware, application can pass on the mempool
information as part of vchan config when the DMA transfer direction is
configured as RTE_DMA_DIR_MEM_TO_DEV.

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Anoob Joseph <anoobj@marvell.com>
---
 lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h
index b157ab7600..f7a6af2528 100644
--- a/lib/dmadev/rte_dmadev.h
+++ b/lib/dmadev/rte_dmadev.h
@@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
 #define RTE_DMA_CAPA_OPS_COPY_SG	RTE_BIT64(33)
 /** Support fill operation. */
 #define RTE_DMA_CAPA_OPS_FILL		RTE_BIT64(34)
+/** Support for source buffer free for mem to dev transfer.
+ *
+ * @note Even though the DMA driver has this capability, it may not support all
+ * mempool drivers. If the mempool is not supported by the DMA driver,
+ * rte_dma_vchan_setup() will fail.
+ **/
+#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE	RTE_BIT64(35)
 /**@}*/
 
 /**
@@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
 	 * @see struct rte_dma_port_param
 	 */
 	struct rte_dma_port_param dst_port;
+	/** Mempool from which source buffer is allocated. Mempool info is used
+	 * for freeing source buffer by hardware when configured direction is
+	 * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by hardware,
+	 * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling rte_dma_copy and
+	 * rte_dma_copy_sg().
+	 *
+	 * @note If the mempool is not supported by the DMA device,
+	 * rte_dma_vchan_setup() will fail.
+	 *
+	 * @see RTE_DMA_OP_FLAG_FREE_SBUF
+	 */
+	struct rte_mempool *mem_to_dev_src_buf_pool;
+
 };
 
 /**
@@ -818,6 +838,13 @@ struct rte_dma_sge {
  * capability bit for this, driver should not return error if this flag was set.
  */
 #define RTE_DMA_OP_FLAG_LLC     RTE_BIT64(2)
+/** Mem to dev source buffer free flag.
+ * Used for freeing source DMA buffer by hardware when the transfer direction is
+ * configured as RTE_DMA_DIR_MEM_TO_DEV.
+ *
+ * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
+ */
+#define RTE_DMA_OP_FLAG_FREE_SBUF	RTE_BIT64(3)
 /**@}*/
 
 /**
-- 
2.25.1

[PATCH v2 2/2] test/dma: add source buffer offload free test

[Amit Prakash Shukla]

Add a test case to validate the functionality of drivers' dma
source buffer offload free. As part of dmadev_autotest, test case
will be executed only if the driver supports source buffer offload
free and if the test is exported by env variable DPDK_ADD_DMA_TEST.

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
---
 app/test/test_dmadev.c | 167 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 166 insertions(+), 1 deletion(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index 6ef875e545..45f1f12b17 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -18,11 +18,37 @@
 
 #define ERR_RETURN(...) do { print_err(__func__, __LINE__, __VA_ARGS__); return -1; } while (0)
 
+#define TEST_RINGSIZE 512
 #define COPY_LEN 1024
 
 static struct rte_mempool *pool;
 static uint16_t id_count;
 
+enum {
+	TEST_PARAM_REMOTE_ADDR = 0,
+	TEST_PARAM_MAX,
+};
+
+static const char * const dma_test_param[] = {
+	[TEST_PARAM_REMOTE_ADDR] = "remote_addr",
+};
+
+static uint64_t env_test_param[TEST_PARAM_MAX];
+
+enum {
+	TEST_SRC_BUF_FREE = 0,
+	TEST_MAX,
+};
+
+struct dma_add_test {
+	const char *name;
+	bool enabled;
+};
+
+struct dma_add_test dma_add_test[] = {
+	[TEST_SRC_BUF_FREE] = {.name = "sbuf_free", .enabled = false},
+};
+
 static void
 __rte_format_printf(3, 4)
 print_err(const char *func, int lineno, const char *format, ...)
@@ -797,10 +823,104 @@ test_burst_capacity(int16_t dev_id, uint16_t vchan)
 	return 0;
 }
 
+static int
+test_sbuf_free(int16_t dev_id, uint16_t vchan)
+{
+#define NR_MBUF 256
+	struct rte_mbuf *src[NR_MBUF], *dst[NR_MBUF];
+	const struct rte_dma_vchan_conf qconf = {
+		.direction = RTE_DMA_DIR_MEM_TO_DEV,
+		.nb_desc = TEST_RINGSIZE,
+		.mem_to_dev_src_buf_pool = pool,
+		.dst_port.port_type = RTE_DMA_PORT_PCIE,
+		/* Assuming pemid as 0. */
+		.dst_port.pcie.coreid = 0,
+	};
+	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;
+	int i, ret = 0;
+
+	if (!dev_init) {
+		/* Stop the device to reconfigure vchan. */
+		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");
+
+		dev_init = true;
+	}
+
+	if (rte_pktmbuf_alloc_bulk(pool, dst, NR_MBUF) != 0)
+		ERR_RETURN("alloc dst mbufs failed.\n");
+
+	for (i = 0; i < NR_MBUF; i++) {
+		/* Using mbuf structure to hold remote iova address. */
+		rte_mbuf_iova_set(dst[i], (rte_iova_t)env_test_param[TEST_PARAM_REMOTE_ADDR]);
+		dst[i]->data_off = 0;
+	}
+
+	/* Capture buffer count before allocating source buffer. */
+	ops = rte_mempool_get_ops(pool->ops_index);
+	buf_cnt1 = ops->get_count(pool);
+
+	if (rte_pktmbuf_alloc_bulk(pool, src, NR_MBUF) != 0) {
+		printf("alloc src mbufs failed.\n");
+		ret = -1;
+		goto done;
+	}
+
+	if ((buf_cnt1 - NR_MBUF) != ops->get_count(pool)) {
+		printf("Buffer count check failed.\n");
+		ret = -1;
+		goto done;
+	}
+
+	for (i = 0; i < NR_MBUF; i++) {
+		ret = rte_dma_copy(dev_id, vchan, rte_mbuf_data_iova(src[i]),
+				rte_mbuf_data_iova(dst[i]), COPY_LEN,
+				RTE_DMA_OP_FLAG_FREE_SBUF);
+
+		if (ret < 0) {
+			printf("rte_dma_copy returned error.\n");
+			goto done;
+		}
+	}
+
+	rte_dma_submit(dev_id, vchan);
+	do {
+		nb_done += rte_dma_completed(dev_id, vchan, (NR_MBUF - nb_done), NULL, &dma_err);
+		if (dma_err)
+			break;
+		/* Sleep for 1 millisecond */
+		rte_delay_us_sleep(1000);
+	} while (retry-- && (nb_done < NR_MBUF));
+
+	buf_cnt2 = ops->get_count(pool);
+	if ((buf_cnt1 != buf_cnt2) || dma_err) {
+		printf("Free source buffer test failed.\n");
+		ret = -1;
+	}
+
+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));
+
+	return ret;
+}
+
 static int
 test_dmadev_instance(int16_t dev_id)
 {
-#define TEST_RINGSIZE 512
 #define CHECK_ERRS    true
 	struct rte_dma_stats stats;
 	struct rte_dma_info info;
@@ -890,6 +1010,12 @@ test_dmadev_instance(int16_t dev_id)
 	else if (runtest("fill", test_enqueue_fill, 1, dev_id, vchan, CHECK_ERRS) < 0)
 		goto err;
 
+	if ((info.dev_capa & RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE) &&
+	    dma_add_test[TEST_SRC_BUF_FREE].enabled == true) {
+		if (runtest("sbuf_free", test_sbuf_free, 128, dev_id, vchan, CHECK_ERRS) < 0)
+			goto err;
+	}
+
 	rte_mempool_free(pool);
 
 	if (rte_dma_stop(dev_id) < 0)
@@ -922,11 +1048,50 @@ test_apis(void)
 	return ret;
 }
 
+static void
+parse_dma_env_var(void)
+{
+	char *dma_env_param_str = getenv("DPDK_ADD_DMA_TEST_PARAM");
+	char *dma_env_test_str = getenv("DPDK_ADD_DMA_TEST");
+	char *params[32] = {0};
+	char *tests[32] = {0};
+	char *var[2] = {0};
+	int n_var = 0;
+	int i, j;
+
+	/* Additional test from commandline. */
+	if (dma_env_test_str && strlen(dma_env_test_str) > 0) {
+		n_var = rte_strsplit(dma_env_test_str, strlen(dma_env_test_str), tests,
+				RTE_DIM(tests), ',');
+		for (i = 0; i < n_var; i++) {
+			for (j = 0; j < TEST_MAX; j++) {
+				if (!strcmp(tests[i], dma_add_test[j].name))
+					dma_add_test[j].enabled = true;
+			}
+		}
+	}
+
+	/* Commandline variables for test */
+	if (dma_env_param_str && strlen(dma_env_param_str) > 0) {
+		n_var = rte_strsplit(dma_env_param_str, strlen(dma_env_param_str), params,
+				       RTE_DIM(params), ',');
+		for (i = 0; i < n_var; i++) {
+			rte_strsplit(params[i], strlen(params[i]), var,	RTE_DIM(var), '=');
+			for (j = 0; j < TEST_PARAM_MAX; j++) {
+				if (!strcmp(var[0], dma_test_param[j]))
+					env_test_param[j] = strtoul(var[1], NULL, 16);
+			}
+		}
+	}
+}
+
 static int
 test_dma(void)
 {
 	int i;
 
+	parse_dma_env_var();
+
 	/* basic sanity on dmadev infrastructure */
 	if (test_apis() < 0)
 		ERR_RETURN("Error performing API tests\n");
-- 
2.25.1

[PATCH v3 0/2] offload support to free dma source buffer

[Amit Prakash Shukla]

This series adds offload support to free source buffer in dma library
and adds a test support in dmadev_autotest to validate the
functionality.

v3:
- Removed unwanted comment from code.

v2:
- Resolved review comments.
- Fixed compilation issue.

v1:
- Implementation from RFC.
- Add test support to validate functionality.

Amit Prakash Shukla (2):
  dmadev: offload to free source buffer
  test/dma: add source buffer offload free test

 app/test/test_dmadev.c  | 166 +++++++++++++++++++++++++++++++++++++++-
 lib/dmadev/rte_dmadev.h |  27 +++++++
 2 files changed, 192 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v3 1/2] dmadev: offload to free source buffer

[Amit Prakash Shukla]

This changeset adds support in DMA library to free source DMA buffer by
hardware. On a supported hardware, application can pass on the mempool
information as part of vchan config when the DMA transfer direction is
configured as RTE_DMA_DIR_MEM_TO_DEV.

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Anoob Joseph <anoobj@marvell.com>
---
 lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h
index b157ab7600..f7a6af2528 100644
--- a/lib/dmadev/rte_dmadev.h
+++ b/lib/dmadev/rte_dmadev.h
@@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
 #define RTE_DMA_CAPA_OPS_COPY_SG	RTE_BIT64(33)
 /** Support fill operation. */
 #define RTE_DMA_CAPA_OPS_FILL		RTE_BIT64(34)
+/** Support for source buffer free for mem to dev transfer.
+ *
+ * @note Even though the DMA driver has this capability, it may not support all
+ * mempool drivers. If the mempool is not supported by the DMA driver,
+ * rte_dma_vchan_setup() will fail.
+ **/
+#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE	RTE_BIT64(35)
 /**@}*/
 
 /**
@@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
 	 * @see struct rte_dma_port_param
 	 */
 	struct rte_dma_port_param dst_port;
+	/** Mempool from which source buffer is allocated. Mempool info is used
+	 * for freeing source buffer by hardware when configured direction is
+	 * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by hardware,
+	 * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling rte_dma_copy and
+	 * rte_dma_copy_sg().
+	 *
+	 * @note If the mempool is not supported by the DMA device,
+	 * rte_dma_vchan_setup() will fail.
+	 *
+	 * @see RTE_DMA_OP_FLAG_FREE_SBUF
+	 */
+	struct rte_mempool *mem_to_dev_src_buf_pool;
+
 };
 
 /**
@@ -818,6 +838,13 @@ struct rte_dma_sge {
  * capability bit for this, driver should not return error if this flag was set.
  */
 #define RTE_DMA_OP_FLAG_LLC     RTE_BIT64(2)
+/** Mem to dev source buffer free flag.
+ * Used for freeing source DMA buffer by hardware when the transfer direction is
+ * configured as RTE_DMA_DIR_MEM_TO_DEV.
+ *
+ * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
+ */
+#define RTE_DMA_OP_FLAG_FREE_SBUF	RTE_BIT64(3)
 /**@}*/
 
 /**
-- 
2.25.1

Re: [PATCH v3 1/2] dmadev: offload to free source buffer

[fengchengwen]

Hi Amit,

On 2023/9/28 19:50, Amit Prakash Shukla wrote:
> This changeset adds support in DMA library to free source DMA buffer by
> hardware. On a supported hardware, application can pass on the mempool
> information as part of vchan config when the DMA transfer direction is
> configured as RTE_DMA_DIR_MEM_TO_DEV.
> 
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Anoob Joseph <anoobj@marvell.com>
> ---
>  lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h
> index b157ab7600..f7a6af2528 100644
> --- a/lib/dmadev/rte_dmadev.h
> +++ b/lib/dmadev/rte_dmadev.h
> @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
>  #define RTE_DMA_CAPA_OPS_COPY_SG	RTE_BIT64(33)
>  /** Support fill operation. */
>  #define RTE_DMA_CAPA_OPS_FILL		RTE_BIT64(34)
> +/** Support for source buffer free for mem to dev transfer.

Support auto free source buffer once the M2D (memory-to-device) transfer completed.

> + *
> + * @note Even though the DMA driver has this capability, it may not support all
> + * mempool drivers. If the mempool is not supported by the DMA driver,
> + * rte_dma_vchan_setup() will fail.

In addition to hardware support, there are requirements for buffer attribute (e.g. only specific mempool support it).

> + **/
> +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE	RTE_BIT64(35)

1) this should follow RTE_DMA_CAPA_HANDLES_ERRORS, because it not new OPS.
2) the name is too long. how abort RTE_DMA_CAPA_AUTO_FREE_M2D_SBUF? I am not sure, suggest get better name.

>  /**@}*/
>  
>  /**
> @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
>  	 * @see struct rte_dma_port_param
>  	 */
>  	struct rte_dma_port_param dst_port;
> +	/** Mempool from which source buffer is allocated. Mempool info is used
> +	 * for freeing source buffer by hardware when configured direction is
> +	 * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by hardware,
> +	 * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling rte_dma_copy and
> +	 * rte_dma_copy_sg().
> +	 *
> +	 * @note If the mempool is not supported by the DMA device,
> +	 * rte_dma_vchan_setup() will fail.
> +	 *
> +	 * @see RTE_DMA_OP_FLAG_FREE_SBUF
> +	 */
> +	struct rte_mempool *mem_to_dev_src_buf_pool;
> +
>  };

Suggest add one extra struct e.g.
struct rte_dma_auto_free_param {
    union {
        struct rte_mempool *pool;
    }
    uint64_t reserved[2]; /**< Reserved for future fields. */
};

In the above conf, we could add a new field: struct rte_dma_auto_free_param  m2d_buf

>  
>  /**
> @@ -818,6 +838,13 @@ struct rte_dma_sge {
>   * capability bit for this, driver should not return error if this flag was set.
>   */
>  #define RTE_DMA_OP_FLAG_LLC     RTE_BIT64(2)
> +/** Mem to dev source buffer free flag.
> + * Used for freeing source DMA buffer by hardware when the transfer direction is
> + * configured as RTE_DMA_DIR_MEM_TO_DEV.
> + *
> + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
> + */
> +#define RTE_DMA_OP_FLAG_FREE_SBUF	RTE_BIT64(3)

Suggest RTE_DMA_OP_FLAG_AUTO_FREE_SBUF

>  /**@}*/
>  
>  /**
> 

The S in SBUF seem useless, because it should not auto free dstbuf in logically.

Maybe we should direct use auto-free (just like silent)

RE: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer

[Amit Prakash Shukla]

Hi Chengwen,

Thanks for the review and feedback. Please find my reply in-line.

Thanks,
Amit Shukla

> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Saturday, October 7, 2023 2:31 PM
> 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>;
> mb@smartsharesystems.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>
> Subject: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Amit,
> 
> On 2023/9/28 19:50, Amit Prakash Shukla wrote:
> > This changeset adds support in DMA library to free source DMA buffer
> > by hardware. On a supported hardware, application can pass on the
> > mempool information as part of vchan config when the DMA transfer
> > direction is configured as RTE_DMA_DIR_MEM_TO_DEV.
> >
> > Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Anoob Joseph <anoobj@marvell.com>
> > ---
> >  lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index
> > b157ab7600..f7a6af2528 100644
> > --- a/lib/dmadev/rte_dmadev.h
> > +++ b/lib/dmadev/rte_dmadev.h
> > @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
> >  #define RTE_DMA_CAPA_OPS_COPY_SG	RTE_BIT64(33)
> >  /** Support fill operation. */
> >  #define RTE_DMA_CAPA_OPS_FILL		RTE_BIT64(34)
> > +/** Support for source buffer free for mem to dev transfer.
> 
> Support auto free source buffer once the M2D (memory-to-device) transfer
> completed.

Ack

> 
> > + *
> > + * @note Even though the DMA driver has this capability, it may not
> > + support all
> > + * mempool drivers. If the mempool is not supported by the DMA
> > + driver,
> > + * rte_dma_vchan_setup() will fail.
> 
> In addition to hardware support, there are requirements for buffer attribute
> (e.g. only specific mempool support it).
> 
> > + **/
> > +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE
> 	RTE_BIT64(35)
> 
> 1) this should follow RTE_DMA_CAPA_HANDLES_ERRORS, because it not
> new OPS.
> 2) the name is too long. how abort
> RTE_DMA_CAPA_AUTO_FREE_M2D_SBUF? I am not sure, suggest get better
> name.

Ack.

> 
> >  /**@}*/
> >
> >  /**
> > @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
> >  	 * @see struct rte_dma_port_param
> >  	 */
> >  	struct rte_dma_port_param dst_port;
> > +	/** Mempool from which source buffer is allocated. Mempool info is
> used
> > +	 * for freeing source buffer by hardware when configured direction is
> > +	 * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by
> hardware,
> > +	 * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling
> rte_dma_copy and
> > +	 * rte_dma_copy_sg().
> > +	 *
> > +	 * @note If the mempool is not supported by the DMA device,
> > +	 * rte_dma_vchan_setup() will fail.
> > +	 *
> > +	 * @see RTE_DMA_OP_FLAG_FREE_SBUF
> > +	 */
> > +	struct rte_mempool *mem_to_dev_src_buf_pool;
> > +
> >  };
> 
> Suggest add one extra struct e.g.
> struct rte_dma_auto_free_param {
>     union {
>         struct rte_mempool *pool;
>     }
>     uint64_t reserved[2]; /**< Reserved for future fields. */ };
> 
> In the above conf, we could add a new field: struct
> rte_dma_auto_free_param  m2d_buf

Ack, will add new struct.
struct rte_dma_auto_free_buf_param {
        struct rte_mempool *pool;
        uint64_t reserved[2]; /**< Reserved for future fields. */ };
};

struct rte_dma_auto_free_buf_param  m2d_sbuf;

> 
> >
> >  /**
> > @@ -818,6 +838,13 @@ struct rte_dma_sge {
> >   * capability bit for this, driver should not return error if this flag was set.
> >   */
> >  #define RTE_DMA_OP_FLAG_LLC     RTE_BIT64(2)
> > +/** Mem to dev source buffer free flag.
> > + * Used for freeing source DMA buffer by hardware when the transfer
> > +direction is
> > + * configured as RTE_DMA_DIR_MEM_TO_DEV.
> > + *
> > + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
> > + */
> > +#define RTE_DMA_OP_FLAG_FREE_SBUF	RTE_BIT64(3)
> 
> Suggest RTE_DMA_OP_FLAG_AUTO_FREE_SBUF

Ack

> 
> >  /**@}*/
> >
> >  /**
> >
> 
> The S in SBUF seem useless, because it should not auto free dstbuf in
> logically.
> 
> Maybe we should direct use auto-free (just like silent)

Re: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer

[fengchengwen]

Hi Amit,

  Rethink again, I suggest use auto-free to denote this feature.

So we could re-define as:
  RTE_DMA_CAPA_M2D_AUTO_FREE
  RTE_DMA_OP_FLAG_AUTO_FREE

struct rte_dma_auto_free_param {
      union {
          struct {
              struct rte_mempool *pool;
          } m2d;
      };
      uint64_t reserved[2]; /**< Reserved for future fields. */ };
};
note: because one vchan only support one DMA direction, so we could use union.

struct rte_dma_vchan_conf {
	enum rte_dma_direction direction;
	struct rte_dma_port_param src_port;
	struct rte_dma_port_param dst_port;
	struct rte_dma_auto_free_param auto_free;
};


As for later extension, if support other type auto-free, so could:
1. add one capa: RTE_DMA_CAPA_XXX_AUTO_FREE
2. modify rte_dma_auto_free_param:
   struct rte_dma_auto_free_param {
      union {
          struct {
              struct rte_mempool *pool;
          } m2d;
          struct {
              // params;
          } xxx;
      };
      uint64_t reserved[2]; /**< Reserved for future fields. */ };
    };


Thanks.

On 2023/10/9 15:00, Amit Prakash Shukla wrote:
> Hi Chengwen,
> 
> Thanks for the review and feedback. Please find my reply in-line.
> 
> Thanks,
> Amit Shukla
> 
>> -----Original Message-----
>> From: fengchengwen <fengchengwen@huawei.com>
>> Sent: Saturday, October 7, 2023 2:31 PM
>> 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>;
>> mb@smartsharesystems.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>
>> Subject: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> Hi Amit,
>>
>> On 2023/9/28 19:50, Amit Prakash Shukla wrote:
>>> This changeset adds support in DMA library to free source DMA buffer
>>> by hardware. On a supported hardware, application can pass on the
>>> mempool information as part of vchan config when the DMA transfer
>>> direction is configured as RTE_DMA_DIR_MEM_TO_DEV.
>>>
>>> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>> Acked-by: Anoob Joseph <anoobj@marvell.com>
>>> ---
>>>  lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
>>>  1 file changed, 27 insertions(+)
>>>
>>> diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index
>>> b157ab7600..f7a6af2528 100644
>>> --- a/lib/dmadev/rte_dmadev.h
>>> +++ b/lib/dmadev/rte_dmadev.h
>>> @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
>>>  #define RTE_DMA_CAPA_OPS_COPY_SG	RTE_BIT64(33)
>>>  /** Support fill operation. */
>>>  #define RTE_DMA_CAPA_OPS_FILL		RTE_BIT64(34)
>>> +/** Support for source buffer free for mem to dev transfer.
>>
>> Support auto free source buffer once the M2D (memory-to-device) transfer
>> completed.
> 
> Ack
> 
>>
>>> + *
>>> + * @note Even though the DMA driver has this capability, it may not
>>> + support all
>>> + * mempool drivers. If the mempool is not supported by the DMA
>>> + driver,
>>> + * rte_dma_vchan_setup() will fail.
>>
>> In addition to hardware support, there are requirements for buffer attribute
>> (e.g. only specific mempool support it).
>>
>>> + **/
>>> +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE
>> 	RTE_BIT64(35)
>>
>> 1) this should follow RTE_DMA_CAPA_HANDLES_ERRORS, because it not
>> new OPS.
>> 2) the name is too long. how abort
>> RTE_DMA_CAPA_AUTO_FREE_M2D_SBUF? I am not sure, suggest get better
>> name.
> 
> Ack.
> 
>>
>>>  /**@}*/
>>>
>>>  /**
>>> @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
>>>  	 * @see struct rte_dma_port_param
>>>  	 */
>>>  	struct rte_dma_port_param dst_port;
>>> +	/** Mempool from which source buffer is allocated. Mempool info is
>> used
>>> +	 * for freeing source buffer by hardware when configured direction is
>>> +	 * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by
>> hardware,
>>> +	 * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling
>> rte_dma_copy and
>>> +	 * rte_dma_copy_sg().
>>> +	 *
>>> +	 * @note If the mempool is not supported by the DMA device,
>>> +	 * rte_dma_vchan_setup() will fail.
>>> +	 *
>>> +	 * @see RTE_DMA_OP_FLAG_FREE_SBUF
>>> +	 */
>>> +	struct rte_mempool *mem_to_dev_src_buf_pool;
>>> +
>>>  };
>>
>> Suggest add one extra struct e.g.
>> struct rte_dma_auto_free_param {
>>     union {
>>         struct rte_mempool *pool;
>>     }
>>     uint64_t reserved[2]; /**< Reserved for future fields. */ };
>>
>> In the above conf, we could add a new field: struct
>> rte_dma_auto_free_param  m2d_buf
> 
> Ack, will add new struct.
> struct rte_dma_auto_free_buf_param {
>         struct rte_mempool *pool;
>         uint64_t reserved[2]; /**< Reserved for future fields. */ };
> };
> 
> struct rte_dma_auto_free_buf_param  m2d_sbuf;
> 
>>
>>>
>>>  /**
>>> @@ -818,6 +838,13 @@ struct rte_dma_sge {
>>>   * capability bit for this, driver should not return error if this flag was set.
>>>   */
>>>  #define RTE_DMA_OP_FLAG_LLC     RTE_BIT64(2)
>>> +/** Mem to dev source buffer free flag.
>>> + * Used for freeing source DMA buffer by hardware when the transfer
>>> +direction is
>>> + * configured as RTE_DMA_DIR_MEM_TO_DEV.
>>> + *
>>> + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
>>> + */
>>> +#define RTE_DMA_OP_FLAG_FREE_SBUF	RTE_BIT64(3)
>>
>> Suggest RTE_DMA_OP_FLAG_AUTO_FREE_SBUF
> 
> Ack
> 
>>
>>>  /**@}*/
>>>
>>>  /**
>>>
>>
>> The S in SBUF seem useless, because it should not auto free dstbuf in
>> logically.
>>
>> Maybe we should direct use auto-free (just like silent)

RE: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer

[Amit Prakash Shukla]

Hi Chengwen,

Ack, I will make the changes in next version of the patch.

Thanks,
Amit Shukla

> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Monday, October 9, 2023 2:29 PM
> 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>;
> mb@smartsharesystems.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>
> Subject: Re: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer
> 
> Hi Amit,
> 
>   Rethink again, I suggest use auto-free to denote this feature.
> 
> So we could re-define as:
>   RTE_DMA_CAPA_M2D_AUTO_FREE
>   RTE_DMA_OP_FLAG_AUTO_FREE
> 
> struct rte_dma_auto_free_param {
>       union {
>           struct {
>               struct rte_mempool *pool;
>           } m2d;
>       };
>       uint64_t reserved[2]; /**< Reserved for future fields. */ }; };
> note: because one vchan only support one DMA direction, so we could use
> union.
> 
> struct rte_dma_vchan_conf {
> 	enum rte_dma_direction direction;
> 	struct rte_dma_port_param src_port;
> 	struct rte_dma_port_param dst_port;
> 	struct rte_dma_auto_free_param auto_free; };
> 
> 
> As for later extension, if support other type auto-free, so could:
> 1. add one capa: RTE_DMA_CAPA_XXX_AUTO_FREE 2. modify
> rte_dma_auto_free_param:
>    struct rte_dma_auto_free_param {
>       union {
>           struct {
>               struct rte_mempool *pool;
>           } m2d;
>           struct {
>               // params;
>           } xxx;
>       };
>       uint64_t reserved[2]; /**< Reserved for future fields. */ };
>     };
> 
> 
> Thanks.
> 

<snip>

[PATCH v3 2/2] test/dma: add source buffer offload free test

[Amit Prakash Shukla]

Add a test case to validate the functionality of drivers' dma
source buffer offload free. As part of dmadev_autotest, test case
will be executed only if the driver supports source buffer offload
free and if the test is exported by env variable DPDK_ADD_DMA_TEST.

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
---
 app/test/test_dmadev.c | 166 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 165 insertions(+), 1 deletion(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index 6ef875e545..0fdfe983af 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -18,11 +18,37 @@
 
 #define ERR_RETURN(...) do { print_err(__func__, __LINE__, __VA_ARGS__); return -1; } while (0)
 
+#define TEST_RINGSIZE 512
 #define COPY_LEN 1024
 
 static struct rte_mempool *pool;
 static uint16_t id_count;
 
+enum {
+	TEST_PARAM_REMOTE_ADDR = 0,
+	TEST_PARAM_MAX,
+};
+
+static const char * const dma_test_param[] = {
+	[TEST_PARAM_REMOTE_ADDR] = "remote_addr",
+};
+
+static uint64_t env_test_param[TEST_PARAM_MAX];
+
+enum {
+	TEST_SRC_BUF_FREE = 0,
+	TEST_MAX,
+};
+
+struct dma_add_test {
+	const char *name;
+	bool enabled;
+};
+
+struct dma_add_test dma_add_test[] = {
+	[TEST_SRC_BUF_FREE] = {.name = "sbuf_free", .enabled = false},
+};
+
 static void
 __rte_format_printf(3, 4)
 print_err(const char *func, int lineno, const char *format, ...)
@@ -797,10 +823,103 @@ test_burst_capacity(int16_t dev_id, uint16_t vchan)
 	return 0;
 }
 
+static int
+test_sbuf_free(int16_t dev_id, uint16_t vchan)
+{
+#define NR_MBUF 256
+	struct rte_mbuf *src[NR_MBUF], *dst[NR_MBUF];
+	const struct rte_dma_vchan_conf qconf = {
+		.direction = RTE_DMA_DIR_MEM_TO_DEV,
+		.nb_desc = TEST_RINGSIZE,
+		.mem_to_dev_src_buf_pool = pool,
+		.dst_port.port_type = RTE_DMA_PORT_PCIE,
+		.dst_port.pcie.coreid = 0,
+	};
+	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;
+	int i, ret = 0;
+
+	if (!dev_init) {
+		/* Stop the device to reconfigure vchan. */
+		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");
+
+		dev_init = true;
+	}
+
+	if (rte_pktmbuf_alloc_bulk(pool, dst, NR_MBUF) != 0)
+		ERR_RETURN("alloc dst mbufs failed.\n");
+
+	for (i = 0; i < NR_MBUF; i++) {
+		/* Using mbuf structure to hold remote iova address. */
+		rte_mbuf_iova_set(dst[i], (rte_iova_t)env_test_param[TEST_PARAM_REMOTE_ADDR]);
+		dst[i]->data_off = 0;
+	}
+
+	/* Capture buffer count before allocating source buffer. */
+	ops = rte_mempool_get_ops(pool->ops_index);
+	buf_cnt1 = ops->get_count(pool);
+
+	if (rte_pktmbuf_alloc_bulk(pool, src, NR_MBUF) != 0) {
+		printf("alloc src mbufs failed.\n");
+		ret = -1;
+		goto done;
+	}
+
+	if ((buf_cnt1 - NR_MBUF) != ops->get_count(pool)) {
+		printf("Buffer count check failed.\n");
+		ret = -1;
+		goto done;
+	}
+
+	for (i = 0; i < NR_MBUF; i++) {
+		ret = rte_dma_copy(dev_id, vchan, rte_mbuf_data_iova(src[i]),
+				rte_mbuf_data_iova(dst[i]), COPY_LEN,
+				RTE_DMA_OP_FLAG_FREE_SBUF);
+
+		if (ret < 0) {
+			printf("rte_dma_copy returned error.\n");
+			goto done;
+		}
+	}
+
+	rte_dma_submit(dev_id, vchan);
+	do {
+		nb_done += rte_dma_completed(dev_id, vchan, (NR_MBUF - nb_done), NULL, &dma_err);
+		if (dma_err)
+			break;
+		/* Sleep for 1 millisecond */
+		rte_delay_us_sleep(1000);
+	} while (retry-- && (nb_done < NR_MBUF));
+
+	buf_cnt2 = ops->get_count(pool);
+	if ((buf_cnt1 != buf_cnt2) || dma_err) {
+		printf("Free source buffer test failed.\n");
+		ret = -1;
+	}
+
+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));
+
+	return ret;
+}
+
 static int
 test_dmadev_instance(int16_t dev_id)
 {
-#define TEST_RINGSIZE 512
 #define CHECK_ERRS    true
 	struct rte_dma_stats stats;
 	struct rte_dma_info info;
@@ -890,6 +1009,12 @@ test_dmadev_instance(int16_t dev_id)
 	else if (runtest("fill", test_enqueue_fill, 1, dev_id, vchan, CHECK_ERRS) < 0)
 		goto err;
 
+	if ((info.dev_capa & RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE) &&
+	    dma_add_test[TEST_SRC_BUF_FREE].enabled == true) {
+		if (runtest("sbuf_free", test_sbuf_free, 128, dev_id, vchan, CHECK_ERRS) < 0)
+			goto err;
+	}
+
 	rte_mempool_free(pool);
 
 	if (rte_dma_stop(dev_id) < 0)
@@ -922,11 +1047,50 @@ test_apis(void)
 	return ret;
 }
 
+static void
+parse_dma_env_var(void)
+{
+	char *dma_env_param_str = getenv("DPDK_ADD_DMA_TEST_PARAM");
+	char *dma_env_test_str = getenv("DPDK_ADD_DMA_TEST");
+	char *params[32] = {0};
+	char *tests[32] = {0};
+	char *var[2] = {0};
+	int n_var = 0;
+	int i, j;
+
+	/* Additional test from commandline. */
+	if (dma_env_test_str && strlen(dma_env_test_str) > 0) {
+		n_var = rte_strsplit(dma_env_test_str, strlen(dma_env_test_str), tests,
+				RTE_DIM(tests), ',');
+		for (i = 0; i < n_var; i++) {
+			for (j = 0; j < TEST_MAX; j++) {
+				if (!strcmp(tests[i], dma_add_test[j].name))
+					dma_add_test[j].enabled = true;
+			}
+		}
+	}
+
+	/* Commandline variables for test */
+	if (dma_env_param_str && strlen(dma_env_param_str) > 0) {
+		n_var = rte_strsplit(dma_env_param_str, strlen(dma_env_param_str), params,
+				       RTE_DIM(params), ',');
+		for (i = 0; i < n_var; i++) {
+			rte_strsplit(params[i], strlen(params[i]), var,	RTE_DIM(var), '=');
+			for (j = 0; j < TEST_PARAM_MAX; j++) {
+				if (!strcmp(var[0], dma_test_param[j]))
+					env_test_param[j] = strtoul(var[1], NULL, 16);
+			}
+		}
+	}
+}
+
 static int
 test_dma(void)
 {
 	int i;
 
+	parse_dma_env_var();
+
 	/* basic sanity on dmadev infrastructure */
 	if (test_apis() < 0)
 		ERR_RETURN("Error performing API tests\n");
-- 
2.25.1

RE: [PATCH v3 2/2] test/dma: add source buffer offload free test

[Anoob Joseph]

> 
> Add a test case to validate the functionality of drivers' dma source buffer
> offload free. As part of dmadev_autotest, test case will be executed only if
> the driver supports source buffer offload free and if the test is exported by
> env variable DPDK_ADD_DMA_TEST.
> 
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>

Acked-by: Anoob Joseph <anoobj@marvell.com>

Re: [PATCH v3 2/2] test/dma: add source buffer offload free test

[fengchengwen]

Hi Amit,

On 2023/9/28 19:50, Amit Prakash Shukla wrote:
> Add a test case to validate the functionality of drivers' dma
> source buffer offload free. As part of dmadev_autotest, test case
> will be executed only if the driver supports source buffer offload
> free and if the test is exported by env variable DPDK_ADD_DMA_TEST.

Why should under control by the env variable?

> 
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> ---
>  app/test/test_dmadev.c | 166 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 165 insertions(+), 1 deletion(-)
> 

...

RE: [EXT] Re: [PATCH v3 2/2] test/dma: add source buffer offload free test

[Amit Prakash Shukla]

> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Saturday, October 7, 2023 3:03 PM
> 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>;
> mb@smartsharesystems.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>
> Subject: [EXT] Re: [PATCH v3 2/2] test/dma: add source buffer offload free
> test
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Amit,
> 
> On 2023/9/28 19:50, Amit Prakash Shukla wrote:
> > Add a test case to validate the functionality of drivers' dma source
> > buffer offload free. As part of dmadev_autotest, test case will be
> > executed only if the driver supports source buffer offload free and if
> > the test is exported by env variable DPDK_ADD_DMA_TEST.
> 
> Why should under control by the env variable?

Added env variable for the setups that do not have PCIe endpoint and do not want to run the test for DMA source buffer free even though the driver supports the feature.

> 
> >
> > Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> > ---
> >  app/test/test_dmadev.c | 166
> > ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 165 insertions(+), 1 deletion(-)
> >
> 
> ...

RE: [PATCH v3 0/2] offload support to free dma source buffer

[Anoob Joseph]

> 
> This series adds offload support to free source buffer in dma library and adds
> a test support in dmadev_autotest to validate the functionality.
> 
> v3:
> - Removed unwanted comment from code.
> 
> v2:
> - Resolved review comments.
> - Fixed compilation issue.
> 
> v1:
> - Implementation from RFC.
> - Add test support to validate functionality.
> 
> Amit Prakash Shukla (2):
>   dmadev: offload to free source buffer
>   test/dma: add source buffer offload free test
> 
>  app/test/test_dmadev.c  | 166
> +++++++++++++++++++++++++++++++++++++++-
>  lib/dmadev/rte_dmadev.h |  27 +++++++
>  2 files changed, 192 insertions(+), 1 deletion(-)
> 
> --
> 2.25.1

Series Acked-by: Anoob Joseph <anoobj@marvell.com>

RE: [PATCH v3 0/2] offload support to free dma source buffer

[Amit Prakash Shukla]

Hi Thomas,

If there are no other review comments, could you please consider merging this series in RC1.

There is a PMD patch pending on this series to get merged.

PMD patch: https://patches.dpdk.org/project/dpdk/patch/20230907082443.1002665-1-amitprakashs@marvell.com/

Thanks,
Amit Shukla

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Thursday, September 28, 2023 5:29 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> mb@smartsharesystems.com; fengchengwen@huawei.com;
> kevin.laatz@intel.com; bruce.richardson@intel.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>; Amit Prakash Shukla
> <amitprakashs@marvell.com>
> Subject: RE: [PATCH v3 0/2] offload support to free dma source buffer
> 
> >
> > This series adds offload support to free source buffer in dma library
> > and adds a test support in dmadev_autotest to validate the functionality.
> >
> > v3:
> > - Removed unwanted comment from code.
> >
> > v2:
> > - Resolved review comments.
> > - Fixed compilation issue.
> >
> > v1:
> > - Implementation from RFC.
> > - Add test support to validate functionality.
> >
> > Amit Prakash Shukla (2):
> >   dmadev: offload to free source buffer
> >   test/dma: add source buffer offload free test
> >
> >  app/test/test_dmadev.c  | 166
> > +++++++++++++++++++++++++++++++++++++++-
> >  lib/dmadev/rte_dmadev.h |  27 +++++++
> >  2 files changed, 192 insertions(+), 1 deletion(-)
> >
> > --
> > 2.25.1
> 
> Series Acked-by: Anoob Joseph <anoobj@marvell.com>

[PATCH v4 0/2] offload support to auto free dma buffer

[Amit Prakash Shukla]

This series adds offload support to auto free buffer in dma library
and adds a test support in dmadev_autotest to validate the functionality.

v4:
- Resolved review comments.

v3:
- Removed unwanted comment from code.

v2:
- Resolved review comments.
- Fixed compilation issue.

v1:
- Implementation from RFC.
- Add test support to validate functionality.

Amit Prakash Shukla (2):
  dmadev: offload to auto free DMA buffer
  test/dma: auto free offload test to free DMA buffer

 app/test/test_dmadev.c  | 167 +++++++++++++++++++++++++++++++++++++++-
 lib/dmadev/rte_dmadev.h |  43 +++++++++++
 2 files changed, 209 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v4 1/2] dmadev: offload to auto free DMA buffer

[Amit Prakash Shukla]

This changeset adds support in DMA library to auto free DMA buffer by
hardware. On a supported hardware, application can pass on the mempool
information as part of vchan config.

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Anoob Joseph <anoobj@marvell.com>
---
 lib/dmadev/rte_dmadev.h | 43 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h
index b157ab7600..493263a5d6 100644
--- a/lib/dmadev/rte_dmadev.h
+++ b/lib/dmadev/rte_dmadev.h
@@ -269,6 +269,14 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
  * must ensure that all memory addresses are valid and accessible by HW.
  */
 #define RTE_DMA_CAPA_HANDLES_ERRORS	RTE_BIT64(6)
+/** Support auto free for source buffer once mem to dev transfer is completed.
+ *
+ * @note Even though the DMA driver has this capability, it may not support all
+ * mempool drivers. If the mempool is not supported by the DMA driver,
+ * rte_dma_vchan_setup() will fail.
+ */
+#define RTE_DMA_CAPA_M2D_AUTO_FREE      RTE_BIT64(7)
+
 /** Support copy operation.
  * This capability start with index of 32, so that it could leave gap between
  * normal capability and ops capability.
@@ -552,6 +560,26 @@ struct rte_dma_port_param {
 	uint64_t reserved[2]; /**< Reserved for future fields. */
 };
 
+/**
+ * A structure used for offload auto free params.
+ */
+struct rte_dma_auto_free_param {
+	union {
+		struct {
+			/**
+			 * Mempool from which buffer is allocated. Mempool info
+			 * is used for freeing buffer by hardware.
+			 *
+			 * @note If the mempool is not supported by the DMA device,
+			 * rte_dma_vchan_setup() will fail.
+			 */
+			struct rte_mempool *pool;
+		} m2d;
+	};
+	/** Reserved for future fields. */
+	uint64_t reserved[2];
+};
+
 /**
  * A structure used to configure a virtual DMA channel.
  *
@@ -581,6 +609,14 @@ struct rte_dma_vchan_conf {
 	 * @see struct rte_dma_port_param
 	 */
 	struct rte_dma_port_param dst_port;
+	/** Buffer params to auto free buffer by hardware. To free the buffer
+	 * by hardware, RTE_DMA_OP_FLAG_AUTO_FREE must be set while calling
+	 * rte_dma_copy and rte_dma_copy_sg().
+	 *
+	 * @see RTE_DMA_OP_FLAG_AUTO_FREE
+	 * @see struct rte_dma_auto_free_param
+	 */
+	struct rte_dma_auto_free_param auto_free;
 };
 
 /**
@@ -818,6 +854,13 @@ struct rte_dma_sge {
  * capability bit for this, driver should not return error if this flag was set.
  */
 #define RTE_DMA_OP_FLAG_LLC     RTE_BIT64(2)
+/** Auto free buffer flag.
+ * Operation with this flag must issue command to hardware to free the DMA
+ * buffer after DMA transfer is completed.
+ *
+ * @see struct rte_dma_vchan_conf::auto_free
+ */
+#define RTE_DMA_OP_FLAG_AUTO_FREE	RTE_BIT64(3)
 /**@}*/
 
 /**
-- 
2.25.1

Re: [PATCH v4 1/2] dmadev: offload to auto free DMA buffer

[fengchengwen]

[-- Attachment #1: Type: text/plain, Size: 568 bytes --]

Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2023/10/9 20:02, Amit Prakash Shukla wrote:
> This changeset adds support in DMA library to auto free DMA buffer by
> hardware. On a supported hardware, application can pass on the mempool
> information as part of vchan config.
>
> Signed-off-by: Amit Prakash Shukla<amitprakashs@marvell.com>
> Acked-by: Morten Brørup<mb@smartsharesystems.com>
> Acked-by: Anoob Joseph<anoobj@marvell.com>
> ---
>   lib/dmadev/rte_dmadev.h | 43 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 43 insertions(+)
> ...

[-- Attachment #2: Type: text/html, Size: 1511 bytes --]

RE: [EXT] Re: [PATCH v4 1/2] dmadev: offload to auto free DMA buffer

[Amit Prakash Shukla]

[-- Attachment #1: Type: text/plain, Size: 1518 bytes --]

Hi Thomas,

Gentle ping.


Could you please consider merging this series in RC1.

Thanks,
Amit Shukla

From: fengchengwen <fengchengwen@huawei.com>
Sent: Tuesday, October 10, 2023 7:10 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 v4 1/2] dmadev: offload to auto free DMA buffer

External Email
________________________________

Acked-by: Chengwen Feng <fengchengwen@huawei.com><mailto:fengchengwen@huawei.com>


On 2023/10/9 20:02, Amit Prakash Shukla wrote:

This changeset adds support in DMA library to auto free DMA buffer by

hardware. On a supported hardware, application can pass on the mempool

information as part of vchan config.



Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com><mailto:amitprakashs@marvell.com>

Acked-by: Morten Brørup <mb@smartsharesystems.com><mailto:mb@smartsharesystems.com>

Acked-by: Anoob Joseph <anoobj@marvell.com><mailto:anoobj@marvell.com>

---

 lib/dmadev/rte_dmadev.h | 43 +++++++++++++++++++++++++++++++++++++++++

 1 file changed, 43 insertions(+)

...

[-- Attachment #2: Type: text/html, Size: 5636 bytes --]

[PATCH v4 2/2] test/dma: auto free offload test to free DMA buffer

[Amit Prakash Shukla]

Add a test case to validate the functionality of drivers' dma
buffer offload auto free. As part of dmadev_autotest, test case
will be executed only if the driver supports buffer offload auto
free and if the test is exported by env variable DPDK_ADD_DMA_TEST.

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
Acked-by: Anoob Joseph <anoobj@marvell.com>
---
 app/test/test_dmadev.c | 167 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 166 insertions(+), 1 deletion(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index 6ef875e545..216f84b6bb 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -18,11 +18,37 @@
 
 #define ERR_RETURN(...) do { print_err(__func__, __LINE__, __VA_ARGS__); return -1; } while (0)
 
+#define TEST_RINGSIZE 512
 #define COPY_LEN 1024
 
 static struct rte_mempool *pool;
 static uint16_t id_count;
 
+enum {
+	TEST_PARAM_REMOTE_ADDR = 0,
+	TEST_PARAM_MAX,
+};
+
+static const char * const dma_test_param[] = {
+	[TEST_PARAM_REMOTE_ADDR] = "remote_addr",
+};
+
+static uint64_t env_test_param[TEST_PARAM_MAX];
+
+enum {
+	TEST_M2D_AUTO_FREE = 0,
+	TEST_MAX,
+};
+
+struct dma_add_test {
+	const char *name;
+	bool enabled;
+};
+
+struct dma_add_test dma_add_test[] = {
+	[TEST_M2D_AUTO_FREE] = {.name = "m2d_auto_free", .enabled = false},
+};
+
 static void
 __rte_format_printf(3, 4)
 print_err(const char *func, int lineno, const char *format, ...)
@@ -797,10 +823,103 @@ test_burst_capacity(int16_t dev_id, uint16_t vchan)
 	return 0;
 }
 
+static int
+test_m2d_auto_free(int16_t dev_id, uint16_t vchan)
+{
+#define NR_MBUF 256
+	struct rte_mbuf *src[NR_MBUF], *dst[NR_MBUF];
+	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,
+	};
+	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;
+	int i, ret = 0;
+
+	if (!dev_init) {
+		/* Stop the device to reconfigure vchan. */
+		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");
+
+		dev_init = true;
+	}
+
+	if (rte_pktmbuf_alloc_bulk(pool, dst, NR_MBUF) != 0)
+		ERR_RETURN("alloc dst mbufs failed.\n");
+
+	for (i = 0; i < NR_MBUF; i++) {
+		/* Using mbuf structure to hold remote iova address. */
+		rte_mbuf_iova_set(dst[i], (rte_iova_t)env_test_param[TEST_PARAM_REMOTE_ADDR]);
+		dst[i]->data_off = 0;
+	}
+
+	/* Capture buffer count before allocating source buffer. */
+	ops = rte_mempool_get_ops(pool->ops_index);
+	buf_cnt1 = ops->get_count(pool);
+
+	if (rte_pktmbuf_alloc_bulk(pool, src, NR_MBUF) != 0) {
+		printf("alloc src mbufs failed.\n");
+		ret = -1;
+		goto done;
+	}
+
+	if ((buf_cnt1 - NR_MBUF) != ops->get_count(pool)) {
+		printf("Buffer count check failed.\n");
+		ret = -1;
+		goto done;
+	}
+
+	for (i = 0; i < NR_MBUF; i++) {
+		ret = rte_dma_copy(dev_id, vchan, rte_mbuf_data_iova(src[i]),
+				   rte_mbuf_data_iova(dst[i]), COPY_LEN,
+				   RTE_DMA_OP_FLAG_AUTO_FREE);
+
+		if (ret < 0) {
+			printf("rte_dma_copy returned error.\n");
+			goto done;
+		}
+	}
+
+	rte_dma_submit(dev_id, vchan);
+	do {
+		nb_done += rte_dma_completed(dev_id, vchan, (NR_MBUF - nb_done), NULL, &dma_err);
+		if (dma_err)
+			break;
+		/* Sleep for 1 millisecond */
+		rte_delay_us_sleep(1000);
+	} while (retry-- && (nb_done < NR_MBUF));
+
+	buf_cnt2 = ops->get_count(pool);
+	if ((buf_cnt1 != buf_cnt2) || dma_err) {
+		printf("Free mem to dev buffer test failed.\n");
+		ret = -1;
+	}
+
+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));
+
+	return ret;
+}
+
 static int
 test_dmadev_instance(int16_t dev_id)
 {
-#define TEST_RINGSIZE 512
 #define CHECK_ERRS    true
 	struct rte_dma_stats stats;
 	struct rte_dma_info info;
@@ -890,6 +1009,13 @@ test_dmadev_instance(int16_t dev_id)
 	else if (runtest("fill", test_enqueue_fill, 1, dev_id, vchan, CHECK_ERRS) < 0)
 		goto err;
 
+	if ((info.dev_capa & RTE_DMA_CAPA_M2D_AUTO_FREE) &&
+	    dma_add_test[TEST_M2D_AUTO_FREE].enabled == true) {
+		if (runtest("m2d_auto_free", test_m2d_auto_free, 128, dev_id, vchan,
+			    CHECK_ERRS) < 0)
+			goto err;
+	}
+
 	rte_mempool_free(pool);
 
 	if (rte_dma_stop(dev_id) < 0)
@@ -922,11 +1048,50 @@ test_apis(void)
 	return ret;
 }
 
+static void
+parse_dma_env_var(void)
+{
+	char *dma_env_param_str = getenv("DPDK_ADD_DMA_TEST_PARAM");
+	char *dma_env_test_str = getenv("DPDK_ADD_DMA_TEST");
+	char *params[32] = {0};
+	char *tests[32] = {0};
+	char *var[2] = {0};
+	int n_var = 0;
+	int i, j;
+
+	/* Additional test from commandline. */
+	if (dma_env_test_str && strlen(dma_env_test_str) > 0) {
+		n_var = rte_strsplit(dma_env_test_str, strlen(dma_env_test_str), tests,
+				RTE_DIM(tests), ',');
+		for (i = 0; i < n_var; i++) {
+			for (j = 0; j < TEST_MAX; j++) {
+				if (!strcmp(tests[i], dma_add_test[j].name))
+					dma_add_test[j].enabled = true;
+			}
+		}
+	}
+
+	/* Commandline variables for test */
+	if (dma_env_param_str && strlen(dma_env_param_str) > 0) {
+		n_var = rte_strsplit(dma_env_param_str, strlen(dma_env_param_str), params,
+				       RTE_DIM(params), ',');
+		for (i = 0; i < n_var; i++) {
+			rte_strsplit(params[i], strlen(params[i]), var,	RTE_DIM(var), '=');
+			for (j = 0; j < TEST_PARAM_MAX; j++) {
+				if (!strcmp(var[0], dma_test_param[j]))
+					env_test_param[j] = strtoul(var[1], NULL, 16);
+			}
+		}
+	}
+}
+
 static int
 test_dma(void)
 {
 	int i;
 
+	parse_dma_env_var();
+
 	/* basic sanity on dmadev infrastructure */
 	if (test_apis() < 0)
 		ERR_RETURN("Error performing API tests\n");
-- 
2.25.1

Re: [PATCH v4 0/2] offload support to auto free dma buffer

[Thomas Monjalon]

09/10/2023 14:02, Amit Prakash Shukla:
> This series adds offload support to auto free buffer in dma library
> and adds a test support in dmadev_autotest to validate the functionality.
> 
> Amit Prakash Shukla (2):
>   dmadev: offload to auto free DMA buffer
>   test/dma: auto free offload test to free DMA buffer

Squashed and applied, thanks.