Re: [dpdk-dev] [PATCH v2] log: support custom log function

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

On Sat,  6 Feb 2021 01:42:04 +0800, Li Feng wrote:
> Currently, the dpdk log is out to stdout/stderr and syslog.
> The rte_openlog_stream could set an external FILE* stream, but it asks the
> consumer to give it a FILE* pointer.
> For C++ or other languages, it's hard to get a libc FILE*.
> 
> Support to set a hook method is another choice for this scenario.

It's "DPDK"; and "hard" sounds vague. I'd state the real issue such that
API to change DPDK log sink mandates the (direct) use of libc in consumer
application. To invoke arbitrary function, consumers have to use facilities
that are non-standard (fopencookie) or OS-specific (pipes).

> 
> Change-Id: I3b2419cc2fe5256205daa8077fd8862a8e58fb6c

What is this?

> Signed-off-by: Li Feng <fengli@smartx.com>
> ---
> v2: simplify the code.
> 
>  lib/librte_eal/include/rte_eal.h | 16 ++++++++++++++++
>  lib/librte_eal/linux/eal_log.c   | 10 ++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> index eaf6469e5..bd6cf554b 100644
> --- a/lib/librte_eal/include/rte_eal.h
> +++ b/lib/librte_eal/include/rte_eal.h
> @@ -501,6 +501,22 @@ rte_eal_mbuf_user_pool_ops(void);
>  const char *
>  rte_eal_get_runtime_dir(void);
>  
> +/**
> + * Usage function typedef used by the application usage function.

It's unrelated to the following typedef purpose, is it?

> + *
> + * Use this function typedef to define a logger formatter.
> + */
> +typedef cookie_write_function_t rte_log_func_t;

"cookie_write_function_t" is not standard C, please write this type
explicitly. POSIX reserves "_t" suffix, "rte_" prefix is enough.

> +
> +/**
> + * Set a customized logger.
> + *
> + * @param logf
> + *   The customized logger function.
> + */
> +void
> +rte_eal_set_log_func(rte_log_func_t *logf);
> +

How about rte_log_sink_set() and a companion rte_log_sink_get() or returning
the previous value when setting a new one? Also, like in fopencookie(), a
cookie argument is needed.

New functions must be marker __rte_experimental and added to .map and .def
files. Please use devtools/checkpatches.sh that will run checks like that for
your commits.

>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/librte_eal/linux/eal_log.c b/lib/librte_eal/linux/eal_log.c
> index 43c8460bf..c0a7a12ab 100644
> --- a/lib/librte_eal/linux/eal_log.c
> +++ b/lib/librte_eal/linux/eal_log.c
> @@ -60,3 +60,13 @@ rte_eal_log_init(const char *id, int facility)
>  
>  	return 0;
>  }
> +
> +/*
> + * Set the customized logger, it will override the default action, which is
> + * writing to syslog and stdout.
> + */
> +void
> +rte_eal_set_log_func(rte_log_func_t *logf)
> +{
> +    console_log_func.write = logf;
> +}