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 4BCA041CBF; Fri, 17 Feb 2023 13:34:11 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2F81840EE3; Fri, 17 Feb 2023 13:34:11 +0100 (CET) Received: from NAM04-MW2-obe.outbound.protection.outlook.com (mail-mw2nam04on2050.outbound.protection.outlook.com [40.107.101.50]) by mails.dpdk.org (Postfix) with ESMTP id 0A07640EE1 for ; Fri, 17 Feb 2023 13:34:09 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CXRamSN2hKGQy/Cx48six18/rpqSR2F8XjjAk8SDGB8HgCxgQsPJHt57UtrmEVtSE4NCe86suV4hqu1V3Nq0RYMEa5JvFXL5zWv0YICn3uMNUc/K9+kbs5nc6Mv4vCpNHfe5E6/rbfy2crQcCDA9hWFZbNLKXRdb3VIzNQ1IbkcKhY5HpchWjj4Dg8OA2CeL9ZXGRRuFnQwy8UeXLb1w9I5C7pOug64bgayD9XTcaTuMBFCnOehVXAAW9aL1RRHtoDHX1ZNA9CQ/cp7TVZ5xelu9vRdU7NNs4Lfg9DlJieHgGiKYAkcVlDk94MpDH599GC2BLxZ5pd1IPdL6GcefOA== 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=jKaoP/oaHy8jtnsiY6gBZm5rzbkk1BdrA4iC+lL8qCs=; b=RVEItZReY0Vs4TS1yjtoR83fJKirtJHDL5nzbTjN2mxnXwOv43JtIc7AFfmPitDmerk4BdIaJSW8V4scWigx5G6U1havgSwh6HMhE1NH5De0Op0MxOpXmFvGy2hV+J3p/OnaWYPPnnhWE1nm88yupx+kMf6k/TOAnoTcDcPjG5lOIgGSJOOaFxrIQEWrqALr6Evhu6ThTxYxbZ68qoSTYXTZf33UCNkSxGQj6RCM9aDM9U/raH7yH0ZEoxQ1ypllqhY1oZmKvm0cVpkkqdwHL/pjdVY5hMTl5eQvEVTpCRM9qIsCWBXaveOIYM7o2pTCLDm4GyvapExI+fAtW9TZHw== 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=jKaoP/oaHy8jtnsiY6gBZm5rzbkk1BdrA4iC+lL8qCs=; b=BjmArEbDHwmQNNaqL15+qB+GUwCKEsCWlL85hF7Qxc+Ajtw+zr3HVmUd0sDTIRZ71CwUWMf6kB5rPeuW1/H3aZuHEYegO70KfgBS3Mg+zy1oEJ3PdSDtWgFjN6wdeGl1hPv57i3xdwGgaVmm07zGJUE66WrTGiI1Snup4YH5ock= 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 BL0PR12MB5507.namprd12.prod.outlook.com (2603:10b6:208:1c4::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6111.15; Fri, 17 Feb 2023 12:34:07 +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.6111.013; Fri, 17 Feb 2023 12:34:06 +0000 Message-ID: <6e18557e-0d7d-c8f8-13f5-4523a1226431@amd.com> Date: Fri, 17 Feb 2023 12:34:00 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.7.2 Content-Language: en-US To: Levend Sayar , junfeng.guo@intel.com Cc: dev@dpdk.org References: <20230216185814.27830-1-levendsayar@gmail.com> <20230216185814.27830-2-levendsayar@gmail.com> From: Ferruh Yigit Subject: Re: [PATCH 2/2] net/gve: add extended statistics In-Reply-To: <20230216185814.27830-2-levendsayar@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P265CA0009.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:62::21) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|BL0PR12MB5507:EE_ X-MS-Office365-Filtering-Correlation-Id: 23605646-e0c3-4a73-f249-08db10e3428b X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: CKHJ+3ASa72WvXLdvLyA/+q/CpdewCBTZ659b/GV6ACvrsJH96tLXzZSXjSSGOCk7MdWo/5sLzvBlpI82KIumqy1ISv/4NcGek5SLNXjE4s5SQojV2LIu0ah907npPk5hed2BpA9D7RZdEB13WE9+fgP7gklDtrpEH08gCXyUvbrkwSoFpKsuJrK9Q9Wzb4VnKdz6y8yoTR46v8L3VpEc/QscxqRqx+4XZB23RVfGM5XPgrlOKDx5AnUkMZ6Ep5Awb9L7Cl49dQkG62ve+HaHdOkBC7Q7bxVerx629K82v5CXK1H8NyINlt0nA3bXINaMxd7A2O4UwZE2Flht9/kCs2ovzUNk0w0sWK0qwp2xiDOEDX3nqbzwXtne8bCvwLWUHhmZwotWwMOK55cY+j9BSFPrVL65CPFPWMssAj3ojCGsF1XqJ/OZksD/uY9KwPzd1weaN9EglJhmOTHgPPVxusw4p41WsQx/AI5Mn5N64oHsXa3G/gkh0z6M0CCE62jBZqLhApVub7HMoqDVzKiYFc05O9cGO0fpgKjxVXowHpStrLn6UgU+L+zXs37RjkdGPFao/Ku41vMkbpX4qyGBplgaVR0d7YTr4ZGmUj3Ch08Nezf1zJbrjuMFzUS1FtP3GKKuFZl5VccgLuIWqRZwEHiVgD5wJzrIKkQSb14ceViwufww+RudEEqTHOSA3SXp4aPXK3nWZCqzoYWidS8SZXodiNuj5ZCvE1sF4ho7M4= 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)(376002)(396003)(346002)(136003)(366004)(451199018)(31686004)(36756003)(66476007)(2906002)(8676002)(8936002)(66556008)(66946007)(30864003)(44832011)(5660300002)(38100700002)(4326008)(86362001)(31696002)(6506007)(6666004)(478600001)(6486002)(316002)(41300700001)(83380400001)(26005)(2616005)(186003)(6512007)(53546011)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eDQwL1RoUnlMNlBGeTV3L21GUWdHRmdPR05qVkxtLzRwYzgxbmZiK1oySTMw?= =?utf-8?B?VDRyUHZ6SWRXMzQ0eXpNSmYwUWRYOUpFM2FIWFRJYnZHNHI2Y1FFLzc0WWFw?= =?utf-8?B?U1lBbVl3MHNHRlErWlJHQnF4RW9Ka0dVNkt0TmNaaSs4R1A0OGlsbkdZdGV5?= =?utf-8?B?aGVTYndUVlNxcCt1b3RIVDd0TzV4Y3g0R2Y0ZzVSbVpPNVJHZnpzeEh0TVdt?= =?utf-8?B?bXA0R3hpeGxvVnFkd0twRXlkVWEzbE9EMHZadXZJeFAvOW82RnZBY2pLM0RK?= =?utf-8?B?RmVwMjkyWWYyV3JoSEoxOU1BWjErK0xqRFNvaDZlSW0rM3hsQVp2c2VxbWlU?= =?utf-8?B?MEJBTHVoK3hqS2Z3YTV5U0J3NjBVY0xJbTN5UU5taVgzblJHWW52VFRYTG0v?= =?utf-8?B?bHh3YitVQWZrVUtEVTFGc0FYV3k2dXVqcVF3eVFuNTRSbkE4cUFnQU03ZkNi?= =?utf-8?B?S1lTNUZ5RGtFeWRVN0k5cExTenR3MzFaV1NRODBhdGJCYUVrMUY4V1NkOGVE?= =?utf-8?B?c3FKWFp4djM1NXQ5aXdwSlo0S2hYdDd2UjFnOERqV3pCSkZtZ0U4Q3hRUUxv?= =?utf-8?B?Z0RjdStRV2tmRWl0bzIrMm5SNGh5VU1qU3ZLVWgyTHhNbDNGOU9SYUdRblVi?= =?utf-8?B?eGNnSmtaT1czb1liaWtxQStXeEVJZVVDWlNVMmVVR0JWeDh3RW9mKzBuc0F5?= =?utf-8?B?V3ArV1YvY2hqOUo5cUU2ZGptb05jbWxYVEVidTExZ0dBNDJsU0FLM1ZKeGdO?= =?utf-8?B?Yktua0ZlcVNJSUo5SWpXUzJGTUFKNDEvZ0xTd3BGMzV3L2wwQ1JDcmQ2d2sy?= =?utf-8?B?Y1pOZU5QOWo2STYxS1FtdjJxaUJCZThTSnlscHcvalNlZmNpd3owQkNpRXl1?= =?utf-8?B?cWRRemdiYkxjdlVjOWtDVEFjY29tRkJIZVpaalZnZm9TWG56NHFFT1ZBeVd1?= =?utf-8?B?d3VRUFFpMnR3eU9nNmRNMFFkdnVpWjFxTHRuaE5aUVc2ODMxOFdBOEVXVmVZ?= =?utf-8?B?SktTMFdFQmMvZGxCc1IzTGlaOVhNdzU4VFBnZ0pqSUlhTEdEWndBeTllTkxP?= =?utf-8?B?Uk5VWWZxSWI4dXI2ZVRsRTc5WWYvZHA0c2g2WFV4WUY4Vk1ZeW1SZkNHWUNo?= =?utf-8?B?dk1SemN0Ykt1cFZLemJSc25BRmx1dW9PTlZLdGR6YU5nbWhzL0NyV0h5bXEr?= =?utf-8?B?VzBraTRkWnVCU0REMnh3cjZLZDViNTRmcGFZUDk2aWFMaktoT3JvK1liMGxG?= =?utf-8?B?bzFlbUxoWnR3cHBzSlpaQklRYzhJTy85cGhtaWRicmkxZlh2bWpWU2VIdG9x?= =?utf-8?B?STkxK21EUjZRSTNLbG5BNkRyVEhMa1JnUkVFTjVOa1V2VW8vbWQ3RDJ6TER6?= =?utf-8?B?am1VZEs2MTlFWGtrN2phSzlHWlRGRFg0bUxvRlRidE5kZG5CNEtkS2ozcits?= =?utf-8?B?dU16NHhSS2Y0NE4vNlczc0N1V2hBQlJhc3JtUE1PSW8vYXdkVFkzSEpsUE95?= =?utf-8?B?bW9DNnMwMDFmekdqK1NLOXkyOEF1WjBTRUg0YnNhR0Y4dGdFdi91RDUvVlh2?= =?utf-8?B?L2NOTGlWODltYXVGeGFzQnlUdGJJTjVFTmI3cklXTGcxbFNaaUpTNUZ1UW1Z?= =?utf-8?B?RlFLalNwMU5wM0VodHh4Q01zRXpiMytXRTZKU2tjT3RqRHdVR3V3VnhXeDBD?= =?utf-8?B?enJ2Y0t3RDg4TE85YjhSUSthZ1h4cE1sM3FBaTF2TUxxR3hwcEpsVGNobTlO?= =?utf-8?B?Q1FxV0hrOHB6TnZ6Rm5BY0xiUWhOTjM4Z25XdlE5MnNUOXZtK0FySHQ5OHhX?= =?utf-8?B?ZG9PVERtNjVrcld5a2VuRGNkd0IyTjYvYTVsMHR6bS9pbnJXY3ZSZzNEdlZH?= =?utf-8?B?dG9jWDJkQTRKcm5FSW1LZ2JYSGNkQWpzT2lEV0RqR1YrWWhXZkdSczVZOFNr?= =?utf-8?B?d29xU0I4YlFqOWxmVVh6bkRBRng1Yjg2NmdVdXYxN09QR1RScDZETnI4OUFm?= =?utf-8?B?ZlZtWEZtMkVWYUVxeWpqVjdHRWNzRnowcmIxb1RjaHVDMk5aT25mb1Z5ZmV4?= =?utf-8?B?c2lWREFla3RZUkY0QkE4cTk5cnBsT2d5SGw2S1R0WUNRTDNBaUhGVzlTcE41?= =?utf-8?Q?bujPxgBijIxdUHAUK6h1CvaSz?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 23605646-e0c3-4a73-f249-08db10e3428b X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Feb 2023 12:34:06.6582 (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: tr1F4oPKqRCt7trlTjqW0Pl9fsyGAaY7l75H2bjCftkgxvfu3APhEHqAybpNdVOx X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR12MB5507 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/16/2023 6:58 PM, Levend Sayar wrote: > Google Virtual NIC PMD is enriched with extended statistics info. Only queue stats added to xstats, can you please highlight this in the commit log? > eth_dev_ops callback names are also synched with eth_dev_ops field names > Renaming eth_dev_ops is not related to xstats, and I am not sure if the change is necessary, can you please drop it from this patch? > Signed-off-by: Levend Sayar > --- > drivers/net/gve/gve_ethdev.c | 152 ++++++++++++++++++++++++++++++----- > drivers/net/gve/gve_rx.c | 8 +- > 2 files changed, 138 insertions(+), 22 deletions(-) > > diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c > index fef2458a16..e31fdce960 100644 > --- a/drivers/net/gve/gve_ethdev.c > +++ b/drivers/net/gve/gve_ethdev.c > @@ -266,7 +266,7 @@ gve_dev_close(struct rte_eth_dev *dev) > } > > static int > -gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) > +gve_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) > { > struct gve_priv *priv = dev->data->dev_private; > > @@ -319,15 +319,12 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) > } > > static int > -gve_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > +gve_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > { > uint16_t i; > > for (i = 0; i < dev->data->nb_tx_queues; i++) { > struct gve_tx_queue *txq = dev->data->tx_queues[i]; > - if (txq == NULL) > - continue; > - I assume check is removed because it is unnecessary, but again not directly related with the patch, can you also drop these from the patch to reduce the noise. > stats->opackets += txq->packets; > stats->obytes += txq->bytes; > stats->oerrors += txq->errors; > @@ -335,9 +332,6 @@ gve_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > > for (i = 0; i < dev->data->nb_rx_queues; i++) { > struct gve_rx_queue *rxq = dev->data->rx_queues[i]; > - if (rxq == NULL) > - continue; > - > stats->ipackets += rxq->packets; > stats->ibytes += rxq->bytes; > stats->ierrors += rxq->errors; > @@ -348,15 +342,12 @@ gve_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > } > > static int > -gve_dev_stats_reset(struct rte_eth_dev *dev) > +gve_stats_reset(struct rte_eth_dev *dev) > { > uint16_t i; > > for (i = 0; i < dev->data->nb_tx_queues; i++) { > struct gve_tx_queue *txq = dev->data->tx_queues[i]; > - if (txq == NULL) > - continue; > - > txq->packets = 0; > txq->bytes = 0; > txq->errors = 0; > @@ -364,9 +355,6 @@ gve_dev_stats_reset(struct rte_eth_dev *dev) > > for (i = 0; i < dev->data->nb_rx_queues; i++) { > struct gve_rx_queue *rxq = dev->data->rx_queues[i]; > - if (rxq == NULL) > - continue; > - > rxq->packets = 0; > rxq->bytes = 0; > rxq->errors = 0; > @@ -377,7 +365,7 @@ gve_dev_stats_reset(struct rte_eth_dev *dev) > } > > static int > -gve_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) > +gve_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) > { > struct gve_priv *priv = dev->data->dev_private; > int err; > @@ -403,20 +391,144 @@ gve_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) > return 0; > } > > +static int > +gve_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats, unsigned int n) > +{ > + if (xstats) { To reduce indentation (and increase readability) you can prefer: `` if !xstats return count; `` > + uint requested = n; uint? C#? Please prefer standard "unsigned int" type. > + uint64_t indx = 0; > + struct rte_eth_xstat *xstat = xstats; > + uint16_t i; > + > + for (i = 0; i < dev->data->nb_rx_queues; i++) { > + struct gve_rx_queue *rxq = dev->data->rx_queues[i]; > + xstat->id = indx++; > + xstat->value = rxq->packets; > + if (--requested == 0) > + return n; And in this case, ethdev layer does the checks and return accordingly, so need to try to fill the stats as much as possible, can you please double check the ethdev API? > + xstat++; > + > + xstat->id = indx++; > + xstat->value = rxq->bytes; > + if (--requested == 0) > + return n; > + xstat++; > + > + xstat->id = indx++; > + xstat->value = rxq->errors; > + if (--requested == 0) > + return n; > + xstat++; > + > + xstat->id = indx++; > + xstat->value = rxq->no_mbufs; > + if (--requested == 0) > + return n; > + xstat++; > + } > + > + for (i = 0; i < dev->data->nb_tx_queues; i++) { > + struct gve_tx_queue *txq = dev->data->tx_queues[i]; > + xstat->id = indx++; > + xstat->value = txq->packets; > + if (--requested == 0) > + return n; > + xstat++; > + > + xstat->id = indx++; > + xstat->value = txq->bytes; > + if (--requested == 0) > + return n; > + xstat++; > + > + xstat->id = indx++; > + xstat->value = txq->errors; > + if (--requested == 0) > + return n; > + xstat++; > + } This approach is error to prone and close to extension, many inplementations prefer to have an itnernal struct to link between names and values, something like: struct name_offset { char name[RTE_ETH_XSTATS_NAME_SIZE]; uint32_t offset; }; 'name' holds the xstat name, for this patch it will be only repeating part of name like 'packets', 'bytes', etc.. and you need to construct the full name on the fly, that is why it you may prefer to add type to above struct to diffrenciate Rx and Tx and use it for name creation, up to you. 'offset' holds the offset of corresponding value in a struct, for you it is "struct gve_rx_queue" & "struct gve_tx_queue", since there are two different structs it helps to create helper macro that gets offset from correct struct. struct name_offset rx_name_offset[] = { { "packets", offsetof(struct gve_rx_queue, packets) }, .... } for (i = 0; i < dev->data->nb_rx_queues; i++) { struct gve_rx_queue *rxq = dev->data->rx_queues[i]; addr = (char *)rxq + rx_name_offset[i].offset; xstats[index++].value = *addr; } for (i = 0; i < dev->data->nb_tx_queues; i++) { struct gve_tx_queue *txq = dev->data->tx_queues[i]; addr = (char *)txq + tx_name_offset[i].offset; xstats[index++].value = *addr; } There are many sample of this in other drivers. And since for this case instead of having fixed numbe of names, there are dynamiccaly changing queue names, > + } > + > + return (dev->data->nb_rx_queues * 4) + (dev->data->nb_tx_queues * 3); When above suggested 'name_offset' struct used, you can use size of it instead of hardcoded 4 & 3 values. With above sample, it becomes: return (dev->data->nb_rx_queues * RTE_DIM(rx_name_offset)) + (dev->data->nb_tx_queues * RTE_DIM(tx_name_offset)); > +} > + > +static int > +gve_xstats_reset(struct rte_eth_dev *dev) > +{ > + return gve_stats_reset(dev); > +} > + > +static int > +gve_xstats_get_names(struct rte_eth_dev *dev, struct rte_eth_xstat_name *xstats_names, > + unsigned int n) > +{ > + if (xstats_names) { > + uint requested = n; > + struct rte_eth_xstat_name *xstats_name = xstats_names; > + uint16_t i; > + > + for (i = 0; i < dev->data->nb_rx_queues; i++) { > + snprintf(xstats_name->name, sizeof(xstats_name->name), > + "rx_q%d_packets", i); > + if (--requested == 0) > + return n; > + xstats_name++; > + snprintf(xstats_name->name, sizeof(xstats_name->name), > + "rx_q%d_bytes", i); > + if (--requested == 0) > + return n; > + xstats_name++; > + snprintf(xstats_name->name, sizeof(xstats_name->name), > + "rx_q%d_errors", i); > + if (--requested == 0) > + return n; > + xstats_name++; > + snprintf(xstats_name->name, sizeof(xstats_name->name), > + "rx_q%d_no_mbufs", i); > + if (--requested == 0) > + return n; > + xstats_name++; > + } > + And again according above samples this becomes: for (i = 0; i < dev->data->nb_rx_queues; i++) { for (j = 0; j < RTE_DIM(rx_name_offset); j++) { snprintf(xstats_names[index++].name, sizeof(), "rx_q%d_%s", i, rx_name_offset[j].name); } > + for (i = 0; i < dev->data->nb_tx_queues; i++) { > + snprintf(xstats_name->name, sizeof(xstats_name->name), > + "tx_q%d_packets", i); > + if (--requested == 0) > + return n; > + xstats_name++; > + snprintf(xstats_name->name, sizeof(xstats_name->name), > + "tx_q%d_bytes", i); > + if (--requested == 0) > + return n; > + xstats_name++; > + snprintf(xstats_name->name, sizeof(xstats_name->name), > + "tx_q%d_errors", i); > + if (--requested == 0) > + return n; > + xstats_name++; > + } > + } > + > + return (dev->data->nb_rx_queues * 4) + (dev->data->nb_tx_queues * 3); > +} > + > static const struct eth_dev_ops gve_eth_dev_ops = { > .dev_configure = gve_dev_configure, > .dev_start = gve_dev_start, > .dev_stop = gve_dev_stop, > .dev_close = gve_dev_close, > - .dev_infos_get = gve_dev_info_get, > + .dev_infos_get = gve_dev_infos_get, > .rx_queue_setup = gve_rx_queue_setup, > .tx_queue_setup = gve_tx_queue_setup, > .rx_queue_release = gve_rx_queue_release, > .tx_queue_release = gve_tx_queue_release, > .link_update = gve_link_update, > - .stats_get = gve_dev_stats_get, > - .stats_reset = gve_dev_stats_reset, > - .mtu_set = gve_dev_mtu_set, > + .stats_get = gve_stats_get, > + .stats_reset = gve_stats_reset, > + .mtu_set = gve_mtu_set, > + .xstats_get = gve_xstats_get, > + .xstats_reset = gve_xstats_reset, > + .xstats_get_names = gve_xstats_get_names, > }; > > static void > diff --git a/drivers/net/gve/gve_rx.c b/drivers/net/gve/gve_rx.c > index 66fbcf3930..7687977003 100644 > --- a/drivers/net/gve/gve_rx.c > +++ b/drivers/net/gve/gve_rx.c > @@ -22,8 +22,10 @@ gve_rx_refill(struct gve_rx_queue *rxq) > if (diag < 0) { > for (i = 0; i < nb_alloc; i++) { > nmb = rte_pktmbuf_alloc(rxq->mpool); > - if (!nmb) > + if (!nmb) { > + rxq->no_mbufs++; Why this is needed, original code is as following: `` for (i = 0; i < nb_alloc; i++) { nmb = rte_pktmbuf_alloc(rxq->mpool); if (!nmb) break; rxq->sw_ring[idx + i] = nmb; } if (i != nb_alloc) { rxq->no_mbufs += nb_alloc - i; nb_alloc = i; } `` "if (i != nb_alloc)" block already takes care of 'rxq->no_mbufs' value, is an additional increment required in the for loop? And change is unrelated with the patch anyway. > break; > + } > rxq->sw_ring[idx + i] = nmb; > } > if (i != nb_alloc) { > @@ -57,8 +59,10 @@ gve_rx_refill(struct gve_rx_queue *rxq) > if (diag < 0) { > for (i = 0; i < nb_alloc; i++) { > nmb = rte_pktmbuf_alloc(rxq->mpool); > - if (!nmb) > + if (!nmb) { > + rxq->no_mbufs++; > break; > + } > rxq->sw_ring[idx + i] = nmb; > } > nb_alloc = i;