[dpdk-dev,1/4] ixgbe: expose extended error statistics

Message ID 1433525705-17041-2-git-send-email-maryam.tahhan@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Tahhan, Maryam June 5, 2015, 5:35 p.m. UTC
  Implement xstats_get() and xstats_reset() in dev_ops for ixgbe to expose
detailed error statistics to DPDK applications.

Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 160 +++++++++++++++++++++++++++++++++------
 1 file changed, 138 insertions(+), 22 deletions(-)
  

Comments

Tahhan, Maryam June 8, 2015, 1:26 p.m. UTC | #1
<snip>
> +	stats->idrop = hw_stats->mngpdc +
> +		hw_stats->fcoerpdc +
> +		total_qbrc;


Should use qprdc instead of total_qbrc

<snip>
  
Stephen Hemminger June 10, 2015, 12:51 a.m. UTC | #2
On Fri,  5 Jun 2015 18:35:02 +0100
Maryam Tahhan <maryam.tahhan@intel.com> wrote:

> Implement xstats_get() and xstats_reset() in dev_ops for ixgbe to expose
> detailed error statistics to DPDK applications.
> 
> Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>

Also, the bug where CRC is included in Tx byte count but
not in the Rx byte count has not been addressed.

You seem to have ignored my earlier patch.
  
Tahhan, Maryam June 10, 2015, 2:24 p.m. UTC | #3
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, June 10, 2015 1:51 AM
> To: Tahhan, Maryam
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/4] ixgbe: expose extended error statistics
> 
> On Fri,  5 Jun 2015 18:35:02 +0100
> Maryam Tahhan <maryam.tahhan@intel.com> wrote:
> 
> > Implement xstats_get() and xstats_reset() in dev_ops for ixgbe to
> > expose detailed error statistics to DPDK applications.
> >
> > Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
> 
> Also, the bug where CRC is included in Tx byte count but not in the Rx byte
> count has not been addressed.
> 
> You seem to have ignored my earlier patch.

I wasn't aware of your patch. But I will have a look.

Best Regards
Maryam
  
Thomas Monjalon June 17, 2015, 12:52 p.m. UTC | #4
2015-06-09 17:51, Stephen Hemminger:
> On Fri,  5 Jun 2015 18:35:02 +0100
> Maryam Tahhan <maryam.tahhan@intel.com> wrote:
> 
> > Implement xstats_get() and xstats_reset() in dev_ops for ixgbe to expose
> > detailed error statistics to DPDK applications.
> > 
> > Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
> 
> Also, the bug where CRC is included in Tx byte count but
> not in the Rx byte count has not been addressed.
> 
> You seem to have ignored my earlier patch.

Stephen, providing a link would be helpful.
Maryam, please check:
	http://dpdk.org/ml/archives/dev/2015-April/016096.html
	http://dpdk.org/dev/patchwork/patch/2495/
  

Patch

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0d9f9b2..f789aba 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,
@@ -330,7 +333,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,
@@ -408,6 +413,34 @@  static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
 	.mac_addr_remove      = ixgbevf_remove_mac_addr,
 };
 
+/* 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_multicast_packets", offsetof(struct ixgbe_hw_stats, mprc)},
+	{"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]))
+
 /**
  * Atomically reads the link status information from global
  * structure rte_eth_dev.
@@ -1739,26 +1772,18 @@  ixgbe_dev_close(struct rte_eth_dev *dev)
 	ixgbe_set_rar(hw, 0, hw->mac.addr, 0, IXGBE_RAH_AV);
 }
 
-/*
- * This function is based on ixgbe_update_stats_counters() in base/ixgbe.c
- */
 static void
-ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
+ixgbe_read_stats_registers(struct ixgbe_hw *hw, struct ixgbe_hw_stats *hw_stats,
+			   uint64_t *total_missed_rx, uint64_t *total_qbrc,
+			   uint64_t *total_qprc, uint64_t *rxnfgpc,
+			   uint64_t *txdgpc)
 {
-	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);
 	uint32_t bprc, lxon, lxoff, total;
-	uint64_t total_missed_rx, total_qbrc, total_qprc;
 	unsigned i;
 
-	total_missed_rx = 0;
-	total_qbrc = 0;
-	total_qprc = 0;
-
 	hw_stats->crcerrs += IXGBE_READ_REG(hw, IXGBE_CRCERRS);
 	hw_stats->illerrc += IXGBE_READ_REG(hw, IXGBE_ILLERRC);
+
 	hw_stats->errbc += IXGBE_READ_REG(hw, IXGBE_ERRBC);
 	hw_stats->mspdc += IXGBE_READ_REG(hw, IXGBE_MSPDC);
 
@@ -1768,7 +1793,7 @@  ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		/* global total per queue */
 		hw_stats->mpc[i] += mp;
 		/* Running comprehensive total for stats display */
