[v2] app/dma-perf: support bi-directional transfer

Message ID 20240227192601.3932913-1-amitprakashs@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] app/dma-perf: support bi-directional transfer |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation warning apply patch failure
ci/github-robot: build success github build: passed
ci/iol-testing warning apply patch failure
ci/Intel-compilation warning apply issues

Commit Message

Amit Prakash Shukla Feb. 27, 2024, 7:26 p.m. UTC
  Adds bi-directional DMA transfer support to test performance.
One DMA device on one core will do mem2dev transfer and another
DMA device on another core will do dev2mem transfer.

Depends-on: series-31252 ("PCI Dev and SG copy support")

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
---
v2:
- Fixed depends on series.

 app/test-dma-perf/benchmark.c | 64 +++++++++++++++++++++++++++--------
 app/test-dma-perf/config.ini  |  5 +++
 app/test-dma-perf/main.c      | 18 +++++++++-
 app/test-dma-perf/main.h      |  1 +
 4 files changed, 73 insertions(+), 15 deletions(-)
  

Comments

Chengwen Feng Feb. 28, 2024, 7:03 a.m. UTC | #1
Hi Amit and Gowrishankar,

It's nature to support multiple dmadev test in one testcase, and the original framework supports it.
But it seem we both complicated it when adding support for non-mem2mem dma test.

The new added "direction" and "vchan_dev" could treat as the dmadev's private configure,
some thing like:
  lcore_dma=lcore10@0000:00:04.2,vchan=0,dir=mem2dev,devtype=pcie,raddr=xxx,coreid=1,pfid=2,vfid=3

then this bi-directional could impl only with config:
  lcore_dma=lcore10@0000:00:04.2,dir=mem2dev,devtype=pcie,raddr=xxx,coreid=1,pfid=2,vfid=3, lcore11@0000:00:04.3,dir=dev2mem,devtype=pcie,raddr=xxx,coreid=1,pfid=2,vfid=3
so that the lcore10 will do mem2dev with device 0000:00:04.2, while lcore11 will do dev2mem with device 0000:00:04.3.

Thanks

