[dpdk-dev] [PATCH 0/4] telemetry logging improvements and cleanup

[dpdk-dev] [PATCH 0/4] telemetry logging improvements and cleanup

[Bruce Richardson]

This set adds support for using the regular rte_log functions from the telemetry
library; avoiding circular dependencies by having EAL register the telemetry
library itself and then passing the required handles to that library as part of
the telemetry_init call.

Beyond this change, the other three patches are cleanups to ensure that all
internal functions are clearly separate from the public APIs. (Patches 3 & 4 may
be merged into a single one on apply, for I've kept them separate for now for
clarity).

Bruce Richardson (4):
  telemetry: use rte_log for logging
  telemetry: make the legacy registration function internal
  telemetry: create internal-only header file
  telemetry: move init function to internal header

 doc/guides/rel_notes/release_21_05.rst        |  5 ++
 lib/librte_eal/freebsd/eal.c                  | 12 +--
 lib/librte_eal/linux/eal.c                    | 12 +--
 lib/librte_metrics/rte_metrics_telemetry.c    |  2 +-
 lib/librte_telemetry/rte_telemetry.h          | 22 ------
 lib/librte_telemetry/telemetry.c              | 74 +++++++++----------
 ...elemetry_legacy.h => telemetry_internal.h} | 37 +++++++++-
 lib/librte_telemetry/telemetry_legacy.c       |  2 +-
 lib/librte_telemetry/version.map              |  2 +-
 9 files changed, 82 insertions(+), 86 deletions(-)
 rename lib/librte_telemetry/{rte_telemetry_legacy.h => telemetry_internal.h} (67%)

--
2.27.0

[dpdk-dev] [PATCH 1/4] telemetry: use rte_log for logging

[Bruce Richardson]

Rather than passing back an error string to the caller, take as input the
rte_log function to use, and just use regular logging.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/freebsd/eal.c         | 10 ++--
 lib/librte_eal/linux/eal.c           | 10 ++--
 lib/librte_telemetry/rte_telemetry.h | 15 ++++--
 lib/librte_telemetry/telemetry.c     | 72 +++++++++++++---------------
 4 files changed, 49 insertions(+), 58 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index e2cdad5283..b8a798bc7d 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -941,15 +941,11 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 	if (!internal_conf->no_telemetry) {
-		const char *error_str = NULL;
+		uint32_t tlog = rte_log_register_type_and_pick_level(
+				"lib.telemetry", RTE_LOG_WARNING);
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
-				&internal_conf->ctrl_cpuset, &error_str)
-				!= 0) {
-			rte_eal_init_alert(error_str);
+				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
 			return -1;
-		}
-		if (error_str != NULL)
-			RTE_LOG(NOTICE, EAL, "%s\n", error_str);
 	}

 	eal_mcfg_complete();
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 6c34ac8903..23e3a32036 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1314,15 +1314,11 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 	if (!internal_conf->no_telemetry) {
-		const char *error_str = NULL;
+		uint32_t tlog = rte_log_register_type_and_pick_level(
+				"lib.telemetry", RTE_LOG_WARNING);
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
-				&internal_conf->ctrl_cpuset, &error_str)
-				!= 0) {
-			rte_eal_init_alert(error_str);
+				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
 			return -1;
-		}
-		if (error_str != NULL)
-			RTE_LOG(NOTICE, EAL, "%s\n", error_str);
 	}

 	eal_mcfg_complete();
diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
index f8e54dc68e..2c3da3ab7f 100644
--- a/lib/librte_telemetry/rte_telemetry.h
+++ b/lib/librte_telemetry/rte_telemetry.h
@@ -292,6 +292,12 @@ __rte_experimental
 int
 rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);

+/**
+ * @internal
+ * Log function type, to allow passing as parameter if necessary
+ */
+typedef int (*rte_log_fn)(uint32_t level, uint32_t logtype, const char *format, ...);
+
 /**
  * @internal
  * Initialize Telemetry.
@@ -300,9 +306,10 @@ rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);
  * The runtime directory of DPDK.
  * @param cpuset
  * The CPU set to be used for setting the thread affinity.
- * @param err_str
- * This err_str pointer should point to NULL on entry. In the case of an error
- * or warning, it will be non-NULL on exit.
+ * @param log_fn
+ * Function pointer to the rte_log function for logging use
+ * @param registered_logtype
+ * The registered log type to use for logging
  *
  * @return
  *  0 on success.
@@ -312,7 +319,7 @@ rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);
 __rte_internal
 int
 rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
-		const char **err_str);
+		rte_log_fn log_fn, uint32_t registered_logtype);

 /**
  * Get a pointer to a container with memory allocated. The container is to be
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index b142729da4..18f2ae2e2f 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -16,6 +16,7 @@
 #include <rte_common.h>
 #include <rte_spinlock.h>
 #include <rte_version.h>
+#include <rte_log.h>

 #include "rte_telemetry.h"
 #include "telemetry_json.h"
@@ -48,7 +49,15 @@ struct socket {
 static struct socket v2_socket; /* socket for v2 telemetry */
 static struct socket v1_socket; /* socket for v1 telemetry */
 #endif /* !RTE_EXEC_ENV_WINDOWS */
