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 48FB741D9C; Tue, 28 Feb 2023 13:33:38 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DD1A4410D4; Tue, 28 Feb 2023 13:33:37 +0100 (CET) Received: from NAM10-BN7-obe.outbound.protection.outlook.com (mail-bn7nam10on2087.outbound.protection.outlook.com [40.107.92.87]) by mails.dpdk.org (Postfix) with ESMTP id 7D2FC4021F for ; Tue, 28 Feb 2023 13:33:36 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JmUI4lwTpJkWxvS0soGMPW/z/SW4liqz4yTtwaEaLSUgz9n2kw0BPlbc+pZNFRXhk2AQhvvLjUs5+G5V19xVAc1f+K578q0B5lhy/3Vf/tbg5HNottzB2EoA194h4WjM6roW7nEPqVnSfuqWXUiTT+2yZCurog9cblznc4++h+JlTMcgYBOCWJNHl5XZaVYdSTa2SdWas95PFVS2WFu2jCefgbKFSYXv5KjXGH7j0VYpzPP9e6E49qji1jQlbRJVc85wGsuwUwBiNhi4/k4HnLW0PISibJgC0lTaV9Ftx9vHQdMEYjG5Gbqk5fwub7aygMd3ONMhk++Yr9JGxptTKQ== 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=PbokuPLWSdQ0ja/uco5qSdySrw1hUdJEbcs4k/SB1ZM=; b=dnZ0pkvanvE9p53XnBfQjYamB+9NBCpuW2+bCNUStQ54N1r6Pv32jow2NnEEHUj8JoBLWp2jV+u2AZ5u9i2hfm4QOretz8cB341A34FuF0OTfKWhHhx1RHnrtMAFBYxheLKOjjTutVqfl9tpCKsBIC4MLd0dSFm7yO9ODtACDUUgxwb51zG8hHqpmjktGK9ckSFimrgyfc/9XWmrQP3wfTsKQGnbVXUYNw9SIAXKFgoFN8MbUC1xtQLeZBDKpSYrW1VBl4eFOUDVvS5CMCoutUq0ySlPKh9/NwJ6hHV4epqBOC1xzX50qput1UcTKGfmz135hyhKdhUteIdn1d5Sxw== 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=PbokuPLWSdQ0ja/uco5qSdySrw1hUdJEbcs4k/SB1ZM=; b=Bmem7SY+F8v8OFXklXpUPs1S/d6OuyFBXdgOsSmnOQGaP0y1jcOrhRsqQ54xtwPZfu+MM4+kLY1Uc9UU6RAzXWAZIjGDPD/uHkT5uLm6Due/3lrDMFzZtHJK+rIUqBDw3ftrKoO1dp1gDlkCaBOkbX3GxZlx0FiIPhgJiIimQ50= 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 PH7PR12MB7939.namprd12.prod.outlook.com (2603:10b6:510:278::18) 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 12:33:32 +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:33:32 +0000 Message-ID: <4ffc2fe6-50ea-4853-3dbb-616006d0d09e@amd.com> Date: Tue, 28 Feb 2023 12:33:26 +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 From: Ferruh Yigit 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> <57607ef8-8be6-967b-9f65-78a63e4e905c@amd.com> In-Reply-To: <57607ef8-8be6-967b-9f65-78a63e4e905c@amd.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P123CA0090.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:138::23) To MN2PR12MB4301.namprd12.prod.outlook.com (2603:10b6:208:1d4::22) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MN2PR12MB4301:EE_|PH7PR12MB7939:EE_ X-MS-Office365-Filtering-Correlation-Id: 77678d8d-a646-4e95-a041-08db1988008b X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: WkV409oy0z/ohXh4vT71ci3BSEpfnRjmOeINKlyakgEfqLTfF5nhJggitgia+58UJtHKTWjuZ8gH3azCvtPYSr/38uKOonv45efAyZkOUrUioIjmgFJdvYrqVgJwhDWyrnU3VEKBFMDxfXqxq+RzsB8pTqR8CxLWgsDk1BKU4/STB1XBEG77Fu5fWjYY8G6wJ3yadgg7TgzBKio9GgJQAvaOhYuvWcMd7MD682jgqK3scAHEFsh2shWAlQ8C2P8bTMxGJJc3HrRZ5WG7A3nCMT0dJnGE0bpDz/DNTFKQ78jK0btZ4vC16A3E11qB/+LxOkWO4viFQvkSMGfsU+IYQti6sd18+JJp6crIZyt6MIurrGx5GGg45kdFo29ubXihSae5MQrN6bN3PKSvFuGe6x+8SuGRRLOJYTtpo2PR3u0+SqkKOsHKcJrU2i6PvEhbGi5NOCQyexa7xHEXkYqhda+/D3MJXrcGHZxUANMQng7hYWISVKB8wWTJBP4x/COpAXTxM5Ypk6+WlBJ13eHPsxHP6jK60wXeKislyfaAsDfjdTFPWbcMgnAtpravDyPMASXujx0FfqFc8sdk6++vIUz6MNJNeG5dbknP+gx59HULTqsGziLIHDmwjJNZz/0nzzrD+vEL86svVyBpZ8v3MzayTY/Rje6b8IqWGZLQDcTnqZ2Kv464surpeHR8GQ/d92CadjbymK9BVSuarcfmHX+iNDAkH133zdfHs3epn3g= 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)(39860400002)(346002)(136003)(396003)(366004)(376002)(451199018)(31686004)(86362001)(36756003)(41300700001)(66946007)(66556008)(66476007)(8936002)(5660300002)(4326008)(8676002)(31696002)(44832011)(2906002)(6916009)(38100700002)(6666004)(6486002)(478600001)(54906003)(316002)(83380400001)(2616005)(186003)(26005)(6512007)(6506007)(53546011)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?d1YyV1JuUm5MZ0YvZXVwSkpQNXg1VzkyZlpZSzdSeWxXS2p0SmE0T0VzNnZs?= =?utf-8?B?ZnZicFZmUE9LM2U4ZXl3Ukt0T0FyRW5XOUE5aXF1MzBRa01vQ0lqRE9FYnhx?= =?utf-8?B?V2tWYW4veFFEaUVzMGppR0lQT2hiMkQwZFJzVXhnLzhPV1BqNmNDalYwaE1S?= =?utf-8?B?UjVlYmYrdENlWENzd2VIYTBYYTRWYmwyRG9PZkF5cnlnZXdlUWt5WmFWOU9v?= =?utf-8?B?azR2czM5cERhUnZzYk5zWnJwQUtLalpUaWZNNTZrU2U5WlZYSzllYllsaDZn?= =?utf-8?B?UFJsdTM4UHRxc2c4bkMvRmFSU0cyb29rblBWWVkvVkxWVHo0WVpZZ2hMb0k4?= =?utf-8?B?VExZSW5YcHFSZ3g2M3dvZ01UdVJKeEZDeTl2Q3dxTEM2aE1zY0xGTEhaeWdo?= =?utf-8?B?djJ5MThweTFrR0t2NFRja1pDOWV1MzFBdGxCYkh4K0RBMGxkZDQvWVJ3eWpM?= =?utf-8?B?YXdDVkoxbkZYQ2g0OEZhVWJnQ0JjVTF2UHBnaUd5UU1xUGZ2eDl5ZzRTdUM0?= =?utf-8?B?dkNpN3hvUGFoc0xjeDQ3cWVmbHpJSzZITUhCUkdyd0lIazN5d2hnNVM3TkVR?= =?utf-8?B?UWpyalRLVCtabDZ1RDU2ak1OR3RJbG12ZG5yWTFmRnEwMGlyTHBMZ1h1OHFV?= =?utf-8?B?d2V0a2NHUEpVSHZZb1crUUIxNUhoSTFiREZXUFRabmowb1dubWNuUFZOb2Yr?= =?utf-8?B?QTRhTjFXcDNNcE1IZnJGWFBTZFd2OE8vK05mcTFlejBJK3ZRanY0REdGOFNH?= =?utf-8?B?R3NOZ3I5S2VoN1phaGt2emZFNWxNaEdmNzU4VVA4cnoxYitldEJ6aEVZKzhp?= =?utf-8?B?QzcyRldjdGQyb3NzY2NPWFBXbWZoOEtKYmpBdjJxQkxNa3dDemphT3RtM2M1?= =?utf-8?B?VncyRGhNWUxodUNXZUxTQXdjUXZ5WDhtTUVhN3E3SDM0QmgySWdEdWpyTTNy?= =?utf-8?B?NDVqQzY3SmlrK3FodGlKZUNJelEzNUxCT0tzNzJMdFhxUGx3U0xMWFdwcThx?= =?utf-8?B?TnlrMlMrZjVXcmVIRVFuZVkxYnl0K1UrODdRUno5eURRNFUvajlBYTBVVmIr?= =?utf-8?B?aFVrUm1QRUdxS1lsazJDNHlGbmNpelhrN1BKdENCNWEwc25tdDZ2VU5tU3k1?= =?utf-8?B?YWo0MThLVTU5RE5ycUVuRW5YbXA4WnRCYUFTaEpMY2YyR1QwZHRQUmxZL0dt?= =?utf-8?B?ejRVUHVvcFNwb1pHcG1ESXRVU3pvai9Vb1A1dkpJOC9nNEJJNVRzd0dxRnUr?= =?utf-8?B?K2p3L1lQb3FYWE4xejN6d2lWWGZjSGRwZHJVU2RQc1BnWkFLT05LY1NTNWhJ?= =?utf-8?B?dUlOaS9CQkhRdlRVeDRhUFRQYWFOSDBhZ1ErMFhUNm5XbkpTYVM2dWhYbFds?= =?utf-8?B?NWRoSktvR1Q0NGN6VGVHME5NSVZMeDlpQk9weXEzYmNYMWZKNlVTZFBNWVRi?= =?utf-8?B?WGRqNVd0Zk5BMU14Vm5yUERoMDlGNHNvbjRQY1ZZck5ub2taOERGZHdzVU45?= =?utf-8?B?Q21uYXBrTGtrby95OFNyUEs2em04QnBjYjZya08xR3QxWDFrUEhKNlFYZno4?= =?utf-8?B?K1dKTUlqVlBDZVQvMzllcm4rdHU4K2FaK2VxTk1yVnYvUExyeHFOelZHMkdr?= =?utf-8?B?ZVoybkNJRGdJNWpVNyt6OUQycWplMERHTno1VkJaOU10TUMyeTRMUk9BSUpp?= =?utf-8?B?a2VmYTY0MXlCVmlhaWR2VjRNeUF2TGZGSm8rOG04ZmZiMWJLWndocDZWQjJO?= =?utf-8?B?ZGdnZzdVR3ZxMkJPdkcwSzJZWlB1U2ZZNExFNzFNQzZCUnJ6cVZLdldVbURo?= =?utf-8?B?bFNia0VUSjF1VTduQ0FTbFJTUUk5YTErbVJldnkwcTI5STZBd0QzUS9IelhP?= =?utf-8?B?VkVDeWtkUnJQNVd1R0tuWERyRUIwbFRIRzR0czBKeVhGVnY1Tlo2bzk5M1du?= =?utf-8?B?QVV1MkFnWHV3eTFQd1ppbTVhUmFhbWcyRmFFODFLUmlEbjB3YWpUNkhyQkU5?= =?utf-8?B?UkorWG02YjFGc05WdEJqamFndUJtN09SdWJuSkU1bDUvTjRHYkZVVEsyYlcx?= =?utf-8?B?aDBBbVQxZ1RPTndjUWtYcXFXYUFoWlQrVXRIUG52a2dNeEZvOWhxbjNKRnJ0?= =?utf-8?Q?Uul04Y0UXQCOu9Mqx30K4BeG2?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 77678d8d-a646-4e95-a041-08db1988008b 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:33:32.0342 (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: s6FWxDkGNI2rcGARDmHuhjRhy0JWsVMY+ybgs+Xl17B/8xqCTiXtJA/qmDznLChD X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB7939 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: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?