[dpdk-dev] [PATCH] lib/telemetry: add support to fetch global metrics

[dpdk-dev] [PATCH] lib/telemetry: add support to fetch global metrics

[Reshma Pattan]

telemetry has support for fetching port based stats
from metrics library.

Metrics library also has global stats which are
not fetched by telemetry, so extend telemetry to
fetch the global metrics.

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 doc/guides/howto/telemetry.rst                |   9 +-
 doc/guides/rel_notes/release_19_08.rst        |   5 +
 lib/librte_telemetry/rte_telemetry.c          | 119 ++++++++++++----
 lib/librte_telemetry/rte_telemetry_internal.h |  31 ++++-
 lib/librte_telemetry/rte_telemetry_parser.c   | 130 +++++++++++++++---
 usertools/dpdk-telemetry-client.py            |  13 +-
 6 files changed, 253 insertions(+), 54 deletions(-)

diff --git a/doc/guides/howto/telemetry.rst b/doc/guides/howto/telemetry.rst
index 00f8f7a85..cacc08216 100644
--- a/doc/guides/howto/telemetry.rst
+++ b/doc/guides/howto/telemetry.rst
@@ -11,9 +11,10 @@ Introduction
 ------------
 
 The ``librte_telemetry`` provides the functionality so that users may query