-		total_missed_rx += hw_stats->mpc[i];
+		*total_missed_rx += hw_stats->mpc[i];
 		if (hw->mac.type == ixgbe_mac_82598EB)
 			hw_stats->rnbc[i] +=
 			    IXGBE_READ_REG(hw, IXGBE_RNBC(i));
@@ -1794,15 +1819,18 @@  ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		    ((uint64_t)IXGBE_READ_REG(hw, IXGBE_QBTC_H(i)) << 32);
 		hw_stats->qprdc[i] += IXGBE_READ_REG(hw, IXGBE_QPRDC(i));
 
-		total_qprc += hw_stats->qprc[i];
-		total_qbrc += hw_stats->qbrc[i];
+		*total_qprc += hw_stats->qprc[i];
+		*total_qbrc += hw_stats->qbrc[i];
 	}
+
 	hw_stats->mlfc += IXGBE_READ_REG(hw, IXGBE_MLFC);
 	hw_stats->mrfc += IXGBE_READ_REG(hw, IXGBE_MRFC);
 	hw_stats->rlec += IXGBE_READ_REG(hw, IXGBE_RLEC);
 
 	/* Note that gprc counts missed packets */
 	hw_stats->gprc += IXGBE_READ_REG(hw, IXGBE_GPRC);
+	*rxnfgpc += IXGBE_READ_REG(hw, IXGBE_RXNFGPC);
+	*txdgpc += IXGBE_READ_REG(hw, IXGBE_TXDGPC);
 
 	if (hw->mac.type != ixgbe_mac_82598EB) {
 		hw_stats->gorc += IXGBE_READ_REG(hw, IXGBE_GORCL);
@@ -1879,6 +1907,27 @@  ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		hw_stats->fcoedwrc += IXGBE_READ_REG(hw, IXGBE_FCOEDWRC);
 		hw_stats->fcoedwtc += IXGBE_READ_REG(hw, IXGBE_FCOEDWTC);
 	}
+}
+
+/* This function is based on ixgbe_update_stats_counters() in ixgbe/ixgbe.c */
+static void
+ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
+{
+	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, rxnfgpc, txdgpc;
+	unsigned i;
+
+	total_missed_rx = 0;
+	total_qbrc = 0;
+	total_qprc = 0;
+	rxnfgpc = 0;
+	txdgpc = 0;
+
+	ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc,
+				   &total_qprc, &rxnfgpc, &txdgpc);
 
 	if (stats == NULL)
 		return;
@@ -1888,7 +1937,6 @@  ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	stats->ibytes = total_qbrc;
 	stats->opackets = hw_stats->gptc;
 	stats->obytes = hw_stats->gotc;
-	stats->imcasts = hw_stats->mprc;
 
 	for (i = 0; i < IXGBE_QUEUE_STAT_COUNTERS; i++) {
 		stats->q_ipackets[i] = hw_stats->qprc[i];
@@ -1900,15 +1948,32 @@  ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 	/* Rx Errors */
 	stats->ibadcrc  = hw_stats->crcerrs;
-	stats->ibadlen  = hw_stats->rlec + hw_stats->ruc + hw_stats->roc;
+	stats->ibadlen  = hw_stats->rlec +
+		hw_stats->ruc +
+		hw_stats->roc;
 	stats->imissed  = total_missed_rx;
-	stats->ierrors  = stats->ibadcrc +
-	                  stats->ibadlen +
-	                  stats->imissed +
-	                  hw_stats->illerrc + hw_stats->errbc;
+	stats->imacerr  = stats->ibadlen +
+		hw_stats->xec +
+		hw_stats->crcerrs +
+		hw_stats->illerrc +
+		hw_stats->errbc +
+		hw_stats->mlfc +
+		hw_stats->mrfc +
+		hw_stats->rfc +
+		hw_stats->rjc +
+		hw_stats->fccrc +
+		hw_stats->fclast;
+	stats->iphyerr  = rxnfgpc - hw_stats->gprc;
+	stats->ierrors  = stats->imacerr +
+		stats->iphyerr +
+		stats->imissed;
+	stats->idrop = hw_stats->mngpdc +
+		hw_stats->fcoerpdc +
+		total_qbrc;
 
 	/* Tx Errors */
 	stats->oerrors  = 0;
+	stats->odrop = txdgpc - hw_stats->gptc;
 
 	/* XON/XOFF pause frames */
 	stats->tx_pause_xon  = hw_stats->lxontxc;
@@ -1936,6 +2001,57 @@  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, rxnfgpc, txdgpc;
+	unsigned i, count = RTE_NB_XSTATS;
+
+	if (n < count)
+		return count;
+
+	total_missed_rx = 0;
+	total_qbrc = 0;
+	total_qprc = 0;
+	rxnfgpc = 0;
+	txdgpc = 0;
+	count = 0;
+
+	ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc,
+				   &total_qprc, &rxnfgpc, &txdgpc);
+
+	if (!xstats)
+		return 0;
+
+	/* 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;
+}
+
+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)
 {