From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A1AE243B5A; Wed, 21 Feb 2024 04:26:07 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 86D70402B9; Wed, 21 Feb 2024 04:26:07 +0100 (CET) Received: from szxga06-in.huawei.com (szxga06-in.huawei.com [45.249.212.32]) by mails.dpdk.org (Postfix) with ESMTP id 3C5D94014F for ; Wed, 21 Feb 2024 04:26:05 +0100 (CET) Received: from mail.maildlp.com (unknown [172.19.163.44]) by szxga06-in.huawei.com (SkyGuard) with ESMTP id 4TfhVc4M9Nz1vtyZ; Wed, 21 Feb 2024 11:25:28 +0800 (CST) Received: from dggpeml500024.china.huawei.com (unknown [7.185.36.10]) by mail.maildlp.com (Postfix) with ESMTPS id A059E140154; Wed, 21 Feb 2024 11:26:02 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Wed, 21 Feb 2024 11:26:02 +0800 Subject: Re: [PATCH v8 2/4] app/dma-perf: add PCI device support To: Gowrishankar Muthukrishnan , , Amit Prakash Shukla CC: , Cheng Jiang , Kevin Laatz , Bruce Richardson , Pavan Nikhilesh , Thomas Monjalon References: <899120d1bf9b775fd2db1abf724950b1ee2509a7.1700650397.git.gmuthukrishn@marvell.com> From: fengchengwen Message-ID: <753efac9-04e4-43d7-f823-ab5a71bb723f@huawei.com> Date: Wed, 21 Feb 2024 11:26:02 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <899120d1bf9b775fd2db1abf724950b1ee2509a7.1700650397.git.gmuthukrishn@marvell.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggpeml500024.china.huawei.com (7.185.36.10) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Amit, There are many commit for dma-perf, I've done a re-review and have a few comments, pls see below: On 2023/11/22 19:06, Gowrishankar Muthukrishnan wrote: > From: Amit Prakash Shukla > > Add support to test performance for "device to memory" and > "memory to device" data transfer. > > Signed-off-by: Amit Prakash Shukla > Acked-by: Anoob Joseph > --- > app/test-dma-perf/benchmark.c | 108 +++++++++++++++++++++++++++++++--- > app/test-dma-perf/config.ini | 37 ++++++++++++ > app/test-dma-perf/main.c | 67 +++++++++++++++++++++ > app/test-dma-perf/main.h | 6 ++ > 4 files changed, 209 insertions(+), 9 deletions(-) > > diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c > index 9b1f58c78c..eaed224c67 100644 > --- a/app/test-dma-perf/benchmark.c > +++ b/app/test-dma-perf/benchmark.c > @@ -127,17 +127,54 @@ cache_flush_buf(__rte_unused struct rte_mbuf **array, > #endif > } > > +static int > +vchan_data_populate(uint32_t dev_id, struct rte_dma_vchan_conf *qconf, > + struct test_configure *cfg) > +{ > + struct rte_dma_info info; > + > + qconf->direction = cfg->transfer_dir; > + > + rte_dma_info_get(dev_id, &info); > + if (!(RTE_BIT64(qconf->direction) & info.dev_capa)) > + return -1; > + > + qconf->nb_desc = cfg->ring_size.cur; > + > + switch (qconf->direction) { > + case RTE_DMA_DIR_MEM_TO_DEV: > + qconf->dst_port.pcie.vfen = 1; > + qconf->dst_port.port_type = RTE_DMA_PORT_PCIE; > + qconf->dst_port.pcie.coreid = cfg->dcoreid; > + qconf->dst_port.pcie.vfid = cfg->vfid; > + qconf->dst_port.pcie.pfid = cfg->pfid; > + break; > + case RTE_DMA_DIR_DEV_TO_MEM: > + qconf->src_port.pcie.vfen = 1; > + qconf->src_port.port_type = RTE_DMA_PORT_PCIE; > + qconf->src_port.pcie.coreid = cfg->scoreid; > + qconf->src_port.pcie.vfid = cfg->vfid; > + qconf->src_port.pcie.pfid = cfg->pfid; > + break; > + case RTE_DMA_DIR_MEM_TO_MEM: > + case RTE_DMA_DIR_DEV_TO_DEV: > + break; > + } > + > + return 0; > +} > + > /* Configuration of device. */ > static void > -configure_dmadev_queue(uint32_t dev_id, uint32_t ring_size) > +configure_dmadev_queue(uint32_t dev_id, struct test_configure *cfg) > { > uint16_t vchan = 0; > struct rte_dma_info info; > struct rte_dma_conf dev_config = { .nb_vchans = 1 }; > - struct rte_dma_vchan_conf qconf = { > - .direction = RTE_DMA_DIR_MEM_TO_MEM, > - .nb_desc = ring_size > - }; > + struct rte_dma_vchan_conf qconf = { 0 }; > + > + if (vchan_data_populate(dev_id, &qconf, cfg) != 0) > + rte_exit(EXIT_FAILURE, "Error with vchan data populate.\n"); > > if (rte_dma_configure(dev_id, &dev_config) != 0) > rte_exit(EXIT_FAILURE, "Error with dma configure.\n"); > @@ -159,7 +196,6 @@ configure_dmadev_queue(uint32_t dev_id, uint32_t ring_size) > static int > config_dmadevs(struct test_configure *cfg) > { > - uint32_t ring_size = cfg->ring_size.cur; > struct lcore_dma_map_t *ldm = &cfg->lcore_dma_map; > uint32_t nb_workers = ldm->cnt; > uint32_t i; > @@ -176,7 +212,7 @@ config_dmadevs(struct test_configure *cfg) > } > > ldm->dma_ids[i] = dev_id; > - configure_dmadev_queue(dev_id, ring_size); > + configure_dmadev_queue(dev_id, cfg); > ++nb_dmadevs; > } > > @@ -302,13 +338,22 @@ do_cpu_mem_copy(void *p) > return 0; > } > > +static void > +dummy_free_ext_buf(void *addr, void *opaque) > +{ > + RTE_SET_USED(addr); > + RTE_SET_USED(opaque); > +} > + > static int > setup_memory_env(struct test_configure *cfg, struct rte_mbuf ***srcs, > struct rte_mbuf ***dsts) > { > + static struct rte_mbuf_ext_shared_info *ext_buf_info; > unsigned int buf_size = cfg->buf_size.cur; > unsigned int nr_sockets; > uint32_t nr_buf = cfg->nr_buf; > + uint32_t i; > > nr_sockets = rte_socket_count(); > if (cfg->src_numa_node >= nr_sockets || > @@ -361,16 +406,47 @@ setup_memory_env(struct test_configure *cfg, struct rte_mbuf ***srcs, > return -1; > } > > + if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM || > + cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV) { > + ext_buf_info = rte_malloc(NULL, sizeof(struct rte_mbuf_ext_shared_info), 0); > + if (ext_buf_info == NULL) { > + printf("Error: ext_buf_info malloc failed.\n"); > + return -1; > + } > + } > + > + if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) { > + 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->raddr, > + (rte_iova_t)cfg->raddr, 0, ext_buf_info); > + rte_mbuf_ext_refcnt_update(ext_buf_info, 1); > + } > + } > + > + if (cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV) { > + 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((*dsts)[i], (void *)cfg->raddr, > + (rte_iova_t)cfg->raddr, 0, ext_buf_info); > + rte_mbuf_ext_refcnt_update(ext_buf_info, 1); > + } > + } > + > return 0; > } > > void > mem_copy_benchmark(struct test_configure *cfg, bool is_dma) > { > - uint16_t i; > + uint32_t i; > uint32_t offset; > unsigned int lcore_id = 0; > - struct rte_mbuf **srcs = NULL, **dsts = NULL; > + struct rte_mbuf **srcs = NULL, **dsts = NULL, **m = NULL; > struct lcore_dma_map_t *ldm = &cfg->lcore_dma_map; > unsigned int buf_size = cfg->buf_size.cur; > uint16_t kick_batch = cfg->kick_batch.cur; > @@ -476,6 +552,20 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma) > avg_cycles_total / nb_workers, bandwidth_total, mops_total); > > out: > + > + if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) > + m = srcs; > + else if (cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV) > + m = dsts; > + > + if (m) { > + for (i = 0; i < nr_buf; i++) > + rte_pktmbuf_detach_extbuf(m[i]); > + > + if (m[0]->shinfo && rte_mbuf_ext_refcnt_read(m[0]->shinfo) == 0) > + rte_free(m[0]->shinfo); > + } > + > /* free mbufs used in the test */ > 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 > > [case2] > +skip=1 > +type=DMA_MEM_COPY > +direction=2 > +raddr=0x200000000 > +scoreid=0 > +dcoreid=0 > +pfid=0 > +vfid=0 > +mem_size=10 > +buf_size=64,4096,2,MUL > +dma_ring_size=1024 > +kick_batch=32 > +src_numa_node=0 > +dst_numa_node=0 > +cache_flush=0 > +test_seconds=2 > +lcore_dma=lcore10@0000:00:04.2, lcore11@0000:00:04.3 > +eal_args=--in-memory --file-prefix=test > + > +[case3] > type=CPU_MEM_COPY > mem_size=10 > buf_size=64,8192,2,MUL > diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c > index 33c3750bb1..3eddf2e40a 100644 > --- a/app/test-dma-perf/main.c > +++ b/app/test-dma-perf/main.c > @@ -16,6 +16,7 @@ > #include > #include > #include > +#include > > #include "main.h" > > @@ -331,9 +332,11 @@ load_configs(const char *path) > struct test_configure *test_case; > char section_name[CFG_NAME_LEN]; > const char *case_type; > + const char *transfer_dir; > const char *lcore_dma; > const char *mem_size_str, *buf_size_str, *ring_size_str, *kick_batch_str; > const char *skip; > + const char *raddr, *scoreid, *dcoreid, *vfid, *pfid; > int args_nr, nb_vp; > bool is_dma; > > @@ -371,6 +374,20 @@ load_configs(const char *path) > if (strcmp(case_type, DMA_MEM_COPY) == 0) { > test_case->test_type = TEST_TYPE_DMA_MEM_COPY; > test_case->test_type_str = DMA_MEM_COPY; > + > + transfer_dir = rte_cfgfile_get_entry(cfgfile, section_name, "direction"); > + if (transfer_dir == NULL) { > + printf("Transfer direction not configured." > + " Defaulting it to MEM to MEM transfer.\n"); > + test_case->transfer_dir = RTE_DMA_DIR_MEM_TO_MEM; > + } else > + test_case->transfer_dir = (uint8_t)atoi(transfer_dir); > + > + if (test_case->transfer_dir >= RTE_DMA_DIR_DEV_TO_DEV) { > + printf("Error: Invalid transfer direction configured.\n"); > + test_case->is_valid = false; > + continue; > + } > is_dma = true; > } else if (strcmp(case_type, CPU_MEM_COPY) == 0) { > test_case->test_type = TEST_TYPE_CPU_MEM_COPY; > @@ -382,6 +399,56 @@ load_configs(const char *path) > continue; > } > > + if (test_case->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV || > + test_case->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM) { > + char *endptr; > + > + raddr = rte_cfgfile_get_entry(cfgfile, section_name, "raddr"); > + if (raddr == NULL) { > + printf("Error: No raddr configured for case%d.\n", i + 1); > + test_case->is_valid = false; > + continue; > + } > + test_case->raddr = strtoull(raddr, &endptr, 16); > + > + vfid = rte_cfgfile_get_entry(cfgfile, section_name, "vfid"); > + if (vfid == NULL) { > + printf("Error: No vfid configured for case%d.\n", i + 1); > + test_case->is_valid = false; > + continue; > + } > + test_case->vfid = (uint16_t)atoi(vfid); > + > + pfid = rte_cfgfile_get_entry(cfgfile, section_name, "pfid"); > + if (pfid == NULL) { > + printf("Error: No pfid configured for case%d.\n", i + 1); > + test_case->is_valid = false; > + continue; > + } > + test_case->pfid = (uint8_t)atoi(pfid); > + > + } > + > + 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); >