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 8B5EA45848; Thu, 22 Aug 2024 19:15:12 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3F1F342F08; Thu, 22 Aug 2024 19:15:12 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.8]) by mails.dpdk.org (Postfix) with ESMTP id EDC3442F07 for ; Thu, 22 Aug 2024 19:15:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1724346910; x=1755882910; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=/5d+mqu64JLCWwe870L/wt/YFbqdg4uvfe06/t68oWk=; b=ZJAE6UnwnTZO/Zfg5vpcud4kYZo4w60brhZSFud4CeCdFLi5tsFOgpAq /C2GhB0WvDKm1AEXc55A+y9FeIdacIYhMkS5tfD7fPMza+Tat8TFRcCp0 nmbcKdEsx5UytzIDFXKOcMBi+MLEppImRi6vO9CDhw3zwBa7SBEByli5w 6q4rMR/axcRpcaX4POW/mXePCG7Ng4Tv6czECxeHDg2WYTGWJh3aBmJKR bE3kOuXepXv6ApGU4rJrbrbje1NQqA3hPm3n2awMoIKpXjHmSyv57/Prp Fq1FWHTV1nZUn1gYPIHONKUiMJ7FMBIex1vT5RvLkQnS2My6hLPviZ0Mg Q==; X-CSE-ConnectionGUID: knAt2UTXQEKiEVr4koyNoQ== X-CSE-MsgGUID: WQREjlpvRDGdQZJDtSEkkw== X-IronPort-AV: E=McAfee;i="6700,10204,11172"; a="40293359" X-IronPort-AV: E=Sophos;i="6.10,167,1719903600"; d="scan'208";a="40293359" Received: from orviesa004.jf.intel.com ([10.64.159.144]) by fmvoesa102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Aug 2024 10:15:09 -0700 X-CSE-ConnectionGUID: OIN/YGFHQVuH+OTj8pKWtw== X-CSE-MsgGUID: w3brGTXfRZy2yBWqAl/18A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.10,167,1719903600"; d="scan'208";a="66448476" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by orviesa004.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 22 Aug 2024 10:15:09 -0700 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.39; Thu, 22 Aug 2024 10:15:08 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) 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.39 via Frontend Transport; Thu, 22 Aug 2024 10:15:08 -0700 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.47) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Thu, 22 Aug 2024 10:15:07 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=WTYJfwSg3kvNc/YyYgpawKmEOuLo/AoiLtqe+EQrJL0vXLDfqK1+chNYLs954yUpcoFbvD1SjWbyBmvDhbbiKbyn9eKRNEBxqySRu7PkBMzUcSUW5tkFA9RBy7af59i4h2Bll1oS2XG/W1XlMIRKQE0a0f8QOaIReqz4PcdqoBeht2jFcfXYjy9rxjFsDQDYWf/CIBuLK3pYw715HX+1bke4Pra59HHkzYcAymZg8D5pIy5oPi8Cg1pAgczDnzrRqtcU7rGw5HFcPvTeNaNfsNdqJ65pkxaYNNSswofIldf/p2N9g/PWyQKUDBAkNCkud8PSuR3Fm9jO5gUhosrnNQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=rWxsxl82fsuOYqcpi/0l5wYS0tdg/2bVVePYxZNU5v8=; b=XT8Uuf0iVgmmHr8RzMtKms4I6cZ/Fo9osrtBVTZk4FWS7IrqtNI9CnHyhRV+O4gxz7R0rwf62UgoNCTn7DfLEAx2Nkq5VPUgWzbfamfHVQh4t3I3M7i1AAheJR4jCUYi06q8+ZxqQmXlxGdCfotUZXS5slm+PiqOccugZBd7bENmtunKGA0t58cP3S7WC1D+TSeHsUnKgVLSwWrPfClrrf+hk4YjR/QJgel9yWDYuvUymSxVXArlz6GLYZNU3WusBDJXlSAVmpOWPeELvOhxn3SeyFehzgSN0/Ym8+6PikZIENYUkitcDcHDLy7n37ZZGcqnuf9Bx/6LWpEa9lItqQ== 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 SN7PR11MB8261.namprd11.prod.outlook.com (2603:10b6:806:26f::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7875.20; Thu, 22 Aug 2024 17:15:04 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b%2]) with mapi id 15.20.7897.014; Thu, 22 Aug 2024 17:14:59 +0000 Date: Thu, 22 Aug 2024 18:14:55 +0100 From: Bruce Richardson To: Ferruh Yigit CC: Subject: Re: [PATCH v2] app/testpmd: show output of commands read from file Message-ID: References: <20240822103604.113246-1-bruce.richardson@intel.com> <20240822104109.116208-2-bruce.richardson@intel.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: X-ClientProxiedBy: DUZPR01CA0112.eurprd01.prod.exchangelabs.com (2603:10a6:10:4bb::11) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|SN7PR11MB8261:EE_ X-MS-Office365-Filtering-Correlation-Id: 53a050ae-6066-4d05-cb47-08dcc2cdf3ff X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|376014|1800799024|366016; X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?6qJ+GnjToUwLDqH2W1a53Ctx/ZDyyW1VHfmq3Xwk8CB7ebhXGEKswF1JKXxD?= =?us-ascii?Q?mQWHCGS+r+ZEFZY/MFvmwtFPs670oK6ql8LPKjwOyQp7HMb+Tp3o1+vZ320L?= =?us-ascii?Q?fMamlWPIn0aLiSXXLvONgiiHX5nBEd+QzFkaT0Fr3Gwf7GtbJyHXMj8b6Qtc?= =?us-ascii?Q?KophLT4zrSb51KSAZuOynLVJGKx0eGfouqFdhdZ1cVhwU9AnDraWGgcQEJVr?= =?us-ascii?Q?F/avuj44HNcfvcQOAYhJoqiC9DAg73+0DvDSqfsWCUZJXLYnb9S6XugGspZF?= =?us-ascii?Q?3UbjBBPijA9DNo1nWt4c4L3p33td1YxbEKwXtaohP9RSEXhXeQEkJSAvtqH8?= =?us-ascii?Q?2NLdFwCT1R849+eLwxtC+mKf34VyHYm16+yurljwZNalQE+x7dgnmq8aRcrA?= =?us-ascii?Q?f+5yPu+SPgZPdS2SpfO5MsRSKk8kcVAXQCzHiQ2oPxT/X3owbbB5M20zN2Dr?= =?us-ascii?Q?o6A9PCr11KJgqUl5Kr8MPUMsbxDH4kD4io6+LAGoCTJ0z8EZypjc8mhnUOmx?= =?us-ascii?Q?H/KJTBlp6YsyjsdvDJ4KCjl99rlqOaexCxJ9PrfeBIH/k/3Q2hOXJnp9yH+7?= =?us-ascii?Q?Fqun5C0hz3pEDtazj3IIkiryZOQnLNtcWTy8SvCn17K2dpXGGmUNPpnr0q8o?= =?us-ascii?Q?vOYkNeIvi4ZtwxccW6qD/r84qiK2eVHN83G3UAkaMTt9KjTxX6qz7JUOtoC/?= =?us-ascii?Q?GcgAgdNwJs4mGTENiI1POM/E5GQruOm6v/rD9/YAV+VOU6f47OHqOmHsdSWo?= =?us-ascii?Q?ULL1EHfnYFG7l08I3J/WjEW6sMh9FmJX+OiU8N9TakzZE8j6lS2aUpBG7HAu?= =?us-ascii?Q?qftYpxPhubHF1T6CeRqBD5U7DYKkMFWZ3UBTVWbqzTB8QvM00p1EKWjgD0MK?= =?us-ascii?Q?zoSPMi0dAB9/NYO4aznpUSj+5v6fW0pdjgXTpETyJUocoJ7Kt8aSFDVCghjo?= =?us-ascii?Q?8oGV8LN0iZi5VuICk0wLYUALMvuCzra0Rd656IhrfgsOmStIv/q1Ce2mHAvN?= =?us-ascii?Q?qT1Ny3P6vyz2bMHYb6uPiVM6weW/msLBh3s2Yfbfx3FLMOWH3UF0Z6Ire85K?= =?us-ascii?Q?BIlkXXVyCgMgNoc1JoRyMvOluiQ97Qr/D1XI6nBG9Jxd84xgkMRtlWw5SqVL?= =?us-ascii?Q?pZcBwAphf6p38UZIupFg+pzDmRyEqs56xaKaN7OHL7H9OJOpxQLduIjnpyYW?= =?us-ascii?Q?dM4FZ+vSGcKuFK2HjqNWyYhSADfN5mJUr3ULhtmf349j+vsQVS6/p5J//Nyo?= =?us-ascii?Q?+VKtTOsxUHD1o1nE3o3Zw6cvTAcDkbAHnkXkwiNvFv4RJA/+GRX55cHh2NE3?= =?us-ascii?Q?j8cOVQCQ4O8JQ4MSFveVZQrviGxDL76XZz1BifEzKlQ0IA=3D=3D?= 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:(13230040)(376014)(1800799024)(366016); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?BgVQ90+JYfg9QE8tr7vMIOdzvYX4RIYxMZJBVxwxE0MzsRg6a/r5hI00qMyj?= =?us-ascii?Q?eusC7zxfGj14ASAqrA2zHr5AEyRjxp3FSOAh777nRlSOTHAOfBDeGZo9zoSg?= =?us-ascii?Q?GMDvPr/RGjcKClV0UEPgfGu/qBgL+uDuucapU9DaS8Qhxz5G+GkiMgjhY6+l?= =?us-ascii?Q?C6P2I3HMdwt/Zp7aSLCIApssDGkD8eWnAsMa6nfC9V70Qvfem444q0plCVOg?= =?us-ascii?Q?7mOnl22CLOPl1O2cwX+a3pzpmkTTYaMywjc0nRA0aCABJah+Jvz8ADL0Nmft?= =?us-ascii?Q?VKZbuaYfl07RTStq12c/WNfKWxhW6NT1GDHWBLFN8QTcI48aaodbVyrZLlS3?= =?us-ascii?Q?EBQt6xafHLWW+U/KgBU4aHlIi4d/s401AU32uy5CcmxQ0wkbxdSRUpOh2gU5?= =?us-ascii?Q?fbpU9nKNXwVhTijdjLuRmlKEuoHx/vAcU1pwArWyoMK2a8nSOChD33A6KY2O?= =?us-ascii?Q?kKbo5lSxu05QqNhOKI7hq9iLB7FzY2Po/R4oFoLywPC7hHYtIQu11vB7OJwv?= =?us-ascii?Q?U1OgDZ0+xXhhPmZX2P8yJwYIwYVfXEaC5566Qx6lTMsPfiwVSC4JPIbLDN6e?= =?us-ascii?Q?3p+k4O97ZmHpaOTixlSFqqy7WwSI4oy61SxQTOQtuUHNi/ihTp6lEE2HwKjb?= =?us-ascii?Q?b+hAxTDXe9lJsNVwkS9k37vTQRjlAwI/z+HXRMmSBFspVGBdggNV/fBIJQJj?= =?us-ascii?Q?7WX3qYsp6e1TLPAO8arT5jljyY3c/1H0M6m3jsIVawEUeCbY/x1pFZc4DZYK?= =?us-ascii?Q?f6hN2+Dx9OYV6fIXFxufxh3EYkjnqtMuz5wyP0+Fm5E0vDdO+fWVgNCK802B?= =?us-ascii?Q?gqYPMG1evHeuERToSQj72Cjfvr9juwFYdNpKAAIAufqM5jOMqHGioikPhyq8?= =?us-ascii?Q?HiUVfNwT6cLN6RJjNb360uQNoQZ67WPOOhPg9HW/vthVAlIkNmbVKHDRFCQa?= =?us-ascii?Q?BTvh1DBO52Z6RoLFaFocS51UlRTpjaTalBVPJDFUvdLtTvkhOLoHSCRHWRPr?= =?us-ascii?Q?VZpBI4g/DxK+cXBiLl9BxLhrGq9ocf6MHyk3AbzGZtiVD4YGhZLnJN9C72Xi?= =?us-ascii?Q?vhyOowt4zxfWva4qStIcuYcseinsqXp052nNeWXrJKQmmLruIrrVa+wu+Kmy?= =?us-ascii?Q?kTPHjS4yjKgkyjneFoTmk/8Elg3ojcK+FSpWO/tGUiEZmg9dp2jr4MG8v9jv?= =?us-ascii?Q?/w1qgeLA0PRvq0lP6Lu8uE+nzbBrLdDOlkFHUfye/ULRKMt+lzSLJE/bMIgm?= =?us-ascii?Q?FMH8V2XraScC+iCTVY4gvzkv9jwEzRRZtJ0ELhMzYzpfqNsdSAfN8+LEvWPx?= =?us-ascii?Q?z/yZz3Rqv2hs9qVGM49u99tmCKO3BZJLVefnfZcM+pqSLfEVPwQLqimk0GMO?= =?us-ascii?Q?jZcPTYGvS4kVJ5YUJoCbGHwU2d9LKfw/6qa6T7lWNrAZ99jtaqZyR5pzTzYq?= =?us-ascii?Q?QY9Hl5gvRJM+bysJn3BoWUR3JOBtZedWfDg5emcPWoCBIgeQ37URSuUB53jH?= =?us-ascii?Q?mOLg1WRxS1hbR0OUawqI9LshxXyDv4cOcvyWfgPcGhHrTVZkjgUjZEYkeyj7?= =?us-ascii?Q?k2jJPHGmXupag10GPgD2nAzHAjL3OspuuIDXipz9wjSH+0QUyOwQILn6apbp?= =?us-ascii?Q?KQ=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 53a050ae-6066-4d05-cb47-08dcc2cdf3ff X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Aug 2024 17:14:59.8457 (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: QXoaI1lzXM2AUJAclhD3VHcA+9jmtVsCDksRXkmu72jSp7jMbs6wC09TbBO1u1hjiQxc/PC6gkytZk5rLeYlRmpQETcKmYztCQKNd/ZlXsQ= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB8261 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 Thu, Aug 22, 2024 at 05:53:27PM +0100, Ferruh Yigit wrote: > On 8/22/2024 11:41 AM, Bruce Richardson wrote: > > Testpmd supports the "--cmdline-file" parameter to read a set of initial > > commands from a file. However, the only indication that this has been > > done successfully on startup is a single-line message, no output from > > the commands is seen. > > > > For user I think it makes sense to see the command [1], only concern is > if someone parsing testpmd output may be impacted on this, although I > expect it should be trivial to update the relevant parsing. > > [1] > Btw, I can still see the command output, I assume because command does > the printf itself, for example for 'show port summary 0' command: > - before patch: > ... > Number of available ports: 2 > Port MAC Address Name Driver Status Link > 0 xx:xx:xx:xx:xx:xx xxxx:xx:xx.x aaaaaaaa up xxx Gbps > ... > > - after patch > ... > testpmd> show port summary 0 > Number of available ports: 2 > Port MAC Address Name Driver Status Link > 0 xx:xx:xx:xx:xx:xx xxxx:xx:xx.x aaaaaaaa up xxx Gbps > ... > > Only difference above is, after patch the command itself also printed. > > That's because the function uses printf itself, which is actually wrong. Any output from a cmdline function should use the "cmdline_printf" call which outputs to the proper cmdline filehandle. > > To improve usability here, we can use cmdline_new rather than > > cmdline_file_new and have the output from the various commands sent to > > stdout, allowing the user to see better what is happening. > > > > Signed-off-by: Bruce Richardson > > > > --- > > v2: use STDOUT_FILENO in place of hard-coded "1" > > --- > > app/test-pmd/cmdline.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > > index b7759e38a8..52e64430d9 100644 > > --- a/app/test-pmd/cmdline.c > > +++ b/app/test-pmd/cmdline.c > > @@ -6,6 +6,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -13431,7 +13432,18 @@ cmdline_read_from_file(const char *filename) > > { > > struct cmdline *cl; > > > > - cl = cmdline_file_new(main_ctx, "testpmd> ", filename); > > + /* cmdline_file_new does not produce any output which is not ideal here. > > + * Much better to show output of the commands, so we open filename directly > > + * and then pass that to cmdline_new with stdout as the output path. > > + */ > > + int fd = open(filename, O_RDONLY); > > + if (fd < 0) { > > + fprintf(stderr, "Failed to open file %s: %s\n", > > + filename, strerror(errno)); > > + return; > > + } > > + > > + cl = cmdline_new(main_ctx, "testpmd> ", fd, STDOUT_FILENO); > > > > Above is almost save as 'cmdline_file_new()' function with only > difference that it uses '-1' for 's_out'. > > If this usecase may be required by others, do you think does it have a > value to pass 's_out' to 'cmdline_file_new()' or have a new version of > API that accepts 's_out' as parameter? > Yes, I thought about this, and actually started implementing a new API for cmdline library to that. However, I decided that, given the complexity here, that it's not really necessary - especially as there is no clear way to do things. The options are: * extend cmdline_file_new to have a flag to echo to stdout (which would be the very common case here). * extend cmdline_file_new to take a file handle - this is more flexible, but slightly less usable. * add a new cmdline_file__new function to echo to stdout. * add a new cmdline_file__new function to write to a filehandle. I don't like breaking the cmdline API (and ABI), so I didn't want to do either #1 or #2, which would be the cleanest solutions. For #3 and #4, naming is hard, and deciding between them is even harder. Given the choice, I prefer #3, as I can't see #4 being very common and we always have cmdline_new as a fallback anyway. Overall, though, I threw away that work, because it didn't seem worth it, for the sake of having the user to do an extra "open" call. > btw, I recognized that 'cmdline' library doesn't have doxygen > documentation, which is a gap to address. Next time when someone asks > for entry level task, we can point this one. > Yep, good idea. /Bruce