On 2024/2/28 3:26, Amit Prakash Shukla wrote:
> Adds bi-directional DMA transfer support to test performance.
> One DMA device on one core will do mem2dev transfer and another
> DMA device on another core will do dev2mem transfer.
> 
> Depends-on: series-31252 ("PCI Dev and SG copy support")
> 
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> ---
> v2:
> - Fixed depends on series.
> 
>  app/test-dma-perf/benchmark.c | 64 +++++++++++++++++++++++++++--------
>  app/test-dma-perf/config.ini  |  5 +++
>  app/test-dma-perf/main.c      | 18 +++++++++-
>  app/test-dma-perf/main.h      |  1 +
>  4 files changed, 73 insertions(+), 15 deletions(-)
> 
> diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
> index 25ed6fa6d0..8a23944763 100644
> --- a/app/test-dma-perf/benchmark.c
> +++ b/app/test-dma-perf/benchmark.c
> @@ -144,12 +144,19 @@ cache_flush_buf(__rte_unused struct rte_mbuf **array,
>  
>  static int
>  vchan_data_populate(uint32_t dev_id, struct rte_dma_vchan_conf *qconf,
> -		    struct test_configure *cfg)
> +		    struct test_configure *cfg, uint16_t dev_num)
>  {
>  	struct rte_dma_info info;
>  
>  	qconf->direction = cfg->transfer_dir;
>  
> +	/* If its a bi-directional test, configure odd device for inbound dma
> +	 * transfer and even device for outbound dma transfer.
> +	 */
> +	if (cfg->is_bidir)
> +		qconf->direction = (dev_num % 2) ? RTE_DMA_DIR_MEM_TO_DEV :
> +				   RTE_DMA_DIR_DEV_TO_MEM;
> +
>  	rte_dma_info_get(dev_id, &info);
>  	if (!(RTE_BIT64(qconf->direction) & info.dev_capa))
>  		return -1;
> @@ -181,14 +188,15 @@ vchan_data_populate(uint32_t dev_id, struct rte_dma_vchan_conf *qconf,
>  
>  /* Configuration of device. */
>  static void
> -configure_dmadev_queue(uint32_t dev_id, struct test_configure *cfg, uint8_t ptrs_max)
> +configure_dmadev_queue(uint32_t dev_id, struct test_configure *cfg, uint8_t ptrs_max,
> +		       uint16_t dev_num)
>  {
>  	uint16_t vchan = 0;
>  	struct rte_dma_info info;
>  	struct rte_dma_conf dev_config = { .nb_vchans = 1 };
>  	struct rte_dma_vchan_conf qconf = { 0 };
>  
> -	if (vchan_data_populate(dev_id, &qconf, cfg) != 0)
> +	if (vchan_data_populate(dev_id, &qconf, cfg, dev_num) != 0)
>  		rte_exit(EXIT_FAILURE, "Error with vchan data populate.\n");
>  
>  	if (rte_dma_configure(dev_id, &dev_config) != 0)
> @@ -235,7 +243,7 @@ config_dmadevs(struct test_configure *cfg)
>  		}
>  
>  		ldm->dma_ids[i] = dev_id;
> -		configure_dmadev_queue(dev_id, cfg, ptrs_max);
> +		configure_dmadev_queue(dev_id, cfg, ptrs_max, nb_dmadevs);
>  		++nb_dmadevs;
>  	}
>  
> @@ -504,7 +512,7 @@ setup_memory_env(struct test_configure *cfg,
>  		}
>  	}
>  
> -	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) {
> +	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM && !cfg->is_bidir) {
>  		ext_buf_info->free_cb = dummy_free_ext_buf;
>  		ext_buf_info->fcb_opaque = NULL;
>  		for (i = 0; i < nr_buf; i++) {
> @@ -516,7 +524,7 @@ setup_memory_env(struct test_configure *cfg,
>  		}
>  	}
>  
> -	if (cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV) {
> +	if (cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV && !cfg->is_bidir) {
>  		ext_buf_info->free_cb = dummy_free_ext_buf;
>  		ext_buf_info->fcb_opaque = NULL;
>  		for (i = 0; i < nr_buf; i++) {
> @@ -528,6 +536,18 @@ setup_memory_env(struct test_configure *cfg,
>  		}
>  	}
>  
> +	if (cfg->is_bidir) {
> +		ext_buf_info->free_cb = dummy_free_ext_buf;
> +		ext_buf_info->fcb_opaque = NULL;
> +		for (i = 0; i < nr_buf; i++) {
> +			/* Using mbuf structure to hold remote iova address. */
> +			rte_pktmbuf_attach_extbuf((*srcs)[i], (void *)(cfg->vchan_dev.raddr +
> +						 (i * buf_size)), (rte_iova_t)(cfg->vchan_dev.raddr +
> +						 (i * buf_size)), 0, ext_buf_info);
> +			rte_mbuf_ext_refcnt_update(ext_buf_info, 1);
> +		}
> +	}
> +
>  	if (cfg->is_sg) {
>  		uint8_t src_ptrs = cfg->src_ptrs;
>  		uint8_t dst_ptrs = cfg->dst_ptrs;
> @@ -649,16 +669,30 @@ mem_copy_benchmark(struct test_configure *cfg)
>  		lcores[i]->nr_buf = (uint32_t)(nr_buf / nb_workers);
>  		lcores[i]->buf_size = buf_size;
>  		lcores[i]->test_secs = test_secs;
> -		lcores[i]->srcs = srcs + offset;
> -		lcores[i]->dsts = dsts + offset;
>  		lcores[i]->scenario_id = cfg->scenario_id;
>  		lcores[i]->lcore_id = lcore_id;
>  
> -		if (cfg->is_sg) {
> -			lcores[i]->src_ptrs = cfg->src_ptrs;
> -			lcores[i]->dst_ptrs = cfg->dst_ptrs;
> -			lcores[i]->src_sges = src_sges + (nr_sgsrc / nb_workers * i);
> -			lcores[i]->dst_sges = dst_sges + (nr_sgdst / nb_workers * i);
> +		/* Number of workers is equal to number of devices. In case of bi-directional
> +		 * dma, use 1 device for mem-to-dev and 1 device for dev-to-mem.
> +		 */
> +		if (cfg->is_dma && cfg->is_bidir && (i % 2 != 0)) {
> +			lcores[i]->dsts = srcs + offset;
> +			lcores[i]->srcs = dsts + offset;
> +			if (cfg->is_sg) {
> +				lcores[i]->dst_ptrs = cfg->src_ptrs;
> +				lcores[i]->src_ptrs = cfg->dst_ptrs;
> +				lcores[i]->dst_sges = src_sges + (nr_sgsrc / nb_workers * i);
> +				lcores[i]->src_sges = dst_sges + (nr_sgdst / nb_workers * i);
> +			}
> +		} else {
> +			lcores[i]->srcs = srcs + offset;
> +			lcores[i]->dsts = dsts + offset;
> +			if (cfg->is_sg) {
> +				lcores[i]->src_ptrs = cfg->src_ptrs;
> +				lcores[i]->dst_ptrs = cfg->dst_ptrs;
> +				lcores[i]->src_sges = src_sges + (nr_sgsrc / nb_workers * i);
> +				lcores[i]->dst_sges = dst_sges + (nr_sgdst / nb_workers * i);
> +			}
>  		}
>  
>  		if (cfg->is_dma) {
> @@ -759,6 +793,8 @@ mem_copy_benchmark(struct test_configure *cfg)
>  		calc_result(buf_size, nr_buf, nb_workers, test_secs,
>  			lcores[i]->worker_info.test_cpl,
>  			&memory, &avg_cycles, &bandwidth, &mops);
> +		if (cfg->is_bidir)
> +			printf("%s direction\n", i % 2 ? "MEM-to-DEV" : "DEV-to-MEM");
>  		output_result(cfg, lcores[i], kick_batch, avg_cycles, buf_size,
>  			nr_buf / nb_workers, memory, bandwidth, mops);
>  		mops_total += mops;
> @@ -772,7 +808,7 @@ mem_copy_benchmark(struct test_configure *cfg)
>  
>  out:
>  
> -	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM)
> +	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM || cfg->is_bidir)
>  		m = srcs;
>  	else if (cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV)
>  		m = dsts;
> diff --git a/app/test-dma-perf/config.ini b/app/test-dma-perf/config.ini
> index 28f6c9d1db..7814ddcdc3 100644
> --- a/app/test-dma-perf/config.ini
> +++ b/app/test-dma-perf/config.ini
> @@ -61,6 +61,10 @@
>  ;    "pfid" denotes PF-id to be used for data transfer
>  ;    "vfid" denotes VF-id of PF-id to be used for data transfer.
>  
> +; "xfer_mode" denotes mode of data transfer. It can take 2 values:
> +;    0 - unidirection transfer based on direction configured (default).
> +;    1 - Bi-directional transfer based on direction configured (mem-to-dev and dev-to-mem).
> +
>  ; =========== End of "mem2dev" and "dev2mem" config parameters. ==============
>  
>  [case1]
> @@ -95,6 +99,7 @@ eal_args=--in-memory --file-prefix=test
>  skip=1
>  type=DMA_MEM_COPY
>  direction=dev2mem
> +xfer_mode=0
>  vchan_dev=raddr=0x200000000,coreid=1,pfid=2,vfid=3
>  mem_size=10
>  buf_size=64,4096,2,MUL
> diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c
> index a27e4c9429..4488890697 100644
> --- a/app/test-dma-perf/main.c
> +++ b/app/test-dma-perf/main.c
> @@ -368,6 +368,7 @@ load_configs(const char *path)
>  	const char *skip;
>  	struct rte_kvargs *kvlist;
>  	const char *vchan_dev;
> +	const char *xfer_mode;
>  	int args_nr, nb_vp;
>  	bool is_dma;
>  
> @@ -421,6 +422,21 @@ load_configs(const char *path)
>  					test_case->transfer_dir = RTE_DMA_DIR_MEM_TO_MEM;
>  				}
>  			}
> +
> +			xfer_mode = rte_cfgfile_get_entry(cfgfile, section_name, "xfer_mode");
> +			if (xfer_mode) {
> +				int xmode = atoi(xfer_mode);
> +				if (xmode == 1) {
> +					if (test_case->transfer_dir == RTE_DMA_DIR_MEM_TO_MEM) {
> +						printf("Error: Invalid configuration. For mem to"
> +						       " mem dma transfer bi-directional cannot be"
> +						       " configured.\n");
> +						test_case->is_valid = false;
> +						continue;
> +					}
> +					test_case->is_bidir = true;
> +				}
> +			}
>  			is_dma = true;
>  		} else if (strcmp(case_type, CPU_MEM_COPY) == 0) {
>  			test_case->test_type = TEST_TYPE_CPU_MEM_COPY;
> @@ -433,7 +449,7 @@ load_configs(const char *path)
>  		}
>  
>  		if (test_case->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV ||
> -			test_case->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) {
> +		    test_case->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) {
>  			vchan_dev = rte_cfgfile_get_entry(cfgfile, section_name, "vchan_dev");
>  			if (vchan_dev == NULL) {
>  				printf("Transfer direction mem2dev and dev2mem"
> diff --git a/app/test-dma-perf/main.h b/app/test-dma-perf/main.h
> index baf149b72b..70f6c393c2 100644
> --- a/app/test-dma-perf/main.h
> +++ b/app/test-dma-perf/main.h
> @@ -67,6 +67,7 @@ struct test_configure {
>  	const char *eal_args;
>  	uint8_t scenario_id;
>  	struct test_vchan_dev_config vchan_dev;
> +	bool is_bidir;
>  };
>  
>  int mem_copy_benchmark(struct test_configure *cfg);
>
  
Amit Prakash Shukla Feb. 28, 2024, 9:38 a.m. UTC | #2
Hi Chengwen,

Please see my reply in-line.

Thanks
Amit Shukla

> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Wednesday, February 28, 2024 12:34 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
> <gmuthukrishn@marvell.com>
> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>
> Subject: [EXT] Re: [PATCH v2] app/dma-perf: support bi-directional transfer
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Amit and Gowrishankar,
> 
> It's nature to support multiple dmadev test in one testcase, and the original
> framework supports it.
> But it seem we both complicated it when adding support for non-mem2mem
> dma test.
> 
> The new added "direction" and "vchan_dev" could treat as the dmadev's
> private configure, some thing like:
> 
> lcore_dma=lcore10@0000:00:04.2,vchan=0,dir=mem2dev,devtype=pcie,radd
> r=xxx,coreid=1,pfid=2,vfid=3
> 
> then this bi-directional could impl only with config:
> 
> lcore_dma=lcore10@0000:00:04.2,dir=mem2dev,devtype=pcie,raddr=xxx,cor
> eid=1,pfid=2,vfid=3,
> lcore11@0000:00:04.3,dir=dev2mem,devtype=pcie,raddr=xxx,coreid=1,pfid=
> 2,vfid=3
> so that the lcore10 will do mem2dev with device 0000:00:04.2, while lcore11
> will do dev2mem with device 0000:00:04.3.

Thanks for the suggestion. I will make the suggested changes and send the next version.
  
Amit Prakash Shukla Feb. 29, 2024, 2:03 p.m. UTC | #3
Hi Chengwen,

I liked your suggestion and tried making changes, but encountered parsing issue for CFG files with line greater than CFG_VALUE_LEN=256(current value set).

There is a discussion on the similar lines in another patch set: https://patchwork.dpdk.org/project/dpdk/patch/20231206112952.1588-1-vipin.varghese@amd.com/.

I believe this patch can be taken as-is and we can come up with the solution when we can increase the CFG_VALUE_LEN as changing CFG_VALUE_LEN in this release is causing ABI breakage.

Thanks,
Amit Shukla

> -----Original Message-----
> From: Amit Prakash Shukla
> Sent: Wednesday, February 28, 2024 3:08 PM
> To: fengchengwen <fengchengwen@huawei.com>; Cheng Jiang
> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
> <gmuthukrishn@marvell.com>
> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>
> Subject: RE: [EXT] Re: [PATCH v2] app/dma-perf: support bi-directional
> transfer
> 
> Hi Chengwen,
> 
> Please see my reply in-line.
> 
> Thanks
> Amit Shukla
> 
> > -----Original Message-----
> > From: fengchengwen <fengchengwen@huawei.com>
> > Sent: Wednesday, February 28, 2024 12:34 PM
> > To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
> > <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
> > <gmuthukrishn@marvell.com>
> > Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
> > <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
> > Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
> > <pbhagavatula@marvell.com>
> > Subject: [EXT] Re: [PATCH v2] app/dma-perf: support bi-directional
> > transfer
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > Hi Amit and Gowrishankar,
> >
> > It's nature to support multiple dmadev test in one testcase, and the
> > original framework supports it.
> > But it seem we both complicated it when adding support for non-
> mem2mem
> > dma test.
> >
> > The new added "direction" and "vchan_dev" could treat as the dmadev's
> > private configure, some thing like:
> >
> >
> lcore_dma=lcore10@0000:00:04.2,vchan=0,dir=mem2dev,devtype=pcie,radd
> > r=xxx,coreid=1,pfid=2,vfid=3
> >
> > then this bi-directional could impl only with config:
> >
> >
> lcore_dma=lcore10@0000:00:04.2,dir=mem2dev,devtype=pcie,raddr=xxx,cor
> > eid=1,pfid=2,vfid=3,
> >
> lcore11@0000:00:04.3,dir=dev2mem,devtype=pcie,raddr=xxx,coreid=1,pfid=
> > 2,vfid=3
> > so that the lcore10 will do mem2dev with device 0000:00:04.2, while
> > lcore11 will do dev2mem with device 0000:00:04.3.
> 
> Thanks for the suggestion. I will make the suggested changes and send the
> next version.
  
Chengwen Feng March 1, 2024, 1:46 a.m. UTC | #4
Hi Amit,

I think this commit will complicated the test, plus futer we may add more test (e.g. fill)

I agree Bruce's advise in the [1], let also support "lcore_dma0/1/2",

User could provide dma info by two way:
1) lcore_dma=, which seperate each dma with ", ", but a maximum of a certain number is allowed.
2) lcore_dma0/1/2/..., each dma device take one line

[1] https://patchwork.dpdk.org/project/dpdk/patch/20231206112952.1588-1-vipin.varghese@amd.com/

Thanks

On 2024/2/29 22:03, Amit Prakash Shukla wrote:
> Hi Chengwen,
> 
> I liked your suggestion and tried making changes, but encountered parsing issue for CFG files with line greater than CFG_VALUE_LEN=256(current value set).
> 
> There is a discussion on the similar lines in another patch set: https://patchwork.dpdk.org/project/dpdk/patch/20231206112952.1588-1-vipin.varghese@amd.com/.
> 
> I believe this patch can be taken as-is and we can come up with the solution when we can increase the CFG_VALUE_LEN as changing CFG_VALUE_LEN in this release is causing ABI breakage.
> 
> Thanks,
> Amit Shukla
> 
>> -----Original Message-----
>> From: Amit Prakash Shukla
>> Sent: Wednesday, February 28, 2024 3:08 PM
>> To: fengchengwen <fengchengwen@huawei.com>; Cheng Jiang
>> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
>> <gmuthukrishn@marvell.com>
>> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
>> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
>> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
>> <pbhagavatula@marvell.com>
>> Subject: RE: [EXT] Re: [PATCH v2] app/dma-perf: support bi-directional
>> transfer
>>
>> Hi Chengwen,
>>
>> Please see my reply in-line.
>>
>> Thanks
>> Amit Shukla
>>
>>> -----Original Message-----
>>> From: fengchengwen <fengchengwen@huawei.com>
>>> Sent: Wednesday, February 28, 2024 12:34 PM
>>> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
>>> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
>>> <gmuthukrishn@marvell.com>
>>> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
>>> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
>>> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
>>> <pbhagavatula@marvell.com>
>>> Subject: [EXT] Re: [PATCH v2] app/dma-perf: support bi-directional
>>> transfer
>>>
>>> External Email
>>>
>>> ----------------------------------------------------------------------
>>> Hi Amit and Gowrishankar,
>>>
>>> It's nature to support multiple dmadev test in one testcase, and the
>>> original framework supports it.
>>> But it seem we both complicated it when adding support for non-
>> mem2mem
>>> dma test.
>>>
>>> The new added "direction" and "vchan_dev" could treat as the dmadev's
>>> private configure, some thing like:
>>>
>>>
>> lcore_dma=lcore10@0000:00:04.2,vchan=0,dir=mem2dev,devtype=pcie,radd
>>> r=xxx,coreid=1,pfid=2,vfid=3
>>>
>>> then this bi-directional could impl only with config:
>>>
>>>
>> lcore_dma=lcore10@0000:00:04.2,dir=mem2dev,devtype=pcie,raddr=xxx,cor
>>> eid=1,pfid=2,vfid=3,
>>>
>> lcore11@0000:00:04.3,dir=dev2mem,devtype=pcie,raddr=xxx,coreid=1,pfid=
>>> 2,vfid=3
>>> so that the lcore10 will do mem2dev with device 0000:00:04.2, while
>>> lcore11 will do dev2mem with device 0000:00:04.3.
>>
>> Thanks for the suggestion. I will make the suggested changes and send the
>> next version.
  
Amit Prakash Shukla March 1, 2024, 8:31 a.m. UTC | #5
Hi Chengwen,

If I'm not wrong, your concern was about config file additions and not about the test as such. If the config file is getting complicated and there are better alternatives, we can minimize the config file changes with this patch and just provide minimum functionality as required and leave it open for future changes. For now, I can document the existing behavior in documentation as "Constraints". Similar approach is followed in other application such as ipsec-secgw https://doc.dpdk.org/guides/sample_app_ug/ipsec_secgw.html#constraints

Constraints:
1. vchan_dev config will be same for all the configured DMA devices.
2. Alternate DMA device will do dev2mem and mem2dev implicitly.
Example:
xfer_mode=1
vchan_dev=raddr=0x200000000,coreid=1,pfid=2,vfid=3
lcore_dma=lcore10@0000:00:04.2, lcore11@0000:00:04.3, lcore12@0000:00:04.4, lcore13@0000:00:04.5

lcore10@0000:00:04.2, lcore12@0000:00:04.4 will do dev2mem and lcore11@0000:00:04.3, lcore13@0000:00:04.5 will do mem2dev.

Thanks,
Amit Shukla

> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Friday, March 1, 2024 7:16 AM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
> <gmuthukrishn@marvell.com>
> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>
> Subject: [EXTERNAL] Re: [EXT] Re: [PATCH v2] app/dma-perf: support bi-
> directional transfer
> 
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
> 
> ----------------------------------------------------------------------
> Hi Amit,
> 
> I think this commit will complicated the test, plus futer we may add more test
> (e.g. fill)
> 
> I agree Bruce's advise in the [1], let also support "lcore_dma0/1/2",
> 
> User could provide dma info by two way:
> 1) lcore_dma=, which seperate each dma with ", ", but a maximum of a certain
> number is allowed.
> 2) lcore_dma0/1/2/..., each dma device take one line
> 
> [1] https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
> 2D1-2Dvipin.varghese-
> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e=
> 
> Thanks
> 
> On 2024/2/29 22:03, Amit Prakash Shukla wrote:
> > Hi Chengwen,
> >
> > I liked your suggestion and tried making changes, but encountered parsing
> issue for CFG files with line greater than CFG_VALUE_LEN=256(current value
> set).
> >
> > There is a discussion on the similar lines in another patch set:
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
> 2D1-2Dvipin.varghese-
> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e= .
> >
> > I believe this patch can be taken as-is and we can come up with the solution
> when we can increase the CFG_VALUE_LEN as changing CFG_VALUE_LEN in
> this release is causing ABI breakage.
> >
> > Thanks,
> > Amit Shukla
> >
> >> -----Original Message-----
> >> From: Amit Prakash Shukla
> >> Sent: Wednesday, February 28, 2024 3:08 PM
> >> To: fengchengwen <fengchengwen@huawei.com>; Cheng Jiang
> >> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
> >> <gmuthukrishn@marvell.com>
> >> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
> >> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
> >> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
> >> <pbhagavatula@marvell.com>
> >> Subject: RE: [EXT] Re: [PATCH v2] app/dma-perf: support
> >> bi-directional transfer
> >>
> >> Hi Chengwen,
> >>
> >> Please see my reply in-line.
> >>
> >> Thanks
> >> Amit Shukla
> >>
> >>> -----Original Message-----
> >>> From: fengchengwen <fengchengwen@huawei.com>
> >>> Sent: Wednesday, February 28, 2024 12:34 PM
> >>> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
> >>> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
> >>> <gmuthukrishn@marvell.com>
> >>> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
> >>> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
> >>> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
> >>> <pbhagavatula@marvell.com>
> >>> Subject: [EXT] Re: [PATCH v2] app/dma-perf: support bi-directional
> >>> transfer
> >>>
> >>> External Email
> >>>
> >>> --------------------------------------------------------------------
> >>> --
> >>> Hi Amit and Gowrishankar,
> >>>
> >>> It's nature to support multiple dmadev test in one testcase, and the
> >>> original framework supports it.
> >>> But it seem we both complicated it when adding support for non-
> >> mem2mem
> >>> dma test.
> >>>
> >>> The new added "direction" and "vchan_dev" could treat as the
> >>> dmadev's private configure, some thing like:
> >>>
> >>>
> >>
> lcore_dma=lcore10@0000:00:04.2,vchan=0,dir=mem2dev,devtype=pcie,radd
> >>> r=xxx,coreid=1,pfid=2,vfid=3
> >>>
> >>> then this bi-directional could impl only with config:
> >>>
> >>>
> >>
> lcore_dma=lcore10@0000:00:04.2,dir=mem2dev,devtype=pcie,raddr=xxx,cor
> >>> eid=1,pfid=2,vfid=3,
> >>>
> >>
> lcore11@0000:00:04.3,dir=dev2mem,devtype=pcie,raddr=xxx,coreid=1,pfid
> >> =
> >>> 2,vfid=3
> >>> so that the lcore10 will do mem2dev with device 0000:00:04.2, while
> >>> lcore11 will do dev2mem with device 0000:00:04.3.
> >>
> >> Thanks for the suggestion. I will make the suggested changes and send
> >> the next version.
  
