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 9E756A00C3; Wed, 7 Dec 2022 17:39:52 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8F04A410D7; Wed, 7 Dec 2022 17:39:52 +0100 (CET) Received: from mail-pj1-f47.google.com (mail-pj1-f47.google.com [209.85.216.47]) by mails.dpdk.org (Postfix) with ESMTP id 190FF40156 for ; Wed, 7 Dec 2022 17:39:52 +0100 (CET) Received: by mail-pj1-f47.google.com with SMTP id t17so18135882pjo.3 for ; Wed, 07 Dec 2022 08:39:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=N6V40eyrBSfOciW5HW3N3Tvql5jmizHLJnKjndheQ00=; b=0ku/E2YVUAYJR0PelJ8rD8c/7gKNgVlnkVtXZ9opOgmC2mkPjMlSt/BTDDtZBITwFb vJFUJsrvxNHS5BfFZh3JxHBEfHyoD5Ux+RcRn9oZVnIT9n53WSsq62qJPmCYqUA0kUf2 ZhITyOx+j1oY0f5OBSpIc8CuftJhxBIx/DDTnSq2KYPxdXFwlXCibh83Oxxs4JL5P4f3 4bOVsdFpnroD1/RAGBnU3GIz7mDD+GYJiyd/oABo8OLtlikQh6Efu169VRVsZuLnvxSt yLF23FvJb6bVRQPw3Hhc2mGiOMrOq3mxCizgwwmCKtICG//h538BwlhUg6QUCiaH059h GH/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=N6V40eyrBSfOciW5HW3N3Tvql5jmizHLJnKjndheQ00=; b=ptPE4gdmusXEfGZ4p80DY4LfGIYqf6MzcdvrdwAXwM+R4Nbs6KkN1gj9v8zifKwJsI EZYqgMa3nOZ0PqP6/nY9e46xtHEujvobZIkvUkA30jTh/bZ7vHRU2rQWWEth2ku8teLe mKHjd9yT4TSTuyNSzRJv7UM0Rtfxlla7kPp6yW4JefXnJhxXjU30RoAw5EeVSTen8NzD NQkroBbWzGof2dOO5GRbd3ewjM9QCVCiSPhqZG7gIIF0qDnZb6bDRbXuQhiv5drmpVQX Cg7Kkv/V2MWpFUA0vstPq/94Aynb7FcNNBZhQqRn+MIKbPRfsmnAQQWOBjA9rqSzh21S 8Spw== X-Gm-Message-State: ANoB5pk+KbXBS9Pejezu/gl6Tl3rDq6NRL1SKqF9N11BMGJ+Dr8gPi47 BTYtkXBcyNu43eHVx8gX7j/bMA== X-Google-Smtp-Source: AA0mqf5Uz3h9IGg7MyIl/CkHp0Ppqzx7odxTb7355FvLvHkzYInbq0DVuKloKrMPDIiZT0GIqRVumw== X-Received: by 2002:a17:902:cec7:b0:189:8f65:5792 with SMTP id d7-20020a170902cec700b001898f655792mr48127950plg.114.1670431191222; Wed, 07 Dec 2022 08:39:51 -0800 (PST) Received: from hermes.local (204-195-120-218.wavecable.com. [204.195.120.218]) by smtp.gmail.com with ESMTPSA id k11-20020a170902ce0b00b00176acd80f69sm11461491plg.102.2022.12.07.08.39.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 07 Dec 2022 08:39:50 -0800 (PST) Date: Wed, 7 Dec 2022 08:39:48 -0800 From: Stephen Hemminger To: Ferruh Yigit Cc: Junfeng Guo , qi.z.zhang@intel.com, jingjing.wu@intel.com, beilei.xing@intel.com, dev@dpdk.org, jeroendb@google.com, rushilg@google.com, jrkim@google.com, Xiaoyun Li Subject: Re: [PATCH] net/gve: add support for basic stats Message-ID: <20221207083948.22af7cae@hermes.local> In-Reply-To: References: <20221124073335.3985214-1-junfeng.guo@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 Wed, 7 Dec 2022 15:09:08 +0000 Ferruh Yigit wrote: > On 11/24/2022 7:33 AM, Junfeng Guo wrote: > > Add support for dev_ops of stats_get and stats_reset. > > > > Queue stats update will be moved into xstat [1], but the basic stats > > items may still be required. So we just keep the remaining ones and > > will implement the queue stats via xstats in the coming release. > > > > [1] > > https://elixir.bootlin.com/dpdk/v22.07/ \ > > source/doc/guides/rel_notes/deprecation.rst#L118 > > > > Signed-off-by: Xiaoyun Li > > Signed-off-by: Junfeng Guo > > <...> > > > +static int > > +gve_dev_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; > > + > > + stats->opackets += txq->packets; > > + stats->obytes += txq->bytes; > > + stats->oerrors += txq->errors; > > Hi Junfeng, Qi, Jingjing, Beilei, > > Above logic looks wrong to me, did you test it? > > If the 'gve_dev_stats_get()' called multiple times (without stats reset > in between), same values will be keep added to stats. > Some hw based implementations does this, because reading from stats > registers automatically reset those registers but this shouldn't be case > for this driver. > > I expect it to be something like: > > local_stats = 0 > foreach queue > local_stats += queue->stats > stats = local_stats The zero of local_stats is unnecessary. The only caller of the PMD stats_get is rte_ethdev_stats_get and it zeros the stats structure before calling the PMD. 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]; memset(stats, 0, sizeof(*stats)); ... stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed; return eth_err(port_id, (*dev->dev_ops->stats_get)(dev, stats)); If any PMD has extra memset in their stats get that could be removed.