Hi Chengwen,
Thanks for the review and feedback. I will send the next version with suggested
changes.
Thanks,
Amit Shukla
> -----Original Message-----
> From: fengchengwen <fengcheng...@huawei.com>
> Sent: Wednesday, February 21, 2024 8:56 AM
> To: Gowrishankar Muthukrishnan <gmuthukri...@marvell.com>;
> dev@dpdk.org; Amit Prakash Shukla <amitpraka...@marvell.com>
> Cc: Anoob Joseph <ano...@marvell.com>; Cheng Jiang
> <honest.ji...@foxmail.com>; Kevin Laatz <kevin.la...@intel.com>; Bruce
> Richardson <bruce.richard...@intel.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavat...@marvell.com>; Thomas Monjalon <tho...@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@0000:00:04.2,
> > lcore11@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);
> >
