From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 1CD93106B for ; Fri, 3 Jul 2015 15:16:29 +0200 (CEST) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1ZB0u1-0006KZ-5N; Fri, 03 Jul 2015 15:21:22 +0200 Message-ID: <55968B1C.6070405@6wind.com> Date: Fri, 03 Jul 2015 15:16:12 +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: <1435323558-169985-1-git-send-email-maryam.tahhan@intel.com> <1435323558-169985-3-git-send-email-maryam.tahhan@intel.com> In-Reply-To: <1435323558-169985-3-git-send-email-maryam.tahhan@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset xstats 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: Fri, 03 Jul 2015 13:16:29 -0000 Hi Maryam, On 06/26/2015 02:59 PM, Maryam Tahhan wrote: > Implement ixgbe_dev_xstats_reset and ixgbe_dev_xstats_get. > > Signed-off-by: Maryam Tahhan > --- > drivers/net/ixgbe/ixgbe_ethdev.c | 85 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 85 insertions(+) > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c > index 927021f..0f62bcb 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -131,7 +131,10 @@ static int ixgbe_dev_link_update(struct rte_eth_dev *dev, > int wait_to_complete); > static void ixgbe_dev_stats_get(struct rte_eth_dev *dev, > struct rte_eth_stats *stats); > +static int ixgbe_dev_xstats_get(struct rte_eth_dev *dev, > + struct rte_eth_xstats *xstats, unsigned n); > static void ixgbe_dev_stats_reset(struct rte_eth_dev *dev); > +static void ixgbe_dev_xstats_reset(struct rte_eth_dev *dev); > static int ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev, > uint16_t queue_id, > uint8_t stat_idx, > @@ -334,7 +337,9 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = { > .allmulticast_disable = ixgbe_dev_allmulticast_disable, > .link_update = ixgbe_dev_link_update, > .stats_get = ixgbe_dev_stats_get, > + .xstats_get = ixgbe_dev_xstats_get, > .stats_reset = ixgbe_dev_stats_reset, > + .xstats_reset = ixgbe_dev_xstats_reset, > .queue_stats_mapping_set = ixgbe_dev_queue_stats_mapping_set, > .dev_infos_get = ixgbe_dev_info_get, > .mtu_set = ixgbe_dev_mtu_set, > @@ -414,6 +419,33 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = { > .set_mc_addr_list = ixgbe_dev_set_mc_addr_list, > }; > > +/* store statistics names and its offset in stats structure */ struct > +rte_ixgbe_xstats_name_off { > + char name[RTE_ETH_XSTATS_NAME_SIZE]; > + unsigned offset; > +}; > + > +static const struct rte_ixgbe_xstats_name_off rte_ixgbe_stats_strings[] = { > + {"rx_illegal_byte_err", offsetof(struct ixgbe_hw_stats, errbc)}, > + {"rx_len_err", offsetof(struct ixgbe_hw_stats, rlec)}, > + {"rx_undersize_count", offsetof(struct ixgbe_hw_stats, ruc)}, > + {"rx_oversize_count", offsetof(struct ixgbe_hw_stats, roc)}, > + {"rx_fragment_count", offsetof(struct ixgbe_hw_stats, rfc)}, > + {"rx_jabber_count", offsetof(struct ixgbe_hw_stats, rjc)}, > + {"l3_l4_xsum_error", offsetof(struct ixgbe_hw_stats, xec)}, > + {"mac_local_fault", offsetof(struct ixgbe_hw_stats, mlfc)}, > + {"mac_remote_fault", offsetof(struct ixgbe_hw_stats, mrfc)}, > + {"mac_short_pkt_discard", offsetof(struct ixgbe_hw_stats, mspdc)}, > + {"fccrc_error", offsetof(struct ixgbe_hw_stats, fccrc)}, > + {"fcoe_drop", offsetof(struct ixgbe_hw_stats, fcoerpdc)}, > + {"fc_last_error", offsetof(struct ixgbe_hw_stats, fclast)}, > + {"rx_broadcast_packets", offsetof(struct ixgbe_hw_stats, bprc)}, > + {"mgmt_pkts_dropped", offsetof(struct ixgbe_hw_stats, mngpdc)}, > +}; > + > +#define RTE_NB_XSTATS (sizeof(rte_ixgbe_stats_strings) / \ > + sizeof(rte_ixgbe_stats_strings[0])) > + Maybe RTE_NB_XSTATS should be renamed in IXGBE_NB_XSTATS? > /** > * Atomically reads the link status information from global > * structure rte_eth_dev. > @@ -1982,6 +2014,59 @@ ixgbe_dev_stats_reset(struct rte_eth_dev *dev) > memset(stats, 0, sizeof(*stats)); > } > > +static int > +ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats, > + unsigned n) > +{ > + struct ixgbe_hw *hw = > + IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + struct ixgbe_hw_stats *hw_stats = > + IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private); > + uint64_t total_missed_rx, total_qbrc, total_qprc, total_qprdc; > + uint64_t rxnfgpc, txdgpc; > + unsigned i, count = RTE_NB_XSTATS; > + > + if (n == 0) > + return count; I think it does not exactly matches the API described in rte_ethdev.h: * @return * - positive value lower or equal to n: success. The return value * is the number of entries filled in the stats table. * - positive value higher than n: error, the given statistics table * is too small. The return value corresponds to the size that should * be given to succeed. The entries in the table are not valid and * shall not be used by the caller. * - negative value on error (invalid port id) I think it should be: if (n < count) return count > + > + total_missed_rx = 0; > + total_qbrc = 0; > + total_qprc = 0; > + total_qprdc = 0; > + rxnfgpc = 0; > + txdgpc = 0; > + count = 0; > + > + ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc, > + &total_qprc, &rxnfgpc, &txdgpc, &total_qprdc); > + > + if (!xstats) > + return 0; this cannot happen except if n == 0. This condition is already tested above, and "count" should be returned. > + > + /* Error stats */ > + for (i = 0; i < RTE_NB_XSTATS; i++) { > + snprintf(xstats[count].name, sizeof(xstats[count].name), > + "%s", rte_ixgbe_stats_strings[i].name); > + xstats[count++].value = *(uint64_t *)(((char *)hw_stats) + > + rte_ixgbe_stats_strings[i].offset); > + } > + > + return count; > +} Shouldn't it be xstats[i] instead of xstats[count] ? Does it work when using "show port in test-pmd"? > + > +static void > +ixgbe_dev_xstats_reset(struct rte_eth_dev *dev) > +{ > + struct ixgbe_hw_stats *stats = > + IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private); > + > + /* HW registers are cleared on read */ > + ixgbe_dev_xstats_get(dev, NULL, RTE_NB_XSTATS); > + > + /* Reset software totals */ > + memset(stats, 0, sizeof(*stats)); > +} > + > static void > ixgbevf_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) > { >