-static char telemetry_log_error[1024]; /* Will contain error on init failure */
+
+static const char *socket_dir;
+static rte_cpuset_t *thread_cpuset;
+static rte_log_fn rte_log_ptr;
+static uint32_t logtype;
+
+#define TMTY_LOG(l, ...) \
+	 rte_log_ptr(RTE_LOG_ ## l, logtype, "TELEMETRY: " __VA_ARGS__)
+
 /* list of command callbacks, with one command registered by default */
 static struct cmd_callback callbacks[TELEMETRY_MAX_CALLBACKS];
 static int num_callbacks; /* How many commands are registered */
@@ -344,9 +353,7 @@ socket_listener(void *socket)
 		struct socket *s = (struct socket *)socket;
 		int s_accepted = accept(s->sock, NULL, NULL);
 		if (s_accepted < 0) {
-			snprintf(telemetry_log_error,
-				sizeof(telemetry_log_error),
-				"Error with accept, telemetry thread quitting");
+			TMTY_LOG(ERR, "Error with accept, telemetry thread quitting\n");
 			return NULL;
 		}
 		if (s->num_clients != NULL) {
@@ -388,9 +395,7 @@ create_socket(char *path)
 {
 	int sock = socket(AF_UNIX, SOCK_SEQPACKET, 0);
 	if (sock < 0) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error with socket creation, %s",
-				strerror(errno));
+		TMTY_LOG(ERR, "Error with socket creation, %s\n", strerror(errno));
 		return -1;
 	}

@@ -398,17 +403,13 @@ create_socket(char *path)
 	strlcpy(sun.sun_path, path, sizeof(sun.sun_path));
 	unlink(sun.sun_path);
 	if (bind(sock, (void *) &sun, sizeof(sun)) < 0) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error binding socket: %s",
-				strerror(errno));
+		TMTY_LOG(ERR, "Error binding socket: %s\n", strerror(errno));
 		sun.sun_path[0] = 0;
 		goto error;
 	}

 	if (listen(sock, 1) < 0) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error calling listen for socket: %s",
-				strerror(errno));
+		TMTY_LOG(ERR, "Error calling listen for socket: %s\n", strerror(errno));
 		goto error;
 	}

@@ -421,35 +422,33 @@ create_socket(char *path)
 }

 static int
-telemetry_legacy_init(const char *runtime_dir, rte_cpuset_t *cpuset)
+telemetry_legacy_init(void)
 {
 	pthread_t t_old;

 	if (num_legacy_callbacks == 1) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-			 "No legacy callbacks, legacy socket not created");
+		TMTY_LOG(WARNING, "No legacy callbacks, legacy socket not created\n");
 		return -1;
 	}

 	v1_socket.fn = legacy_client_handler;
 	if ((size_t) snprintf(v1_socket.path, sizeof(v1_socket.path),
-			"%s/telemetry", runtime_dir)
-			>= sizeof(v1_socket.path)) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error with socket binding, path too long");
+			"%s/telemetry", socket_dir) >= sizeof(v1_socket.path)) {
+		TMTY_LOG(ERR, "Error with socket binding, path too long\n");
 		return -1;
 	}
 	v1_socket.sock = create_socket(v1_socket.path);
 	if (v1_socket.sock < 0)
 		return -1;
 	pthread_create(&t_old, NULL, socket_listener, &v1_socket);
-	pthread_setaffinity_np(t_old, sizeof(*cpuset), cpuset);
+	pthread_setaffinity_np(t_old, sizeof(*thread_cpuset), thread_cpuset);

+	TMTY_LOG(DEBUG, "Legacy telemetry socket initialized ok\n");
 	return 0;
 }

 static int
-telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
+telemetry_v2_init(void)
 {
 	pthread_t t_new;

@@ -461,10 +460,9 @@ telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
 	rte_telemetry_register_cmd("/help", command_help,
 			"Returns help text for a command. Parameters: string command");
 	v2_socket.fn = client_handler;
-	if (strlcpy(v2_socket.path, get_socket_path(runtime_dir, 2),
+	if (strlcpy(v2_socket.path, get_socket_path(socket_dir, 2),
 			sizeof(v2_socket.path)) >= sizeof(v2_socket.path)) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error with socket binding, path too long");
+		TMTY_LOG(ERR, "Error with socket binding, path too long\n");
 		return -1;
 	}

@@ -472,7 +470,7 @@ telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
 	if (v2_socket.sock < 0)
 		return -1;
 	pthread_create(&t_new, NULL, socket_listener, &v2_socket);
-	pthread_setaffinity_np(t_new, sizeof(*cpuset), cpuset);
+	pthread_setaffinity_np(t_new, sizeof(*thread_cpuset), thread_cpuset);
 	atexit(unlink_sockets);

 	return 0;
@@ -482,23 +480,17 @@ telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)

 int32_t
 rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
-		const char **err_str)
+		rte_log_fn log_fn, uint32_t registered_logtype)
 {
+	socket_dir = runtime_dir;
+	thread_cpuset = cpuset;
+	rte_log_ptr = log_fn;
+	logtype = registered_logtype;
 #ifndef RTE_EXEC_ENV_WINDOWS
-	if (telemetry_v2_init(runtime_dir, cpuset) != 0) {
-		*err_str = telemetry_log_error;
+	if (telemetry_v2_init() != 0)
 		return -1;
-	}
-	if (telemetry_legacy_init(runtime_dir, cpuset) != 0) {
-		*err_str = telemetry_log_error;
-	}
-#else /* RTE_EXEC_ENV_WINDOWS */
-	RTE_SET_USED(runtime_dir);
-	RTE_SET_USED(cpuset);
-	RTE_SET_USED(err_str);
-
-	snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-		"DPDK Telemetry is not supported on Windows.");
+	TMTY_LOG(DEBUG, "Telemetry initialized ok\n");
+	telemetry_legacy_init();
 #endif /* RTE_EXEC_ENV_WINDOWS */

 	return 0;
--
2.27.0

Re: [dpdk-dev] [PATCH 1/4] telemetry: use rte_log for logging

[Power, Ciara]

>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
>Sent: Wednesday 10 March 2021 17:24
>To: dev@dpdk.org
>Cc: Richardson, Bruce <bruce.richardson@intel.com>; Laatz, Kevin
><kevin.laatz@intel.com>
>Subject: [dpdk-dev] [PATCH 1/4] telemetry: use rte_log for logging
>
>Rather than passing back an error string to the caller, take as input the rte_log
>function to use, and just use regular logging.
>
>Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>---
> lib/librte_eal/freebsd/eal.c         | 10 ++--
> lib/librte_eal/linux/eal.c           | 10 ++--
> lib/librte_telemetry/rte_telemetry.h | 15 ++++--
> lib/librte_telemetry/telemetry.c     | 72 +++++++++++++---------------
> 4 files changed, 49 insertions(+), 58 deletions(-)
>
<snip>


Acked-by: Ciara Power <ciara.power@intel.com>

[dpdk-dev] [PATCH 2/4] telemetry: make the legacy registration function internal

[Bruce Richardson]

