[dpdk-dev] [PATCH v4 0/8] Expose IXGBE extended stats to DPDK apps

[dpdk-dev] [PATCH v4 0/8] Expose IXGBE extended stats to DPDK apps

[Maryam Tahhan]

This patch set implements xstats_get() and xstats_reset() in dev_ops for
ixgbe to expose detailed error statistics to DPDK applications. The
dump_cfg application was extended to demonstrate the usage of
retrieving statistics for DPDK interfaces and renamed to proc_info
in order reflect this new functionality. This patch set also removes non
generic statistics from the statistics strings at the ethdev level and
marks the relevant registers as depricated in struct rte_eth_stats.

v2:
 - Fixed patch dependencies.
 - Broke down patches into smaller logical changes.

v3:
 - Removes non-generic stats fields in rte_stats_strings and deprecates
   the fields related to them in struct rte_eth_stats.
 - Modifies rte_eth_xstats_get() to return generic stats and extended stats.

v4:
 - Replace count use in the loop in ixgbe_dev_xstats_get() function definition with i.
 - Breakdown "ixgbe: add NIC specific stats removed from ethdev" into two patches, one
   that adds the stats and another that extends ierrors to include more error stats.
 - Remove second call to ixgbe_dev_xstats_get() from rte_eth_xstats_get().

Maryam Tahhan (8):
  ixgbe: move stats register reads to a new function
  ixgbe: add functions to get and reset xstats
  ethdev: expose extended error stats
  ethdev: remove HW specific stats in stats structs
  ixgbe: add NIC specific stats removed from ethdev
  ixgbe: return more errors in ierrors
  app: remove dump_cfg
  app: add a new app proc_info

 MAINTAINERS                      |   4 +
 app/Makefile                     |   2 +-
 app/dump_cfg/Makefile            |  45 ----
 app/dump_cfg/main.c              |  92 -------
 app/proc_info/Makefile           |  45 ++++
 app/proc_info/main.c             | 512 +++++++++++++++++++++++++++++++++++++++
 doc/guides/rel_notes/abi.rst     |  11 +
 drivers/net/ixgbe/ixgbe_ethdev.c | 194 ++++++++++++---
 lib/librte_ether/rte_ethdev.c    |  34 ++-
 lib/librte_ether/rte_ethdev.h    |  30 ++-
 mk/rte.sdktest.mk                |   4 +-
 11 files changed, 767 insertions(+), 206 deletions(-)
 delete mode 100644 app/dump_cfg/Makefile
 delete mode 100644 app/dump_cfg/main.c
 create mode 100644 app/proc_info/Makefile
 create mode 100644 app/proc_info/main.c

-- 
1.9.3

[dpdk-dev] [PATCH v4 1/8] ixgbe: move stats register reads to a new function

[Maryam Tahhan]

Move stats register reads to ixgbe_read_stats_registers() as it will be
used by the functions to retrieve stats and extended stats.

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

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index f18550c..927021f 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1761,24 +1761,16 @@ 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,
+						   uint64_t *total_qprdc)
 {
-	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);
@@ -1790,7 +1782,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));
@@ -1814,10 +1806,11 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		hw_stats->qbtc[i] += IXGBE_READ_REG(hw, IXGBE_QBTC_L(i));
 		hw_stats->qbtc[i] +=
 		    ((uint64_t)IXGBE_READ_REG(hw, IXGBE_QBTC_H(i)) << 32);
-		hw_stats->qprdc[i] += IXGBE_READ_REG(hw, IXGBE_QPRDC(i));
+		*total_qprdc += 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);
@@ -1825,6 +1818,8 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 	/* 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);
@@ -1902,6 +1897,35 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		hw_stats->fcoedwtc += IXGBE_READ_REG(hw, IXGBE_FCOEDWTC);
 	}
 
+	/* Flow Director Stats registers */
+	hw_stats->fdirmatch += IXGBE_READ_REG(hw, IXGBE_FDIRMATCH);
+	hw_stats->fdirmiss += IXGBE_READ_REG(hw, IXGBE_FDIRMISS);
+}
+
+/*
+ * 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, total_qprdc;
+	uint64_t rxnfgpc, txdgpc;
+	unsigned i;
+
+	total_missed_rx = 0;
+	total_qbrc = 0;
+	total_qprc = 0;
+	total_qprdc = 0;
+	rxnfgpc = 0;
+	txdgpc = 0;
+
+	ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc,
+			&total_qprc, &rxnfgpc, &txdgpc, &total_qprdc);
+
 	if (stats == NULL)
 		return;
 
@@ -1930,7 +1954,9 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	                  hw_stats->illerrc + hw_stats->errbc;
 
 	/* Tx Errors */
-	stats->oerrors  = 0;
+	/*txdgpc: packets that are DMA'ed*/
+	/*gptc: packets that are sent*/
+	stats->oerrors  = txdgpc - hw_stats->gptc;
 
 	/* XON/XOFF pause frames */
 	stats->tx_pause_xon  = hw_stats->lxontxc;
@@ -1939,8 +1965,6 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	stats->rx_pause_xoff = hw_stats->lxoffrxc;
 
 	/* Flow Director Stats registers */
-	hw_stats->fdirmatch += IXGBE_READ_REG(hw, IXGBE_FDIRMATCH);
-	hw_stats->fdirmiss += IXGBE_READ_REG(hw, IXGBE_FDIRMISS);
 	stats->fdirmatch = hw_stats->fdirmatch;
 	stats->fdirmiss = hw_stats->fdirmiss;
 }
-- 
1.9.3

[dpdk-dev] [PATCH v4 2/8] ixgbe: add functions to get and reset xstats

[Maryam Tahhan]

