BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpftool: fix output for skipping kernel config check
@ 2022-12-06  4:35 Chethan Suresh
  2022-12-08 23:32 ` Andrii Nakryiko
  0 siblings, 1 reply; 6+ messages in thread
From: Chethan Suresh @ 2022-12-06  4:35 UTC (permalink / raw)
  To: quentin, bpf; +Cc: Chethan Suresh, Kenta Tada

When bpftool feature does not find kernel config files
under default path, do not output CONFIG_XYZ is not set.
Skip kernel config check and continue.

Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
---
 tools/bpf/bpftool/feature.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 36cf0f1517c9..316c4a01bdb7 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -487,14 +487,14 @@ static void probe_kernel_image_config(const char *define_prefix)
 	}
 
 end_parse:
-	if (file)
+	if (file) {
 		gzclose(file);
-
-	for (i = 0; i < ARRAY_SIZE(options); i++) {
-		if (define_prefix && !options[i].macro_dump)
-			continue;
-		print_kernel_option(options[i].name, values[i], define_prefix);
-		free(values[i]);
+		for (i = 0; i < ARRAY_SIZE(options); i++) {
+			if (define_prefix && !options[i].macro_dump)
+				continue;
+			print_kernel_option(options[i].name, values[i], define_prefix);
+			free(values[i]);
+		}
 	}
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next] bpftool: fix output for skipping kernel config check
  2022-12-06  4:35 Chethan Suresh
@ 2022-12-08 23:32 ` Andrii Nakryiko
  2022-12-13  4:42   ` Chethan Suresh
  0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2022-12-08 23:32 UTC (permalink / raw)
  To: Chethan Suresh; +Cc: quentin, bpf, Kenta Tada

On Mon, Dec 5, 2022 at 8:41 PM Chethan Suresh <chethan.suresh@sony.com> wrote:
>
> When bpftool feature does not find kernel config files
> under default path, do not output CONFIG_XYZ is not set.
> Skip kernel config check and continue.
>
> Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
> Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> ---
>  tools/bpf/bpftool/feature.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 36cf0f1517c9..316c4a01bdb7 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -487,14 +487,14 @@ static void probe_kernel_image_config(const char *define_prefix)
>         }
>
>  end_parse:
> -       if (file)
> +       if (file) {

There are two error conditions when file != NULL but we actually don't
read kconfig contents. Please handle those properly, otherwise all the
same confusion will keep happening.

>                 gzclose(file);
> -
> -       for (i = 0; i < ARRAY_SIZE(options); i++) {
> -               if (define_prefix && !options[i].macro_dump)
> -                       continue;
> -               print_kernel_option(options[i].name, values[i], define_prefix);
> -               free(values[i]);
> +               for (i = 0; i < ARRAY_SIZE(options); i++) {
> +                       if (define_prefix && !options[i].macro_dump)
> +                               continue;
> +                       print_kernel_option(options[i].name, values[i], define_prefix);
> +                       free(values[i]);
> +               }
>         }
>  }
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next] bpftool: fix output for skipping kernel config check
  2022-12-08 23:32 ` Andrii Nakryiko
@ 2022-12-13  4:42   ` Chethan Suresh
  0 siblings, 0 replies; 6+ messages in thread
From: Chethan Suresh @ 2022-12-13  4:42 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: quentin, bpf, Kenta.Tada

On Thu, Dec 08, 2022 at 03:32:33PM -0800, Andrii Nakryiko wrote:
> On Mon, Dec 5, 2022 at 8:41 PM Chethan Suresh <chethan.suresh@sony.com> wrote:
> >
> > When bpftool feature does not find kernel config files
> > under default path, do not output CONFIG_XYZ is not set.
> > Skip kernel config check and continue.
> >
> > Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
> > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> > ---
> >  tools/bpf/bpftool/feature.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> > index 36cf0f1517c9..316c4a01bdb7 100644
> > --- a/tools/bpf/bpftool/feature.c
> > +++ b/tools/bpf/bpftool/feature.c
> > @@ -487,14 +487,14 @@ static void probe_kernel_image_config(const char *define_prefix)
> >         }
> >
> >  end_parse:
> > -       if (file)
> > +       if (file) {
> 
> There are two error conditions when file != NULL but we actually don't
> read kconfig contents. Please handle those properly, otherwise all the
> same confusion will keep happening.
As I understand, the check should skip when file != NULL itself rather
than handling it in end_parse.
I'll send the updated patch based on review.
> 
> >                 gzclose(file);
> > -
> > -       for (i = 0; i < ARRAY_SIZE(options); i++) {
> > -               if (define_prefix && !options[i].macro_dump)
> > -                       continue;
> > -               print_kernel_option(options[i].name, values[i], define_prefix);
> > -               free(values[i]);
> > +               for (i = 0; i < ARRAY_SIZE(options); i++) {
> > +                       if (define_prefix && !options[i].macro_dump)
> > +                               continue;
> > +                       print_kernel_option(options[i].name, values[i], define_prefix);
> > +                       free(values[i]);
> > +               }
> >         }
> >  }
> >
> > --
> > 2.17.1
> >


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next] bpftool: fix output for skipping kernel config check
@ 2023-01-09  2:37 Chethan Suresh
  2023-01-09 10:19 ` Quentin Monnet
  2023-01-11  2:00 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 6+ messages in thread