The function for registration of callbacks for legacy telemetry was
documented as internal-only in the API documents, but marked as
experimental in the version.map file. Since this is an internal-only
function, for consistency we update the version mapping to have it as
internal.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 doc/guides/rel_notes/release_21_05.rst      | 5 +++++
 lib/librte_telemetry/rte_telemetry_legacy.h | 2 +-
 lib/librte_telemetry/version.map            | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index 23f7f0bff9..1624f43d73 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -117,6 +117,11 @@ ABI Changes
 
 * No ABI change that would break compatibility with 20.11.
 
+* The experimental function ``rte_telemetry_legacy_register`` has been
+  removed from the public API and is now an internal-only function. This
+  function was already marked as internal in the API documentation for it,
+  and was not for use by external applications.
+
 
 Known Issues
 ------------
diff --git a/lib/librte_telemetry/rte_telemetry_legacy.h b/lib/librte_telemetry/rte_telemetry_legacy.h
index c83f9a8d90..fb44740186 100644
--- a/lib/librte_telemetry/rte_telemetry_legacy.h
+++ b/lib/librte_telemetry/rte_telemetry_legacy.h
@@ -78,7 +78,7 @@ legacy_client_handler(void *sock_id);
  *  @return
  *  -ENOENT if max callbacks limit has been reached.
  */
-__rte_experimental
+__rte_internal
 int
 rte_telemetry_legacy_register(const char *cmd,
 		enum rte_telemetry_legacy_data_req data_req,
diff --git a/lib/librte_telemetry/version.map b/lib/librte_telemetry/version.map
index ec0ebc1bec..bde80ce29b 100644
--- a/lib/librte_telemetry/version.map
+++ b/lib/librte_telemetry/version.map
@@ -14,12 +14,12 @@ EXPERIMENTAL {
 	rte_tel_data_start_array;
 	rte_tel_data_start_dict;
 	rte_tel_data_string;
-	rte_telemetry_legacy_register;
 	rte_telemetry_register_cmd;
 
 	local: *;
 };
 
 INTERNAL {
+	rte_telemetry_legacy_register;
 	rte_telemetry_init;
 };
-- 
2.27.0

Re: [dpdk-dev] [PATCH 2/4] telemetry: make the legacy registration function internal

[Power, Ciara]

>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
>Sent: Wednesday 10 March 2021 17:24
>To: dev@dpdk.org
>Cc: Richardson, Bruce <bruce.richardson@intel.com>; Laatz, Kevin
><kevin.laatz@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
><nhorman@tuxdriver.com>
>Subject: [dpdk-dev] [PATCH 2/4] telemetry: make the legacy registration
>function internal
>
>The function for registration of callbacks for legacy telemetry was
>documented as internal-only in the API documents, but marked as
>experimental in the version.map file. Since this is an internal-only function, for
>consistency we update the version mapping to have it as internal.
>
>Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>---
> doc/guides/rel_notes/release_21_05.rst      | 5 +++++
> lib/librte_telemetry/rte_telemetry_legacy.h | 2 +-
> lib/librte_telemetry/version.map            | 2 +-
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
<snip>


Acked-by: Ciara Power <ciara.power@intel.com>

[dpdk-dev] [PATCH 3/4] telemetry: create internal-only header file

[Bruce Richardson]

The header file containing the legacy telemetry function prototypes was all
internal-only, so we rename the file to be an internal-only one to make it
clearer it's not for installation.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_metrics/rte_metrics_telemetry.c                  | 2 +-
 lib/librte_telemetry/telemetry.c                            | 2 +-
 .../{rte_telemetry_legacy.h => telemetry_internal.h}        | 6 +++---
 lib/librte_telemetry/telemetry_legacy.c                     | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)
 rename lib/librte_telemetry/{rte_telemetry_legacy.h => telemetry_internal.h} (93%)

diff --git a/lib/librte_metrics/rte_metrics_telemetry.c b/lib/librte_metrics/rte_metrics_telemetry.c
index b8ee56ef01..bbc6a79f88 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.c
+++ b/lib/librte_metrics/rte_metrics_telemetry.c
@@ -7,7 +7,7 @@
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
 #ifdef RTE_LIB_TELEMETRY
-#include <rte_telemetry_legacy.h>
+#include <telemetry_internal.h>
 #endif
 
 #include "rte_metrics.h"
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index 18f2ae2e2f..c3c988e972 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -21,7 +21,7 @@
 #include "rte_telemetry.h"
 #include "telemetry_json.h"
 #include "telemetry_data.h"
-#include "rte_telemetry_legacy.h"
+#include "telemetry_internal.h"
 
 #define MAX_CMD_LEN 56
 #define MAX_HELP_LEN 64
diff --git a/lib/librte_telemetry/rte_telemetry_legacy.h b/lib/librte_telemetry/telemetry_internal.h
similarity index 93%
rename from lib/librte_telemetry/rte_telemetry_legacy.h
rename to lib/librte_telemetry/telemetry_internal.h
index fb44740186..ad076b9119 100644
--- a/lib/librte_telemetry/rte_telemetry_legacy.h
+++ b/lib/librte_telemetry/telemetry_internal.h
@@ -2,8 +2,8 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
-#ifndef _RTE_TELEMETRY_LEGACY_H_
-#define _RTE_TELEMETRY_LEGACY_H_
+#ifndef _RTE_TELEMETRY_INTERNAL_H_
+#define _RTE_TELEMETRY_INTERNAL_H_
 
 #include <rte_compat.h>
 #include "rte_telemetry.h"
@@ -14,7 +14,7 @@
  * @b EXPERIMENTAL: this API may change without prior notice
 
  * @file
- * RTE Telemetry Legacy
+ * RTE Telemetry Legacy and internal definitions
  *
  ***/
 
diff --git a/lib/librte_telemetry/telemetry_legacy.c b/lib/librte_telemetry/telemetry_legacy.c
index edd76ca359..5e9af37db1 100644
--- a/lib/librte_telemetry/telemetry_legacy.c
+++ b/lib/librte_telemetry/telemetry_legacy.c
@@ -15,7 +15,7 @@
 #include <rte_common.h>
 #include <rte_spinlock.h>
 
