DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC 0/6] replace telemetry with process_info
@ 2019-12-05 17:31 Ciara Power
  2019-12-05 17:31 ` [dpdk-dev] [RFC 1/6] process-info: introduce process-info library Ciara Power
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Ciara Power @ 2019-12-05 17:31 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson

From: Bruce Richardson <bruce.richardson@intel.com>

This patchset proposes a new library, called "process-info" for now, to
replace the existing telemetry library in DPDK. (Name subject to change
if someone can propose a better one).

The existing telemetry library provides useful capabilities if used:
  - Creates a unix socket on the system to allow external programs
    connect and gather stats about the DPDK process.
  - Supports outputting the xstats for various network cards on the
    system.
  - Can be used to output any other information exported to the metrics
    library, e.g. by applications.
  - Uses JSON message format, which is widely supported by other
    languages and systems.
  - Is supported by a plugin to collectd.

However, the library also has some issues and limitations that could be
improved upon:
  - Has a dependency on libjansson for JSON processing, so is disabled
    by default.
  - Tied entirely to the metrics library for statistics.
  - No support for sending non-stats data, e.g. something as simple as
    DPDK version string.
  - All data gathering functions are in the library itself, which also
    means…
  - No support for libraries or drivers to present their own
    information via the library.

We therefore propose to keep the good points of the existing library,
but change the way things work to rectify the downsides.
This leads to the following design choices in the library:
  - Keep the existing idea of using a unix socket for connection (just
    simplifying the connection handling).
  - We would like to use JSON format, where possible, but the jansson
    library dependency is problematic. However, creating JSON-encoded
    data is easier than trying to parse JSON in C code, so we can keep
    the JSON output format for processing by e.g. collectd and other
    tools, while simplifying the input to be plain text commands:
	- Commands in this RFC are designed to all start with "/" for
	  consistency
	- Any parameters to the commands, e.g. the specific port to get
	  stats for, are placed after a comma ","
  - Have the library only handle socket creation and input handling.
    All data gathering should be provided by functions external to the
    library registered by other components, e.g. have ethdev library
    provide the function to query NIC xstats, etc.
  - Have the library directly initialized by EAL by default, so that
    unless an app explicitly does not want the support, monitoring is
    available on all DPDK apps.

The obvious question that remains to be answered here is: "why a new
library rather than just fixing the old one?"

The answer is that we have conflicts between the final two design
choices above, which require that the library be built early in the
build as other libraries will call into it to register callbacks, and
the desire to keep backward compatibility e.g. for use with collectd
plugin, which requires the existing library code be kept around and
built - as it is now - at the end of the build process since it calls
into other DPDK libraries. We therefore cannot have one library that
meets both requirements, hence the replacement which allows us to
maintain backward compatibility by just leaving the old lib in place
until e.g. 20.11. 

This is also why the new library is called "process_info", since the
name "telemetry" is already taken. Suggestions for a better name
welcome.

Bruce Richardson (4):
  process-info: introduce process-info library
  eal: integrate process-info library
  usertools: add process-info python script
  ethdev: add callback support for process-info

Ciara Power (2):
  rawdev: add callback support for process-info
  examples/l3fwd-power: enable use of process-info

 config/common_base                            |   5 +
 examples/l3fwd-power/main.c                   |  83 ++-----
 lib/Makefile                                  |   3 +-
 lib/librte_eal/common/eal_common_options.c    |  75 ++++++
 lib/librte_eal/common/eal_internal_cfg.h      |   1 +
 lib/librte_eal/common/eal_options.h           |   5 +
 lib/librte_eal/freebsd/eal/Makefile           |   1 +
 lib/librte_eal/freebsd/eal/eal.c              |  14 ++
 lib/librte_eal/linux/eal/Makefile             |   1 +
 lib/librte_eal/linux/eal/eal.c                |  15 ++
 lib/librte_eal/meson.build                    |   2 +-
 lib/librte_ethdev/Makefile                    |   2 +-
 lib/librte_ethdev/rte_ethdev.c                |  73 ++++++
 lib/librte_process_info/Makefile              |  26 ++
 lib/librte_process_info/meson.build           |   8 +
 lib/librte_process_info/process_info.c        | 231 ++++++++++++++++++
 lib/librte_process_info/rte_process_info.h    |  25 ++
 .../rte_process_info_version.map              |   6 +
 lib/librte_rawdev/Makefile                    |   3 +-
 lib/librte_rawdev/meson.build                 |   1 +
 lib/librte_rawdev/rte_rawdev.c                |  82 +++++++
 lib/meson.build                               |   1 +
 mk/rte.app.mk                                 |   1 +
 usertools/test_new_telemetry.py               |  28 +++
 24 files changed, 630 insertions(+), 62 deletions(-)
 create mode 100644 lib/librte_process_info/Makefile
 create mode 100644 lib/librte_process_info/meson.build
 create mode 100644 lib/librte_process_info/process_info.c
 create mode 100644 lib/librte_process_info/rte_process_info.h
 create mode 100644 lib/librte_process_info/rte_process_info_version.map
 create mode 100755 usertools/test_new_telemetry.py

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [RFC 1/6] process-info: introduce process-info library
  2019-12-05 17:31 [dpdk-dev] [RFC 0/6] replace telemetry with process_info Ciara Power
@ 2019-12-05 17:31 ` Ciara Power
  2019-12-05 17:31 ` [dpdk-dev] [RFC 2/6] eal: integrate " Ciara Power
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Power @ 2019-12-05 17:31 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Ciara Power

From: Bruce Richardson <bruce.richardson@intel.com>

This patch introduces the process_info library. The library does socket
and connection management on a local unix socket, and allows the calling
of callback functions on receipt of a message from a connected client.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Ciara Power <ciara.power@intel.com>

---
The library is not yet initialised to run, this will come in a later
patch integrating with the EAL library.
---
 config/common_base                            |   5 +
 lib/Makefile                                  |   1 +
 lib/librte_process_info/Makefile              |  26 ++
 lib/librte_process_info/meson.build           |   8 +
 lib/librte_process_info/process_info.c        | 231 ++++++++++++++++++
 lib/librte_process_info/rte_process_info.h    |  25 ++
 .../rte_process_info_version.map              |   6 +
 lib/meson.build                               |   1 +
 mk/rte.app.mk                                 |   1 +
 9 files changed, 304 insertions(+)
 create mode 100644 lib/librte_process_info/Makefile
 create mode 100644 lib/librte_process_info/meson.build
 create mode 100644 lib/librte_process_info/process_info.c
 create mode 100644 lib/librte_process_info/rte_process_info.h
 create mode 100644 lib/librte_process_info/rte_process_info_version.map

diff --git a/config/common_base b/config/common_base
index 7dec7ed45..5f49b934f 100644
--- a/config/common_base
+++ b/config/common_base
@@ -1093,3 +1093,8 @@ CONFIG_RTE_APP_CRYPTO_PERF=y
 # Compile the eventdev application
 #
 CONFIG_RTE_APP_EVENTDEV=y
+
+#
+# Compile the process_info library
+#
+CONFIG_RTE_LIBRTE_PROCESS_INFO=y
diff --git a/lib/Makefile b/lib/Makefile
index 46b91ae1a..466cd04ef 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -4,6 +4,7 @@
 include $(RTE_SDK)/mk/rte.vars.mk
 
 DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs
+DIRS-$(CONFIG_RTE_LIBRTE_PROCESS_INFO) += librte_process_info
 DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal
 DEPDIRS-librte_eal := librte_kvargs
 DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci
diff --git a/lib/librte_process_info/Makefile b/lib/librte_process_info/Makefile
new file mode 100644
index 000000000..67b9a9f20
--- /dev/null
+++ b/lib/librte_process_info/Makefile
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2019 Intel Corporation
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_process_info.a
+
+CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
+CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
+CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include/arch/$(ARCH_DIR)/
+CFLAGS += -pthread
+LDLIBS += -pthread
+
+EXPORT_MAP := rte_process_info_version.map
+
+LIBABIVER := 1
+
+# all source are stored in SRCS-y
+SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) := process_info.c
+
+# install includes
+INCS := rte_process_info.h
+SYMLINK-y-include := $(INCS)
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_process_info/meson.build b/lib/librte_process_info/meson.build
new file mode 100644
index 000000000..42850e816
--- /dev/null
+++ b/lib/librte_process_info/meson.build
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2019 Intel Corporation
+
+includes = [global_inc]
+includes += include_directories('../librte_eal/common/include/arch/' + arch_subdir)
+
+sources = files('process_info.c')
+headers = files('rte_process_info.h')
diff --git a/lib/librte_process_info/process_info.c b/lib/librte_process_info/process_info.c
new file mode 100644
index 000000000..3959e24fa
--- /dev/null
+++ b/lib/librte_process_info/process_info.c
@@ -0,0 +1,231 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Intel Corporation
+ */
+
+#include <unistd.h>
+#include <pthread.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <dlfcn.h>
+
+#include <rte_string_fns.h>
+#include <rte_common.h>
+#include <rte_spinlock.h>
+
+#include "rte_process_info.h"
+
+#define MAX_CMD_LEN 56
+
+static int
+list_commands(const char *cmd __rte_unused, const char *params __rte_unused,
+		char *buffer, int buf_len);
+
+struct cmd_callback {
+	char cmd[MAX_CMD_LEN];
+	process_info_cb fn;
+};
+
+static int sock = -1;
+static struct sockaddr_un sun;
+static char process_info_log_error[1024];
+static struct cmd_callback callbacks[PROCESS_INFO_MAX_CALLBACKS] = {
+		{ .cmd = "/", .fn = list_commands },
+};
+static int num_callbacks = 1;
+static rte_spinlock_t callback_sl = RTE_SPINLOCK_INITIALIZER;
+
+/* we won't link against libbsd, so just always use DPDKs-specific version */
+#ifndef strlcpy
+#define strlcpy rte_strlcpy
+#endif
+
+int
+rte_process_info_register(const char *cmd, process_info_cb fn)
+{
+	int i = 0;
+
+	if (strlen(cmd) >= MAX_CMD_LEN || fn == NULL)
+		return -EINVAL;
+	if (num_callbacks >= PROCESS_INFO_MAX_CALLBACKS)
+		return -ENOENT;
+	if (cmd[0] != '/')
+		return -EINVAL;
+
+	rte_spinlock_lock(&callback_sl);
+	while (i < num_callbacks && strcmp(cmd, callbacks[i].cmd) > 0)
+		i++;
+	if (i != num_callbacks)
+		memmove(callbacks + i + 1, callbacks + i,
+			sizeof(struct cmd_callback) * (num_callbacks - i));
+
+	strlcpy(callbacks[i].cmd, cmd, MAX_CMD_LEN);
+	callbacks[i].fn = fn;
+	num_callbacks++;
+	rte_spinlock_unlock(&callback_sl);
+
+	return 0;
+}
+
+static int
+list_commands(const char *cmd __rte_unused, const char *params __rte_unused,
+		char *buffer, int buf_len)
+{
+	int i, used = 0;
+
+	used += strlcpy(buffer, "[", buf_len);
+	for (i = 0; i < num_callbacks; i++)
+		used += snprintf(buffer + used, buf_len - used, "\"%s\",",
+				callbacks[i].cmd);
+	buffer[used - 1] = ']';
+	return used;
+}
+
+static void
+perform_command(process_info_cb fn, const char *cmd, const char *param, int s)
+{
+	char out_buf[1024 * 12];
+
+	int used = snprintf(out_buf,
+			sizeof(out_buf), "{\"%s\":", cmd);
+	int ret = fn(cmd, param, out_buf + used, sizeof(out_buf) - used);
+	if (ret < 0) {
+		used += strlcpy(out_buf + used, "null}\n",
+				sizeof(out_buf) - used);
+		if (write(s, out_buf, used) < 0)
+			perror("Error writing to socket");
+		return;
+	}
+	used += ret;
+	used += strlcpy(out_buf + used, "}\n", sizeof(out_buf) - used);
+	if (write(s, out_buf, used) < 0)
+		perror("Error writing to socket");
+}
+
+static int
+unknown_command(const char *cmd __rte_unused, const char *params __rte_unused,
+		char *buffer, int buf_len)
+{
+	return snprintf(buffer, buf_len, "null");
+}
+
+static void *
+client_handler(void *sock_id)
+{
+	int s = (int)(uintptr_t)sock_id;
+	char buffer[1024];
+
+	/* receive data is not null terminated */
+	int bytes = read(s, buffer, sizeof(buffer));
+	buffer[bytes] = 0;
+
+	while (bytes > 0) {
+		const char *cmd = strtok(buffer, ",");
+		const char *param = strtok(NULL, ",");
+		process_info_cb fn = unknown_command;
+		int i;
+
+		rte_spinlock_lock(&callback_sl);
+		for (i = 0; i < num_callbacks; i++)
+			if (strcmp(cmd, callbacks[i].cmd) == 0) {
+				fn = callbacks[i].fn;
+				break;
+			}
+
+		rte_spinlock_unlock(&callback_sl);
+		perform_command(fn, cmd, param, s);
+
+		bytes = read(s, buffer, sizeof(buffer));
+		buffer[bytes] = 0;
+	}
+	close(s);
+	return NULL;
+}
+
+static void *
+socket_listener(void *unused __rte_unused)
+{
+	while (1) {
+		pthread_t th;
+		int s = accept(sock, NULL, NULL);
+		if (s < 0) {
+			snprintf(process_info_log_error,
+					sizeof(process_info_log_error),
+					"Error with accept, process_info thread"
+					" quitting\n");
+			return NULL;
+		}
+		pthread_create(&th, NULL, client_handler, (void *)(uintptr_t)s);
+		pthread_detach(th);
+	}
+	return NULL;
+}
+
+static inline char *
+get_socket_path(const char *runtime_dir)
+{
+	static char path[PATH_MAX];
+
+	if (strlen(runtime_dir) == 0)
+		snprintf(path, sizeof(path), "/tmp/dpdk_process_info.%d",
+				getpid());
+	else
+		snprintf(path, sizeof(path), "%s/process_info.%d",
+				runtime_dir, getpid());
+	return path;
+}
+
+static void
+unlink_socket(void)
+{
+	if (sun.sun_path[0])
+		unlink(sun.sun_path);
+}
+
+int
+rte_process_info_init(const char *runtime_dir, const char **err_str)
+{
+	pthread_t t;
+
+	sock = socket(AF_UNIX, SOCK_SEQPACKET, 0);
+	if (sock < 0) {
+		snprintf(process_info_log_error, sizeof(process_info_log_error),
+				"Error with socket creation, %s",
+				strerror(errno));
+		*err_str = process_info_log_error;
+		return -1;
+	}
+
+	sun.sun_family = AF_UNIX;
+	if (strlcpy(sun.sun_path, get_socket_path(runtime_dir),
+			sizeof(sun.sun_path)) >= sizeof(sun.sun_path)) {
+		snprintf(process_info_log_error, sizeof(process_info_log_error),
+				"Error with socket binding, path too long");
+		goto error;
+	}
+	if (bind(sock, (void *)&sun, sizeof(sun)) < 0) {
+		snprintf(process_info_log_error, sizeof(process_info_log_error),
+				"Error binding socket: %s",
+				strerror(errno));
+		sun.sun_path[0] = 0;
+		goto error;
+	}
+
+	if (listen(sock, 1) < 0) {
+		snprintf(process_info_log_error, sizeof(process_info_log_error),
+				"Error calling listen for socket: %s",
+				strerror(errno));
+		goto error;
+	}
+
+	pthread_create(&t, NULL, socket_listener, NULL);
+	atexit(unlink_socket);
+
+	return 0;
+
+error:
+	close(sock);
+	unlink_socket();
+	sock = -1;
+	*err_str = process_info_log_error;
+	return -1;
+}
diff --git a/lib/librte_process_info/rte_process_info.h b/lib/librte_process_info/rte_process_info.h
new file mode 100644
index 000000000..d8d86ef57
--- /dev/null
+++ b/lib/librte_process_info/rte_process_info.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Intel Corporation
+ */
+
+#ifndef _RTE_PROCESS_INFO_H_
+#define _RTE_PROCESS_INFO_H_
+
+#include <stdint.h>
+#include <rte_compat.h>
+
+#define PROCESS_INFO_MAX_CALLBACKS 64
+
+/* callback returns json data in buffer, up to buf_len long.
+ * returns length of buffer used on success, negative on error.
+ */
+typedef int (*process_info_cb)(const char *cmd, const char *params,
+		char *buffer, int buf_len);
+
+__rte_experimental
+int rte_process_info_register(const char *cmd, process_info_cb fn);
+
+__rte_experimental
+int rte_process_info_init(const char *runtime_dir, const char **err_str);
+
+#endif
diff --git a/lib/librte_process_info/rte_process_info_version.map b/lib/librte_process_info/rte_process_info_version.map
new file mode 100644
index 000000000..7e6f68d5a
--- /dev/null
+++ b/lib/librte_process_info/rte_process_info_version.map
@@ -0,0 +1,6 @@
+EXPERIMENTAL {
+	global:
+		rte_process_info_init;
+		rte_process_info_register;
+	local: *;
+};
diff --git a/lib/meson.build b/lib/meson.build
index 6ceb5e756..fe11a02c8 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -10,6 +10,7 @@
 # core libs which are widely reused, so their deps are kept to a minimum.
 libraries = [
 	'kvargs', # eal depends on kvargs
+	'process_info', # basic info querying capability about dpdk processes
 	'eal', # everything depends on eal
 	'ring', 'mempool', 'mbuf', 'net', 'meter', 'ethdev', 'pci', # core
 	'cmdline',
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 05ea034b9..3c61cbb67 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -77,6 +77,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH)           += -lrte_hash
 _LDLIBS-$(CONFIG_RTE_LIBRTE_MEMBER)         += -lrte_member
 _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lrte_vhost
 _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)         += -lrte_kvargs
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PROCESS_INFO)   += -lrte_process_info
 _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)           += -lrte_mbuf
 _LDLIBS-$(CONFIG_RTE_LIBRTE_NET)            += -lrte_net
 _LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER)          += -lrte_ethdev
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [RFC 2/6] eal: integrate process-info library
  2019-12-05 17:31 [dpdk-dev] [RFC 0/6] replace telemetry with process_info Ciara Power
  2019-12-05 17:31 ` [dpdk-dev] [RFC 1/6] process-info: introduce process-info library Ciara Power
