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 C4B33423FE; Tue, 17 Jan 2023 16:45:04 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6E2D1400EF; Tue, 17 Jan 2023 16:45:04 +0100 (CET) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mails.dpdk.org (Postfix) with ESMTP id 5FCBE400D4 for ; Tue, 17 Jan 2023 16:45:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1673970302; x=1705506302; h=date:from:to:cc:subject:message-id:references: content-transfer-encoding:in-reply-to:mime-version; bh=LH8e61AT/kBfaw9x9/zlO0eNhUQScKfXHchH+WdjNt8=; b=hOigzxvpq1/WsTH/a2vLWuW15CEe4Rcqsya7SfZ6UZF/RO716oijOgJy 3JRCG0ROifrRgPixxIJ2RX6KEPAY0ieMQwZXlylfy6MGaf0/OYcp+JOMz UsXTKVZ3nboSL2xQuG76ZL5qWSvh0KlPBzaYHJkxNZOEs25Umql7GWIIA F1VrkOGvo+w7VSkTsD1DXnFav8+H9AMKPB6+x8heja54oiqgVbLz/jnTh /vqYxZnQDGoPjQ2U5u368j4vpUxTxAm+9pMKZoEkuG3SOB7loVt9XmOt9 zg3vU+nWFv8NYMVkmhN1mCWnMRVWMqKMqz8b16NSgH5Z0/wIEFPEN0Wdw g==; X-IronPort-AV: E=McAfee;i="6500,9779,10592"; a="312594734" X-IronPort-AV: E=Sophos;i="5.97,224,1669104000"; d="scan'208";a="312594734" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Jan 2023 07:44:45 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10592"; a="691631718" X-IronPort-AV: E=Sophos;i="5.97,224,1669104000"; d="scan'208";a="691631718" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by orsmga001.jf.intel.com with ESMTP; 17 Jan 2023 07:44:45 -0800 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Tue, 17 Jan 2023 07:44:45 -0800 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Tue, 17 Jan 2023 07:44:44 -0800 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Tue, 17 Jan 2023 07:44:44 -0800 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.108) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Tue, 17 Jan 2023 07:44:43 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=DxhxWzWVw/8+gu9DzHWEoHyQGyWT6vIIrfAlx8gHpgOYUvQz3DfH6+K3E17V3DuuTOycTcW1gTRoCmNYl52CgOyon9xrNdOx3dAYfYxVENs93KrGjaSIxS8LL4ZQ0jSq/W5EM1OBSotq7t+KpSwMPdiMoiE1x2kxa3gyXkSYgKoRJOweGM4MeydUstEhcUcb8J6PNpaGiUUrnxQCwL97m0S1hERz/Oh4eeVexzUMlits1I77CDZgYdAklJkr1DOq8rzQlCQS2T3WI8BpLvogAiJloj3d1MdlyecLvN0+KPFKdFM/RRPfWEITzlLEinp/YxQzj0SWePwsFmB8Q0wVxQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=LphoMZaj4X2bgkHxJZrGuUpN51B67IuOsj/pr8sgQzI=; b=YIhbPivTXFEgVho1uxRr2Wx+NCNNls1J0cFHupE4TE4opJ+zzaHC9/6Jm5jPj+WA+nXu9b+E/D7QYTD78eqCLoDwoLqAZZ0Qlu5hJKkygxEcNwCCRxWrCNwX367cVOEXJu/rhYg40AgoR5C1MBliiYv4KyzC0zcGT/OaZYrUVywofpFPViKaqOJqd7Gv57gnlqaeVbGb3zQZ4WfIaB1UhCzvhhKgH0Xfca2AZJFmHkeFy3p9OIwScbMGbNzgzJPSzRBBEIk96+p9cXtx59nzxkQZtbDhpQv8zmInh14zYUjMqsyUWbBlj+7iq+jU+MP7KmKwJz7+8zkq4Nn+1fX5hA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) by DM4PR11MB6478.namprd11.prod.outlook.com (2603:10b6:8:89::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5986.19; Tue, 17 Jan 2023 15:44:36 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::4d9f:6867:2d53:9ee]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::4d9f:6867:2d53:9ee%7]) with mapi id 15.20.5986.023; Tue, 17 Jan 2023 15:44:36 +0000 Date: Tue, 17 Jan 2023 15:44:27 +0000 From: Bruce Richardson To: Cheng Jiang CC: , , , , , , , Subject: Re: [PATCH v3] app/dma-perf: introduce dma-perf application Message-ID: References: <20221220010619.31829-1-cheng1.jiang@intel.com> <20230117120526.39375-1-cheng1.jiang@intel.com> Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230117120526.39375-1-cheng1.jiang@intel.com> X-ClientProxiedBy: LO3P123CA0007.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:ba::12) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|DM4PR11MB6478:EE_ X-MS-Office365-Filtering-Correlation-Id: f4b39535-9737-4fb4-5346-08daf8a1bca6 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: ySqmfz50TTLHBcsdKbA5JKCw0kx50/PxgrRXzgr2LNp7z0ia4TgIGz7HpkYSAcy3iLj8PfqYN63Fm5WNGOGslaHIwmGouoErOdCTobzwe8v/Q4HOG/SgjcoN3VS/yWLH7AYx88itALKEN0htWKQhWziHYnZlhoGwP4jW9Fa8CIFE1tkiQ2vsrjXQSCa+eyO/cnIa048kTTdPaOA+VbrMR+avtitXriBiAUHbSP8Q9GlKxEX4nu8HXhsEHYqQPf5a/xWzklyP10VtrmUgCBRx9KrYbUv73/YkkL5OBUdPkys/djp4SHGIqfAdOuTWDAN5jEUAmLGv3IbAK9GsmxsLcfwyAbcJTXxTOeabiU7go7shJ01/peIIz4r/w+ZXGCnza7lGY6i4LNRI12Ag1cnZQJ+QlQDalEQ48sF9m6XjxYHVeYtdOc3l2s57wALC2ze9G/VVMVVo506obyLV5DlQG8HyNvabTJ0NV5VMVHYivlGV2QTv5RKwRbKxqExZ3QRRwkgIZYl6HiMX4wTyRAWds57fxdhdFFoJb6hv61uk2pBLwFsx/0GXmaaDUKGbHBlFnvmiIxzzjpGeeK4R5ugjd1oYm5de6jTmXIh8VRbXMcNTzvTvrqwzgtpieihAMM3Rj6e3h77W5YkOTjdd7IeafQ== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DS0PR11MB7309.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(39860400002)(376002)(346002)(366004)(136003)(396003)(451199015)(44832011)(66556008)(5660300002)(8936002)(6862004)(41300700001)(6636002)(2906002)(316002)(66946007)(4326008)(66476007)(8676002)(82960400001)(6486002)(478600001)(38100700002)(26005)(107886003)(6512007)(6506007)(186003)(6666004)(66574015)(30864003)(86362001)(83380400001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?OsSK0GVDSSillxe1ZWPAXyGYt/01GdMIlNHkx+J5WcVY978XcyV8RkXcGD?= =?iso-8859-1?Q?FOun2Pgrp8bx4AW54fyw5DUsZOs3vNseqs7RBRcymlKitThEfw9RSEgzq0?= =?iso-8859-1?Q?4lsWnhNB1ozqcrV0yVqa+yqsYtVNxqGvpJtf/IQBg7CdyAAu7pNSV5tVK+?= =?iso-8859-1?Q?C6KsKToXeZT5iC4Jd+YWKESWJpTr0KJWqWX5E0F2wc/mGW9CdspcwwViir?= =?iso-8859-1?Q?h2YzNtUusteA9NuzhYj0fbECvngTbFkKFSwRONUa6QSya8nIpaIGody7op?= =?iso-8859-1?Q?du7bS5PXcMDIMi7rzRBo3wx8j4Vgro4SAa38ygnFAOLRyvC9Xt8XrJ8CFO?= =?iso-8859-1?Q?MavfT5EpLIG0dWs+0C+i0DORQns/ES48w+0fQwD14pDngwa5BOF13UYBfM?= =?iso-8859-1?Q?N6KG1AhSPv/cVpOwCOVwPKnXcsGxf4ncfhky9TiRGbZy1QondeANClT+GA?= =?iso-8859-1?Q?kPx1ah/2NBRfUZVytlvS7WM91jHxvJ5wdUCdDp+oXmCFLhV6hdQQN2kfoa?= =?iso-8859-1?Q?NVHxywqTzO9cgrlfYrnznZfdzlMSerWQfuYD0QajXpuoS79WRKP8kKirnA?= =?iso-8859-1?Q?JuBDam2JdTnEWzEqCrIjGzE3UvF8TdVvnTE+Qa1JpPIEnq9vFBnzl+ZjhQ?= =?iso-8859-1?Q?cydoTqaEqnpNVvtnVoSEMCWePo5bYzqCKTUzNXaYNDgsdXP/aGr6XAhlyO?= =?iso-8859-1?Q?tC3FZ+FFvBk09nDlBtGadCbOfTUlWJEVIKN63piSWvkULt3gal19KVWL0W?= =?iso-8859-1?Q?zDp2LnqOTkxrp2hzur19maY3HLRLpBluN5zzDOPdmeBg/OO/Qh0E0zfMI4?= =?iso-8859-1?Q?da//2nhHzuV+Qq9sC/Vx8DW/Rzjje/T7EwADlifklEhMzt/pBB30wBS+X0?= =?iso-8859-1?Q?sYL5iecwis4ecCe3AHiDCdOY3sSoliXhfr2BATcmocube/GSGbY/iwBd2d?= =?iso-8859-1?Q?kU1AG4Ra3T4qqoGL0dErwP0+aGXQo7K5/Q3gCJojly+WHrREjX8pIcXWWx?= =?iso-8859-1?Q?TWrCUqQIGs/pF06Bz4LLhUrg2Jad1L7Pfuo8opbbSSIbo24t7rE8OT4/oo?= =?iso-8859-1?Q?mGmYuSCEN/5mIhosp7pATKnFETqj32+fA8aHxkgoW//a8Q/oENI1hHg1iF?= =?iso-8859-1?Q?n6JNAbCHgwVykVsMZneVDAokv4OfxOYBniCpkqpRgZ9Z93AO7rV2vbwcPU?= =?iso-8859-1?Q?aTp7sehnGbWdtXfHUuszfos8CJZgefL+9tL7nxox0ALwZjuVMB7r6AfSli?= =?iso-8859-1?Q?xc3L9Pq8XYt5tbOtJDbaEmv9KDLeZfarPmKQklixOhocdDlvEOurQ2vN9e?= =?iso-8859-1?Q?1t6Mj0PJA/egCVYnx8IrYPJqbeZnLrL/69paj1BltwZUGRSKpp3Z5IMC9G?= =?iso-8859-1?Q?Pg467A9GmgJ4It0SLXLar4hSyLHiuYk4RowDH16EOtpOz+v+rCivrbt9Y/?= =?iso-8859-1?Q?rkG25C4936JFKFkysPYtJ8t7elv3dQBGQOgpDVVDFa44tPYM8C58HDzjKJ?= =?iso-8859-1?Q?x6V6UDfLN48xq+29yYjGHezE+19J3ugyCGoF8m0b1jBbwX6Csi4+nbbGkp?= =?iso-8859-1?Q?s9eFDX32mwVvyfV6cPf8/ZKQi/Z86765mD8S93jG6/Z5TPPABkCCocfRiJ?= =?iso-8859-1?Q?qSNo//2TVt/Ej8wjXT6tLtobDjfPAIVDDUem6L7YQJjApyiuxmaXYMPR3E?= =?iso-8859-1?Q?RQ1+9i92nZGUHVPARdg=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: f4b39535-9737-4fb4-5346-08daf8a1bca6 X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2023 15:44:36.7286 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: wmBvxnvzQl5x8aZzPtYIMpEAv2lP3fQXz6cqoxaac2Ok0B+cVy6dlS3Q2OAO3lIV482U3YVEcXyJ+yfz4IYzIIKNXtrEHCo5b4Lz55BwZdk= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6478 X-OriginatorOrg: intel.com 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 On Tue, Jan 17, 2023 at 12:05:26PM +0000, 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 > Signed-off-by: Jiayu Hu > Signed-off-by: Yuan Wang > Acked-by: Morten Brørup Hi, more review comments inline below. /Bruce > --- > eal_args=--legacy-mem --file-prefix=test Why using legact-mem mode? Rather than these options, just use "--in-memory" to avoid any conflicts. While this is only an example config, we should steer people away from legacy memory mode. > diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c > new file mode 100644 > index 0000000000..8041f5fdaf > --- /dev/null > +++ b/app/test-dma-perf/main.c > @@ -0,0 +1,434 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2022 Intel Corporation > + */ > + > +#include > +#if !defined(RTE_EXEC_ENV_LINUX) > + > +int > +main(int argc, char *argv[]) > +{ > + printf("OS not supported, skipping test\n"); > + return 0; > +} > + > +#else > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#include "main.h" > +#include "benchmark.h" > + > +#define CSV_HDR_FMT "Case %u : %s,lcore,DMA,buffer size,nr_buf,memory(MB),cycle,bandwidth(Gbps),OPS\n" > + > +#define MAX_EAL_PARAM_NB 100 > +#define MAX_EAL_PARAM_LEN 1024 > + > +#define DMA_MEM_COPY "DMA_MEM_COPY" > +#define CPU_MEM_COPY "CPU_MEM_COPY" > + > +#define MAX_PARAMS_PER_ENTRY 4 > + > +enum { > + TEST_TYPE_NONE = 0, > + TEST_TYPE_DMA_MEM_COPY, > + TEST_TYPE_CPU_MEM_COPY > +}; > + > +#define MAX_TEST_CASES 16 > +static struct test_configure test_cases[MAX_TEST_CASES]; > + > +char output_str[MAX_WORKER_NB][MAX_OUTPUT_STR_LEN]; > + > +static FILE *fd; > + > +static void > +output_csv(bool need_blankline) > +{ > + uint32_t i; > + > + if (need_blankline) { > + fprintf(fd, "%s", ",,,,,,,,\n"); > + fprintf(fd, "%s", ",,,,,,,,\n"); you don't need the "%s" here. The string you are outputting is constant. > + } > + > + for (i = 0; i < RTE_DIM(output_str); i++) { > + if (output_str[i][0]) { > + fprintf(fd, "%s", output_str[i]); > + memset(output_str[i], 0, MAX_OUTPUT_STR_LEN); Rather than zeroing the whole string with memset, would "output_str[i][0] = '\0';" not work instead? > + } > + } > + > + fflush(fd); > +} > + > +static void > +output_env_info(void) > +{ > + snprintf(output_str[0], MAX_OUTPUT_STR_LEN, "test environment:\n"); > + snprintf(output_str[1], MAX_OUTPUT_STR_LEN, "frequency,%" PRIu64 "\n", rte_get_timer_hz()); > + > + output_csv(true); > +} > + > +static void > +output_header(uint32_t case_id, struct test_configure *case_cfg) > +{ > + snprintf(output_str[0], MAX_OUTPUT_STR_LEN, > + CSV_HDR_FMT, case_id, case_cfg->test_type_str); > + > + output_csv(true); > +} > + > +static void > +run_test_case(struct test_configure *case_cfg) > +{ > + switch (case_cfg->test_type) { > + case TEST_TYPE_DMA_MEM_COPY: > + dma_mem_copy_benchmark(case_cfg); > + break; > + case TEST_TYPE_CPU_MEM_COPY: > + cpu_mem_copy_benchmark(case_cfg); > + break; > + default: > + printf("Unknown test type. %s\n", case_cfg->test_type_str); > + break; > + } > +} > + > +static void > +run_test(uint32_t case_id, struct test_configure *case_cfg) > +{ > + uint32_t i; > + uint32_t nb_lcores = rte_lcore_count(); > + struct test_configure_entry *mem_size = &case_cfg->mem_size; > + struct test_configure_entry *buf_size = &case_cfg->buf_size; > + struct test_configure_entry *ring_size = &case_cfg->ring_size; > + struct test_configure_entry *kick_batch = &case_cfg->kick_batch; > + struct test_configure_entry *var_entry = NULL; > + > + for (i = 0; i < RTE_DIM(output_str); i++) > + memset(output_str[i], 0, MAX_OUTPUT_STR_LEN); > + > + if (nb_lcores <= case_cfg->nb_workers) { > + printf("Case %u: Not enough lcores (%u) for all workers (%u).\n", > + case_id, nb_lcores, case_cfg->nb_workers); > + return; > + } > + > + RTE_LOG(INFO, DMA, "Number of used lcores: %u.\n", nb_lcores); > + > + if (mem_size->incr != 0) > + var_entry = mem_size; > + > + if (buf_size->incr != 0) > + var_entry = buf_size; > + > + if (ring_size->incr != 0) > + var_entry = ring_size; > + > + if (kick_batch->incr != 0) > + var_entry = kick_batch; > + > + case_cfg->scenario_id = 0; > + > + output_header(case_id, case_cfg); > + > + if (var_entry) { Things may be a bit simpler if instead of branching here, you initialize var_entry to a null var_entry i.e. struct test_configure_entry dummy = { 0 }; struct test_configure_entry *var_entry = &dummy; This gives you a single-iteration loop in the case where there is nothing to vary. > + for (var_entry->cur = var_entry->first; var_entry->cur <= var_entry->last;) { > + case_cfg->scenario_id++; > + printf("\nRunning scenario %d\n", case_cfg->scenario_id); > + > + run_test_case(case_cfg); > + output_csv(false); > + > + if (var_entry->op == OP_MUL) > + var_entry->cur *= var_entry->incr; > + else > + var_entry->cur += var_entry->incr; > + > + > + } > + } else { > + run_test_case(case_cfg); > + output_csv(false); > + } > +} > + > +static int > +parse_entry(const char *value, struct test_configure_entry *entry) > +{ > + char input[255] = {0}; > + char *args[MAX_PARAMS_PER_ENTRY]; > + int args_nr = -1; > + > + strncpy(input, value, 254); > + if (*input == '\0') > + goto out; > + > + args_nr = rte_strsplit(input, strlen(input), args, MAX_PARAMS_PER_ENTRY, ','); > + if (args_nr <= 0) > + goto out; > + > + entry->cur = entry->first = (uint32_t)atoi(args[0]); > + entry->last = args_nr > 1 ? (uint32_t)atoi(args[1]) : 0; > + entry->incr = args_nr > 2 ? (uint32_t)atoi(args[2]) : 0; > + > + if (args_nr > 3) { > + if (!strcmp(args[3], "MUL")) > + entry->op = OP_MUL; > + else > + entry->op = OP_ADD; This means accepting invalid input. I think you should check the value against "ADD" so as to reject values like "SUB". > + } else > + entry->op = OP_NONE; > +out: > + return args_nr; > +} > + > +static void > +load_configs(void) > +{ > + struct rte_cfgfile *cfgfile; > + int nb_sections, i; > + struct test_configure *test_case; > + char **sections_name; > + const char *section_name, *case_type; > + const char *mem_size_str, *buf_size_str, *ring_size_str, *kick_batch_str; > + int args_nr, nb_vp; > + > + sections_name = malloc(MAX_TEST_CASES * sizeof(char *)); > + for (i = 0; i < MAX_TEST_CASES; i++) > + sections_name[i] = malloc(CFG_NAME_LEN * sizeof(char *)); > + I don't think you need to do this work, allocating space for a bunch of section names. From the example, it looks like the sections should be called "case1", "case2" etc., so you can just iterate through those sections, rather than allowing sections to have arbitrary names. > + cfgfile = rte_cfgfile_load("./config.ini", 0); > + if (!cfgfile) { > + printf("Open configure file error.\n"); > + exit(1); > + } Don't hard-code the config file name. This should be taken from a commandline parameter, so that one can have collections of different test cases. > + > + nb_sections = rte_cfgfile_num_sections(cfgfile, NULL, 0); > + if (nb_sections > MAX_TEST_CASES) { > + printf("Error: The maximum number of cases is %d.\n", MAX_TEST_CASES); > + exit(1); > + } > + rte_cfgfile_sections(cfgfile, sections_name, MAX_TEST_CASES); > + for (i = 0; i < nb_sections; i++) { Iterate through names here, built up dynamically to save memory space. > + test_case = &test_cases[i]; > + section_name = sections_name[i]; > + case_type = rte_cfgfile_get_entry(cfgfile, section_name, "type"); > + if (!case_type) { > + printf("Error: No case type in case %d\n.", i + 1); > + exit(1); > + } > + if (!strcmp(case_type, DMA_MEM_COPY)) { Coding standard for DPDK requires this to be "strcmp(...) == 0" rather than using "!" operator. > + test_case->test_type = TEST_TYPE_DMA_MEM_COPY; > + test_case->test_type_str = DMA_MEM_COPY; > + } else if (!strcmp(case_type, CPU_MEM_COPY)) { > + test_case->test_type = TEST_TYPE_CPU_MEM_COPY; > + test_case->test_type_str = CPU_MEM_COPY; > + } else { > + printf("Error: Cannot find case type %s.\n", case_type); > + exit(1); > + } > + > + nb_vp = 0; > + > + 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, > + section_name, "dst_numa_node")); > + > + mem_size_str = rte_cfgfile_get_entry(cfgfile, section_name, "mem_size"); > + args_nr = parse_entry(mem_size_str, &test_case->mem_size); > + if (args_nr < 0) { > + printf("parse error\n"); > + break; > + } else if (args_nr > 1) > + nb_vp++; > + > + buf_size_str = rte_cfgfile_get_entry(cfgfile, section_name, "buf_size"); > + args_nr = parse_entry(buf_size_str, &test_case->buf_size); > + if (args_nr < 0) { > + printf("parse error\n"); > + break; > + } else if (args_nr > 1) > + nb_vp++; > + > + ring_size_str = rte_cfgfile_get_entry(cfgfile, section_name, "dma_ring_size"); > + args_nr = parse_entry(ring_size_str, &test_case->ring_size); > + if (args_nr < 0) { > + printf("parse error\n"); > + break; > + } else if (args_nr > 1) > + nb_vp++; > + > + kick_batch_str = rte_cfgfile_get_entry(cfgfile, section_name, "kick_batch"); > + args_nr = parse_entry(kick_batch_str, &test_case->kick_batch); > + if (args_nr < 0) { > + printf("parse error\n"); > + break; > + } else if (args_nr > 1) > + nb_vp++; > + > + if (nb_vp > 2) { > + printf("%s, variable parameters can only have one.\n", section_name); Reword to: "Error, each section can only have a single variable parameter" Also, comparison should be ">= 2" (or "> 1") rather than "> 2", which would allow 2 as a valid value. > + break; > + } > + > + test_case->cache_flush = > + (int)atoi(rte_cfgfile_get_entry(cfgfile, section_name, "cache_flush")); > + test_case->repeat_times = > + (uint32_t)atoi(rte_cfgfile_get_entry(cfgfile, > + section_name, "repeat_times")); > + test_case->nb_workers = > + (uint16_t)atoi(rte_cfgfile_get_entry(cfgfile, > + section_name, "worker_threads")); > + test_case->mpool_iter_step = > + (uint16_t)atoi(rte_cfgfile_get_entry(cfgfile, > + section_name, "mpool_iter_step")); > + > + test_case->eal_args = rte_cfgfile_get_entry(cfgfile, section_name, "eal_args"); > + } > + > + rte_cfgfile_close(cfgfile); > + for (i = 0; i < MAX_TEST_CASES; i++) { > + if (sections_name[i] != NULL) > + free(sections_name[i]); Two points here: 1. You don't need to check for NULL before calling "free()". Free just does nothing if passed a null pointer 2. None of these values should be NULL anyway, and you need to check the return from the malloc call. If you *do* keep the current way of reading sections (and I recommend you don't - see my comments above), you need to check that each malloc succeeds or else the call to "rte_cfgfile_sections" will try and do a strlcpy to a null pointer. > + } > + free(sections_name); > +} > + > +/* 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 new_argc, token_nb; > + > + new_argc = argc; > + > + for (i = 0; i < argc; i++) > + strcpy(new_argv[i], argv[i]); I'm not sure we have a guarantee that new_argv will be big enough, do we? Better to use strlcpy just in case here. > + > + if (eal_args) { > + strcpy(args, eal_args); Use strlcpy for safety. > + 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 __maybe_unused, char *argv[] __maybe_unused) > +{ > + int ret; > + uint32_t i, nb_lcores; > + pid_t cpid, wpid; > + int wstatus; > + char args[MAX_EAL_PARAM_NB][MAX_EAL_PARAM_LEN]; > + char *pargs[100]; char *pargs[MAX_EAL_PARAM_NB] ?? > + int new_argc; > + > + > + memset(args, 0, sizeof(args)); > + for (i = 0; i < 100; i++) RTE_DIM(pargs) > + pargs[i] = args[i]; > + > + load_configs(); > + fd = fopen("./test_result.csv", "w"); Like the config file, the result output file should be configurable. Perhaps it should be based off the config file name? test1.ini => test1_result.csv config.ini => config_result.csv > + if (!fd) { > + printf("Open output CSV file error.\n"); > + return 0; > + } > + fclose(fd); > + > + /* loop each case, run it */ > + for (i = 0; i < MAX_TEST_CASES; i++) { > + if (test_cases[i].test_type != TEST_TYPE_NONE) { Flip this condition to reduce indentation: if (test_cases[i].test_type == TEST_TYPE_NONE) 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", i + 1); > + > + if (test_cases[i].eal_args) { > + new_argc = append_eal_args(argc, argv, > + test_cases[i].eal_args, pargs); > + > + ret = rte_eal_init(new_argc, pargs); > + } else { You don't need this if-else here. The append_eal_args function handles a NULL parameter, so unconditionally call append_eal_args and then eal_init(new_argc, pargs). We won't notice the different in init time, but the code would be clearer. > + ret = rte_eal_init(argc, argv); > + } > + 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("./test_result.csv", "a"); > + if (!fd) { > + printf("Open output CSV file error.\n"); > + return 0; > + } > + > + if (i == 0) > + output_env_info(); Beware that you have a condition above to skip any test cases where "test_type == TEST_TYPE_NONE". Therefore, if the first test is of type NONE, the env_info will never be printed. > + run_test(i + 1, &test_cases[i]); > + > + /* clean up the EAL */ > + rte_eal_cleanup(); > + > + fclose(fd); > + > + printf("\nCase %u completed.\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", > + WEXITSTATUS(wstatus)); > + else if (WIFSIGNALED(wstatus)) > + printf("Case process killed by signal %d\n", > + WTERMSIG(wstatus)); > + else if (WIFSTOPPED(wstatus)) > + printf("Case process stopped by signal %d\n", > + WSTOPSIG(wstatus)); > + else if (WIFCONTINUED(wstatus)) > + printf("Case process continued.\n"); > + else > + printf("Case process unknown terminated.\n"); > + } > + } > + } > + > + printf("Bye...\n"); > + return 0; > +} > + > +#endif > diff --git a/app/test-dma-perf/main.h b/app/test-dma-perf/main.h > new file mode 100644 > index 0000000000..a8fcf4f34d > --- /dev/null > +++ b/app/test-dma-perf/main.h > @@ -0,0 +1,57 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2022 Intel Corporation > + */ > + > +#ifndef _MAIN_H_ > +#define _MAIN_H_ > + > + > +#include > +#include > + > +#ifndef __maybe_unused > +#define __maybe_unused __rte_unused > +#endif > + > +#define MAX_WORKER_NB 128 > +#define MAX_OUTPUT_STR_LEN 512 > + > +#define RTE_LOGTYPE_DMA RTE_LOGTYPE_USER1 > + While there are a number of RTE_LOG calls in the app, I also see a number of regular printfs for output. Again, please standardize on one output - if using just printf, you can drop this line. > +extern char output_str[MAX_WORKER_NB][MAX_OUTPUT_STR_LEN]; > + > +typedef enum { > + OP_NONE = 0, > + OP_ADD, > + OP_MUL > +} alg_op_type; > + > +struct test_configure_entry { > + uint32_t first; > + uint32_t last; > + uint32_t incr; > + alg_op_type op; > + uint32_t cur; > +}; > + > +struct test_configure { > + uint8_t test_type; > + const char *test_type_str; > + uint16_t src_numa_node; > + uint16_t dst_numa_node; > + uint16_t opcode; > + bool is_dma; > + struct test_configure_entry mem_size; > + struct test_configure_entry buf_size; > + struct test_configure_entry ring_size; > + struct test_configure_entry kick_batch; > + uint32_t cache_flush; > + uint32_t nr_buf; > + uint32_t repeat_times; > + uint32_t nb_workers; > + uint16_t mpool_iter_step; > + const char *eal_args; > + uint8_t scenario_id; > +}; > + > +#endif /* _MAIN_H_ */ > diff --git a/app/test-dma-perf/meson.build b/app/test-dma-perf/meson.build > new file mode 100644 > index 0000000000..001f67f6c1 > --- /dev/null > +++ b/app/test-dma-perf/meson.build > @@ -0,0 +1,20 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2019-2022 Intel Corporation 2023 now > + > +# meson file, for building this example as part of a main DPDK build. > +# > +# To build this example as a standalone application with an already-installed > +# DPDK instance, use 'make' Drop this comment. The test apps in "app" folder are for building as part of DPDK only, there is no makefile to use. > + > +if is_windows > + build = false > + reason = 'not supported on Windows' > + subdir_done() > +endif > + > +deps += ['dmadev', 'mbuf', 'cfgfile'] > + > +sources = files( > + 'main.c', > + 'benchmark.c', > +) > -- > 2.35.1 >