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 5D8F741C5F; Fri, 10 Feb 2023 12:55:47 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4E8DE42670; Fri, 10 Feb 2023 12:55:47 +0100 (CET) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2056.outbound.protection.outlook.com [40.107.243.56]) by mails.dpdk.org (Postfix) with ESMTP id A08B342670 for ; Fri, 10 Feb 2023 12:55:45 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RNSyV4fDfGL1XSSCT69UOUwbO7SLZ0Ni5CjJ5dA6wOge4UVwYhPmm8Vpnven1xZ2H9Ddcz+QZ6U+l0SUB9t+1l7Cqnlb0j3Y9jNpfdC8N6c3ih9dx9HZ3oBhzknK+7nKZYerXmk2OFyKCxoVebJl7ptd9fum9UqyLRcz+nrV9gJ8AeMQjaMRbub/+e+ykx+r16AhuArddR9O/X6gUH5xtBRLxkV2jagrTsQ1jAnDYsmFaTygHPI+338+4SpoISAmx6anPlQ5ac9OFzMm58cGwrMR0PYKVlbQ6z3p5oL2Yfwz+GaDG78bVW0jv99unWNDV/ZTkzPs1auXngqx31eTew== 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=yR26rhAchobxu1uDQV3JUW2XP2/RcortaCPAePcbVMQ=; b=DcoCQFh368U2GnbPjzHOSs3SOtbf4dg8kE/RIrZ0TwxestStvVtknjdez/6KdTQvd9ODIl3+RVDfr00op4/EJkkw3GLkSPi24nHgNzjqwOe4GEt9VNFnJPFUWd1OWZshEpTcSYR7wt6Y4qo+69gq/oj+xRp+XivNOlZ7I6ue5GocpD4KT45vi+WF3wo6W5A4Whw2uhSFrb6edTfzI/BZ9dppa3z9HgHCQxRdzF8tv/d1Ybp/RvEeHTf+aBLmTgF2vz9Q08k0ghcqVrGG7uhF1Z10WUuhqQUUH26HN5B13VTh2DaxdC3twgNoZUPrw7UMzmv2A93R2b0rN7P6FO+xBw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=yR26rhAchobxu1uDQV3JUW2XP2/RcortaCPAePcbVMQ=; b=iKIKP2jG/9Ovp4JCkhRVX9XCKpt/dg5/WdezY7L4ZBL3qybeFrBCXap/WLzsuAeN97sbKfwr6o1cAXhOBpexcqcXJjyn4C//c3wiFegkGcEL0afQxcrHKE8nQr16utjD9vNqc7b2Y2XZGdYyh14PcdYGpQJmdarQmQt7NXrY5Ow= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by CY8PR12MB8314.namprd12.prod.outlook.com (2603:10b6:930:7b::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6086.21; Fri, 10 Feb 2023 11:55:43 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48%8]) with mapi id 15.20.6086.021; Fri, 10 Feb 2023 11:55:43 +0000 Message-ID: Date: Fri, 10 Feb 2023 11:55:37 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH] ethdev: telemetry xstats support hide zero Content-Language: en-US To: fengchengwen , thomas@monjalon.net, andrew.rybchenko@oktetlabs.ru Cc: dev@dpdk.org, ciara.power@intel.com, bruce.richardson@intel.com References: <20230209030755.48028-1-fengchengwen@huawei.com> <5e68e212-4391-e32a-91c1-2fd1874e758c@amd.com> From: Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P123CA0329.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:18c::10) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|CY8PR12MB8314:EE_ X-MS-Office365-Filtering-Correlation-Id: 1b3146d8-0789-419d-a061-08db0b5dbcc2 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 68n2x7xnirp7UfCCcjXBgZdKcDsxSNc2kqkxC8cqqxCE7tiTyoeK0YAT3uWf9sUqGZN5oZwixKUJzQbGuw9LoGp5nY56eSkrr9ahDB2atYTaebpcEC7T9aTvITMXtBfjidT1kSbzDhbENFbUkCc4pfiScAbEMrCTy7hVTzG/urpyA9PW2r1WlhFYAF6gaoRh9dEx0zKsODV53mKO8TLQ1os12KhJ7EqZ/ITdLEkNgCa2OhBfVYtBFcYJO5Msr+KAVQyHjE7x8tlAXg3lZFpqG13s5zh5cvp7I1I+7bAqnjB68IHCykqYH6dHmYLPc920bWriTImxNrQSjwjvaLa2O6rGmJH5fX+LvMGjnf0aXRIbJRZ6bozBULY/zHNUT8ZuT6Gt0zYbUWswufxexCLr/ws/pST3EFwbT6l7cd2fRiaI7C3ChvlNQNgy9r1SSclj/Bd8X8tGy8oeCrn8sqJ67ZSGObMaG7zJ9U2RXHZskBg58LtEBZPXwLaKobhS749k4vqRCMgKP3HKnV+aT58M4YUl4reQ3BpsT4dz7tvJwYugpPzlSVZhqUMRue6lo3OQoc6iRwO1JCgzRvKgyX2PuwMsgWX+/+pnYeT/j9KetWhhHKrepeGAL2D1zmfWAn36XXwLI2RR9UIZB0xiGbIxPbAHlfORg946N0aYMJca43bh0muBev5ocHSvZQxbRXVn5YSIikUSMzZEe6p9G3Xadm62U9FQ5LSVDaAjN3k/rTM= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(39860400002)(136003)(396003)(376002)(346002)(366004)(451199018)(31696002)(86362001)(36756003)(38100700002)(66476007)(66946007)(66556008)(41300700001)(316002)(8676002)(4326008)(8936002)(5660300002)(44832011)(2906002)(83380400001)(478600001)(6666004)(2616005)(186003)(26005)(6512007)(53546011)(6506007)(6486002)(31686004)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SlQ0NG9ybEQ2MzlGSzNBZ3ZWY1N3NFBZNFdNcjduOVJ2M1lBaXlCdWNNUllZ?= =?utf-8?B?ZFZwR1hUU2VrWFlSS2tVc2dGN21UWmF1bVZTaFRDMWROazJlZHhNclFyMWJw?= =?utf-8?B?dFZXdklzT2RPLzJuSEozSUhlNHRSOHRsNEJ5UllYREtRN3VxSDc5TXdzNGpN?= =?utf-8?B?cWRjU1ZadnFmSWVHNGxMY0FVaHNySlFnQk1VRzlsYlVxMnhzSDZUYnkxWGk0?= =?utf-8?B?MVZYdjRxMGF5RFVrN2hFT3NYTjZCVDd1QzZwWXJOTWRHSVljSXBDNXMxd0ww?= =?utf-8?B?VlR1VSs2NGE0OEovWDVoeko5cjZ4anRjWTA2S1FxVU9qc282RlhSZHI2SlJP?= =?utf-8?B?cmtxczFwTUpLT3hUdk1HSkE4MkVFWjNtK0MxQmRYMFRiYVhNOXRXbkR4OTN2?= =?utf-8?B?RStqWUhUMDBBYzYzR3hFeis1cWFPRWc1dlczTnR3UGdFejVVd1pwdk95QnNi?= =?utf-8?B?Y2RDZ082T0JWdEJDeldGUzZnaW8zU1FsRmFyaFd0VURQMmpZN3NoTzlPMDAv?= =?utf-8?B?cFU0ekx5UkM4dmF6VzF0VU9xczlPU1ZhTUU5SE1MSXBOTGI1cUdMek5SUDRM?= =?utf-8?B?dTY1TGRXOE9Gc2hna0tlUVoxbHdZZDMrUUQxUlVUOGZERzVZelZqM3dBRDBV?= =?utf-8?B?Y2VOaHFGcHdxK2ZuY09LVUFSTWxHeWJsMVNWbUV1YTJXcEsxSDVLTmNYNkN6?= =?utf-8?B?ZjdOcU5tZlFQbk9tMzloWFgyNDdUNlRnTXRjMFhwSVhQMzFnMElVNkpHZmp5?= =?utf-8?B?VEZVMGpoYUpzdWNmaGc1WjMzUk9iWHZOOCtKUnhWRHNUaUo0dGJPR2VldWg2?= =?utf-8?B?WjYyYjYxdnh3bGIzbkQ2RlNDOUdJOTlLN0IrZU1XcjNjVjN5YmRyQUIxM09G?= =?utf-8?B?dlc2RVA5SU1DU1Z3V3IvR0o0VEpkQ012eGxoVFpUd3c2bGQzZU95ZXdrWnVG?= =?utf-8?B?ZncwOGdub2J0Z3NMOXhRNmhIRmxDN3NLTm9qR2lwTHNIeitoVXJVcEtKS1o1?= =?utf-8?B?MngycWxJaGI4dXVqbmxUeDhIY0MvcWp4eWRXcUl0b1o3ZVhNRHpkUkczNEZK?= =?utf-8?B?Z1RZWWEzTDRpbXZOeGo4NHhMZ2thTTBqZUJSaEpZSERlcFVXYStiQnBsOFlN?= =?utf-8?B?VDhZNk8xb0lIL2MzWUNLeU5JQnYzMWlXTm5wY2pNT1BWbnZMTURwVDFaOHFp?= =?utf-8?B?NDB2ZVdTcEduQ05nVHl0VU9XSXNUTGZNZmZCYjFFdC85NU9kcWxUSW90MUsz?= =?utf-8?B?SFk3SVJYVGJIUHdKeEVBWXJBTWJqeUxHNWVwY0pOTjAvRkRZWTJnUVVDcDFF?= =?utf-8?B?Q25Cd2t2Rnl3NFJIQ21BNVhqMWM1Q0k5N0RhYmlCK1dFZEZqdnEraGNBK3ln?= =?utf-8?B?bUd3Zm1SdkIwVmdmckgzNW95ak1WdWVUTUVwMjNwTW13Ni9hQ1hyN0xWNFpB?= =?utf-8?B?Mi9mQldnWjJkVEdBbitSaktQV1hFaG9BUFVzbzV5LzNuMWdMVVdPY0t2M3ZY?= =?utf-8?B?TVlVM2Z3MXZURVZCSWJRR3lWV3Zic3FhdTBpdzhUeGQxOVdteE0rSGpHT0RQ?= =?utf-8?B?OUlaNFprWUZ2MGJkZ0FKY3ozVXFEa2kvSVNrdFU2L3hyTDd3MC9KUXhpMWkz?= =?utf-8?B?eGVVRHQ2dkRqL282bmNQZXRTV3hpSTRPRitIK2hQYzdTdndJeGprKzRoZkZ1?= =?utf-8?B?bk1kdE1Ed0VjTEk1S1k3dGEycElzUVVTT0cxNERndkIwUHppUWZrYU5hRlJn?= =?utf-8?B?aklXRHVrdUsxcktQa0VDdWFtd2lTaHg5YmdxQUVtTkV5bEM4Qk4xM2tOSzVx?= =?utf-8?B?U3l1RzIyUHo4ck4xdm9PaUZiVXdxQkEzMDFmMmpZckRFV2RFcTlhcEhoSjB3?= =?utf-8?B?WDBoYXFXaFpyYks3NEswMDkxdWlDSUJtYm40SEU0STFSYzZhTElMVkVTYUhB?= =?utf-8?B?emdrQitpMXZHUE5SWGF2VjNrc0wvQ0YvWE4zeXR3Nnp2eU9jc0dESm4wc0Nl?= =?utf-8?B?a0J4SldycWxYaWp4WFM0SENTWXBtSzJFU3NiMHU5TElLWDlrVU01NjM3RUV6?= =?utf-8?B?S0tBSVU3aWZ6TnlnWk1UTmxkV0YwZldCbTQrRDB4VHVpSGNrZVF5eUdPMzB3?= =?utf-8?Q?Oh7+klWpo7TfsqmaWtu96YEMZ?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1b3146d8-0789-419d-a061-08db0b5dbcc2 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Feb 2023 11:55:43.1667 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: iEj7wqKknBTL89KVWIWvtM8krR21G1PpjM4ZvANm0cY5+fJ2WruT2bZBILkuJwRy X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB8314 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 2/10/2023 1:23 AM, fengchengwen wrote: > On 2023/2/10 7:30, Ferruh Yigit wrote: >> On 2/9/2023 3:07 AM, Chengwen Feng wrote: >>> The number of xstats may be large, after the hide zero option is added, >>> only non-zero values can be displayed. >>> >> >> Hi Chengwen, >> >> No objection on the functionality, we have similar config option in >> testpmd too, but I have some question on telemetry side of things: >> >> 1) optional parameters, I don't know if there is a defined way to handle >> this in telemetry but with current method only one optional parameter >> can be supported and it has to be the last one. In the feature if a new >> mandatory option required, this changes the user interface. To prevent >> this, is it possible to use "key=value" syntax, like >> "/ethdev/xstats,0,hide_zero=true" >> >> This way order or existence of multiple options doesn't matter. > > Yes, KV (just like PMD's runtime parameters) is more flexible for multiple optional parameters. > AFAIK, only some dmadev commands have optional parameters (which using this patch way to identify). > > Until there are more parameters, I think we can keep the status quo. > I think better to start using it with first optional parameter, which is this patch, otherwise it will be pushing the work to next contributor. >> >> >> 2) Where should be this functionality, it is possible to filter out zero >> values either in dpdk side or telemetry client side. >> Just for this one I think both options are OK, but if there is a >> possibility to have multiple and complex filtering, I think that should >> go to the client side since this is not really task of the dpdk library. > > Agree. > This patch just target who using telemetry.py directly, it's valuable in this scenario. > If clients supports filtering, they could use original way (don't add options). > OK, I don't have strong option, if there is no objection we can have this functionality in dpdk side. > Thanks. > >> >> >>> Signed-off-by: Chengwen Feng >>> --- >>> lib/ethdev/rte_ethdev.c | 23 +++++++++++++++++------ >>> 1 file changed, 17 insertions(+), 6 deletions(-) >>> >>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >>> index d25db35f7f..8f79ae45d5 100644 >>> --- a/lib/ethdev/rte_ethdev.c >>> +++ b/lib/ethdev/rte_ethdev.c >>> @@ -5870,20 +5870,28 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused, >>> { >>> struct rte_eth_xstat *eth_xstats; >>> struct rte_eth_xstat_name *xstat_names; >>> + char *end_param, *hide_param; >>> int port_id, num_xstats; >>> + int hide_zero = 0; >>> int i, ret; >>> - char *end_param; >>> >>> if (params == NULL || strlen(params) == 0 || !isdigit(*params)) >>> return -1; >>> >>> port_id = strtoul(params, &end_param, 0); >>> - if (*end_param != '\0') >>> - RTE_ETHDEV_LOG(NOTICE, >>> - "Extra parameters passed to ethdev telemetry command, ignoring"); >>> if (!rte_eth_dev_is_valid_port(port_id)) >>> return -1; >>> >>> + if (*end_param != '\0') { >>> + hide_param = strtok(end_param, ","); >>> + if (hide_param == NULL || strlen(hide_param) == 0 || !isdigit(*hide_param)) >>> + return -EINVAL; >>> + hide_zero = strtoul(hide_param, &end_param, 0); >>> + if (*end_param != '\0') >>> + RTE_ETHDEV_LOG(NOTICE, >>> + "Extra parameters passed to ethdev telemetry command, ignoring"); >>> + } >>> + >>> num_xstats = rte_eth_xstats_get(port_id, NULL, 0); >>> if (num_xstats < 0) >>> return -1; >>> @@ -5908,9 +5916,12 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused, >>> } >>> >>> rte_tel_data_start_dict(d); >>> - for (i = 0; i < num_xstats; i++) >>> + for (i = 0; i < num_xstats; i++) { >>> + if (hide_zero != 0 && eth_xstats[i].value == 0) >>> + continue; >>> rte_tel_data_add_dict_uint(d, xstat_names[i].name, >>> eth_xstats[i].value); >>> + } >>> free(eth_xstats); >>> return 0; >>> } >>> @@ -6328,7 +6339,7 @@ RTE_INIT(ethdev_init_telemetry) >>> rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stats, >>> "Returns the common stats for a port. Parameters: int port_id"); >>> rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats, >>> - "Returns the extended stats for a port. Parameters: int port_id"); >>> + "Returns the extended stats for a port. Parameters: int port_id, bool hide_zero (Optional, non-zero indicates hide zero xstats)"); >>> #ifndef RTE_EXEC_ENV_WINDOWS >>> rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv, >>> "Returns dump private information for a port. Parameters: int port_id"); >> >> . >>