@ 2019-12-05 17:31 ` Ciara Power
  2019-12-05 17:31 ` [dpdk-dev] [RFC 3/6] usertools: add process-info python script Ciara Power
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Power @ 2019-12-05 17:31 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Ciara Power

From: Bruce Richardson <bruce.richardson@intel.com>

Integrate the process info library into the EAL.
 - Initialize the process-info library as part of EAL init.
   This can be disabled using an EAL parameter.
 - Register commands to provide some basic info from EAL.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 lib/Makefile                               |  2 +-
 lib/librte_eal/common/eal_common_options.c | 75 ++++++++++++++++++++++
 lib/librte_eal/common/eal_internal_cfg.h   |  1 +
 lib/librte_eal/common/eal_options.h        |  5 ++
 lib/librte_eal/freebsd/eal/Makefile        |  1 +
 lib/librte_eal/freebsd/eal/eal.c           | 14 ++++
 lib/librte_eal/linux/eal/Makefile          |  1 +
 lib/librte_eal/linux/eal/eal.c             | 15 +++++
 lib/librte_eal/meson.build                 |  2 +-
 9 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/lib/Makefile b/lib/Makefile
index 466cd04ef..cc61176da 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -6,7 +6,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
 DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs
 DIRS-$(CONFIG_RTE_LIBRTE_PROCESS_INFO) += librte_process_info
 DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal
-DEPDIRS-librte_eal := librte_kvargs
+DEPDIRS-librte_eal := librte_kvargs librte_process_info
 DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci
 DEPDIRS-librte_pci := librte_eal
 DIRS-$(CONFIG_RTE_LIBRTE_RING) += librte_ring
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index a7f9c5f9b..0839a523d 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -25,6 +25,7 @@
 #include <rte_version.h>
 #include <rte_devargs.h>
 #include <rte_memcpy.h>
+#include <rte_process_info.h>
 
 #include "eal_internal_cfg.h"
 #include "eal_options.h"
@@ -82,6 +83,7 @@ eal_long_options[] = {
 	{OPT_LEGACY_MEM,        0, NULL, OPT_LEGACY_MEM_NUM       },
 	{OPT_SINGLE_FILE_SEGMENTS, 0, NULL, OPT_SINGLE_FILE_SEGMENTS_NUM},
 	{OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
+	{OPT_NO_PROCESS_INFO,   0, NULL, OPT_NO_PROCESS_INFO_NUM  },
 	{0,                     0, NULL, 0                        }
 };
 
@@ -126,6 +128,75 @@ static int master_lcore_parsed;
 static int mem_parsed;
 static int core_parsed;
 
+static char **eal_args;
+static char **eal_app_args;
+
+#define EAL_PARAM_REQ "/eal:params"
+#define EAL_APP_PARAM_REQ "/eal:app_params"
+
+/* callback handler for process_info library to report out EAL flags */
+int
+handle_eal_info_request(const char *cmd, const char *params __rte_unused,
+		char *buffer, int buf_len)
+{
+	char **args;
+	int used = 0;
+	int i = 0;
+
+	if (strcmp(cmd, EAL_PARAM_REQ) == 0)
+		args = eal_args;
+	else if (strcmp(cmd, EAL_APP_PARAM_REQ) == 0)
+		args = eal_app_args;
+	else /* version */
+		return snprintf(buffer, buf_len, "\"%s\"", rte_version());
+
+	if (args == NULL || args[0] == NULL)
+		return snprintf(buffer, buf_len, "[]"); /* empty list */
+
+	used = strlcpy(buffer, "[", buf_len);
+	while (args[i] != NULL)
+		used += snprintf(buffer + used, buf_len - used, "\"%s\",",
+				args[i++]);
+	buffer[used - 1] = ']';
+	return used;
+}
+
+int
+eal_save_args(int argc, char **argv)
+{
+	int i, j;
+
+	/* clone argv to report out later. We overprovision, but
+	 * this does not waste huge amounts of memory
+	 */
+	eal_args = calloc(argc + 1, sizeof(*eal_args));
+	if (eal_args == NULL)
+		return -1;
+
+	for (i = 0; i < argc; i++) {
+		eal_args[i] = strdup(argv[i]);
+		if (strcmp(argv[i], "--") == 0)
+			break;
+	}
+	eal_args[i++] = NULL; /* always finish with NULL */
+	rte_process_info_register(EAL_PARAM_REQ, handle_eal_info_request);
+
+	/* allow reporting of any app args we know about too */
+	if (i == argc)
+		return 0;
+
+	eal_app_args = calloc(argc - i + 1, sizeof(*eal_args));
+	if (eal_app_args == NULL)
+		return -1;
+
+	for (j = 0; i < argc; j++, i++)
+		eal_app_args[j] = strdup(argv[i]);
+	eal_app_args[j] = NULL;
+	rte_process_info_register(EAL_APP_PARAM_REQ, handle_eal_info_request);
+
+	return 0;
+}
+
 static int
 eal_option_device_add(enum rte_devtype type, const char *optarg)
 {
@@ -1446,6 +1517,9 @@ eal_parse_common_option(int opt, const char *optarg,
 			return -1;
 		}
 		break;
+	case OPT_NO_PROCESS_INFO_NUM:
+		conf->no_process_info = 1;
+		break;
 
 	/* don't know what to do, leave this to caller */
 	default:
@@ -1687,6 +1761,7 @@ eal_common_usage(void)
 	       "  --"OPT_IN_MEMORY"   Operate entirely in memory. This will\n"
 	       "                      disable secondary process support\n"
 	       "  --"OPT_BASE_VIRTADDR"     Base virtual address\n"
+	       "  --"OPT_NO_PROCESS_INFO"   Disable process info\n"
 	       "\nEAL options for DEBUG use only:\n"
 	       "  --"OPT_HUGE_UNLINK"       Unlink hugepage files after init\n"
 	       "  --"OPT_NO_HUGE"           Use malloc instead of hugetlbfs\n"
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index a42f34923..c8226cb44 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -82,6 +82,7 @@ struct internal_config {
 	rte_cpuset_t ctrl_cpuset;         /**< cpuset for ctrl threads */
 	volatile unsigned int init_complete;
 	/**< indicates whether EAL has completed initialization */
+	unsigned int no_process_info;   /**< true to disable Process Info */
 };
 extern struct internal_config internal_config; /**< Global EAL configuration. */
 
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index 9855429e5..0692666ef 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -69,6 +69,8 @@ enum {
 	OPT_IOVA_MODE_NUM,
 #define OPT_MATCH_ALLOCATIONS  "match-allocations"
 	OPT_MATCH_ALLOCATIONS_NUM,
+#define OPT_NO_PROCESS_INFO    "no-process-info"
+	OPT_NO_PROCESS_INFO_NUM,
 	OPT_LONG_MAX_NUM
 };
 
@@ -84,5 +86,8 @@ int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
 enum rte_proc_type_t eal_proc_type_detect(void);
 int eal_plugins_init(void);
+int eal_save_args(int argc, char **argv);
+int handle_eal_info_request(const char *cmd, const char *params __rte_unused,
+		char *buffer, int buf_len);
 
 #endif /* EAL_OPTIONS_H */
diff --git a/lib/librte_eal/freebsd/eal/Makefile b/lib/librte_eal/freebsd/eal/Makefile
index b160b5790..3fd0d5edd 100644
--- a/lib/librte_eal/freebsd/eal/Makefile
+++ b/lib/librte_eal/freebsd/eal/Makefile
@@ -19,6 +19,7 @@ LDLIBS += -lexecinfo
 LDLIBS += -lpthread
 LDLIBS += -lgcc_s
 LDLIBS += -lrte_kvargs
+LDLIBS += -lrte_process_info
 
 EXPORT_MAP := ../../rte_eal_version.map
 
diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index 6ae37e7e6..6d52b035f 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -44,6 +44,7 @@
 #include <rte_option.h>
 #include <rte_atomic.h>
 #include <malloc_heap.h>
+#include <rte_process_info.h>
 
 #include "eal_private.h"
 #include "eal_thread.h"
@@ -724,6 +725,9 @@ rte_eal_init(int argc, char **argv)
 
 	eal_reset_internal_config(&internal_config);
 
+	/* clone argv to report out later in telemetry */
+	eal_save_args(argc, argv);
+
 	/* set log level as early as possible */
 	eal_log_level_parse(argc, argv);
 
@@ -952,6 +956,16 @@ rte_eal_init(int argc, char **argv)
 		rte_eal_init_alert("Cannot clear runtime directory\n");
 		return -1;
 	}
+	if (!internal_config.no_process_info)  {
+		const char *error_str;
+		if (rte_process_info_init(rte_eal_get_runtime_dir(),
+				&error_str) != 0) {
+			rte_eal_init_alert(error_str);
+			return -1;
+		}
+		rte_process_info_register("/eal:version",
+				handle_eal_info_request);
+	}
 
 	eal_mcfg_complete();
 
diff --git a/lib/librte_eal/linux/eal/Makefile b/lib/librte_eal/linux/eal/Makefile
index e70cf104a..d09d2b4d0 100644
--- a/lib/librte_eal/linux/eal/Makefile
+++ b/lib/librte_eal/linux/eal/Makefile
@@ -23,6 +23,7 @@ LDLIBS += -lpthread
 LDLIBS += -lgcc_s
 LDLIBS += -lrt
 LDLIBS += -lrte_kvargs
+LDLIBS += -lrte_process_info
 ifeq ($(CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES),y)
 LDLIBS += -lnuma
 endif
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index c4233ec3c..e191be6e0 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -50,6 +50,7 @@
 #include <malloc_heap.h>
 #include <rte_vfio.h>
 #include <rte_option.h>
+#include <rte_process_info.h>
 
 #include "eal_private.h"
 #include "eal_thread.h"
@@ -987,6 +988,9 @@ rte_eal_init(int argc, char **argv)
 
 	eal_reset_internal_config(&internal_config);
 
+	/* clone argv to report out later in telemetry */
+	eal_save_args(argc, argv);
+
 	/* set log level as early as possible */
 	eal_log_level_parse(argc, argv);
 
@@ -1291,6 +1295,17 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
+	if (!internal_config.no_process_info)  {
+		const char *error_str;
+		if (rte_process_info_init(rte_eal_get_runtime_dir(),
+				&error_str) != 0) {
+			rte_eal_init_alert(error_str);
+			return -1;
+		}
+		rte_process_info_register("/eal:version",
+				handle_eal_info_request);
+	}
+
 	eal_mcfg_complete();
 
 	/* Call each registered callback, if enabled */
diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
index 4be5118ce..89ee3b6b8 100644
--- a/lib/librte_eal/meson.build
+++ b/lib/librte_eal/meson.build
@@ -13,7 +13,7 @@ dpdk_conf.set('RTE_EXEC_ENV_' + exec_env.to_upper(), 1)
 subdir(exec_env + '/eal')
 
 allow_experimental_apis = true
-deps += 'kvargs'
+deps += ['kvargs', 'process_info']
 if dpdk_conf.has('RTE_USE_LIBBSD')
 	ext_deps += libbsd
 endif
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [RFC 3/6] usertools: add process-info python script
  2019-12-05 17:31 [dpdk-dev] [RFC 0/6] replace telemetry with process_info Ciara Power
  2019-12-05 17:31 ` [dpdk-dev] [RFC 1/6] process-info: introduce process-info library Ciara Power
  2019-12-05 17:31 ` [dpdk-dev] [RFC 2/6] eal: integrate " Ciara Power
@ 2019-12-05 17:31 ` Ciara Power
  2019-12-05 17:31 ` [dpdk-dev] [RFC 4/6] ethdev: add callback support for process-info Ciara Power
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Power @ 2019-12-05 17:31 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Ciara Power

From: Bruce Richardson <bruce.richardson@intel.com>

This patch adds a python script that can be used with the process_info
library. It connects as a client to the process_info socket, and allows
the user send a command and see the JSON response.

The example usage below shows the script connecting to the process_info
socket, and sending two basic commands entered by the user. The response
for each command is shown below the user input.

Connecting to /var/run/dpdk/rte/process_info.22103
--> /
{"/":["/","/eal:app_params","/eal:params","/eal:version"]}

--> /eal:version
{"/eal:version":"DPDK 20.02.0-rc0"}

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 usertools/test_new_telemetry.py | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100755 usertools/test_new_telemetry.py

diff --git a/usertools/test_new_telemetry.py b/usertools/test_new_telemetry.py
new file mode 100755
index 000000000..84243ffae
--- /dev/null
+++ b/usertools/test_new_telemetry.py
@@ -0,0 +1,28 @@
+#! /usr/bin/python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2019 Intel Corporation
+
+import socket
+import os
+import sys
+import time
+import glob
+
+def handle_socket(path):
+    print("Connecting to " + path)
+    try:
+        fd.connect(path)
+    except OSError:
+        return
+    text = input('--> ')
+    while (text != "quit"):
+        fd.send(text.encode())
+        print(fd.recv(4096).decode())
+        text = input('--> ')
+    fd.close()
+
+fd = socket.socket(socket.AF_UNIX, socket.SOCK_SEQPACKET)
+for f in glob.glob('/var/run/dpdk/*/process_info.*'):
+  handle_socket(f)
+for f in glob.glob('/run/user/%d/dpdk/*/process_info.*' % os.getuid()):
+  handle_socket(f)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [RFC 4/6] ethdev: add callback support for process-info
  2019-12-05 17:31 [dpdk-dev] [RFC 0/6] replace telemetry with process_info Ciara Power
                   ` (2 preceding siblings ...)
  2019-12-05 17:31 ` [dpdk-dev] [RFC 3/6] usertools: add process-info python script Ciara Power
@ 2019-12-05 17:31 ` Ciara Power
  2019-12-05 17:31 ` [dpdk-dev] [RFC 5/6] rawdev: " Ciara Power
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Power @ 2019-12-05 17:31 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Ciara Power

From: Bruce Richardson <bruce.richardson@intel.com>

The ethdev library now registers commands with process_info, and
implements the callback functions. These commands allow the list of
ethdev ports and the stats for a port to be queried.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 lib/librte_ethdev/Makefile     |  2 +-
 lib/librte_ethdev/rte_ethdev.c | 73 ++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile
index b627e4e23..f586f83de 100644
--- a/lib/librte_ethdev/Makefile
+++ b/lib/librte_ethdev/Makefile
@@ -12,7 +12,7 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 LDLIBS += -lrte_net -lrte_eal -lrte_mempool -lrte_ring
-LDLIBS += -lrte_mbuf -lrte_kvargs -lrte_meter
+LDLIBS += -lrte_mbuf -lrte_kvargs -lrte_meter -lrte_process_info
 
 EXPORT_MAP := rte_ethdev_version.map
 
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 6e9cb243e..b0b5ceec5 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -38,6 +38,7 @@
 #include <rte_kvargs.h>
 #include <rte_class.h>
 #include <rte_ether.h>
+#include <rte_process_info.h>
 
 #include "rte_ethdev.h"
 #include "rte_ethdev_driver.h"
@@ -5190,9 +5191,81 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
 	return result;
 }
 
