[PATCH 0/3] xstats queue handling

[PATCH 0/3] xstats queue handling

[Harry van Haaren]

This patchset modifies how queue statistics are presented by
rte_eth_xstats_get() and each PMD's xstats_get().

Generic stats from the rte_eth_stats struct are presented by rte, and each
PMD can augment those stats with extra stats that are available (if any).

Currently ixgbe and i40e are the only NICs supporting queue xstats, and
they have been updated to conform with the new method of presentation.


Harry van Haaren (3):
  ethdev: xstats generic Q stats refactor
  ixgbe: refactor xstats queue handling
  i40e: refactor xstats queue handling

 drivers/net/i40e/i40e_ethdev.c   | 112 +++++++++++++++++++--------------
 drivers/net/ixgbe/ixgbe_ethdev.c | 132 ++++++++++++++++-----------------------
 lib/librte_ether/rte_ethdev.c    |  10 +--
 3 files changed, 120 insertions(+), 134 deletions(-)

-- 
1.9.1

[PATCH 1/3] ethdev: xstats generic Q stats refactor

[Harry van Haaren]

This patch refactors the generic queue stats to be exposed
by rte_ethdev_xstats_get().

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 lib/librte_ether/rte_ethdev.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index e0e1dca..b464f30 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1515,7 +1515,8 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats,
 	dev = &rte_eth_devices[port_id];
 
 	/* Return generic statistics */
-	count = RTE_NB_STATS;
+	count = RTE_NB_STATS + (dev->data->nb_rx_queues * RTE_NB_RXQ_STATS) +
+		(dev->data->nb_tx_queues * RTE_NB_TXQ_STATS);
 
 	/* implemented by the driver */
 	if (dev->dev_ops->xstats_get != NULL) {
@@ -1527,9 +1528,6 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats,
 
 		if (xcount < 0)
 			return xcount;
-	} else {
-		count += dev->data->nb_rx_queues * RTE_NB_RXQ_STATS;
-		count += dev->data->nb_tx_queues * RTE_NB_TXQ_STATS;
 	}
 
 	if (n < count + xcount)
@@ -1549,10 +1547,6 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats,
 		xstats[count++].value = val;
 	}
 
-	/* if xstats_get() is implemented by the PMD, the Q stats are done */
-	if (dev->dev_ops->xstats_get != NULL)
-		return count + xcount;
-
 	/* per-rxq stats */
 	for (q = 0; q < dev->data->nb_rx_queues; q++) {
 		for (i = 0; i < RTE_NB_RXQ_STATS; i++) {
-- 
1.9.1

[PATCH 2/3] ixgbe: refactor xstats queue handling

[Harry van Haaren]

This patch refactors the queue handling. Generic queue stats are
handled by rte_eth_xstats_get() and the ixgbe_xstats_get() exposes
only the extra stats.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 132 ++++++++++++++++-----------------------
 1 file changed, 53 insertions(+), 79 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0b0bbcf..19ddb52 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -613,11 +613,25 @@ static const struct rte_ixgbe_xstats_name_off rte_ixgbe_stats_strings[] = {
 			   sizeof(rte_ixgbe_stats_strings[0]))
 
 /* Per-queue statistics */
-#define IXBGE_NB_8_PER_Q_STATS (8 * 7)
-#define IXBGE_NB_16_PER_Q_STATS (16 * 5)
-#define IXGBE_NB_Q_STATS (IXBGE_NB_8_PER_Q_STATS + IXBGE_NB_16_PER_Q_STATS)
+static const struct rte_ixgbe_xstats_name_off rte_ixgbe_rxq_strings[] = {
+	{"mbuf_allocation_errors", offsetof(struct ixgbe_hw_stats, rnbc)},
+	{"dropped", offsetof(struct ixgbe_hw_stats, mpc)},
+	{"xon_packets", offsetof(struct ixgbe_hw_stats, pxonrxc)},
+	{"xoff_packets", offsetof(struct ixgbe_hw_stats, pxoffrxc)},
+};
+
+#define IXGBE_NB_RXQ_PRIO_STATS (sizeof(rte_ixgbe_rxq_strings) / \
+			   sizeof(rte_ixgbe_rxq_strings[0]))
+
+static const struct rte_ixgbe_xstats_name_off rte_ixgbe_txq_strings[] = {
+	{"xon_packets", offsetof(struct ixgbe_hw_stats, pxontxc)},
+	{"xoff_packets", offsetof(struct ixgbe_hw_stats, pxofftxc)},
+	{"xon_to_xoff_packets", offsetof(struct ixgbe_hw_stats,
+		pxon2offc)},
+};
 
-#define IXGBE_NB_XSTATS (IXGBE_NB_HW_STATS + IXGBE_NB_Q_STATS)
+#define IXGBE_NB_TXQ_PRIO_STATS (sizeof(rte_ixgbe_txq_strings) / \
+			   sizeof(rte_ixgbe_txq_strings[0]))
 
 static const struct rte_ixgbe_xstats_name_off rte_ixgbevf_stats_strings[] = {
 	{"rx_multicast_packets", offsetof(struct ixgbevf_hw_stats, vfmprc)},
@@ -2513,6 +2527,13 @@ ixgbe_dev_stats_reset(struct rte_eth_dev *dev)
 	memset(stats, 0, sizeof(*stats));
 }
 