Chengwen Feng March 1, 2024, 9:30 a.m. UTC | #6
Hi Amit,

On 2024/3/1 16:31, Amit Prakash Shukla wrote:
> Hi Chengwen,
> 
> If I'm not wrong, your concern was about config file additions and not about the test as such. If the config file is getting complicated and there are better alternatives, we can minimize the config file changes with this patch and just provide minimum functionality as required and leave it open for future changes. For now, I can document the existing behavior in documentation as "Constraints". Similar approach is followed in other application such as ipsec-secgw https://doc.dpdk.org/guides/sample_app_ug/ipsec_secgw.html#constraints

Yes, I prefer enable different test just by modify configuration file, and then limit the number of entries at the same time.

This commit is bi-direction transfer, it is fixed, maybe later we should test 3/4 for mem2dev while 1/4 for dev2mem.

sometime we may need evaluate performance of one dma channel for mem2mem, while another channel for mem2dev, we can't do
this in current implement (because vchan_dev is for all DMA channel).

So I prefer restrict DMA non-mem2mem's config (include dir/type/coreid/pfid/vfid/raddr) as the dma device's private configuration.

Thanks

> 
> Constraints:
> 1. vchan_dev config will be same for all the configured DMA devices.
> 2. Alternate DMA device will do dev2mem and mem2dev implicitly.
> Example:
> xfer_mode=1
> vchan_dev=raddr=0x200000000,coreid=1,pfid=2,vfid=3
> lcore_dma=lcore10@0000:00:04.2, lcore11@0000:00:04.3, lcore12@0000:00:04.4, lcore13@0000:00:04.5
> 
> lcore10@0000:00:04.2, lcore12@0000:00:04.4 will do dev2mem and lcore11@0000:00:04.3, lcore13@0000:00:04.5 will do mem2dev.
> 
> Thanks,
> Amit Shukla
> 
>> -----Original Message-----
>> From: fengchengwen <fengchengwen@huawei.com>
>> Sent: Friday, March 1, 2024 7:16 AM
>> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
>> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
>> <gmuthukrishn@marvell.com>
>> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
>> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
>> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
>> <pbhagavatula@marvell.com>
>> Subject: [EXTERNAL] Re: [EXT] Re: [PATCH v2] app/dma-perf: support bi-
>> directional transfer
>>
>> Prioritize security for external emails: Confirm sender and content safety
>> before clicking links or opening attachments
>>
>> ----------------------------------------------------------------------
>> Hi Amit,
>>
>> I think this commit will complicated the test, plus futer we may add more test
>> (e.g. fill)
>>
>> I agree Bruce's advise in the [1], let also support "lcore_dma0/1/2",
>>
>> User could provide dma info by two way:
>> 1) lcore_dma=, which seperate each dma with ", ", but a maximum of a certain
>> number is allowed.
>> 2) lcore_dma0/1/2/..., each dma device take one line
>>
>> [1] https://urldefense.proofpoint.com/v2/url?u=https-
>> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
>> 2D1-2Dvipin.varghese-
>> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
>> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
>> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
>> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e=
>>
>> Thanks
>>
>> On 2024/2/29 22:03, Amit Prakash Shukla wrote:
>>> Hi Chengwen,
>>>
>>> I liked your suggestion and tried making changes, but encountered parsing
>> issue for CFG files with line greater than CFG_VALUE_LEN=256(current value
>> set).
>>>
>>> There is a discussion on the similar lines in another patch set:
>> https://urldefense.proofpoint.com/v2/url?u=https-
>> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
>> 2D1-2Dvipin.varghese-
>> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
>> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
>> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
>> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e= .
>>>
>>> I believe this patch can be taken as-is and we can come up with the solution
>> when we can increase the CFG_VALUE_LEN as changing CFG_VALUE_LEN in
>> this release is causing ABI breakage.
>>>
>>> Thanks,
>>> Amit Shukla
>>>
>>>> -----Original Message-----
>>>> From: Amit Prakash Shukla
>>>> Sent: Wednesday, February 28, 2024 3:08 PM
>>>> To: fengchengwen <fengchengwen@huawei.com>; Cheng Jiang
>>>> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
>>>> <gmuthukrishn@marvell.com>
>>>> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
>>>> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
>>>> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
>>>> <pbhagavatula@marvell.com>
>>>> Subject: RE: [EXT] Re: [PATCH v2] app/dma-perf: support
>>>> bi-directional transfer
>>>>
>>>> Hi Chengwen,
>>>>
>>>> Please see my reply in-line.
>>>>
>>>> Thanks
>>>> Amit Shukla
>>>>
>>>>> -----Original Message-----
>>>>> From: fengchengwen <fengchengwen@huawei.com>
>>>>> Sent: Wednesday, February 28, 2024 12:34 PM
>>>>> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
>>>>> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
>>>>> <gmuthukrishn@marvell.com>
>>>>> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
>>>>> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
>>>>> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
>>>>> <pbhagavatula@marvell.com>
>>>>> Subject: [EXT] Re: [PATCH v2] app/dma-perf: support bi-directional
>>>>> transfer
>>>>>
>>>>> External Email
>>>>>
>>>>> --------------------------------------------------------------------
>>>>> --
>>>>> Hi Amit and Gowrishankar,
>>>>>
>>>>> It's nature to support multiple dmadev test in one testcase, and the
>>>>> original framework supports it.
>>>>> But it seem we both complicated it when adding support for non-
>>>> mem2mem
>>>>> dma test.
>>>>>
>>>>> The new added "direction" and "vchan_dev" could treat as the
>>>>> dmadev's private configure, some thing like:
>>>>>
>>>>>
>>>>
>> lcore_dma=lcore10@0000:00:04.2,vchan=0,dir=mem2dev,devtype=pcie,radd
>>>>> r=xxx,coreid=1,pfid=2,vfid=3
>>>>>
>>>>> then this bi-directional could impl only with config:
>>>>>
>>>>>
>>>>
>> lcore_dma=lcore10@0000:00:04.2,dir=mem2dev,devtype=pcie,raddr=xxx,cor
>>>>> eid=1,pfid=2,vfid=3,
>>>>>
>>>>
>> lcore11@0000:00:04.3,dir=dev2mem,devtype=pcie,raddr=xxx,coreid=1,pfid
>>>> =
>>>>> 2,vfid=3
>>>>> so that the lcore10 will do mem2dev with device 0000:00:04.2, while
>>>>> lcore11 will do dev2mem with device 0000:00:04.3.
>>>>
>>>> Thanks for the suggestion. I will make the suggested changes and send
>>>> the next version.
  