Implement ixgbe_dev_xstats_reset and ixgbe_dev_xstats_get.

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

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 927021f..7feb03c 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 IXGBE_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.
@@ -1982,6 +2014,61 @@ 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 = IXGBE_NB_XSTATS;
+
+	if (n < count)
+		return count;
+
+	total_missed_rx = 0;
+	total_qbrc = 0;
+	total_qprc = 0;
+	total_qprdc = 0;
+	rxnfgpc = 0;
+	txdgpc = 0;
+
+	ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc,
+							   &total_qprc, &rxnfgpc, &txdgpc, &total_qprdc);
+
+	/* If this is a reset xstats is NULL, and we have cleared the
+	 * registers by reading them.
+	 */
+	if (!xstats)
+		return 0;
+
+	/* Extended stats */
+	for (i = 0; i < IXGBE_NB_XSTATS; i++) {
+		snprintf(xstats[i].name, sizeof(xstats[i].name),
+				"%s", rte_ixgbe_stats_strings[i].name);
+		xstats[i].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, IXGBE_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)
 {
-- 
1.9.3

[dpdk-dev] [PATCH v4 3/8] ethdev: expose extended error stats

[Maryam Tahhan]

Extend rte_eth_xstats_get to retrieve additional stats from the device
driver as well the ethdev generic stats.

Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
---
 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;
 }
 
 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);
+		if (xcount < 0)
+			return xcount;
+	}
+
 	if (n < count)
-		return count;
+		return count + xcount;
 
 	/* now fill the xstats structure */
-
 	count = 0;
 	rte_eth_stats_get(port_id, &eth_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 */
-- 
1.9.3

[dpdk-dev] [PATCH v4 4/8] ethdev: remove HW specific stats in stats structs

[Maryam Tahhan]

Remove non generic stats in rte_stats_strings and mark the relevant
fields in struct rte_eth_stats as deprecated.

Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
---
 doc/guides/rel_notes/abi.rst  | 11 +++++++++++
 lib/librte_ether/rte_ethdev.c |  9 ---------
 lib/librte_ether/rte_ethdev.h | 30 ++++++++++++++++++++----------
 3 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/doc/guides/rel_notes/abi.rst b/doc/guides/rel_notes/abi.rst
index 110c486..90fe7fa 100644
--- a/doc/guides/rel_notes/abi.rst
+++ b/doc/guides/rel_notes/abi.rst
@@ -12,3 +12,14 @@ Examples of Deprecation Notices
 
 Deprecation Notices
 -------------------
+* The following fields have been deprecated in rte_eth_stats:
+  * uint64_t imissed
+  * uint64_t ibadcrc
+  * uint64_t ibadlen
+  * uint64_t imcasts
+  * uint64_t fdirmatch
+  * uint64_t fdirmiss
+  * uint64_t tx_pause_xon
+  * uint64_t rx_pause_xon
+  * uint64_t tx_pause_xoff
+  * uint64_t rx_pause_xoff
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index e392f60..003c548 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -142,17 +142,8 @@ static const struct rte_eth_xstats_name_off rte_stats_strings[] = {
 	{"rx_bytes", offsetof(struct rte_eth_stats, ibytes)},
 	{"tx_bytes", offsetof(struct rte_eth_stats, obytes)},
 	{"tx_errors", offsetof(struct rte_eth_stats, oerrors)},
-	{"rx_missed_errors", offsetof(struct rte_eth_stats, imissed)},
-	{"rx_crc_errors", offsetof(struct rte_eth_stats, ibadcrc)},
-	{"rx_bad_length_errors", offsetof(struct rte_eth_stats, ibadlen)},
 	{"rx_errors", offsetof(struct rte_eth_stats, ierrors)},
 	{"alloc_rx_buff_failed", offsetof(struct rte_eth_stats, rx_nombuf)},
-	{"fdir_match", offsetof(struct rte_eth_stats, fdirmatch)},
-	{"fdir_miss", offsetof(struct rte_eth_stats, fdirmiss)},
-	{"tx_flow_control_xon", offsetof(struct rte_eth_stats, tx_pause_xon)},
-	{"rx_flow_control_xon", offsetof(struct rte_eth_stats, rx_pause_xon)},
-	{"tx_flow_control_xoff", offsetof(struct rte_eth_stats, tx_pause_xoff)},
-	{"rx_flow_control_xoff", offsetof(struct rte_eth_stats, rx_pause_xoff)},
 };
 #define RTE_NB_STATS (sizeof(rte_stats_strings) / sizeof(rte_stats_strings[0]))
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index f1219ac..a38d49a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -193,19 +193,29 @@ struct rte_eth_stats {
 	uint64_t opackets;  /**< Total number of successfully transmitted packets.*/
 	uint64_t ibytes;    /**< Total number of successfully received bytes. */
 	uint64_t obytes;    /**< Total number of successfully transmitted bytes. */
-	uint64_t imissed;   /**< Total of RX missed packets (e.g full FIFO). */
-	uint64_t ibadcrc;   /**< Total of RX packets with CRC error. */
-	uint64_t ibadlen;   /**< Total of RX packets with bad length. */
+	/**< Deprecated; Total of RX missed packets (e.g full FIFO). */
+	uint64_t imissed;
+	/**< Deprecated; Total of RX packets with CRC error. */
+	uint64_t ibadcrc;
+	/**< Deprecated; Total of RX packets with bad length. */
+	uint64_t ibadlen;
 	uint64_t ierrors;   /**< Total number of erroneous received packets. */
 	uint64_t oerrors;   /**< Total number of failed transmitted packets. */
-	uint64_t imcasts;   /**< Total number of multicast received packets. */
+	uint64_t imcasts;
+	/**< Deprecated; Total number of multicast received packets. */
 	uint64_t rx_nombuf; /**< Total number of RX mbuf allocation failures. */
-	uint64_t fdirmatch; /**< Total number of RX packets matching a filter. */
-	uint64_t fdirmiss;  /**< Total number of RX packets not matching any filter. */
-	uint64_t tx_pause_xon;  /**< Total nb. of XON pause frame sent. */
-	uint64_t rx_pause_xon;  /**< Total nb. of XON pause frame received. */
-	uint64_t tx_pause_xoff; /**< Total nb. of XOFF pause frame sent. */
-	uint64_t rx_pause_xoff; /**< Total nb. of XOFF pause frame received. */
+	uint64_t fdirmatch;
+	/**< Deprecated; Total number of RX packets matching a filter. */
+	uint64_t fdirmiss;
+	/**< Deprecated; Total number of RX packets not matching any filter. */
+	uint64_t tx_pause_xon;
+	 /**< Deprecated; Total nb. of XON pause frame sent. */
+	uint64_t rx_pause_xon;
+	/**< Deprecated; Total nb. of XON pause frame received. */
+	uint64_t tx_pause_xoff;
+	/**< Deprecated; Total nb. of XOFF pause frame sent. */
+	uint64_t rx_pause_xoff;
+	/**< Deprecated; Total nb. of XOFF pause frame received. */
 	uint64_t q_ipackets[RTE_ETHDEV_QUEUE_STAT_CNTRS];
 	/**< Total number of queue RX packets. */
 	uint64_t q_opackets[RTE_ETHDEV_QUEUE_STAT_CNTRS];
-- 
1.9.3

[dpdk-dev] [PATCH v4 5/8] ixgbe: add NIC specific stats removed from ethdev

[Maryam Tahhan]

Add the NIC/HW specific stats that were removed from rte_ethdev.c to
the extended stats in ixgbe.

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

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 5971d41..6b1f32f 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -440,7 +440,15 @@ static const struct rte_ixgbe_xstats_name_off rte_ixgbe_stats_strings[] = {
 	{"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)},
+	{"rx_phy_multicast_packets", offsetof(struct ixgbe_hw_stats, mprc)},
 	{"mgmt_pkts_dropped", offsetof(struct ixgbe_hw_stats, mngpdc)},
+	{"rx_crc_errors", offsetof(struct ixgbe_hw_stats, crcerrs)},
+	{"fdir_match", offsetof(struct ixgbe_hw_stats, fdirmatch)},
+	{"fdir_miss", offsetof(struct ixgbe_hw_stats, fdirmiss)},
+	{"tx_flow_control_xon", offsetof(struct ixgbe_hw_stats, lxontxc)},
+	{"rx_flow_control_xon", offsetof(struct ixgbe_hw_stats, lxonrxc)},
+	{"tx_flow_control_xoff", offsetof(struct ixgbe_hw_stats, lxofftxc)},
+	{"rx_flow_control_xoff", offsetof(struct ixgbe_hw_stats, lxoffrxc)},
 };
 
 #define IXGBE_NB_XSTATS (sizeof(rte_ixgbe_stats_strings) /	\
@@ -1966,7 +1974,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];
@@ -1977,28 +1984,17 @@ 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->imissed  = total_missed_rx;
-	stats->ierrors  = stats->ibadcrc +
-	                  stats->ibadlen +
-	                  stats->imissed +
+	stats->ierrors  = hw_stats->crcerrs +
+	                  hw_stats->rlec +
+	                  hw_stats->ruc +
+	                  hw_stats->roc +
+	                  total_missed_rx +
 	                  hw_stats->illerrc + hw_stats->errbc;
 
 	/* Tx Errors */
 	/*txdgpc: packets that are DMA'ed*/
 	/*gptc: packets that are sent*/
 	stats->oerrors  = txdgpc - hw_stats->gptc;
-
-	/* XON/XOFF pause frames */
-	stats->tx_pause_xon  = hw_stats->lxontxc;
-	stats->rx_pause_xon  = hw_stats->lxonrxc;
-	stats->tx_pause_xoff = hw_stats->lxofftxc;
-	stats->rx_pause_xoff = hw_stats->lxoffrxc;
-
-	/* Flow Director Stats registers */
-	stats->fdirmatch = hw_stats->fdirmatch;
-	stats->fdirmiss = hw_stats->fdirmiss;
 }
 
 static void
@@ -2105,6 +2101,7 @@ ixgbevf_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	stats->opackets = hw_stats->vfgptc;
 	stats->obytes = hw_stats->vfgotc;
 	stats->imcasts = hw_stats->vfmprc;
+	/* stats->imcasts should be removed as imcasts is deprecated */
 }
 
 static void
