On 2020-03-18 20:02, jer...@marvell.com wrote:
> From: Jerin Jacob <jer...@marvell.com>
>
> Define eal_trace_init() and eal_trace_fini() EAL interface
> functions that rte_eal_init() and rte_eal_cleanup() function can be
> use to initialize and finalize the trace subsystem.
> eal_trace_init() function will add the following
> functionality if trace is enabled through EAL command line param.
>
> - Test for trace registration failure.
> - Test for duplicate trace name registration
> - Generate UUID ver 4.
> - Create a trace directory
>
> Signed-off-by: Jerin Jacob <jer...@marvell.com>
> Signed-off-by: Sunil Kumar Kori <sk...@marvell.com>
> ---
> lib/librte_eal/common/eal_common_trace.c | 54 ++++++
> .../common/eal_common_trace_utils.c | 173 ++++++++++++++++++
> lib/librte_eal/common/eal_trace.h | 21 +++
> lib/librte_eal/common/meson.build | 1 +
> lib/librte_eal/freebsd/eal/Makefile | 1 +
> lib/librte_eal/linux/eal/Makefile | 1 +
> 6 files changed, 251 insertions(+)
> create mode 100644 lib/librte_eal/common/eal_common_trace_utils.c
>
> diff --git a/lib/librte_eal/common/eal_common_trace.c
> b/lib/librte_eal/common/eal_common_trace.c
> index f8855e09b..abb221cf3 100644
> --- a/lib/librte_eal/common/eal_common_trace.c
> +++ b/lib/librte_eal/common/eal_common_trace.c
> @@ -22,6 +22,60 @@ RTE_DEFINE_PER_LCORE(int, ctf_count);
> static struct trace_point_head tp_list = STAILQ_HEAD_INITIALIZER(tp_list);
> static struct trace trace;
>
> +struct trace*
> +trace_obj_get(void)
> +{
> + return &trace;
> +}
> +
> +struct trace_point_head *
> +trace_list_head_get(void)
> +{
> + return &tp_list;
> +}
> +
> +int
> +eal_trace_init(void)
> +{
> + /* One of the Trace registration failed */
> + if (trace.register_errno) {
> + rte_errno = trace.register_errno;
> + goto fail;
> + }
> +
> + if (rte_trace_global_is_disabled())
> + return 0;
> +
> + rte_spinlock_init(&trace.lock);
> +
> + /* Is duplicate trace name registered */
> + if (trace_has_duplicate_entry())
> + goto fail;
> +
> + /* Generate UUID ver 4 with total size of events and number of events */
> + trace_uuid_generate();
> +
> + /* Create trace directory */
> + if (trace_mkdir())
> + goto fail;
> +
> +
> + rte_trace_global_mode_set(trace.mode);
> +
> + return 0;
> +
> +fail:
> + trace_err("failed to initialize trace [%s]", rte_strerror(rte_errno));
> + return -rte_errno;
> +}
> +
> +void
> +eal_trace_fini(void)
> +{
> + if (rte_trace_global_is_disabled())
> + return;
> +}
> +
> bool
> rte_trace_global_is_enabled(void)
> {
> diff --git a/lib/librte_eal/common/eal_common_trace_utils.c
> b/lib/librte_eal/common/eal_common_trace_utils.c
> new file mode 100644
> index 000000000..f7d59774c
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_common_trace_utils.c
> @@ -0,0 +1,173 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2020 Marvell International Ltd.
> + */
> +
> +#include <fnmatch.h>
> +#include <pwd.h>
> +#include <sys/stat.h>
> +#include <time.h>
> +
> +#include <rte_common.h>
> +#include <rte_errno.h>
> +#include <rte_string_fns.h>
> +
> +#include "eal_filesystem.h"
> +#include "eal_trace.h"
> +
> +static bool
> +trace_entry_compare(const char *name)
> +{
> + struct trace_point_head *tp_list = trace_list_head_get();
> + struct trace_point *tp;
> + int count = 0;
> +
> + STAILQ_FOREACH(tp, tp_list, next) {
> + if (strncmp(tp->name, name, TRACE_POINT_NAME_SIZE) == 0)
> + count++;
> + if (count > 1) {
> + trace_err("found duplicate entry %s", name);
> + rte_errno = EEXIST;
> + return 1;
Maybe an assertion would be better here, especially since the caller
doesn't seem to care about the fact the list is inconsistent.
Change 1 -> true.
> + }
> + }
> + return 0;
return false;
> +}
> +
> +bool
> +trace_has_duplicate_entry(void)
> +{
> + struct trace_point_head *tp_list = trace_list_head_get();
> + struct trace_point *tp;
> +
> + /* Is duplicate trace name registered */
> + STAILQ_FOREACH(tp, tp_list, next)
> + if (trace_entry_compare(tp->name))
> + return true;
> +
> + return false;
> +}
> +
> +void
> +trace_uuid_generate(void)
> +{
> + struct trace_point_head *tp_list = trace_list_head_get();
> + struct trace *trace = trace_obj_get();
> + struct trace_point *tp;
> + uint64_t sz_total = 0;
> +
> + /* Go over the registered trace points to get total size of events */
> + STAILQ_FOREACH(tp, tp_list, next) {
> + const uint16_t sz = *tp->handle & __RTE_TRACE_FIELD_SIZE_MASK;
> + sz_total += sz;
> + }
> +
> + rte_uuid_t uuid = RTE_UUID_INIT(sz_total, trace->nb_trace_points,
> + 0x4370, 0x8f50, 0x222ddd514176ULL);
> + rte_uuid_copy(trace->uuid, uuid);
> +}
> +
> +static int
> +trace_session_name_generate(char *trace_dir)
> +{
> + struct tm *tm_result;
> + time_t tm;
> + int rc;
> +
> + tm = time(NULL);
> + if ((int)tm == -1)
> + goto fail;
> +
> + tm_result = localtime(&tm);
> + if (tm_result == NULL)
> + goto fail;
> +
> + rc = rte_strscpy(trace_dir,
> + eal_get_hugefile_prefix(), TRACE_PREFIX_LEN);
> + if (rc == -E2BIG)
> + rc = TRACE_PREFIX_LEN;
> + trace_dir[rc++] = '-';
> +
> + rc = strftime(trace_dir + rc, TRACE_DIR_STR_LEN - rc,
> + "%Y-%m-%d-%p-%I-%M-%S", tm_result);
> + if (rc == 0)
> + goto fail;
> +
> + return rc;
> +fail:
> + rte_errno = errno;
> + return -rte_errno;
> +}
> +
> +static int
> +trace_dir_default_path_get(char *dir_path)
> +{
> + struct trace *trace = trace_obj_get();
> + uint32_t size = sizeof(trace->dir);
> + struct passwd *pwd;
> + char *home_dir;
> +
> + /* First check for shell environment variable */
> + home_dir = getenv("HOME");
In case of the application being started with 'sudo', $HOME will not be
"/root", but rather the original user. This sounds like a bug to me.
> + if (home_dir == NULL) {
> + /* Fallback to password file entry */
> + pwd = getpwuid(getuid());
> + if (pwd == NULL)
> + return -EINVAL;
> +
> + home_dir = pwd->pw_dir;
> + }
> +
> + /* Append dpdk-traces to directory */
> + if (snprintf(dir_path, size, "%s/dpdk-traces/", home_dir) < 0)
> + return -ENAMETOOLONG;
> +
> + return 0;
> +}
> +
> +int
> +trace_mkdir(void)
> +{
> + struct trace *trace = trace_obj_get();
> + char session[TRACE_DIR_STR_LEN];
> + char *dir_path;
> + int rc;
> +
> + if (!trace->dir_offset) {
> + dir_path = (char *)calloc(1, sizeof(trace->dir));
calloc() returns a void pointer, so no need to cast it.
> + if (dir_path == NULL) {
> + trace_err("fail to allocate memory\n");
> + return -ENOMEM;
> + }
> +
> + rc = trace_dir_default_path_get(dir_path);
> + if (rc < 0) {
> + trace_err("fail to get default path\n");
> + free(dir_path);
> + return rc;
> + }
> +
> + }
> +
> + /* Create the path if it t exist, no "mkdir -p" available here */
> + rc = mkdir(trace->dir, 0700);
> + if (rc < 0 && errno != EEXIST) {
> + trace_err("mkdir %s failed [%s]", trace->dir, strerror(errno));
> + rte_errno = errno;
> + return -rte_errno;
> + }
> +
> + rc = trace_session_name_generate(session);
> + if (rc < 0)
> + return rc;
> +
> + rc = mkdir(trace->dir, 0700);
> + if (rc < 0) {
> + trace_err("mkdir %s failed [%s]", trace->dir, strerror(errno));
> + rte_errno = errno;
> + return -rte_errno;
> + }
> +
> + RTE_LOG(INFO, EAL, "Trace dir: %s\n", trace->dir);
> + return 0;
Is dir_path leaked? I can't find a free(), except in one of the error cases.
> +}
> +
> diff --git a/lib/librte_eal/common/eal_trace.h
> b/lib/librte_eal/common/eal_trace.h
> index 9f90a5d17..10c2f03ac 100644
> --- a/lib/librte_eal/common/eal_trace.h
> +++ b/lib/librte_eal/common/eal_trace.h
> @@ -5,7 +5,9 @@
> #ifndef __EAL_TRACE_H
> #define __EAL_TRACE_H
>
> +#include <rte_spinlock.h>
> #include <rte_trace.h>
> +#include <rte_uuid.h>
>
> #define trace_err(fmt, args...)\
> RTE_LOG(ERR, EAL, "%s():%u " fmt "\n",\
> @@ -15,6 +17,8 @@
> RTE_LOG(CRIT, EAL, "%s():%u " fmt "\n",\
> __func__, __LINE__, ## args)
>
> +#define TRACE_PREFIX_LEN 12
> +#define TRACE_DIR_STR_LEN (sizeof("YYYY-mm-dd-AM-HH-MM-SS") +
> TRACE_PREFIX_LEN)
> #define TRACE_CTF_FIELD_SIZE 384
> #define TRACE_POINT_NAME_SIZE 64
>
> @@ -26,11 +30,15 @@ struct trace_point {
> };
>
> struct trace {
> + char dir[PATH_MAX];
> + int dir_offset;
> int register_errno;
> bool global_status;
> enum rte_trace_mode_e mode;
> + rte_uuid_t uuid;
> uint32_t level;
> uint32_t nb_trace_points;
> + rte_spinlock_t lock;
> };
>
> /* Helper functions */
> @@ -41,7 +49,20 @@ trace_id_get(rte_trace_t trace)
> __RTE_TRACE_FIELD_ID_SHIFT;
> }
>
> +/* Trace object functions */
> +struct trace *trace_obj_get(void);
> +
> /* Trace point list functions */
> STAILQ_HEAD(trace_point_head, trace_point);
> +struct trace_point_head *trace_list_head_get(void);
> +
> +/* Util functions */
> +bool trace_has_duplicate_entry(void);
> +void trace_uuid_generate(void);
> +int trace_mkdir(void);
> +
> +/* EAL interface */
> +int eal_trace_init(void);
> +void eal_trace_fini(void);
>
> #endif /* __EAL_TRACE_H */
> diff --git a/lib/librte_eal/common/meson.build
> b/lib/librte_eal/common/meson.build
> index 88c14ebe5..716a255d2 100644
> --- a/lib/librte_eal/common/meson.build
> +++ b/lib/librte_eal/common/meson.build
> @@ -29,6 +29,7 @@ common_sources = files(
> 'eal_common_thread.c',
> 'eal_common_timer.c',
> 'eal_common_trace.c',
> + 'eal_common_trace_utils.c',
> 'eal_common_uuid.c',
> 'hotplug_mp.c',
> 'malloc_elem.c',
> diff --git a/lib/librte_eal/freebsd/eal/Makefile
> b/lib/librte_eal/freebsd/eal/Makefile
> index b2fcc4212..8c444da02 100644
> --- a/lib/librte_eal/freebsd/eal/Makefile
> +++ b/lib/librte_eal/freebsd/eal/Makefile
> @@ -61,6 +61,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += eal_common_proc.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += eal_common_fbarray.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += eal_common_uuid.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += eal_common_trace.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += eal_common_trace_utils.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_malloc.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += hotplug_mp.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += malloc_elem.c
> diff --git a/lib/librte_eal/linux/eal/Makefile
> b/lib/librte_eal/linux/eal/Makefile
> index 95470d3bb..bd9993e58 100644
> --- a/lib/librte_eal/linux/eal/Makefile
> +++ b/lib/librte_eal/linux/eal/Makefile
> @@ -69,6 +69,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += eal_common_proc.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += eal_common_fbarray.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += eal_common_uuid.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += eal_common_trace.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += eal_common_trace_utils.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_malloc.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += hotplug_mp.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += malloc_elem.c