-#include "rte_telemetry_legacy.h"
+#include "telemetry_internal.h"
 
 #define MAX_LEN 128
 #define BUF_SIZE 1024
-- 
2.27.0

Re: [dpdk-dev] [PATCH 3/4] telemetry: create internal-only header file

[Power, Ciara]

>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
>Sent: Wednesday 10 March 2021 17:24
>To: dev@dpdk.org
>Cc: Richardson, Bruce <bruce.richardson@intel.com>; Laatz, Kevin
><kevin.laatz@intel.com>
>Subject: [dpdk-dev] [PATCH 3/4] telemetry: create internal-only header file
>
>The header file containing the legacy telemetry function prototypes was all
>internal-only, so we rename the file to be an internal-only one to make it
>clearer it's not for installation.
>
>Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>---
> lib/librte_metrics/rte_metrics_telemetry.c                  | 2 +-
> lib/librte_telemetry/telemetry.c                            | 2 +-
> .../{rte_telemetry_legacy.h => telemetry_internal.h}        | 6 +++---
> lib/librte_telemetry/telemetry_legacy.c                     | 2 +-
> 4 files changed, 6 insertions(+), 6 deletions(-)  rename
>lib/librte_telemetry/{rte_telemetry_legacy.h => telemetry_internal.h} (93%)
>
<snip>


Acked-by: Ciara Power <ciara.power@intel.com>

[dpdk-dev] [PATCH 4/4] telemetry: move init function to internal header

[Bruce Richardson]

The rte_telemetry_init() function is for EAL use only, so can be moved to
the internal header rather than being in the public one.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/freebsd/eal.c              |  2 +-
 lib/librte_eal/linux/eal.c                |  2 +-
 lib/librte_telemetry/rte_telemetry.h      | 29 -----------------------
 lib/librte_telemetry/telemetry_internal.h | 29 +++++++++++++++++++++++
 4 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index b8a798bc7d..be02f70510 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -42,7 +42,7 @@
 #include <rte_version.h>
 #include <rte_vfio.h>
 #include <malloc_heap.h>
-#include <rte_telemetry.h>
+#include <telemetry_internal.h>
 
 #include "eal_private.h"
 #include "eal_thread.h"
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 23e3a32036..da2d6602a8 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -49,8 +49,8 @@
 #include <rte_version.h>
 #include <malloc_heap.h>
 #include <rte_vfio.h>
-#include <rte_telemetry.h>
 
+#include <telemetry_internal.h>
 #include "eal_private.h"
 #include "eal_thread.h"
 #include "eal_internal_cfg.h"
diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
index 2c3da3ab7f..334ea8ddd4 100644
--- a/lib/librte_telemetry/rte_telemetry.h
+++ b/lib/librte_telemetry/rte_telemetry.h
@@ -292,35 +292,6 @@ __rte_experimental
 int
 rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);
 
-/**
- * @internal
- * Log function type, to allow passing as parameter if necessary
- */
-typedef int (*rte_log_fn)(uint32_t level, uint32_t logtype, const char *format, ...);
-
-/**
- * @internal
- * Initialize Telemetry.
- *
- * @param runtime_dir
- * The runtime directory of DPDK.
- * @param cpuset
- * The CPU set to be used for setting the thread affinity.
- * @param log_fn
- * Function pointer to the rte_log function for logging use
- * @param registered_logtype
- * The registered log type to use for logging
- *
- * @return
- *  0 on success.
- * @return
- *  -1 on failure.
- */
-__rte_internal
-int
-rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
-		rte_log_fn log_fn, uint32_t registered_logtype);
-
 /**
  * Get a pointer to a container with memory allocated. The container is to be
  * used embedded within an existing telemetry dict/array.
diff --git a/lib/librte_telemetry/telemetry_internal.h b/lib/librte_telemetry/telemetry_internal.h
index ad076b9119..601f16cc96 100644
--- a/lib/librte_telemetry/telemetry_internal.h
+++ b/lib/librte_telemetry/telemetry_internal.h
@@ -84,4 +84,33 @@ rte_telemetry_legacy_register(const char *cmd,
 		enum rte_telemetry_legacy_data_req data_req,
 		telemetry_legacy_cb fn);
 
+/**
+ * @internal
+ * Log function type, to allow passing as parameter if necessary
+ */
+typedef int (*rte_log_fn)(uint32_t level, uint32_t logtype, const char *format, ...);
+
+/**
+ * @internal
+ * Initialize Telemetry.
+ *
+ * @param runtime_dir
+ * The runtime directory of DPDK.
+ * @param cpuset
+ * The CPU set to be used for setting the thread affinity.
+ * @param log_fn
+ * Function pointer to the rte_log function for logging use
+ * @param registered_logtype
+ * The registered log type to use for logging
+ *
+ * @return
+ *  0 on success.
+ * @return
+ *  -1 on failure.
+ */
+__rte_internal
+int
+rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
+		rte_log_fn log_fn, uint32_t registered_logtype);
+
 #endif
-- 
2.27.0

Re: [dpdk-dev] [PATCH 4/4] telemetry: move init function to internal header

[Power, Ciara]

>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
>Sent: Wednesday 10 March 2021 17:25
>To: dev@dpdk.org
>Cc: Richardson, Bruce <bruce.richardson@intel.com>; Laatz, Kevin
><kevin.laatz@intel.com>
>Subject: [dpdk-dev] [PATCH 4/4] telemetry: move init function to internal
>header
>
>The rte_telemetry_init() function is for EAL use only, so can be moved to the
>internal header rather than being in the public one.
>
>Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>---
> lib/librte_eal/freebsd/eal.c              |  2 +-
> lib/librte_eal/linux/eal.c                |  2 +-
> lib/librte_telemetry/rte_telemetry.h      | 29 -----------------------
> lib/librte_telemetry/telemetry_internal.h | 29 +++++++++++++++++++++++
> 4 files changed, 31 insertions(+), 31 deletions(-)
>
<snip>


Acked-by: Ciara Power <ciara.power@intel.com>

Re: [dpdk-dev] [PATCH 0/4] telemetry logging improvements and cleanup

[Thomas Monjalon]