+/* This function calculates the number of xstats based on the current config */
+static unsigned
+ixgbe_xstats_calc_num(void) {
+	return IXGBE_NB_HW_STATS + (IXGBE_NB_RXQ_PRIO_STATS * 8) +
+		(IXGBE_NB_TXQ_PRIO_STATS * 8);
+}
+
 static int
 ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats,
 					 unsigned n)
@@ -2522,7 +2543,9 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats,
 	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;
-	unsigned i, count = IXGBE_NB_XSTATS;
+	unsigned i, stat, count = 0;
+
+	count = ixgbe_xstats_calc_num();
 
 	if (n < count)
 		return count;
@@ -2551,81 +2574,30 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats,
 		count++;
 	}
 
-	/* Per-Q stats, with 8 queues available */
-	for (i = 0; i < 8; i++) {
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "rx_q%u_mbuf_allocation_errors", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, rnbc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "rx_q%u_missed_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, mpc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "rx_q%u_xon_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, pxonrxc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "tx_q%u_xon_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, pxontxc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "rx_q%u_xoff_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, pxoffrxc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "tx_q%u_xoff_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, pxofftxc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "xx_q%u_xon_to_xoff_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, pxon2offc[i]));
-		count++;
+	/* RX Priority Stats */
+	for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
+		for (i = 0; i < 8; i++) {
+			snprintf(xstats[count].name, sizeof(xstats[count].name),
+				 "rx_priority%u_%s", i,
+				 rte_ixgbe_rxq_strings[stat].name);
+			xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
+					rte_ixgbe_rxq_strings[stat].offset +
+					(sizeof(uint64_t) * i));
+			count++;
+		}
 	}
 
-	for (i = 0; i < 16; i++) {
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "rx_q%u_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, qprc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "rx_q%u_bytes", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, qbrc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "tx_q%u_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, qptc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "tx_q%u_bytes", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, qbtc[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "rx_q%u_dropped", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct ixgbe_hw_stats, qprdc[i]));
-		count++;
+	/* TX Priority Stats */
+	for (stat = 0; stat < IXGBE_NB_TXQ_PRIO_STATS; stat++) {
+		for (i = 0; i < 8; i++) {
+			snprintf(xstats[count].name, sizeof(xstats[count].name),
+				 "tx_priority%u_%s", i,
+				 rte_ixgbe_txq_strings[stat].name);
+			xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
+					rte_ixgbe_txq_strings[stat].offset +
+					(sizeof(uint64_t) * i));
+			count++;
+		}
 	}
 
 	return count;
@@ -2637,8 +2609,10 @@ ixgbe_dev_xstats_reset(struct rte_eth_dev *dev)
 	struct ixgbe_hw_stats *stats =
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
 
+	unsigned count = ixgbe_xstats_calc_num();
+
 	/* HW registers are cleared on read */
-	ixgbe_dev_xstats_get(dev, NULL, IXGBE_NB_XSTATS);
+	ixgbe_dev_xstats_get(dev, NULL, count);
 
 	/* Reset software totals */
 	memset(stats, 0, sizeof(*stats));
-- 
1.9.1

[PATCH 3/3] i40e: refactor xstats queue handling

[Harry van Haaren]

