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 C5E2541D9C; Tue, 28 Feb 2023 13:24:16 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 72317410D4; Tue, 28 Feb 2023 13:24:16 +0100 (CET) Received: from NAM12-MW2-obe.outbound.protection.outlook.com (mail-mw2nam12on2049.outbound.protection.outlook.com [40.107.244.49]) by mails.dpdk.org (Postfix) with ESMTP id 4DEC64021F for ; Tue, 28 Feb 2023 13:24:15 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UvTHCTudwu9FMloNOwFn012qRZcF63sl6YaA3aB4k77gY2zQFqMGEDvjqew6EYrRj0Qv6L3vTxAFQ5pGVqI7rU28a+NeuO3cBkjK5GV023SID0+j7bTTPDF0yc9WIptq8KPjTgQNrquSlLXfpMw3A1ZdLhLkYPR9clROqS/qgPTHy8tvNCGo1u2Q0bRqEq9vB5OOtihwF7wLoS7d3Rvjq8nMQiwtlxTIKpS1qW7uhsycmrJ2yuwH6WW1pdTu5U1XI8PpDZtoJcQEdTx0zZLfVJVvc5kKYDT6Vxz7+SH8ZaTWajP/B68xJ76BMgYgp0t0xqFO2P+XjoGptGb0Er9NuQ== 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=Cfr7SW0190PI+1yi1cN2W5iDo4/ex0B39FNobGQLMRg=; b=U/6geFKUA5v39abBVJ9X/5/S1g2FUp1Qslt+nizVq9Z6pS1yXugvWP2smV54OkKpghnDXR8yHbm38vJ7VV8eRqvXSjCB8DAbk2ysUMA635PzQIEpj8zoyq8gnv7GBtmW9UsVgxMLWBIzSKsd4p2GLvpOu+pgZ70ZWR+Qnx+RoF5kOx/cR55IxUDlxT2wxV/Cq78ejVgcqGdcVtLm78DnJfPEWaCnX6ALwpoDTu2qqPuErKT2vU2ZSI1vpgmyt2r8nn3EUTiR/9VyWb+xD0Cz7nwoHliK6yMfDVKGT5K2Ik7M4jsFuy+g3Yr4f+NNOIuWV7UsgiQgPlcx4oeSVguHpQ== 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=Cfr7SW0190PI+1yi1cN2W5iDo4/ex0B39FNobGQLMRg=; b=p7IpsRYedJ4sh9oRWFpaJ1CbzGMQmaeocLpMOvBnw4u9HoouMXmjMnYcM3yax+GYbG5eTY577bFM0llPHqmfWfw23AmfNDDFBELoQDREB3U+Vq7fOOz9wxwoJAL7EaImQTJU0VZxRs0A/3LoLuasfIMBbw+aguV5Ba7NZkBPwy4= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from MN2PR12MB4301.namprd12.prod.outlook.com (2603:10b6:208:1d4::22) by SJ2PR12MB8690.namprd12.prod.outlook.com (2603:10b6:a03:540::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6134.29; Tue, 28 Feb 2023 12:24:11 +0000 Received: from MN2PR12MB4301.namprd12.prod.outlook.com ([fe80::80ae:e5ed:4fa7:2ad7]) by MN2PR12MB4301.namprd12.prod.outlook.com ([fe80::80ae:e5ed:4fa7:2ad7%9]) with mapi id 15.20.6134.030; Tue, 28 Feb 2023 12:24:11 +0000 Message-ID: <57607ef8-8be6-967b-9f65-78a63e4e905c@amd.com> Date: Tue, 28 Feb 2023 12:24:05 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH v7 18/21] net/cpfl: add HW statistics Content-Language: en-US To: Bruce Richardson Cc: "Liu, Mingxia" , "dev@dpdk.org" , "Xing, Beilei" , "Zhang, Yuying" , "Wu, Jingjing" References: <20230213021956.2953088-1-mingxia.liu@intel.com> <20230216003010.3439881-1-mingxia.liu@intel.com> <20230216003010.3439881-19-mingxia.liu@intel.com> <7450f46b-fc91-8a1e-c9f7-90ff1ca56d8e@amd.com> From: Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P123CA0037.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:152::6) To MN2PR12MB4301.namprd12.prod.outlook.com (2603:10b6:208:1d4::22) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MN2PR12MB4301:EE_|SJ2PR12MB8690:EE_ X-MS-Office365-Filtering-Correlation-Id: 384d704a-6b4a-4340-85b7-08db1986b20a X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: zKDyZ2CUxlpMDvOfgQOAKovvs7KKVBJPFwvYlehqNfft4FR2Db1FnE2YS7jUf8auk5+Rl84zWrCIq8qp0IuLrFacYLxvC7LsRyqHluJFN+yrYEevcOpX+Ixyqbqdm6y239UbMOFyVJbjTv8rF+vjXvu2C5q9fImyBNVzbwgjEhJCdHu87O0I0DdRMYaB73oR+HSbDSTgnm0mzfzDG6E8/sqmIEa3DhIydQFcMPETaaxd2PxTMSJBAVzsCyQ/16FnVJTc1qkQww5CIRoKfCmA9QQrEJxL+cmoWNd7dd5bDuuEDOenLgIQGOTk6aWGgDVx+V2rARlb/LpgTzR3G7P3eo5jiOAkTAzAVDL+lb8kmA7kE9glw7OIQ2+ttxE1gVQ12B+kIW7LQMMfw9NHMqlcPBzMQv48GcYpz42uS1m+iRuaWTl5TS0cjjF4ny7SC5YQozWc9MBY3u0vzXfX5TBzpHvGcZpqDhb3LMrXo3GdtMun55mM/3Uly11hg08UcAli8l57bXLl2P+d8MrPUoJOqkiQsAb7WRDxQCFJjrVWKAsu9yd5Nky5choNTUPeULrVspYfESTMA2PHhNtj66pr2cDvmEoNEFtPFURsxlJrfRy8vmh9RWETOj1KZ2kWLIhsmJE8PY+rpmlscqjAuQLK3eQmQXugB4KcjePR6M6dxJvT+3MPKvPax3MDYDNPtz4EbBUAviyfashE0DoJB7AaW6gZ5xJwxVH552vz0Pn3fqw= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR12MB4301.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(376002)(346002)(136003)(396003)(366004)(39860400002)(451199018)(2906002)(31686004)(44832011)(5660300002)(8936002)(36756003)(41300700001)(66556008)(66476007)(66946007)(54906003)(86362001)(31696002)(316002)(2616005)(8676002)(6916009)(6486002)(4326008)(478600001)(6666004)(38100700002)(6506007)(6512007)(53546011)(186003)(26005)(83380400001)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bjdRLzIvaVAvQzRaT0Y1VFpIUXZsWUUyOUpneXpLK3N0NWNwMHBGQlNTcGNh?= =?utf-8?B?d1ZROVRHUlRaNmF2T0ZFT1ozeEplTWsrWGFRZ3BtM1lCOXljZFoyOEZWOFFv?= =?utf-8?B?NDJ0VWZydmplTVpBN1JXMXZ3dW5FR1ZwZjVFTThCRDFBN2d2UUJkSzl4TDZk?= =?utf-8?B?Ym42TzdUbGVZcjlsUXRCd0htbHFtTlVIS2RaTjRWK2d0Q2hSa2VsdTE2VDZG?= =?utf-8?B?b29BcTNGczdKWURXb0UvWWZtc1V5V3lnelJydVJrQVlHVmlWU0ZmTXYweGJZ?= =?utf-8?B?YWJ6ME9ZMjlkcmRtdzlSKzY5QUdMaFhobGlnRERDUWRWTDNQTG5JcCsyQ3RC?= =?utf-8?B?V040bktwa2JDTU5TRE5WUWgvc1RvVy9KNXRyaTRsaE9vbHNqcVlJcnp3dXNa?= =?utf-8?B?MW9zcHE0ZER3OFdEdWtXVGJia3krZWN5NHh1emJPT2pSVFBGWlhiQ041bGFH?= =?utf-8?B?R0RjNE45SlFiTnRmcmlyeTRZSU1mTDBoN2lVaWpsR054b1Q4ZS8yMXVieG96?= =?utf-8?B?SVY5cXJ0MnZTUHpyck9zampEOVlPeWhpVS8xVzNsMFA1akNyZzJWR08wZjFL?= =?utf-8?B?ZHcyVVFzdXFCcUhFY1dGRDltanFiQ21vR3lHclNGQ1ZDUmJCQWkxbHVRL09j?= =?utf-8?B?eHlhM2w2Ukcxa3NqSW5FdERMVkovN2tjVS9scWdpR1R5QU95bU9sRnRrc3Yr?= =?utf-8?B?YjNobldQNkhUYmIvTURscFRvMDJIN0N2UGd6clNCbUdyQ1d3b0JOVWZFVjdL?= =?utf-8?B?RGEyVi82ZWd4Y3J4dzN0ak9kYzBQclJWcVFrdUk5NndyMU5VRUNVUllnc215?= =?utf-8?B?cmNQVTBZbEw1K2UzcXN6WGR2VzdjWUFKeTNGdmlCcEFXOW9GbUlMVXJPaG93?= =?utf-8?B?WE5lRXI3M2FRUXpEWFN5d0M0ZzQ1dlRmQnl6RVExdytiUVBpNDZmRW5nRExK?= =?utf-8?B?VjFUMzVoYU0xbFZTUTdzZkJIeWJoTXBwLzE2WHcvbllKMC9qalZCVmovb3hH?= =?utf-8?B?LzhkZFJHLzNBTjVpNGdpOTBEUFVFeDY0blM4bm0vQ2N6QVJyR3cwNXRQbHFZ?= =?utf-8?B?RElNWTRDSUNPRmlFaldOcDdLWi8wcE5JeVZhbm04QkF3ZTF0TjQrZ2QvaXZ5?= =?utf-8?B?dU1JeW1MM1dIamtOaFVEWDhCVXM4ZTFKMzFpbW01N25nYU8vOEYvbmRCN0Jh?= =?utf-8?B?UHIwU2toeU5wWjhJcldqM2hNSC9kcVVKaWhENjZyc2wzS1dKdXBWaHZEY0Ns?= =?utf-8?B?ZHBybjVORldYdnFZUEE5a3dDNzNnZ1BzbUg5Z0tLK3lRNUlDeDZ4dERRRlpX?= =?utf-8?B?U0lWMm5FUjhBaDF0SXU2M1E2dTNnOXVBNWVXZ2hzRmF3V1ZqZ05Geitkdng5?= =?utf-8?B?UVptY1lvVC9PaGNieXZsMExNd2kvMWlzTDRrYVJHTnh2alJHcVo2WVhwR3Bk?= =?utf-8?B?MDR2ckRtd3pYSDJ1NFNFWnlyMFM1QXIwcnh1V3JXVmwwUHN3Q0VpVlVVQVVr?= =?utf-8?B?ek16QWxsSnZjRVNBSnBSMGNScEgzamlGbVdYSEV3L0U5MU01UzE4d3RmUDd5?= =?utf-8?B?UTVkRXo5M2hQRzlLanUyOHZybmQ3ZTFtZnRJRHFSdzYwMkNkQzFOc1E5Yzd1?= =?utf-8?B?eWlkR1Fya3VqdEkrY2l5alZtejhRMStLVnNmL214S1RQM2pZU01ZaW1QUFVK?= =?utf-8?B?R3liNXRJSzhGNzBPemgyT0l6S1BLK1FZam5ESnhnWXlmaXJxQjdDNmR5dVBI?= =?utf-8?B?RmxqSWJkMTJDRVFxR2cwUmlSZmVEQUFOL1VyWWZQZFYyblRTL2tOUklNMGkz?= =?utf-8?B?M3QwQmdtb1dtdWVQcHY2cFcxVnpmNUgyT1ZTb2N4UGpHTEpRZUZrK2xvTE1R?= =?utf-8?B?MEVtcHFZdmtWbVp4cHMra0J3SXBVOTZIQktqSFJaYWdvWjZ3TEVtWHFyQWVo?= =?utf-8?B?QUF1NENKdmZUQzJkOFpUNTAyTlRiQ2wrOEw1cWI0ellqTzVkMU15Ym80WXli?= =?utf-8?B?QVFyTmVCYVJZWStZb3VtaXMwRWZNSjdtQTB4ajlYZjU4R2Mxb01GOER1TWRT?= =?utf-8?B?RlNKeVZvL0dsOHF6K0t1TUZpZGdnZmdLMmJVbmdZZ0VDWVl6R2laQktRUnVr?= =?utf-8?Q?WxUXdGaugo9E8bxxHzskqJAHA?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 384d704a-6b4a-4340-85b7-08db1986b20a X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB4301.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2023 12:24:10.9349 (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: 6y7T1Z8QKldRBO7BAhi3/1XjVoCPTsxXIVoNXURRGDCJbw0psC+MucSwef1TeZW5 X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR12MB8690 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/28/2023 12:12 PM, Bruce Richardson wrote: > On Tue, Feb 28, 2023 at 12:04:53PM +0000, Ferruh Yigit wrote: >> On 2/28/2023 11:47 AM, Liu, Mingxia wrote: >> >> Comment moved down, please don't top post, it makes very hard to follow >> discussion. >> >>>> -----Original Message----- From: Ferruh Yigit >>>> Sent: Tuesday, February 28, 2023 6:02 PM To: Liu, Mingxia >>>> ; dev@dpdk.org; Xing, Beilei >>>> ; Zhang, Yuying >>>> Subject: Re: [PATCH v7 18/21] net/cpfl: add HW statistics >>>> >>>> On 2/28/2023 6:46 AM, Liu, Mingxia wrote: >>>>> >>>>> >>>>>> -----Original Message----- From: Ferruh Yigit >>>>>> Sent: Tuesday, February 28, 2023 5:52 AM To: Liu, Mingxia >>>>>> ; dev@dpdk.org; Xing, Beilei >>>>>> ; Zhang, Yuying >>>>>> Subject: Re: [PATCH v7 18/21] net/cpfl: add HW statistics >>>>>> >>>>>> On 2/16/2023 12:30 AM, Mingxia Liu wrote: >>>>>>> This patch add hardware packets/bytes statistics. >>>>>>> >>>>>>> Signed-off-by: Mingxia Liu >>>>>> >>>>>> <...> >>>>>> >>>>>>> +static int +cpfl_dev_stats_get(struct rte_eth_dev *dev, struct >>>>>>> rte_eth_stats +*stats) { + struct idpf_vport *vport = + >>>>>>> (struct idpf_vport *)dev->data->dev_private; + struct >>>>>>> virtchnl2_vport_stats *pstats = NULL; + int ret; + + ret = >>>>>>> idpf_vc_stats_query(vport, &pstats); + if (ret == 0) { + >>>>>>> uint8_t crc_stats_len = (dev->data- dev_conf.rxmode.offloads & + >>>>>>> RTE_ETH_RX_OFFLOAD_KEEP_CRC) ? >>>>>> 0 : >>>>>>> + RTE_ETHER_CRC_LEN; + + >>>>>>> idpf_vport_stats_update(&vport->eth_stats_offset, pstats); + >>>>>>> stats->ipackets = pstats->rx_unicast + pstats->rx_multicast + + >>>>>>> pstats->rx_broadcast - pstats->rx_discards; + >>>>>>> stats->opackets = pstats->tx_broadcast + pstats->tx_multicast >>>>>> + >>>>>>> + pstats->tx_unicast; >>>>>>> + stats->imissed = pstats->rx_discards; + >>>>>>> stats->oerrors = pstats->tx_errors + pstats->tx_discards; + >>>>>>> stats->ibytes = pstats->rx_bytes; + stats->ibytes -= >>>>>>> stats->ipackets * crc_stats_len; + stats->obytes = >>>>>>> pstats->tx_bytes; + + dev->data->rx_mbuf_alloc_failed = >>>>>>> +cpfl_get_mbuf_alloc_failed_stats(dev); >>>>>> >>>>>> 'dev->data->rx_mbuf_alloc_failed' is also used by telemetry, >>>>>> updating here only in stats_get() will make it wrong for telemetry. >>>>>> >>>>>> Is it possible to update 'dev->data->rx_mbuf_alloc_failed' whenever >>>>>> alloc failed? (alongside 'rxq->rx_stats.mbuf_alloc_failed'). >>>>> [Liu, Mingxia] As I know, rte_eth_dev_data is not a public structure >>>>> provided >>>> to user, user need to access through rte_ethdev APIs. >>>>> Because we already put rx and tx burst func to common/idpf which has >>>>> no >>>> dependcy with ethdev lib. If I update >>>> "dev->data->rx_mbuf_alloc_failed" >>>>> when allocate mbuf fails, it will break the design of our common/idpf >>>> interface to net/cpfl or net.idpf. >>>>> >>>>> And I didn't find any reference of 'dev->data->rx_mbuf_alloc_failed' >>>>> in lib >>>> code. >>>>> >>>> >>>> Please check 'eth_dev_handle_port_info()' function. As I said this is >>>> used by telemetry, not directly exposed to the user. >>>> >>>> I got the design concern, perhaps you can put a brief limitation to >>>> the driver documentation. >>>> >>> OK, got it. >>> >>> As our previous design did have flaws. And if we don't want to affect >>> correctness of telemetry, we have to redesign the idpf common module >>> code, which means a lot of work to do, so can we lower the priority of >>> this issue? >>> >> I don't believe this is urgent, can you but a one line limitation to the >> documentation for now, and fix it later? >> >> And for the fix, updating 'dev->data->rx_mbuf_alloc_failed' where ever >> 'rxq->rx_stats.mbuf_alloc_failed' updated is easy, although you may need >> to store 'dev->data' in rxq struct for this. >> >> But, I think it is also fair to question the assumption telemetry has >> that 'rx_mbuf_alloc_fail' is always available data, and consider moving >> it to the 'eth_dev_handle_port_stats()' handler. +Bruce for comment. >> > > That's not really a telemetry assumption, it's one from the stats, > structure. Telemetry just outputs the contents of data reported by ethdev > stats, and rx_nombuf is just one of those fields. > Not talking about 'rx_nombuf' in 'eth_dev_handle_port_stats()', but talking about 'rx_mbuf_alloc_fail' in 'eth_dev_handle_port_info()', should telemetry return interim 'eth_dev->data->rx_mbuf_alloc_failed' value, specially when 'rx_nombuf' is available? Because at least for this driver returned 'rx_mbuf_alloc_fail' value will be wrong, I believe that is same for 'idpf' driver.