telemetry: fix error when using ports of different types

Message ID 20181219115950.46386-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series telemetry: fix error when using ports of different types |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Bruce Richardson Dec. 19, 2018, 11:59 a.m. UTC
  Different NIC ports can have different numbers of xstats on them, which
means that we can't just use the xstats list from the first port registered
in the telemetry library. Instead, we need to check the type of each port -
by checking its ops structure pointer - and register each port type once
with the metrics lib.

CC: stable@dpdk.org
Fixes: fdbdb3f9ce46 ("telemetry: add initial connection socket")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_telemetry/rte_telemetry.c          | 40 ++++++++++++++-----
 lib/librte_telemetry/rte_telemetry_internal.h |  2 +-
 2 files changed, 32 insertions(+), 10 deletions(-)
  

Comments

Thomas Monjalon Dec. 20, 2018, 11:51 p.m. UTC | #1
19/12/2018 12:59, Bruce Richardson:
> Different NIC ports can have different numbers of xstats on them, which
> means that we can't just use the xstats list from the first port registered
> in the telemetry library. Instead, we need to check the type of each port -
> by checking its ops structure pointer - and register each port type once
> with the metrics lib.
> 
> CC: stable@dpdk.org
> Fixes: fdbdb3f9ce46 ("telemetry: add initial connection socket")
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Kevin, any comment please?
  
Kevin Laatz Dec. 21, 2018, 1:27 p.m. UTC | #2
On 19/12/2018 11:59, Bruce Richardson wrote:
> Different NIC ports can have different numbers of xstats on them, which
> means that we can't just use the xstats list from the first port registered
> in the telemetry library. Instead, we need to check the type of each port -
> by checking its ops structure pointer - and register each port type once
> with the metrics lib.
>
> CC: stable@dpdk.org
> Fixes: fdbdb3f9ce46 ("telemetry: add initial connection socket")
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Acked-by: Kevin Laatz <kevin.laatz@intel.com>
  
Thomas Monjalon Dec. 21, 2018, 3:33 p.m. UTC | #3
21/12/2018 14:27, Laatz, Kevin:
> On 19/12/2018 11:59, Bruce Richardson wrote:
> > Different NIC ports can have different numbers of xstats on them, which
> > means that we can't just use the xstats list from the first port registered
> > in the telemetry library. Instead, we need to check the type of each port -
> > by checking its ops structure pointer - and register each port type once
> > with the metrics lib.
> >
> > CC: stable@dpdk.org
> > Fixes: fdbdb3f9ce46 ("telemetry: add initial connection socket")
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Acked-by: Kevin Laatz <kevin.laatz@intel.com>

Applied, thanks
  

Patch

diff --git a/lib/librte_telemetry/rte_telemetry.c b/lib/librte_telemetry/rte_telemetry.c
index 016431f12..7fb247eaa 100644
--- a/lib/librte_telemetry/rte_telemetry.c
+++ b/lib/librte_telemetry/rte_telemetry.c
@@ -558,7 +558,7 @@  rte_telemetry_send_ports_stats_values(uint32_t *metric_ids, int num_metric_ids,
 		}
 
 		ret = rte_telemetry_update_metrics_ethdev(telemetry,
-				port_ids[i], telemetry->reg_index);
+				port_ids[i], telemetry->reg_index[i]);
 		if (ret < 0) {
 			TELEMETRY_LOG_ERR("Failed to update ethdev metrics");
 			return -1;
@@ -658,23 +658,45 @@  rte_telemetry_reg_ethdev_to_metrics(uint16_t port_id)
 static int32_t
 rte_telemetry_initial_accept(struct telemetry_impl *telemetry)
 {
+	struct driver_index {
+		const void *dev_ops;
+		int reg_index;
+	} drv_idx[RTE_MAX_ETHPORTS];
+	int nb_drv_idx = 0;
 	uint16_t pid;
 	int ret;
 	int selftest = 0;
 
 	RTE_ETH_FOREACH_DEV(pid) {
-		telemetry->reg_index = rte_telemetry_reg_ethdev_to_metrics(pid);
-		break;
-	}
+		int i;
+		/* Different device types have different numbers of stats, so
+		 * first check if the stats for this type of device have
+		 * already been registered
+		 */
+		for (i = 0; i < nb_drv_idx; i++) {
+			if (rte_eth_devices[pid].dev_ops == drv_idx[i].dev_ops) {
+				telemetry->reg_index[pid] = drv_idx[i].reg_index;
+				break;
+			}
+		}
+		if (i < nb_drv_idx)
+			continue; /* we found a match, go to next port */
 
-	if (telemetry->reg_index < 0) {
-		TELEMETRY_LOG_ERR("Failed to register ethdev metrics");
-		return -1;
+		/* No match, register a new set of xstats for this port */
+		ret = rte_telemetry_reg_ethdev_to_metrics(pid);
+		if (ret < 0) {
+			TELEMETRY_LOG_ERR("Failed to register ethdev metrics");
+			return -1;
+		}
+		telemetry->reg_index[pid] = ret;
+		drv_idx[nb_drv_idx].dev_ops = rte_eth_devices[pid].dev_ops;
+		drv_idx[nb_drv_idx].reg_index = ret;
+		nb_drv_idx++;
 	}
 
 	telemetry->metrics_register_done = 1;
 	if (selftest) {
-		ret = rte_telemetry_socket_messaging_testing(telemetry->reg_index,
+		ret = rte_telemetry_socket_messaging_testing(telemetry->reg_index[0],
 				telemetry->server_fd);
 		if (ret < 0)
 			return -1;
@@ -1299,7 +1321,7 @@  rte_telemetry_socket_messaging_testing(int index, int socket)
 	}
 
 	telemetry->server_fd = socket;
-	telemetry->reg_index = index;
+	telemetry->reg_index[0] = index;
 	TELEMETRY_LOG_INFO("Beginning Telemetry socket message Selftest");
 	rte_telemetry_socket_test_setup(telemetry, &send_fd, &recv_fd);
 	TELEMETRY_LOG_INFO("Register valid client test");
diff --git a/lib/librte_telemetry/rte_telemetry_internal.h b/lib/librte_telemetry/rte_telemetry_internal.h
index de7afda30..c298c3919 100644
--- a/lib/librte_telemetry/rte_telemetry_internal.h
+++ b/lib/librte_telemetry/rte_telemetry_internal.h
@@ -36,7 +36,7 @@  typedef struct telemetry_impl {
 	pthread_t thread_id;
 	int thread_status;
 	uint32_t socket_id;
-	int reg_index;
+	int reg_index[RTE_MAX_ETHPORTS];
 	int metrics_register_done;
 	TAILQ_HEAD(, telemetry_client) client_list_head;
 	struct telemetry_client *request_client;