10/03/2021 18:24, Bruce Richardson:
> This set adds support for using the regular rte_log functions from the telemetry
> library; avoiding circular dependencies by having EAL register the telemetry
> library itself and then passing the required handles to that library as part of
> the telemetry_init call.
> 
> Beyond this change, the other three patches are cleanups to ensure that all
> internal functions are clearly separate from the public APIs. (Patches 3 & 4 may
> be merged into a single one on apply, for I've kept them separate for now for
> clarity).
> 
> Bruce Richardson (4):
>   telemetry: use rte_log for logging
>   telemetry: make the legacy registration function internal
>   telemetry: create internal-only header file
>   telemetry: move init function to internal header

Now that your patch "eal: fix querying DPDK version at runtime"
is in main branch, please could you rebase this series?

Re: [dpdk-dev] [PATCH 0/4] telemetry logging improvements and cleanup

[Bruce Richardson]

On Wed, Mar 24, 2021 at 10:11:55PM +0100, Thomas Monjalon wrote:
> 10/03/2021 18:24, Bruce Richardson:
> > This set adds support for using the regular rte_log functions from the telemetry
> > library; avoiding circular dependencies by having EAL register the telemetry
> > library itself and then passing the required handles to that library as part of
> > the telemetry_init call.
> > 
> > Beyond this change, the other three patches are cleanups to ensure that all
> > internal functions are clearly separate from the public APIs. (Patches 3 & 4 may
> > be merged into a single one on apply, for I've kept them separate for now for
> > clarity).
> > 
> > Bruce Richardson (4):
> >   telemetry: use rte_log for logging
> >   telemetry: make the legacy registration function internal
> >   telemetry: create internal-only header file
> >   telemetry: move init function to internal header
> 
> Now that your patch "eal: fix querying DPDK version at runtime"
> is in main branch, please could you rebase this series?
> 
Sure, will do.

[dpdk-dev] [PATCH v2 0/4] telemetry logging improvements and cleanup

[Bruce Richardson]

This set adds support for using the regular rte_log functions from the telemetry
library; avoiding circular dependencies by having EAL register the telemetry
library itself and then passing the required handles to that library as part of
the telemetry_init call.

Beyond this change, the other three patches are cleanups to ensure that all
internal functions are clearly separate from the public APIs. (Patches 3 & 4 may
be merged into a single one on apply, for I've kept them separate for now for
clarity).

V2: Rebased on latest main branch.

Bruce Richardson (4):
  telemetry: use rte_log for logging
  telemetry: make the legacy registration function internal
  telemetry: rename internal-only header file
  telemetry: move init function to internal header

 doc/guides/rel_notes/release_21_05.rst        |  5 ++
 lib/librte_eal/freebsd/eal.c                  | 12 +--
 lib/librte_eal/linux/eal.c                    | 12 +--
 lib/librte_metrics/rte_metrics_telemetry.c    |  2 +-
 lib/librte_telemetry/rte_telemetry.h          | 25 ------
 lib/librte_telemetry/telemetry.c              | 76 +++++++++----------
 ...elemetry_legacy.h => telemetry_internal.h} | 41 +++++++++-
 lib/librte_telemetry/telemetry_legacy.c       |  2 +-
 lib/librte_telemetry/version.map              |  2 +-
 9 files changed, 87 insertions(+), 90 deletions(-)
 rename lib/librte_telemetry/{rte_telemetry_legacy.h => telemetry_internal.h} (65%)

--
2.27.0

[dpdk-dev] [PATCH v2 1/4] telemetry: use rte_log for logging

[Bruce Richardson]

Rather than passing back an error string to the caller, take as input the
rte_log function to use, and just use regular logging.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Ciara Power <ciara.power@intel.com>
---
 lib/librte_eal/freebsd/eal.c         | 10 ++--
 lib/librte_eal/linux/eal.c           | 10 ++--
 lib/librte_telemetry/rte_telemetry.h | 15 ++++--
 lib/librte_telemetry/telemetry.c     | 74 +++++++++++++---------------
 4 files changed, 50 insertions(+), 59 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 62320d610..97ce9976c 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -941,16 +941,12 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 	if (!internal_conf->no_telemetry) {
-		const char *error_str = NULL;
+		uint32_t tlog = rte_log_register_type_and_pick_level(
+				"lib.telemetry", RTE_LOG_WARNING);
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
 				rte_version(),
-				&internal_conf->ctrl_cpuset, &error_str)
-				!= 0) {
-			rte_eal_init_alert(error_str);
+				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
 			return -1;
-		}
-		if (error_str != NULL)
-			RTE_LOG(NOTICE, EAL, "%s\n", error_str);
 	}
 
 	eal_mcfg_complete();
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 9ffb4b331..f6dd67b6d 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1314,16 +1314,12 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 	if (!internal_conf->no_telemetry) {
-		const char *error_str = NULL;
+		uint32_t tlog = rte_log_register_type_and_pick_level(
+				"lib.telemetry", RTE_LOG_WARNING);
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
 				rte_version(),
-				&internal_conf->ctrl_cpuset, &error_str)
-				!= 0) {
-			rte_eal_init_alert(error_str);
+				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
 			return -1;
-		}
-		if (error_str != NULL)
-			RTE_LOG(NOTICE, EAL, "%s\n", error_str);
 	}
 
 	eal_mcfg_complete();
diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
index 027b048d7..d38894b97 100644
--- a/lib/librte_telemetry/rte_telemetry.h
+++ b/lib/librte_telemetry/rte_telemetry.h
@@ -294,6 +294,12 @@ rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);
 
 #ifdef RTE_HAS_CPUSET
 
+/**
+ * @internal
+ * Log function type, to allow passing as parameter if necessary
+ */
+typedef int (*rte_log_fn)(uint32_t level, uint32_t logtype, const char *format, ...);
+
 /**
  * @internal
  * Initialize Telemetry.
@@ -302,9 +308,10 @@ rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);
  * The runtime directory of DPDK.
  * @param cpuset
  * The CPU set to be used for setting the thread affinity.
- * @param err_str
- * This err_str pointer should point to NULL on entry. In the case of an error
- * or warning, it will be non-NULL on exit.
+ * @param log_fn
+ * Function pointer to the rte_log function for logging use
+ * @param registered_logtype
+ * The registered log type to use for logging
  *
  * @return
  *  0 on success.
@@ -314,7 +321,7 @@ rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);
 __rte_internal
 int
 rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
-		const char **err_str);
+		rte_log_fn log_fn, uint32_t registered_logtype);
 
 #endif /* RTE_HAS_CPUSET */
 
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index 14b4ff5ea..042136b82 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -15,6 +15,7 @@
 #include <rte_string_fns.h>
 #include <rte_common.h>
 #include <rte_spinlock.h>