-- 
1.9.3

[dpdk-dev] [PATCH v4 6/8] ixgbe: return more errors in ierrors

[Maryam Tahhan]

ierrors only included a base set of error statistics, this patch adds
additional error registers to ierrors.

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

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 6b1f32f..05371ec 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1989,7 +1989,16 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	                  hw_stats->ruc +
 	                  hw_stats->roc +
 	                  total_missed_rx +
-	                  hw_stats->illerrc + hw_stats->errbc;
+	                  hw_stats->illerrc +
+	                  hw_stats->errbc +
+	                  hw_stats->xec +
+	                  hw_stats->mlfc +
+	                  hw_stats->mrfc +
+	                  hw_stats->rfc +
+	                  hw_stats->rjc +
+	                  hw_stats->fccrc +
+	                  hw_stats->fclast +
+	                  (rxnfgpc - hw_stats->gprc); /* PHY Errors*/
 
 	/* Tx Errors */
 	/*txdgpc: packets that are DMA'ed*/
-- 
1.9.3

[dpdk-dev] [PATCH v4 7/8] app: remove dump_cfg

[Maryam Tahhan]

Remove the dump_cfg application, this will be replaced by a new app
called proc_info that will implement the same functionality as dump_cfg
and extend it to retrieve statistics for DPDK ports.

Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
---
 app/Makefile          |  1 -
 app/dump_cfg/Makefile | 45 -------------------------
 app/dump_cfg/main.c   | 92 ---------------------------------------------------
 3 files changed, 138 deletions(-)
 delete mode 100644 app/dump_cfg/Makefile
 delete mode 100644 app/dump_cfg/main.c

diff --git a/app/Makefile b/app/Makefile
index 50c670b..81bd222 100644
--- a/app/Makefile
+++ b/app/Makefile
@@ -36,6 +36,5 @@ DIRS-$(CONFIG_RTE_LIBRTE_ACL) += test-acl
 DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += test-pipeline
 DIRS-$(CONFIG_RTE_TEST_PMD) += test-pmd
 DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_test
