[dpdk-dev] [PATCH 1/2] lib: refactor basic stats code

[dpdk-dev] [PATCH 1/2] lib: refactor basic stats code

[Elza Mathew]

Moved the code to get the basic stats names and values
into static functions.

Signed-off-by: Elza Mathew <elza.mathew@intel.com>
---
 lib/librte_ether/rte_ethdev.c | 156 ++++++++++++++++++++++++------------------
 1 file changed, 91 insertions(+), 65 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 318af28..dfe8e65 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1641,6 +1641,45 @@ struct rte_eth_dev *
 	return -EINVAL;
 }
 
+/* retrieve basic stats names */
+static int
+rte_eth_basic_stats_get_names(struct rte_eth_dev *dev,
+	struct rte_eth_xstat_name *xstats_names)
+{
+	int cnt_used_entries = 0;
+	uint32_t idx, id_queue;
+	uint16_t num_q;
+
+	for (idx = 0; idx < RTE_NB_STATS; idx++) {
+		snprintf(xstats_names[cnt_used_entries].name,
+			sizeof(xstats_names[0].name),
+			"%s", rte_stats_strings[idx].name);
+		cnt_used_entries++;
+	}
+	num_q = RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+	for (id_queue = 0; id_queue < num_q; id_queue++) {
+		for (idx = 0; idx < RTE_NB_RXQ_STATS; idx++) {
+			snprintf(xstats_names[cnt_used_entries].name,
+				sizeof(xstats_names[0].name),
+				"rx_q%u%s",
+				id_queue, rte_rxq_stats_strings[idx].name);
+			cnt_used_entries++;
+		}
+
+	}
+	num_q = RTE_MIN(dev->data->nb_tx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+	for (id_queue = 0; id_queue < num_q; id_queue++) {
+		for (idx = 0; idx < RTE_NB_TXQ_STATS; idx++) {
+			snprintf(xstats_names[cnt_used_entries].name,
+				sizeof(xstats_names[0].name),
+				"tx_q%u%s",
+				id_queue, rte_txq_stats_strings[idx].name);
+			cnt_used_entries++;
+		}
+	}
+	return cnt_used_entries;
+}
+
 /* retrieve ethdev extended statistics names */
 int
 rte_eth_xstats_get_names_by_id(uint16_t port_id,
@@ -1739,8 +1778,6 @@ struct rte_eth_dev *
 	int cnt_used_entries;
 	int cnt_expected_entries;
 	int cnt_driver_entries;
-	uint32_t idx, id_queue;
-	uint16_t num_q;
 
 	cnt_expected_entries = get_xstats_count(port_id);
 	if (xstats_names == NULL || cnt_expected_entries < 0 ||
@@ -1749,35 +1786,9 @@ struct rte_eth_dev *
 
 	/* port_id checked in get_xstats_count() */
 	dev = &rte_eth_devices[port_id];
-	cnt_used_entries = 0;
 
-	for (idx = 0; idx < RTE_NB_STATS; idx++) {
-		snprintf(xstats_names[cnt_used_entries].name,
-			sizeof(xstats_names[0].name),
-			"%s", rte_stats_strings[idx].name);
-		cnt_used_entries++;
-	}
-	num_q = RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
-	for (id_queue = 0; id_queue < num_q; id_queue++) {
-		for (idx = 0; idx < RTE_NB_RXQ_STATS; idx++) {
-			snprintf(xstats_names[cnt_used_entries].name,
-				sizeof(xstats_names[0].name),
-				"rx_q%u%s",
-				id_queue, rte_rxq_stats_strings[idx].name);
-			cnt_used_entries++;
-		}
-
-	}
-	num_q = RTE_MIN(dev->data->nb_tx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
-	for (id_queue = 0; id_queue < num_q; id_queue++) {
-		for (idx = 0; idx < RTE_NB_TXQ_STATS; idx++) {
-			snprintf(xstats_names[cnt_used_entries].name,
-				sizeof(xstats_names[0].name),
-				"tx_q%u%s",
-				id_queue, rte_txq_stats_strings[idx].name);
-			cnt_used_entries++;
-		}
-	}
+	cnt_used_entries = rte_eth_basic_stats_get_names(
+		dev, xstats_names);
 
 	if (dev->dev_ops->xstats_get_names != NULL) {
 		/* If there are any driver-specific xstats, append them
@@ -1795,6 +1806,54 @@ struct rte_eth_dev *
 	return cnt_used_entries;
 }
 
+
+static int
+rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats)
+{
+	struct rte_eth_dev *dev;
+	struct rte_eth_stats eth_stats;
+	unsigned int count = 0, i, q;
+	uint64_t val, *stats_ptr;
+	uint16_t nb_rxqs, nb_txqs;
+
+	rte_eth_stats_get(port_id, &eth_stats);
+	dev = &rte_eth_devices[port_id];
+
+	nb_rxqs = RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+	nb_txqs = RTE_MIN(dev->data->nb_tx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+
+	/* global stats */
+	for (i = 0; i < RTE_NB_STATS; i++) {
+		stats_ptr = RTE_PTR_ADD(&eth_stats,
+					rte_stats_strings[i].offset);
+		val = *stats_ptr;
+		xstats[count++].value = val;
+	}
+
+	/* per-rxq stats */
+	for (q = 0; q < nb_rxqs; q++) {
+		for (i = 0; i < RTE_NB_RXQ_STATS; i++) {
+			stats_ptr = RTE_PTR_ADD(&eth_stats,
+					rte_rxq_stats_strings[i].offset +
+					q * sizeof(uint64_t));
+			val = *stats_ptr;
+			xstats[count++].value = val;
+		}
+	}
+
+	/* per-txq stats */
+	for (q = 0; q < nb_txqs; q++) {
+		for (i = 0; i < RTE_NB_TXQ_STATS; i++) {
+			stats_ptr = RTE_PTR_ADD(&eth_stats,
+					rte_txq_stats_strings[i].offset +
+					q * sizeof(uint64_t));
+			val = *stats_ptr;
+			xstats[count++].value = val;
+		}
+	}
+	return count;
+}
+
 /* retrieve ethdev extended statistics */
 int
 rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids,
@@ -1873,11 +1932,9 @@ struct rte_eth_dev *
 rte_eth_xstats_get(uint16_t port_id, struct rte_eth_xstat *xstats,
 	unsigned int n)
 {
-	struct rte_eth_stats eth_stats;
 	struct rte_eth_dev *dev;
-	unsigned int count = 0, i, q;
+	unsigned int count = 0, i;
 	signed int xcount = 0;
-	uint64_t val, *stats_ptr;
 	uint16_t nb_rxqs, nb_txqs;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
@@ -1908,38 +1965,7 @@ struct rte_eth_dev *
 		return count + xcount;
 
 	/* now fill the xstats structure */
-	count = 0;
-	rte_eth_stats_get(port_id, &eth_stats);
-
-	/* global stats */
-	for (i = 0; i < RTE_NB_STATS; i++) {
-		stats_ptr = RTE_PTR_ADD(&eth_stats,
-					rte_stats_strings[i].offset);
-		val = *stats_ptr;
-		xstats[count++].value = val;
-	}
-
-	/* per-rxq stats */
-	for (q = 0; q < nb_rxqs; q++) {
-		for (i = 0; i < RTE_NB_RXQ_STATS; i++) {
-			stats_ptr = RTE_PTR_ADD(&eth_stats,
-					rte_rxq_stats_strings[i].offset +
-					q * sizeof(uint64_t));
-			val = *stats_ptr;
-			xstats[count++].value = val;
-		}
-	}
-
-	/* per-txq stats */
-	for (q = 0; q < nb_txqs; q++) {
-		for (i = 0; i < RTE_NB_TXQ_STATS; i++) {
-			stats_ptr = RTE_PTR_ADD(&eth_stats,
-					rte_txq_stats_strings[i].offset +
-					q * sizeof(uint64_t));
-			val = *stats_ptr;
-			xstats[count++].value = val;
-		}
-	}
+	count = rte_eth_basic_stats_get(port_id, xstats);
 
 	for (i = 0; i < count; i++)
 		xstats[i].id = i;
-- 
1.9.1

[dpdk-dev] [PATCH 2/2] lib: optimize _xstats_by_ids APIs

[Elza Mathew]

Introduced a check to detect if the stats IDs being
requested are all basic stats IDs. In that case,
ensured that only the basic stats would be retrieved.
Previously, both basic stats and xstats were being
retrieved even if all the IDs were basic stats IDs.

Signed-off-by: Elza Mathew <elza.mathew@intel.com>
---
 lib/librte_ether/rte_ethdev.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index dfe8e65..210600d 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1688,7 +1688,9 @@ struct rte_eth_dev *
 {
 	struct rte_eth_xstat_name *xstats_names_copy;
 	unsigned int no_basic_stat_requested = 1;
+	unsigned int no_ext_stat_requested = 1;
 	unsigned int expected_entries;
+	unsigned int basic_count;
 	struct rte_eth_dev *dev;
 	unsigned int i;
 	int ret;
@@ -1696,6 +1698,7 @@ struct rte_eth_dev *
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	basic_count = get_xstats_basic_count(dev);
 	ret = get_xstats_count(port_id);
 	if (ret < 0)
 		return ret;
@@ -1713,7 +1716,6 @@ struct rte_eth_dev *
 		return -EINVAL;
 
 	if (ids && dev->dev_ops->xstats_get_names_by_id != NULL && size > 0) {
-		unsigned int basic_count = get_xstats_basic_count(dev);
 		uint64_t ids_copy[size];
 
 		for (i = 0; i < size; i++) {
@@ -1752,8 +1754,22 @@ struct rte_eth_dev *
 		return -ENOMEM;
 	}
 
+	if (ids) {
+		for (i = 0; i < size; i++) {
+			if (ids[i] > basic_count) {
+				no_ext_stat_requested = 0;
+				break;
+			}
+		}
+	}
+
 	/* Fill xstats_names_copy structure */
-	rte_eth_xstats_get_names(port_id, xstats_names_copy, expected_entries);
+	if (ids && no_ext_stat_requested) {
+		rte_eth_basic_stats_get_names(dev, xstats_names_copy);
+	} else {
+		rte_eth_xstats_get_names(port_id, xstats_names_copy,
+			expected_entries);
+	}
 
 	/* Filter stats */
 	for (i = 0; i < size; i++) {
@@ -1860,7 +1876,9 @@ struct rte_eth_dev *
 			 uint64_t *values, unsigned int size)
 {
 	unsigned int no_basic_stat_requested = 1;
+	unsigned int no_ext_stat_requested = 1;
 	unsigned int num_xstats_filled;
+	unsigned int basic_count;
 	uint16_t expected_entries;
 	struct rte_eth_dev *dev;
 	unsigned int i;
@@ -1870,6 +1888,7 @@ struct rte_eth_dev *
 	expected_entries = get_xstats_count(port_id);
 	struct rte_eth_xstat xstats[expected_entries];
 	dev = &rte_eth_devices[port_id];
+	basic_count = get_xstats_basic_count(dev);
 
 	/* Return max number of stats if no ids given */
 	if (!ids) {
@@ -1904,8 +1923,21 @@ struct rte_eth_dev *
 					values, size);
 	}
 
+	if (ids) {
+		for (i = 0; i < size; i++) {
+			if (ids[i] > basic_count) {
+				no_ext_stat_requested = 0;
+				break;
+			}
+		}
+	}
+
 	/* Fill the xstats structure */
-	ret = rte_eth_xstats_get(port_id, xstats, expected_entries);
+	if (ids && no_ext_stat_requested)
+		ret = rte_eth_basic_stats_get(port_id, xstats);
+	else
+		ret = rte_eth_xstats_get(port_id, xstats, expected_entries);
+
 	if (ret < 0)
 		return ret;
 	num_xstats_filled = (unsigned int)ret;
-- 
1.9.1

Re: [dpdk-dev] [PATCH 1/2] lib: refactor basic stats code

[Daly, Lee]

> Subject: [dpdk-dev] [PATCH 1/2] lib: refactor basic stats code
> 
> Moved the code to get the basic stats names and values into static functions.
> 
> Signed-off-by: Elza Mathew <elza.mathew@intel.com>

Reviewed-by: Lee Daly <lee.daly@intel.com>

Re: [dpdk-dev] [PATCH 2/2] lib: optimize _xstats_by_ids APIs

[Daly, Lee]

> Subject: [dpdk-dev] [PATCH 2/2] lib: optimize _xstats_by_ids APIs
> 
> Introduced a check to detect if the stats IDs being requested are all basic
> stats IDs. In that case, ensured that only the basic stats would be retrieved.
> Previously, both basic stats and xstats were being retrieved even if all the IDs
> were basic stats IDs.
> 
> Signed-off-by: Elza Mathew <elza.mathew@intel.com>

Reviewed-by: Lee Daly <lee.daly@intel.com>

Re: [dpdk-dev] [PATCH 1/2] lib: refactor basic stats code

[Ferruh Yigit]

On 12/11/2017 1:27 PM, Daly, Lee wrote:
>> Subject: [dpdk-dev] [PATCH 1/2] lib: refactor basic stats code
>>
>> Moved the code to get the basic stats names and values into static functions.
>>
>> Signed-off-by: Elza Mathew <elza.mathew@intel.com>
> 
> Reviewed-by: Lee Daly <lee.daly@intel.com>

Series applied to dpdk-next-net/master, thanks.