diff mbox series

log: support custom log function

Message ID 20210205112433.1681853-1-fengli@smartx.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series log: support custom log function |

Checks

Context Check Description
ci/Intel-compilation fail Compilation issues
ci/intel-Testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot warning Travis build: failed
ci/iol-testing fail Testing issues
ci/checkpatch warning coding style issues

Commit Message

Li Feng Feb. 5, 2021, 11:24 a.m. UTC 
Currently, the dpdk log is out to stdout/stderr and syslog.
We should support to output the log to another please, e.g. file or
glog.

Signed-off-by: Li Feng <fengli@smartx.com>
---
 lib/librte_eal/common/eal_private.h | 10 ----------
 lib/librte_eal/include/rte_eal.h    | 22 ++++++++++++++++++++++
 lib/librte_eal/linux/eal.c          |  2 +-
 lib/librte_eal/linux/eal_log.c      | 13 ++++++++-----
 lib/librte_eal/windows/eal.c        |  2 +-
 lib/librte_eal/windows/eal_log.c    |  2 +-
 6 files changed, 33 insertions(+), 18 deletions(-)

Comments

Feng Li Feb. 5, 2021, 11:31 a.m. UTC | #1 
Li Feng <fengli@smartx.com> 于2021年2月5日周五 下午7:24写道：
>
> Currently, the dpdk log is out to stdout/stderr and syslog.
> We should support to output the log to another please, e.g. file or
> glog.
Sorry, sed 's/please/place/g'.
>
> Signed-off-by: Li Feng <fengli@smartx.com>
> ---
>  lib/librte_eal/common/eal_private.h | 10 ----------
>  lib/librte_eal/include/rte_eal.h    | 22 ++++++++++++++++++++++
>  lib/librte_eal/linux/eal.c          |  2 +-
>  lib/librte_eal/linux/eal_log.c      | 13 ++++++++-----
>  lib/librte_eal/windows/eal.c        |  2 +-
>  lib/librte_eal/windows/eal_log.c    |  2 +-
>  6 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index 4684c4c7d..8cfb399b8 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -143,16 +143,6 @@ int rte_eal_memory_init(void);
>   */
>  int rte_eal_timer_init(void);
>
> -/**
> - * Init the default log stream
> - *
> - * This function is private to EAL.
> - *
> - * @return
> - *   0 on success, negative on error
> - */
> -int rte_eal_log_init(const char *id, int facility);
> -
>  /**
>   * Save the log regexp for later
>   */
> diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> index eaf6469e5..f07d89f5f 100644
> --- a/lib/librte_eal/include/rte_eal.h
> +++ b/lib/librte_eal/include/rte_eal.h
> @@ -114,6 +114,28 @@ int rte_eal_iopl_init(void);
>   */
>  int rte_eal_init(int argc, char **argv);
>
> +/**
> + * Usage function typedef used by the application usage function.
> + *
> + * Use this function typedef to define a logger formatter.
> + */
> +typedef cookie_write_function_t rte_log_func_t;
> +
> +/**
> + * Init the default log stream
> + *
> + * @param id
> + *   The openlog's first argument.
> + * @param facility
> + *   The openlog's third argument.
> + * @param logf
> + *   The customized logger function, if it's set, the id and facility will
> + *   be ignored.
> + * @return
> + *   0 on success, negative on error
> + */
> +int rte_eal_log_init(const char *id, int facility, rte_log_func_t *logf);
> +
>  /**
>   * Clean up the Environment Abstraction Layer (EAL)
>   *
> diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
> index 32b48c3de..62747b3a7 100644
> --- a/lib/librte_eal/linux/eal.c
> +++ b/lib/librte_eal/linux/eal.c
> @@ -1160,7 +1160,7 @@ rte_eal_init(int argc, char **argv)
>  #endif
>         }
>
> -       if (rte_eal_log_init(logid, internal_conf->syslog_facility) < 0) {
> +       if (rte_eal_log_init(logid, internal_conf->syslog_facility, NULL) < 0) {
>                 rte_eal_init_alert("Cannot init logging.");
>                 rte_errno = ENOMEM;
>                 __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> diff --git a/lib/librte_eal/linux/eal_log.c b/lib/librte_eal/linux/eal_log.c
> index 43c8460bf..f985b3d36 100644
> --- a/lib/librte_eal/linux/eal_log.c
> +++ b/lib/librte_eal/linux/eal_log.c
> @@ -37,18 +37,21 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)
>         return ret;
>  }
>
> -static cookie_io_functions_t console_log_func = {
> -       .write = console_log_write,
> -};
> -
>  /*
>   * set the log to default function, called during eal init process,
>   * once memzones are available.
>   */
>  int
> -rte_eal_log_init(const char *id, int facility)
> +rte_eal_log_init(const char *id, int facility, rte_log_func_t *logf)
>  {
>         FILE *log_stream;
> +       cookie_io_functions_t console_log_func;
> +
> +       if (logf) {
> +               console_log_func.write = logf;
> +       } else {
> +               console_log_func.write = console_log_write;
> +       }
>
>         log_stream = fopencookie(NULL, "w+", console_log_func);
>         if (log_stream == NULL)
> diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c
> index 1e5f6576f..74fdd076a 100644
> --- a/lib/librte_eal/windows/eal.c
> +++ b/lib/librte_eal/windows/eal.c
> @@ -266,7 +266,7 @@ rte_eal_init(int argc, char **argv)
>                 eal_get_internal_configuration();
>         int ret;
>
> -       rte_eal_log_init(NULL, 0);
> +       rte_eal_log_init(NULL, 0, NULL);
>
>         eal_log_level_parse(argc, argv);
>
> diff --git a/lib/librte_eal/windows/eal_log.c b/lib/librte_eal/windows/eal_log.c
> index 875981f13..31f556908 100644
> --- a/lib/librte_eal/windows/eal_log.c
> +++ b/lib/librte_eal/windows/eal_log.c
> @@ -6,7 +6,7 @@
>
>  /* set the log to default function, called during eal init process. */
>  int
> -rte_eal_log_init(__rte_unused const char *id, __rte_unused int facility)
> +rte_eal_log_init(__rte_unused const char *id, __rte_unused int facility, rte_log_func_t *logf)
>  {
>         rte_openlog_stream(stderr);
>
> --
> 2.29.2
>
David Marchand Feb. 5, 2021, 11:55 a.m. UTC | #2 
On Fri, Feb 5, 2021 at 12:25 PM Li Feng <fengli@smartx.com> wrote:
>
> Currently, the dpdk log is out to stdout/stderr and syslog.
> We should support to output the log to another please, e.g. file or
> glog.

Why not use rte_openlog_stream() / rte_log_get_stream() ?
Feng Li Feb. 5, 2021, 12:22 p.m. UTC | #3 
David Marchand <david.marchand@redhat.com> 于2021年2月5日周五 下午7:56写道：
>
> On Fri, Feb 5, 2021 at 12:25 PM Li Feng <fengli@smartx.com> wrote:
> >
> > Currently, the dpdk log is out to stdout/stderr and syslog.
> > We should support to output the log to another please, e.g. file or
> > glog.
>
> Why not use rte_openlog_stream() / rte_log_get_stream() ?
>
>
> --
> David Marchand
>
rte_openlog_stream seems it could set an external FILE* stream.
However, in the glog or C++ io stream, it's stream like std::cout.
I think it's hard to use a FILE* stream to unify them.

Use a custom logger function may be a better choice.
Dmitry Kozlyuk Feb. 5, 2021, 2:08 p.m. UTC | #4 
On Fri, 5 Feb 2021 20:22:36 +0800, Feng Li wrote:
> David Marchand <david.marchand@redhat.com> 于2021年2月5日周五 下午7:56写道：
> >
> > On Fri, Feb 5, 2021 at 12:25 PM Li Feng <fengli@smartx.com> wrote:  
> > >
> > > Currently, the dpdk log is out to stdout/stderr and syslog.
> > > We should support to output the log to another please, e.g. file or
> > > glog.  
> >
> > Why not use rte_openlog_stream() / rte_log_get_stream() ?
> >
> >
> > --
> > David Marchand
> >  
> rte_openlog_stream seems it could set an external FILE* stream.
> However, in the glog or C++ io stream, it's stream like std::cout.
> I think it's hard to use a FILE* stream to unify them.

Also, FILE* is not a language-agnostic interface. Rust/D/etc consumers can't
use this API unless they obtain a FILE* from a compatible libc.
Stephen Hemminger Feb. 5, 2021, 4:10 p.m. UTC | #5 
On Fri, 5 Feb 2021 12:55:48 +0100
David Marchand <david.marchand@redhat.com> wrote:

> On Fri, Feb 5, 2021 at 12:25 PM Li Feng <fengli@smartx.com> wrote:
> >
> > Currently, the dpdk log is out to stdout/stderr and syslog.
> > We should support to output the log to another please, e.g. file or
> > glog.  
> 
> Why not use rte_openlog_stream() / rte_log_get_stream() ?

Agree, I have used rte_openlog_stream several times to do this.
Stephen Hemminger Feb. 5, 2021, 4:39 p.m. UTC | #6 
On Fri, 5 Feb 2021 20:22:36 +0800
Feng Li <lifeng1519@gmail.com> wrote:

> David Marchand <david.marchand@redhat.com> 于2021年2月5日周五 下午7:56写道：
> >
> > On Fri, Feb 5, 2021 at 12:25 PM Li Feng <fengli@smartx.com> wrote:  
> > >
> > > Currently, the dpdk log is out to stdout/stderr and syslog.
> > > We should support to output the log to another please, e.g. file or
> > > glog.  
> >
> > Why not use rte_openlog_stream() / rte_log_get_stream() ?
> >
> >
> > --
> > David Marchand
> >  
> rte_openlog_stream seems it could set an external FILE* stream.
> However, in the glog or C++ io stream, it's stream like std::cout.
> I think it's hard to use a FILE* stream to unify them.
> 
> Use a custom logger function may be a better choice.

A FILE * stream can be created that uses any other function under
the covers. See fopencookie(3)
Feng Li Feb. 5, 2021, 4:54 p.m. UTC | #7 
Stephen Hemminger <stephen@networkplumber.org> 于2021年2月6日周六 上午12:11写道：
>
> On Fri, 5 Feb 2021 12:55:48 +0100
> David Marchand <david.marchand@redhat.com> wrote:
>
> > On Fri, Feb 5, 2021 at 12:25 PM Li Feng <fengli@smartx.com> wrote:
> > >
> > > Currently, the dpdk log is out to stdout/stderr and syslog.
> > > We should support to output the log to another please, e.g. file or
> > > glog.
> >
> > Why not use rte_openlog_stream() / rte_log_get_stream() ?
>
> Agree, I have used rte_openlog_stream several times to do this.
As Dmitry Kozlyuk said, FILE* is convenient for the C/libc, but not
for other languages.
C++ standard IO stream is the case.
Feng Li Feb. 5, 2021, 5:06 p.m. UTC | #8 
Feng Li <lifeng1519@gmail.com> 于2021年2月6日周六 上午12:54写道：
>
> Stephen Hemminger <stephen@networkplumber.org> 于2021年2月6日周六 上午12:11写道：
> >
> > On Fri, 5 Feb 2021 12:55:48 +0100
> > David Marchand <david.marchand@redhat.com> wrote:
> >
> > > On Fri, Feb 5, 2021 at 12:25 PM Li Feng <fengli@smartx.com> wrote:
> > > >
> > > > Currently, the dpdk log is out to stdout/stderr and syslog.
> > > > We should support to output the log to another please, e.g. file or
> > > > glog.
> > >
> > > Why not use rte_openlog_stream() / rte_log_get_stream() ?
> >
> > Agree, I have used rte_openlog_stream several times to do this.
> As Dmitry Kozlyuk said, FILE* is convenient for the C/libc, but not
> for other languages.
> C++ standard IO stream is the case.

The usage is like this:
| +static ssize_t eal_log_func(void* c, const char* buf, size_t size) {
| +    int glog_level = google::GLOG_INFO;
| +    (void)c;
| +    google::LogMessage(__FILE__, __LINE__, glog_level).stream()
| +        << "[DPDK/" << __FUNCTION__ << "] " << std::string(buf, size);
| +    return 0;
| +}
diff mbox series

Patch

diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 4684c4c7d..8cfb399b8 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -143,16 +143,6 @@  int rte_eal_memory_init(void);
  */
 int rte_eal_timer_init(void);
 
-/**
- * Init the default log stream
- *
- * This function is private to EAL.
- *
- * @return
- *   0 on success, negative on error
- */
-int rte_eal_log_init(const char *id, int facility);
-
 /**
  * Save the log regexp for later
  */
diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
index eaf6469e5..f07d89f5f 100644
--- a/lib/librte_eal/include/rte_eal.h
+++ b/lib/librte_eal/include/rte_eal.h
@@ -114,6 +114,28 @@  int rte_eal_iopl_init(void);
  */
 int rte_eal_init(int argc, char **argv);
 
+/**
+ * Usage function typedef used by the application usage function.
+ *
+ * Use this function typedef to define a logger formatter.
+ */
+typedef cookie_write_function_t rte_log_func_t;
+
+/**
+ * Init the default log stream
+ *
+ * @param id
+ *   The openlog's first argument.
+ * @param facility
+ *   The openlog's third argument.
+ * @param logf
+ *   The customized logger function, if it's set, the id and facility will
+ *   be ignored.
+ * @return
+ *   0 on success, negative on error
+ */
+int rte_eal_log_init(const char *id, int facility, rte_log_func_t *logf);
+
 /**
  * Clean up the Environment Abstraction Layer (EAL)
  *
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 32b48c3de..62747b3a7 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1160,7 +1160,7 @@  rte_eal_init(int argc, char **argv)
 #endif
 	}
 
-	if (rte_eal_log_init(logid, internal_conf->syslog_facility) < 0) {
+	if (rte_eal_log_init(logid, internal_conf->syslog_facility, NULL) < 0) {
 		rte_eal_init_alert("Cannot init logging.");
 		rte_errno = ENOMEM;
 		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
diff --git a/lib/librte_eal/linux/eal_log.c b/lib/librte_eal/linux/eal_log.c
index 43c8460bf..f985b3d36 100644
--- a/lib/librte_eal/linux/eal_log.c
+++ b/lib/librte_eal/linux/eal_log.c
@@ -37,18 +37,21 @@  console_log_write(__rte_unused void *c, const char *buf, size_t size)
 	return ret;
 }
 
-static cookie_io_functions_t console_log_func = {
-	.write = console_log_write,
-};
-
 /*
  * set the log to default function, called during eal init process,
  * once memzones are available.
  */
 int
-rte_eal_log_init(const char *id, int facility)
+rte_eal_log_init(const char *id, int facility, rte_log_func_t *logf)
 {
 	FILE *log_stream;
+	cookie_io_functions_t console_log_func;
+
+	if (logf) {
+		console_log_func.write = logf;
+	} else {
+		console_log_func.write = console_log_write;
+	}
 
 	log_stream = fopencookie(NULL, "w+", console_log_func);
 	if (log_stream == NULL)
diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c
index 1e5f6576f..74fdd076a 100644
--- a/lib/librte_eal/windows/eal.c
+++ b/lib/librte_eal/windows/eal.c
@@ -266,7 +266,7 @@  rte_eal_init(int argc, char **argv)
 		eal_get_internal_configuration();
 	int ret;
 
-	rte_eal_log_init(NULL, 0);
+	rte_eal_log_init(NULL, 0, NULL);
 
 	eal_log_level_parse(argc, argv);
 
diff --git a/lib/librte_eal/windows/eal_log.c b/lib/librte_eal/windows/eal_log.c
index 875981f13..31f556908 100644
--- a/lib/librte_eal/windows/eal_log.c
+++ b/lib/librte_eal/windows/eal_log.c
@@ -6,7 +6,7 @@ 
 
 /* set the log to default function, called during eal init process. */
 int
-rte_eal_log_init(__rte_unused const char *id, __rte_unused int facility)
+rte_eal_log_init(__rte_unused const char *id, __rte_unused int facility, rte_log_func_t *logf)
 {
 	rte_openlog_stream(stderr);