From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; Wed, 7 Dec 2022 17:39:52 +0100 (CET)
Received: by mail-pj1-f47.google.com with SMTP id t17so18135882pjo.3
for <dev@dpdk.org>; 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 <stephen@networkplumber.org>
To: Ferruh Yigit <ferruh.yigit@amd.com>
Cc: Junfeng Guo <junfeng.guo@intel.com>, 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
<xiaoyun.li@intel.com>
Subject: Re: [PATCH] net/gve: add support for basic stats
Message-ID: <20221207083948.22af7cae@hermes.local>
In-Reply-To: <fa58bec0-c7e7-3c9f-fb03-7c4b773bc4be@amd.com>
References: <20221124073335.3985214-1-junfeng.guo@intel.com>
<fa58bec0-c7e7-3c9f-fb03-7c4b773bc4be@amd.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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
On Wed, 7 Dec 2022 15:09:08 +0000
Ferruh Yigit <ferruh.yigit@amd.com> 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 <xiaoyun.li@intel.com>
> > Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
>
> <...>
>
> > +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.