-DIRS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += dump_cfg
 
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/app/dump_cfg/Makefile b/app/dump_cfg/Makefile
deleted file mode 100644
index 3257127..0000000
--- a/app/dump_cfg/Makefile
+++ /dev/null
@@ -1,45 +0,0 @@
-#   BSD LICENSE
-#
-#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
-#   All rights reserved.
-#
-#   Redistribution and use in source and binary forms, with or without
-#   modification, are permitted provided that the following conditions
-#   are met:
-#
-#     * Redistributions of source code must retain the above copyright
-#       notice, this list of conditions and the following disclaimer.
-#     * Redistributions in binary form must reproduce the above copyright
-#       notice, this list of conditions and the following disclaimer in
-#       the documentation and/or other materials provided with the
-#       distribution.
-#     * Neither the name of Intel Corporation nor the names of its
-#       contributors may be used to endorse or promote products derived
-#       from this software without specific prior written permission.
-#
-#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-include $(RTE_SDK)/mk/rte.vars.mk
-
-APP = dump_cfg
-
-CFLAGS += $(WERROR_FLAGS)
-
-# all source are stored in SRCS-y
-
-SRCS-y := main.c
-
-# this application needs libraries first
-DEPDIRS-y += lib
-
-include $(RTE_SDK)/mk/rte.app.mk
diff --git a/app/dump_cfg/main.c b/app/dump_cfg/main.c
deleted file mode 100644
index 127dbb1..0000000
--- a/app/dump_cfg/main.c
+++ /dev/null
@@ -1,92 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include <stdio.h>
-#include <string.h>
-#include <stdint.h>
-#include <errno.h>
-#include <stdarg.h>
-#include <inttypes.h>
-#include <sys/queue.h>
-#include <stdlib.h>
-
-#include <rte_memory.h>
-#include <rte_memzone.h>
-#include <rte_launch.h>
-#include <rte_tailq.h>
-#include <rte_eal.h>
-#include <rte_per_lcore.h>
-#include <rte_lcore.h>
-#include <rte_debug.h>
-#include <rte_log.h>
-#include <rte_atomic.h>
-#include <rte_branch_prediction.h>
-#include <rte_string_fns.h>
-
-int
-main(int argc, char **argv)
-{
-	int ret;
-	int i;
-	char c_flag[] = "-c1";
-	char n_flag[] = "-n4";
-	char mp_flag[] = "--proc-type=secondary";
-	char *argp[argc + 3];
-	argp[0] = argv[0];
-	argp[1] = c_flag;
-	argp[2] = n_flag;
-	argp[3] = mp_flag;
-
-	for(i = 1; i < argc; i++) {
-		argp[i + 3] = argv[i];
-	}
-	argc += 3;
-
-	ret = rte_eal_init(argc, argp);
-	if (ret < 0)
-		rte_panic("Cannot init EAL\n");
-
-	printf("----------- MEMORY_SEGMENTS -----------\n");
-	rte_dump_physmem_layout(stdout);
-	printf("--------- END_MEMORY_SEGMENTS ---------\n");
-
-	printf("------------ MEMORY_ZONES -------------\n");
-	rte_memzone_dump(stdout);
-	printf("---------- END_MEMORY_ZONES -----------\n");
-
-	printf("------------- TAIL_QUEUES -------------\n");
-	rte_dump_tailq(stdout);
-	printf("---------- END_TAIL_QUEUES ------------\n");
-
-	return 0;
-}
-- 
1.9.3

[dpdk-dev] [PATCH v4 8/8] app: add a new app proc_info

[Maryam Tahhan]

proc_info displays statistics information including extened stats for
given DPDK ports and dumps the memory information for DPDK.

Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
---
 MAINTAINERS            |   4 +
 app/Makefile           |   1 +
 app/proc_info/Makefile |  45 +++++
 app/proc_info/main.c   | 512 +++++++++++++++++++++++++++++++++++++++++++++++++
 mk/rte.sdktest.mk      |   4 +-
 5 files changed, 564 insertions(+), 2 deletions(-)
 create mode 100644 app/proc_info/Makefile
 create mode 100644 app/proc_info/main.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 5476a73..5d1c5e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -494,3 +494,7 @@ F: examples/tep_termination/
 F: examples/vmdq/
 F: examples/vmdq_dcb/
 F: doc/guides/sample_app_ug/vmdq_dcb_forwarding.rst
+
+M: Maryam Tahhan <maryam.tahhan@intel.com>
+M: John McNamara <john.mcnamara@intel.com>
+F: app/proc_info/
diff --git a/app/Makefile b/app/Makefile
index 81bd222..88c0bad 100644
--- a/app/Makefile
+++ b/app/Makefile
@@ -36,5 +36,6 @@ DIRS-$(CONFIG_RTE_LIBRTE_ACL) += test-acl
 DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += test-pipeline
 DIRS-$(CONFIG_RTE_TEST_PMD) += test-pmd
 DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_test