Amit Prakash Shukla March 1, 2024, 10:59 a.m. UTC | #7
Hi Chengwen,

Please find my reply in-line.

Thanks,
Amit Shukla

> Hi Amit,
> 
> On 2024/3/1 16:31, Amit Prakash Shukla wrote:
> > Hi Chengwen,
> >
> > If I'm not wrong, your concern was about config file additions and not
> > about the test as such. If the config file is getting complicated and
> > there are better alternatives, we can minimize the config file changes
> > with this patch and just provide minimum functionality as required and
> > leave it open for future changes. For now, I can document the existing
> > behavior in documentation as "Constraints". Similar approach is
> > followed in other application such as ipsec-secgw
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__doc.dpdk.org_guid
> > es_sample-5Fapp-5Fug_ipsec-5Fsecgw.html-
> 23constraints&d=DwICaQ&c=nKjWe
> >
> c2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR69VnJLdSnADun7zLaXG1p5Rs7pXihE
> &m=UXlZ
> >
> 1CWj8uotMMmYQ4e7wtBXj4geBwcMUirlqFw0pZzSlOIIAVjWaPgcaXtni370&
> s=haaehrX
> > QSEG6EFRW8w2sHKUTU75aJX7ML8vM-0mJsAI&e=
> 
> Yes, I prefer enable different test just by modify configuration file, and then
> limit the number of entries at the same time.
> 
> This commit is bi-direction transfer, it is fixed, maybe later we should test 3/4
> for mem2dev while 1/4 for dev2mem.

