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 8FC11A00C5; Mon, 19 Dec 2022 20:17:44 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3159440395; Mon, 19 Dec 2022 20:17:44 +0100 (CET) Received: from mail-vs1-f41.google.com (mail-vs1-f41.google.com [209.85.217.41]) by mails.dpdk.org (Postfix) with ESMTP id 503484021D for ; Mon, 19 Dec 2022 20:17:42 +0100 (CET) Received: by mail-vs1-f41.google.com with SMTP id k185so9754628vsc.2 for ; Mon, 19 Dec 2022 11:17:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=ez+DNXS+l3yn1hrn03qp8SEuludGoq0NvIn+Qe1bWTQ=; b=DyOjfRY76EG996FHTMCiuMdrY8yNLZDmyVU1OXGxT/LcdffC1DYXAI4iKOh55VL7mR HTgo9P44lG5XeT9wl2xica1EjDJ+Kuvjxqd02shDF1UMeQBXfzLNLBTFkQ3bgIpCln0C 7SZmVLvN8EqJUyBUq5bT3hO4RGFz2+qyolGrKhO9r83j8Sx54H7vibsPL0lgF4DRrTMi zRiAJX4wIHI4NBx3Lt0l0RF+dOgpnqiXJLbzMlfl31PPkEPkYFTCel9X5ey4eSL9DshT aroX9E/uZIpwH4HOjHVo2Xy5Pifvul3/7OoTqg/mYqPGfb2jcGjKaoIeKju50wEqMYWB LlZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ez+DNXS+l3yn1hrn03qp8SEuludGoq0NvIn+Qe1bWTQ=; b=SUKcaqpfxlM7A5i5z49iO23JkS3Rvlbkc+TxTs4EET2PxBjv4KltIuE6szhs1AehOJ SRawnJ7zQBLfQvxrCprArZqsKjIysI51cRllhruVkMSZgiTmw7LQo9iWmlaicTfP0pt7 HKEbYAKhgxMZcYE9xb/oST4WPkTEJo+w3/S5+YIXvWZyHeXTUDiaUCO4vvXK5VmFpEDg nB+oJ0ywM4iaLMLm69fn1qOTJMKlmpiRXm7xA3tC3ixf8ECV+MPQ3XVQlwsz9WYq0I2J 7xbONDGu+OrTDHPKbIOv5p73Ymcw8BGRuiUJUlgZZXlvaMStOEaRxLeWFwd15hjMmCJg vyPw== X-Gm-Message-State: ANoB5pl64PqqJ9C3nud0vzjccVCt5dbQxD6dmSBP1EH4S2By6LrbDINu wM2VNRuFhOAJ+Ekh92m7Rz0CUobZCdLTlgvJ1ykGrA== X-Google-Smtp-Source: AA0mqf7lqtzasppPVcoHfWvW/BKz0PWr1zK/BYqsPxB0lr1ix9DSqtajiscQToWMu+zwXbNuh2fTFppzSMZTblWfbVg= X-Received: by 2002:a05:6102:2262:b0:3b2:edd4:b645 with SMTP id v2-20020a056102226200b003b2edd4b645mr11164891vsd.60.1671477461579; Mon, 19 Dec 2022 11:17:41 -0800 (PST) MIME-Version: 1.0 References: <20221124073335.3985214-1-junfeng.guo@intel.com> <20221207083948.22af7cae@hermes.local> In-Reply-To: <20221207083948.22af7cae@hermes.local> From: Rushil Gupta Date: Mon, 19 Dec 2022 11:17:30 -0800 Message-ID: Subject: Re: [PATCH] net/gve: add support for basic stats To: Stephen Hemminger Cc: Ferruh Yigit , Junfeng Guo , qi.z.zhang@intel.com, jingjing.wu@intel.com, beilei.xing@intel.com, dev@dpdk.org, jeroendb@google.com, jrkim@google.com, Xiaoyun Li , Joshua Washington Content-Type: text/plain; charset="UTF-8" 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 Hi all Josh just found out some inconsistencies in the Tx/Rx statistics sum for all ports. Not sure if we can screenshot here but it goes like this: Tx-dropped for port0: 447034 Tx-dropped for port1: 0 Accumulated forward statistics for all ports: 807630 Please note that this issue is only with Tx-dropped (not Tx-packets/Tx-total). On Wed, Dec 7, 2022 at 8:39 AM Stephen Hemminger wrote: > > 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.