+DIRS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += proc_info
 
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/app/proc_info/Makefile b/app/proc_info/Makefile
new file mode 100644
index 0000000..6759547
--- /dev/null
+++ b/app/proc_info/Makefile
@@ -0,0 +1,45 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of Intel Corporation nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+APP = proc_info
+
+CFLAGS += $(WERROR_FLAGS)
+
+# all source are stored in SRCS-y
+
+SRCS-y := main.c
+
+# this application needs libraries first
+DEPDIRS-y += lib
+
+include $(RTE_SDK)/mk/rte.app.mk
diff --git a/app/proc_info/main.c b/app/proc_info/main.c
new file mode 100644
index 0000000..b2a495a
--- /dev/null
+++ b/app/proc_info/main.c
@@ -0,0 +1,512 @@
+/*
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+#include <errno.h>
+#include <stdarg.h>
+#include <inttypes.h>
+#include <sys/queue.h>
+#include <stdlib.h>
+#include <getopt.h>
+
+#include <rte_eal.h>
+#include <rte_config.h>
+#include <rte_common.h>
+#include <rte_debug.h>
+#include <rte_ethdev.h>
+#include <rte_malloc.h>
+#include <rte_memory.h>
+#include <rte_memzone.h>
+#include <rte_launch.h>
+#include <rte_tailq.h>
+#include <rte_per_lcore.h>
+#include <rte_lcore.h>
+#include <rte_debug.h>
+#include <rte_log.h>
+#include <rte_atomic.h>
+#include <rte_branch_prediction.h>
+#include <rte_string_fns.h>
+
+/* Maximum long option length for option parsing. */
+#define MAX_LONG_OPT_SZ 64
+#define RTE_LOGTYPE_APP RTE_LOGTYPE_USER1
+#define MAX_TX_QUEUE_STATS_MAPPINGS 1024 /* MAX_PORT of 32 @ 32 tx_queues/port */
+#define MAX_RX_QUEUE_STATS_MAPPINGS 4096 /* MAX_PORT of 32 @ 128 rx_queues/port */
+
+/**< mask of enabled ports */
+static uint32_t enabled_port_mask;
+/**< Enable stats. */
+static uint32_t enable_stats;
+/**< Enable xstats. */
+static uint32_t enable_xstats;
+/**< Enable stats reset. */
+static uint32_t reset_stats;
+/**< Enable xstats reset. */
+static uint32_t reset_xstats;
+/**< Enable memory info. */
+static uint32_t mem_info;
+/**
+ * The data structure associated with each port.
+ */
+struct rte_port {
+	uint8_t tx_queue_stats_mapping_enabled;
+	uint8_t rx_queue_stats_mapping_enabled;
+};
+
+struct rte_port *ports;	       /**< For all probed ethernet ports. */
+
+struct queue_stats_mappings {
+	uint8_t port_id;
+	uint16_t queue_id;
+	uint8_t stats_counter_id;
+} __rte_cache_aligned;
+
+static struct queue_stats_mappings
+		tx_queue_stats_mappings_array[MAX_TX_QUEUE_STATS_MAPPINGS];
+static struct queue_stats_mappings
+		rx_queue_stats_mappings_array[MAX_RX_QUEUE_STATS_MAPPINGS];
+
+static struct queue_stats_mappings *tx_queue_stats_mappings =
+		tx_queue_stats_mappings_array;
+static struct queue_stats_mappings *rx_queue_stats_mappings =
+		rx_queue_stats_mappings_array;
+
+static uint16_t nb_tx_queue_stats_mappings;
+static uint16_t nb_rx_queue_stats_mappings;
+
+/*
+ * Configurable number of RX/TX queues.
+ */
+static uint16_t nb_rxq = 1; /**< Number of RX queues per port. */
+static uint16_t nb_txq = 1; /**< Number of TX queues per port. */
+
+/**< display usage */
+static void
+proc_info_usage(const char *prgname)
+{
+	printf("%s [EAL options] -- -p PORTMASK\n"
+		"  -m to display DPDK memory zones, segments and TAILQ information\n"
+		"  -p PORTMASK: hexadecimal bitmask of ports to retrieve stats for\n"
+		"  --stats: to display port statistics, enabled by default\n"
+		"  --xstats: to display extended port statistics, disabled by "
+			"default\n"
+		"  --stats-reset: to reset port statistics\n"
+		"  --xstats-reset: to reset port extended statistics\n",
+		prgname);
+	return;
+}
+
+/*
+ * Parse the portmask provided at run time.
+ */
+static int
+parse_portmask(const char *portmask)
+{
+	char *end = NULL;
+	unsigned long pm;
+
+	errno = 0;
+
+	/* parse hexadecimal string */
+	pm = strtoul(portmask, &end, 16);
+	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0') ||
+		(errno != 0)) {
+		printf("%s ERROR parsing the port mask\n", __func__);
+		return -1;
+	}
+
+	if (pm == 0)
+		return -1;
+
+	return pm;
+
+}
+
+/* Parse the argument given in the command line of the application */
+static int
+proc_info_parse_args(int argc, char **argv)
+{
+	int opt;
+	int option_index;
+	char *prgname = argv[0];
+	static struct option long_option[] = {
+		{"stats", 0, NULL, 0},
+		{"stats-reset", 0, NULL, 0},
+		{"xstats", 0, NULL, 0},
+		{"xstats-reset", 0, NULL, 0},
+		{NULL, 0, 0, 0}
+	};
+
+	if (argc == 1)
+		proc_info_usage(prgname);
+
+	/* Parse command line */
+	while ((opt = getopt_long(argc, argv, "p:m",
+			long_option, &option_index)) != EOF) {
+		switch (opt) {
+		/* portmask */
+		case 'p':
+			enabled_port_mask = parse_portmask(optarg);
+			if (enabled_port_mask == 0) {
+				printf("invalid portmask\n");
+				proc_info_usage(prgname);
+				return -1;
+			}
+			break;
+		case 'm':
+			mem_info = 1;
+			break;
+		case 0:
+			/* Print stats */
+			if (!strncmp(long_option[option_index].name, "stats",
+					MAX_LONG_OPT_SZ))
+				enable_stats = 1;
+			/* Print xstats */
+			else if (!strncmp(long_option[option_index].name, "xstats",
+					MAX_LONG_OPT_SZ))
+				enable_xstats = 1;
+			/* Reset stats */
+			if (!strncmp(long_option[option_index].name, "stats-reset",
+					MAX_LONG_OPT_SZ))
+				reset_stats = 1;
+			/* Reset xstats */
+			else if (!strncmp(long_option[option_index].name, "xstats-reset",
+					MAX_LONG_OPT_SZ))
+				reset_xstats = 1;
+			break;
+
+		default:
+			proc_info_usage(prgname);
+			return -1;
+		}
+	}
+	return 0;
+}
+
+static void
+meminfo_display(void)
+{
+	printf("----------- MEMORY_SEGMENTS -----------\n");
+	rte_dump_physmem_layout(stdout);
+	printf("--------- END_MEMORY_SEGMENTS ---------\n");
+
+	printf("------------ MEMORY_ZONES -------------\n");
+	rte_memzone_dump(stdout);
+	printf("---------- END_MEMORY_ZONES -----------\n");
+
+	printf("------------- TAIL_QUEUES -------------\n");
+	rte_dump_tailq(stdout);
+	printf("---------- END_TAIL_QUEUES ------------\n");
+}
+
+static void
+nic_stats_display(uint8_t port_id)
+{
+	struct rte_eth_stats stats;
+	uint8_t i;
+	struct rte_port *port = &ports[port_id];
+
+	static const char *nic_stats_border = "########################";
+
+	rte_eth_stats_get(port_id, &stats);
+	printf("\n  %s NIC statistics for port %-2d %s\n",
+		   nic_stats_border, port_id, nic_stats_border);
+
+	if ((!port->rx_queue_stats_mapping_enabled) && (!port->tx_queue_stats_mapping_enabled)) {
+		printf("  RX-packets: %-10"PRIu64" RX-missed: %-10"PRIu64" RX-bytes:  "
+		       "%-"PRIu64"\n",
+		       stats.ipackets, stats.imissed, stats.ibytes);
+		printf("  RX-badcrc:  %-10"PRIu64" RX-badlen: %-10"PRIu64" RX-errors: "
+		       "%-"PRIu64"\n",
+		       stats.ibadcrc, stats.ibadlen, stats.ierrors);
+		printf("  RX-nombuf:  %-10"PRIu64"\n",
+		       stats.rx_nombuf);
+		printf("  TX-packets: %-10"PRIu64" TX-errors: %-10"PRIu64" TX-bytes:  "
+		       "%-"PRIu64"\n",
+		       stats.opackets, stats.oerrors, stats.obytes);
+	} else {
+		printf("  RX-packets:              %10"PRIu64"    RX-errors: %10"PRIu64
+		       "    RX-bytes: %10"PRIu64"\n",
+		       stats.ipackets, stats.ierrors, stats.ibytes);
+		printf("  RX-badcrc:               %10"PRIu64"    RX-badlen: %10"PRIu64
+		       "  RX-errors:  %10"PRIu64"\n",
+		       stats.ibadcrc, stats.ibadlen, stats.ierrors);
+		printf("  RX-nombuf:               %10"PRIu64"\n",
+		       stats.rx_nombuf);
+		printf("  TX-packets:              %10"PRIu64"    TX-errors: %10"PRIu64
+		       "    TX-bytes: %10"PRIu64"\n",
+		       stats.opackets, stats.oerrors, stats.obytes);
+	}
+
+	if (port->rx_queue_stats_mapping_enabled) {
+		printf("\n");
+		for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS; i++) {
+			printf("  Stats reg %2d RX-packets: %10"PRIu64
+			       "    RX-errors: %10"PRIu64
+			       "    RX-bytes: %10"PRIu64"\n",
+			       i, stats.q_ipackets[i], stats.q_errors[i], stats.q_ibytes[i]);
+		}
+	}
+	if (port->tx_queue_stats_mapping_enabled) {
+		printf("\n");
+		for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS; i++) {
+			printf("  Stats reg %2d TX-packets: %10"PRIu64
+			       "                             TX-bytes: %10"PRIu64"\n",
+			       i, stats.q_opackets[i], stats.q_obytes[i]);
+		}
+	}
+
+	/* Display statistics of XON/XOFF pause frames, if any. */
+	if ((stats.tx_pause_xon  | stats.rx_pause_xon |
+		 stats.tx_pause_xoff | stats.rx_pause_xoff) > 0) {
+		printf("  RX-XOFF:    %-10"PRIu64" RX-XON:    %-10"PRIu64"\n",
+			   stats.rx_pause_xoff, stats.rx_pause_xon);
+		printf("  TX-XOFF:    %-10"PRIu64" TX-XON:    %-10"PRIu64"\n",
+			   stats.tx_pause_xoff, stats.tx_pause_xon);
+	}
+	printf("  %s############################%s\n",
+		   nic_stats_border, nic_stats_border);
+}
+
+static void
+nic_stats_clear(uint8_t port_id)
+{
+	printf("\n Clearing NIC stats for port %d\n", port_id);
+	rte_eth_stats_reset(port_id);
+	printf("\n  NIC statistics for port %d cleared\n", port_id);
+}
+
+static void
+nic_xstats_display(uint8_t port_id)
+{
+	struct rte_eth_xstats *xstats;
+	int len, ret, i;
+	static const char *nic_stats_border = "########################";
+
+	len = rte_eth_xstats_get(port_id, NULL, 0);
+	if (len < 0) {
+		printf("Cannot get xstats count\n");
+		return;
+	}
+	xstats = malloc(sizeof(xstats[0]) * len);
+	if (xstats == NULL) {
+		printf("Cannot allocate memory for xstats\n");
+		return;
+	}
+
+	printf("###### NIC extended statistics for port %-2d #########\n",
+			   port_id);
+	printf("%s############################\n",
+			   nic_stats_border);
+	ret = rte_eth_xstats_get(port_id, xstats, len);
+	if (ret < 0 || ret > len) {
+		printf("Cannot get xstats\n");
+		free(xstats);
+		return;
+	}
+
+	for (i = 0; i < len; i++)
+		printf("%s: %"PRIu64"\n", xstats[i].name, xstats[i].value);
+
+	printf("%s############################\n",
+			   nic_stats_border);
+	free(xstats);
+}
+
+static void
+nic_xstats_clear(uint8_t port_id)
+{
+	printf("\n Clearing NIC xstats for port %d\n", port_id);
+	rte_eth_xstats_reset(port_id);
+	printf("\n  NIC extended statistics for port %d cleared\n", port_id);
+}
+
+static int
+set_tx_queue_stats_mapping_registers(uint8_t port_id, struct rte_port *port)
+{
+	uint16_t i;
+	int diag;
+	uint8_t mapping_found = 0;
+
+	for (i = 0; i < nb_tx_queue_stats_mappings; i++) {
+		if ((tx_queue_stats_mappings[i].port_id == port_id) &&
+				(tx_queue_stats_mappings[i].queue_id < nb_txq)) {
+			diag = rte_eth_dev_set_tx_queue_stats_mapping(port_id,
+					tx_queue_stats_mappings[i].queue_id,
+					tx_queue_stats_mappings[i].stats_counter_id);
+			if (diag != 0)
+				return diag;
+			mapping_found = 1;
+		}
+	}
+
+	if (mapping_found)
+		port->tx_queue_stats_mapping_enabled = 1;
+
+	return 0;
+}
+
+static int
+set_rx_queue_stats_mapping_registers(uint8_t port_id, struct rte_port *port)
+{
+	uint16_t i;
+	int diag;
+	uint8_t mapping_found = 0;
+
+	for (i = 0; i < nb_rx_queue_stats_mappings; i++) {
+		if ((rx_queue_stats_mappings[i].port_id == port_id) &&
+				(rx_queue_stats_mappings[i].queue_id < nb_rxq)) {
+			diag = rte_eth_dev_set_rx_queue_stats_mapping(port_id,
+					rx_queue_stats_mappings[i].queue_id,
+					rx_queue_stats_mappings[i].stats_counter_id);
+			if (diag != 0)
+				return diag;
+			mapping_found = 1;
+		}
+	}
+
+	if (mapping_found)
+		port->rx_queue_stats_mapping_enabled = 1;
+
+	return 0;
+}
+
+static void
+map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port *port)
+{
+	int diag = 0;
+
+	diag = set_tx_queue_stats_mapping_registers(pi, port);
+	if (diag != 0) {
+		if (diag == -ENOTSUP) {
+			port->tx_queue_stats_mapping_enabled = 0;
+			printf("TX queue stats mapping not supported port id=%d\n",
+					pi);
+		} else
+			rte_exit(EXIT_FAILURE,
+					"set_tx_queue_stats_mapping_registers "
+					"failed for port id=%d diag=%d\n",
+					pi, diag);
+	}
+
+	diag = set_rx_queue_stats_mapping_registers(pi, port);
+	if (diag != 0) {
+		if (diag == -ENOTSUP) {
+			port->rx_queue_stats_mapping_enabled = 0;
+			printf("RX queue stats mapping not supported port id=%d\n",
+					pi);
+		} else
+			rte_exit(EXIT_FAILURE,
+					"set_rx_queue_stats_mapping_registers "
+					"failed for port id=%d diag=%d\n",
+					pi, diag);
+	}
+}
+
+int
+main(int argc, char **argv)
+{
+	int ret;
+	int i;
+	char c_flag[] = "-c1";
+	char n_flag[] = "-n4";
+	char mp_flag[] = "--proc-type=secondary";
+	char *argp[argc + 3];
+	uint8_t nb_ports;
+
+	argp[0] = argv[0];
+	argp[1] = c_flag;
+	argp[2] = n_flag;
+	argp[3] = mp_flag;
+
+	for (i = 1; i < argc; i++)
+		argp[i + 3] = argv[i];
+
+	argc += 3;
+
+	ret = rte_eal_init(argc, argp);
+	if (ret < 0)
+		rte_panic("Cannot init EAL\n");
+
+	argc -= ret;
+	argv += (ret - 3);
+
+	/* parse app arguments */
+	ret = proc_info_parse_args(argc, argv);
+	if (ret < 0)
+		rte_exit(EXIT_FAILURE, "Invalid argument\n");
+
+	if (mem_info) {
+		meminfo_display();
+		return 0;
+	}
+
+	nb_ports = rte_eth_dev_count();
+	if (nb_ports == 0)
+		rte_exit(EXIT_FAILURE, "No Ethernet ports - bye\n");
+
+
+	if (nb_ports > RTE_MAX_ETHPORTS)
+		nb_ports = RTE_MAX_ETHPORTS;
+
+	/* Configuration of Ethernet ports. */
+	ports = rte_zmalloc("proc_info: ports",
+				sizeof(struct rte_port) * nb_ports,
+				RTE_CACHE_LINE_SIZE);
+	if (ports == NULL)
+		rte_exit(EXIT_FAILURE,
+				"rte_zmalloc(%d struct rte_port) failed\n",
+				nb_ports);
+
+	/* If no port mask was specified*/
+	if (enabled_port_mask == 0)
+		enabled_port_mask = 0xffff;
+
+	for (i = 0; i < nb_ports; i++) {
+		map_port_queue_stats_mapping_registers(i, &ports[i]);
+		if (enabled_port_mask & (1 << i)) {
+			if (enable_stats)
+				nic_stats_display(i);
+			else if (enable_xstats)
+				nic_xstats_display(i);
+			else if (reset_stats)
+				nic_stats_clear(i);
+			else if (reset_xstats)
+				nic_xstats_clear(i);
+		}
+	}
+
+	return 0;
+}
diff --git a/mk/rte.sdktest.mk b/mk/rte.sdktest.mk
index 2696142..59a29de 100644
--- a/mk/rte.sdktest.mk
+++ b/mk/rte.sdktest.mk
@@ -66,7 +66,7 @@ test fast_test ring_test mempool_test perf_test:
 	fi
 
 # this is a special target to ease the pain of running coverage tests