+#include <rte_log.h>
 
 #include "rte_telemetry.h"
 #include "telemetry_json.h"
@@ -49,7 +50,14 @@ static struct socket v1_socket; /* socket for v1 telemetry */
 #endif /* !RTE_EXEC_ENV_WINDOWS */
 
 static const char *telemetry_version; /* save rte_version */
-static char telemetry_log_error[1024]; /* Will contain error on init failure */
+static const char *socket_dir;        /* runtime directory */
+static rte_cpuset_t *thread_cpuset;
+static rte_log_fn rte_log_ptr;
+static uint32_t logtype;
+
+#define TMTY_LOG(l, ...) \
+        rte_log_ptr(RTE_LOG_ ## l, logtype, "TELEMETRY: " __VA_ARGS__)
+
 /* list of command callbacks, with one command registered by default */
 static struct cmd_callback callbacks[TELEMETRY_MAX_CALLBACKS];
 static int num_callbacks; /* How many commands are registered */
@@ -345,9 +353,7 @@ socket_listener(void *socket)
 		struct socket *s = (struct socket *)socket;
 		int s_accepted = accept(s->sock, NULL, NULL);
 		if (s_accepted < 0) {
-			snprintf(telemetry_log_error,
-				sizeof(telemetry_log_error),
-				"Error with accept, telemetry thread quitting");
+			TMTY_LOG(ERR, "Error with accept, telemetry thread quitting\n");
 			return NULL;
 		}
 		if (s->num_clients != NULL) {
@@ -389,9 +395,7 @@ create_socket(char *path)
 {
 	int sock = socket(AF_UNIX, SOCK_SEQPACKET, 0);
 	if (sock < 0) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error with socket creation, %s",
-				strerror(errno));
+		TMTY_LOG(ERR, "Error with socket creation, %s\n", strerror(errno));
 		return -1;
 	}
 
@@ -399,17 +403,13 @@ create_socket(char *path)
 	strlcpy(sun.sun_path, path, sizeof(sun.sun_path));
 	unlink(sun.sun_path);
 	if (bind(sock, (void *) &sun, sizeof(sun)) < 0) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error binding socket: %s",
-				strerror(errno));
+		TMTY_LOG(ERR, "Error binding socket: %s\n", strerror(errno));
 		sun.sun_path[0] = 0;
 		goto error;
 	}
 
 	if (listen(sock, 1) < 0) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error calling listen for socket: %s",
-				strerror(errno));
+		TMTY_LOG(ERR, "Error calling listen for socket: %s\n", strerror(errno));
 		goto error;
 	}
 
@@ -422,35 +422,33 @@ create_socket(char *path)
 }
 
 static int
-telemetry_legacy_init(const char *runtime_dir, rte_cpuset_t *cpuset)
+telemetry_legacy_init(void)
 {
 	pthread_t t_old;
 
 	if (num_legacy_callbacks == 1) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-			 "No legacy callbacks, legacy socket not created");
+		TMTY_LOG(WARNING, "No legacy callbacks, legacy socket not created\n");
 		return -1;
 	}
 
 	v1_socket.fn = legacy_client_handler;
 	if ((size_t) snprintf(v1_socket.path, sizeof(v1_socket.path),
-			"%s/telemetry", runtime_dir)
-			>= sizeof(v1_socket.path)) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error with socket binding, path too long");
+			"%s/telemetry", socket_dir) >= sizeof(v1_socket.path)) {
+		TMTY_LOG(ERR, "Error with socket binding, path too long\n");
 		return -1;
 	}
 	v1_socket.sock = create_socket(v1_socket.path);
 	if (v1_socket.sock < 0)
 		return -1;
 	pthread_create(&t_old, NULL, socket_listener, &v1_socket);
-	pthread_setaffinity_np(t_old, sizeof(*cpuset), cpuset);
+	pthread_setaffinity_np(t_old, sizeof(*thread_cpuset), thread_cpuset);
 
+	TMTY_LOG(DEBUG, "Legacy telemetry socket initialized ok\n");
 	return 0;
 }
 
 static int
-telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
+telemetry_v2_init(void)
 {
 	pthread_t t_new;
 
@@ -462,10 +460,9 @@ telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
 	rte_telemetry_register_cmd("/help", command_help,
 			"Returns help text for a command. Parameters: string command");
 	v2_socket.fn = client_handler;
-	if (strlcpy(v2_socket.path, get_socket_path(runtime_dir, 2),
+	if (strlcpy(v2_socket.path, get_socket_path(socket_dir, 2),
 			sizeof(v2_socket.path)) >= sizeof(v2_socket.path)) {
-		snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-				"Error with socket binding, path too long");
+		TMTY_LOG(ERR, "Error with socket binding, path too long\n");
 		return -1;
 	}
 
@@ -473,7 +470,7 @@ telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
 	if (v2_socket.sock < 0)
 		return -1;
 	pthread_create(&t_new, NULL, socket_listener, &v2_socket);
-	pthread_setaffinity_np(t_new, sizeof(*cpuset), cpuset);
+	pthread_setaffinity_np(t_new, sizeof(*thread_cpuset), thread_cpuset);
 	atexit(unlink_sockets);
 
 	return 0;
@@ -482,25 +479,20 @@ telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
 #endif /* !RTE_EXEC_ENV_WINDOWS */
 
 int32_t
-rte_telemetry_init(const char *runtime_dir, const char *rte_version,
-		rte_cpuset_t *cpuset, const char **err_str)
+rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
+		rte_log_fn log_fn, uint32_t registered_logtype)
 {
 	telemetry_version = rte_version;
+	socket_dir = runtime_dir;
+	thread_cpuset = cpuset;
+	rte_log_ptr = log_fn;
+	logtype = registered_logtype;
+
 #ifndef RTE_EXEC_ENV_WINDOWS
-	if (telemetry_v2_init(runtime_dir, cpuset) != 0) {
-		*err_str = telemetry_log_error;
+	if (telemetry_v2_init() != 0)
 		return -1;
-	}
-	if (telemetry_legacy_init(runtime_dir, cpuset) != 0) {
-		*err_str = telemetry_log_error;
-	}
-#else /* RTE_EXEC_ENV_WINDOWS */
-	RTE_SET_USED(runtime_dir);
-	RTE_SET_USED(cpuset);
-	RTE_SET_USED(err_str);
-
-	snprintf(telemetry_log_error, sizeof(telemetry_log_error),
-		"DPDK Telemetry is not supported on Windows.");
+	TMTY_LOG(DEBUG, "Telemetry initialized ok\n");
+	telemetry_legacy_init();
 #endif /* RTE_EXEC_ENV_WINDOWS */
 
 	return 0;
-- 
2.27.0

Re: [dpdk-dev] [PATCH v2 1/4] telemetry: use rte_log for logging

[David Marchand]

On Thu, Mar 25, 2021 at 2:57 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> Rather than passing back an error string to the caller, take as input the
> rte_log function to use, and just use regular logging.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Ciara Power <ciara.power@intel.com>

I guess this replaces
http://patchwork.dpdk.org/project/dpdk/patch/20210308222339.819494-1-thomas@monjalon.net/
?


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 1/4] telemetry: use rte_log for logging

[Bruce Richardson]

On Thu, Mar 25, 2021 at 03:09:32PM +0100, David Marchand wrote:
> On Thu, Mar 25, 2021 at 2:57 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > Rather than passing back an error string to the caller, take as input the
> > rte_log function to use, and just use regular logging.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Acked-by: Ciara Power <ciara.power@intel.com>
> 
> I guess this replaces
> http://patchwork.dpdk.org/project/dpdk/patch/20210308222339.819494-1-thomas@monjalon.net/
> ?
Yes, it would do.

[dpdk-dev] [PATCH v2 2/4] telemetry: make the legacy registration function internal

[Bruce Richardson]

The function for registration of callbacks for legacy telemetry was
documented as internal-only in the API documents, but marked as
experimental in the version.map file. Since this is an internal-only
function, for consistency we update the version mapping to have it as
internal.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Ciara Power <ciara.power@intel.com>
---
 doc/guides/rel_notes/release_21_05.rst      | 5 +++++
 lib/librte_telemetry/rte_telemetry_legacy.h | 2 +-
 lib/librte_telemetry/version.map            | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index 8e686cc62..fb965fec3 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -148,6 +148,11 @@ ABI Changes
 
 * No ABI change that would break compatibility with 20.11.
 
+* The experimental function ``rte_telemetry_legacy_register`` has been
+  removed from the public API and is now an internal-only function. This
+  function was already marked as internal in the API documentation for it,
+  and was not for use by external applications.
+
 
 Known Issues
 ------------
diff --git a/lib/librte_telemetry/rte_telemetry_legacy.h b/lib/librte_telemetry/rte_telemetry_legacy.h
index c83f9a8d9..fb4474018 100644
--- a/lib/librte_telemetry/rte_telemetry_legacy.h
+++ b/lib/librte_telemetry/rte_telemetry_legacy.h
@@ -78,7 +78,7 @@ legacy_client_handler(void *sock_id);
  *  @return
  *  -ENOENT if max callbacks limit has been reached.
  */
-__rte_experimental
+__rte_internal
 int
 rte_telemetry_legacy_register(const char *cmd,
 		enum rte_telemetry_legacy_data_req data_req,
diff --git a/lib/librte_telemetry/version.map b/lib/librte_telemetry/version.map
index ec0ebc1be..bde80ce29 100644
--- a/lib/librte_telemetry/version.map
+++ b/lib/librte_telemetry/version.map
@@ -14,12 +14,12 @@ EXPERIMENTAL {
 	rte_tel_data_start_array;
 	rte_tel_data_start_dict;
 	rte_tel_data_string;
-	rte_telemetry_legacy_register;
 	rte_telemetry_register_cmd;
 
 	local: *;
 };
 
 INTERNAL {
+	rte_telemetry_legacy_register;
 	rte_telemetry_init;
 };
-- 
2.27.0

[dpdk-dev] [PATCH v2 3/4] telemetry: rename internal-only header file

[Bruce Richardson]

The header file containing the legacy telemetry function prototypes was all
internal-only, so we rename the file to be an internal-only one to make it
clearer it's not for installation.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Ciara Power <ciara.power@intel.com>
---
 lib/librte_metrics/rte_metrics_telemetry.c                  | 2 +-
 lib/librte_telemetry/telemetry.c                            | 2 +-
 .../{rte_telemetry_legacy.h => telemetry_internal.h}        | 6 +++---
 lib/librte_telemetry/telemetry_legacy.c                     | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)
 rename lib/librte_telemetry/{rte_telemetry_legacy.h => telemetry_internal.h} (93%)

diff --git a/lib/librte_metrics/rte_metrics_telemetry.c b/lib/librte_metrics/rte_metrics_telemetry.c
index 795bd29fe..c24990d92 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.c
+++ b/lib/librte_metrics/rte_metrics_telemetry.c
@@ -5,7 +5,7 @@
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
 #ifdef RTE_LIB_TELEMETRY
-#include <rte_telemetry_legacy.h>
+#include <telemetry_internal.h>
 #endif
 
 #include "rte_metrics.h"
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index 042136b82..7e08afd22 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -20,7 +20,7 @@
 #include "rte_telemetry.h"
 #include "telemetry_json.h"
 #include "telemetry_data.h"
-#include "rte_telemetry_legacy.h"
+#include "telemetry_internal.h"
 
 #define MAX_CMD_LEN 56
 #define MAX_HELP_LEN 64
diff --git a/lib/librte_telemetry/rte_telemetry_legacy.h b/lib/librte_telemetry/telemetry_internal.h
similarity index 93%
rename from lib/librte_telemetry/rte_telemetry_legacy.h
rename to lib/librte_telemetry/telemetry_internal.h
index fb4474018..ad076b911 100644
--- a/lib/librte_telemetry/rte_telemetry_legacy.h
+++ b/lib/librte_telemetry/telemetry_internal.h
@@ -2,8 +2,8 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
-#ifndef _RTE_TELEMETRY_LEGACY_H_
-#define _RTE_TELEMETRY_LEGACY_H_
+#ifndef _RTE_TELEMETRY_INTERNAL_H_
+#define _RTE_TELEMETRY_INTERNAL_H_
 
 #include <rte_compat.h>
 #include "rte_telemetry.h"
@@ -14,7 +14,7 @@
  * @b EXPERIMENTAL: this API may change without prior notice
 
  * @file
- * RTE Telemetry Legacy
+ * RTE Telemetry Legacy and internal definitions
  *
  ***/
 
diff --git a/lib/librte_telemetry/telemetry_legacy.c b/lib/librte_telemetry/telemetry_legacy.c
index edd76ca35..5e9af37db 100644
--- a/lib/librte_telemetry/telemetry_legacy.c
+++ b/lib/librte_telemetry/telemetry_legacy.c
@@ -15,7 +15,7 @@
 #include <rte_common.h>
 #include <rte_spinlock.h>
 
-#include "rte_telemetry_legacy.h"
+#include "telemetry_internal.h"
 
 #define MAX_LEN 128
 #define BUF_SIZE 1024
-- 
2.27.0

[dpdk-dev] [PATCH v2 4/4] telemetry: move init function to internal header

[Bruce Richardson]

The rte_telemetry_init() function is for EAL use only, so can be moved to
the internal header rather than being in the public one.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Ciara Power <ciara.power@intel.com>
---
 lib/librte_eal/freebsd/eal.c              |  2 +-
 lib/librte_eal/linux/eal.c                |  2 +-
 lib/librte_telemetry/rte_telemetry.h      | 32 ----------------------
 lib/librte_telemetry/telemetry_internal.h | 33 +++++++++++++++++++++++
 4 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 97ce9976c..32442e5ba 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -42,7 +42,7 @@
 #include <rte_version.h>
 #include <rte_vfio.h>
 #include <malloc_heap.h>
-#include <rte_telemetry.h>
+#include <telemetry_internal.h>
 
 #include "eal_private.h"
 #include "eal_thread.h"
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index f6dd67b6d..abbb53774 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -49,8 +49,8 @@
 #include <rte_version.h>
 #include <malloc_heap.h>
 #include <rte_vfio.h>
-#include <rte_telemetry.h>
 
+#include <telemetry_internal.h>
 #include "eal_private.h"
 #include "eal_thread.h"
 #include "eal_internal_cfg.h"
diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
index d38894b97..fd57718c2 100644
--- a/lib/librte_telemetry/rte_telemetry.h
+++ b/lib/librte_telemetry/rte_telemetry.h
@@ -292,38 +292,6 @@ __rte_experimental
 int
 rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);
 
