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 2A4D5A0093; Tue, 10 May 2022 13:29:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D477B406B4; Tue, 10 May 2022 13:29:48 +0200 (CEST) Received: from NAM11-DM6-obe.outbound.protection.outlook.com (mail-dm6nam11on2061.outbound.protection.outlook.com [40.107.223.61]) by mails.dpdk.org (Postfix) with ESMTP id BAD1C4069D; Tue, 10 May 2022 13:29:47 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=e18OosO42hFQkVSA1eC9LNvHw1cQPc4Eeffpe/uiFhsZSM9yxg3jauiAGW1ZgUWCSJQ3iKVqOYWu6yfazhPgckQAuR/04BrSe3lJloG1g4kuRn3lNuraNus69PpEyBYY7PUKb+EehxXUXI0DD04JMRLmkA+OYSksKSZtPBSzqzqJwwmHpO8RfYB4RCAvF7UFfz0SKKLmrf6GmwQHdfNeqaWoDVyzy/s+ytPmYddpJ54J2xD5ZVX5USWVNID13lbPggh7lclK7VLF6xes3pMp3a8aAg2dxi9nrswuZ8DuxLZmLIeuPE305Mtt1mf2XQvMrAbVZQ7pFYibdDZJCPjiJg== 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=kilxzIxqWNF7qUFJOLaOZ/1h6UBdcoLUTSaoPtjw/1U=; b=eahczZprt0CO7pJ0z4Oiczj5Jm7Of9LYubcIYqf9BsBCslaYitigBK6LBbh3FfOUu2cFRG9EYkZnvMNTw042nuVn29VWLL1I6HMMXX0bvV5+1V4ER1ZEgCDgDhVDOJK9/f4OnAnNWIl+MUdL5t5jtsspcQ06wodNuMWn+J5GlUUKdDChdaoJj/q0RAI1cnfOhN+JsGS9kgBMVNyZyQo7iKyxeu8QsqL3murfTff3nuSU+EWPMkrGvwtqoCroYzkCIAjctR3rNquSXc2slDArC4jiUQTat3FmZi6SPme4UIeErTUd2DAH1iH/2HJpwzgO3+p8tppjib/k5Qh2UXEQaA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 149.199.80.198) smtp.rcpttodomain=microsoft.com smtp.mailfrom=xilinx.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=xilinx.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xilinx.onmicrosoft.com; s=selector2-xilinx-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=kilxzIxqWNF7qUFJOLaOZ/1h6UBdcoLUTSaoPtjw/1U=; b=gSX+JoD4ZdIe7DJALE2ReG2Uelg56MzvWbBoYEa9LJMSGeEeYKlreBLtUTO1PvPxbnxLLwFGvCqFehpeHSc7EDyR8C6/rANTUB1o12fyMx3b/+KTRCkswqPsNiWYzX7CSqfyEQIdJ5ngESwUgUQovGjmhNN19ffbT/lx1uQj2NA= Received: from BN8PR04CA0050.namprd04.prod.outlook.com (2603:10b6:408:d4::24) by CY4PR0201MB3553.namprd02.prod.outlook.com (2603:10b6:910:8e::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5227.22; Tue, 10 May 2022 11:29:45 +0000 Received: from BN1NAM02FT045.eop-nam02.prod.protection.outlook.com (2603:10b6:408:d4:cafe::c4) by BN8PR04CA0050.outlook.office365.com (2603:10b6:408:d4::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5227.22 via Frontend Transport; Tue, 10 May 2022 11:29:44 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 149.199.80.198) smtp.mailfrom=xilinx.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=xilinx.com; Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates 149.199.80.198 as permitted sender) receiver=protection.outlook.com; client-ip=149.199.80.198; helo=xir-pvapexch02.xlnx.xilinx.com; Received: from xir-pvapexch02.xlnx.xilinx.com (149.199.80.198) by BN1NAM02FT045.mail.protection.outlook.com (10.13.2.156) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.5227.15 via Frontend Transport; Tue, 10 May 2022 11:29:44 +0000 Received: from xir-pvapexch02.xlnx.xilinx.com (172.21.17.17) by xir-pvapexch02.xlnx.xilinx.com (172.21.17.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.14; Tue, 10 May 2022 12:29:43 +0100 Received: from smtp.xilinx.com (172.21.105.197) by xir-pvapexch02.xlnx.xilinx.com (172.21.17.17) with Microsoft SMTP Server id 15.1.2176.14 via Frontend Transport; Tue, 10 May 2022 12:29:43 +0100 Envelope-to: longli@microsoft.com, stephen@networkplumber.org, longli@linuxonhyperv.com, dev@dpdk.org, sthemmin@microsoft.com, stable@dpdk.org Received: from [10.71.116.89] (port=23545) by smtp.xilinx.com with esmtp (Exim 4.90) (envelope-from ) id 1noO3j-0008K6-6e; Tue, 10 May 2022 12:29:43 +0100 Message-ID: Date: Tue, 10 May 2022 12:29:42 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [Patch v2] net/netvsc: report correct stats values Content-Language: en-US To: Long Li , Stephen Hemminger CC: "longli@linuxonhyperv.com" , "dev@dpdk.org" , Stephen Hemminger , "stable@dpdk.org" References: <1648143948-17821-1-git-send-email-longli@linuxonhyperv.com> <7f51e773-6ded-b736-fb02-5e3b391353b9@xilinx.com> <20220426154524.49502217@hermes.local> <924d7398-6c78-6318-52f3-d671edfc8aad@xilinx.com> <04de7df6-3d4a-21e5-7be5-15f2ef88be16@xilinx.com> <99a629d6-642e-db25-eeaa-a9eceec577cb@xilinx.com> <7809f41b-c21b-5ebe-d830-91015edb0cb8@xilinx.com> <20220505094026.22e74f43@hermes.local> <973e1c08-6fca-7664-72a3-0a25f5b73686@xilinx.com> From: Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 68c7c716-93f8-4ec5-fd80-08da327861fe X-MS-TrafficTypeDiagnostic: CY4PR0201MB3553:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: SbU+w8GjCncJrbQg4cNSZ4s8/HgFYudOyqTjIY92176Krgs/phiVUbfvK2kMUPq6h0MBsY++qcQfsiw5CzciRKSJDlbC7qdeHB+OqhbMdz+t1S4ZiM+qJefULWzwXRDTMAXjL+RiP8zfVua+i1PUxKNrIuKarE89q9l+Ov+RZK+BrGdmqlGb08yyX9UJmL+aMF4u7ABiZTlBzjm8AxX/QPrjToyZp0qt52KpcD1r/q+FBgRLDhZv7JAEbmrEXEEqPnmgf4rWBMa+3Vf6GiAIlkFkVlcFammxfkjBKyhNVCbrNsDhTeUp4lFxY9fKsxCCnH7n5ry5KjfQhS2/x964/zcN9QbdHTBEs54ruMBPBb6H3rFFqtUv6uspJzfASHzlAkCME5TRh3oLJyM5UtkMbwDNVHimZjCS0iz6PFCRozUDxeXt6wCdSIFCvh82kFwEX1iFJVax1kcpxM/qBBZeFpexj5Q3j59l4oY7QpdYXMZ42JH+R6+OgsWldYL5cZ9fq/F3lqA3P52fvPkImf9FEvynk4/nie1OVC5Am9pe4l4vFRDIJT+EyggyUIg37qxS1XawxDY2GhozhOy65GdrOAYJQHSuV7jQoLvFh51CtwLxyQWLl1OtR7YL1JnRYYSsDXusi0xCXpyFI0EVAPL1oNFbF6SIKpql2J3RgMEbY2EmMzDolOLqhj8r5SK9oEp4hSLbGmmwfMQ8EkBCkJAgG5fg0/ZV4cK5JHTPAQQh5RY= X-Forefront-Antispam-Report: CIP:149.199.80.198; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:xir-pvapexch02.xlnx.xilinx.com; PTR:unknown-80-198.xilinx.com; CAT:NONE; SFS:(13230001)(4636009)(36840700001)(40470700004)(46966006)(2616005)(31696002)(9786002)(44832011)(5660300002)(54906003)(8936002)(110136005)(2906002)(426003)(47076005)(186003)(316002)(4326008)(336012)(36860700001)(40460700003)(8676002)(70206006)(70586007)(508600001)(7636003)(82310400005)(53546011)(356005)(83380400001)(31686004)(26005)(36756003)(50156003)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 May 2022 11:29:44.6193 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 68c7c716-93f8-4ec5-fd80-08da327861fe X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c; Ip=[149.199.80.198]; Helo=[xir-pvapexch02.xlnx.xilinx.com] X-MS-Exchange-CrossTenant-AuthSource: BN1NAM02FT045.eop-nam02.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR0201MB3553 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 5/10/2022 6:33 AM, Long Li wrote: >> Subject: Re: [Patch v2] net/netvsc: report correct stats values >> >> On 5/5/2022 5:40 PM, Stephen Hemminger wrote: >>> On Thu, 5 May 2022 17:28:38 +0100 >>> Ferruh Yigit wrote: >>> >>>> On 5/4/2022 7:38 PM, Long Li wrote: >>>>>> Subject: Re: [Patch v2] net/netvsc: report correct stats values >>>>>> >>>>>> On 5/3/2022 9:48 PM, Long Li wrote: >>>>>>>> Subject: Re: [Patch v2] net/netvsc: report correct stats values >>>>>>>> >>>>>>>> On 5/3/2022 8:14 PM, Long Li wrote: >>>>>>>>>> Subject: Re: [Patch v2] net/netvsc: report correct stats values >>>>>>>>>> >>>>>>>>>> On 5/3/2022 7:18 PM, Long Li wrote: >>>>>>>>>>>> Subject: Re: [Patch v2] net/netvsc: report correct stats >>>>>>>>>>>> values >>>>>>>>>>>> >>>>>>>>>>>> On Tue, 26 Apr 2022 22:56:14 +0100 Ferruh Yigit >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>>> if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) { >>>>>>>>>>>>>> - stats->q_opackets[i] = txq->stats.packets; >>>>>>>>>>>>>> - stats->q_obytes[i] = txq->stats.bytes; >>>>>>>>>>>>>> + stats->q_opackets[i] += txq- >>> stats.packets; >>>>>>>>>>>>>> + stats->q_obytes[i] += txq->stats.bytes; >>>>>>>>>>>>> >>>>>>>>>>>>> This is per queue stats, 'stats->q_opackets[i]', in next >>>>>>>>>>>>> iteration of the loop, 'i' will be increased and 'txq' will >>>>>>>>>>>>> be updated, so as far as I can see the above change has no affect. >>>>>>>>>>>> >>>>>>>>>>>> Agree, that is why it was just assignment originally. >>>>>>>>>>> >>>>>>>>>>> The condition here is a little different. NETVSC is a master >>>>>>>>>>> device with >>>>>>>>>> another PMD running as a slave. When reporting stats values, it >>>>>>>>>> needs to add the values from the slave PMD. The original code >>>>>>>>>> just overwrites the values from its slave PMD. >>>>>>>>>> >>>>>>>>>> Where the initial values are coming from, 'hn_vf_stats_get()'? >>>>>>>>>> >>>>>>>>>> If 'hn_vf_stats_get()' fills the stats, what are the values >>>>>>>>>> kept in >>>>>>>>>> 'txq- >>>>>>>>> stats.*' >>>>>>>>>> in above updated loop? >>>>>>>>> >>>>>>>>> Yes, hn_vf_stats_get() fills in the stats from the slave PMD. >>>>>>>>> txq->stats >>>>>>>> values are from the master PMD. Those values are different and >>>>>>>> accounted separated from the values from the slave PMD. >>>>>>>> >>>>>>>> I see, since this is a little different than what most of the >>>>>>>> PMDs do, can you please put a little more info to the commit log? >>>>>>>> Or perhaps can add some comments to the code. >>>>>>> >>>>>>> Ok, will do. >>>>>>> >>>>>>>> >>>>>>>> And still 'stats->rx_nombuf' change is not required right? If so >>>>>>>> can you remove it in the next version? >>>>>>> >>>>>>> It is still needed. NETVSC unconditionally calls the slave PMD to >>>>>>> receive >>>>>> packets, even if it can't allocate a mbuf to receive a synthetic >>>>>> packet itself. The accounting of rx_nombuf is valid because the >>>>>> synthetic packets (to NETVSC) and VF packets (to slave PMD) are routed >> separately from Hyper-V. >>>>>> >>>>>> I am not referring to the "+=" update, my comment was because >>>>>> 'stats- >>>>>>> rx_nombuf' is overwritten in 'rte_eth_stats_get()' [1]. >>>>>> Is it still required? >>>>> >>>>> Yes, it is still needed. NETVSC calls the rte_eth_stats_get() on its slave PMD >> first, and stats->rx_nombuf is updated (overwritten) for its slave PMD. Afte that, >> it needs to add to its own dev->data->rx_mbuf_alloc_failed back to stats- >>> rx_nombuf. >>>>> >>>> >>>> But its own stat also will be overwritten (not in PMD function, but >>>> in ethdev layer). >>>> 'stats->rx_nombuf' assignment in the PMD seems has no effect and can >>>> be removed. >>>> >>>> I can't see how it is needed, can you please put a call stack to describe? >>> >>> This here: >>> >>> >>> int >>> rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats) { >>> struct rte_eth_dev *dev; >>> >>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >>> dev = &rte_eth_devices[port_id]; >>> >>> if (stats == NULL) { >>> RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u stats to >> NULL\n", >>> port_id); >>> return -EINVAL; >>> } >>> >>> memset(stats, 0, sizeof(*stats)); >>> >>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->stats_get, -ENOTSUP); >>> stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed; >>> return eth_err(port_id, (*dev->dev_ops->stats_get)(dev, stats)); } >>> >>> Will fill in rx_nombuf from the current rx_mbuf_alloc_failed. >>> But it happens before the PMD specific stats function. >>> >> >> I keep seeing the ethdev assignment as *after* the dev_ops, but it is not [1], so >> code is OK as it is. > > Hi Ferruh, > > Do you still want me to send a v3, or this patch is good as it is? > Yes can you please send a v3 with some more description in the commit log on the special case for the PMD, and perhaps some comments in the code. Thanks, ferruh > Long > >> >> >> [1] >> It seems assignment was after but it is fixed on the way: >> Commit 53ecfa24fbcd ("ethdev: fix overwriting driver-specific stats")