[PATCH bpf-next] libbpf: configure log verbosity with env variable

[PATCH bpf-next] libbpf: configure log verbosity with env variable

[Mykyta]

From: Mykyta Yatsenko <yatsenko@meta.com>

Configure logging verbosity by setting LIBBPF_LOG_LEVEL environment
variable, which is applied only to default logger. Once user set their
custom logging callback, it is up to them to handle filtering.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 Documentation/bpf/libbpf/libbpf_overview.rst |  7 ++++++
 tools/lib/bpf/libbpf.c                       | 24 +++++++++++++++++++-
 tools/lib/bpf/libbpf.h                       |  5 +++-
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/bpf/libbpf/libbpf_overview.rst b/Documentation/bpf/libbpf/libbpf_overview.rst
index f36a2d4ffea2..982dfd71a13d 100644
--- a/Documentation/bpf/libbpf/libbpf_overview.rst
+++ b/Documentation/bpf/libbpf/libbpf_overview.rst
@@ -219,6 +219,13 @@ compilation and skeleton generation. Using Libbpf-rs will make building user
 space part of the BPF application easier. Note that the BPF program themselves
 must still be written in plain C.
 
+libbpf logging
+==============
+
+By default, libbpf logs informational and warning messages to stderr. The verbosity of these
+messages can be controlled by setting the environment variable LIBBPF_LOG_LEVEL to either warn,
+info, or debug. A custom log callback can be set using ``libbpf_set_print()``.
+
 Additional Documentation
 ========================
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5401f2df463d..d0465ca74afc 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -229,7 +229,29 @@ static const char * const prog_type_name[] = {
 static int __base_pr(enum libbpf_print_level level, const char *format,
 		     va_list args)
 {
-	if (level == LIBBPF_DEBUG)
+	static enum libbpf_print_level min_level = LIBBPF_INFO;
+	static const char *env_var = "LIBBPF_LOG_LEVEL";
+	static bool initialized;
+
+	if (!initialized) {
+		char *verbosity;
+
+		initialized = true;
+		verbosity = getenv(env_var);
+		if (verbosity) {
+			if (strcasecmp(verbosity, "warn") == 0)
+				min_level = LIBBPF_WARN;
+			else if (strcasecmp(verbosity, "debug") == 0)
+				min_level = LIBBPF_DEBUG;
+			else if (strcasecmp(verbosity, "info") == 0)
+				min_level = LIBBPF_INFO;
+			else
+				fprintf(stderr, "Unexpected value of %s env variable\n", env_var);
+		}
+	}
+
+	/* if too verbose, skip logging  */
+	if (level > min_level)
 		return 0;
 
 	return vfprintf(stderr, format, args);
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index c3f77d9260fe..26e4e35528c5 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -98,7 +98,10 @@ typedef int (*libbpf_print_fn_t)(enum libbpf_print_level level,
 
 /**
  * @brief **libbpf_set_print()** sets user-provided log callback function to
- * be used for libbpf warnings and informational messages.
+ * be used for libbpf warnings and informational messages. If the user callback
+ * is not set, messages are logged to stderr by default. The verbosity of these
+ * messages can be controlled by setting the environment variable
+ * LIBBPF_LOG_LEVEL to either warn, info, or debug.
  * @param fn The log print function. If NULL, libbpf won't print anything.
  * @return Pointer to old print function.
  *
-- 
2.45.0

Re: [PATCH bpf-next] libbpf: configure log verbosity with env variable

[Andrii Nakryiko]

On Fri, May 24, 2024 at 6:21 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Configure logging verbosity by setting LIBBPF_LOG_LEVEL environment
> variable, which is applied only to default logger. Once user set their
> custom logging callback, it is up to them to handle filtering.
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  Documentation/bpf/libbpf/libbpf_overview.rst |  7 ++++++
>  tools/lib/bpf/libbpf.c                       | 24 +++++++++++++++++++-
>  tools/lib/bpf/libbpf.h                       |  5 +++-
>  3 files changed, 34 insertions(+), 2 deletions(-)
>

I did a few tweaks, mentioned below, and applied to bpf-next. Thanks!

> diff --git a/Documentation/bpf/libbpf/libbpf_overview.rst b/Documentation/bpf/libbpf/libbpf_overview.rst
> index f36a2d4ffea2..982dfd71a13d 100644
> --- a/Documentation/bpf/libbpf/libbpf_overview.rst
> +++ b/Documentation/bpf/libbpf/libbpf_overview.rst
> @@ -219,6 +219,13 @@ compilation and skeleton generation. Using Libbpf-rs will make building user
>  space part of the BPF application easier. Note that the BPF program themselves
>  must still be written in plain C.
>
> +libbpf logging
> +==============
> +
> +By default, libbpf logs informational and warning messages to stderr. The verbosity of these
> +messages can be controlled by setting the environment variable LIBBPF_LOG_LEVEL to either warn,
> +info, or debug. A custom log callback can be set using ``libbpf_set_print()``.
> +

reformatted this to use line length consistent with the rest of the document

>  Additional Documentation
>  ========================
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 5401f2df463d..d0465ca74afc 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -229,7 +229,29 @@ static const char * const prog_type_name[] = {
>  static int __base_pr(enum libbpf_print_level level, const char *format,
>                      va_list args)
>  {
> -       if (level == LIBBPF_DEBUG)
> +       static enum libbpf_print_level min_level = LIBBPF_INFO;
> +       static const char *env_var = "LIBBPF_LOG_LEVEL";

no need for this to be static

> +       static bool initialized;
> +
> +       if (!initialized) {
> +               char *verbosity;
> +
> +               initialized = true;
> +               verbosity = getenv(env_var);
> +               if (verbosity) {
> +                       if (strcasecmp(verbosity, "warn") == 0)
> +                               min_level = LIBBPF_WARN;
> +                       else if (strcasecmp(verbosity, "debug") == 0)
> +                               min_level = LIBBPF_DEBUG;
> +                       else if (strcasecmp(verbosity, "info") == 0)
> +                               min_level = LIBBPF_INFO;
> +                       else
> +                               fprintf(stderr, "Unexpected value of %s env variable\n", env_var);

I've added "libbpf: " prefix and expanded the message with supported values.

> +               }
> +       }
> +
> +       /* if too verbose, skip logging  */
> +       if (level > min_level)
>                 return 0;
>
>         return vfprintf(stderr, format, args);
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index c3f77d9260fe..26e4e35528c5 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -98,7 +98,10 @@ typedef int (*libbpf_print_fn_t)(enum libbpf_print_level level,
>
>  /**
>   * @brief **libbpf_set_print()** sets user-provided log callback function to
> - * be used for libbpf warnings and informational messages.
> + * be used for libbpf warnings and informational messages. If the user callback
> + * is not set, messages are logged to stderr by default. The verbosity of these
> + * messages can be controlled by setting the environment variable
> + * LIBBPF_LOG_LEVEL to either warn, info, or debug.
>   * @param fn The log print function. If NULL, libbpf won't print anything.
>   * @return Pointer to old print function.
>   *
> --
> 2.45.0
>

Re: [PATCH bpf-next] libbpf: configure log verbosity with env variable

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Fri, 24 May 2024 14:18:40 +0100 you wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Configure logging verbosity by setting LIBBPF_LOG_LEVEL environment
> variable, which is applied only to default logger. Once user set their
> custom logging callback, it is up to them to handle filtering.
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> 
> [...]

Here is the summary with links:
  - [bpf-next] libbpf: configure log verbosity with env variable
    https://git.kernel.org/bpf/bpf-next/c/eb4e7726279a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH bpf-next] libbpf: configure log verbosity with env variable

[Mykyta]

From: Mykyta Yatsenko <yatsenko@meta.com>

Configure logging verbosity by setting LIBBPF_LOG environment
variable. Only applying to default logger. Once user set their custom
logging callback, it is up to them to filter.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 tools/lib/bpf/libbpf.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5401f2df463d..8805607073da 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -229,7 +229,23 @@ static const char * const prog_type_name[] = {
 static int __base_pr(enum libbpf_print_level level, const char *format,
 		     va_list args)
 {
-	if (level == LIBBPF_DEBUG)
+	static enum libbpf_print_level env_level = LIBBPF_INFO;
+	static bool initialized;
+
+	if (!initialized) {
+		char *verbosity;
+
+		initialized = true;
+		verbosity = getenv("LIBBPF_LOG");
+		if (verbosity) {
+			if (strcmp(verbosity, "warn") == 0)
+				env_level = LIBBPF_WARN;
+			else if (strcmp(verbosity, "debug") == 0)
+				env_level = LIBBPF_DEBUG;
+		}
+	}
+
+	if (env_level < level)
 		return 0;
 
 	return vfprintf(stderr, format, args);
-- 
2.45.0

Re: [PATCH bpf-next] libbpf: configure log verbosity with env variable

[Andrii Nakryiko]

On Thu, May 23, 2024 at 1:53 PM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Configure logging verbosity by setting LIBBPF_LOG environment
> variable. Only applying to default logger. Once user set their custom
> logging callback, it is up to them to filter.
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  tools/lib/bpf/libbpf.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 5401f2df463d..8805607073da 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -229,7 +229,23 @@ static const char * const prog_type_name[] = {
>  static int __base_pr(enum libbpf_print_level level, const char *format,
>                      va_list args)
>  {
> -       if (level == LIBBPF_DEBUG)
> +       static enum libbpf_print_level env_level = LIBBPF_INFO;

this doesn't have to come from just the environment, we might add an
API to set the default log level programmatically (in the future, not
saying we should do it right now). Maybe more generic "min_level" or
"log_level"?

> +       static bool initialized;
> +
> +       if (!initialized) {
> +               char *verbosity;
> +
> +               initialized = true;
> +               verbosity = getenv("LIBBPF_LOG");

I'd be confused whether LIBBPF_LOG is log *level* or some sort of file
path to which default log should go (and who knows, maybe we'll add
that). So let's maybe call it LIBBPF_LOG_LEVEL=... ?

> +               if (verbosity) {
> +                       if (strcmp(verbosity, "warn") == 0)

let's do case-insensitive comparison?

> +                               env_level = LIBBPF_WARN;
> +                       else if (strcmp(verbosity, "debug") == 0)
> +                               env_level = LIBBPF_DEBUG;

let's handle the "info" level as well?

and for unrecognized value, let's print a warning directly with fprintf(stderr)?

Let's also add this LIBBPF_LOG_LEVEL envvar description to the doc
comment for libbpf_set_print() and a short section somewhere in
Documentation/bpf/libbpf/libbpf_overview.rst ?

> +               }
> +       }
> +
> +       if (env_level < level)

super nitpicky, sorry, but my brain refuses to intuitively understand
this, can you invert the condition please (and maybe add a comment):

/* if too verbose, skip logging */
if (level > min_level)
    return 0;

But in general this is a useful feature, thanks!

pw-bot: cr

>                 return 0;
>
>         return vfprintf(stderr, format, args);
> --
> 2.45.0
>