This patch refactors the queue and priority statistic handling.
Generic queue stats are presented by rte_eth_xstats_get(), and the
i40e_xstats_get() exposes only the extra stats.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 112 ++++++++++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 47 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index ddf3d38..c3f0235 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -490,6 +490,9 @@ static const struct rte_i40e_xstats_name_off rte_i40e_stats_strings[] = {
 	{"tx_dropped", offsetof(struct i40e_eth_stats, tx_discards)},
 };
 
+#define I40E_NB_ETH_XSTATS (sizeof(rte_i40e_stats_strings) / \
+		sizeof(rte_i40e_stats_strings[0]))
+
 static const struct rte_i40e_xstats_name_off rte_i40e_hw_port_strings[] = {
 	{"tx_link_down_dropped", offsetof(struct i40e_hw_port_stats,
 		tx_dropped_link_down)},
@@ -556,15 +559,30 @@ static const struct rte_i40e_xstats_name_off rte_i40e_hw_port_strings[] = {
 		rx_lpi_count)},
 };
 
-/* Q Stats: 5 stats are exposed for each queue, implemented in xstats_get() */
-#define I40E_NB_HW_PORT_Q_STATS (8 * 5)
-
-#define I40E_NB_ETH_XSTATS (sizeof(rte_i40e_stats_strings) / \
-		sizeof(rte_i40e_stats_strings[0]))
 #define I40E_NB_HW_PORT_XSTATS (sizeof(rte_i40e_hw_port_strings) / \
 		sizeof(rte_i40e_hw_port_strings[0]))
-#define I40E_NB_XSTATS (I40E_NB_ETH_XSTATS + I40E_NB_HW_PORT_XSTATS + \
-		I40E_NB_HW_PORT_Q_STATS)
+
+static const struct rte_i40e_xstats_name_off rte_i40e_rxq_prio_strings[] = {
+	{"xon_packets", offsetof(struct i40e_hw_port_stats,
+		priority_xon_rx)},
+	{"xoff_packets", offsetof(struct i40e_hw_port_stats,
+		priority_xoff_rx)},
+};
+
+#define I40E_NB_RXQ_PRIO_XSTATS (sizeof(rte_i40e_rxq_prio_strings) / \
+		sizeof(rte_i40e_rxq_prio_strings[0]))
+
+static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
+	{"xon_packets", offsetof(struct i40e_hw_port_stats,
+		priority_xon_tx)},
+	{"xoff_packets", offsetof(struct i40e_hw_port_stats,
+		priority_xoff_tx)},
+	{"xon_to_xoff_packets", offsetof(struct i40e_hw_port_stats,
+		priority_xon_2_xoff)},
+};
+
+#define I40E_NB_TXQ_PRIO_XSTATS (sizeof(rte_i40e_txq_prio_strings) / \
+		sizeof(rte_i40e_txq_prio_strings[0]))
 
 static struct eth_driver rte_i40e_pmd = {
 	.pci_drv = {
@@ -2124,6 +2142,14 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	PMD_DRV_LOG(DEBUG, "***************** PF stats end ********************");
 }
 
+static uint32_t
+i40e_xstats_calc_num(void)
+{
+	return I40E_NB_ETH_XSTATS + I40E_NB_HW_PORT_XSTATS +
+		(I40E_NB_RXQ_PRIO_XSTATS * 8) +
+		(I40E_NB_TXQ_PRIO_XSTATS * 8);
+}
+
 static void
 i40e_dev_xstats_reset(struct rte_eth_dev *dev)
 {
@@ -2145,18 +2171,20 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats,
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	unsigned i, count = 0;
+	unsigned i, count, prio;
 	struct i40e_hw_port_stats *hw_stats = &pf->stats;
 
-	if (n < I40E_NB_XSTATS)
-		return I40E_NB_XSTATS;
+	count = i40e_xstats_calc_num();
+	if (n < count)
+		return count;
 
 	i40e_read_stats_registers(pf, hw);
 
-	/* Reset */
 	if (xstats == NULL)
 		return 0;
 
+	count = 0;
+
 	/* Get stats from i40e_eth_stats struct */
 	for (i = 0; i < I40E_NB_ETH_XSTATS; i++) {
 		snprintf(xstats[count].name, sizeof(xstats[count].name),
@@ -2175,45 +2203,35 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats,
 		count++;
 	}
 
-	/* Get per-queue stats from i40e_hw_port struct */
-	for (i = 0; i < 8; i++) {
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "rx_q%u_xon_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct i40e_hw_port_stats,
-					 priority_xon_rx[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "rx_q%u_xoff_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct i40e_hw_port_stats,
-					 priority_xoff_rx[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "tx_q%u_xon_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct i40e_hw_port_stats,
-					 priority_xon_tx[i]));
-		count++;
-
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "tx_q%u_xoff_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct i40e_hw_port_stats,
-					 priority_xoff_tx[i]));
-		count++;
+	for (i = 0; i < I40E_NB_RXQ_PRIO_XSTATS; i++) {
+		for (prio = 0; prio < 8; prio++) {
+			snprintf(xstats[count].name,
+				 sizeof(xstats[count].name),
+				 "rx_priority%u_%s", prio,
+				 rte_i40e_rxq_prio_strings[i].name);
+			xstats[count].value =
+				*(uint64_t *)(((char *)hw_stats) +
+				rte_i40e_rxq_prio_strings[i].offset +
+				(sizeof(uint64_t) * prio));
+			count++;
+		}
+	}
 
-		snprintf(xstats[count].name, sizeof(xstats[count].name),
-			 "xx_q%u_xon_to_xoff_priority_packets", i);
-		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
-				offsetof(struct i40e_hw_port_stats,
-					 priority_xon_2_xoff[i]));
-		count++;
+	for (i = 0; i < I40E_NB_TXQ_PRIO_XSTATS; i++) {
+		for (prio = 0; prio < 8; prio++) {
+			snprintf(xstats[count].name,
+				 sizeof(xstats[count].name),
+				 "tx_priority%u_%s", prio,
+				 rte_i40e_txq_prio_strings[i].name);
+			xstats[count].value =
+				*(uint64_t *)(((char *)hw_stats) +
+				rte_i40e_txq_prio_strings[i].offset +
+				(sizeof(uint64_t) * prio));
+			count++;
+		}
 	}
 
-	return I40E_NB_XSTATS;
+	return count;
 }
 
 /* Reset the statistics */
