From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Marchand Subject: [PATCH] app/testpmd: add sanity checks when retrieving xstats Date: Thu, 7 Jun 2018 10:15:23 +0200 Message-ID: <1528359323-22885-1-git-send-email-david.marchand@6wind.com> Cc: wenzhuo.lu@intel.com, jingjing.wu@intel.com, bernard.iremonger@intel.com, remy.horton@intel.com, stable@dpdk.org To: dev@dpdk.org Return-path: Received: from mail-wm0-f67.google.com (mail-wm0-f67.google.com [74.125.82.67]) by dpdk.org (Postfix) with ESMTP id 0FA6E1B01F for ; Thu, 7 Jun 2018 10:15:29 +0200 (CEST) Received: by mail-wm0-f67.google.com with SMTP id q4-v6so2044022wmq.1 for ; Thu, 07 Jun 2018 01:15:29 -0700 (PDT) List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 --- @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