[dpdk-dev] [PATCH v3 0/7] Expose IXGBE extended stats to DPDK apps

[dpdk-dev] [PATCH v3 0/7] 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.

Maryam Tahhan (7):
  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
  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 | 192 ++++++++++++---
 lib/librte_ether/rte_ethdev.c    |  29 ++-
 lib/librte_ether/rte_ethdev.h    |  30 ++-
 mk/rte.sdktest.mk                |   4 +-
 11 files changed, 762 insertions(+), 204 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 v3 1/7] 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 v3 2/7] 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 | 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]))
+
 /**
  * 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;
+
+	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;
+
+	/* 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)
 {
-- 
1.9.3

Re: [dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset xstats

[Olivier MATZ]

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 <maryam.tahhan@intel.com>
> ---
>  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)
>  {
> 

Re: [dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset xstats

[Olivier MATZ]

On 07/03/2015 03:16 PM, Olivier MATZ wrote:
> 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 <maryam.tahhan@intel.com>
>> ---
>>  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"?

ok I missed the 'count = 0' above.
So why using count instead of i ?

Also, I think it would be clearer to use the constant IXGBE_NB_XSTATS
instead of count at the beginning of the function.


> 
>> +
>> +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)
>>  {
>>

Re: [dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset xstats

[Tahhan, Maryam]

<snip>
> >
> >> +
> >> +	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"?
> 
> ok I missed the 'count = 0' above.
> So why using count instead of i ?
> 
> Also, I think it would be clearer to use the constant IXGBE_NB_XSTATS instead
> of count at the beginning of the function.
> 
> 

Hi Olivier

Actually, count should not be 0, it should be n, which is the passed in index from rte_eth_xstats_get()...

Because we fill out the generic stats first in rte_eth_xstats_get() then we call ixgbe_dev_xstats_get to fill out the rest.

I will fix this up
> >
> >> +
> >> +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)  {
> >>

Re: [dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset xstats

[Tahhan, Maryam]

> 
> 
> <snip>
> > >
> > >> +
> > >> +	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"?
> >
> > ok I missed the 'count = 0' above.
> > So why using count instead of i ?
> >
> > Also, I think it would be clearer to use the constant IXGBE_NB_XSTATS
> > instead of count at the beginning of the function.
> >
> >
> 
> Hi Olivier
> 
> Actually, count should not be 0, it should be n, which is the passed in index
> from rte_eth_xstats_get()...
> 
> Because we fill out the generic stats first in rte_eth_xstats_get() then we call
> ixgbe_dev_xstats_get to fill out the rest.
> 
> I will fix this up
Hi Olivier, 

I confused this change with a subsequent patch in the patch set... so yes for this patch you are correct we can just use i... and leave count as 0. 

in a subsequent patch I modify count to start at n, which is a passed in index from in rte_eth_xstats_get()...

so I will change count for i in the loop here. 


> > >
> > >> +
> > >> +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)  {
> > >>

Re: [dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset xstats

[Tahhan, Maryam]

Best Regards, 
Maryam

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Friday, July 3, 2015 2:16 PM
> To: Tahhan, Maryam; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset
> xstats
> 
> 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 <maryam.tahhan@intel.com>
> > ---
> >  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
> 

I was using 0 on the first call just to return the size of the extended stats structure that needs to be allocated by the application. It's passed as 0 from ethdev_get_stats to retrieve the size. But I will update it.
> 
> > +
> > +	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 is the mechanism used to reset stats for normal stats and xstats. The reset functions call the stats function with the nullified stats structure. So I think this is ok to leave in, I will add a comment about it being part of the reset functionality

> 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] ?

No, i is just used to cycle through the stats string structure but we use the count for xstats because that is the index that is passed in from  rte_eth_xstats_get(). Remember we are filling out the xstats structure with generic stats at the ethdev level first and then passing the xstats structure to the driver for the rest of it to be filled out.
> 
> Does it work when using "show port in test-pmd"?


Yes, I tested it before patch submission and it works.

> 
> > +
> > +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)  {
> >

[dpdk-dev] [PATCH v3 3/7] 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 |  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;
 }
 
-- 
1.9.3

Re: [dpdk-dev] [PATCH v3 3/7] ethdev: expose extended error stats

[Olivier MATZ]

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

[dpdk-dev] [PATCH v3 4/7] 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 f00a6ee..957b13f 100644
--- a/doc/guides/rel_notes/abi.rst
+++ b/doc/guides/rel_notes/abi.rst
@@ -38,3 +38,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 6451621..8176baf 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -125,17 +125,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

Re: [dpdk-dev] [PATCH v3 4/7] ethdev: remove HW specific stats in stats structs

[Kyle Larose]

On Fri, Jun 26, 2015 at 8:59 AM, Maryam Tahhan <maryam.tahhan@intel.com>
wrote:

> 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 f00a6ee..957b13f 100644
> --- a/doc/guides/rel_notes/abi.rst
> +++ b/doc/guides/rel_notes/abi.rst
> @@ -38,3 +38,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
>
>
Are CRC errors (ibadcrc) truly hardware specific? Which NIC (aside from
purely virtual ones) does not have a MAC which does frame checksumming?
Likewise, which NIC doesn't drop because the PCI bus/cpu/etc is too busy to
pull packets off of it (imissed)?

Debugging interactions with NICs is hard enough with only CRC errors and
missed packets to go on. Without those it is close to impossible. CRC
errors are almost guaranteed any time a bare-metal application is deployed:
dirty fiber, bad SFPs, etc. How will users of the application be able to
determine why their packets are dropping if they can only see "in errors"?

I understand that we want to avoid placing too much useless information
into these statistics structures. However, without a hardware-independent
way of accessing fairly standard networking-equipment diagnostics, I feel
like any real-world application using DPDK will be terribly cumbersome to
build: every single one will need to develop an abstraction layer which
detects the attached NICs, and loads an appropriate driver to integrate
with the xstats api.

Is there any plan for such an API? If not, is it really a good idea to
deprecate these stats?

Thanks,

Kyle

Re: [dpdk-dev] [PATCH v3 4/7] ethdev: remove HW specific stats in stats structs

[Tahhan, Maryam]

On Fri, Jun 26, 2015 at 8:59 AM, Maryam Tahhan <maryam.tahhan@intel.com<mailto:maryam.tahhan@intel.com>> wrote:
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<mailto: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 f00a6ee..957b13f 100644
--- a/doc/guides/rel_notes/abi.rst
+++ b/doc/guides/rel_notes/abi.rst
@@ -38,3 +38,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

Are CRC errors (ibadcrc) truly hardware specific? Which NIC (aside from purely virtual ones) does not have a MAC which does frame checksumming? Likewise, which NIC doesn't drop because the PCI bus/cpu/etc is too busy to pull packets off of it (imissed)?
Debugging interactions with NICs is hard enough with only CRC errors and missed packets to go on. Without those it is close to impossible. CRC errors are almost guaranteed any time a bare-metal application is deployed: dirty fiber, bad SFPs, etc. How will users of the application be able to determine why their packets are dropping if they can only see "in errors"?
I understand that we want to avoid placing too much useless information into these statistics structures. However, without a hardware-independent way of accessing fairly standard networking-equipment diagnostics, I feel like any real-world application using DPDK will be terribly cumbersome to build: every single one will need to develop an abstraction layer which detects the attached NICs, and loads an appropriate driver to integrate with the xstats api.

Is there any plan for such an API? If not, is it really a good idea to deprecate these stats?
Thanks,
Kyle

Hi Kyle

If it’s just for debug/diagnostic purposes that this information is being used then I would recommend using the proc_info app which is already integrated with xstats will give you detailed error statistics. It runs as a DPDK secondary process.

I’m not sure about crcerrors and imissed, I had taken feedback to my previous version of the patches onboard and made the changes based on that.

Regards
Maryam

Re: [dpdk-dev] [PATCH v3 4/7] ethdev: remove HW specific stats in stats structs

[Kyle Larose]

Hi Maryam,

I was not aware of the proc_info app. Is there any documentation on dpdk.org
about it, or should I browse the code?

Thanks,

Kyle

On Fri, Jun 26, 2015 at 10:30 AM, Tahhan, Maryam <maryam.tahhan@intel.com>
wrote:

>
>
> On Fri, Jun 26, 2015 at 8:59 AM, Maryam Tahhan <maryam.tahhan@intel.com>
> wrote:
>
> 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 f00a6ee..957b13f 100644
> --- a/doc/guides/rel_notes/abi.rst
> +++ b/doc/guides/rel_notes/abi.rst
> @@ -38,3 +38,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
>
>
>
> Are CRC errors (ibadcrc) truly hardware specific? Which NIC (aside from
> purely virtual ones) does not have a MAC which does frame checksumming?
> Likewise, which NIC doesn't drop because the PCI bus/cpu/etc is too busy to
> pull packets off of it (imissed)?
>
> Debugging interactions with NICs is hard enough with only CRC errors and
> missed packets to go on. Without those it is close to impossible. CRC
> errors are almost guaranteed any time a bare-metal application is deployed:
> dirty fiber, bad SFPs, etc. How will users of the application be able to
> determine why their packets are dropping if they can only see "in errors"?
>
> I understand that we want to avoid placing too much useless information
> into these statistics structures. However, without a hardware-independent
> way of accessing fairly standard networking-equipment diagnostics, I feel
> like any real-world application using DPDK will be terribly cumbersome to
> build: every single one will need to develop an abstraction layer which
> detects the attached NICs, and loads an appropriate driver to integrate
> with the xstats api.
>
>
>
> Is there any plan for such an API? If not, is it really a good idea to
> deprecate these stats?
>
> Thanks,
>
> Kyle
>
>
>
> Hi Kyle
>
>
>
> If it’s just for debug/diagnostic purposes that this information is being
> used then I would recommend using the proc_info app which is already
> integrated with xstats will give you detailed error statistics. It runs as
> a DPDK secondary process.
>
>
>
> I’m not sure about crcerrors and imissed, I had taken feedback to my
> previous version of the patches onboard and made the changes based on that.
>
>
>
> Regards
>
> Maryam
>

Re: [dpdk-dev] [PATCH v3 4/7] ethdev: remove HW specific stats in stats structs

[Tahhan, Maryam]

> Hi Maryam,
> I was not aware of the proc_info app. Is there any documentation on dpdk.org about it, or should I browse the code?
> Thanks,
> Kyle

Hi Kyle,
It's the last patch in the patchset I posted. I haven't written up the documentation yet, but will do so, for now feel free to peruse the code. If you have any questions let me know.

Regards
Maryam

[dpdk-dev] [PATCH v3 5/7] 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 | 43 ++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 099e464..b4cb297 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 RTE_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,25 @@ 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 +
-	                  hw_stats->illerrc + hw_stats->errbc;
-
+	stats->ierrors  = hw_stats->crcerrs +
+	                  hw_stats->rlec +
+	                  hw_stats->ruc +
+	                  hw_stats->roc +
+	                  total_missed_rx +
+	                  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*/
 	/*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
@@ -2102,6 +2106,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 v3 6/7] 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 v3 7/7] 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 54f0973..5184b31 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -488,3 +488,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 v3 0/7] Expose IXGBE extended stats to DPDK apps

[Tahhan, Maryam]

> 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.
> 
> Maryam Tahhan (7):
>   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
>   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 | 192 ++++++++++++---
>  lib/librte_ether/rte_ethdev.c    |  29 ++-
>  lib/librte_ether/rte_ethdev.h    |  30 ++-
>  mk/rte.sdktest.mk                |   4 +-
>  11 files changed, 762 insertions(+), 204 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

Hi Olivier,
I posted a new patch set with the suggested mods. Let me know if there are any issues.

Thanks in advance.

Best Regards, 
Maryam