From: Chethan Suresh @ 2023-01-09  2:37 UTC (permalink / raw)
  To: quentin, bpf; +Cc: Chethan Suresh, Kenta Tada

When bpftool feature does not find kernel config
files under default path or wrong format,
do not output CONFIG_XYZ is not set.
Skip kernel config check and continue.

Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
---
 tools/bpf/bpftool/feature.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 36cf0f1517c9..da16e6a27ccc 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -486,16 +486,16 @@ static void probe_kernel_image_config(const char *define_prefix)
 		}
 	}
 
-end_parse:
-	if (file)
-		gzclose(file);
-
 	for (i = 0; i < ARRAY_SIZE(options); i++) {
 		if (define_prefix && !options[i].macro_dump)
 			continue;
 		print_kernel_option(options[i].name, values[i], define_prefix);
 		free(values[i]);
 	}
+
+end_parse:
+	if (file)
+		gzclose(file);
 }
 
 static bool probe_bpf_syscall(const char *define_prefix)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next] bpftool: fix output for skipping kernel config check
  2023-01-09  2:37 [PATCH bpf-next] bpftool: fix output for skipping kernel config check Chethan Suresh
@ 2023-01-09 10:19 ` Quentin Monnet
  2023-01-11  2:00 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: Quentin Monnet @ 2023-01-09 10:19 UTC (permalink / raw)
  To: Chethan Suresh, bpf; +Cc: Kenta Tada

2023-01-09 08:07 UTC+0530 ~ Chethan Suresh <chethan.suresh@sony.com>
> When bpftool feature does not find kernel config
> files under default path or wrong format,
> do not output CONFIG_XYZ is not set.
> Skip kernel config check and continue.
> 
> Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
> Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> ---
>  tools/bpf/bpftool/feature.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 36cf0f1517c9..da16e6a27ccc 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -486,16 +486,16 @@ static void probe_kernel_image_config(const char *define_prefix)
>  		}
>  	}
>  
> -end_parse:
> -	if (file)
> -		gzclose(file);
> -
>  	for (i = 0; i < ARRAY_SIZE(options); i++) {
>  		if (define_prefix && !options[i].macro_dump)
>  			continue;
>  		print_kernel_option(options[i].name, values[i], define_prefix);
>  		free(values[i]);
>  	}
> +
> +end_parse:
> +	if (file)
> +		gzclose(file);
>  }
>  
>  static bool probe_bpf_syscall(const char *define_prefix)

Thanks!

This will remove the output for the kernel config options in case the
config file is not found, including from the JSON output. I can't
remember the motivation for printing negative values in that case, at
the time. I'm somewhat concerned to see the JSON entries disappear when
the config file is missing. Ideally, we should have an alternative state
for when config file is not here; but even using a specific string in
that case could play badly with scripts expecting that the kconfig
option is set if the JSON probing output contains anything else than
'null' for that entry.

So I think we can go with this patch indeed, and see if anyone complains
about it. After all, this is consistent with what we do elsewhere: We
skip entirely most probes if BPF_SYSCALL is not set, or probes for
helpers related to unsupported program types.

Acked-by: Quentin Monnet <quentin@isovalent.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next] bpftool: fix output for skipping kernel config check
  2023-01-09  2:37 [PATCH bpf-next] bpftool: fix output for skipping kernel config check Chethan Suresh
  2023-01-09 10:19 ` Quentin Monnet
@ 2023-01-11  2:00 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-01-11  2:00 UTC (permalink / raw)
  To: Chethan Suresh; +Cc: quentin, bpf, Kenta.Tada

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:

On Mon,  9 Jan 2023 08:07:42 +0530 you wrote:
> When bpftool feature does not find kernel config
> files under default path or wrong format,
> do not output CONFIG_XYZ is not set.
> Skip kernel config check and continue.
> 
> Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
> Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> 
> [...]

Here is the summary with links:
  - [bpf-next] bpftool: fix output for skipping kernel config check
    https://git.kernel.org/bpf/bpf-next/c/75514e4c6619

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-11  2:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-09  2:37 [PATCH bpf-next] bpftool: fix output for skipping kernel config check Chethan Suresh
2023-01-09 10:19 ` Quentin Monnet
2023-01-11  2:00 ` patchwork-bot+netdevbpf
  -- strict thread matches above, loose matches on Subject: below --
2022-12-06  4:35 Chethan Suresh
2022-12-08 23:32 ` Andrii Nakryiko
2022-12-13  4:42   ` Chethan Suresh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox