DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] eal: remove redundant id field in xstats name lookups
@ 2016-07-01 13:16 Remy Horton
  2016-07-01 14:04 ` Thomas Monjalon
  0 siblings, 1 reply; 2+ messages in thread
From: Remy Horton @ 2016-07-01 13:16 UTC (permalink / raw)
  To: thomas.monjalon; +Cc: dev

For all drivers that currently implement xstats, the id field in the
rte_eth_stats_name structure equals the entry's array index. This
patch eliminates the redundant id field as a direct index lookup is
faster than a search for the matching id field.

Signed-off-by: Remy Horton <remy.horton@intel.com>
---
 app/proc_info/main.c                    | 11 +++--------
 app/test-pmd/config.c                   | 12 ++++--------
 doc/guides/prog_guide/poll_mode_drv.rst | 18 +++++++-----------
 drivers/net/e1000/igb_ethdev.c          |  2 --
 drivers/net/fm10k/fm10k_ethdev.c        |  3 ---
 drivers/net/i40e/i40e_ethdev.c          |  8 --------
 drivers/net/i40e/i40e_ethdev_vf.c       |  1 -
 drivers/net/ixgbe/ixgbe_ethdev.c        |  7 -------
 drivers/net/virtio/virtio_ethdev.c      |  4 ----
 lib/librte_ether/rte_ethdev.c           |  3 ---
 lib/librte_ether/rte_ethdev.h           |  1 -
 11 files changed, 14 insertions(+), 56 deletions(-)

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index f2063fa..6dc0bbb 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -246,7 +246,6 @@ nic_xstats_display(uint8_t port_id)
 	struct rte_eth_xstat_name *xstats_names;
 	struct rte_eth_xstat *xstats;
 	int len, ret, i;
-	int idx_name;
 	static const char *nic_stats_border = "########################";
 
 	len = rte_eth_xstats_get_names(port_id, NULL, 0);
@@ -284,13 +283,9 @@ nic_xstats_display(uint8_t port_id)
 	}
 
 	for (i = 0; i < len; i++)
-		for (idx_name = 0; idx_name < len; idx_name++)
-			if (xstats_names[idx_name].id == xstats[i].id) {
-				printf("%s: %"PRIu64"\n",
-					xstats_names[idx_name].name,
-					xstats[i].value);
-				break;
-			}
+		printf("%s: %"PRIu64"\n",
+			xstats_names[i].name,
+			xstats[i].value);
 
 	printf("%s############################\n",
 			   nic_stats_border);
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 10f0a36..89e3f66e 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -256,7 +256,7 @@ void
 nic_xstats_display(portid_t port_id)
 {
 	struct rte_eth_xstat *xstats;
-	int cnt_xstats, idx_xstat, idx_name;
+	int cnt_xstats, idx_xstat;
 	struct rte_eth_xstat_name *xstats_names;
 
 	printf("###### NIC extended statistics for port %-2d\n", port_id);
@@ -298,13 +298,9 @@ nic_xstats_display(portid_t port_id)
 
 	/* Display xstats */
 	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++)
-		for (idx_name = 0; idx_name < cnt_xstats; idx_name++)
-			if (xstats_names[idx_name].id == xstats[idx_xstat].id) {
-				printf("%s: %"PRIu64"\n",
-					xstats_names[idx_name].name,
-					xstats[idx_xstat].value);
-				break;
-			}
+		printf("%s: %"PRIu64"\n",
+			xstats_names[idx_xstat].name,
+			xstats[idx_xstat].value);
 	free(xstats_names);
 	free(xstats);
 }
diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
index 802fb8f..bf3ea9f 100644
--- a/doc/guides/prog_guide/poll_mode_drv.rst
+++ b/doc/guides/prog_guide/poll_mode_drv.rst
@@ -309,17 +309,13 @@ functions:
   information.
 
 Each ``struct rte_eth_xstat`` contains an identifier and value pair, and
-each ``struct rte_eth_xstat_name`` contains an identifier and string pair.
-Each identifier within ``struct rte_eth_xstat`` must have a corresponding
-entry in ``struct rte_eth_xstat_name`` with a matching identifier. These
-identifiers, as well as the number of extended statistic exposed, must
-remain constant during runtime.
-
-Note that extended statistic identifiers are driver-specific, and hence
-might not be the same for different ports. Although it is expected that
-drivers will make the identifiers used within ``struct rte_eth_xstat`` and
-``struct rte_eth_xstat_name`` entries match the entries' array index, this
-property should not be relied on by applications for lookups.
+each ``struct rte_eth_xstat_name`` contains a string. Each identifier
+within the ``struct rte_eth_xstat`` lookup array must have a corresponding
+entry in the ``struct rte_eth_xstat_name`` lookup array. Within the latter
+the index of the entry is the identifier the string is associated with.
+These identifiers, as well as the number of extended statistic exposed, must
+remain constant during runtime. Note that extended statistic identifiers are
+driver-specific, and hence might not be the same for different ports.
 
 A naming scheme exists for the strings exposed to clients of the API. This is
 to allow scraping of the API for statistics of interest. The naming scheme uses
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index b822992..928feaa 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1713,7 +1713,6 @@ static int eth_igb_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	for (i = 0; i < IGB_NB_XSTATS; i++) {
 		snprintf(xstats_names[i].name, sizeof(xstats_names[i].name),
 			 "%s", rte_igb_stats_strings[i].name);
-		xstats_names[i].id = i;
 	}
 
 	return IGB_NB_XSTATS;
@@ -1800,7 +1799,6 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 			snprintf(xstats_names[i].name,
 				sizeof(xstats_names[i].name), "%s",
 				rte_igbvf_stats_strings[i].name);
-			xstats_names[i].id = i;
 		}
 	return IGBVF_NB_XSTATS;
 }
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index ce053b0..b90a135 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1270,7 +1270,6 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 			snprintf(xstats_names[count].name,
 				sizeof(xstats_names[count].name),
 				"%s", fm10k_hw_stats_strings[count].name);
-			xstats_names[count].id = count;
 			count++;
 		}
 
@@ -1281,7 +1280,6 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 					sizeof(xstats_names[count].name),
 					"rx_q%u_%s", q,
 					fm10k_hw_stats_rx_q_strings[i].name);
-				xstats_names[count].id = count;
 				count++;
 			}
 			for (i = 0; i < FM10K_NB_TX_Q_XSTATS; i++) {
@@ -1289,7 +1287,6 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 					sizeof(xstats_names[count].name),
 					"tx_q%u_%s", q,
 					fm10k_hw_stats_tx_q_strings[i].name);
-				xstats_names[count].id = count;
 				count++;
 			}
 		}
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index f94ad87..72e72d7 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2226,7 +2226,6 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 		snprintf(xstats_names[count].name,
 			 sizeof(xstats_names[count].name),
 			 "%s", rte_i40e_stats_strings[i].name);
-		xstats_names[count].id = count;
 		count++;
 	}
 
@@ -2235,7 +2234,6 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 		snprintf(xstats_names[count].name,
 			sizeof(xstats_names[count].name),
 			 "%s", rte_i40e_hw_port_strings[i].name);
-		xstats_names[count].id = count;
 		count++;
 	}
 
@@ -2245,7 +2243,6 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 				 sizeof(xstats_names[count].name),
 				 "rx_priority%u_%s", prio,
 				 rte_i40e_rxq_prio_strings[i].name);
-			xstats_names[count].id = count;
 			count++;
 		}
 	}
@@ -2256,7 +2253,6 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 				 sizeof(xstats_names[count].name),
 				 "tx_priority%u_%s", prio,
 				 rte_i40e_txq_prio_strings[i].name);
-			xstats_names[count].id = count;
 			count++;
 		}
 	}
@@ -2285,7 +2281,6 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 
 	/* Get stats from i40e_eth_stats struct */
 	for (i = 0; i < I40E_NB_ETH_XSTATS; i++) {
-		xstats[count].id = count;
 		xstats[count].value = *(uint64_t *)(((char *)&hw_stats->eth) +
 			rte_i40e_stats_strings[i].offset);
 		count++;
@@ -2293,7 +2288,6 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 
 	/* Get individiual stats from i40e_hw_port struct */
 	for (i = 0; i < I40E_NB_HW_PORT_XSTATS; i++) {
-		xstats[count].id = count;
 		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_i40e_hw_port_strings[i].offset);
 		count++;
@@ -2301,7 +2295,6 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 
 	for (i = 0; i < I40E_NB_RXQ_PRIO_XSTATS; i++) {
 		for (prio = 0; prio < 8; prio++) {
-			xstats[count].id = count;
 			xstats[count].value =
 				*(uint64_t *)(((char *)hw_stats) +
 				rte_i40e_rxq_prio_strings[i].offset +
@@ -2312,7 +2305,6 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 
 	for (i = 0; i < I40E_NB_TXQ_PRIO_XSTATS; i++) {
 		for (prio = 0; prio < 8; prio++) {
-			xstats[count].id = count;
 			xstats[count].value =
 				*(uint64_t *)(((char *)hw_stats) +
 				rte_i40e_txq_prio_strings[i].offset +
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 37af399..e8a7517 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -999,7 +999,6 @@ static int i40evf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 			snprintf(xstats_names[i].name,
 				sizeof(xstats_names[i].name),
 				"%s", rte_i40evf_stats_strings[i].name);
-			xstats_names[i].id = i;
 		}
 	return I40EVF_NB_XSTATS;
 }
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index e11a431..6fe9f64 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2734,7 +2734,6 @@ static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 
 		/* Extended stats from ixgbe_hw_stats */
 		for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
-			xstats_names[count].id = count;
 			snprintf(xstats_names[count].name,
 				sizeof(xstats_names[count].name),
 				"%s",
@@ -2745,7 +2744,6 @@ static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 		/* RX Priority Stats */
 		for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
 			for (i = 0; i < IXGBE_NB_RXQ_PRIO_VALUES; i++) {
-				xstats_names[count].id = count;
 				snprintf(xstats_names[count].name,
 					sizeof(xstats_names[count].name),
 					"rx_priority%u_%s", i,
@@ -2757,7 +2755,6 @@ static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 		/* TX Priority Stats */
 		for (stat = 0; stat < IXGBE_NB_TXQ_PRIO_STATS; stat++) {
 			for (i = 0; i < IXGBE_NB_TXQ_PRIO_VALUES; i++) {
-				xstats_names[count].id = count;
 				snprintf(xstats_names[count].name,
 					sizeof(xstats_names[count].name),
 					"tx_priority%u_%s", i,
@@ -2818,7 +2815,6 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	/* Extended stats from ixgbe_hw_stats */
 	count = 0;
 	for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
-		xstats[count].id = count;
 		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 				rte_ixgbe_stats_strings[i].offset);
 		count++;
@@ -2827,7 +2823,6 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	/* RX Priority Stats */
 	for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
 		for (i = 0; i < IXGBE_NB_RXQ_PRIO_VALUES; i++) {
-			xstats[count].id = count;
 			xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 					rte_ixgbe_rxq_strings[stat].offset +
 					(sizeof(uint64_t) * i));
@@ -2838,7 +2833,6 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	/* TX Priority Stats */
 	for (stat = 0; stat < IXGBE_NB_TXQ_PRIO_STATS; stat++) {
 		for (i = 0; i < IXGBE_NB_TXQ_PRIO_VALUES; i++) {
-			xstats[count].id = count;
 			xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 					rte_ixgbe_txq_strings[stat].offset +
 					(sizeof(uint64_t) * i));
@@ -2909,7 +2903,6 @@ ixgbevf_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 
 	/* Extended stats */
 	for (i = 0; i < IXGBEVF_NB_XSTATS; i++) {
-		xstats[i].id = i;
 		xstats[i].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_ixgbevf_stats_strings[i].offset);
 	}
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index a833740..bef7411 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -735,7 +735,6 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 					sizeof(xstats_names[count].name),
 					"rx_q%u_%s", i,
 					rte_virtio_q_stat_strings[t].name);
-				xstats_names[count].id = count;
 				count++;
 			}
 		}
@@ -749,7 +748,6 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 					sizeof(xstats_names[count].name),
 					"tx_q%u_%s", i,
 					rte_virtio_q_stat_strings[t].name);
-				xstats_names[count].id = count;
 				count++;
 			}
 		}
@@ -780,7 +778,6 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		unsigned t;
 
 		for (t = 0; t < VIRTIO_NB_Q_XSTATS; t++) {
-			xstats[count].id = count;
 			xstats[count].value = *(uint64_t *)(((char *)rxvq) +
 				rte_virtio_q_stat_strings[t].offset);
 			count++;
@@ -796,7 +793,6 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		unsigned t;
 
 		for (t = 0; t < VIRTIO_NB_Q_XSTATS; t++) {
-			xstats[count].id = count;
 			xstats[count].value = *(uint64_t *)(((char *)txvq) +
 				rte_virtio_q_stat_strings[t].offset);
 			count++;
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 42aaef7..eac260f 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1557,7 +1557,6 @@ rte_eth_xstats_get_names(uint8_t port_id,
 		cnt_used_entries = 0;
 
 	for (idx = 0; idx < RTE_NB_STATS; idx++) {
-		xstats_names[cnt_used_entries].id = cnt_used_entries;
 		snprintf(xstats_names[cnt_used_entries].name,
 			sizeof(xstats_names[0].name),
 			"%s", rte_stats_strings[idx].name);
@@ -1565,7 +1564,6 @@ rte_eth_xstats_get_names(uint8_t port_id,
 	}
 	for (id_queue = 0; id_queue < dev->data->nb_rx_queues; id_queue++) {
 		for (idx = 0; idx < RTE_NB_RXQ_STATS; idx++) {
-			xstats_names[cnt_used_entries].id = cnt_used_entries;
 			snprintf(xstats_names[cnt_used_entries].name,
 				sizeof(xstats_names[0].name),
 				"rx_q%u%s",
@@ -1576,7 +1574,6 @@ rte_eth_xstats_get_names(uint8_t port_id,
 	}
 	for (id_queue = 0; id_queue < dev->data->nb_tx_queues; id_queue++) {
 		for (idx = 0; idx < RTE_NB_TXQ_STATS; idx++) {
-			xstats_names[cnt_used_entries].id = cnt_used_entries;
 			snprintf(xstats_names[cnt_used_entries].name,
 				sizeof(xstats_names[0].name),
 				"tx_q%u%s",
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index bd93bf6..331bbe1 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -930,7 +930,6 @@ struct rte_eth_xstat {
  */
 struct rte_eth_xstat_name {
 	char name[RTE_ETH_XSTATS_NAME_SIZE];
-	uint64_t id;
 };
 
 #define ETH_DCB_NUM_TCS    8
-- 
2.5.5

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [dpdk-dev] [PATCH v1] eal: remove redundant id field in xstats name lookups
  2016-07-01 13:16 [dpdk-dev] [PATCH v1] eal: remove redundant id field in xstats name lookups Remy Horton
@ 2016-07-01 14:04 ` Thomas Monjalon
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Monjalon @ 2016-07-01 14:04 UTC (permalink / raw)
  To: Remy Horton; +Cc: dev

2016-07-01 14:16, Remy Horton:
> For all drivers that currently implement xstats, the id field in the
> rte_eth_stats_name structure equals the entry's array index. This
> patch eliminates the redundant id field as a direct index lookup is
> faster than a search for the matching id field.
> 
> Signed-off-by: Remy Horton <remy.horton@intel.com>

Suggested-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-07-01 14:04 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-01 13:16 [dpdk-dev] [PATCH v1] eal: remove redundant id field in xstats name lookups Remy Horton
2016-07-01 14:04 ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).