[dpdk-dev] [PATCH 0/2] introduce dev_ops to get extended statistics of a device

[dpdk-dev] [PATCH 0/2] introduce dev_ops to get extended statistics of a device

[Olivier Matz]

The generic statistics structure is getting bigger as new statistics are
added in specific devices. For instance, fdir, tx_pause or loopback
stats do not apply on virtual devices. It won't be possible to add every
specific statistics in this generic stats structure, but on the other
hand these specific statistics are useful for debugging purpose.

This 2 patches introduces xstats_get() and xstats_reset() in
dev_ops. When registered by a device, it can be used to provide
arbitrary statistics that are identified by a name, as done by eththool
in kernel.

After that, some statistics could be moved from the generic structure to
this new framework, but it will be part of another patch series as it
should be discussed first.

Olivier Matz (2):
  ethdev: add a new rte_eth_xstats_get method to retrieve extended
    statistics
  testpmd: add a new command to get the extended statistics of a port

 app/test-pmd/cmdline.c        |  22 +++++--
 app/test-pmd/config.c         |  34 +++++++++++
 app/test-pmd/testpmd.h        |   2 +
 lib/librte_ether/rte_ethdev.c | 137 ++++++++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h |  60 +++++++++++++++++-
 5 files changed, 247 insertions(+), 8 deletions(-)

-- 
2.0.1

[dpdk-dev] [PATCH 1/2] ethdev: add a new rte_eth_xstats_get method to retrieve extended statistics

[Olivier Matz]

This method can be implemented by a poll mode driver to provide
non-standard statistics (which are not part of the generic statistics
structure). Each statistic is returned in a generic form: "name" and
"value" and can be used to dump PMD-specific statistics in the same way
than ethtool in linux kernel.

If the PMD does not provide the xstats_get and xstats_set functions, the
ethdev API will return the generic statistics in the xstats format
(name, value).

This commit opens the door for a clean-up of the generic statistics
structure, only keeping statistics that are really common to all PMDs
and moving specific ones into the xstats API.

Reviewed-by: David Marchand <david.marchand@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_ether/rte_ethdev.c | 137 ++++++++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h |  60 +++++++++++++++++-
 2 files changed, 195 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fd1010a..b71b679 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -114,6 +114,48 @@ static uint8_t nb_ports = 0;
 /* spinlock for eth device callbacks */
 static rte_spinlock_t rte_eth_dev_cb_lock = RTE_SPINLOCK_INITIALIZER;
 
