On Thursday 23 August 2018 05:38 PM, Ciara Power wrote:
This patch adds the infrastructure and initial code for the
telemetry library.
A virtual device is used for telemetry, which is included in
this patch. To use telemetry, a command-line argument must be
used - "--vdev=telemetry".
Control threads are used to get CPU cycles for telemetry, which
are configured in this patch also.
Signed-off-by: Ciara Power <ciara.po...@intel.com>
Signed-off-by: Brian Archbold <brian.archb...@intel.com>
---
[...]
/rte_pmd_telemetry_version.map
new file mode 100644
index 0000000..a73e0f2
--- /dev/null
+++ b/drivers/telemetry/telemetry/rte_pmd_telemetry_version.map
@@ -0,0 +1,9 @@
+DPDK_18.05 {
^^^^^
I think you want 18.11 here.
+ global:
+
+ telemetry_probe;
+ telemetry_remove;
+
+ local: *;
+
+};
[...]
diff --git a/lib/Makefile b/lib/Makefile
index afa604e..8cbd035 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -105,6 +105,8 @@ DEPDIRS-librte_gso := librte_eal librte_mbuf librte_ethdev
librte_net
DEPDIRS-librte_gso += librte_mempool
DIRS-$(CONFIG_RTE_LIBRTE_BPF) += librte_bpf
DEPDIRS-librte_bpf := librte_eal librte_mempool librte_mbuf librte_ethdev
+DIRS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += librte_telemetry
+DEPDIRS-librte_telemetry := librte_eal librte_metrics librte_ethdev
ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
diff --git a/lib/librte_telemetry/Makefile b/lib/librte_telemetry/Makefile
new file mode 100644
index 0000000..bda3788
--- /dev/null
+++ b/lib/librte_telemetry/Makefile
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_telemetry.a
+
+CFLAGS += -O3
+CFLAGS += -I$(SRCDIR)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
+LDLIBS += -lrte_eal -lrte_ethdev
+LDLIBS += -lrte_metrics
+
+EXPORT_MAP := rte_telemetry_version.map
+
+LIBABIVER := 1
+
+# library source files
+SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) := rte_telemetry.c
+
+# export include files
+SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include := rte_telemetry.h
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_telemetry/meson.build b/lib/librte_telemetry/meson.build
new file mode 100644
index 0000000..7716076
--- /dev/null
+++ b/lib/librte_telemetry/meson.build
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+sources = files('rte_telemetry.c')
+headers = files('rte_telemetry.h', 'rte_telemetry_internal.h')
+deps += ['metrics', 'ethdev']
+cflags += '-DALLOW_EXPERIMENTAL_API'
diff --git a/lib/librte_telemetry/rte_telemetry.c
b/lib/librte_telemetry/rte_telemetry.c
new file mode 100644
index 0000000..8d7b0e3
--- /dev/null
+++ b/lib/librte_telemetry/rte_telemetry.c
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include <unistd.h>
+
+#include <rte_eal.h>
+#include <rte_ethdev.h>
+#include <rte_metrics.h>
+
+#include "rte_telemetry.h"
+#include "rte_telemetry_internal.h"
+
+#define SLEEP_TIME 10
+
+static telemetry_impl *static_telemetry;
+
+static int32_t
+rte_telemetry_run(void *userdata)
+{
+ struct telemetry_impl *telemetry = (struct telemetry_impl *)userdata;
+ if (!telemetry) {
+ TELEMETRY_LOG_WARN("Warning - TELEMETRY could not be "
+ "initialised\n");
Your 'TELEMETRY_LOG_WARNING' already includes a '\n' in its definition.
This would add another one. Can you re-check?
+ return -1;
+ }
+
+ return 0;
+}
+
+static void
+*rte_telemetry_run_thread_func(void *userdata)
+{
+ int ret;
+ struct telemetry_impl *telemetry = (struct telemetry_impl *)userdata;
+
+ if (!telemetry) {
+ TELEMETRY_LOG_ERR("Error - %s passed a NULL instance\n",
+ __func__);
I might be picky - but this is an internal function spawned using
rte_ctrl_thread_create which already has a check whether the argument
(static_telemetry) is NULL or not. So, this is like duplicating that work.
+ pthread_exit(0);
+ }
+
+ while (telemetry->thread_status) {
+ rte_telemetry_run(telemetry);
+ ret = usleep(SLEEP_TIME);
+ if (ret < 0)
+ TELEMETRY_LOG_ERR("Error - Calling thread could not be"
+ " put to sleep\n");
If the calling thread couldn't be put to sleep, you would continue
looping without sleeping? Wouldn't that simply hog your CPU? Or, is that
expected design?
+ }
+ pthread_exit(0);
+}
+
+int32_t
+rte_telemetry_init(uint32_t socket_id)
+{
+ int ret;
+ pthread_attr_t attr;
+ const char *telemetry_ctrl_thread = "telemetry";
+
+ if (static_telemetry) {
+ TELEMETRY_LOG_WARN("Warning - TELEMETRY structure already "
+ "initialised\n");
+ return -EALREADY;
+ }
+
+ static_telemetry = calloc(1, sizeof(struct telemetry_impl));
+ if (!static_telemetry) {
+ TELEMETRY_LOG_ERR("Error - Memory could not be allocated\n");
+ return -ENOMEM;
+ }
+
+ static_telemetry->socket_id = socket_id;
+ rte_metrics_init(static_telemetry->socket_id);
+ pthread_attr_init(&attr);
+ ret = rte_ctrl_thread_create(&static_telemetry->thread_id,
+ telemetry_ctrl_thread, &attr, rte_telemetry_run_thread_func,
+ (void *)static_telemetry);
+ static_telemetry->thread_status = 1;
+ if (ret < 0) {
+ ret = rte_telemetry_cleanup();
+ if (ret < 0)
+ TELEMETRY_LOG_ERR("Error - TELEMETRY cleanup failed\n");
+ return -EPERM;
+ }
+ return 0;
+}
+
+int32_t
+rte_telemetry_cleanup(void)
+{
+ struct telemetry_impl *telemetry = static_telemetry;
+ telemetry->thread_status = 0;
+ pthread_join(telemetry->thread_id, NULL);
+ free(telemetry);
+ static_telemetry = NULL;
+ return 0;
Maybe if you could use be a little more liberal with new lines, it would
be slightly easier to read.
But again, that is my personal opinion.
+}
+
+int telemetry_log_level;
+RTE_INIT(rte_telemetry_log_init);
+
+static void
+rte_telemetry_log_init(void)
+{
+ telemetry_log_level = rte_log_register("lib.telemetry");
+ if (telemetry_log_level >= 0)
+ rte_log_set_level(telemetry_log_level, RTE_LOG_ERR);
+}
[...]
+#endif
diff --git a/lib/librte_telemetry/rte_telemetry_version.map
b/lib/librte_telemetry/rte_telemetry_version.map
new file mode 100644
index 0000000..efd437d
--- /dev/null
+++ b/lib/librte_telemetry/rte_telemetry_version.map
@@ -0,0 +1,6 @@
+DPDK_18.05 {
^^^^^^
I think you want it to be 18.11 here.
+ global:
+
+ rte_telemetry_init;
+ local: *;
+};
[...]