-# this runs all the autotests, cmdline_test script and dump_cfg
+# this runs all the autotests, cmdline_test script and proc_info
 coverage:
 	@mkdir -p $(AUTOTEST_DIR) ; \
 	cd $(AUTOTEST_DIR) ; \
@@ -78,7 +78,7 @@ coverage:
 			$(RTE_OUTPUT)/app/test \
 			$(RTE_TARGET) \
 			$(BLACKLIST) $(WHITELIST) ; \
-		$(RTE_OUTPUT)/app/dump_cfg --file-prefix=ring_perf ; \
+		$(RTE_OUTPUT)/app/proc_info --file-prefix=ring_perf -- -m; \
 	else \
 		echo "No test found, please do a 'make build' first, or specify O=" ;\
 	fi
-- 
1.9.3

Re: [dpdk-dev] [PATCH v4 8/8] app: add a new app proc_info

[Thomas Monjalon]

2015-07-05 18:40, Maryam Tahhan:
> proc_info displays statistics information including extened stats for

typo: extended

> given DPDK ports and dumps the memory information for DPDK.
> 
> Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
> ---
>  MAINTAINERS            |   4 +
>  app/Makefile           |   1 +
>  app/proc_info/Makefile |  45 +++++
>  app/proc_info/main.c   | 512 +++++++++++++++++++++++++++++++++++++++++++++++++
>  mk/rte.sdktest.mk      |   4 +-
>  5 files changed, 564 insertions(+), 2 deletions(-)