+/* store statistics names and its offset in stats structure  */
+struct rte_eth_xstats_name_off {
+	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	unsigned offset;
+};
+
+static struct rte_eth_xstats_name_off rte_stats_strings[] = {
+	 {"rx_packets", offsetof(struct rte_eth_stats, ipackets)},
+	 {"tx_packets", offsetof(struct rte_eth_stats, opackets)},
+	 {"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]))
+
+static struct rte_eth_xstats_name_off rte_rxq_stats_strings[] = {
+	{"rx_packets", offsetof(struct rte_eth_stats, q_ipackets)},
+	{"rx_bytes", offsetof(struct rte_eth_stats, q_ibytes)},
+};
+#define RTE_NB_RXQ_STATS (sizeof(rte_rxq_stats_strings) /	\
+		sizeof(rte_rxq_stats_strings[0]))
+
+static struct rte_eth_xstats_name_off rte_txq_stats_strings[] = {
+	{"tx_packets", offsetof(struct rte_eth_stats, q_opackets)},
+	{"tx_bytes", offsetof(struct rte_eth_stats, q_obytes)},
+	{"tx_errors", offsetof(struct rte_eth_stats, q_errors)},
+};
+#define RTE_NB_TXQ_STATS (sizeof(rte_txq_stats_strings) /	\
+		sizeof(rte_txq_stats_strings[0]))
+
+
 /**
  * The user application callback description.
  *
@@ -1201,6 +1243,101 @@ rte_eth_stats_reset(uint8_t port_id)
 	(*dev->dev_ops->stats_reset)(dev);
 }
 
+/* retrieve ethdev extended statistics */
+int
+rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats,
+	unsigned n)
+{
+	struct rte_eth_stats eth_stats;
+	struct rte_eth_dev *dev;
+	unsigned count, i, q;
+	uint64_t val;
+	char *stats_ptr;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return -1;
+	}
+	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 */
+	count = RTE_NB_STATS;
+	count += dev->data->nb_rx_queues * RTE_NB_RXQ_STATS;
+	count += dev->data->nb_tx_queues * RTE_NB_TXQ_STATS;
+	if (n < count)
+		return count;
+
+	/* now fill the xstats structure */
+
+	count = 0;
+	memset(&eth_stats, 0, sizeof(eth_stats));
+	rte_eth_stats_get(port_id, &eth_stats);
+
+	/* global stats */
+	for (i = 0; i < RTE_NB_STATS; i++) {
+		stats_ptr = (char *)&eth_stats + rte_stats_strings[i].offset;
+		val = *(uint64_t *)stats_ptr;
+		snprintf(xstats[count].name, sizeof(xstats[count].name),
+			"%s", rte_stats_strings[i].name);
+		xstats[count++].value = val;
+	}
+
+	/* per-rxq stats */
+	for (q = 0; q < dev->data->nb_rx_queues; q++) {
+		for (i = 0; i < RTE_NB_RXQ_STATS; i++) {
+			stats_ptr = (char *)&eth_stats;
+			stats_ptr += rte_rxq_stats_strings[i].offset;
+			stats_ptr += q * sizeof(uint64_t);
+			val = *(uint64_t *)stats_ptr;
+			snprintf(xstats[count].name, sizeof(xstats[count].name),
+				"rx_queue_%u_%s", q,
+				rte_rxq_stats_strings[i].name);
+			xstats[count++].value = val;
+		}
+	}
+
+	/* per-txq stats */
+	for (q = 0; q < dev->data->nb_tx_queues; q++) {
+		for (i = 0; i < RTE_NB_TXQ_STATS; i++) {
+			stats_ptr = (char *)&eth_stats;
+			stats_ptr += rte_txq_stats_strings[i].offset;
+			stats_ptr += q * sizeof(uint64_t);
+			val = *(uint64_t *)stats_ptr;
+			snprintf(xstats[count].name, sizeof(xstats[count].name),
+				"tx_queue_%u_%s", q,
+				rte_txq_stats_strings[i].name);
+			xstats[count++].value = val;
+		}
+	}
+
+	return count;
+}
+
+/* reset ethdev extended statistics */
+void
+rte_eth_xstats_reset(uint8_t port_id)
+{
+	struct rte_eth_dev *dev;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return;
+	}
+	dev = &rte_eth_devices[port_id];
+
+	/* implemented by the driver */
+	if (dev->dev_ops->xstats_reset != NULL) {
+		(*dev->dev_ops->xstats_reset)(dev);
+		return;
+	}
+
+	/* fallback to default */
+	rte_eth_stats_reset(port_id);
+}
 
 static int
 set_queue_stats_mapping(uint8_t port_id, uint16_t queue_id, uint8_t stat_idx,
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 50df654..4076c33 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -908,6 +908,21 @@ struct rte_eth_dev_info {
 	uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
 };
 
+/** Maximum name length for a extended statitics counter */
+#define RTE_ETH_XSTATS_NAME_SIZE 64
+
+/**
+ * An Ethernet device extended statistic structure
+ *
+ * This structure is used by ethdev->eth_xstats_get() to provide
+ * statistics that are not provided in the generic rte_eth_stats
+ * structure.
+ */
+struct rte_eth_xstats {
+	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	uint64_t value;
+};
+
 struct rte_eth_dev;
 
 struct rte_eth_dev_callback;
@@ -1028,6 +1043,13 @@ typedef void (*eth_stats_get_t)(struct rte_eth_dev *dev,
 typedef void (*eth_stats_reset_t)(struct rte_eth_dev *dev);
 /**< @internal Reset global I/O statistics of an Ethernet device to 0. */
 
+typedef int (*eth_xstats_get_t)(struct rte_eth_dev *dev,
+	struct rte_eth_xstats *stats, unsigned n);
+/**< @internal Get extended stats of an Ethernet device. */
+
+typedef void (*eth_xstats_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Reset extended stats of an Ethernet device. */
+
 typedef int (*eth_queue_stats_mapping_set_t)(struct rte_eth_dev *dev,
 					     uint16_t queue_id,
 					     uint8_t stat_idx,
@@ -1376,8 +1398,10 @@ struct eth_dev_ops {
 	eth_allmulticast_enable_t  allmulticast_enable;/**< RX multicast ON. */
 	eth_allmulticast_disable_t allmulticast_disable;/**< RX multicast OF. */
 	eth_link_update_t          link_update;   /**< Get device link state. */
-	eth_stats_get_t            stats_get;     /**< Get device statistics. */
-	eth_stats_reset_t          stats_reset;   /**< Reset device statistics. */
+	eth_stats_get_t            stats_get;     /**< Get generic device statistics. */
+	eth_stats_reset_t          stats_reset;   /**< Reset generic device statistics. */
+	eth_xstats_get_t           xstats_get;    /**< Get extended device statistics. */
+	eth_xstats_reset_t         xstats_reset;  /**< Reset extended device statistics. */
 	eth_queue_stats_mapping_set_t queue_stats_mapping_set;
 	/**< Configure per queue stat counter mapping. */
 	eth_dev_infos_get_t        dev_infos_get; /**< Get device info. */
@@ -2005,6 +2029,38 @@ extern void rte_eth_stats_get(uint8_t port_id, struct rte_eth_stats *stats);
 extern void rte_eth_stats_reset(uint8_t port_id);
 
 /**
+ * Retrieve extended statistics of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param stats
+ *   A pointer to a table of structure of type *rte_eth_xstats*
+ *   to be filled with the values of device statistics names and value.
+ *   This parameter can be set to NULL if n is 0.
+ * @param n
+ *   The size of the stats table, which should be large enough to store
+ *   all the statistics of the device.
+ * @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. Thereturn 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)
+ */
+extern int rte_eth_xstats_get(uint8_t port_id,
+	struct rte_eth_xstats *xstats, unsigned n);
+
+/**
+ * Reset extended statistics of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ */
+extern void rte_eth_xstats_reset(uint8_t port_id);
+
+/**
  *  Set a mapping for the specified transmit queue to the specified per-queue
  *  statistics counter.
  *
-- 
2.0.1

[dpdk-dev] [PATCH 2/2] testpmd: add a new command to get the extended statistics of a port

[Olivier Matz]

Add a new token in "show port" command to dump the extended statistics
of a device. It validates the new xstats framework added in previous commit.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test-pmd/cmdline.c | 22 ++++++++++++++++------
 app/test-pmd/config.c  | 34 ++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 345be11..972ef8c 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -183,14 +183,14 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"Display:\n"
 			"--------\n\n"
 
-			"show port (info|stats|fdir|stat_qmap) (port_id|all)\n"
+			"show port (info|stats|xstats|fdir|stat_qmap) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
 			"show port rss-hash [key]\n"
 			"    Display the RSS hash functions and RSS hash key"
 			" of port X\n\n"
 
-			"clear port (info|stats|fdir|stat_qmap) (port_id|all)\n"
+			"clear port (info|stats|xstats|fdir|stat_qmap) (port_id|all)\n"
 			"    Clear information for port_id, or all.\n\n"
 
 			"show config (rxtx|cores|fwd)\n"
@@ -5015,12 +5015,18 @@ static void cmd_showportall_parsed(void *parsed_result,
 		if (!strcmp(res->what, "stats"))
 			for (i = 0; i < nb_ports; i++)
 				nic_stats_clear(i);
+		else if (!strcmp(res->what, "xstats"))
+			for (i = 0; i < nb_ports; i++)
+				nic_xstats_clear(i);
 	} else if (!strcmp(res->what, "info"))
 		for (i = 0; i < nb_ports; i++)
 			port_infos_display(i);
 	else if (!strcmp(res->what, "stats"))
 		for (i = 0; i < nb_ports; i++)
 			nic_stats_display(i);
+	else if (!strcmp(res->what, "xstats"))
+		for (i = 0; i < nb_ports; i++)
+			nic_xstats_display(i);
 	else if (!strcmp(res->what, "fdir"))
 		for (i = 0; i < nb_ports; i++)
 			fdir_get_infos(i);
@@ -5036,13 +5042,13 @@ cmdline_parse_token_string_t cmd_showportall_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_showportall_result, port, "port");
 cmdline_parse_token_string_t cmd_showportall_what =
 	TOKEN_STRING_INITIALIZER(struct cmd_showportall_result, what,
-				 "info#stats#fdir#stat_qmap");
+				 "info#stats#xstats#fdir#stat_qmap");
 cmdline_parse_token_string_t cmd_showportall_all =
 	TOKEN_STRING_INITIALIZER(struct cmd_showportall_result, all, "all");
 cmdline_parse_inst_t cmd_showportall = {
 	.f = cmd_showportall_parsed,
 	.data = NULL,
-	.help_str = "show|clear port info|stats|fdir|stat_qmap all",
+	.help_str = "show|clear port info|stats|xstats|fdir|stat_qmap all",
 	.tokens = {
 		(void *)&cmd_showportall_show,
 		(void *)&cmd_showportall_port,
@@ -5068,10 +5074,14 @@ static void cmd_showport_parsed(void *parsed_result,
 	if (!strcmp(res->show, "clear")) {
 		if (!strcmp(res->what, "stats"))
 			nic_stats_clear(res->portnum);
+		else if (!strcmp(res->what, "xstats"))
+			nic_xstats_clear(res->portnum);
 	} else if (!strcmp(res->what, "info"))
 		port_infos_display(res->portnum);
 	else if (!strcmp(res->what, "stats"))
 		nic_stats_display(res->portnum);
+	else if (!strcmp(res->what, "xstats"))
+		nic_xstats_display(res->portnum);
 	else if (!strcmp(res->what, "fdir"))
 		 fdir_get_infos(res->portnum);
 	else if (!strcmp(res->what, "stat_qmap"))
@@ -5085,14 +5095,14 @@ cmdline_parse_token_string_t cmd_showport_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_showport_result, port, "port");
 cmdline_parse_token_string_t cmd_showport_what =
 	TOKEN_STRING_INITIALIZER(struct cmd_showport_result, what,
-				 "info#stats#fdir#stat_qmap");
+				 "info#stats#xstats#fdir#stat_qmap");
 cmdline_parse_token_num_t cmd_showport_portnum =
 	TOKEN_NUM_INITIALIZER(struct cmd_showport_result, portnum, INT32);
 
 cmdline_parse_inst_t cmd_showport = {
 	.f = cmd_showport_parsed,
 	.data = NULL,
-	.help_str = "show|clear port info|stats|fdir|stat_qmap X (X = port number)",
+	.help_str = "show|clear port info|stats|xstats|fdir|stat_qmap X (X = port number)",
 	.tokens = {
 		(void *)&cmd_showport_show,
 		(void *)&cmd_showport_port,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index c72f6ee..0a42ee9 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -199,6 +199,40 @@ nic_stats_clear(portid_t port_id)
 	printf("\n  NIC statistics for port %d cleared\n", port_id);
 }
 
+void
+nic_xstats_display(portid_t port_id)
+{
+	struct rte_eth_xstats *xstats;
+	int len, ret, i;
+
+	printf("###### NIC extended statistics for port %-2d\n", port_id);
+
+	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;
+	}
+	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);
+	free(xstats);
+}
+
+void
+nic_xstats_clear(portid_t port_id)
+{
+	rte_eth_xstats_reset(port_id);
+}
 
 void
 nic_stats_mapping_display(portid_t port_id)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index ac86bfe..c698c41 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -448,6 +448,8 @@ void launch_args_parse(int argc, char** argv);
 void prompt(void);
 void nic_stats_display(portid_t port_id);
 void nic_stats_clear(portid_t port_id);
+void nic_xstats_display(portid_t port_id);
+void nic_xstats_clear(portid_t port_id);
 void nic_stats_mapping_display(portid_t port_id);
 void port_infos_display(portid_t port_id);
 void fwd_lcores_config_display(void);
-- 
2.0.1

Re: [dpdk-dev] [PATCH 0/2] introduce dev_ops to get extended statistics of a device

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Wednesday, July 23, 2014 5:29 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 0/2] introduce dev_ops to get extended statistics of
> a device
> 
> The generic statistics structure is getting bigger as new statistics are
> added in specific devices. For instance, fdir, tx_pause or loopback
> stats do not apply on virtual devices. It won't be possible to add every
> specific statistics in this generic stats structure, but on the other
> hand these specific statistics are useful for debugging purpose.
> 
> This 2 patches introduces xstats_get() and xstats_reset() in
> dev_ops. When registered by a device, it can be used to provide
> arbitrary statistics that are identified by a name, as done by eththool
> in kernel.
> 
> After that, some statistics could be moved from the generic structure to
> this new framework, but it will be part of another patch series as it
> should be discussed first.
> 

I like the idea, so Ack on the concept. :-)

Re: [dpdk-dev] [PATCH 0/2] introduce dev_ops to get extended statistics of a device

[Thomas Monjalon]

2014-07-23 18:41, Richardson, Bruce:
> > The generic statistics structure is getting bigger as new statistics are
> > added in specific devices. For instance, fdir, tx_pause or loopback
> > stats do not apply on virtual devices. It won't be possible to add every
> > specific statistics in this generic stats structure, but on the other
> > hand these specific statistics are useful for debugging purpose.
> > 
> > This 2 patches introduces xstats_get() and xstats_reset() in
> > dev_ops. When registered by a device, it can be used to provide
> > arbitrary statistics that are identified by a name, as done by eththool
> > in kernel.
> > 
> > After that, some statistics could be moved from the generic structure to
> > this new framework, but it will be part of another patch series as it
> > should be discussed first.
> 
> I like the idea, so Ack on the concept. :-)

Applied with some minor fixes in the comments.

Next step is to move some fields from rte_eth_stats to extended stats.
First candidates to move are the VF-only counters.

Thanks
-- 
Thomas