+static int
+handle_port_list(const char *cmd __rte_unused,
+		const char *params __rte_unused,
+		char *buffer, int buf_len)
+{
+	int used = 0;
+	int port_id;
+
+	used = strlcpy(buffer, "[", buf_len);
+	RTE_ETH_FOREACH_DEV(port_id)
+		used += snprintf(buffer + used, buf_len - used, "%d,", port_id);
+	buffer[used - 1] = ']';
+	return used;
+}
+
+static int
+handle_port_stats(const char *cmd __rte_unused,
+		const char *params,
+		char *buffer, int buf_len)
+{
+	struct rte_eth_xstat *eth_xstats;
+	struct rte_eth_xstat_name *xstat_names;
+	int port_id, num_xstats;
+	int i, ret;
+	int used = 0;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	port_id = atoi(params);
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return -1;
+
+	used = strlcpy(buffer, "{", buf_len);
+
+	num_xstats = rte_eth_xstats_get(port_id, NULL, 0);
+	if (num_xstats < 0)
+		return -1;
+
+	/* use one malloc for both names and stats */
+	eth_xstats = malloc((sizeof(struct rte_eth_xstat) +
+			sizeof(struct rte_eth_xstat_name)) * num_xstats);
+	if (eth_xstats == NULL)
+		return -1;
+	xstat_names = (void *)&eth_xstats[num_xstats];
+
+	ret = rte_eth_xstats_get_names(port_id, xstat_names, num_xstats);
+	if (ret < 0 || ret > num_xstats) {
+		free(eth_xstats);
+		return -1;
+	}
+
+	ret = rte_eth_xstats_get(port_id, eth_xstats, num_xstats);
+	if (ret < 0 || ret > num_xstats) {
+		free(eth_xstats);
+		return -1;
+	}
+
+	for (i = 0; i < num_xstats; i++) {
+		ret = snprintf(buffer + used, buf_len - used, "\"%s\":%"PRIu64",",
+				xstat_names[i].name, eth_xstats[i].value);
+		if (ret + used >= buf_len)
+			break;
+		used += ret;
+	}
+
+	buffer[used - 1] = '}';
+	return used;
+}
+
 RTE_INIT(ethdev_init_log)
 {
 	rte_eth_dev_logtype = rte_log_register("lib.ethdev");
 	if (rte_eth_dev_logtype >= 0)
 		rte_log_set_level(rte_eth_dev_logtype, RTE_LOG_INFO);
+	rte_process_info_register("/ethdev:list", handle_port_list);
+	rte_process_info_register("/ethdev:stats", handle_port_stats);
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [RFC 5/6] rawdev: add callback support for process-info
  2019-12-05 17:31 [dpdk-dev] [RFC 0/6] replace telemetry with process_info Ciara Power
                   ` (3 preceding siblings ...)
  2019-12-05 17:31 ` [dpdk-dev] [RFC 4/6] ethdev: add callback support for process-info Ciara Power
@ 2019-12-05 17:31 ` Ciara Power
  2019-12-05 17:31 ` [dpdk-dev] [RFC 6/6] examples/l3fwd-power: enable use of process-info Ciara Power
  2020-02-05 15:21 ` [dpdk-dev] [RFC 0/6] replace telemetry with process_info David Marchand
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Power @ 2019-12-05 17:31 UTC (permalink / raw)
  To: dev; +Cc: Ciara Power, Bruce Richardson

The rawdev library now registers commands with process_info, and
implements the corresponding callback functions. These allow a list of
rawdev devices and stats for a rawdev port to be queried.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 lib/librte_rawdev/Makefile     |  3 +-
 lib/librte_rawdev/meson.build  |  1 +
 lib/librte_rawdev/rte_rawdev.c | 82 ++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/lib/librte_rawdev/Makefile b/lib/librte_rawdev/Makefile
index 7dd1197dc..6e1d4ea05 100644
--- a/lib/librte_rawdev/Makefile
+++ b/lib/librte_rawdev/Makefile
@@ -9,7 +9,8 @@ LIB = librte_rawdev.a
 # build flags
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
-LDLIBS += -lrte_eal
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+LDLIBS += -lrte_eal -lrte_process_info
 
 # library source files
 SRCS-y += rte_rawdev.c
diff --git a/lib/librte_rawdev/meson.build b/lib/librte_rawdev/meson.build
index a20fbdc04..dcd37ad49 100644
--- a/lib/librte_rawdev/meson.build
+++ b/lib/librte_rawdev/meson.build
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2018 Intel Corporation
 
+allow_experimental_apis = true
 sources = files('rte_rawdev.c')
 headers = files('rte_rawdev.h', 'rte_rawdev_pmd.h')
diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c
index b6f1e1c77..477e12af6 100644
--- a/lib/librte_rawdev/rte_rawdev.c
+++ b/lib/librte_rawdev/rte_rawdev.c
@@ -29,6 +29,7 @@
 #include <rte_common.h>
 #include <rte_malloc.h>
 #include <rte_errno.h>
+#include <rte_process_info.h>
 
 #include "rte_rawdev.h"
 #include "rte_rawdev_pmd.h"
@@ -544,9 +545,90 @@ rte_rawdev_pmd_release(struct rte_rawdev *rawdev)
 	return 0;
 }
 
+static int
+handle_dev_list(const char *cmd __rte_unused,
+		const char *params __rte_unused,
+		char *buffer, int buf_len)
+{
+	int used = 0;
+	int i;
+
+	used = strlcpy(buffer, "[", buf_len);
+	for (i = 0; i < rawdev_globals.nb_devs; i++)
+		if (rte_rawdevices[i].attached == RTE_RAWDEV_ATTACHED)
+			used += snprintf(buffer + used, buf_len - used, "%d,",
+				rte_rawdevices[i].dev_id);
+
+	buffer[used - 1] = ']';
+	return used;
+}
+
+static int
+handle_dev_stats(const char *cmd __rte_unused,
+		const char *params,
+		char *buffer, int buf_len)
+{
+	uint64_t *rawdev_xstats;
+	struct rte_rawdev_xstats_name *xstat_names;
+	int dev_id, num_xstats, i, ret;
+	int used = 0;
+	unsigned int *ids;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	dev_id = atoi(params);
+	if (!rte_rawdev_pmd_is_valid_dev(dev_id))
+		return -1;
+
+	used = strlcpy(buffer, "{", buf_len);
+
+	num_xstats = xstats_get_count(dev_id);
+	if (num_xstats < 0)
+		return -1;
+
+	/* use one malloc for names, stats and ids */
+	rawdev_xstats = malloc((sizeof(uint64_t) +
+			sizeof(struct rte_rawdev_xstats_name) +
+			sizeof(unsigned int)) * num_xstats);
+	if (rawdev_xstats == NULL)
+		return -1;
+	xstat_names = (void *)&rawdev_xstats[num_xstats];
+	ids = (void *)&xstat_names[num_xstats];
+
+	ret = rte_rawdev_xstats_names_get(dev_id, xstat_names, num_xstats);
+	if (ret < 0 || ret > num_xstats) {
+		free(rawdev_xstats);
+		return -1;
+	}
+
+	for (i = 0; i < num_xstats; i++)
+		ids[i] = i;
+
+	ret = rte_rawdev_xstats_get(dev_id, ids, rawdev_xstats, num_xstats);
+	if (ret < 0 || ret > num_xstats) {
+		free(rawdev_xstats);
+		return -1;
+	}
+
+	for (i = 0; i < num_xstats; i++) {
+		ret = snprintf(buffer + used, buf_len - used, "\"%s\":%"PRIu64",",
+				xstat_names[i].name, rawdev_xstats[i]);
+		if (ret + used >= buf_len)
+			break;
+		used += ret;
+	}
+
+	buffer[used - 1] = '}';
+	free(rawdev_xstats);
+	return used;
+}
+
 RTE_INIT(librawdev_init_log)
 {
 	librawdev_logtype = rte_log_register("lib.rawdev");
 	if (librawdev_logtype >= 0)
 		rte_log_set_level(librawdev_logtype, RTE_LOG_INFO);
+	rte_process_info_register("/rawdev:list", handle_dev_list);
+	rte_process_info_register("/rawdev:stats", handle_dev_stats);
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [RFC 6/6] examples/l3fwd-power: enable use of process-info
  2019-12-05 17:31 [dpdk-dev] [RFC 0/6] replace telemetry with process_info Ciara Power
                   ` (4 preceding siblings ...)
  2019-12-05 17:31 ` [dpdk-dev] [RFC 5/6] rawdev: " Ciara Power
@ 2019-12-05 17:31 ` Ciara Power
  2020-02-05 15:21 ` [dpdk-dev] [RFC 0/6] replace telemetry with process_info David Marchand
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Power @ 2019-12-05 17:31 UTC (permalink / raw)
  To: dev; +Cc: Ciara Power, Bruce Richardson

The l3fwd-power example app now registers a stats command with
process_info, and provides a callback function to handle formatting the
power stats.

Using the process-info library in this app replaces the previous method
of making l3fwd-power stats available in the metrics library, for use
with telemetry.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 examples/l3fwd-power/main.c | 83 +++++++++++--------------------------
 1 file changed, 25 insertions(+), 58 deletions(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index d049d8a5d..939b3dd88 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -45,7 +45,7 @@
 #include <rte_power.h>
 #include <rte_spinlock.h>
 #include <rte_power_empty_poll.h>
-#include <rte_metrics.h>
+#include <rte_process_info.h>
 
 #include "perf_core.h"
 #include "main.h"
@@ -131,7 +131,7 @@
 #define EMPTY_POLL_MED_THRESHOLD 350000UL
 #define EMPTY_POLL_HGH_THRESHOLD 580000UL
 
-
+#define NUM_TELSTATS RTE_DIM(telstats_strings)
 
 static uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT;
 static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
@@ -154,11 +154,6 @@ volatile bool quit_signal;
 static struct  ep_params *ep_params;
 static struct  ep_policy policy;
 static long  ep_med_edpi, ep_hgh_edpi;
-/* timer to update telemetry every 500ms */
-static struct rte_timer telemetry_timer;
-
-/* stats index returned by metrics lib */
-int telstats_index;
 
 struct telstats_name {
 	char name[RTE_ETH_XSTATS_NAME_SIZE];
@@ -187,9 +182,6 @@ enum busy_rate {
 #define MIN_CYCLES  1500000ULL
 #define MAX_CYCLES 22000000ULL
 
-/* (500ms) */
-#define TELEMETRY_INTERVALS_PER_SEC 2
-
 static int parse_ptype; /**< Parse packet type using rx callback, and */
 			/**< disabled by default */
 
@@ -2087,17 +2079,21 @@ init_power_library(void)
 	}
 	return ret;
 }
-static void
-update_telemetry(__attribute__((unused)) struct rte_timer *tim,
-		__attribute__((unused)) void *arg)
+
+static int
+handle_app_stats(const char *cmd __rte_unused,
+		const char *params __rte_unused,
+		char *buffer, int buf_len)
 {
 	unsigned int lcore_id = rte_lcore_id();
 	struct lcore_conf *qconf;
 	uint64_t app_eps = 0, app_fps = 0, app_br = 0;
-	uint64_t values[3] = {0};
-	int ret;
+	uint64_t values[NUM_TELSTATS];
+	int ret, used = 0;
+	uint32_t i;
 	uint64_t count = 0;
 
+	used = strlcpy(buffer, "{", buf_len);
 	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
 		qconf = &lcore_conf[lcore_id];
 		if (qconf->n_rx_queue == 0)
@@ -2114,32 +2110,21 @@ update_telemetry(__attribute__((unused)) struct rte_timer *tim,
 		values[0] = app_eps/count;
 		values[1] = app_fps/count;
 		values[2] = app_br/count;
-	} else {
-		values[0] = 0;
-		values[1] = 0;
-		values[2] = 0;
+	} else
+		memset(values, 0, sizeof(uint64_t) * NUM_TELSTATS);
+
+	for (i = 0; i < NUM_TELSTATS; i++) {
+		ret = snprintf(buffer + used, buf_len - used, "\"%s\":%"PRIu64",",
+				telstats_strings[i].name, values[i]);
+		if (ret + used >= buf_len)
+			break;
+		used += ret;
 	}
 
-	ret = rte_metrics_update_values(RTE_METRICS_GLOBAL, telstats_index,
-					values, RTE_DIM(values));
-	if (ret < 0)
-		RTE_LOG(WARNING, POWER, "failed to update metrcis\n");
+	buffer[used - 1] = '}';
+	return used;
 }
-static void
-telemetry_setup_timer(void)
-{
-	int lcore_id = rte_lcore_id();
-	uint64_t hz = rte_get_timer_hz();
-	uint64_t ticks;
 
-	ticks = hz / TELEMETRY_INTERVALS_PER_SEC;
-	rte_timer_reset_sync(&telemetry_timer,
-			ticks,
-			PERIODICAL,
-			lcore_id,
-			update_telemetry,
-			NULL);
-}
 static void
 empty_poll_setup_timer(void)
 {
@@ -2176,8 +2161,6 @@ launch_timer(unsigned int lcore_id)
 
 	if (app_mode == APP_MODE_EMPTY_POLL)
 		empty_poll_setup_timer();
-	else
-		telemetry_setup_timer();
 
 	cycles_10ms = rte_get_timer_hz() / 100;
 
@@ -2196,7 +2179,6 @@ launch_timer(unsigned int lcore_id)
 	return 0;
 }
 
-
 int
 main(int argc, char **argv)
 {
@@ -2212,8 +2194,6 @@ main(int argc, char **argv)
 	uint32_t dev_rxq_num, dev_txq_num;
 	uint8_t nb_rx_queue, queue, socketid;
 	uint16_t portid;
-	uint8_t num_telstats = RTE_DIM(telstats_strings);
-	const char *ptr_strings[num_telstats];
 
 	/* catch SIGINT and restore cpufreq governor to ondemand */
 	signal(SIGINT, signal_exit_now);
@@ -2507,29 +2487,16 @@ main(int argc, char **argv)
 		rte_eal_mp_remote_launch(main_empty_poll_loop, NULL,
 				SKIP_MASTER);
 	} else {
-		unsigned int i;
-
-		/* Init metrics library */
-		rte_metrics_init(rte_socket_id());
-		/** Register stats with metrics library */
-		for (i = 0; i < num_telstats; i++)
-			ptr_strings[i] = telstats_strings[i].name;
-
-		ret = rte_metrics_reg_names(ptr_strings, num_telstats);
-		if (ret >= 0)
-			telstats_index = ret;
-		else
-			rte_exit(EXIT_FAILURE, "failed to register metrics names");
-
 		RTE_LCORE_FOREACH_SLAVE(lcore_id) {
 			rte_spinlock_init(&stats[lcore_id].telemetry_lock);
 		}
-		rte_timer_init(&telemetry_timer);
+		rte_process_info_register("/l3fwd-power:stats",
+				handle_app_stats);
 		rte_eal_mp_remote_launch(main_telemetry_loop, NULL,
 						SKIP_MASTER);
 	}
 
-	if (app_mode == APP_MODE_EMPTY_POLL || app_mode == APP_MODE_TELEMETRY)
+	if (app_mode == APP_MODE_EMPTY_POLL)
 		launch_timer(rte_lcore_id());
 
 	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC 0/6] replace telemetry with process_info
  2019-12-05 17:31 [dpdk-dev] [RFC 0/6] replace telemetry with process_info Ciara Power
                   ` (5 preceding siblings ...)
  2019-12-05 17:31 ` [dpdk-dev] [RFC 6/6] examples/l3fwd-power: enable use of process-info Ciara Power
@ 2020-02-05 15:21 ` David Marchand
  2020-02-05 17:12   ` Bruce Richardson
  6 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2020-02-05 15:21 UTC (permalink / raw)
  To: Ciara Power, Bruce Richardson; +Cc: dev, Emma Foley

Hello Ciara, Bruce,

On Thu, Dec 5, 2019 at 6:34 PM Ciara Power <ciara.power@intel.com> wrote:
>
> From: Bruce Richardson <bruce.richardson@intel.com>
>
> This patchset proposes a new library, called "process-info" for now, to
> replace the existing telemetry library in DPDK. (Name subject to change
> if someone can propose a better one).
>
> The existing telemetry library provides useful capabilities if used:
>   - Creates a unix socket on the system to allow external programs
>     connect and gather stats about the DPDK process.
>   - Supports outputting the xstats for various network cards on the
>     system.
>   - Can be used to output any other information exported to the metrics
>     library, e.g. by applications.
>   - Uses JSON message format, which is widely supported by other
>     languages and systems.
>   - Is supported by a plugin to collectd.
>
> However, the library also has some issues and limitations that could be
> improved upon:
>   - Has a dependency on libjansson for JSON processing, so is disabled
>     by default.
>   - Tied entirely to the metrics library for statistics.
>   - No support for sending non-stats data, e.g. something as simple as
>     DPDK version string.
>   - All data gathering functions are in the library itself, which also
>     means…
>   - No support for libraries or drivers to present their own
>     information via the library.
>
> We therefore propose to keep the good points of the existing library,
> but change the way things work to rectify the downsides.
> This leads to the following design choices in the library:
>   - Keep the existing idea of using a unix socket for connection (just
>     simplifying the connection handling).
>   - We would like to use JSON format, where possible, but the jansson
>     library dependency is problematic. However, creating JSON-encoded
>     data is easier than trying to parse JSON in C code, so we can keep
>     the JSON output format for processing by e.g. collectd and other
>     tools, while simplifying the input to be plain text commands:
>         - Commands in this RFC are designed to all start with "/" for
>           consistency
>         - Any parameters to the commands, e.g. the specific port to get
>           stats for, are placed after a comma ","
>   - Have the library only handle socket creation and input handling.
>     All data gathering should be provided by functions external to the
>     library registered by other components, e.g. have ethdev library
>     provide the function to query NIC xstats, etc.
>   - Have the library directly initialized by EAL by default, so that
>     unless an app explicitly does not want the support, monitoring is
>     available on all DPDK apps.
>
> The obvious question that remains to be answered here is: "why a new
> library rather than just fixing the old one?"
>
> The answer is that we have conflicts between the final two design
> choices above, which require that the library be built early in the
> build as other libraries will call into it to register callbacks, and
> the desire to keep backward compatibility e.g. for use with collectd
> plugin, which requires the existing library code be kept around and
> built - as it is now - at the end of the build process since it calls
> into other DPDK libraries. We therefore cannot have one library that
> meets both requirements, hence the replacement which allows us to
> maintain backward compatibility by just leaving the old lib in place
> until e.g. 20.11.
>
> This is also why the new library is called "process_info", since the
> name "telemetry" is already taken. Suggestions for a better name
> welcome.

The only user of the rte_telemetry api I could find is the (not yet
merged [1]) dpdk collectd plugin.

How will this impact it?
Can we expect compatibility?


1: https://github.com/collectd/collectd/pull/3273

-- 
David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC 0/6] replace telemetry with process_info
  2020-02-05 15:21 ` [dpdk-dev] [RFC 0/6] replace telemetry with process_info David Marchand
@ 2020-02-05 17:12   ` Bruce Richardson
  0 siblings, 0 replies; 9+ messages in thread
From: Bruce Richardson @ 2020-02-05 17:12 UTC (permalink / raw)
  To: David Marchand; +Cc: Ciara Power, dev, Emma Foley

On Wed, Feb 05, 2020 at 04:21:16PM +0100, David Marchand wrote:
> Hello Ciara, Bruce,
> 
> On Thu, Dec 5, 2019 at 6:34 PM Ciara Power <ciara.power@intel.com> wrote:
> >
> > From: Bruce Richardson <bruce.richardson@intel.com>
> >
> > This patchset proposes a new library, called "process-info" for now, to
> > replace the existing telemetry library in DPDK. (Name subject to change
> > if someone can propose a better one).
> >
> > The existing telemetry library provides useful capabilities if used:
> >   - Creates a unix socket on the system to allow external programs
> >     connect and gather stats about the DPDK process.
> >   - Supports outputting the xstats for various network cards on the
> >     system.
> >   - Can be used to output any other information exported to the metrics
> >     library, e.g. by applications.
> >   - Uses JSON message format, which is widely supported by other
> >     languages and systems.
> >   - Is supported by a plugin to collectd.
> >
> > However, the library also has some issues and limitations that could be
> > improved upon:
> >   - Has a dependency on libjansson for JSON processing, so is disabled
> >     by default.
> >   - Tied entirely to the metrics library for statistics.
> >   - No support for sending non-stats data, e.g. something as simple as
> >     DPDK version string.
> >   - All data gathering functions are in the library itself, which also
> >     means…
> >   - No support for libraries or drivers to present their own
> >     information via the library.
> >
> > We therefore propose to keep the good points of the existing library,
> > but change the way things work to rectify the downsides.
> > This leads to the following design choices in the library:
> >   - Keep the existing idea of using a unix socket for connection (just
> >     simplifying the connection handling).
> >   - We would like to use JSON format, where possible, but the jansson
> >     library dependency is problematic. However, creating JSON-encoded
> >     data is easier than trying to parse JSON in C code, so we can keep
> >     the JSON output format for processing by e.g. collectd and other
> >     tools, while simplifying the input to be plain text commands:
> >         - Commands in this RFC are designed to all start with "/" for
> >           consistency
> >         - Any parameters to the commands, e.g. the specific port to get
> >           stats for, are placed after a comma ","
> >   - Have the library only handle socket creation and input handling.
> >     All data gathering should be provided by functions external to the
> >     library registered by other components, e.g. have ethdev library
> >     provide the function to query NIC xstats, etc.
> >   - Have the library directly initialized by EAL by default, so that
> >     unless an app explicitly does not want the support, monitoring is
> >     available on all DPDK apps.
> >
> > The obvious question that remains to be answered here is: "why a new
> > library rather than just fixing the old one?"
> >
> > The answer is that we have conflicts between the final two design
> > choices above, which require that the library be built early in the
> > build as other libraries will call into it to register callbacks, and
> > the desire to keep backward compatibility e.g. for use with collectd
> > plugin, which requires the existing library code be kept around and
> > built - as it is now - at the end of the build process since it calls
> > into other DPDK libraries. We therefore cannot have one library that
> > meets both requirements, hence the replacement which allows us to
> > maintain backward compatibility by just leaving the old lib in place
> > until e.g. 20.11.
> >
> > This is also why the new library is called "process_info", since the
> > name "telemetry" is already taken. Suggestions for a better name
> > welcome.
> 
> The only user of the rte_telemetry api I could find is the (not yet
> merged [1]) dpdk collectd plugin.
> 
> How will this impact it?
> Can we expect compatibility?
> 
> 
> 1: https://github.com/collectd/collectd/pull/3273
> 
Yes, we are aware of this, and we are investigating compatibility options.
Hopefully, we'll have more on this to share in 20.05 timeframe, as we do
more prototyping and investigation.

/Bruce

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-02-05 17:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-05 17:31 [dpdk-dev] [RFC 0/6] replace telemetry with process_info Ciara Power
2019-12-05 17:31 ` [dpdk-dev] [RFC 1/6] process-info: introduce process-info library Ciara Power
2019-12-05 17:31 ` [dpdk-dev] [RFC 2/6] eal: integrate " Ciara Power
2019-12-05 17:31 ` [dpdk-dev] [RFC 3/6] usertools: add process-info python script Ciara Power
2019-12-05 17:31 ` [dpdk-dev] [RFC 4/6] ethdev: add callback support for process-info Ciara Power
2019-12-05 17:31 ` [dpdk-dev] [RFC 5/6] rawdev: " Ciara Power
2019-12-05 17:31 ` [dpdk-dev] [RFC 6/6] examples/l3fwd-power: enable use of process-info Ciara Power
2020-02-05 15:21 ` [dpdk-dev] [RFC 0/6] replace telemetry with process_info David Marchand
2020-02-05 17:12   ` Bruce Richardson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).