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 E758D41D9D; Tue, 28 Feb 2023 14:34:53 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D77B1410D4; Tue, 28 Feb 2023 14:34:53 +0100 (CET) Received: from NAM04-MW2-obe.outbound.protection.outlook.com (mail-mw2nam04on2063.outbound.protection.outlook.com [40.107.101.63]) by mails.dpdk.org (Postfix) with ESMTP id 378544021F for ; Tue, 28 Feb 2023 14:34:52 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fkaUcgpeo2MCyukU2yVaYG00jCLuiir2yZj/xtWXjpbGq3g3R2eulayztF191kL1U2DpRyUvpafoVRJt7DXHWc7rc+alynaNTm0d81xbpOAFm5i69lwfQpXNZcJMiHM9o/9PndqIacWwt1CJjb5n/R3VR8g7t086VtzeVmMsSO6y3i9mMS6BQ85Ph5hRLFHX0/O+ETpjVY7l66V4N1Ty5YLWDOOX0xkeszc+h+MND5fX87yC6ojNklT+336GcpsLqII3kCi8Vq20R7RG5M4aTIY7mwpEdh+2FmDQOcd06r/AnKV8xkrqlW8kJAE9irZSNHKjgi5dGDyYaf8x0qd7Vg== 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=4iFSmQWGdyxplAfksN8Eb+vP6YkctV0X58jpfRlRinA=; b=ZqCdGQVt23DKes75TUYZL7IP3I/n09DNA+HhSaV5JlzDe6jsTwPbyYdoIpKj+7LR8R9Vx2xewubCenObhh/XI12EXVMzOc1/NaE+WWJPGuzowGePbkLDaSel9F9a4K2FBz6oBr13huXRo1EyVXWHZXzz8CLj3TkHL1C1Nbj+rQ39Z/lnI9x0LR3YK23Z3qno7GiV4YzhqeS2qrVwsv/mJPyOEejXSY7mxGU7XVsf0kvKNpAcpLvkY1WsoSYz/RHKbtD5D1uRidFuYc0p9jVQ02F69zIVkXEZfybze0tvb9kvO1e9hUDYh6MVi4rOes3117uaihWRhzKEPeol69Trlg== 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=4iFSmQWGdyxplAfksN8Eb+vP6YkctV0X58jpfRlRinA=; b=qtbceHYS2+Ykfyhl57lCwvSCmsBAX+qFwRpIugiOEG564UFMH5gFMH6G/kn25ze+G9xV5YSGCS8C2FQJIp4KO3MCwC523DLWfJCbHa6J9g3b2Ii4Yz8vLALJ+sr4ieZF2PMNaUhLvhUafgfh67mk4nub6tIwsVc+QfvG5SwN6Xs= 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 SJ0PR12MB5504.namprd12.prod.outlook.com (2603:10b6:a03:3ad::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6134.30; Tue, 28 Feb 2023 13:34:49 +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 13:34:48 +0000 Message-ID: Date: Tue, 28 Feb 2023 13:34:43 +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: "Zhang, Qi Z" , "Richardson, Bruce" 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> <57607ef8-8be6-967b-9f65-78a63e4e905c@amd.com> <4ffc2fe6-50ea-4853-3dbb-616006d0d09e@amd.com> From: Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P265CA0234.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:315::11) To MN2PR12MB4301.namprd12.prod.outlook.com (2603:10b6:208:1d4::22) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MN2PR12MB4301:EE_|SJ0PR12MB5504:EE_ X-MS-Office365-Filtering-Correlation-Id: 74f5945c-f8ee-45c5-6ef3-08db19908fd3 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Q322MaQNelqKJUuMgD3m5NGskYzrzaOrVvpxi2qC9nhZqYiSaYNNDxATNvriINAh+adJCeVM1VY9q+O4fJzfpBNz3vwL4PJaxRmYJu+1NjN0ysgDIlJlv+ylf45A3rD/ga5I7e9Fve/6GMy5qQ89CIOFgJCP0J+N8HTwSCapfOGSIg4aP87qw/htEUT9dGyISdGYhm/DrrJuaxQBut5uvPx9+mf/mgx+8NosFSf7lH9fADdo7pEovd11FaG0OAH5Zi5zXBr9OEWdOpp+JMO9FLZpaXgj+knVIzzivLTGB/gS7rVnh7k2yLAEcr/e+GYinuoiAbxyzSb/GQ5Hbi2QFC86MrL7O3vtNJmzbXYxn5T8ech3bFZsX2x/4SJkCaobJbvwWf6ccUqQvp5XUqysuyiaptt4HttHJ7pjNNbrVEtIGrWwDVbgdCpZt2LlV12gQVCxvi0yfBcc/0H0uu138BYAKqK2EgkuoxOG+Aq7OjhfkxfmMtUdepW+r/9RZtFhZA//k1LuGmIwsck9VbdPwYKB0Gr88xqtx1fCz2ybnKzZUHzWgop/oQa7g7fCfdm2zVCbRq2xuSGwkM1a2S3Hh53sP3h11jn2z7LCHiTQMk2sMVazGcxe8oTKBRvyb7oEcjjBKZ8idDwLbKOy2FhZa5nRbSTll4GI9ah8w6ixq1OOZGZA/Mguu+SUItkwPtNo7YjS1A6n38GfgA/exPHcECvww8bpg5kvCuLSmy1acBs= 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)(366004)(136003)(346002)(396003)(39860400002)(376002)(451199018)(31686004)(8936002)(5660300002)(44832011)(31696002)(186003)(8676002)(66946007)(41300700001)(66556008)(66476007)(26005)(4326008)(86362001)(316002)(54906003)(83380400001)(6486002)(110136005)(36756003)(478600001)(53546011)(2906002)(6512007)(6506007)(6666004)(2616005)(38100700002)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?M2RHM0dHU3I4TzFyQWI1MDBHa0l1VDNxakc5VTRwanl6cHpwdEJJd0FZUm5s?= =?utf-8?B?UDllSFF4am02aDdlT2VCTCtYV2RYZXk2Y0pVdDRsbVNSWEdiWnIvMFJTT1Rh?= =?utf-8?B?SVZ4bE1xT1dFelVjQ3NNN1Fxdy9rZkhZVENxSGRzMEJiUXRJOVpISWxuSUlC?= =?utf-8?B?eTBld2tiZlozZTdiUzNNbEpQTkdxTWF4SzFnb1FmS3R1S0w3WWRoa0c0Nk5q?= =?utf-8?B?anhXWGd1dG8rei9Ia1pXZ2RsamhyVTJWeEx6Qzd2cWdBUnBWYzZ3TnIxZy9w?= =?utf-8?B?bHhKY3dsVit0d0xlVkVCUFVhTHJEcVNOSmhocEliaWErWTNhVTVLZlA2dzNp?= =?utf-8?B?Nm1PelJPMmZWMEFpaTgwZ0hLNVBheEZZQWtJZE1FeDZORzdITmt5TmNtL1JM?= =?utf-8?B?VjhvU29vcWJmUjVXS0ErMjNRclAzVFUvK0Q0VjFBRDFGVU9VQXZzTG9yUFMw?= =?utf-8?B?SEdQTzd2NGZQL1BwVkF2UlNDQnlZditjdXNiQkRyRWVNZ2svNDNsaDUwdFl1?= =?utf-8?B?YVQvUUVDMDdvd29YTkRrckUvNllOTktqOVowZEVrUENWZTNJTlE1c2dwNG1z?= =?utf-8?B?blV1L0JPU0E4OEdFQ2NzQmExbkExdk55T0tWazRPUGY4Ni9DYkZlRXRMcFdM?= =?utf-8?B?VTVYc3FZYW1VVlR2c3EreTlSVHNpRVZQZHdrVm45akViRzBYVWJiejM5VDVR?= =?utf-8?B?dkd1R2gvL25XUEJxZHNFNTBldzFaOVM4blU3aWdsajEyRnRKUXNkWnFoamQ4?= =?utf-8?B?cmg4dnF6QU4zNkJXdjdBbk93eUk2WEI2U1RaUi9XejZUa0crb2IyODA3aG4v?= =?utf-8?B?NEZTVFYweEduak03NzBXMkJqYk9IRC9BZEZvQzZDV1V1RHRvT1VNOURLdWZT?= =?utf-8?B?a29QR1VOakV2OGZrWEkrY3JhL3FkRDJ5UlFUZUNIdEZwZlVCMllnaWJEc0or?= =?utf-8?B?YlNMMDhibWtldkxaUXg4c3FwNFJ1NjlQcmFPYktZa2lVbzhwbE04NERLNldR?= =?utf-8?B?L2c0WkRUaS93SjRQVy9MWWdYWktnSitJQXRBVWZMUnhnckRxTXZtNFpDQ1Zr?= =?utf-8?B?U0hualJ6Ri9xQjdKc3pGRHJmRkYvZEM5Q2laajM0Q3pBUVlRNFBmeE1yeS9x?= =?utf-8?B?emN4bTRSZTVWKzloV2dLb2pYekZxY3dVTkc1TDFqOUlZMGFEMUJUYy9kZFpW?= =?utf-8?B?VTBuTzJzeW5FWUErdytYVU1CWmlXVTdwWDZzM1YyMkkwYUhqOTgrNnlQKzcr?= =?utf-8?B?NkU4TlN0eXhxWUtueTJFN1oyeWdLNmpmTFhxYWw1aVNmR2NxanNib3VBcFJT?= =?utf-8?B?UDRsQXg2OHVYMGVaMnYwSVEyV3k2ZnF5WHl3NXppZmxQRWVnRU45ZlBMVDl2?= =?utf-8?B?YkFyN05XdFlYUXZoZ05wVGNFNjlCd2x0VzF6aFk2ZjR2dTZLb2pwRGNLb0Jy?= =?utf-8?B?cjNtUStaM3hHUkp2R3pvU0R6c3ZHMW9ZdXU4S1FITlFpWXN3UEptTHlNYm02?= =?utf-8?B?dTErYTZ1bTdSZjJURklQR09IL0V0QXBFVEtPQW5SZE9oTDgwdU5RMTc4Nysy?= =?utf-8?B?SVFWMEYvY2NLZmtTQUtXV1JhZnFMMjc3cSt5S2Y5VlFudmJUSTlzR0tvQ25L?= =?utf-8?B?L1o1NWtLVFVtYU5SVjhadHhTaCtJQ21XdTNsNC9jWllZVk1ZMURTZnNOUHNF?= =?utf-8?B?QlQrWG8vSHhreUwyNlBocWJFOU5IOGxKTGlxckpGQktLRy94M0RmaDZlQjM4?= =?utf-8?B?cytRaHBsYlJNYXk1MlNMcTdiblMvZnh2RDlYUUNPblpUUjR2bW12a1RpL2tF?= =?utf-8?B?MEhPc25ZcUV4NENNUERhQUNyRWFHb21BamhQYjRyVWphUU4yRFljZkNoam4x?= =?utf-8?B?bERuRkVJMlhBQTRHNGVrdGE5aEUrMGp4UFRMaFV2OW9VUVBpdUNFTWVUNU4x?= =?utf-8?B?TEQ3UGJ2Y0xRZUZDdVI2aVlQMVpnbXdrNXl5WktGQlFYZnQ1UXVIVHZFQVlj?= =?utf-8?B?ZzVQQjI1OXhoSGQ3VWU5S2k5eGtQbG9NdHBObktaekdHYkhpUUpWTndmK2pl?= =?utf-8?B?Z2lzZmoyWmdPZzJQOElneVJndCtpZkhlWEFLU21rbEtJbm16N1ozQnV0eTVm?= =?utf-8?Q?TOx/eFuReMct17N8E1JriEnGt?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 74f5945c-f8ee-45c5-6ef3-08db19908fd3 X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB4301.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2023 13:34:48.4071 (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: zaTcHWekyHvy/01OtVB+GMjjkgAop5zJmkyG4IGHmmTb3nw4uT6SUZImlUbazbDB X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR12MB5504 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 1:29 PM, Zhang, Qi Z wrote: > > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Tuesday, February 28, 2023 8:33 PM >> To: Richardson, Bruce >> Cc: Liu, Mingxia ; dev@dpdk.org; Xing, Beilei >> ; Zhang, Yuying ; Wu, >> Jingjing >> Subject: Re: [PATCH v7 18/21] net/cpfl: add HW statistics >> >> On 2/28/2023 12:24 PM, Ferruh Yigit wrote: >>> 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. >>> >>> >> >> Or, let me rephrase like this, >> 'eth_dev->data->rx_mbuf_alloc_failed' is not returned to user directly via >> ethdev APIs, but it is via telemetry. >> >> I think it is not guaranteed that this value will be correct at any given time as >> telemetry assumes, so should we remove it from telemetry? > > May not be necessary, PMD should be able to give the right number, this is something we can fix in idpf and cpfl PMD, to align with other PMD. Thanks Qi, Ok to have drivers aligned to common usage. Still, for telemetry we can consider removing 'rx_mbuf_alloc_fail', user can get that information from 'rx_nombuf'.