-- 
1.9.1

Re: [PATCH 3/3] i40e: refactor xstats queue handling

[Tahhan, Maryam]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Harry van Haaren
> Sent: Friday, November 6, 2015 2:13 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 3/3] i40e: refactor xstats queue handling
> 
> This patch refactors the queue and priority statistic handling.
> Generic queue stats are presented by rte_eth_xstats_get(), and the
> i40e_xstats_get() exposes only the extra stats.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---

Acked-by: Maryam Tahhan <maryam.tahhan@intel.com>

Re: [PATCH 2/3] ixgbe: refactor xstats queue handling

[Tahhan, Maryam]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Harry van Haaren
> Sent: Friday, November 6, 2015 2:13 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/3] ixgbe: refactor xstats queue handling
> 
> This patch refactors the queue handling. Generic queue stats are handled by
> rte_eth_xstats_get() and the ixgbe_xstats_get() exposes only the extra stats.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
Acked-by: Maryam Tahhan <maryam.tahhan@intel.com>

Re: [PATCH 1/3] ethdev: xstats generic Q stats refactor

[Tahhan, Maryam]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Harry van Haaren
> Sent: Friday, November 6, 2015 2:13 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/3] ethdev: xstats generic Q stats refactor
> 
> This patch refactors the generic queue stats to be exposed by
> rte_ethdev_xstats_get().
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---

Acked-by: Maryam Tahhan <maryam.tahhan@intel.com>

Re: [PATCH 0/3] xstats queue handling

[Thomas Monjalon]

2015-11-06 14:12, Harry van Haaren:
> This patchset modifies how queue statistics are presented by
> rte_eth_xstats_get() and each PMD's xstats_get().
> 
> Generic stats from the rte_eth_stats struct are presented by rte, and each
> PMD can augment those stats with extra stats that are available (if any).
> 
> Currently ixgbe and i40e are the only NICs supporting queue xstats, and
> they have been updated to conform with the new method of presentation.
> 
> 
> Harry van Haaren (3):
>   ethdev: xstats generic Q stats refactor
>   ixgbe: refactor xstats queue handling
>   i40e: refactor xstats queue handling

Applied, thanks