Agreed. We will add this later after the base functionality is merged. I will send next version with constraints listed. Can I assume next version is good for merge?

> 
> sometime we may need evaluate performance of one dma channel for
> mem2mem, while another channel for mem2dev, we can't do this in current
> implement (because vchan_dev is for all DMA channel).

We are okay with extending it later. As you said, we are still deciding how the configuration file should look like.

> 
> So I prefer restrict DMA non-mem2mem's config (include
> dir/type/coreid/pfid/vfid/raddr) as the dma device's private configuration.
> 
> Thanks
> 
> >
> > Constraints:
> > 1. vchan_dev config will be same for all the configured DMA devices.
> > 2. Alternate DMA device will do dev2mem and mem2dev implicitly.
> > Example:
> > xfer_mode=1
> > vchan_dev=raddr=0x200000000,coreid=1,pfid=2,vfid=3
> > lcore_dma=lcore10@0000:00:04.2, lcore11@0000:00:04.3,
> > lcore12@0000:00:04.4, lcore13@0000:00:04.5
> >
> > lcore10@0000:00:04.2, lcore12@0000:00:04.4 will do dev2mem and
> lcore11@0000:00:04.3, lcore13@0000:00:04.5 will do mem2dev.
> >
> > Thanks,
> > Amit Shukla
> >
> >> -----Original Message-----
> >> From: fengchengwen <fengchengwen@huawei.com>
> >> Sent: Friday, March 1, 2024 7:16 AM
> >> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
> >> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
> >> <gmuthukrishn@marvell.com>
> >> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
> >> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
> >> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
> >> <pbhagavatula@marvell.com>
> >> Subject: [EXTERNAL] Re: [EXT] Re: [PATCH v2] app/dma-perf: support
> >> bi- directional transfer
> >>
> >> Prioritize security for external emails: Confirm sender and content
> >> safety before clicking links or opening attachments
> >>
> >> ---------------------------------------------------------------------
> >> -
> >> Hi Amit,
> >>
> >> I think this commit will complicated the test, plus futer we may add
> >> more test (e.g. fill)
> >>
> >> I agree Bruce's advise in the [1], let also support "lcore_dma0/1/2",
> >>
> >> User could provide dma info by two way:
> >> 1) lcore_dma=, which seperate each dma with ", ", but a maximum of a
> >> certain number is allowed.
> >> 2) lcore_dma0/1/2/..., each dma device take one line
> >>
> >> [1] https://urldefense.proofpoint.com/v2/url?u=https-
> >> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
> >> 2D1-2Dvipin.varghese-
> >>
> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
> >> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
> >>
> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
> >> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e=
> >>
> >> Thanks
> >>
> >> On 2024/2/29 22:03, Amit Prakash Shukla wrote:
> >>> Hi Chengwen,
> >>>
> >>> I liked your suggestion and tried making changes, but encountered
> >>> parsing
> >> issue for CFG files with line greater than CFG_VALUE_LEN=256(current
> >> value set).
> >>>
> >>> There is a discussion on the similar lines in another patch set:
> >> https://urldefense.proofpoint.com/v2/url?u=https-
> >> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
> >> 2D1-2Dvipin.varghese-
> >>
> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
> >> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
> >>
> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
> >> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e= .
> >>>
> >>> I believe this patch can be taken as-is and we can come up with the
> >>> solution
> >> when we can increase the CFG_VALUE_LEN as changing CFG_VALUE_LEN in
> >> this release is causing ABI breakage.
> >>>
> >>> Thanks,
> >>> Amit Shukla
> >>>