-#ifdef RTE_HAS_CPUSET
-
-/**
- * @internal
- * Log function type, to allow passing as parameter if necessary
- */
-typedef int (*rte_log_fn)(uint32_t level, uint32_t logtype, const char *format, ...);
-
-/**
- * @internal
- * Initialize Telemetry.
- *
- * @param runtime_dir
- * The runtime directory of DPDK.
- * @param cpuset
- * The CPU set to be used for setting the thread affinity.
- * @param log_fn
- * Function pointer to the rte_log function for logging use
- * @param registered_logtype
- * The registered log type to use for logging
- *
- * @return
- *  0 on success.
- * @return
- *  -1 on failure.
- */
-__rte_internal
-int
-rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
-		rte_log_fn log_fn, uint32_t registered_logtype);
-
-#endif /* RTE_HAS_CPUSET */
 
 /**
  * Get a pointer to a container with memory allocated. The container is to be
diff --git a/lib/librte_telemetry/telemetry_internal.h b/lib/librte_telemetry/telemetry_internal.h
index ad076b911..6c5200604 100644
--- a/lib/librte_telemetry/telemetry_internal.h
+++ b/lib/librte_telemetry/telemetry_internal.h
@@ -84,4 +84,37 @@ rte_telemetry_legacy_register(const char *cmd,
 		enum rte_telemetry_legacy_data_req data_req,
 		telemetry_legacy_cb fn);
 
+#ifdef RTE_HAS_CPUSET
+
+/**
+ * @internal
+ * Log function type, to allow passing as parameter if necessary
+ */
+typedef int (*rte_log_fn)(uint32_t level, uint32_t logtype, const char *format, ...);
+
+/**
+ * @internal
+ * Initialize Telemetry.
+ *
+ * @param runtime_dir
+ * The runtime directory of DPDK.
+ * @param cpuset
+ * The CPU set to be used for setting the thread affinity.
+ * @param log_fn
+ * Function pointer to the rte_log function for logging use
+ * @param registered_logtype
+ * The registered log type to use for logging
+ *
+ * @return
+ *  0 on success.
+ * @return
+ *  -1 on failure.
+ */
+__rte_internal
+int
+rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
+		rte_log_fn log_fn, uint32_t registered_logtype);
+
+#endif /* RTE_HAS_CPUSET */
+
 #endif
-- 
2.27.0

Re: [dpdk-dev] [PATCH v2 0/4] telemetry logging improvements and cleanup

[Thomas Monjalon]

25/03/2021 14:57, Bruce Richardson:
> This set adds support for using the regular rte_log functions from the telemetry
> library; avoiding circular dependencies by having EAL register the telemetry
> library itself and then passing the required handles to that library as part of
> the telemetry_init call.
> 
> Beyond this change, the other three patches are cleanups to ensure that all
> internal functions are clearly separate from the public APIs. (Patches 3 & 4 may
> be merged into a single one on apply, for I've kept them separate for now for
> clarity).
> 
> V2: Rebased on latest main branch.
> 
> Bruce Richardson (4):
>   telemetry: use rte_log for logging
>   telemetry: make the legacy registration function internal
>   telemetry: rename internal-only header file
>   telemetry: move init function to internal header

Applied, thanks.