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 5CACF41D9C; Tue, 28 Feb 2023 13:05:04 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4EA4D41151; Tue, 28 Feb 2023 13:05:04 +0100 (CET) Received: from NAM10-BN7-obe.outbound.protection.outlook.com (mail-bn7nam10on2085.outbound.protection.outlook.com [40.107.92.85]) by mails.dpdk.org (Postfix) with ESMTP id 2E2CB41140 for ; Tue, 28 Feb 2023 13:05:02 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hNADJx4z7btilOPpzDeb2Q0IC8zPAWFjGysmb1sPQI3F5Y6V2j7oxu30+JN4jcJ44bh4Ep6sPZm1UqUzNQzcgu0aSD3mqwvj06nBlL5hnv/Pax85Txj6c51CuTnj8sSgE0gDoBGEx0Uiam+967W6xk+QCtgoqXHHa+4Z9dwaflDxYu/NChJyoF71qEy0Gk1ALff25Ycs6WmWWRxsdRK6q9GKPDTYLQPqLJbePMJskwZu5jEHs+qBHkhQMEOze4b+i1EVKdXyYIAeGYmxZGXtEIIgufa329bOWyDqs1oHtfuM3ppLaOmTT3R3FuSTmV7feZbsD9HEZMvr1PS3bOccqg== 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=EtRwnkkJVVmjoi5xJJYWp1q5TbfKOzGw+3K3veR+i94=; b=ld98E88vjTuFpOIpHVX/7y4/neTP580sY5NSwqhN4dab/Y7rh2chlxyPb0+LULUWpXcBT99lWQ6kAq4LyfAW6EF9xAK9w77Fshw/8qBaGBqxegHh+Yk4ZmMYRN9B4SD6AxuTYCdJvP6bRoZgcl/fi+nGrIt7UGthmunDWcuL5VrvGqbDxa/HI2ipibEXCPyygbUhgDdMkbm44kuSHDKsKjhAlExuGl2cj1Ws6nCI/r7TU3YRA0JE7B/P2MjsJLXgobN9IZAGSdOmIGRBv6MlIgK6uLy8t78Fu7U3LtonfmUpeHr6fVageI5abj74SZhVz5tsSJr7FNLQWx92v+VkOg== 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=EtRwnkkJVVmjoi5xJJYWp1q5TbfKOzGw+3K3veR+i94=; b=Xf5uCTxTrR4uaD9oDWhSW6Ifm/LTzXsqnmSTUzREQ3Y13ljZ/SI0O1PxTpzzK5yjyqSZV0OuwMgLPx3+Mmhndz/3Ck4OXac/jhm6ST12OEkL3FgiBBV/uv/wsydN7WyaLriw+Ehc0oiDxxBk+WocACOuxzMC3ugvN7ANDm8zPvg= 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 DM4PR12MB5794.namprd12.prod.outlook.com (2603:10b6:8:61::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6134.25; Tue, 28 Feb 2023 12:05:00 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48%7]) with mapi id 15.20.6134.030; Tue, 28 Feb 2023 12:04:59 +0000 Message-ID: Date: Tue, 28 Feb 2023 12:04:53 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Content-Language: en-US To: "Liu, Mingxia" , "dev@dpdk.org" , "Xing, Beilei" , "Zhang, Yuying" , Bruce Richardson Cc: "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 Subject: Re: [PATCH v7 18/21] net/cpfl: add HW statistics In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P123CA0199.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:1a5::6) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|DM4PR12MB5794:EE_ X-MS-Office365-Filtering-Correlation-Id: e9c30b57-6155-440a-921b-08db198403fc X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: HqyxzXH5iLP92NX9zUDYfZSYh1Uvn115lXYnMnpcvyQnNVawK6lCMVb2PQZv1riySZzrHRPWvG67RZgiKXW2sIooy0SqZuREynjwErvviIZaUA+eUqVOSmROjno70r1o3Ejq+tPcHSI4Q4ekkC1szJfEGe3sDwAn2b4qJ4C44VIaPoAu8TvHueV3cTP3dRnllYJXtHgznYazwTVNq/TMRfyM8vdRADiYbB8HRvptjNMqNi73jigQJMZ6M8mJ+kjbVwIWRig28Y0pz2HCzfNKEMVQKkvC4VFxczl8uiAJAhNZjaoQKt7M28xhaAcY9WYgRILDEobRsm3aFtqrribV+L/sHo0tqS2ScUzS4MVav1eAu9rZLsZI9f45XUSradIgKNE56R/rzkArfCpJuEThbwRHTujaQM4KI96iStXNtzdX/hDOurXoZCZca0gAWO7fA5rkhbYtuUX50YbOyD9Z88wW3+D7ADHYReWwsLGC7TSvQMRwXe1njuvs3XgA5dpmI6SelkyWdAArNuy8kHwAb7kFDeptn9qkm61cx7IUwHcpjlBr5NF5N8Zur7X7F1HBcS6/WAig91RjVYi4yuX1vrBj9KhraMJi14QAPlAqaGisWnVwEXBaKZFLFwkqFRvsz4IGLB5KEg836Wy4IveKkvS2Skfp2B+/cwkajKDrsXrvbFV1/2iD+gQmWRHMjdjaMaIPhE3CZP4Da7i9I3dXvco60tcL7LJsAIJMInqluyQ= 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)(346002)(366004)(396003)(376002)(136003)(451199018)(36756003)(86362001)(66556008)(186003)(41300700001)(66476007)(110136005)(478600001)(4326008)(8676002)(31696002)(66946007)(8936002)(6506007)(316002)(5660300002)(2906002)(26005)(83380400001)(38100700002)(6666004)(53546011)(44832011)(6512007)(2616005)(31686004)(6486002)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V2dBak9saWNuUVlpNE9sZDVkcDBzOUNwSDV2ZTY1b3ZRSnFEWko4by9Ld2Z5?= =?utf-8?B?WlMvZjZmRzVtMHJsZ3ZDWmhNNnh4UG1hbXJpWEx3RDBCQTF2WVFnVVlnRWFG?= =?utf-8?B?R09LSnk5VkJRZjNwYVNQR3NJOG1rdW9MMmRIRjZnek1GYWtZZzVyakJBb1hR?= =?utf-8?B?a0tjNldiSUpia1VudmgzMkxKU3U3bkJ5QS94L3RBN1hlSDZXQ1REMzc2dzZm?= =?utf-8?B?WmoxTnhRZTRWM3ZpK1ZkdFIyOGlqcFJWZ0VYNFl3S04yQ0VIRy9jT1dEQkpV?= =?utf-8?B?Vi9vcWhrV1NnakZLLzA1RWY2bkJBQlpkU09IR1FRTjkvRUU0UTkxa01jNDhL?= =?utf-8?B?aUVzcTRtVmszekpGaHpuWmRDOVBGd2ZzWkt3eDllSXA0dFE0OTFzSHNWemNN?= =?utf-8?B?cUVrbUdWa0Jva21ReGJseUxwQ3BBSTlmUWgvdTFYdmtrTjk5MGd2UEYrQUU2?= =?utf-8?B?azJ1M0U5VDJ4T3hxZ0V3U2lYV1JFUHQxZWF2cTBhR1JIeHF0YndCRGo2UGYw?= =?utf-8?B?b2xiSWtxbHUyNk9QU25Lc0dhL25VOXJOWWNWU2ovTTJPQlhCSWtuR25kMGVm?= =?utf-8?B?KytRaUtnZmdNbDcyaXJrN1RDNnFMSStRb2IvVXhNU3A0NWlNaksxdmNpNTlu?= =?utf-8?B?YjE3V0lybHkyTmQydW1ISCthQzdvTGk2K2dlRCt0bzlnVnQwZnR2c1VESHBC?= =?utf-8?B?YWVVVDBiYWZlRWwzemJWQ3FyNDJtSGM5QzkxWlpjK2ZDVVBBazN5ZExhbGZ5?= =?utf-8?B?WHAzUU9Gakx0eldGZWdWWXhNcGJOTGh1SjFubWd4ZVdGemJrbmVoaGI4Zko1?= =?utf-8?B?WWpVTjdKblNZK0JPR3hPR2dCZTVUK0FNaUVIbVEvTlFPSVM5OUZmYWJ0S3Jr?= =?utf-8?B?YmJ5NmMzd0hGdzJrVG9oYVlock9aQjBUYVkwNHRsZVI0UDBjZjk5eHh1c21V?= =?utf-8?B?OFNmNTMycXFpbFRKZDNFWWZpREdoWS9KcWs3WHRJbEN6Tzg3ZDI2K1EzR0ln?= =?utf-8?B?eTMrLzBSZnQwZ0dkRUszL2MzcGVjUTRuQVpJQ3l3VUhYbksvYTRMMWRIekVr?= =?utf-8?B?MytNb1ZrcWwzY2FKRS83OGFmc2EwUmFYYkFVSXRRUG5DT3MvS0k3RjV5SmVQ?= =?utf-8?B?OUZpbXlBeXJpNU9uRXM5eVdJMnVmUkRqREVpRkljRktUd2E3cHhUQUlTZndr?= =?utf-8?B?MkdwVmV3aklPUWEzcCtJR3czWC93R2VVb0czWmJrYjhnQS9RTDVuQ0M1ZEJV?= =?utf-8?B?YVZtZENxNDFEVm1QbUppTmFzK1N4SURxTXQrT3NpYU1seGJLamc4WWdKTXRy?= =?utf-8?B?dUFNOHBkRzY0eHJGZG1JTEVlUGhxVlcveG1jMlN5VmZRRHI1SU5ydG9HRjRY?= =?utf-8?B?NFlTS1pwa1EzQUlCNjBpTVZYQXdNd0xQRDRobXR1Slh2UFQzTlM5Yzg3N2w2?= =?utf-8?B?RzU0NCtKL0tnUHVWTzdOV0puVXRWeHE3MHBnMU9QV0FmbkdkT1NGY2w5dzBp?= =?utf-8?B?bW9xK0w1d05mUGJ4aUxnT2xoaTIzWUFlbDRtOFNhbzhrd2x4Zk1YcDhEcEkw?= =?utf-8?B?VzRhemhISDlSZmFvOGRKSHQ1VW4wMmFsbVc0U3RCNkhWUXhtVnVRRHpQV05P?= =?utf-8?B?TXVRMXprTHdCblV4Q1RRZWJGV1h4Ym1sYTA4ejlMclBmUjN6QlZGZEszTFV6?= =?utf-8?B?N0M5WEw3TElRSmJDRGc3R1pmekFVWXZTMjdWZ21jOUo1cUJoUGdLbnR3ck9D?= =?utf-8?B?TExGcmE1RmR4N3dMSURUUkJweW8raDZmMnEyM202SGhMdXN3dXJGYzI1czJl?= =?utf-8?B?NEExK0xrN1lqZ21pMmNIR2dMcTRFc2xTRkpndmZCTlZ4QmxwVnVqUnRNSFRs?= =?utf-8?B?elZLYUs0RzIveWRIN0FKb0FaNXcvWW1kTHcwUEFZSFVMcnZERStQYUVPTGcv?= =?utf-8?B?aVNLaXBmdDE0dnY1YTUxSEhEZWpGRU5Rcnlad3NPSFFnREw4aThQR3RmNG1a?= =?utf-8?B?cmxKS1I0cGJIZ2tjbkpJbFhGVFVSZlk2OUNCcWIyc3dlb2RSakM1ekJyZTFQ?= =?utf-8?B?dXVMNTQ0U05oc0RVNkhpQ2V6Tkg3bDBnUHVjeDlFaU5palJha2FtbEI0OUxW?= =?utf-8?Q?7Mnd0DD7U2Ymbzjv5QcASvo/u?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: e9c30b57-6155-440a-921b-08db198403fc X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2023 12:04:59.8505 (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: jHPEsS1+QlZBTHb+79iK8jm8LqvsRqbCJOvMU0gGqKe7M35UTtgJ+vKXyfRpY+PU X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5794 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 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.