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 CD5FEA00C2; Fri, 6 Jan 2023 18:33:19 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 859F340141; Fri, 6 Jan 2023 18:33:19 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id 6C202400EF for ; Fri, 6 Jan 2023 18:33:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1673026397; x=1704562397; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=xTvjgG7BMlCU6Rns5ZlJ0z3MPMq6DV/ZFqth1tlFSYE=; b=Q8oj5OuMiUUTSOeW8ml6+PEiDjzU9ZK5Xp+ME+5JmQFJkGVquZnr9Qf0 ZekEWWnbMFuqnMxt7JpVhAsHsYL7WETpyW+oGCcYsQVk4euq+xLbESWr1 bqeo8waTeAfp46hPDRkpYy2N5d+niWcmaeCkgLC7855663h6UbC6zX8AP UPMSOBhIDS3iVyjEs6CCGbKmXUQZW0Yr1npRjWuwx7vxBxwE8ONNqw22D zp2rIe8xuk2NBk46zySt97f60Uuc1ZCkfDc0a4ariHK6IPu4wyTf520q3 aR7WmKgry4rWi6yrWAcEaNybVI1Lp5cw94v20i26sl+AjGoGhlg9+mzoH A==; X-IronPort-AV: E=McAfee;i="6500,9779,10582"; a="306037936" X-IronPort-AV: E=Sophos;i="5.96,305,1665471600"; d="scan'208";a="306037936" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Jan 2023 09:33:16 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10582"; a="829972152" X-IronPort-AV: E=Sophos;i="5.96,305,1665471600"; d="scan'208";a="829972152" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by orsmga005.jf.intel.com with ESMTP; 06 Jan 2023 09:33:15 -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; Fri, 6 Jan 2023 09:33:15 -0800 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.16 via Frontend Transport; Fri, 6 Jan 2023 09:33:15 -0800 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.43) 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.16; Fri, 6 Jan 2023 09:33:14 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=E6vYFI6KQ7LkcJh2EU7LALP5hOx82qTvhxtyP36zrMg0zEjYfi7h/jTcpu8p368IydYAizevdGEOHmo8gq4qhINshu00uiVsBepwR/rWzF6lOp3ZMy7QSnnhS1uUMEKZjlnZoY1O2zl/+Hbfq3dPt9RA9Udhznsz1EGMe2rUSBZgn7XbOhTPBuaiqrQzsTi89J1KUn+zHUPvmjhC4rZ/8ZommkQy2GOyr96FyFPAO8YDCcrTmM39mFxwcS6WxRXgjoS1OIdOHFNZfVnD+WdodHZVqxOnoSpdNfAsHdtqxB+0FtYjlBwQAfK10Mvo3D2ZMjIII6Ph4sS4CfYxBWmXmA== 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=0+T/GYJmHlMDCxTDtVG0byex+aB1pOB6RDAvCRbj7m8=; b=kvzrQn9nYx+UyELlD8QbwBMQ7jne1L/oZkzNk969LkPj3xYlTPaGslVwgjP/rZl+sV3pgInEXnjYic4zACp3Jb91a58Yf5ECG6vhiq3/5FWpBEZMux2OcXenw5+7HG3Bs6MqNfBNAe0ZxnAycfJXgQ2xylL9pfP1GgFbKVBgt9Qz55S5Mp0q5zoP6O93EmsuygV0atEN+4hDgzH+RVgjYTkamTBgHUBymwF5hT+Z2ubH3Gi5tbY2Z5IeSWmTZeB90Jo7ZuxFXw/7xsbGhCFLwvKKJaBc3KxW6NSokgxe8mCtPXqPNuUXXBKEmKzIOYNs/TBktDk5ylm/C9lp1WVkWw== 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 BL1PR11MB6002.namprd11.prod.outlook.com (2603:10b6:208:386::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5944.19; Fri, 6 Jan 2023 17:33:12 +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.5944.019; Fri, 6 Jan 2023 17:33:12 +0000 Date: Fri, 6 Jan 2023 17:33:04 +0000 From: Bruce Richardson To: fengchengwen CC: , , , , , Subject: Re: [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict Message-ID: References: <20221219090723.29356-1-fengchengwen@huawei.com> <20221219090723.29356-3-fengchengwen@huawei.com> <82dd43d3-661f-64dc-8ce0-a7b6b2a62d79@huawei.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: X-ClientProxiedBy: LO4P265CA0214.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:33a::9) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|BL1PR11MB6002:EE_ X-MS-Office365-Filtering-Correlation-Id: 86665000-17bf-4cd6-9056-08daf00c15db X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: C1lPafkcSoo92NKvD+Kk+zqmS1aPTp64dDyf99zzR9F1hHbNu5yw6dK6oeiz2gx6iMpyrND7haTeYl1yioKtHEHrja/wBsJIWTXM8DUY7GpY7FQu2MOGHiIafaj9IV05MfEWLrFlrefjY1hJwNnW2ArJ+EbU0VSsKinJ4anUGOMPTdBFfNu0M23jckxQkE366qKzHZ9Mz5rh7/i4tyxpIj+0j+Of7HYqgBRM9GKRGkYypOTHNPUGRKX0Oj1Bd9W9VtToDfr0DnFN9dF1o7yDLm0bQu8BGODajHQfIJSbkkA56ISkR5dtJFCj6o67mzWe8HzH7JPwBdNxUcWiVb7fsl7+rxbdcZwOe5axQoMc56FArbUcQlf1pBZVX8zUt2v5Zk0e1QPli5lX+l0Ets07ZUoy6c5Be5UM35NLyzBoox8L0KX+b2ac9UhzW1Apf8zQrTZuqh4ovCw6M320h6X3b1c5FtbOD1iEytqgSp32zHAulx50MX5H21QLhTT5T0KUy0eWK7FFbMlEFC952nP4qbxoUP/5zqCcIBrkiu0hRSsa166xuZqM2GF87Xv1Ehc1JMbXIIsL61x7406sQ3au9MqLzomElACKt25Kd6nYxRAW8j+QpTKjrwnhl7I9I1EoCY+GxxpZdP2XRvpv2trIGw== 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)(376002)(366004)(39860400002)(346002)(396003)(136003)(451199015)(44832011)(2906002)(5660300002)(6666004)(6916009)(41300700001)(66556008)(66476007)(8936002)(8676002)(66946007)(4326008)(26005)(86362001)(478600001)(6486002)(186003)(316002)(107886003)(53546011)(6506007)(82960400001)(6512007)(38100700002); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ndKM1A+pi44Fol/Lz/FawA0ik5VKvx9oSAkgcJXwmhMo3AAmgXqWMwLDQV81?= =?us-ascii?Q?VYsJdFXVkn7LDfzTWISbOUV305Wc80+oYJ4aIj7FckvAUDAu2edxHHNwJlMe?= =?us-ascii?Q?eHKUy0gjtRens61+Oy0SHD1+0qDdo/XiEGiH05ZceYgnHq+H3b3uwQTRlElR?= =?us-ascii?Q?wlRYSP+Px7L8Zq0SILFUWwDDX4Tp9gMDq3PCkfkDwwcEWuv5pbvx8dUpvwzm?= =?us-ascii?Q?KSK9wh1MhapZhzPFeLoh4WYA2EVQG5DqlrK16wgJ9BQlSeX69J3gOBMOLxAr?= =?us-ascii?Q?OMOpTEZyNyTtHDvfLjIfmFpEga4/bHrMhmC3wZj9RwHb3A2hCH0YwjLfn28z?= =?us-ascii?Q?JlMLTMNyzU/8V64XYemnDMMN7jMSgUtKVfJmTr9CVQg5HsVZlSNYL5lKhyDG?= =?us-ascii?Q?a6ZvZw9HCQsd1gs5zR6zLXT94lYjj+hPxfP2Ey4+/1zSKLQc0nZdvykfFVq6?= =?us-ascii?Q?YR3fq4xDLjNGZD6Vh/oJIh1IPNlLvR39irIQsuWUhLiTtlOkSpO1RGdqq92u?= =?us-ascii?Q?A1XRQKK1ezp6xb4cOf/52WVrBL9HPJ+Fqm6Yd7VJTqEBMeD/l4glTGYJg0Xd?= =?us-ascii?Q?B62qXQ+wX/jb09ua1Zz03+a3kjPYnpF4j1SzBS+GuVW5xGMXxVHxjNQ/wvD7?= =?us-ascii?Q?4/yRGHbCnNirnI+BJSk2spk4Uk4UiFVF9oWjT0hOmxYXNxSPIIiIO5OR3oT6?= =?us-ascii?Q?DoX2tOFHxviZna3i0ZBB769w/5huPt18LU8n0x1ah9PcsI9R9MOnm57TpGQx?= =?us-ascii?Q?V9LBDa4jSfMZOip+4LOGDDjHRMu3L8nusaHsZTf+oLzRtx2KwaWu0kxpS5Mw?= =?us-ascii?Q?bAyZzqpn5Anm5nzJsmpaECjXIALKTChe9gAdw/Y+Z2MklKFHNpQBLFZkHxhn?= =?us-ascii?Q?kYkRCGeKItqg6xNbUw/xK6YnQpvxvMqgV7U7qhMHhhJku0H4ZRkZdvQpmKaN?= =?us-ascii?Q?5frLzp3JaCn9aaMbf06Enu90+Q4QapDyUGviE99YuEFPPxggGx7XCnvyE2GW?= =?us-ascii?Q?Py6rjclgpQ7Rvxzo5DCx2MjBZ56mroSdg69JLd1uULfQE6LS3lKhG0LjEBd8?= =?us-ascii?Q?+I1SBdjjy8nLigZBxj2fmKrFo9o8mc5T0nPX1BvYu4sxThyZSiQchfaYU2op?= =?us-ascii?Q?eKu16OhSTh7uE9OajfLNwmRxRcX+/WvjKWn1GM62lVmUcd+Txn81SXEx4Ban?= =?us-ascii?Q?yV82VBDckJ9lRBPsNpscHY7TH9dPvH6bdzxGo+5azQmtqhYi5xuqKd1m6s3B?= =?us-ascii?Q?jei3BPuiF18hqFG2dvGQ1cffdZW0CG0n7eNPyUctDAW2CuP1dHgdoPINrnTN?= =?us-ascii?Q?xWa1cJ+8VdCM66FC38p68S7Rop03d6KexYGJPM0Txjf/gXxDIHhprLJvVgMn?= =?us-ascii?Q?PmQ2aAXKnbUmWLgl+6Q931dshlxP2NGteWb9NhHnwFtEYg/r5aeeI3R17O7Y?= =?us-ascii?Q?uVgZs9HCLMz5q66+6J2s2UBkOhR9I9UKZ0Su/5WGR5RQg/EdiMa3XrbOkhKf?= =?us-ascii?Q?eMp5uRy/woQmCfyDt6tH11uanpuu+APUynZkDtBDvRwMPdYG18O+wXAbzIGT?= =?us-ascii?Q?/5r/4A0MlnGc5NrkCWKhue+Ebx1l+frHO1O5O8aLVzGUJBpJQyW0jXNPJFQx?= =?us-ascii?Q?3g=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 86665000-17bf-4cd6-9056-08daf00c15db X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jan 2023 17:33:12.5518 (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: gHtoa6c1flq09fYoIqeSnV2cZrALKq7j2PXR6IFumpvL62ophcw766Z0QXHD0dxSeRlGBv0e1AqpRaXQZJQiF3yBlyDsyVpRL6MoXbC/lb0= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR11MB6002 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 Fri, Jan 06, 2023 at 04:07:45PM +0000, Bruce Richardson wrote: > On Mon, Dec 26, 2022 at 12:53:57PM +0800, fengchengwen wrote: > > On 2022/12/19 17:33, Bruce Richardson wrote: > > > On Mon, Dec 19, 2022 at 09:07:20AM +0000, Chengwen Feng wrote: > > >> When telemetry callback didn't set dict and return a non-negative > > >> number, the telemetry will repeat to display the last result. > > >> > > >> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality") > > >> Cc: stable@dpdk.org > > >> > > >> Signed-off-by: Chengwen Feng > > >> --- > > > > > > Hi Chengwen, > > > > > > I'm a little curious about this bug. Can you describe some steps to > > > reproduce it as I'm curious as to exactly what is happening. The fix seems > > > a little strange to me so I'd like to investigate a little more to see if > > > other approaches might work. > > > > Hi Bruce, > > > > Sorry for late reply. > > > > The steps: > > 1. applay "[PATCH v5 1/5] dmadev: support stats reset telemetry command" > > 2. compile > > 3. start dpdk-dma: dpdk-dma -a DMA.BDF -a NIC.BDF -- -c hw > > 4. start telemetry, and execute /dmadev/stats,0, and then /dmadev/stats_reset,0 > > the output of /dmadev/stats_reset,0 will be the same of previous cmd "/dmadev/stats,0" > > e.g. my environment: > > --> /dmadev/stats,0 > > { > > "/dmadev/stats": { > > "submitted": 23, > > "completed": 23, > > "errors": 0 > > } > > } > > --> /dmadev/stats_reset,0 > > { > > "/dmadev/stats_reset": { > > "submitted": 23, > > "completed": 23, > > "errors": 0 > > } > > } > > > > The rootcause is that the /dmadev/stats_reset don't set the outer parameter "struct rte_tel_data *info" > > and return zero. > > > Thanks for the fuller explanation, I'll hopefully test it out myself. > > However, in the meantime, looking at the telemetry library code, would the > following change work rather than explicitly always setting the telemetry > data to a dictionary by default? Zeroing the data by default sets it to a > null return which is what you probably want as default rather than an empty > dictionary. (And it's also a smaller diff) > > /Bruce > > diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c > index 8fbb4f3060..7b905355cd 100644 > --- a/lib/telemetry/telemetry.c > +++ b/lib/telemetry/telemetry.c > @@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s) > static void > perform_command(telemetry_cb fn, const char *cmd, const char *param, int s) > { > - struct rte_tel_data data; > + struct rte_tel_data data = {0}; > > int ret = fn(cmd, param, &data); > if (ret < 0) { > I've handily reproduced the issue using the instructions you gave above, thanks for those. Based on that, and looking a little deeper: * I think it is an error with the reset function not to initialize and complete the return value. I believe that for each telemetry callback we should require that the callback fill in a valid value on success. For reset, some options could be just a string "OK", or an array just containing "[0]", or similar. * Given that point above, I do agree though that the "data" parameter should be properly initialized on entry to the callbacks. However, I feel that the correct init should be the empty/null value, as is given in the patch above. Regards, /Bruce