-metrics from incoming port traffic. The application which initializes packet
-forwarding will act as the server, sending metrics to the requesting application
-which acts as the client.
+metrics from incoming port traffic and global stats(application stats).
+The application which initializes packet forwarding will act as the server,
+sending metrics to the requesting application which acts as the client.
+
 
 In DPDK, applications are used to initialize the ``telemetry``. To view incoming
 traffic on featured ports, the application should be run first (ie. after ports
@@ -79,7 +80,7 @@ any DPDK application is applicable.
    the menu.
 
 #. Send traffic to any or all available ports from a traffic generator.
-   Select a query option(recursive or singular polling).
+   Select a query option(recursive or singular polling or global stats).
    The metrics will then be displayed on the client terminal in JSON format.
 
 #. Once finished, unregister the client using the menu command.
diff --git a/doc/guides/rel_notes/release_19_08.rst b/doc/guides/rel_notes/release_19_08.rst
index b9510f93a..a0bf9923a 100644
--- a/doc/guides/rel_notes/release_19_08.rst
+++ b/doc/guides/rel_notes/release_19_08.rst
@@ -54,6 +54,11 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+* **Updated telemetry library for global metrics support.**
+
+  Updated ``librte_telemetry`` to fetch the global metrics from the
+  ``librte_metrics`` library.
+
 
 Removed Items
 -------------
diff --git a/lib/librte_telemetry/rte_telemetry.c b/lib/librte_telemetry/rte_telemetry.c
index b852630c5..dabf2c775 100644
--- a/lib/librte_telemetry/rte_telemetry.c
+++ b/lib/librte_telemetry/rte_telemetry.c
@@ -449,17 +449,14 @@ rte_telemetry_json_format_port(struct telemetry_impl *telemetry,
 
 static int32_t
 rte_telemetry_encode_json_format(struct telemetry_impl *telemetry,
-	uint32_t *port_ids, uint32_t num_port_ids, uint32_t *metric_ids,
-	uint32_t num_metric_ids, char **json_buffer)
+	struct telemetry_encode_param *ep, char **json_buffer)
 {
 	int ret;
 	json_t *root, *ports;
 	uint32_t i;
-
-	if (num_port_ids <= 0 || num_metric_ids <= 0) {
-		TELEMETRY_LOG_ERR("Please provide port and metric ids to query");
-		goto einval_fail;
-	}
+	uint32_t port_id;
+	uint32_t num_port_ids;
+	uint32_t num_metric_ids;
 
 	ports = json_array();
 	if (ports == NULL) {
@@ -467,20 +464,47 @@ rte_telemetry_encode_json_format(struct telemetry_impl *telemetry,
 		goto eperm_fail;
 	}
 
-	for (i = 0; i < num_port_ids; i++) {
-		if (!rte_eth_dev_is_valid_port(port_ids[i])) {
-			TELEMETRY_LOG_ERR("Port: %d invalid", port_ids[i]);
+	if (ep->type == PORT_STATS) {
+		num_port_ids = ep->pp.num_port_ids;
+		num_metric_ids = ep->pp.num_metric_ids;
+
+		if (num_port_ids <= 0 || num_metric_ids <= 0) {
+			TELEMETRY_LOG_ERR("Please provide port and metric ids to query");
 			goto einval_fail;
 		}
-	}
 
-	for (i = 0; i < num_port_ids; i++) {
-		ret = rte_telemetry_json_format_port(telemetry, port_ids[i],
-			ports, metric_ids, num_metric_ids);
+		for (i = 0; i < num_port_ids; i++) {
+			port_id = ep->pp.port_ids[i];
+			if (!rte_eth_dev_is_valid_port(port_id)) {
+				TELEMETRY_LOG_ERR("Port: %d invalid",
+							port_id);
+				goto einval_fail;
+			}
+		}
+
+		for (i = 0; i < num_port_ids; i++) {
+			port_id = ep->pp.port_ids[i];
+			ret = rte_telemetry_json_format_port(telemetry,
+					port_id, ports, &ep->pp.metric_ids[0],
+					num_metric_ids);
+			if (ret < 0) {
+				TELEMETRY_LOG_ERR("Format port in JSON failed");
+				return -1;
+			}
+		}
+	} else if (ep->type == GLOBAL_STATS) {
+		/* Request Global Metrics */
+		ret = rte_telemetry_json_format_port(telemetry,
+				RTE_METRICS_GLOBAL,
+				ports, &ep->gp.metric_ids[0],
+				ep->gp.num_metric_ids);
 		if (ret < 0) {
-			TELEMETRY_LOG_ERR("Format port in JSON failed");
+			TELEMETRY_LOG_ERR(" Request Global Metrics Failed");
 			return -1;
 		}
+	} else {
+		TELEMETRY_LOG_ERR(" Invalid metrics type in encode params");
+		goto einval_fail;
 	}
 
 	root = json_object();
@@ -520,8 +544,8 @@ rte_telemetry_encode_json_format(struct telemetry_impl *telemetry,
 }
 
 int32_t
-rte_telemetry_send_ports_stats_values(uint32_t *metric_ids, int num_metric_ids,
-	uint32_t *port_ids, int num_port_ids, struct telemetry_impl *telemetry)
+rte_telemetry_send_global_stats_values(struct telemetry_encode_param *ep,
+	struct telemetry_impl *telemetry)
 {
 	int ret, i;
 	char *json_buffer = NULL;
@@ -531,42 +555,77 @@ rte_telemetry_send_ports_stats_values(uint32_t *metric_ids, int num_metric_ids,
 		return -1;
 	}
 
-	if (metric_ids == NULL) {
-		TELEMETRY_LOG_ERR("Invalid metric_ids array");
+	if (ep->gp.num_metric_ids < 0) {
+		TELEMETRY_LOG_ERR("Invalid num_metric_ids, must be positive");
 		goto einval_fail;
 	}
 
-	if (num_metric_ids < 0) {
-		TELEMETRY_LOG_ERR("Invalid num_metric_ids, must be positive");
+	ret = rte_telemetry_encode_json_format(telemetry, ep,
+		&json_buffer);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("JSON encode function failed");
+		return -1;
+	}
+
+	ret = rte_telemetry_write_to_socket(telemetry, json_buffer);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Could not write to socket");
+		return -1;
+	}
+
+	return 0;
+
+einval_fail:
+	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+	if (ret < 0)
+		TELEMETRY_LOG_ERR("Could not send error");
+	return -1;
+}
+
+int32_t
+rte_telemetry_send_ports_stats_values(struct telemetry_encode_param *ep,
+	struct telemetry_impl *telemetry)
+{
+	int ret, i;
+	char *json_buffer = NULL;
+	uint32_t port_id;
+
+	if (telemetry == NULL) {
+		TELEMETRY_LOG_ERR("Invalid telemetry argument");
+		return -1;
+	}
+
+	if (ep == NULL) {
+		TELEMETRY_LOG_ERR("Invalid encode param argument");
 		goto einval_fail;
 	}
 
-	if (port_ids == NULL) {
-		TELEMETRY_LOG_ERR("Invalid port_ids array");
+	if (ep->pp.num_metric_ids < 0) {
+		TELEMETRY_LOG_ERR("Invalid num_metric_ids, must be positive");
 		goto einval_fail;
 	}
 
-	if (num_port_ids < 0) {
+	if (ep->pp.num_port_ids < 0) {
 		TELEMETRY_LOG_ERR("Invalid num_port_ids, must be positive");
 		goto einval_fail;
 	}
 
-	for (i = 0; i < num_port_ids; i++) {
-		if (!rte_eth_dev_is_valid_port(port_ids[i])) {
-			TELEMETRY_LOG_ERR("Port: %d invalid", port_ids[i]);
+	for (i = 0; i < ep->pp.num_port_ids; i++) {
+		port_id = ep->pp.port_ids[i];
+		if (!rte_eth_dev_is_valid_port(port_id)) {
+			TELEMETRY_LOG_ERR("Port: %d invalid", port_id);
 			goto einval_fail;
 		}
 
 		ret = rte_telemetry_update_metrics_ethdev(telemetry,
-				port_ids[i], telemetry->reg_index[i]);
+				port_id, telemetry->reg_index[i]);
 		if (ret < 0) {
 			TELEMETRY_LOG_ERR("Failed to update ethdev metrics");
 			return -1;
 		}
 	}
 
-	ret = rte_telemetry_encode_json_format(telemetry, port_ids,
-		num_port_ids, metric_ids, num_metric_ids, &json_buffer);
+	ret = rte_telemetry_encode_json_format(telemetry, ep, &json_buffer);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("JSON encode function failed");
 		return -1;
diff --git a/lib/librte_telemetry/rte_telemetry_internal.h b/lib/librte_telemetry/rte_telemetry_internal.h
index c298c3919..6a91d0c44 100644
--- a/lib/librte_telemetry/rte_telemetry_internal.h
+++ b/lib/librte_telemetry/rte_telemetry_internal.h
@@ -24,6 +24,8 @@ extern int telemetry_log_level;
 #define TELEMETRY_LOG_INFO(fmt, args...) \
 	TELEMETRY_LOG(INFO, fmt, ## args)
 
+#define MAX_METRICS 256
+
 typedef struct telemetry_client {
 	char *file_path;
 	int fd;
@@ -48,6 +50,28 @@ enum rte_telemetry_parser_actions {
 	ACTION_DELETE = 2
 };
 
+enum rte_telemetry_stats_type {
+	PORT_STATS = 0,
+	GLOBAL_STATS = 1
+};
+
+/* @internal */
+struct telemetry_encode_param {
+	enum rte_telemetry_stats_type type;
+	union {
+		struct port_param {
+			uint32_t num_metric_ids;
+			uint32_t metric_ids[MAX_METRICS];
+			uint32_t num_port_ids;
+			uint32_t port_ids[RTE_MAX_ETHPORTS];
+		} pp;
+		struct global_param {
+			uint32_t num_metric_ids;
+			uint32_t metric_ids[MAX_METRICS];
+		} gp;
+	};
+};
+
 int32_t
 rte_telemetry_parse_client_message(struct telemetry_impl *telemetry, char *buf);
 
@@ -72,10 +96,13 @@ int32_t
 rte_telemetry_is_port_active(int port_id);
 
 int32_t
-rte_telemetry_send_ports_stats_values(uint32_t *metric_ids, int num_metric_ids,
-	uint32_t *port_ids, int num_port_ids, struct telemetry_impl *telemetry);
+rte_telemetry_send_ports_stats_values(struct telemetry_encode_param *ep,
+	struct telemetry_impl *telemetry);
 
 int32_t
 rte_telemetry_socket_messaging_testing(int index, int socket);
 
+int32_t
+rte_telemetry_send_global_stats_values(struct telemetry_encode_param *ep,
+	struct telemetry_impl *telemetry);
 #endif
diff --git a/lib/librte_telemetry/rte_telemetry_parser.c b/lib/librte_telemetry/rte_telemetry_parser.c
index 9bc16eef4..3a2fa0b49 100644
--- a/lib/librte_telemetry/rte_telemetry_parser.c
+++ b/lib/librte_telemetry/rte_telemetry_parser.c
@@ -258,8 +258,9 @@ rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
 	int ret, num_metrics, i, p;
 	struct rte_metric_value *values;
 	uint64_t num_port_ids = 0;
-	uint32_t port_ids[RTE_MAX_ETHPORTS];
+	struct telemetry_encode_param ep;
 
+	memset(&ep, 0, sizeof(ep));
 	if (telemetry == NULL) {
 		TELEMETRY_LOG_ERR("Invalid telemetry argument");
 		return -1;
@@ -310,10 +311,8 @@ rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
 		return -1;
 	}
 
-	uint32_t stat_ids[num_metrics];
-
 	RTE_ETH_FOREACH_DEV(p) {
-		port_ids[num_port_ids] = p;
+		ep.pp.port_ids[num_port_ids] = p;
 		num_port_ids++;
 	}
 
@@ -327,16 +326,22 @@ rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
 		goto fail;
 	}
 
-	ret = rte_metrics_get_values(port_ids[0], values, num_metrics);
+	ret = rte_metrics_get_values(ep.pp.port_ids[0], values, num_metrics);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("Could not get stat values");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
 		goto fail;
 	}
 	for (i = 0; i < num_metrics; i++)
-		stat_ids[i] = values[i].key;
+		ep.pp.metric_ids[i] = values[i].key;
+
+	ep.pp.num_port_ids = num_port_ids;
+	ep.pp.num_metric_ids = num_metrics;
+	ep.type = PORT_STATS;
 
-	ret = rte_telemetry_send_ports_stats_values(stat_ids, num_metrics,
-		port_ids, num_port_ids, telemetry);
+	ret = rte_telemetry_send_ports_stats_values(&ep, telemetry);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("Sending ports stats values failed");
 		goto fail;
@@ -349,6 +354,93 @@ rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
 	return -1;
 }
 
+int32_t
+rte_telemetry_command_global_stat_values(struct telemetry_impl *telemetry,
+	 int action, json_t *data)
+{
+	int ret, num_metrics, i, p;
+	struct rte_metric_value *values;
+	struct telemetry_encode_param ep;
+
+	memset(&ep, 0, sizeof(ep));
+	if (telemetry == NULL) {
+		TELEMETRY_LOG_ERR("Invalid telemetry argument");
+		return -1;
+	}
+
+	if (action != ACTION_GET) {
+		TELEMETRY_LOG_WARN("Invalid action for this command");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+		return -1;
+	}
+
+	if (json_is_object(data)) {
+		TELEMETRY_LOG_WARN("Invalid data provided for this command");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+		return -1;
+	}
+
+	num_metrics = rte_metrics_get_values(RTE_METRICS_GLOBAL, NULL, 0);
+	if (num_metrics < 0) {
+		TELEMETRY_LOG_ERR("Cannot get metrics count");
+
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+
+		return -1;
+	} else if (num_metrics == 0) {
+		TELEMETRY_LOG_ERR("No metrics to display (none have been registered)");
+
+		ret = rte_telemetry_send_error_response(telemetry, -EPERM);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+
+		return -1;
+	}
+
+	values = malloc(sizeof(struct rte_metric_value) * num_metrics);
+	if (values == NULL) {
+		TELEMETRY_LOG_ERR("Cannot allocate memory");
+		ret = rte_telemetry_send_error_response(telemetry,
+			 -ENOMEM);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+		return -1;
+	}
+
+	ret = rte_metrics_get_values(RTE_METRICS_GLOBAL, values, num_metrics);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Could not get stat values");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+		goto fail;
+	}
+	for (i = 0; i < num_metrics; i++)
+		ep.gp.metric_ids[i] = values[i].key;
+
+	ep.gp.num_metric_ids = num_metrics;
+	ep.type = GLOBAL_STATS;
+
+	ret = rte_telemetry_send_global_stats_values(&ep, telemetry);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Sending global stats values failed");
+		goto fail;
+	}
+
+	free(values);
+	return 0;
+
+fail:
+	free(values);
+	return -1;
+}
+
 int32_t
 rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
 	*telemetry, int action, json_t *data)
@@ -356,13 +448,15 @@ rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
 	int ret;
 	json_t *port_ids_json = json_object_get(data, "ports");
 	json_t *stat_names_json = json_object_get(data, "stats");
-	uint64_t num_port_ids = json_array_size(port_ids_json);
 	uint64_t num_stat_names = json_array_size(stat_names_json);
 	const char *stat_names[num_stat_names];
-	uint32_t port_ids[num_port_ids], stat_ids[num_stat_names];
+	struct telemetry_encode_param ep;
 	size_t index;
 	json_t *value;
 
+	ep.pp.num_port_ids = json_array_size(port_ids_json);
+	ep.pp.num_metric_ids = num_stat_names;
+	memset(&ep, 0, sizeof(ep));
 	if (telemetry == NULL) {
 		TELEMETRY_LOG_ERR("Invalid telemetry argument");
 		return -1;
@@ -402,8 +496,8 @@ rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
 				TELEMETRY_LOG_ERR("Could not send error");
 			return -1;
 		}
-		port_ids[index] = json_integer_value(value);
-		ret = rte_telemetry_is_port_active(port_ids[index]);
+		ep.pp.port_ids[index] = json_integer_value(value);
+		ret = rte_telemetry_is_port_active(ep.pp.port_ids[index]);
 		if (ret < 1) {
 			ret = rte_telemetry_send_error_response(telemetry,
 				-EINVAL);
@@ -427,15 +521,15 @@ rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
 		stat_names[index] = json_string_value(value);
 	}
 
-	ret = rte_telemetry_stat_names_to_ids(telemetry, stat_names, stat_ids,
-		num_stat_names);
+	ret = rte_telemetry_stat_names_to_ids(telemetry, stat_names,
+		ep.pp.metric_ids, num_stat_names);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("Could not convert stat names to IDs");
 		return -1;
 	}
 
-	ret = rte_telemetry_send_ports_stats_values(stat_ids, num_stat_names,
-		port_ids, num_port_ids, telemetry);
+	ep.type = PORT_STATS;
+	ret = rte_telemetry_send_ports_stats_values(&ep, telemetry);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("Sending ports stats values failed");
 		return -1;
@@ -480,6 +574,10 @@ rte_telemetry_parse_command(struct telemetry_impl *telemetry, int action,
 		{
 			.text = "ports_all_stat_values",
 			.fn = &rte_telemetry_command_ports_all_stat_values
+		},
+		{
+			.text = "global_stat_values",
+			.fn = &rte_telemetry_command_global_stat_values
 		}
 	};
 
diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
index ce0c7a905..572ff56bb 100755
--- a/usertools/dpdk-telemetry-client.py
+++ b/usertools/dpdk-telemetry-client.py
@@ -12,6 +12,7 @@
 METRICS_REQ = "{\"action\":0,\"command\":\"ports_all_stat_values\",\"data\":null}"
 API_REG = "{\"action\":1,\"command\":\"clients\",\"data\":{\"client_path\":\""
 API_UNREG = "{\"action\":2,\"command\":\"clients\",\"data\":{\"client_path\":\""
+GLOBAL_METRICS_REQ = "{\"action\":0,\"command\":\"global_stat_values\",\"data\":null}"
 DEFAULT_FP = "/var/run/dpdk/default_client"
 
 class Socket:
@@ -79,12 +80,18 @@ def repeatedlyRequestMetrics(self, sleep_time): # Recursively requests metrics f
             self.requestMetrics()
             time.sleep(sleep_time)
 
+    def requestGlobalMetrics(self): #Requests global metrics for given client
+        self.socket.client_fd.send(GLOBAL_METRICS_REQ)
+        data = self.socket.client_fd.recv(BUFFER_SIZE)
+        print "\nResponse: \n", str(data)
+
     def interactiveMenu(self, sleep_time): # Creates Interactive menu within the script
-        while self.choice != 3:
+        while self.choice != 4:
             print("\nOptions Menu")
             print("[1] Send for Metrics for all ports")
             print("[2] Send for Metrics for all ports recursively")
-            print("[3] Unregister client")
+            print("[3] Send for global Metrics")
+            print("[4] Unregister client")
 
             try:
                 self.choice = int(input("\n:"))
@@ -95,6 +102,8 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
                 elif self.choice == 2:
                     self.repeatedlyRequestMetrics(sleep_time)
                 elif self.choice == 3:
+                    self.requestGlobalMetrics()
+                elif self.choice == 4:
                     self.unregister()
                     self.unregistered = 1
                 else:
-- 
2.21.0

Re: [dpdk-dev] [PATCH] lib/telemetry: add support to fetch global metrics

[Laatz, Kevin]

On 17/05/2019 17:07, Reshma Pattan wrote:
> telemetry has support for fetching port based stats
> from metrics library.
>
> Metrics library also has global stats which are
> not fetched by telemetry, so extend telemetry to
> fetch the global metrics.
>
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
>   doc/guides/howto/telemetry.rst                |   9 +-
>   doc/guides/rel_notes/release_19_08.rst        |   5 +
>   lib/librte_telemetry/rte_telemetry.c          | 119 ++++++++++++----
>   lib/librte_telemetry/rte_telemetry_internal.h |  31 ++++-
>   lib/librte_telemetry/rte_telemetry_parser.c   | 130 +++++++++++++++---
>   usertools/dpdk-telemetry-client.py            |  13 +-
>   6 files changed, 253 insertions(+), 54 deletions(-)

Looks good to me, thanks!


Acked-by: Kevin Laatz <kevin.laatz@intel.com>

Re: [dpdk-dev] [PATCH] lib/telemetry: add support to fetch global metrics

[Thomas Monjalon]

17/05/2019 19:07, Reshma Pattan:
> telemetry has support for fetching port based stats
> from metrics library.
> 
> Metrics library also has global stats which are
> not fetched by telemetry, so extend telemetry to
> fetch the global metrics.
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---

I see some errors with GCC:

rte_telemetry_parser.c:362:27: error: unused variable ‘p’

rte_telemetry.c:550:11: error: unused variable ‘i’

rte_telemetry.c:613:16: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’

Re: [dpdk-dev] [PATCH] lib/telemetry: add support to fetch global metrics

[Pattan, Reshma]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]

<snip>

> 
> I see some errors with GCC:
> 
> rte_telemetry_parser.c:362:27: error: unused variable ‘p’
> 
> rte_telemetry.c:550:11: error: unused variable ‘i’
> 
> rte_telemetry.c:613:16: error: comparison of integer expressions of different
> signedness: ‘int’ and ‘uint32_t’

Hmm, yes these have to be fixed, strange they were not caught on my board with  gcc (GCC) 9.0.1 20190312 (Red Hat 9.0.1-0.10)

Will fix and send v2.

Thanks,
Reshma

[dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global metrics

[Reshma Pattan]

telemetry has support for fetching port based stats
from metrics library.

Metrics library also has global stats which are
not fetched by telemetry, so extend telemetry to
fetch the global metrics.

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
Acked-by: Kevin Laatz <kevin.laatz@intel.com>
---
v2: fix GCC compilation issues.
rebase with latest release notes.
---
 doc/guides/howto/telemetry.rst                |   9 +-
 doc/guides/rel_notes/release_19_08.rst        |   5 +
 lib/librte_telemetry/rte_telemetry.c          | 122 +++++++++++-----
 lib/librte_telemetry/rte_telemetry_internal.h |  31 ++++-
 lib/librte_telemetry/rte_telemetry_parser.c   | 130 +++++++++++++++---
 usertools/dpdk-telemetry-client.py            |  13 +-
 6 files changed, 255 insertions(+), 55 deletions(-)

diff --git a/doc/guides/howto/telemetry.rst b/doc/guides/howto/telemetry.rst
index 00f8f7a85..cacc08216 100644
--- a/doc/guides/howto/telemetry.rst
+++ b/doc/guides/howto/telemetry.rst
@@ -11,9 +11,10 @@ Introduction
 ------------
 
 The ``librte_telemetry`` provides the functionality so that users may query
-metrics from incoming port traffic. The application which initializes packet
-forwarding will act as the server, sending metrics to the requesting application
-which acts as the client.
+metrics from incoming port traffic and global stats(application stats).
+The application which initializes packet forwarding will act as the server,
+sending metrics to the requesting application which acts as the client.
+
 
 In DPDK, applications are used to initialize the ``telemetry``. To view incoming
 traffic on featured ports, the application should be run first (ie. after ports
@@ -79,7 +80,7 @@ any DPDK application is applicable.
    the menu.
 
 #. Send traffic to any or all available ports from a traffic generator.
-   Select a query option(recursive or singular polling).
+   Select a query option(recursive or singular polling or global stats).
    The metrics will then be displayed on the client terminal in JSON format.
 
 #. Once finished, unregister the client using the menu command.
diff --git a/doc/guides/rel_notes/release_19_08.rst b/doc/guides/rel_notes/release_19_08.rst
index 8c3932d06..083a738e6 100644
--- a/doc/guides/rel_notes/release_19_08.rst
+++ b/doc/guides/rel_notes/release_19_08.rst
@@ -88,6 +88,11 @@ New Features
   * Added multi-queue support to allow one af_xdp vdev with multiple netdev
     queues
 
+* **Updated telemetry library for global metrics support.**
+
+  Updated ``librte_telemetry`` to fetch the global metrics from the
+  ``librte_metrics`` library.
+
 
 Removed Items
 -------------
diff --git a/lib/librte_telemetry/rte_telemetry.c b/lib/librte_telemetry/rte_telemetry.c
index b852630c5..a4b82c97f 100644
--- a/lib/librte_telemetry/rte_telemetry.c
+++ b/lib/librte_telemetry/rte_telemetry.c
@@ -449,17 +449,14 @@ rte_telemetry_json_format_port(struct telemetry_impl *telemetry,
 
 static int32_t
 rte_telemetry_encode_json_format(struct telemetry_impl *telemetry,
-	uint32_t *port_ids, uint32_t num_port_ids, uint32_t *metric_ids,
-	uint32_t num_metric_ids, char **json_buffer)
+	struct telemetry_encode_param *ep, char **json_buffer)
 {
 	int ret;
 	json_t *root, *ports;
 	uint32_t i;
-
-	if (num_port_ids <= 0 || num_metric_ids <= 0) {
-		TELEMETRY_LOG_ERR("Please provide port and metric ids to query");
-		goto einval_fail;
-	}
+	uint32_t port_id;
+	uint32_t num_port_ids;
+	uint32_t num_metric_ids;
 
 	ports = json_array();
 	if (ports == NULL) {
@@ -467,20 +464,47 @@ rte_telemetry_encode_json_format(struct telemetry_impl *telemetry,
 		goto eperm_fail;
 	}
 
-	for (i = 0; i < num_port_ids; i++) {
-		if (!rte_eth_dev_is_valid_port(port_ids[i])) {
-			TELEMETRY_LOG_ERR("Port: %d invalid", port_ids[i]);
+	if (ep->type == PORT_STATS) {
+		num_port_ids = ep->pp.num_port_ids;
+		num_metric_ids = ep->pp.num_metric_ids;
+
+		if (num_port_ids <= 0 || num_metric_ids <= 0) {
+			TELEMETRY_LOG_ERR("Please provide port and metric ids to query");
 			goto einval_fail;
 		}
-	}
 
-	for (i = 0; i < num_port_ids; i++) {
-		ret = rte_telemetry_json_format_port(telemetry, port_ids[i],
-			ports, metric_ids, num_metric_ids);
+		for (i = 0; i < num_port_ids; i++) {
+			port_id = ep->pp.port_ids[i];
+			if (!rte_eth_dev_is_valid_port(port_id)) {
+				TELEMETRY_LOG_ERR("Port: %d invalid",
+							port_id);
+				goto einval_fail;
+			}
+		}
+
+		for (i = 0; i < num_port_ids; i++) {
+			port_id = ep->pp.port_ids[i];
+			ret = rte_telemetry_json_format_port(telemetry,
+					port_id, ports, &ep->pp.metric_ids[0],
+					num_metric_ids);
+			if (ret < 0) {
+				TELEMETRY_LOG_ERR("Format port in JSON failed");
+				return -1;
+			}
+		}
+	} else if (ep->type == GLOBAL_STATS) {
+		/* Request Global Metrics */
+		ret = rte_telemetry_json_format_port(telemetry,
+				RTE_METRICS_GLOBAL,
+				ports, &ep->gp.metric_ids[0],
+				ep->gp.num_metric_ids);
 		if (ret < 0) {
-			TELEMETRY_LOG_ERR("Format port in JSON failed");
+			TELEMETRY_LOG_ERR(" Request Global Metrics Failed");
 			return -1;
 		}
+	} else {
+		TELEMETRY_LOG_ERR(" Invalid metrics type in encode params");
+		goto einval_fail;
 	}
 
 	root = json_object();
@@ -520,10 +544,10 @@ rte_telemetry_encode_json_format(struct telemetry_impl *telemetry,
 }
 
 int32_t
-rte_telemetry_send_ports_stats_values(uint32_t *metric_ids, int num_metric_ids,
-	uint32_t *port_ids, int num_port_ids, struct telemetry_impl *telemetry)
+rte_telemetry_send_global_stats_values(struct telemetry_encode_param *ep,
+	struct telemetry_impl *telemetry)
 {
-	int ret, i;
+	int ret;
 	char *json_buffer = NULL;
 
 	if (telemetry == NULL) {
@@ -531,42 +555,78 @@ rte_telemetry_send_ports_stats_values(uint32_t *metric_ids, int num_metric_ids,
 		return -1;
 	}
 
-	if (metric_ids == NULL) {
-		TELEMETRY_LOG_ERR("Invalid metric_ids array");
+	if (ep->gp.num_metric_ids < 0) {
+		TELEMETRY_LOG_ERR("Invalid num_metric_ids, must be positive");
 		goto einval_fail;
 	}
 
-	if (num_metric_ids < 0) {
-		TELEMETRY_LOG_ERR("Invalid num_metric_ids, must be positive");
+	ret = rte_telemetry_encode_json_format(telemetry, ep,
+		&json_buffer);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("JSON encode function failed");
+		return -1;
+	}
+
+	ret = rte_telemetry_write_to_socket(telemetry, json_buffer);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Could not write to socket");
+		return -1;
+	}
+
+	return 0;
+
+einval_fail:
+	ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+	if (ret < 0)
+		TELEMETRY_LOG_ERR("Could not send error");
+	return -1;
+}
+
+int32_t
+rte_telemetry_send_ports_stats_values(struct telemetry_encode_param *ep,
+	struct telemetry_impl *telemetry)
+{
+	int ret;
+	char *json_buffer = NULL;
+	uint32_t port_id;
+	unsigned int i;
+
+	if (telemetry == NULL) {
+		TELEMETRY_LOG_ERR("Invalid telemetry argument");
+		return -1;
+	}
+
+	if (ep == NULL) {
+		TELEMETRY_LOG_ERR("Invalid encode param argument");
 		goto einval_fail;
 	}
 
-	if (port_ids == NULL) {
-		TELEMETRY_LOG_ERR("Invalid port_ids array");
+	if (ep->pp.num_metric_ids < 0) {
+		TELEMETRY_LOG_ERR("Invalid num_metric_ids, must be positive");
 		goto einval_fail;
 	}
 
-	if (num_port_ids < 0) {
+	if (ep->pp.num_port_ids < 0) {
 		TELEMETRY_LOG_ERR("Invalid num_port_ids, must be positive");
 		goto einval_fail;
 	}
 
-	for (i = 0; i < num_port_ids; i++) {
-		if (!rte_eth_dev_is_valid_port(port_ids[i])) {
-			TELEMETRY_LOG_ERR("Port: %d invalid", port_ids[i]);
+	for (i = 0; i < ep->pp.num_port_ids; i++) {
+		port_id = ep->pp.port_ids[i];
+		if (!rte_eth_dev_is_valid_port(port_id)) {
+			TELEMETRY_LOG_ERR("Port: %d invalid", port_id);
 			goto einval_fail;
 		}
 
 		ret = rte_telemetry_update_metrics_ethdev(telemetry,
-				port_ids[i], telemetry->reg_index[i]);
+				port_id, telemetry->reg_index[i]);
 		if (ret < 0) {
 			TELEMETRY_LOG_ERR("Failed to update ethdev metrics");
 			return -1;
 		}
 	}
 
-	ret = rte_telemetry_encode_json_format(telemetry, port_ids,
-		num_port_ids, metric_ids, num_metric_ids, &json_buffer);
+	ret = rte_telemetry_encode_json_format(telemetry, ep, &json_buffer);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("JSON encode function failed");
 		return -1;
diff --git a/lib/librte_telemetry/rte_telemetry_internal.h b/lib/librte_telemetry/rte_telemetry_internal.h
index c298c3919..6a91d0c44 100644
--- a/lib/librte_telemetry/rte_telemetry_internal.h
+++ b/lib/librte_telemetry/rte_telemetry_internal.h
@@ -24,6 +24,8 @@ extern int telemetry_log_level;
 #define TELEMETRY_LOG_INFO(fmt, args...) \
 	TELEMETRY_LOG(INFO, fmt, ## args)
 
+#define MAX_METRICS 256
+
 typedef struct telemetry_client {
 	char *file_path;
 	int fd;
@@ -48,6 +50,28 @@ enum rte_telemetry_parser_actions {
 	ACTION_DELETE = 2
 };
 
+enum rte_telemetry_stats_type {
+	PORT_STATS = 0,
+	GLOBAL_STATS = 1
+};
+
+/* @internal */
+struct telemetry_encode_param {
+	enum rte_telemetry_stats_type type;
+	union {
+		struct port_param {
+			uint32_t num_metric_ids;
+			uint32_t metric_ids[MAX_METRICS];
+			uint32_t num_port_ids;
+			uint32_t port_ids[RTE_MAX_ETHPORTS];
+		} pp;
+		struct global_param {
+			uint32_t num_metric_ids;
+			uint32_t metric_ids[MAX_METRICS];
+		} gp;
+	};
+};
+
 int32_t
 rte_telemetry_parse_client_message(struct telemetry_impl *telemetry, char *buf);
 
@@ -72,10 +96,13 @@ int32_t
 rte_telemetry_is_port_active(int port_id);
 
 int32_t
-rte_telemetry_send_ports_stats_values(uint32_t *metric_ids, int num_metric_ids,
-	uint32_t *port_ids, int num_port_ids, struct telemetry_impl *telemetry);
+rte_telemetry_send_ports_stats_values(struct telemetry_encode_param *ep,
+	struct telemetry_impl *telemetry);
 
 int32_t
 rte_telemetry_socket_messaging_testing(int index, int socket);
 
+int32_t
+rte_telemetry_send_global_stats_values(struct telemetry_encode_param *ep,
+	struct telemetry_impl *telemetry);
 #endif
diff --git a/lib/librte_telemetry/rte_telemetry_parser.c b/lib/librte_telemetry/rte_telemetry_parser.c
index 9bc16eef4..32e76a487 100644
--- a/lib/librte_telemetry/rte_telemetry_parser.c
+++ b/lib/librte_telemetry/rte_telemetry_parser.c
@@ -258,8 +258,9 @@ rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
 	int ret, num_metrics, i, p;
 	struct rte_metric_value *values;
 	uint64_t num_port_ids = 0;
-	uint32_t port_ids[RTE_MAX_ETHPORTS];
+	struct telemetry_encode_param ep;
 
+	memset(&ep, 0, sizeof(ep));
 	if (telemetry == NULL) {
 		TELEMETRY_LOG_ERR("Invalid telemetry argument");
 		return -1;
@@ -310,10 +311,8 @@ rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
 		return -1;
 	}
 
-	uint32_t stat_ids[num_metrics];
-
 	RTE_ETH_FOREACH_DEV(p) {
-		port_ids[num_port_ids] = p;
+		ep.pp.port_ids[num_port_ids] = p;
 		num_port_ids++;
 	}
 
@@ -327,16 +326,22 @@ rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
 		goto fail;
 	}
 
-	ret = rte_metrics_get_values(port_ids[0], values, num_metrics);
+	ret = rte_metrics_get_values(ep.pp.port_ids[0], values, num_metrics);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("Could not get stat values");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
 		goto fail;
 	}
 	for (i = 0; i < num_metrics; i++)
-		stat_ids[i] = values[i].key;
+		ep.pp.metric_ids[i] = values[i].key;
+
+	ep.pp.num_port_ids = num_port_ids;
+	ep.pp.num_metric_ids = num_metrics;
+	ep.type = PORT_STATS;
 
-	ret = rte_telemetry_send_ports_stats_values(stat_ids, num_metrics,
-		port_ids, num_port_ids, telemetry);
+	ret = rte_telemetry_send_ports_stats_values(&ep, telemetry);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("Sending ports stats values failed");
 		goto fail;
@@ -349,6 +354,93 @@ rte_telemetry_command_ports_all_stat_values(struct telemetry_impl *telemetry,
 	return -1;
 }
 
+int32_t
+rte_telemetry_command_global_stat_values(struct telemetry_impl *telemetry,
+	 int action, json_t *data)
+{
+	int ret, num_metrics, i;
+	struct rte_metric_value *values;
+	struct telemetry_encode_param ep;
+
+	memset(&ep, 0, sizeof(ep));
+	if (telemetry == NULL) {
+		TELEMETRY_LOG_ERR("Invalid telemetry argument");
+		return -1;
+	}
+
+	if (action != ACTION_GET) {
+		TELEMETRY_LOG_WARN("Invalid action for this command");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+		return -1;
+	}
+
+	if (json_is_object(data)) {
+		TELEMETRY_LOG_WARN("Invalid data provided for this command");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+		return -1;
+	}
+
+	num_metrics = rte_metrics_get_values(RTE_METRICS_GLOBAL, NULL, 0);
+	if (num_metrics < 0) {
+		TELEMETRY_LOG_ERR("Cannot get metrics count");
+
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+
+		return -1;
+	} else if (num_metrics == 0) {
+		TELEMETRY_LOG_ERR("No metrics to display (none have been registered)");
+
+		ret = rte_telemetry_send_error_response(telemetry, -EPERM);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+
+		return -1;
+	}
+
+	values = malloc(sizeof(struct rte_metric_value) * num_metrics);
+	if (values == NULL) {
+		TELEMETRY_LOG_ERR("Cannot allocate memory");
+		ret = rte_telemetry_send_error_response(telemetry,
+			 -ENOMEM);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+		return -1;
+	}
+
+	ret = rte_metrics_get_values(RTE_METRICS_GLOBAL, values, num_metrics);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Could not get stat values");
+		ret = rte_telemetry_send_error_response(telemetry, -EINVAL);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Could not send error");
+		goto fail;
+	}
+	for (i = 0; i < num_metrics; i++)
+		ep.gp.metric_ids[i] = values[i].key;
+
+	ep.gp.num_metric_ids = num_metrics;
+	ep.type = GLOBAL_STATS;
+
+	ret = rte_telemetry_send_global_stats_values(&ep, telemetry);
+	if (ret < 0) {
+		TELEMETRY_LOG_ERR("Sending global stats values failed");
+		goto fail;
+	}
+
+	free(values);
+	return 0;
+
+fail:
+	free(values);
+	return -1;
+}
+
 int32_t
 rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
 	*telemetry, int action, json_t *data)
@@ -356,13 +448,15 @@ rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
 	int ret;
 	json_t *port_ids_json = json_object_get(data, "ports");
 	json_t *stat_names_json = json_object_get(data, "stats");
-	uint64_t num_port_ids = json_array_size(port_ids_json);
 	uint64_t num_stat_names = json_array_size(stat_names_json);
 	const char *stat_names[num_stat_names];
-	uint32_t port_ids[num_port_ids], stat_ids[num_stat_names];
+	struct telemetry_encode_param ep;
 	size_t index;
 	json_t *value;
 
+	ep.pp.num_port_ids = json_array_size(port_ids_json);
+	ep.pp.num_metric_ids = num_stat_names;
+	memset(&ep, 0, sizeof(ep));
 	if (telemetry == NULL) {
 		TELEMETRY_LOG_ERR("Invalid telemetry argument");
 		return -1;
@@ -402,8 +496,8 @@ rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
 				TELEMETRY_LOG_ERR("Could not send error");
 			return -1;
 		}
-		port_ids[index] = json_integer_value(value);
-		ret = rte_telemetry_is_port_active(port_ids[index]);
+		ep.pp.port_ids[index] = json_integer_value(value);
+		ret = rte_telemetry_is_port_active(ep.pp.port_ids[index]);
 		if (ret < 1) {
 			ret = rte_telemetry_send_error_response(telemetry,
 				-EINVAL);
@@ -427,15 +521,15 @@ rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
 		stat_names[index] = json_string_value(value);
 	}
 
-	ret = rte_telemetry_stat_names_to_ids(telemetry, stat_names, stat_ids,
-		num_stat_names);
+	ret = rte_telemetry_stat_names_to_ids(telemetry, stat_names,
+		ep.pp.metric_ids, num_stat_names);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("Could not convert stat names to IDs");
 		return -1;
 	}
 
-	ret = rte_telemetry_send_ports_stats_values(stat_ids, num_stat_names,
-		port_ids, num_port_ids, telemetry);
+	ep.type = PORT_STATS;
+	ret = rte_telemetry_send_ports_stats_values(&ep, telemetry);
 	if (ret < 0) {
 		TELEMETRY_LOG_ERR("Sending ports stats values failed");
 		return -1;
@@ -480,6 +574,10 @@ rte_telemetry_parse_command(struct telemetry_impl *telemetry, int action,
 		{
 			.text = "ports_all_stat_values",
 			.fn = &rte_telemetry_command_ports_all_stat_values
+		},
+		{
+			.text = "global_stat_values",
+			.fn = &rte_telemetry_command_global_stat_values
 		}
 	};
 
diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
index ce0c7a905..572ff56bb 100755
--- a/usertools/dpdk-telemetry-client.py
+++ b/usertools/dpdk-telemetry-client.py
@@ -12,6 +12,7 @@
 METRICS_REQ = "{\"action\":0,\"command\":\"ports_all_stat_values\",\"data\":null}"
 API_REG = "{\"action\":1,\"command\":\"clients\",\"data\":{\"client_path\":\""
 API_UNREG = "{\"action\":2,\"command\":\"clients\",\"data\":{\"client_path\":\""
+GLOBAL_METRICS_REQ = "{\"action\":0,\"command\":\"global_stat_values\",\"data\":null}"
 DEFAULT_FP = "/var/run/dpdk/default_client"
 
 class Socket:
@@ -79,12 +80,18 @@ def repeatedlyRequestMetrics(self, sleep_time): # Recursively requests metrics f
             self.requestMetrics()
             time.sleep(sleep_time)
 
+    def requestGlobalMetrics(self): #Requests global metrics for given client
+        self.socket.client_fd.send(GLOBAL_METRICS_REQ)
+        data = self.socket.client_fd.recv(BUFFER_SIZE)
+        print "\nResponse: \n", str(data)
+
     def interactiveMenu(self, sleep_time): # Creates Interactive menu within the script
-        while self.choice != 3:
+        while self.choice != 4:
             print("\nOptions Menu")
             print("[1] Send for Metrics for all ports")
             print("[2] Send for Metrics for all ports recursively")
-            print("[3] Unregister client")
+            print("[3] Send for global Metrics")
+            print("[4] Unregister client")
 
             try:
                 self.choice = int(input("\n:"))
@@ -95,6 +102,8 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
                 elif self.choice == 2:
                     self.repeatedlyRequestMetrics(sleep_time)
                 elif self.choice == 3:
+                    self.requestGlobalMetrics()
+                elif self.choice == 4:
                     self.unregister()
                     self.unregistered = 1
                 else:
-- 
2.21.0

Re: [dpdk-dev] [PATCH] lib/telemetry: add support to fetch global metrics

[Pattan, Reshma]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]

<snip>

> 
> I see some errors with GCC:

On which GCC version the errors are shown?  

> 
> rte_telemetry_parser.c:362:27: error: unused variable ‘p’
> 
> rte_telemetry.c:550:11: error: unused variable ‘i’
> 
> rte_telemetry.c:613:16: error: comparison of integer expressions of different
> signedness: ‘int’ and ‘uint32_t’

Re: [dpdk-dev] [PATCH] lib/telemetry: add support to fetch global metrics

[Thomas Monjalon]

18/06/2019 18:55, Pattan, Reshma:
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> 
> > I see some errors with GCC:
> 
> On which GCC version the errors are shown?

GCC 8.3

Given the type of the errors I think it should be visible
with older versions.

> > rte_telemetry_parser.c:362:27: error: unused variable ‘p’
> > 
> > rte_telemetry.c:550:11: error: unused variable ‘i’
> > 
> > rte_telemetry.c:613:16: error: comparison of integer expressions of different
> > signedness: ‘int’ and ‘uint32_t’

Re: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global metrics

[Ferruh Yigit]

On 6/18/2019 2:49 PM, Reshma Pattan wrote:
> telemetry has support for fetching port based stats
> from metrics library.
> 
> Metrics library also has global stats which are
> not fetched by telemetry, so extend telemetry to
> fetch the global metrics.
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> Acked-by: Kevin Laatz <kevin.laatz@intel.com>

Hi Reshma, Kevin,

Not related to this patch, but when telemetry library enabled, it is causing
some warnings [1] with gcc9

The [-Waddress-of-packed-member] warnings already disable for gcc, but you need
following update to "lib/librte_telemetry/Makefile":
 -CFLAGS += -I$(SRCDIR)
 +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR)

Can you please take care of this?



[1]
In file included from .../dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:160,
                 from .../dpdk/lib/librte_telemetry/rte_telemetry_parser.c:13:
.../dpdk/x86_64-native-linuxapp-gcc/include/rte_ether.h: In function
‘rte_is_broadcast_ether_addr’:
.../dpdk/x86_64-native-linuxapp-gcc/include/rte_ether.h:152:2: warning:
converting a packed ‘const struct rte_ether_addr’ pointer (alignment 1) to a
‘unaligned_uint16_t’ {aka ‘const short unsigned int’} pointer (alignment 2) may
result in an unaligned pointer value [-Waddress-of-packed-member]
  152 |  const unaligned_uint16_t *ea_words = (const unaligned_uint16_t *)ea;
      |  ^~~~~

Re: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global metrics

[Laatz, Kevin]

> Hi Reshma, Kevin,
> 
> Not related to this patch, but when telemetry library enabled, it is causing
> some warnings [1] with gcc9
> 
> The [-Waddress-of-packed-member] warnings already disable for gcc, but
> you need following update to "lib/librte_telemetry/Makefile":
>  -CFLAGS += -I$(SRCDIR)
>  +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR)
> 
> Can you please take care of this?
> 

Sure, I'll look into it. Thanks.

> 
> 
> [1]
> In file included from .../dpdk/x86_64-native-linuxapp-
> gcc/include/rte_ethdev.h:160,
>                  from .../dpdk/lib/librte_telemetry/rte_telemetry_parser.c:13:
> .../dpdk/x86_64-native-linuxapp-gcc/include/rte_ether.h: In function
> ‘rte_is_broadcast_ether_addr’:
> .../dpdk/x86_64-native-linuxapp-gcc/include/rte_ether.h:152:2: warning:
> converting a packed ‘const struct rte_ether_addr’ pointer (alignment 1) to a
> ‘unaligned_uint16_t’ {aka ‘const short unsigned int’} pointer (alignment 2)
> may result in an unaligned pointer value [-Waddress-of-packed-member]
>   152 |  const unaligned_uint16_t *ea_words = (const unaligned_uint16_t
> *)ea;
>       |  ^~~~~

Re: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global metrics

[Thomas Monjalon]

18/06/2019 15:49, Reshma Pattan:
> telemetry has support for fetching port based stats
> from metrics library.
> 
> Metrics library also has global stats which are
> not fetched by telemetry, so extend telemetry to
> fetch the global metrics.
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> Acked-by: Kevin Laatz <kevin.laatz@intel.com>
> ---
> v2: fix GCC compilation issues.
> rebase with latest release notes.

Applied, thanks

Re: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global metrics

[Chautru, Nicolas]

Hi Reshma, 
I am still seeing build issue when rebasing to this : ie. checking uint32_t is negative in rte_telemetry.c:558
Thanks
Nic

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
Sent: Monday, June 24, 2019 7:54 AM
To: Pattan, Reshma <reshma.pattan@intel.com>
Cc: dev@dpdk.org; Laatz, Kevin <kevin.laatz@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global metrics

18/06/2019 15:49, Reshma Pattan:
> telemetry has support for fetching port based stats from metrics 
> library.
> 
> Metrics library also has global stats which are not fetched by 
> telemetry, so extend telemetry to fetch the global metrics.
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> Acked-by: Kevin Laatz <kevin.laatz@intel.com>
> ---
> v2: fix GCC compilation issues.
> rebase with latest release notes.

Applied, thanks

Re: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global metrics

[Pattan, Reshma]

> -----Original Message-----
> From: Chautru, Nicolas
> Sent: Monday, June 24, 2019 6:37 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Pattan, Reshma
> <reshma.pattan@intel.com>
> Cc: dev@dpdk.org; Laatz, Kevin <kevin.laatz@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global
> metrics
> 
> Hi Reshma,
> I am still seeing build issue when rebasing to this : ie. checking uint32_t is
> negative in rte_telemetry.c:558 Thanks Nic
> 

Which compiler  and version , can you give the more details.
<snip>

Re: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global metrics

[Pattan, Reshma]

> -----Original Message-----
> From: Pattan, Reshma
> Sent: Tuesday, June 25, 2019 9:39 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: dev@dpdk.org; Laatz, Kevin <kevin.laatz@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch global
> metrics
> 
> 
> 
> > -----Original Message-----
> > From: Chautru, Nicolas
> > Sent: Monday, June 24, 2019 6:37 PM
> > To: Thomas Monjalon <thomas@monjalon.net>; Pattan, Reshma
> > <reshma.pattan@intel.com>
> > Cc: dev@dpdk.org; Laatz, Kevin <kevin.laatz@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2] lib/telemetry: add support to fetch
> > global metrics
> >
> > Hi Reshma,
> > I am still seeing build issue when rebasing to this : ie. checking
> > uint32_t is negative in rte_telemetry.c:558 Thanks Nic
> >
> 
> Which compiler  and version , can you give the more details.
> <snip>

Hi,

WERROR_FLAGS is missing in the make file so we missed to see these errors, me and Kevin are looking into this. Kevin will send the patch.

Thanks,
Reshma