From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: [PATCH v3 3/7] ethdev: expose extended error stats Date: Fri, 03 Jul 2015 15:27:57 +0200 Message-ID: <55968DDD.6090409@6wind.com> References: <1435323558-169985-1-git-send-email-maryam.tahhan@intel.com> <1435323558-169985-4-git-send-email-maryam.tahhan@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit To: Maryam Tahhan , dev@dpdk.org Return-path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 668CC9E5 for ; Fri, 3 Jul 2015 15:28:08 +0200 (CEST) In-Reply-To: <1435323558-169985-4-git-send-email-maryam.tahhan@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Maryam, On 06/26/2015 02:59 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 | 2 +- > lib/librte_ether/rte_ethdev.c | 20 ++++++++++++++------ > 2 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c > index 0f62bcb..099e464 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -2035,7 +2035,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats, > total_qprdc = 0; > rxnfgpc = 0; > txdgpc = 0; > - count = 0; > + count = n; > > ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc, > &total_qprc, &rxnfgpc, &txdgpc, &total_qprdc); > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 02cd07f..6451621 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -1741,7 +1741,7 @@ 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, xcount = 0, i, q; > uint64_t val, *stats_ptr; > > if (!rte_eth_dev_is_valid_port(port_id)) { > @@ -1751,14 +1751,18 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats, > > 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 additional count size */ > + xcount = (*dev->dev_ops->xstats_get)(dev, xstats, 0); > + count += xcount; > + } > + > if (n < count) > return count; > > @@ -1805,6 +1809,10 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats, > } > } > > + /* Display stats after the generic stats*/ > + if (dev->dev_ops->xstats_get != NULL) > + (*dev->dev_ops->xstats_get)(dev, xstats, count); > + > return count; > } > I think we can avoid to have 2 calls to dev->dev_ops->xstats_get. The first call can be removed. The second call could be as below: if (dev->dev_ops->xstats_get != NULL) { ret = (*dev->dev_ops->xstats_get)(dev, &xstats[count], n - count); if (ret < 0) return ret; return ret + count; } - if the driver returns -1, the error code is propagated - if the driver returns a positive value, it is added to "count", which is the number of generic stats. It can be higher than "n" if the xstats table is too small Regards, Olivier