<snip>
  
Chengwen Feng March 7, 2024, 1:41 p.m. UTC | #8
Hi Amit,

On 2024/3/1 18:59, Amit Prakash Shukla wrote:
> Hi Chengwen,
> 
> Please find my reply in-line.
> 
> Thanks,
> Amit Shukla
> 
>> Hi Amit,
>>
>> On 2024/3/1 16:31, Amit Prakash Shukla wrote:
>>> Hi Chengwen,
>>>
>>> If I'm not wrong, your concern was about config file additions and not
>>> about the test as such. If the config file is getting complicated and
>>> there are better alternatives, we can minimize the config file changes
>>> with this patch and just provide minimum functionality as required and
>>> leave it open for future changes. For now, I can document the existing
>>> behavior in documentation as "Constraints". Similar approach is
>>> followed in other application such as ipsec-secgw
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__doc.dpdk.org_guid
>>> es_sample-5Fapp-5Fug_ipsec-5Fsecgw.html-
>> 23constraints&d=DwICaQ&c=nKjWe
>>>
>> c2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR69VnJLdSnADun7zLaXG1p5Rs7pXihE
>> &m=UXlZ
>>>
>> 1CWj8uotMMmYQ4e7wtBXj4geBwcMUirlqFw0pZzSlOIIAVjWaPgcaXtni370&
>> s=haaehrX
>>> QSEG6EFRW8w2sHKUTU75aJX7ML8vM-0mJsAI&e=
>>
>> Yes, I prefer enable different test just by modify configuration file, and then
>> limit the number of entries at the same time.
>>
>> This commit is bi-direction transfer, it is fixed, maybe later we should test 3/4
>> for mem2dev while 1/4 for dev2mem.
> 
> Agreed. We will add this later after the base functionality is merged. I will send next version with constraints listed. Can I assume next version is good for merge?

I suggest do it all at once in [1].

[1] https://patches.dpdk.org/project/dpdk/cover/cover.1709210551.git.gmuthukrishn@marvell.com/

Thanks

