From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f169.google.com (mail-wi0-f169.google.com [209.85.212.169]) by dpdk.org (Postfix) with ESMTP id 92C85595E for ; Wed, 15 Jul 2015 11:19:05 +0200 (CEST) Received: by wibud3 with SMTP id ud3so75570371wib.1 for ; Wed, 15 Jul 2015 02:19:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=TEfJ+Kz/6ET8tHVl+WAcXdT+eFaIYj1hKA8DsoNXnA4=; b=KfikTbpMzbw+p9rNDU/8ssOjFDPSkm+0nPJAWL2rpl+2c2TF72kdi5EBBCi3uyHw0d sCTLUc7KhmSPHZdV7sdH1ToiFT/vHXPVknMFlgrpOPdgbID3bpEpo93jkNJ6UAwMrPey 4+iNRXuXEXrPpheRK+FxDARKV8GuvGoZ5s1alHQwxWVFCgKQuJNQ2lwJ6ZlKnEzaveYP l8z2Vpnrvnc4ptu8GcYUAFVwm8KzFr8SkETYQKGdVFO8RenRX40BPDg93I3Rr1K5j7Xi ny6Nlf5x6KLK7evcA3dp4DNd6BykF8TmDY+ecHmySDzM/WmBTB2l/x2kQoMTN9UlEzTX 2Y6Q== X-Gm-Message-State: ALoCoQkCeWwaKHhBlRdAJdvlOLmTqMdnZGNvsK55LBzHu6Xp2FeVMgCr0JGx8VMssuCHvCzZUKHA X-Received: by 10.194.81.67 with SMTP id y3mr6197423wjx.7.1436951945373; Wed, 15 Jul 2015 02:19:05 -0700 (PDT) Received: from [10.16.0.195] (6wind.net2.nerim.net. [213.41.151.210]) by smtp.gmail.com with ESMTPSA id gw7sm25667370wib.15.2015.07.15.02.19.04 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 15 Jul 2015 02:19:04 -0700 (PDT) Message-ID: <55A62582.9090208@6wind.com> Date: Wed, 15 Jul 2015 11:18:58 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.7.0 MIME-Version: 1.0 To: Maryam Tahhan , dev@dpdk.org References: <1436118000-132275-1-git-send-email-maryam.tahhan@intel.com> <1436118000-132275-4-git-send-email-maryam.tahhan@intel.com> In-Reply-To: <1436118000-132275-4-git-send-email-maryam.tahhan@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v4 3/8] ethdev: expose extended error stats X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Jul 2015 09:19:05 -0000 Hi Maryam, The API of the driver-specific function should be the same than the generic one, described in rte_ethdev.h. What I mean is: - the xstats pointer is the place where the xstats should be written by the driver - the n argument is the number of entries in the xstats structure The driver should not have to worry about the generic stats written by the generic layer. Please see some comments below to fix it. On 07/05/2015 07:39 PM, Maryam Tahhan wrote: > Extend rte_eth_xstats_get to retrieve additional stats from the device > driver as well the ethdev generic stats. > > Signed-off-by: Maryam Tahhan > --- > drivers/net/ixgbe/ixgbe_ethdev.c | 7 ++++--- > lib/librte_ether/rte_ethdev.c | 25 ++++++++++++++++--------- > 2 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c > index 7feb03c..5971d41 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -2035,6 +2035,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats, > total_qprdc = 0; > rxnfgpc = 0; > txdgpc = 0; > + count = n - IXGBE_NB_XSTATS; > > ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc, > &total_qprc, &rxnfgpc, &txdgpc, &total_qprdc); > @@ -2047,13 +2048,13 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats, > > /* Extended stats */ > for (i = 0; i < IXGBE_NB_XSTATS; i++) { > - snprintf(xstats[i].name, sizeof(xstats[i].name), > + snprintf(xstats[count].name, sizeof(xstats[i].name), > "%s", rte_ixgbe_stats_strings[i].name); > - xstats[i].value = *(uint64_t *)(((char *)hw_stats) + > + xstats[count++].value = *(uint64_t *)(((char *)hw_stats) + > rte_ixgbe_stats_strings[i].offset); > } > > - return count; > + return IXGBE_NB_XSTATS; The modifications in ixgbe driver can be removed: the patch 2/8 already provides everything that is required. > } > > static void > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index da915db..e392f60 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -1681,26 +1681,33 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats, > { > struct rte_eth_stats eth_stats; > struct rte_eth_dev *dev; > - unsigned count, i, q; > + unsigned count = 0, i, q; > + signed xcount = 0; > uint64_t val, *stats_ptr; > > VALID_PORTID_OR_ERR_RET(port_id, -EINVAL); > > dev = &rte_eth_devices[port_id]; > > - /* implemented by the driver */ > - if (dev->dev_ops->xstats_get != NULL) > - return (*dev->dev_ops->xstats_get)(dev, xstats, n); > - > - /* else, return generic statistics */ > + /* Return generic statistics */ > count = RTE_NB_STATS; > count += dev->data->nb_rx_queues * RTE_NB_RXQ_STATS; > count += dev->data->nb_tx_queues * RTE_NB_TXQ_STATS; > + > + /* implemented by the driver */ > + if (dev->dev_ops->xstats_get != NULL) { > + /* Retrieve the xstats from the driver at the end of the > + * xstats struct. > + */ > + xcount = (*dev->dev_ops->xstats_get)(dev, xstats, n); it should be: xcount = (*dev->dev_ops->xstats_get)(dev, &xstats[count], (n > count) ? n - count : 0); > + if (xcount < 0) > + return xcount; > + } > + > if (n < count) > - return count; > + return count + xcount; it should be: if (n < count + xcount) return count + xcount; > > /* now fill the xstats structure */ > - > count = 0; > rte_eth_stats_get(port_id, ð_stats); > > @@ -1742,7 +1749,7 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats, > } > } > > - return count; > + return count + xcount; > } > > /* reset ethdev extended statistics */ > Regards, Olivier