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 <amitpraka...@marvell.com>
>
> Add support to test performance for "device to memory" and
> "memory to device" data transfer.
>
> Signed-off-by: Amit Prakash Shukla <amitpraka...@marvell.com>
> Acked-by: Anoob Joseph <ano...@marvell.com>
> ---
> 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 <rte_cfgfile.h>
> #include <rte_string_fns.h>
> #include <rte_lcore.h>
> +#include <rte_dmadev.h>
>
> #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);
>
