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 6BAF542414; Thu, 19 Jan 2023 08:18:11 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4C9D3410DD; Thu, 19 Jan 2023 08:18:11 +0100 (CET) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 05C054068E for ; Thu, 19 Jan 2023 08:18:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1674112689; x=1705648689; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=QCSBjTLYXs22AT7o3pxl+8+bhXcGDq7nBFgoYDa3v2Y=; b=EYcIRFTUX+0mDnw3v3jaok/ppev735Qpnx4SfZmeRxOYab8JnVkAZ3p/ FgZB/9lblTRlPA4pkzmW/GoRHsoCbrPMw+trkbo9ePlqQ3D2FsspMQXaO cA3/QQAccWv3RLGhMcqR+E5CuhoCoylIexrEvp3YN2wkSVcKsKiC7jgEo Xr1dih/GoWglwSGBQr/sysIrYh4ol/s1hoDWGdgWtmh9CK5zp7U8YUbjQ 1BS286XWY5D3/211/typFSPz6yZo80VXfFYXOVaB1vzVZdSINlnrCg3j6 qXlqdf2pnNZiEdbOKBxhl6XaH1TiSgUv19JwIRGNvGtl2UIP5uWnSRvIC Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10594"; a="389712045" X-IronPort-AV: E=Sophos;i="5.97,228,1669104000"; d="scan'208";a="389712045" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jan 2023 23:18:08 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10594"; a="833892902" X-IronPort-AV: E=Sophos;i="5.97,228,1669104000"; d="scan'208";a="833892902" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by orsmga005.jf.intel.com with ESMTP; 18 Jan 2023 23:18:07 -0800 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Wed, 18 Jan 2023 23:18:07 -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; Wed, 18 Jan 2023 23:18:07 -0800 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) 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; Wed, 18 Jan 2023 23:18:07 -0800 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.107) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Wed, 18 Jan 2023 23:18:05 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YCvXlfsUNrU9rCzvEqtEvXG8Sihd4hYLYDJSNGtBLGoX34a9tAgh6ieK4+0B2i/EwqABpCXQ1hicQ85tPcknFe+LoJ8354Kp/mF1Zkamkx50Lk+LMfcDiDiSL7eqjOd4R8jUyf46hX+Vl2KnTF9iLBUBr6baBrau/32BewydIQRvP8cPjKGc4RHdHpuymRwBwvsokky6iakxl5C6hRTXNSgM0QHVVG9esmQkZtU5pz/nxzR6EwNo9KoVQv8TqoCWm72Q8SG/Hw+47WMdhebJK0AbNG5Qo7l/eiGp7yNu3wKQP1z9/gPWC/hQURI/OlhRLPF5fnnE7qx7AbVXJo5cKg== 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=Pq+WyEbxLCm2/cLhEOlbrW5b6ARrIx/G9yi99jJEiV4=; b=IL3iixXGQ/qdIj3ejkIqIVi55RqhJiZ5eHEvmHEt7RtrV7Utk7F3favtuR8lkgY4Op7xodTMHjqSZGg3vKHt8ndLX8wAAbB9gXPQLKs+VtT+I96Z+9847gi6PnqcnhOLXg2ttTGDa7OZDqF6OUjEgf0xDWe8y4QFbCO2aKNFXrW2j6Wag0TT9VHtx/1/kMgjVL9g7jQT+uSnV0AgdVM5zjYrZQ0rkmfYJ0gTnBO1dF5DEsM0CEavTmeGjSpxNcqPsTVvaOl+T1tmfBrHvuT2728pWMN3shkyGQ0i4NbOHnRlKLaYA0SL6Q20U7M5DsEzJrMG/alrsNNbWh/nhfM7JA== 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 Received: from SN7PR11MB7019.namprd11.prod.outlook.com (2603:10b6:806:2ae::22) by DS0PR11MB7832.namprd11.prod.outlook.com (2603:10b6:8:f5::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6002.24; Thu, 19 Jan 2023 07:18:03 +0000 Received: from SN7PR11MB7019.namprd11.prod.outlook.com ([fe80::bacc:6241:115b:b26]) by SN7PR11MB7019.namprd11.prod.outlook.com ([fe80::bacc:6241:115b:b26%8]) with mapi id 15.20.5986.023; Thu, 19 Jan 2023 07:18:03 +0000 From: "Jiang, Cheng1" To: "Richardson, Bruce" CC: "thomas@monjalon.net" , "mb@smartsharesystems.com" , "dev@dpdk.org" , "Hu, Jiayu" , "Ding, Xuan" , "Ma, WenwuX" , "Wang, YuanX" , "He, Xingguang" Subject: RE: [PATCH v3] app/dma-perf: introduce dma-perf application Thread-Topic: [PATCH v3] app/dma-perf: introduce dma-perf application Thread-Index: AQHZKnL8tM8ec+sHQUWegU+S/Kqmva6iwEOAgAKNdUA= Date: Thu, 19 Jan 2023 07:18:03 +0000 Message-ID: References: <20221220010619.31829-1-cheng1.jiang@intel.com> <20230117120526.39375-1-cheng1.jiang@intel.com> In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: SN7PR11MB7019:EE_|DS0PR11MB7832:EE_ x-ms-office365-filtering-correlation-id: 30a48505-943d-4381-4f12-08daf9ed4d9e x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: FyE9BJusQB/GVP31ZbOHICoLSi+X1o3NjD11zAIjw4bshA3h0GCj0sOjDBKmNWCOl7YON2UbozT5oImyn0qMiU5E5Nl9LH1FbOSxpDzCY8TO0ULBaJ2UsXyizqOcjVnnQ3fT8ogfvIzL9GFjCLBkT7E3RpfvdX3b9Q2nuRH1W8ksCoDPqNHQRBGJQqyPmyNAqkI2UZKG2K7TneW2mUl17XRwCGAuqoSBLVfGV8JvIfg9r9Hz3PM+XP0P526KYpOxch7RA+7Iql2Rj6MaXNn0ZxfniBnW3Em/Uovx/EyHNOivbmqDA6JRDV92V9P5tyMKUAdPipCBnGcxoOC5aQkkNWV8cxhw46hDgheQF1BxtBEHgSUarWMIho4G8fc7a/7wBsINH3tdeG9SpNT4AUT/JhJjrs193b0/Hn2mO/IMDF+mQbj5VwoX8CJRs750+lAjKQ82deEeDBt5M/aDqhU/tLrh/t2LqrGt/Hy6BMOB+hM64e6720wb19SH7ZG7ChdEgT6SLkI4gcGwBf/5w1b172WXUkxbQuR7XIlgNYihzUlvipZrOnxtx2LT6/sW7jsnpY4UL4rqA7rvmYcVJrhMkxqgGMXveoX7nLpVIgN6N8R6iIxqT1botf0vuMJLe6UFIOIwEms5YHaFlbO2inq4aB9JFCcGLueiJvT/dDha4epgG0hHj2eZP5rNHm7iGOzlE5gBN0sNOKTPDaWJ0U9j+Q== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:SN7PR11MB7019.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(39860400002)(376002)(136003)(346002)(366004)(396003)(451199015)(86362001)(66476007)(122000001)(55016003)(2906002)(30864003)(66556008)(52536014)(5660300002)(8936002)(6862004)(33656002)(66946007)(82960400001)(76116006)(38100700002)(316002)(107886003)(7696005)(54906003)(6636002)(53546011)(71200400001)(6506007)(38070700005)(478600001)(66574015)(66446008)(8676002)(64756008)(4326008)(41300700001)(9686003)(186003)(83380400001)(26005)(579004); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?MCp8PqdbZiSKO0gyuAhNTQVPeinxg9SBkfncWqITnXnBU7sQ2iXbPjeacy?= =?iso-8859-1?Q?o5y9T4pDr28WuCvYzjLmpm8ra8h/SEomPOmKF2fZ91pRqSrvvq2p6+1IrF?= =?iso-8859-1?Q?UGT56rU4hKkWmQeoYwlVZOvA2cacay0vkhmzg1LdFG0RAqtSEE2DS2w0sr?= =?iso-8859-1?Q?Mqhpv4knZvuYHURpCwTdcgaStqyVN7LGjSd0TWAS3PSQS5MWhClrGuSrQg?= =?iso-8859-1?Q?wGbIYNiTcf9eXW2AtZeBaKR+LwroHfuV8bWKbpp+4+QzIotqCKyE7S22TC?= =?iso-8859-1?Q?RwPVdx7lc7rFsVEyaqNM5bpyny+QTEGmT4wYojnTlWosLjNIibY2hmbWIW?= =?iso-8859-1?Q?UG0JBTwoKTEaptOTLW+uAAGNUoXp58lmss2QofOhcA+nMmL8RAn2M0HI7/?= =?iso-8859-1?Q?0DA/o7/k/Rn8vnzWtRkrhWKAdYKdxkKELfMRBWBpHi9UKBxTUUDVmET8yD?= =?iso-8859-1?Q?+Jx7USoLl2AftgYU7yrbpo53TzLUlaAtwsAm8BqNo71e8aqDWr4RozDZkJ?= =?iso-8859-1?Q?UObPdk6/qvMenWbeJXMmuogY5WUFIPyBqZfAnKCvgshkbvkcjOoAC/MMAz?= =?iso-8859-1?Q?iwf4bF+uhtz9mQDyP1OIfDe7XCijCQpAUisJdFMtNucSAHb20vbCCHPrlv?= =?iso-8859-1?Q?2cSeZU5bJgw8tqMtvI5rQexGF1uDia0AslNJQFGCEwxCv8XtC9UbzPsBJP?= =?iso-8859-1?Q?bQu9qaxHo4gYmcFimk4MpFnCTKft35JKYCH+3QWJNMZV4mr8H/dKmuO8X/?= =?iso-8859-1?Q?nI/HUhrovDGtlz4Pkzrq5B36yKtw0Eaqm4LMEu2OiiC+PFFLXysJis2lG9?= =?iso-8859-1?Q?RxJYtblc9ANm4sasb+uKlIocV5EaWA4hXFZvHcsiThHG1Dmg1rvALGZLrz?= =?iso-8859-1?Q?+eRetqR/H6YSKACq0UKlFO4S6B4lXbPOHxJ+XYbcQp22mlJtl72jjrQu20?= =?iso-8859-1?Q?8Aj4oIL/K98zHBGhG54pK/Tt6XnAJGhahYT87T2HQGS7+Ht8SP1jikD3hY?= =?iso-8859-1?Q?EiENzqj6TypC+A85MbjoVPsjyAjEZ//tjYr+JlKNrt+/YmmKKsVRIk+6KB?= =?iso-8859-1?Q?1eiW5Afq6dN8KQoDbYsJX5MnDYCkhO0I5e6ZFZ2mfiwcyr06VqW25F/+OC?= =?iso-8859-1?Q?3GFGaKqcD3jXLwS0Q+e9110UliKYS/qGzourmkCJBUFXFnKAfs1Y10a6vf?= =?iso-8859-1?Q?8sTQIjo8MxsOS65lq72o3zdedrO1eObi8gKc2ZjegqWkAzMo+U4PFYBbIs?= =?iso-8859-1?Q?VWzt3S+KqyLMCVu+60ftfHlPjqWQKKMOFr32OKVKpqJLmVwu4iY/9MxBbq?= =?iso-8859-1?Q?T1BdidfgJcDpYrXry2ifc2g+isLvz2ZbHo2ub/XL0UbmIjWx2wuS1SDoua?= =?iso-8859-1?Q?XW6iXhtA3UNzIYOQ9gdx7T/2thHUtE/6i9W6ROfeVQNRUM6gn8DlztqjMz?= =?iso-8859-1?Q?d2MmDs9DATxlE00LqPvpM/niO3EBoClxLIFVWqOMsawZFuQmk7PLrU4ZQf?= =?iso-8859-1?Q?DqN2SQqYrEdEa6mUDohWvIgpGYo4rglPec4XPsBBmB5gu06xJZR/uvrHbM?= =?iso-8859-1?Q?lVkm592lcGHSPNk0KJr+YI93VNoYQcnO+faYNH3M6l687KMUcPeMb4mx8U?= =?iso-8859-1?Q?ONvAsRz4zrO1Krp8rAE00xIjmiFiwD6bfH?= Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: SN7PR11MB7019.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 30a48505-943d-4381-4f12-08daf9ed4d9e X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Jan 2023 07:18:03.0664 (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: GxZC/h4mbbaXZ0fo5XxIm61lm1GpO9+ZUUS3XqyqLgdim0QeISzx3rq8/LAKB52uaaLjLa923m0p0TqyKyHDpQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7832 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 Hi Bruce, Replies are inline. Really appreciate your comments. I'll fix it in the next version. Thanks, Cheng > -----Original Message----- > From: Richardson, Bruce > Sent: Tuesday, January 17, 2023 11:44 PM > To: Jiang, Cheng1 > Cc: thomas@monjalon.net; mb@smartsharesystems.com; dev@dpdk.org; Hu, > Jiayu ; Ding, Xuan ; Ma, WenwuX > ; Wang, YuanX ; He, > Xingguang > Subject: Re: [PATCH v3] app/dma-perf: introduce dma-perf application >=20 > 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=F8rup >=20 > Hi, >=20 > more review comments inline below. >=20 > /Bruce >=20 > > --- >=20 > >=20 > > eal_args=3D--legacy-mem --file-prefix=3Dtest >=20 > Why using legact-mem mode? Rather than these options, just use "--in-memo= ry" > to avoid any conflicts. While this is only an example config, we should s= teer > people away from legacy memory mode. Ok, got it. I'll fix it. >=20 > > 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 =3D 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. Sure, sorry about that. > > + } > > + > > + for (i =3D 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); >=20 > Rather than zeroing the whole string with memset, would "output_str[i][0]= =3D > '\0';" not work instead? Good point. I'll try it in the next version.=20 >=20 > > + } > > + } > > + > > + 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 =3D rte_lcore_count(); > > + struct test_configure_entry *mem_size =3D &case_cfg->mem_size; > > + struct test_configure_entry *buf_size =3D &case_cfg->buf_size; > > + struct test_configure_entry *ring_size =3D &case_cfg->ring_size; > > + struct test_configure_entry *kick_batch =3D &case_cfg->kick_batch; > > + struct test_configure_entry *var_entry =3D NULL; > > + > > + for (i =3D 0; i < RTE_DIM(output_str); i++) > > + memset(output_str[i], 0, MAX_OUTPUT_STR_LEN); > > + > > + if (nb_lcores <=3D 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 !=3D 0) > > + var_entry =3D mem_size; > > + > > + if (buf_size->incr !=3D 0) > > + var_entry =3D buf_size; > > + > > + if (ring_size->incr !=3D 0) > > + var_entry =3D ring_size; > > + > > + if (kick_batch->incr !=3D 0) > > + var_entry =3D kick_batch; > > + > > + case_cfg->scenario_id =3D 0; > > + > > + output_header(case_id, case_cfg); > > + > > + if (var_entry) { >=20 > Things may be a bit simpler if instead of branching here, you initialize = var_entry > to a null var_entry i.e. >=20 > struct test_configure_entry dummy =3D { 0 }; > struct test_configure_entry *var_entry =3D &dummy; >=20 > This gives you a single-iteration loop in the case where there is nothing= to vary. I'll consider it, thanks! >=20 > > + for (var_entry->cur =3D var_entry->first; var_entry->cur <=3D > 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 =3D=3D OP_MUL) > > + var_entry->cur *=3D var_entry->incr; > > + else > > + var_entry->cur +=3D 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] =3D {0}; > > + char *args[MAX_PARAMS_PER_ENTRY]; > > + int args_nr =3D -1; > > + > > + strncpy(input, value, 254); > > + if (*input =3D=3D '\0') > > + goto out; > > + > > + args_nr =3D rte_strsplit(input, strlen(input), args, > MAX_PARAMS_PER_ENTRY, ','); > > + if (args_nr <=3D 0) > > + goto out; > > + > > + entry->cur =3D entry->first =3D (uint32_t)atoi(args[0]); > > + entry->last =3D args_nr > 1 ? (uint32_t)atoi(args[1]) : 0; > > + entry->incr =3D args_nr > 2 ? (uint32_t)atoi(args[2]) : 0; > > + > > + if (args_nr > 3) { > > + if (!strcmp(args[3], "MUL")) > > + entry->op =3D OP_MUL; > > + else > > + entry->op =3D OP_ADD; >=20 > This means accepting invalid input. I think you should check the value ag= ainst > "ADD" so as to reject values like "SUB". You are right, thanks! I'll fix it. >=20 > > + } else > > + entry->op =3D 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 =3D malloc(MAX_TEST_CASES * sizeof(char *)); > > + for (i =3D 0; i < MAX_TEST_CASES; i++) > > + sections_name[i] =3D malloc(CFG_NAME_LEN * sizeof(char *)); > > + >=20 > I don't think you need to do this work, allocating space for a bunch of s= ection > names. From the example, it looks like the sections should be called "cas= e1", > "case2" etc., so you can just iterate through those sections, rather than= allowing > sections to have arbitrary names. Good point, I'll try it in the next version. >=20 > > + cfgfile =3D rte_cfgfile_load("./config.ini", 0); > > + if (!cfgfile) { > > + printf("Open configure file error.\n"); > > + exit(1); > > + } >=20 > Don't hard-code the config file name. This should be taken from a command= line > parameter, so that one can have collections of different test cases. Yes, you are right, I'll fix it. >=20 > > + > > + nb_sections =3D 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 =3D 0; i < nb_sections; i++) { >=20 > Iterate through names here, built up dynamically to save memory space. Sure. >=20 > > + test_case =3D &test_cases[i]; > > + section_name =3D sections_name[i]; > > + case_type =3D 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)) { >=20 > Coding standard for DPDK requires this to be "strcmp(...) =3D=3D 0" rathe= r than using > "!" operator. OK, got it. >=20 > > + test_case->test_type =3D TEST_TYPE_DMA_MEM_COPY; > > + test_case->test_type_str =3D DMA_MEM_COPY; > > + } else if (!strcmp(case_type, CPU_MEM_COPY)) { > > + test_case->test_type =3D TEST_TYPE_CPU_MEM_COPY; > > + test_case->test_type_str =3D CPU_MEM_COPY; > > + } else { > > + printf("Error: Cannot find case type %s.\n", case_type); > > + exit(1); > > + } > > + > > + nb_vp =3D 0; > > + > > + test_case->src_numa_node =3D > (int)atoi(rte_cfgfile_get_entry(cfgfile, > > + section_name, > "src_numa_node")); > > + test_case->dst_numa_node =3D > (int)atoi(rte_cfgfile_get_entry(cfgfile, > > + section_name, > "dst_numa_node")); > > + > > + mem_size_str =3D rte_cfgfile_get_entry(cfgfile, section_name, > "mem_size"); > > + args_nr =3D 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 =3D rte_cfgfile_get_entry(cfgfile, section_name, > "buf_size"); > > + args_nr =3D 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 =3D rte_cfgfile_get_entry(cfgfile, section_name, > "dma_ring_size"); > > + args_nr =3D 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 =3D rte_cfgfile_get_entry(cfgfile, section_name, > "kick_batch"); > > + args_nr =3D 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); >=20 > Reword to: "Error, each section can only have a single variable parameter= " > Also, comparison should be ">=3D 2" (or "> 1") rather than "> 2", which w= ould > allow 2 as a valid value. Sure, thanks. >=20 > > + break; > > + } > > + > > + test_case->cache_flush =3D > > + (int)atoi(rte_cfgfile_get_entry(cfgfile, section_name, > "cache_flush")); > > + test_case->repeat_times =3D > > + (uint32_t)atoi(rte_cfgfile_get_entry(cfgfile, > > + section_name, "repeat_times")); > > + test_case->nb_workers =3D > > + (uint16_t)atoi(rte_cfgfile_get_entry(cfgfile, > > + section_name, "worker_threads")); > > + test_case->mpool_iter_step =3D > > + (uint16_t)atoi(rte_cfgfile_get_entry(cfgfile, > > + section_name, "mpool_iter_step")); > > + > > + test_case->eal_args =3D rte_cfgfile_get_entry(cfgfile, > section_name, "eal_args"); > > + } > > + > > + rte_cfgfile_close(cfgfile); > > + for (i =3D 0; i < MAX_TEST_CASES; i++) { > > + if (sections_name[i] !=3D NULL) > > + free(sections_name[i]); >=20 > Two points here: >=20 > 1. You don't need to check for NULL before calling "free()". Free just do= es > nothing if passed a null pointer >=20 > 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 th= at > each malloc succeeds or else the call to "rte_cfgfile_sections" > will try and do a strlcpy to a null pointer. Sure, got it! >=20 > > + } > > + 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] =3D {0}; > > + int new_argc, token_nb; > > + > > + new_argc =3D argc; > > + > > + for (i =3D 0; i < argc; i++) > > + strcpy(new_argv[i], argv[i]); >=20 > 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. I agree with you, we don't have a guarantee for that. I'll fix it. >=20 > > + > > + if (eal_args) { > > + strcpy(args, eal_args); >=20 > Use strlcpy for safety. Sure. >=20 > > + token_nb =3D rte_strsplit(args, strlen(args), > > + tokens, MAX_EAL_PARAM_NB, ' '); > > + for (i =3D 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]; >=20 > char *pargs[MAX_EAL_PARAM_NB] ?? Sure, sorry about that. >=20 > > + int new_argc; > > + > > + > > + memset(args, 0, sizeof(args)); > > + for (i =3D 0; i < 100; i++) >=20 > RTE_DIM(pargs) Sure. >=20 > > + pargs[i] =3D args[i]; > > + > > + load_configs(); > > + fd =3D fopen("./test_result.csv", "w"); >=20 > Like the config file, the result output file should be configurable. > Perhaps it should be based off the config file name? >=20 > test1.ini =3D> test1_result.csv > config.ini =3D> config_result.csv Good point, thanks! >=20 > > + if (!fd) { > > + printf("Open output CSV file error.\n"); > > + return 0; > > + } > > + fclose(fd); > > + > > + /* loop each case, run it */ > > + for (i =3D 0; i < MAX_TEST_CASES; i++) { > > + if (test_cases[i].test_type !=3D TEST_TYPE_NONE) { >=20 > Flip this condition to reduce indentation: >=20 > if (test_cases[i].test_type =3D=3D TEST_TYPE_NONE) > continue; Sure. >=20 > > + cpid =3D fork(); > > + if (cpid < 0) { > > + printf("Fork case %d failed.\n", i + 1); > > + exit(EXIT_FAILURE); > > + } else if (cpid =3D=3D 0) { > > + printf("\nRunning case %u\n", i + 1); > > + > > + if (test_cases[i].eal_args) { > > + new_argc =3D append_eal_args(argc, > argv, > > + test_cases[i].eal_args, pargs); > > + > > + ret =3D rte_eal_init(new_argc, pargs); > > + } else { >=20 > 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. OK, got it. >=20 > > + ret =3D rte_eal_init(argc, argv); > > + } > > + if (ret < 0) > > + rte_exit(EXIT_FAILURE, "Invalid EAL > arguments\n"); > > + > > + /* Check lcores. */ > > + nb_lcores =3D rte_lcore_count(); > > + if (nb_lcores < 2) > > + rte_exit(EXIT_FAILURE, > > + "There should be at least 2 > worker lcores.\n"); > > + > > + fd =3D fopen("./test_result.csv", "a"); > > + if (!fd) { > > + printf("Open output CSV file error.\n"); > > + return 0; > > + } > > + > > + if (i =3D=3D 0) > > + output_env_info(); >=20 > Beware that you have a condition above to skip any test cases where "test= _type > =3D=3D TEST_TYPE_NONE". Therefore, if the first test is of type NONE, the= env_info > will never be printed. I'll fix it in the next version. >=20 > > + 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 =3D waitpid(cpid, &wstatus, 0); > > + if (wpid =3D=3D -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 > > + >=20 > 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. Sure, I'll fix it. >=20 > > +extern char output_str[MAX_WORKER_NB][MAX_OUTPUT_STR_LEN]; > > + > > +typedef enum { > > + OP_NONE =3D 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 >=20 > 2023 now Got it. >=20 > > + > > +# 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' >=20 > Drop this comment. The test apps in "app" folder are for building as part= of > DPDK only, there is no makefile to use. Sure. >=20 > > + > > +if is_windows > > + build =3D false > > + reason =3D 'not supported on Windows' > > + subdir_done() > > +endif > > + > > +deps +=3D ['dmadev', 'mbuf', 'cfgfile'] > > + > > +sources =3D files( > > + 'main.c', > > + 'benchmark.c', > > +) > > -- > > 2.35.1 > >