On 6/27/2024 8:38 AM, Serhii Iliushyk wrote:
> Add ntnic specific implementation for logging
>
> Signed-off-by: Serhii Iliushyk <sil-...@napatech.com>
> ---
> drivers/net/ntnic/meson.build | 2 +
> drivers/net/ntnic/ntlog/include/ntlog.h | 49 +++++++++++++++++++++++
> drivers/net/ntnic/ntlog/ntlog.c | 53 +++++++++++++++++++++++++
> drivers/net/ntnic/ntnic_ethdev.c | 2 +
> 4 files changed, 106 insertions(+)
> create mode 100644 drivers/net/ntnic/ntlog/include/ntlog.h
> create mode 100644 drivers/net/ntnic/ntlog/ntlog.c
>
> diff --git a/drivers/net/ntnic/meson.build b/drivers/net/ntnic/meson.build
> index 227949eacb..b1ba8a860f 100644
> --- a/drivers/net/ntnic/meson.build
> +++ b/drivers/net/ntnic/meson.build
> @@ -15,10 +15,12 @@ cflags += [
> # includes
> includes = [
> include_directories('.'),
> + include_directories('ntlog/include'),
>
Why have 'include' folder under 'ntlog', but not put log related headers
directly under 'ntlog' folder?
> ]
>
> # all sources
> sources = files(
> + 'ntlog/ntlog.c',
> 'ntnic_ethdev.c',
> )
> # END
> diff --git a/drivers/net/ntnic/ntlog/include/ntlog.h
> b/drivers/net/ntnic/ntlog/include/ntlog.h
> new file mode 100644
> index 0000000000..58dcce0580
> --- /dev/null
> +++ b/drivers/net/ntnic/ntlog/include/ntlog.h
> @@ -0,0 +1,49 @@
> +/*
> + * SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2023 Napatech A/S
> + */
> +
> +#ifndef NTOSS_SYSTEM_NTLOG_H
> +#define NTOSS_SYSTEM_NTLOG_H
> +
> +#include <stdarg.h>
> +#include <stdint.h>
> +#include <rte_log.h>
> +
> +extern int nt_logtype;
> +
> +#define NT_DRIVER_NAME "ntnic"
> +
> +#define NT_PMD_DRV_LOG(level, ...) \
> + rte_log(RTE_LOG_ ## level, nt_logtype, \
> + RTE_FMT(NT_DRIVER_NAME ": " \
> + RTE_FMT_HEAD(__VA_ARGS__, ""), \
> + RTE_FMT_TAIL(__VA_ARGS__, "")))
> +
> +
> +#define NT_LOG_ERR(...) NT_PMD_DRV_LOG(ERR, __VA_ARGS__)
> +#define NT_LOG_WRN(...) NT_PMD_DRV_LOG(WARNING, __VA_ARGS__)
> +#define NT_LOG_INF(...) NT_PMD_DRV_LOG(INFO, __VA_ARGS__)
> +#define NT_LOG_DBG(...) NT_PMD_DRV_LOG(DEBUG, __VA_ARGS__)
> +
> +#define NT_LOG(level, module, ...) \
> + NT_LOG_##level(#module ": " #level ":" __VA_ARGS__)
> +
> +#define NT_LOG_DBGX(level, module, ...) \
> + rte_log(RTE_LOG_ ##level, nt_logtype, \
> + RTE_FMT(NT_DRIVER_NAME #module ": [%s:%u]" \
> + RTE_FMT_HEAD(__VA_ARGS__, ""),
> __func__, __LINE__, \
> + RTE_FMT_TAIL(__VA_ARGS__, "")))
> +/*
> + * nt log helper functions
> + * to create a string for NT_LOG usage to output a one-liner log
> + * to use when one single function call to NT_LOG is not optimal - that is
> + * you do not know the number of parameters at programming time or it is
> variable
> + */
> +char *ntlog_helper_str_alloc(const char *sinit);
> +
> +void ntlog_helper_str_add(char *s, const char *format, ...);
> +
> +void ntlog_helper_str_free(char *s);
> +
> +#endif /* NTOSS_SYSTEM_NTLOG_H */
> diff --git a/drivers/net/ntnic/ntlog/ntlog.c b/drivers/net/ntnic/ntlog/ntlog.c
> new file mode 100644
> index 0000000000..2732a9e857
> --- /dev/null
> +++ b/drivers/net/ntnic/ntlog/ntlog.c
> @@ -0,0 +1,53 @@
> +/*
> + * SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2023 Napatech A/S
> + */
> +
> +#include "ntlog.h"
> +
> +#include <stdarg.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +
> +#include <rte_log.h>
> +#include <rte_string_fns.h>
> +
> +#define NTLOG_HELPER_STR_SIZE_MAX (1024)
> +
> +RTE_LOG_REGISTER_DEFAULT(nt_logtype, INFO)
> +
>
Mostly default log type is 'NOTICE' to reduce the noise. Unless there is
justification, can you please set default log to notice for consistency.
> +char *ntlog_helper_str_alloc(const char *sinit)
> +{
> + char *s = malloc(NTLOG_HELPER_STR_SIZE_MAX);
> +
> + if (!s)
> + return NULL;
> +
> + if (sinit)
> + snprintf(s, NTLOG_HELPER_STR_SIZE_MAX, "%s", sinit);
> +
>
Should it put a space after 'sinit'?
One sample of usage is like:
ntlog_helper_str_alloc("Register::read");
ntlog_helper_str_add(tmp_string, "(Dev: %s, ....
Won't this cause an output like:
"Register::read(Dev: ..."
Is this what intended, or is "Register::read (Dev: ..." intended?
<...>
