[EXT] Re: [PATCH v8 2/4] app/dma-perf: add PCI device support

[Amit Prakash Shukla]

Hi Chengwen,

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

Thanks,
Amit Shukla

> -----Original Message-----
> From: fengchengwen <fengchengwen at huawei.com>
> Sent: Wednesday, February 21, 2024 8:56 AM
> To: Gowrishankar Muthukrishnan <gmuthukrishn at marvell.com>;
> dev at dpdk.org; Amit Prakash Shukla <amitprakashs at marvell.com>
> Cc: Anoob Joseph <anoobj at marvell.com>; Cheng Jiang
> <honest.jiang at foxmail.com>; Kevin Laatz <kevin.laatz at intel.com>; Bruce
> Richardson <bruce.richardson at intel.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula at marvell.com>; Thomas Monjalon <thomas at monjalon.net>
> Subject: [EXT] Re: [PATCH v8 2/4] app/dma-perf: add PCI device support
> 
> External Email
> 
> ----------------------------------------------------------------------

<snip>

> >  	if (srcs != NULL)
> >  		rte_pktmbuf_free_bulk(srcs, nr_buf); diff --git
> > a/app/test-dma-perf/config.ini b/app/test-dma-perf/config.ini index
> > 4d59234b2a..cddcf93c6e 100644
> > --- a/app/test-dma-perf/config.ini
> > +++ b/app/test-dma-perf/config.ini
> > @@ -38,6 +38,23 @@
> >
> >  ; "skip" To skip a test-case set skip to 1.
> >
> > +; Parameters to be configured for data transfers from "mem to dev" and
> "dev to mem":
> > +;
> >
> +==================================================================
> ===
> > +============= ; "direction" denotes the direction of data transfer.
> > +It can take 3 values:
> > +;    0 - mem to mem transfer
> > +;    1 - mem to dev transfer
> > +;    2 - dev to mem transfer
> 
> I prefer readable string not number, for examples:
> mem2mem
> mem2dev
> dev2mem
> 
> > +; If not specified the default value is 0 (mem to mem transfer).
> > +
> > +; "raddr" remote iova address for "mem to dev" and "dev to mem" transfer.
> > +
> > +; "scoreid" denotes source PCIe core index.
> > +; "dcoreid" denotes destination PCIe core index.
> > +; "pfid" denotes PF-id to be used for data transfer ; "vfid" denotes
> > +VF-id of PF-id to be used for data transfer.
> 
> too many entries, and it all about pcie, the 'struct rte_dma_port_param' future
> may support other bus.
> 
> Suggest the entry is vchan_dev, user could input some thing like 1.
> vchan_dev=bus=pcie,coreid=1,pfid=0,vfid=1,addr=xxx
>    add add descriptor, only valid when direction is one of mem2dev or
> dev2mem
> 
> It could use kvargs library to parse the value of entry vchan_dev
> 
> > +
> > +; =========== End of "mem to dev" and "dev to mem" config parameters.
> > +==============
> > +
> >  [case1]
> >  type=DMA_MEM_COPY
> >  mem_size=10
> > @@ -52,6 +69,26 @@ lcore_dma=lcore10 at 0000:00:04.2,
> > lcore11 at 0000:00:04.3  eal_args=--in-memory --file-prefix=test
> >

<snip>

> > +
> > +		if (test_case->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) {
> > +			scoreid = rte_cfgfile_get_entry(cfgfile, section_name,
> "scoreid");
> > +			if (scoreid == NULL) {
> > +				printf("Error: No scoreid configured for
> case%d.\n", i + 1);
> > +				test_case->is_valid = false;
> > +				continue;
> > +			}
> > +			test_case->scoreid = (uint8_t)atoi(scoreid);
> > +		}
> > +
> > +		if (test_case->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV) {
> > +			dcoreid = rte_cfgfile_get_entry(cfgfile, section_name,
> "dcoreid");
> > +			if (dcoreid == NULL) {
> > +				printf("Error: No dcoreid configured for
> case%d.\n", i + 1);
> > +				test_case->is_valid = false;
> > +				continue;
> > +			}
> > +			test_case->dcoreid = (uint8_t)atoi(dcoreid);
> > +		}
> > +
> 
> suggest add a subfunction to wrap parsing device's config.
> 
> 
> >  		test_case->src_numa_node =
> (int)atoi(rte_cfgfile_get_entry(cfgfile,
> >
> 	section_name, "src_numa_node"));
> >  		test_case->dst_numa_node =
> (int)atoi(rte_cfgfile_get_entry(cfgfile,
> > diff --git a/app/test-dma-perf/main.h b/app/test-dma-perf/main.h index
> > 32670151af..8ac3270fba 100644
> > --- a/app/test-dma-perf/main.h
> > +++ b/app/test-dma-perf/main.h
> > @@ -42,6 +42,7 @@ struct test_configure {
> >  	bool is_valid;
> >  	bool is_skip;
> >  	uint8_t test_type;
> > +	uint8_t transfer_dir;
> >  	const char *test_type_str;
> >  	uint16_t src_numa_node;
> >  	uint16_t dst_numa_node;
> > @@ -57,6 +58,11 @@ struct test_configure {
> >  	uint16_t test_secs;
> >  	const char *eal_args;
> >  	uint8_t scenario_id;
> > +	uint8_t scoreid;
> > +	uint8_t dcoreid;
> > +	uint8_t pfid;
> > +	uint16_t vfid;
> > +	uintptr_t raddr;
> 
> suggest create new struct:
> struct test_vchan_dev_config {
> 	struct rte_dma_port_param port;
> 	uintptr_t addr;
> };
> 
> So defined as:
>   struct test_vchan_dev_config vchan_dev;
> 
> Thanks
> 
> >  };
> >
> >  void mem_copy_benchmark(struct test_configure *cfg, bool is_dma);
> >