DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@6wind.com>
To: dev@dpdk.org
Cc: wenzhuo.lu@intel.com, jingjing.wu@intel.com,
	bernard.iremonger@intel.com, remy.horton@intel.com,
	stable@dpdk.org
Subject: [dpdk-dev] [PATCH] app/testpmd: add sanity checks when retrieving xstats
Date: Thu,  7 Jun 2018 10:15:23 +0200	[thread overview]
Message-ID: <1528359323-22885-1-git-send-email-david.marchand@6wind.com> (raw)

Testpmd should not expect the xstats names and values arrays to be
aligned: neither the arrays sizes, nor the order in which the values are.

This hid some bugs where pmds would either return wrong values count or
invalid statistics indexes.

Link: http://dpdk.org/browse/dpdk/commit/?id=5fd4d049692b2fde8bf49c7461b18180a8fd2545
Link: http://dpdk.org/dev/patchwork/patch/40705/

Signed-off-by: David Marchand <david.marchand@6wind.com>
---

@stable: when this goes in, I recommend backporting this to all existing
branches, as it makes it easier to show this kind of pmds bugs.

---
 app/test-pmd/config.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 97020fb..8edb80c 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -210,9 +210,11 @@ nic_stats_clear(portid_t port_id)
 void
 nic_xstats_display(portid_t port_id)
 {
-	struct rte_eth_xstat *xstats;
-	int cnt_xstats, idx_xstat;
 	struct rte_eth_xstat_name *xstats_names;
+	struct rte_eth_xstat *xstats;
+	int cnt_xnames;
+	int cnt_xstats;
+	int idx_xstat;
 
 	printf("###### NIC extended statistics for port %-2d\n", port_id);
 	if (!rte_eth_dev_is_valid_port(port_id)) {
@@ -221,33 +223,34 @@ nic_xstats_display(portid_t port_id)
 	}
 
 	/* Get count */
-	cnt_xstats = rte_eth_xstats_get_names(port_id, NULL, 0);
-	if (cnt_xstats  < 0) {
+	cnt_xnames = rte_eth_xstats_get_names(port_id, NULL, 0);
+	if (cnt_xnames  < 0) {
 		printf("Error: Cannot get count of xstats\n");
 		return;
 	}
 
 	/* Get id-name lookup table */
-	xstats_names = malloc(sizeof(struct rte_eth_xstat_name) * cnt_xstats);
+	xstats_names = malloc(sizeof(struct rte_eth_xstat_name) * cnt_xnames);
 	if (xstats_names == NULL) {
 		printf("Cannot allocate memory for xstats lookup\n");
 		return;
 	}
-	if (cnt_xstats != rte_eth_xstats_get_names(
-			port_id, xstats_names, cnt_xstats)) {
+	if (cnt_xnames != rte_eth_xstats_get_names(
+			port_id, xstats_names, cnt_xnames)) {
 		printf("Error: Cannot get xstats lookup\n");
 		free(xstats_names);
 		return;
 	}
 
 	/* Get stats themselves */
-	xstats = malloc(sizeof(struct rte_eth_xstat) * cnt_xstats);
+	xstats = malloc(sizeof(struct rte_eth_xstat) * cnt_xnames);
 	if (xstats == NULL) {
 		printf("Cannot allocate memory for xstats\n");
 		free(xstats_names);
 		return;
 	}
-	if (cnt_xstats != rte_eth_xstats_get(port_id, xstats, cnt_xstats)) {
+	cnt_xstats = rte_eth_xstats_get(port_id, xstats, cnt_xnames);
+	if (cnt_xstats > cnt_xnames) {
 		printf("Error: Unable to get xstats\n");
 		free(xstats_names);
 		free(xstats);
@@ -256,10 +259,15 @@ nic_xstats_display(portid_t port_id)
 
 	/* Display xstats */
 	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) {
+		if (xstats[idx_xstat].id > (uint64_t)cnt_xnames) {
+			printf("Error: Invalid statistic index: %"PRId64
+			       ", max %d\n", xstats[idx_xstat].id, cnt_xnames);
+			continue;
+		}
 		if (xstats_hide_zero && !xstats[idx_xstat].value)
 			continue;
 		printf("%s: %"PRIu64"\n",
-			xstats_names[idx_xstat].name,
+			xstats_names[xstats[idx_xstat].id].name,
 			xstats[idx_xstat].value);
 	}
 	free(xstats_names);
-- 
2.7.4

             reply	other threads:[~2018-06-07  8:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-07  8:15 David Marchand [this message]
2018-06-12  7:09 ` David Marchand
2018-06-12  8:38 ` Remy Horton
2018-06-13 15:39 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2018-06-14  6:39   ` Remy Horton
2018-06-14 10:55     ` Ferruh Yigit
2018-06-14 19:33       ` Remy Horton
2018-06-28 14:16 ` [dpdk-dev] [PATCH] ethdev: document xstats API assumption Ferruh Yigit
2018-06-29  7:15   ` David Marchand
2018-07-04 17:38     ` Ferruh Yigit
2018-07-03 16:11   ` Van Haaren, Harry
2018-07-07 17:45   ` Andrew Rybchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1528359323-22885-1-git-send-email-david.marchand@6wind.com \
    --to=david.marchand@6wind.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=jingjing.wu@intel.com \
    --cc=remy.horton@intel.com \
    --cc=stable@dpdk.org \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).