> 
>>
>> sometime we may need evaluate performance of one dma channel for
>> mem2mem, while another channel for mem2dev, we can't do this in current
>> implement (because vchan_dev is for all DMA channel).
> 
> We are okay with extending it later. As you said, we are still deciding how the configuration file should look like.
> 
>>
>> So I prefer restrict DMA non-mem2mem's config (include
>> dir/type/coreid/pfid/vfid/raddr) as the dma device's private configuration.
>>
>> Thanks
>>
>>>
>>> Constraints:
>>> 1. vchan_dev config will be same for all the configured DMA devices.
>>> 2. Alternate DMA device will do dev2mem and mem2dev implicitly.
>>> Example:
>>> xfer_mode=1
>>> vchan_dev=raddr=0x200000000,coreid=1,pfid=2,vfid=3
>>> lcore_dma=lcore10@0000:00:04.2, lcore11@0000:00:04.3,
>>> lcore12@0000:00:04.4, lcore13@0000:00:04.5
>>>
>>> lcore10@0000:00:04.2, lcore12@0000:00:04.4 will do dev2mem and
>> lcore11@0000:00:04.3, lcore13@0000:00:04.5 will do mem2dev.
>>>
>>> Thanks,
>>> Amit Shukla
>>>
>>>> -----Original Message-----
>>>> From: fengchengwen <fengchengwen@huawei.com>
>>>> Sent: Friday, March 1, 2024 7:16 AM
>>>> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
>>>> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
>>>> <gmuthukrishn@marvell.com>
>>>> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
>>>> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
>>>> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
>>>> <pbhagavatula@marvell.com>
>>>> Subject: [EXTERNAL] Re: [EXT] Re: [PATCH v2] app/dma-perf: support
>>>> bi- directional transfer
>>>>
>>>> Prioritize security for external emails: Confirm sender and content
>>>> safety before clicking links or opening attachments
>>>>
>>>> ---------------------------------------------------------------------
>>>> -
>>>> Hi Amit,
>>>>
>>>> I think this commit will complicated the test, plus futer we may add
>>>> more test (e.g. fill)
>>>>
>>>> I agree Bruce's advise in the [1], let also support "lcore_dma0/1/2",
>>>>
>>>> User could provide dma info by two way:
>>>> 1) lcore_dma=, which seperate each dma with ", ", but a maximum of a
>>>> certain number is allowed.
>>>> 2) lcore_dma0/1/2/..., each dma device take one line
>>>>
>>>> [1] https://urldefense.proofpoint.com/v2/url?u=https-
>>>> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
>>>> 2D1-2Dvipin.varghese-
>>>>
>> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
>>>> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
>>>>
>> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
>>>> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e=
>>>>
>>>> Thanks
>>>>
>>>> On 2024/2/29 22:03, Amit Prakash Shukla wrote:
>>>>> Hi Chengwen,
>>>>>
>>>>> I liked your suggestion and tried making changes, but encountered
>>>>> parsing
>>>> issue for CFG files with line greater than CFG_VALUE_LEN=256(current
>>>> value set).
>>>>>
>>>>> There is a discussion on the similar lines in another patch set:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-
>>>> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
>>>> 2D1-2Dvipin.varghese-
>>>>
>> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
>>>> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
>>>>
>> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
>>>> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e= .
>>>>>
>>>>> I believe this patch can be taken as-is and we can come up with the
>>>>> solution
>>>> when we can increase the CFG_VALUE_LEN as changing CFG_VALUE_LEN in
>>>> this release is causing ABI breakage.
>>>>>
>>>>> Thanks,
>>>>> Amit Shukla
>>>>>
> 
> <snip>
>
  

Patch

diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
index 25ed6fa6d0..8a23944763 100644
--- a/app/test-dma-perf/benchmark.c
+++ b/app/test-dma-perf/benchmark.c
@@ -144,12 +144,19 @@  cache_flush_buf(__rte_unused struct rte_mbuf **array,
 
 static int
 vchan_data_populate(uint32_t dev_id, struct rte_dma_vchan_conf *qconf,
-		    struct test_configure *cfg)
+		    struct test_configure *cfg, uint16_t dev_num)
 {
 	struct rte_dma_info info;
 
 	qconf->direction = cfg->transfer_dir;
 
+	/* If its a bi-directional test, configure odd device for inbound dma
+	 * transfer and even device for outbound dma transfer.
+	 */
+	if (cfg->is_bidir)
+		qconf->direction = (dev_num % 2) ? RTE_DMA_DIR_MEM_TO_DEV :
+				   RTE_DMA_DIR_DEV_TO_MEM;
+
 	rte_dma_info_get(dev_id, &info);
 	if (!(RTE_BIT64(qconf->direction) & info.dev_capa))
 		return -1;
@@ -181,14 +188,15 @@  vchan_data_populate(uint32_t dev_id, struct rte_dma_vchan_conf *qconf,
 
 /* Configuration of device. */
 static void
-configure_dmadev_queue(uint32_t dev_id, struct test_configure *cfg, uint8_t ptrs_max)
+configure_dmadev_queue(uint32_t dev_id, struct test_configure *cfg, uint8_t ptrs_max,
+		       uint16_t dev_num)
 {
 	uint16_t vchan = 0;
 	struct rte_dma_info info;
 	struct rte_dma_conf dev_config = { .nb_vchans = 1 };
 	struct rte_dma_vchan_conf qconf = { 0 };
 
-	if (vchan_data_populate(dev_id, &qconf, cfg) != 0)
+	if (vchan_data_populate(dev_id, &qconf, cfg, dev_num) != 0)
 		rte_exit(EXIT_FAILURE, "Error with vchan data populate.\n");
 
 	if (rte_dma_configure(dev_id, &dev_config) != 0)
@@ -235,7 +243,7 @@  config_dmadevs(struct test_configure *cfg)
 		}
 
 		ldm->dma_ids[i] = dev_id;
-		configure_dmadev_queue(dev_id, cfg, ptrs_max);
+		configure_dmadev_queue(dev_id, cfg, ptrs_max, nb_dmadevs);
 		++nb_dmadevs;
 	}
 
@@ -504,7 +512,7 @@  setup_memory_env(struct test_configure *cfg,
 		}
 	}
 