A doc would be useful.

> +DIRS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += proc_info

Why only Linux?

> +#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.

Too old :)

> +	if ((!port->rx_queue_stats_mapping_enabled) && (!port->tx_queue_stats_mapping_enabled)) {
> +		printf("  RX-packets: %-10"PRIu64" RX-missed: %-10"PRIu64" RX-bytes:  "
> +		       "%-"PRIu64"\n",
> +		       stats.ipackets, stats.imissed, stats.ibytes);
> +		printf("  RX-badcrc:  %-10"PRIu64" RX-badlen: %-10"PRIu64" RX-errors: "
> +		       "%-"PRIu64"\n",
> +		       stats.ibadcrc, stats.ibadlen, stats.ierrors);
> +		printf("  RX-nombuf:  %-10"PRIu64"\n",
> +		       stats.rx_nombuf);
> +		printf("  TX-packets: %-10"PRIu64" TX-errors: %-10"PRIu64" TX-bytes:  "
> +		       "%-"PRIu64"\n",
> +		       stats.opackets, stats.oerrors, stats.obytes);
> +	} else {
> +		printf("  RX-packets:              %10"PRIu64"    RX-errors: %10"PRIu64
> +		       "    RX-bytes: %10"PRIu64"\n",
> +		       stats.ipackets, stats.ierrors, stats.ibytes);
> +		printf("  RX-badcrc:               %10"PRIu64"    RX-badlen: %10"PRIu64
> +		       "  RX-errors:  %10"PRIu64"\n",
> +		       stats.ibadcrc, stats.ibadlen, stats.ierrors);
> +		printf("  RX-nombuf:               %10"PRIu64"\n",
> +		       stats.rx_nombuf);
> +		printf("  TX-packets:              %10"PRIu64"    TX-errors: %10"PRIu64
> +		       "    TX-bytes: %10"PRIu64"\n",
> +		       stats.opackets, stats.oerrors, stats.obytes);
> +	}

