DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Jiang, Cheng1" <cheng1.jiang@intel.com>
To: huangdengdui <huangdengdui@huawei.com>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	"mb@smartsharesystems.com" <mb@smartsharesystems.com>,
	"Xia, Chenbo" <chenbo.xia@intel.com>,
	"amitprakashs@marvell.com" <amitprakashs@marvell.com>,
	"anoobj@marvell.com" <anoobj@marvell.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Hu, Jiayu" <jiayu.hu@intel.com>,
	"Ding, Xuan" <xuan.ding@intel.com>,
	"Ma, WenwuX" <wenwux.ma@intel.com>,
	"Wang, YuanX" <yuanx.wang@intel.com>,
	"He, Xingguang" <xingguang.he@intel.com>
Subject: RE: [PATCH v6] app/dma-perf: introduce dma-perf application
Date: Wed, 14 Jun 2023 06:40:08 +0000	[thread overview]
Message-ID: <SN7PR11MB7019085BCA59D5FFBA629FA3DC5AA@SN7PR11MB7019.namprd11.prod.outlook.com> (raw)
In-Reply-To: <8b8949c1-739a-59f9-fd7d-a0e232c30252@huawei.com>

Hi,

Thanks for your comments, replies are inline.

Thanks a again,
Cheng

> -----Original Message-----
> From: huangdengdui <huangdengdui@huawei.com>
> Sent: Tuesday, June 13, 2023 8:55 PM
> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; thomas@monjalon.net;
> Richardson, Bruce <bruce.richardson@intel.com>;
> mb@smartsharesystems.com; Xia, Chenbo <chenbo.xia@intel.com>;
> amitprakashs@marvell.com; anoobj@marvell.com
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang,
> YuanX <yuanx.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>
> Subject: Re: [PATCH v6] app/dma-perf: introduce dma-perf application
> 
> Hi Cheng,
> 
> Few comments inline. Please check.
> 
> Thanks,
> Dengdui
> 
> On 2023/6/13 12:31, Cheng Jiang wrote:
> > There are many high-performance DMA devices supported in DPDK now,
> and
> > these DMA devices can also be integrated into other modules of DPDK as
> > accelerators, such as Vhost. Before integrating DMA into applications,
> > developers need to know the performance of these DMA devices in
> > various scenarios and the performance of CPUs in the same scenario,
> > such as different buffer lengths. Only in this way can we know the
> > target performance of the application accelerated by using them. This
> > patch introduces a high-performance testing tool, which supports
> > comparing the performance of CPU and DMA in different scenarios
> > automatically with a pre-set config file. Memory Copy performance test are
> supported for now.
> >
> > Signed-off-by: Cheng Jiang <cheng1.jiang@intel.com>
> > Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> > Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Chenbo Xia <chenbo.xia@intel.com>
> > ---
> > v6:
> >   improved code based on Anoob's comments;
> >   fixed some code structure issues;
> > v5:
> >   fixed some LONG_LINE warnings;
> > v4:
> >   fixed inaccuracy of the memory footprint display;
> > v3:
> >   fixed some typos;
> > v2:
> >   added lcore/dmadev designation;
> >   added error case process;
> >   removed worker_threads parameter from config.ini;
> >   improved the logs;
> >   improved config file;
> >
> >  app/meson.build               |   1 +
> >  app/test-dma-perf/benchmark.c | 477 ++++++++++++++++++++++++++++
> > app/test-dma-perf/config.ini  |  59 ++++
> >  app/test-dma-perf/main.c      | 569
> ++++++++++++++++++++++++++++++++++
> >  app/test-dma-perf/main.h      |  69 +++++
> >  app/test-dma-perf/meson.build |  17 +
> >  6 files changed, 1192 insertions(+)
> >  create mode 100644 app/test-dma-perf/benchmark.c  create mode
> 100644
> > app/test-dma-perf/config.ini  create mode 100644
> > app/test-dma-perf/main.c  create mode 100644 app/test-dma-perf/main.h
> > create mode 100644 app/test-dma-perf/meson.build
> >
> > diff --git a/app/meson.build b/app/meson.build index
> > 74d2420f67..4fc1a83eba 100644
> > --- a/app/meson.build
> > +++ b/app/meson.build
> > @@ -19,6 +19,7 @@ apps = [
> >          'test-cmdline',
> >          'test-compress-perf',
> >          'test-crypto-perf',
> > +        'test-dma-perf',
> >          'test-eventdev',
> >          'test-fib',
> >          'test-flow-perf',
> > diff --git a/app/test-dma-perf/benchmark.c
> > b/app/test-dma-perf/benchmark.c new file mode 100644 index
> > 0000000000..bc1ca82297
> > --- /dev/null
> > +++ b/app/test-dma-perf/benchmark.c
> > @@ -0,0 +1,477 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2023 Intel Corporation  */
> > +
> 
> <snip>
> 
> > +static inline int
> > +__rte_format_printf(3, 4)
> > +print_err(const char *func, int lineno, const char *format, ...) {
> > +	va_list ap;
> > +	int ret;
> > +
> > +	ret = fprintf(stderr, "In %s:%d - ", func, lineno);
> > +	va_start(ap, format);
> > +	ret += vfprintf(stderr, format, ap);
> > +	va_end(ap);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline void
> > +calc_result(uint32_t buf_size, uint32_t nr_buf, uint16_t nb_workers,
> uint16_t test_secs,
> > +				uint32_t total_cnt, float *memory, uint32_t
> *ave_cycle,
> > +				float *bandwidth, float *mops)
> > +{
> > +	*memory = (float)(buf_size * (nr_buf / nb_workers) * 2) / (1024 *
> 1024);
> > +	*ave_cycle = test_secs * rte_get_timer_hz() / total_cnt;
> > +	*bandwidth = (buf_size * 8 * (rte_get_timer_hz() / (float)*ave_cycle))
> / 1000000000;
> > +	*mops = (float)rte_get_timer_hz() / *ave_cycle / 1000000;
> 
> The value of ave_cycle may be 0.
> 
> *mops = (float)(total_cnt / test_secs) / 1000000; ?

OK, it makes sense to me. I'll fix it in the next version.

> 
> > +}
> > +
> > +static void
> > +output_result(uint8_t scenario_id, uint32_t lcore_id, char *dma_name,
> uint64_t ave_cycle,
> > +			uint32_t buf_size, uint32_t nr_buf, float memory,
> > +			float bandwidth, float mops, bool is_dma) {
> > +	if (is_dma)
> > +		printf("lcore %u, DMA %s:\n", lcore_id, dma_name);
> > +	else
> > +		printf("lcore %u\n", lcore_id);
> > +
> > +	printf("average cycles/op: %" PRIu64 ", buffer size: %u, nr_buf: %u,
> memory: %.2lfMB, frequency: %" PRIu64 ".\n",
> > +			ave_cycle, buf_size, nr_buf, memory,
> rte_get_timer_hz());
> > +	printf("Average bandwidth: %.3lfGbps, MOps: %.3lf\n", bandwidth,
> > +mops);
> > +
> > +	if (is_dma)
> > +		snprintf(output_str[lcore_id], MAX_OUTPUT_STR_LEN,
> CSV_LINE_DMA_FMT,
> > +			scenario_id, lcore_id, dma_name, buf_size,
> > +			nr_buf, memory, ave_cycle, bandwidth, mops);
> > +	else
> > +		snprintf(output_str[lcore_id], MAX_OUTPUT_STR_LEN,
> CSV_LINE_CPU_FMT,
> > +			scenario_id, lcore_id, buf_size,
> > +			nr_buf, memory, ave_cycle, bandwidth, mops); }
> > +
> > +static inline void
> > +cache_flush_buf(__maybe_unused struct rte_mbuf **array,
> > +		__maybe_unused uint32_t buf_size,
> > +		__maybe_unused uint32_t nr_buf)
> > +{
> > +#ifdef RTE_ARCH_X86_64
> > +	char *data;
> > +	struct rte_mbuf **srcs = array;
> > +	uint32_t i, offset;
> > +
> > +	for (i = 0; i < nr_buf; i++) {
> > +		data = rte_pktmbuf_mtod(srcs[i], char *);
> > +		for (offset = 0; offset < buf_size; offset += 64)
> > +			__builtin_ia32_clflush(data + offset);
> > +	}
> > +#endif
> > +}
> > +
> 
> <snip>
> 
> > +
> > +/* Parse the argument given in the command line of the application */
> > +static int append_eal_args(int argc, char **argv, const char
> > +*eal_args, char **new_argv) {
> > +	int i;
> > +	char *tokens[MAX_EAL_PARAM_NB];
> > +	char args[MAX_EAL_PARAM_LEN] = {0};
> > +	int token_nb, new_argc = 0;
> > +
> > +	for (i = 0; i < argc; i++) {
> > +		if ((strcmp(argv[i], CMDLINE_CONFIG_ARG) == 0) ||
> > +				(strcmp(argv[i], CMDLINE_RESULT_ARG) == 0))
> {
> > +			i++;
> > +			continue;
> > +		}
> > +		strlcpy(new_argv[new_argc], argv[i],
> sizeof(new_argv[new_argc]));
> 
> The type_of argv[new_argc] is *char. Cannot use sizeof().
> 
> strcpy(new_argv[new_argc], argv[i]); or strlcpy(new_argv[new_argc], argv[i],
> MAX_EAL_PARAM_LEN); ?

Yes, it's a mistake, thanks for pointing out! I'll fix it in the next version.

> 
> > +		new_argc++;
> > +	}
> > +
> > +	if (eal_args) {
> > +		strlcpy(args, eal_args, sizeof(args));
> > +		token_nb = rte_strsplit(args, strlen(args),
> > +					tokens, MAX_EAL_PARAM_NB, ' ');
> > +		for (i = 0; i < token_nb; i++)
> > +			strcpy(new_argv[new_argc++], tokens[i]);
> > +	}
> > +
> > +	return new_argc;
> > +}
> > +
> > +int
> > +main(int argc, char *argv[])
> > +{
> > +	int ret;
> > +	uint16_t case_nb;
> > +	uint32_t i, nb_lcores;
> > +	pid_t cpid, wpid;
> > +	int wstatus;
> > +	char args[MAX_EAL_PARAM_NB][MAX_EAL_PARAM_LEN];
> > +	char *pargs[MAX_EAL_PARAM_NB];
> > +	char *cfg_path_ptr = NULL;
> > +	char *rst_path_ptr = NULL;
> > +	char rst_path[PATH_MAX];
> > +	int new_argc;
> > +	bool is_first_case = true;
> > +
> > +	memset(args, 0, sizeof(args));
> > +
> > +	for (i = 0; i < RTE_DIM(pargs); i++)
> > +		pargs[i] = args[i];
> > +
> > +	for (i = 0; i < (uint32_t)argc; i++) {
> > +		if (strncmp(argv[i], CMDLINE_CONFIG_ARG,
> MAX_LONG_OPT_SZ) == 0)
> > +			cfg_path_ptr = argv[i + 1];
> > +		if (strncmp(argv[i], CMDLINE_RESULT_ARG,
> MAX_LONG_OPT_SZ) == 0)
> > +			rst_path_ptr = argv[i + 1];
> > +	}
> > +	if (cfg_path_ptr == NULL) {
> > +		printf("Config file not assigned.\n");
> > +		return -1;
> > +	}
> > +	if (rst_path_ptr == NULL) {
> > +		strlcpy(rst_path, cfg_path_ptr, PATH_MAX);
> > +		strcat(strtok(basename(rst_path), "."), "_result.csv");
> > +		rst_path_ptr = rst_path;
> > +	}
> > +
> > +	case_nb = load_configs(cfg_path_ptr);
> > +	fd = fopen(rst_path_ptr, "w");
> > +	if (fd == NULL) {
> > +		printf("Open output CSV file error.\n");
> > +		return -1;
> > +	}
> > +	fclose(fd);
> > +
> > +	for (i = 0; i < case_nb; i++) {
> > +		if (test_cases[i].test_type == TEST_TYPE_NONE) {
> > +			printf("No test type in test case %d.\n\n", i + 1);
> > +			continue;
> > +		}
> > +		if (!test_cases[i].is_valid) {
> > +			printf("Invalid test case %d.\n\n", i + 1);
> > +			continue;
> > +		}
> > +
> > +		cpid = fork();
> > +		if (cpid < 0) {
> > +			printf("Fork case %d failed.\n", i + 1);
> > +			exit(EXIT_FAILURE);
> > +		} else if (cpid == 0) {
> > +			printf("\nRunning case %u\n\n", i + 1);
> > +
> > +			new_argc = append_eal_args(argc, argv,
> test_cases[i].eal_args, pargs);
> > +			ret = rte_eal_init(new_argc, pargs);
> > +			if (ret < 0)
> > +				rte_exit(EXIT_FAILURE, "Invalid EAL
> arguments\n");
> > +
> > +			/* Check lcores. */
> > +			nb_lcores = rte_lcore_count();
> > +			if (nb_lcores < 2)
> > +				rte_exit(EXIT_FAILURE,
> > +					"There should be at least 2 worker
> lcores.\n");
> > +
> > +			fd = fopen(rst_path_ptr, "a");
> > +			if (!fd) {
> > +				printf("Open output CSV file error.\n");
> > +				return 0;
> > +			}
> > +
> > +			if (is_first_case) {
> > +				output_env_info();
> > +				is_first_case = false;
> > +			}
> > +			run_test(i + 1, &test_cases[i]);
> > +
> > +			/* clean up the EAL */
> > +			rte_eal_cleanup();
> > +
> > +			fclose(fd);
> > +
> > +			printf("\nCase %u completed.\n\n", i + 1);
> > +
> > +			exit(EXIT_SUCCESS);
> > +		} else {
> > +			wpid = waitpid(cpid, &wstatus, 0);
> > +			if (wpid == -1) {
> > +				printf("waitpid error.\n");
> > +				exit(EXIT_FAILURE);
> > +			}
> > +
> > +			if (WIFEXITED(wstatus))
> > +				printf("Case process exited. status %d\n\n",
> > +					WEXITSTATUS(wstatus));
> > +			else if (WIFSIGNALED(wstatus))
> > +				printf("Case process killed by signal %d\n\n",
> > +					WTERMSIG(wstatus));
> > +			else if (WIFSTOPPED(wstatus))
> > +				printf("Case process stopped by
> signal %d\n\n",
> > +					WSTOPSIG(wstatus));
> > +			else if (WIFCONTINUED(wstatus))
> > +				printf("Case process continued.\n\n");
> > +			else
> > +				printf("Case process unknown
> terminated.\n\n");
> > +		}
> > +	}
> > +
> > +	printf("Bye...\n");
> > +	return 0;
> > +}
> 
> <snip>

  reply	other threads:[~2023-06-14  6:40 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-20  7:22 [PATCH] " Cheng Jiang
2023-05-17  6:16 ` [PATCH v2] " Cheng Jiang
2023-05-17  7:31 ` [PATCH v3] " Cheng Jiang
2023-06-08  5:03 ` [PATCH v4] " Cheng Jiang
2023-06-08  8:27   ` Xia, Chenbo
2023-06-08  8:38     ` Jiang, Cheng1
2023-06-08  8:43 ` [PATCH v5] " Cheng Jiang
2023-06-09 11:44   ` [EXT] " Anoob Joseph
2023-06-12  7:40     ` Jiang, Cheng1
2023-06-09 14:03   ` Amit Prakash Shukla
2023-06-12  8:26     ` Jiang, Cheng1
2023-06-13  4:51       ` Jiang, Cheng1
2023-06-13  7:34         ` Amit Prakash Shukla
2023-06-13  4:31 ` [PATCH v6] " Cheng Jiang
2023-06-13 12:55   ` huangdengdui
2023-06-14  6:40     ` Jiang, Cheng1 [this message]
2023-06-15  5:21   ` [EXT] " Anoob Joseph
2023-06-15  8:01     ` Jiang, Cheng1
2023-06-15  8:44       ` Anoob Joseph
2023-06-15 14:05         ` Jiang, Cheng1
2023-06-15 15:47           ` Anoob Joseph
2023-06-16  2:56             ` Jiang, Cheng1
2023-06-16  6:32               ` Anoob Joseph
2023-06-16  8:43                 ` Jiang, Cheng1
2023-06-16  9:48                   ` Anoob Joseph
2023-06-16 10:52                     ` Anoob Joseph
2023-06-16 15:15                       ` Jiang, Cheng1
2023-06-17  4:35                         ` Jiang, Cheng1
2023-06-19  5:48                           ` Anoob Joseph
2023-06-19  6:21                             ` Jiang, Cheng1
2023-06-18  5:34                         ` Jiang, Cheng1
2023-06-19  5:25                           ` Anoob Joseph
2023-06-19  6:17                             ` Jiang, Cheng1
2023-06-18 12:26 ` [PATCH v7] " Cheng Jiang
2023-06-20  6:53 ` [PATCH v8] " Cheng Jiang
2023-06-23  6:52   ` [EXT] " Anoob Joseph
2023-06-24 11:52     ` Jiang, Cheng1
2023-06-26  5:41       ` Anoob Joseph
2023-06-26 10:02         ` Jiang, Cheng1
2023-06-26  9:41 ` [PATCH v9] " Cheng Jiang
2023-06-28  1:20 ` [PATCH v10] " Cheng Jiang
2023-06-28  4:42   ` [EXT] " Anoob Joseph
2023-06-28  6:06   ` Ling, WeiX
2023-06-29  9:08   ` Thomas Monjalon
2023-06-29 12:50     ` Jiang, Cheng1
2023-06-29 13:19       ` Thomas Monjalon
2023-06-29 13:24         ` Jiang, Cheng1
2023-06-29  9:38   ` Thomas Monjalon
2023-06-29 12:51     ` Jiang, Cheng1
2023-06-29 13:14 ` [PATCH v11] " Cheng Jiang
2023-07-03  8:20   ` fengchengwen
2023-07-07  9:56     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SN7PR11MB7019085BCA59D5FFBA629FA3DC5AA@SN7PR11MB7019.namprd11.prod.outlook.com \
    --to=cheng1.jiang@intel.com \
    --cc=amitprakashs@marvell.com \
    --cc=anoobj@marvell.com \
    --cc=bruce.richardson@intel.com \
    --cc=chenbo.xia@intel.com \
    --cc=dev@dpdk.org \
    --cc=huangdengdui@huawei.com \
    --cc=jiayu.hu@intel.com \
    --cc=mb@smartsharesystems.com \
    --cc=thomas@monjalon.net \
    --cc=wenwux.ma@intel.com \
    --cc=xingguang.he@intel.com \
    --cc=xuan.ding@intel.com \
    --cc=yuanx.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).