-	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) {
+	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM && !cfg->is_bidir) {
 		ext_buf_info->free_cb = dummy_free_ext_buf;
 		ext_buf_info->fcb_opaque = NULL;
 		for (i = 0; i < nr_buf; i++) {
@@ -516,7 +524,7 @@  setup_memory_env(struct test_configure *cfg,
 		}
 	}
 
-	if (cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV) {
+	if (cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV && !cfg->is_bidir) {
 		ext_buf_info->free_cb = dummy_free_ext_buf;
 		ext_buf_info->fcb_opaque = NULL;
 		for (i = 0; i < nr_buf; i++) {
@@ -528,6 +536,18 @@  setup_memory_env(struct test_configure *cfg,
 		}
 	}
 
+	if (cfg->is_bidir) {
+		ext_buf_info->free_cb = dummy_free_ext_buf;
+		ext_buf_info->fcb_opaque = NULL;
+		for (i = 0; i < nr_buf; i++) {
+			/* Using mbuf structure to hold remote iova address. */
+			rte_pktmbuf_attach_extbuf((*srcs)[i], (void *)(cfg->vchan_dev.raddr +
+						 (i * buf_size)), (rte_iova_t)(cfg->vchan_dev.raddr +
+						 (i * buf_size)), 0, ext_buf_info);
+			rte_mbuf_ext_refcnt_update(ext_buf_info, 1);
+		}
+	}
+
 	if (cfg->is_sg) {
 		uint8_t src_ptrs = cfg->src_ptrs;
 		uint8_t dst_ptrs = cfg->dst_ptrs;
@@ -649,16 +669,30 @@  mem_copy_benchmark(struct test_configure *cfg)
 		lcores[i]->nr_buf = (uint32_t)(nr_buf / nb_workers);
 		lcores[i]->buf_size = buf_size;
 		lcores[i]->test_secs = test_secs;
-		lcores[i]->srcs = srcs + offset;
-		lcores[i]->dsts = dsts + offset;
 		lcores[i]->scenario_id = cfg->scenario_id;
 		lcores[i]->lcore_id = lcore_id;
 
-		if (cfg->is_sg) {
-			lcores[i]->src_ptrs = cfg->src_ptrs;
-			lcores[i]->dst_ptrs = cfg->dst_ptrs;
-			lcores[i]->src_sges = src_sges + (nr_sgsrc / nb_workers * i);
-			lcores[i]->dst_sges = dst_sges + (nr_sgdst / nb_workers * i);
+		/* Number of workers is equal to number of devices. In case of bi-directional
+		 * dma, use 1 device for mem-to-dev and 1 device for dev-to-mem.
+		 */
+		if (cfg->is_dma && cfg->is_bidir && (i % 2 != 0)) {
+			lcores[i]->dsts = srcs + offset;
+			lcores[i]->srcs = dsts + offset;
+			if (cfg->is_sg) {
+				lcores[i]->dst_ptrs = cfg->src_ptrs;
+				lcores[i]->src_ptrs = cfg->dst_ptrs;
+				lcores[i]->dst_sges = src_sges + (nr_sgsrc / nb_workers * i);
+				lcores[i]->src_sges = dst_sges + (nr_sgdst / nb_workers * i);
+			}
+		} else {
+			lcores[i]->srcs = srcs + offset;
+			lcores[i]->dsts = dsts + offset;
+			if (cfg->is_sg) {
+				lcores[i]->src_ptrs = cfg->src_ptrs;
+				lcores[i]->dst_ptrs = cfg->dst_ptrs;
+				lcores[i]->src_sges = src_sges + (nr_sgsrc / nb_workers * i);
+				lcores[i]->dst_sges = dst_sges + (nr_sgdst / nb_workers * i);
+			}
 		}
 
 		if (cfg->is_dma) {
@@ -759,6 +793,8 @@  mem_copy_benchmark(struct test_configure *cfg)
 		calc_result(buf_size, nr_buf, nb_workers, test_secs,
 			lcores[i]->worker_info.test_cpl,
 			&memory, &avg_cycles, &bandwidth, &mops);
+		if (cfg->is_bidir)
+			printf("%s direction\n", i % 2 ? "MEM-to-DEV" : "DEV-to-MEM");
 		output_result(cfg, lcores[i], kick_batch, avg_cycles, buf_size,
 			nr_buf / nb_workers, memory, bandwidth, mops);
 		mops_total += mops;
@@ -772,7 +808,7 @@  mem_copy_benchmark(struct test_configure *cfg)
 
 out:
 
-	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM)
+	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM || cfg->is_bidir)
 		m = srcs;
 	else if (cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV)
 		m = dsts;
diff --git a/app/test-dma-perf/config.ini b/app/test-dma-perf/config.ini
index 28f6c9d1db..7814ddcdc3 100644
--- a/app/test-dma-perf/config.ini
+++ b/app/test-dma-perf/config.ini
@@ -61,6 +61,10 @@ 
 ;    "pfid" denotes PF-id to be used for data transfer
 ;    "vfid" denotes VF-id of PF-id to be used for data transfer.
 
+; "xfer_mode" denotes mode of data transfer. It can take 2 values:
+;    0 - unidirection transfer based on direction configured (default).
+;    1 - Bi-directional transfer based on direction configured (mem-to-dev and dev-to-mem).
+
 ; =========== End of "mem2dev" and "dev2mem" config parameters. ==============
 
 [case1]
@@ -95,6 +99,7 @@  eal_args=--in-memory --file-prefix=test
 skip=1
 type=DMA_MEM_COPY
 direction=dev2mem
+xfer_mode=0
 vchan_dev=raddr=0x200000000,coreid=1,pfid=2,vfid=3
 mem_size=10
 buf_size=64,4096,2,MUL
diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c
index a27e4c9429..4488890697 100644
--- a/app/test-dma-perf/main.c
+++ b/app/test-dma-perf/main.c
@@ -368,6 +368,7 @@  load_configs(const char *path)
 	const char *skip;
 	struct rte_kvargs *kvlist;
 	const char *vchan_dev;
+	const char *xfer_mode;
 	int args_nr, nb_vp;
 	bool is_dma;
 
@@ -421,6 +422,21 @@  load_configs(const char *path)
 					test_case->transfer_dir = RTE_DMA_DIR_MEM_TO_MEM;
 				}
 			}
+
+			xfer_mode = rte_cfgfile_get_entry(cfgfile, section_name, "xfer_mode");
+			if (xfer_mode) {
+				int xmode = atoi(xfer_mode);
+				if (xmode == 1) {
+					if (test_case->transfer_dir == RTE_DMA_DIR_MEM_TO_MEM) {
+						printf("Error: Invalid configuration. For mem to"
+						       " mem dma transfer bi-directional cannot be"
+						       " configured.\n");
+						test_case->is_valid = false;
+						continue;
+					}
+					test_case->is_bidir = true;
+				}
+			}
 			is_dma = true;
 		} else if (strcmp(case_type, CPU_MEM_COPY) == 0) {
 			test_case->test_type = TEST_TYPE_CPU_MEM_COPY;
@@ -433,7 +449,7 @@  load_configs(const char *path)
 		}
 
 		if (test_case->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV ||
-			test_case->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) {
+		    test_case->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) {
 			vchan_dev = rte_cfgfile_get_entry(cfgfile, section_name, "vchan_dev");
 			if (vchan_dev == NULL) {
 				printf("Transfer direction mem2dev and dev2mem"
diff --git a/app/test-dma-perf/main.h b/app/test-dma-perf/main.h
index baf149b72b..70f6c393c2 100644
--- a/app/test-dma-perf/main.h
+++ b/app/test-dma-perf/main.h
@@ -67,6 +67,7 @@  struct test_configure {
 	const char *eal_args;
 	uint8_t scenario_id;
 	struct test_vchan_dev_config vchan_dev;
+	bool is_bidir;
 };
 
 int mem_copy_benchmark(struct test_configure *cfg);