Why the formatting is different depending of device-specific stats mapping?

> +	if (port->rx_queue_stats_mapping_enabled) {
> +		printf("\n");
> +		for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS; i++) {
> +			printf("  Stats reg %2d RX-packets: %10"PRIu64
> +			       "    RX-errors: %10"PRIu64
> +			       "    RX-bytes: %10"PRIu64"\n",
> +			       i, stats.q_ipackets[i], stats.q_errors[i], stats.q_ibytes[i]);
> +		}
> +	}

Why restricting stats per queue to devices requiring a mapping?

> -		$(RTE_OUTPUT)/app/dump_cfg --file-prefix=ring_perf ; \

This should have been removed in previous patch.

Re: [dpdk-dev] [PATCH v4 8/8] app: add a new app proc_info

[Tahhan, Maryam]

> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, July 9, 2015 2:40 AM
> To: Tahhan, Maryam
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 8/8] app: add a new app proc_info
> 
> 2015-07-05 18:40, Maryam Tahhan:
> > proc_info displays statistics information including extened stats for
> 
> typo: extended
> 
> > given DPDK ports and dumps the memory information for DPDK.
> >
> > Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
> > ---
> >  MAINTAINERS            |   4 +
> >  app/Makefile           |   1 +
> >  app/proc_info/Makefile |  45 +++++
> >  app/proc_info/main.c   | 512
> +++++++++++++++++++++++++++++++++++++++++++++++++
> >  mk/rte.sdktest.mk      |   4 +-
> >  5 files changed, 564 insertions(+), 2 deletions(-)
> 
> A doc would be useful.

I will add a doc
> 
> > +DIRS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += proc_info
> 
> Why only Linux?

Dump_cfg was only supported on Linux also, I've only tested it on Linux.
> 
> > +#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> 
> Too old :)
> 
> > +	if ((!port->rx_queue_stats_mapping_enabled) && (!port-
> >tx_queue_stats_mapping_enabled)) {
> > +		printf("  RX-packets: %-10"PRIu64" RX-missed: %-10"PRIu64"
> RX-bytes:  "
> > +		       "%-"PRIu64"\n",
> > +		       stats.ipackets, stats.imissed, stats.ibytes);
> > +		printf("  RX-badcrc:  %-10"PRIu64" RX-badlen: %-10"PRIu64"
> RX-errors: "
> > +		       "%-"PRIu64"\n",
> > +		       stats.ibadcrc, stats.ibadlen, stats.ierrors);
> > +		printf("  RX-nombuf:  %-10"PRIu64"\n",
> > +		       stats.rx_nombuf);
> > +		printf("  TX-packets: %-10"PRIu64" TX-errors: %-10"PRIu64"
> TX-bytes:  "
> > +		       "%-"PRIu64"\n",
> > +		       stats.opackets, stats.oerrors, stats.obytes);
> > +	} else {
> > +		printf("  RX-packets:              %10"PRIu64"    RX-errors:
> %10"PRIu64
> > +		       "    RX-bytes: %10"PRIu64"\n",
> > +		       stats.ipackets, stats.ierrors, stats.ibytes);
> > +		printf("  RX-badcrc:               %10"PRIu64"    RX-badlen:
> %10"PRIu64
> > +		       "  RX-errors:  %10"PRIu64"\n",
> > +		       stats.ibadcrc, stats.ibadlen, stats.ierrors);
> > +		printf("  RX-nombuf:               %10"PRIu64"\n",
> > +		       stats.rx_nombuf);
> > +		printf("  TX-packets:              %10"PRIu64"    TX-errors:
> %10"PRIu64
> > +		       "    TX-bytes: %10"PRIu64"\n",
> > +		       stats.opackets, stats.oerrors, stats.obytes);
> > +	}
> 
> Why the formatting is different depending of device-specific stats mapping?

I will update this and remove the deprecated stats from being printed as well.
> 
> > +	if (port->rx_queue_stats_mapping_enabled) {
> > +		printf("\n");
> > +		for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS; i++) {
> > +			printf("  Stats reg %2d RX-packets: %10"PRIu64
> > +			       "    RX-errors: %10"PRIu64
> > +			       "    RX-bytes: %10"PRIu64"\n",
> > +			       i, stats.q_ipackets[i], stats.q_errors[i],
> stats.q_ibytes[i]);
> > +		}
> > +	}
> 
> Why restricting stats per queue to devices requiring a mapping?

I will change this
> 
> > -		$(RTE_OUTPUT)/app/dump_cfg --file-prefix=ring_perf ; \
> 
> This should have been removed in previous patch.

I will remove this in the previous patch

Re: [dpdk-dev] [PATCH v4 3/8] ethdev: expose extended error stats

[Olivier MATZ]

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 <maryam.tahhan@intel.com>
> ---
>   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, &eth_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

Re: [dpdk-dev] [PATCH v4 3/8] ethdev: expose extended error stats

[Tahhan, Maryam]

> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, July 15, 2015 10:19 AM
> To: Tahhan, Maryam; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 3/8] ethdev: expose extended error stats
> 
> 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.
> 

Hi Olivier
I have sent an updated patch set with the fixes recommended below.

Thanks for reviewing
Maryam
> 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 <maryam.tahhan@intel.com>
> > ---
> >   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, &eth_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