All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names
@ 2022-03-31 15:45 Milan Landaverde
  2022-03-31 15:45 ` [PATCH bpf-next 1/3] bpf/bpftool: add syscall prog type Milan Landaverde
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Milan Landaverde @ 2022-03-31 15:45 UTC (permalink / raw)
  To: bpf
  Cc: milan, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, quentin, davemarchevsky, sdf, netdev,
	linux-kernel

With the addition of the syscall prog type we should now
be able to see feature probe info for that prog type:

    $ bpftool feature probe kernel
    ...
    eBPF program_type syscall is available
    ...
    eBPF helpers supported for program type syscall:
        ...
        - bpf_sys_bpf
        - bpf_sys_close

And for the link types, their names should aid in
the output.

Before:
    $ bpftool link show
    50: type 7  prog 5042
	    bpf_cookie 0
	    pids vfsstat(394433)

After:
    $ bpftool link show
    57: perf_event  prog 5058
	    bpf_cookie 0
	    pids vfsstat(394725)

Milan Landaverde (3):
  bpf/bpftool: add syscall prog type
  bpf/bpftool: add missing link types
  bpf/bpftool: handle libbpf_probe_prog_type errors

 tools/bpf/bpftool/feature.c | 2 +-
 tools/bpf/bpftool/link.c    | 2 ++
 tools/bpf/bpftool/prog.c    | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

--
2.32.0


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

* [PATCH bpf-next 1/3] bpf/bpftool: add syscall prog type
  2022-03-31 15:45 [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names Milan Landaverde
@ 2022-03-31 15:45 ` Milan Landaverde
  2022-04-01 16:04   ` Quentin Monnet
  2022-03-31 15:45 ` [PATCH bpf-next 2/3] bpf/bpftool: add missing link types Milan Landaverde
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Milan Landaverde @ 2022-03-31 15:45 UTC (permalink / raw)
  To: bpf
  Cc: milan, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, quentin, davemarchevsky, sdf, netdev,
	linux-kernel

In addition to displaying the program type in bpftool prog show
this enables us to be able to query bpf_prog_type_syscall
availability through feature probe as well as see
which helpers are available in those programs (such as
bpf_sys_bpf and bpf_sys_close)

Signed-off-by: Milan Landaverde <milan@mdaverde.com>
---
 tools/bpf/bpftool/prog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index bc4e05542c2b..8643b37d4e43 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -68,6 +68,7 @@ const char * const prog_type_name[] = {
 	[BPF_PROG_TYPE_EXT]			= "ext",
 	[BPF_PROG_TYPE_LSM]			= "lsm",
 	[BPF_PROG_TYPE_SK_LOOKUP]		= "sk_lookup",
+	[BPF_PROG_TYPE_SYSCALL]			= "syscall",
 };
 
 const size_t prog_type_name_size = ARRAY_SIZE(prog_type_name);
-- 
2.32.0


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

* [PATCH bpf-next 2/3] bpf/bpftool: add missing link types
  2022-03-31 15:45 [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names Milan Landaverde
  2022-03-31 15:45 ` [PATCH bpf-next 1/3] bpf/bpftool: add syscall prog type Milan Landaverde
@ 2022-03-31 15:45 ` Milan Landaverde
  2022-04-01 16:05   ` Quentin Monnet
  2022-03-31 15:45 ` [PATCH bpf-next 3/3] bpf/bpftool: handle libbpf_probe_prog_type errors Milan Landaverde
  2022-04-04 22:00 ` [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names patchwork-bot+netdevbpf
  3 siblings, 1 reply; 14+ messages in thread
From: Milan Landaverde @ 2022-03-31 15:45 UTC (permalink / raw)
  To: bpf
  Cc: milan, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, quentin, davemarchevsky, sdf, netdev,
	linux-kernel

Will display the link type names in bpftool link show output

Signed-off-by: Milan Landaverde <milan@mdaverde.com>
---
 tools/bpf/bpftool/link.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 97dec81950e5..9392ef390828 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -20,6 +20,8 @@ static const char * const link_type_name[] = {
 	[BPF_LINK_TYPE_CGROUP]			= "cgroup",
 	[BPF_LINK_TYPE_ITER]			= "iter",
 	[BPF_LINK_TYPE_NETNS]			= "netns",
+	[BPF_LINK_TYPE_XDP]				= "xdp",
+	[BPF_LINK_TYPE_PERF_EVENT]		= "perf_event",
 };
 
 static struct hashmap *link_table;
-- 
2.32.0


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

* [PATCH bpf-next 3/3] bpf/bpftool: handle libbpf_probe_prog_type errors
  2022-03-31 15:45 [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names Milan Landaverde
  2022-03-31 15:45 ` [PATCH bpf-next 1/3] bpf/bpftool: add syscall prog type Milan Landaverde
  2022-03-31 15:45 ` [PATCH bpf-next 2/3] bpf/bpftool: add missing link types Milan Landaverde
@ 2022-03-31 15:45 ` Milan Landaverde
  2022-04-01 16:05   ` Quentin Monnet
  2022-04-04 22:00 ` [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names patchwork-bot+netdevbpf
  3 siblings, 1 reply; 14+ messages in thread
From: Milan Landaverde @ 2022-03-31 15:45 UTC (permalink / raw)
  To: bpf
  Cc: milan, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, quentin, davemarchevsky, sdf, netdev,
	linux-kernel

Previously [1], we were using bpf_probe_prog_type which returned a
bool, but the new libbpf_probe_bpf_prog_type can return a negative
error code on failure. This change decides for bpftool to declare
a program type is not available on probe failure.

[1] https://lore.kernel.org/bpf/20220202225916.3313522-3-andrii@kernel.org/

Signed-off-by: Milan Landaverde <milan@mdaverde.com>
---
 tools/bpf/bpftool/feature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index c2f43a5d38e0..b2fbaa7a6b15 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -564,7 +564,7 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types,
 
 		res = probe_prog_type_ifindex(prog_type, ifindex);
 	} else {
-		res = libbpf_probe_bpf_prog_type(prog_type, NULL);
+		res = libbpf_probe_bpf_prog_type(prog_type, NULL) > 0;
 	}
 
 #ifdef USE_LIBCAP
-- 
2.32.0


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

* Re: [PATCH bpf-next 1/3] bpf/bpftool: add syscall prog type
  2022-03-31 15:45 ` [PATCH bpf-next 1/3] bpf/bpftool: add syscall prog type Milan Landaverde
@ 2022-04-01 16:04   ` Quentin Monnet
  2022-04-01 18:40     ` Andrii Nakryiko
  0 siblings, 1 reply; 14+ messages in thread
From: Quentin Monnet @ 2022-04-01 16:04 UTC (permalink / raw)
  To: Milan Landaverde, bpf
  Cc: ast, daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
	kpsingh, davemarchevsky, sdf, netdev, linux-kernel

2022-03-31 11:45 UTC-0400 ~ Milan Landaverde <milan@mdaverde.com>
> In addition to displaying the program type in bpftool prog show
> this enables us to be able to query bpf_prog_type_syscall
> availability through feature probe as well as see
> which helpers are available in those programs (such as
> bpf_sys_bpf and bpf_sys_close)
> 
> Signed-off-by: Milan Landaverde <milan@mdaverde.com>
> ---
>  tools/bpf/bpftool/prog.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index bc4e05542c2b..8643b37d4e43 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -68,6 +68,7 @@ const char * const prog_type_name[] = {
>  	[BPF_PROG_TYPE_EXT]			= "ext",
>  	[BPF_PROG_TYPE_LSM]			= "lsm",
>  	[BPF_PROG_TYPE_SK_LOOKUP]		= "sk_lookup",
> +	[BPF_PROG_TYPE_SYSCALL]			= "syscall",
>  };
>  
>  const size_t prog_type_name_size = ARRAY_SIZE(prog_type_name);

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

Thanks! This one should have been caught by CI :/. Instead it complains
when you add it. This is because BPF_PROG_TYPE_SYSCALL in the UAPI
header has a comment next to it, and the regex used in
tools/testing/selftests/bpf/test_bpftool_synctypes.py to extract the
program types does not account for it. The fix should be:

------
diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
index 6bf21e47882a..cd239cbfd80c 100755
--- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
+++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
@@ -180,7 +180,7 @@ class FileExtractor(object):
         @enum_name: name of the enum to parse
         """
         start_marker = re.compile(f'enum {enum_name} {{\n')
-        pattern = re.compile('^\s*(BPF_\w+),?$')
+        pattern = re.compile('^\s*(BPF_\w+),?( /\* .* \*/)?$')
         end_marker = re.compile('^};')
         parser = BlockParser(self.reader)
         parser.search_block(start_marker)
------

I can submit this separately as a patch.

Quentin

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

* Re: [PATCH bpf-next 2/3] bpf/bpftool: add missing link types
  2022-03-31 15:45 ` [PATCH bpf-next 2/3] bpf/bpftool: add missing link types Milan Landaverde
@ 2022-04-01 16:05   ` Quentin Monnet
  2022-04-04 21:55     ` Andrii Nakryiko
  0 siblings, 1 reply; 14+ messages in thread
From: Quentin Monnet @ 2022-04-01 16:05 UTC (permalink / raw)
  To: Milan Landaverde, bpf
  Cc: ast, daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
	kpsingh, davemarchevsky, sdf, netdev, linux-kernel

2022-03-31 11:45 UTC-0400 ~ Milan Landaverde <milan@mdaverde.com>
> Will display the link type names in bpftool link show output
> 
> Signed-off-by: Milan Landaverde <milan@mdaverde.com>
> ---
>  tools/bpf/bpftool/link.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index 97dec81950e5..9392ef390828 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
> @@ -20,6 +20,8 @@ static const char * const link_type_name[] = {
>  	[BPF_LINK_TYPE_CGROUP]			= "cgroup",
>  	[BPF_LINK_TYPE_ITER]			= "iter",
>  	[BPF_LINK_TYPE_NETNS]			= "netns",
> +	[BPF_LINK_TYPE_XDP]				= "xdp",
> +	[BPF_LINK_TYPE_PERF_EVENT]		= "perf_event",

Since this goes into bpf-next, we should add BPF_LINK_TYPE_KPROBE_MULTI
as well.

Quentin

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

* Re: [PATCH bpf-next 3/3] bpf/bpftool: handle libbpf_probe_prog_type errors
  2022-03-31 15:45 ` [PATCH bpf-next 3/3] bpf/bpftool: handle libbpf_probe_prog_type errors Milan Landaverde
@ 2022-04-01 16:05   ` Quentin Monnet
  2022-04-01 18:42     ` Andrii Nakryiko
  0 siblings, 1 reply; 14+ messages in thread
From: Quentin Monnet @ 2022-04-01 16:05 UTC (permalink / raw)
  To: Milan Landaverde, bpf
  Cc: ast, daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
	kpsingh, davemarchevsky, sdf, netdev, linux-kernel

2022-03-31 11:45 UTC-0400 ~ Milan Landaverde <milan@mdaverde.com>
> Previously [1], we were using bpf_probe_prog_type which returned a
> bool, but the new libbpf_probe_bpf_prog_type can return a negative
> error code on failure. This change decides for bpftool to declare
> a program type is not available on probe failure.
> 
> [1] https://lore.kernel.org/bpf/20220202225916.3313522-3-andrii@kernel.org/
> 
> Signed-off-by: Milan Landaverde <milan@mdaverde.com>
> ---
>  tools/bpf/bpftool/feature.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index c2f43a5d38e0..b2fbaa7a6b15 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -564,7 +564,7 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types,
>  
>  		res = probe_prog_type_ifindex(prog_type, ifindex);
>  	} else {
> -		res = libbpf_probe_bpf_prog_type(prog_type, NULL);
> +		res = libbpf_probe_bpf_prog_type(prog_type, NULL) > 0;
>  	}
>  
>  #ifdef USE_LIBCAP

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

Thanks!

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

* Re: [PATCH bpf-next 1/3] bpf/bpftool: add syscall prog type
  2022-04-01 16:04   ` Quentin Monnet
@ 2022-04-01 18:40     ` Andrii Nakryiko
  2022-04-01 21:20       ` Quentin Monnet
  0 siblings, 1 reply; 14+ messages in thread
From: Andrii Nakryiko @ 2022-04-01 18:40 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Milan Landaverde, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin Lau, Song Liu, Yonghong Song,
	john fastabend, KP Singh, Dave Marchevsky, Stanislav Fomichev,
	Networking, open list

On Fri, Apr 1, 2022 at 9:04 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2022-03-31 11:45 UTC-0400 ~ Milan Landaverde <milan@mdaverde.com>
> > In addition to displaying the program type in bpftool prog show
> > this enables us to be able to query bpf_prog_type_syscall
> > availability through feature probe as well as see
> > which helpers are available in those programs (such as
> > bpf_sys_bpf and bpf_sys_close)
> >
> > Signed-off-by: Milan Landaverde <milan@mdaverde.com>
> > ---
> >  tools/bpf/bpftool/prog.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > index bc4e05542c2b..8643b37d4e43 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -68,6 +68,7 @@ const char * const prog_type_name[] = {
> >       [BPF_PROG_TYPE_EXT]                     = "ext",
> >       [BPF_PROG_TYPE_LSM]                     = "lsm",
> >       [BPF_PROG_TYPE_SK_LOOKUP]               = "sk_lookup",
> > +     [BPF_PROG_TYPE_SYSCALL]                 = "syscall",
> >  };
> >
> >  const size_t prog_type_name_size = ARRAY_SIZE(prog_type_name);
>
> Reviewed-by: Quentin Monnet <quentin@isovalent.com>
>
> Thanks! This one should have been caught by CI :/. Instead it complains
> when you add it. This is because BPF_PROG_TYPE_SYSCALL in the UAPI
> header has a comment next to it, and the regex used in
> tools/testing/selftests/bpf/test_bpftool_synctypes.py to extract the
> program types does not account for it. The fix should be:
>
> ------
> diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> index 6bf21e47882a..cd239cbfd80c 100755
> --- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> +++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> @@ -180,7 +180,7 @@ class FileExtractor(object):
>          @enum_name: name of the enum to parse
>          """
>          start_marker = re.compile(f'enum {enum_name} {{\n')
> -        pattern = re.compile('^\s*(BPF_\w+),?$')
> +        pattern = re.compile('^\s*(BPF_\w+),?( /\* .* \*/)?$')

small nit: do you need those spaces inside /* and */? why make
unnecessary assumptions about proper formatting? ;)

>          end_marker = re.compile('^};')
>          parser = BlockParser(self.reader)
>          parser.search_block(start_marker)
> ------
>
> I can submit this separately as a patch.
>
> Quentin

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

* Re: [PATCH bpf-next 3/3] bpf/bpftool: handle libbpf_probe_prog_type errors
  2022-04-01 16:05   ` Quentin Monnet
@ 2022-04-01 18:42     ` Andrii Nakryiko
  2022-04-01 21:33       ` Quentin Monnet
  0 siblings, 1 reply; 14+ messages in thread
From: Andrii Nakryiko @ 2022-04-01 18:42 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Milan Landaverde, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin Lau, Song Liu, Yonghong Song,
	john fastabend, KP Singh, Dave Marchevsky, Stanislav Fomichev,
	Networking, open list

On Fri, Apr 1, 2022 at 9:05 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2022-03-31 11:45 UTC-0400 ~ Milan Landaverde <milan@mdaverde.com>
> > Previously [1], we were using bpf_probe_prog_type which returned a
> > bool, but the new libbpf_probe_bpf_prog_type can return a negative
> > error code on failure. This change decides for bpftool to declare
> > a program type is not available on probe failure.
> >
> > [1] https://lore.kernel.org/bpf/20220202225916.3313522-3-andrii@kernel.org/
> >
> > Signed-off-by: Milan Landaverde <milan@mdaverde.com>
> > ---
> >  tools/bpf/bpftool/feature.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> > index c2f43a5d38e0..b2fbaa7a6b15 100644
> > --- a/tools/bpf/bpftool/feature.c
> > +++ b/tools/bpf/bpftool/feature.c
> > @@ -564,7 +564,7 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types,
> >
> >               res = probe_prog_type_ifindex(prog_type, ifindex);
> >       } else {
> > -             res = libbpf_probe_bpf_prog_type(prog_type, NULL);
> > +             res = libbpf_probe_bpf_prog_type(prog_type, NULL) > 0;
> >       }
> >
> >  #ifdef USE_LIBCAP
>

A completely unrelated question to you, Quentin. How hard is bpftool's
dependency on libcap? We've recently removed libcap from selftests, I
wonder if it would be possible to do that for bpftool as well to
reduce amount of shared libraries bpftool depends on.

> Reviewed-by: Quentin Monnet <quentin@isovalent.com>
>
> Thanks!

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

* Re: [PATCH bpf-next 1/3] bpf/bpftool: add syscall prog type
  2022-04-01 18:40     ` Andrii Nakryiko
@ 2022-04-01 21:20       ` Quentin Monnet
  0 siblings, 0 replies; 14+ messages in thread
From: Quentin Monnet @ 2022-04-01 21:20 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Milan Landaverde, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin Lau, Song Liu, Yonghong Song,
	john fastabend, KP Singh, Dave Marchevsky, Stanislav Fomichev,
	Networking, open list

On Fri, 1 Apr 2022 at 19:40, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Apr 1, 2022 at 9:04 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >
> > 2022-03-31 11:45 UTC-0400 ~ Milan Landaverde <milan@mdaverde.com>
> > > In addition to displaying the program type in bpftool prog show
> > > this enables us to be able to query bpf_prog_type_syscall
> > > availability through feature probe as well as see
> > > which helpers are available in those programs (such as
> > > bpf_sys_bpf and bpf_sys_close)
> > >
> > > Signed-off-by: Milan Landaverde <milan@mdaverde.com>
> > > ---
> > >  tools/bpf/bpftool/prog.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > > index bc4e05542c2b..8643b37d4e43 100644
> > > --- a/tools/bpf/bpftool/prog.c
> > > +++ b/tools/bpf/bpftool/prog.c
> > > @@ -68,6 +68,7 @@ const char * const prog_type_name[] = {
> > >       [BPF_PROG_TYPE_EXT]                     = "ext",
> > >       [BPF_PROG_TYPE_LSM]                     = "lsm",
> > >       [BPF_PROG_TYPE_SK_LOOKUP]               = "sk_lookup",
> > > +     [BPF_PROG_TYPE_SYSCALL]                 = "syscall",
> > >  };
> > >
> > >  const size_t prog_type_name_size = ARRAY_SIZE(prog_type_name);
> >
> > Reviewed-by: Quentin Monnet <quentin@isovalent.com>
> >
> > Thanks! This one should have been caught by CI :/. Instead it complains
> > when you add it. This is because BPF_PROG_TYPE_SYSCALL in the UAPI
> > header has a comment next to it, and the regex used in
> > tools/testing/selftests/bpf/test_bpftool_synctypes.py to extract the
> > program types does not account for it. The fix should be:
> >
> > ------
> > diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> > b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> > index 6bf21e47882a..cd239cbfd80c 100755
> > --- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> > +++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> > @@ -180,7 +180,7 @@ class FileExtractor(object):
> >          @enum_name: name of the enum to parse
> >          """
> >          start_marker = re.compile(f'enum {enum_name} {{\n')
> > -        pattern = re.compile('^\s*(BPF_\w+),?$')
> > +        pattern = re.compile('^\s*(BPF_\w+),?( /\* .* \*/)?$')
>
> small nit: do you need those spaces inside /* and */? why make
> unnecessary assumptions about proper formatting? ;)

No I don't need the spaces, I'll remove them indeed, thanks. I'll send
the patch next week.

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

* Re: [PATCH bpf-next 3/3] bpf/bpftool: handle libbpf_probe_prog_type errors
  2022-04-01 18:42     ` Andrii Nakryiko
@ 2022-04-01 21:33       ` Quentin Monnet
  2022-04-03 23:51         ` Andrii Nakryiko
  0 siblings, 1 reply; 14+ messages in thread
From: Quentin Monnet @ 2022-04-01 21:33 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Milan Landaverde, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin Lau, Song Liu, Yonghong Song,
	john fastabend, KP Singh, Dave Marchevsky, Stanislav Fomichev,
	Networking, open list

On Fri, 1 Apr 2022 at 19:42, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Apr 1, 2022 at 9:05 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >
> > 2022-03-31 11:45 UTC-0400 ~ Milan Landaverde <milan@mdaverde.com>
> > > Previously [1], we were using bpf_probe_prog_type which returned a
> > > bool, but the new libbpf_probe_bpf_prog_type can return a negative
> > > error code on failure. This change decides for bpftool to declare
> > > a program type is not available on probe failure.
> > >
> > > [1] https://lore.kernel.org/bpf/20220202225916.3313522-3-andrii@kernel.org/
> > >
> > > Signed-off-by: Milan Landaverde <milan@mdaverde.com>
> > > ---
> > >  tools/bpf/bpftool/feature.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> > > index c2f43a5d38e0..b2fbaa7a6b15 100644
> > > --- a/tools/bpf/bpftool/feature.c
> > > +++ b/tools/bpf/bpftool/feature.c
> > > @@ -564,7 +564,7 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types,
> > >
> > >               res = probe_prog_type_ifindex(prog_type, ifindex);
> > >       } else {
> > > -             res = libbpf_probe_bpf_prog_type(prog_type, NULL);
> > > +             res = libbpf_probe_bpf_prog_type(prog_type, NULL) > 0;
> > >       }
> > >
> > >  #ifdef USE_LIBCAP
> >
>
> A completely unrelated question to you, Quentin. How hard is bpftool's
> dependency on libcap? We've recently removed libcap from selftests, I
> wonder if it would be possible to do that for bpftool as well to
> reduce amount of shared libraries bpftool depends on.

There's not a super-strong dependency on it. It's used in feature
probing, for two things.

First one is to be accurate when we check that the user has the right
capabilities for probing efficiently the system. A workaround consists
in checking that we run with uid=0 (root), although it's less
accurate.

Second thing is probing as an unprivileged user: if bpftool is run to
probe as root but with the "unprivileged" keyword, libcap is used to
drop the CAP_SYS_ADMIN and run the probes without it. I don't know if
there's an easy alternative to libcap for that. Also I don't know how
many people use this feature, but I remember that this was added
because there was some demand at the time, so presumably there are
users relying on this.

This being said, libcap is optional for compiling bpftool, so you
should be able to have it work just as well if the library is not
available on the system? Basically you'd just lose the ability to
probe as an unprivileged user. Do you need to remove the optional
dependency completely?

Quentin

PS: Not directly related but since we're talking of libcap, we
recently discovered that the lib is apparently changing errno when it
maybe shouldn't and plays badly with batch mode:
https://stackoverflow.com/questions/71608181/bpf-xdp-bpftool-batch-file-returns-error-reading-batch-file-failed-opera

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

* Re: [PATCH bpf-next 3/3] bpf/bpftool: handle libbpf_probe_prog_type errors
  2022-04-01 21:33       ` Quentin Monnet
@ 2022-04-03 23:51         ` Andrii Nakryiko
  0 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2022-04-03 23:51 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Milan Landaverde, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin Lau, Song Liu, Yonghong Song,
	john fastabend, KP Singh, Dave Marchevsky, Stanislav Fomichev,
	Networking, open list

On Fri, Apr 1, 2022 at 2:33 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> On Fri, 1 Apr 2022 at 19:42, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Apr 1, 2022 at 9:05 AM Quentin Monnet <quentin@isovalent.com> wrote:
> > >
> > > 2022-03-31 11:45 UTC-0400 ~ Milan Landaverde <milan@mdaverde.com>
> > > > Previously [1], we were using bpf_probe_prog_type which returned a
> > > > bool, but the new libbpf_probe_bpf_prog_type can return a negative
> > > > error code on failure. This change decides for bpftool to declare
> > > > a program type is not available on probe failure.
> > > >
> > > > [1] https://lore.kernel.org/bpf/20220202225916.3313522-3-andrii@kernel.org/
> > > >
> > > > Signed-off-by: Milan Landaverde <milan@mdaverde.com>
> > > > ---
> > > >  tools/bpf/bpftool/feature.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> > > > index c2f43a5d38e0..b2fbaa7a6b15 100644
> > > > --- a/tools/bpf/bpftool/feature.c
> > > > +++ b/tools/bpf/bpftool/feature.c
> > > > @@ -564,7 +564,7 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types,
> > > >
> > > >               res = probe_prog_type_ifindex(prog_type, ifindex);
> > > >       } else {
> > > > -             res = libbpf_probe_bpf_prog_type(prog_type, NULL);
> > > > +             res = libbpf_probe_bpf_prog_type(prog_type, NULL) > 0;
> > > >       }
> > > >
> > > >  #ifdef USE_LIBCAP
> > >
> >
> > A completely unrelated question to you, Quentin. How hard is bpftool's
> > dependency on libcap? We've recently removed libcap from selftests, I
> > wonder if it would be possible to do that for bpftool as well to
> > reduce amount of shared libraries bpftool depends on.
>
> There's not a super-strong dependency on it. It's used in feature
> probing, for two things.
>
> First one is to be accurate when we check that the user has the right
> capabilities for probing efficiently the system. A workaround consists
> in checking that we run with uid=0 (root), although it's less
> accurate.
>
> Second thing is probing as an unprivileged user: if bpftool is run to
> probe as root but with the "unprivileged" keyword, libcap is used to
> drop the CAP_SYS_ADMIN and run the probes without it. I don't know if
> there's an easy alternative to libcap for that. Also I don't know how
> many people use this feature, but I remember that this was added
> because there was some demand at the time, so presumably there are
> users relying on this.
>
> This being said, libcap is optional for compiling bpftool, so you
> should be able to have it work just as well if the library is not
> available on the system? Basically you'd just lose the ability to
> probe as an unprivileged user. Do you need to remove the optional
> dependency completely?

Well, see recent patches from Martin:

82cb2b30773e bpf: selftests: Remove libcap usage from test_progs
b1c2768a82b9 bpf: selftests: Remove libcap usage from test_verifier
663af70aabb7 bpf: selftests: Add helpers to directly use the capget
and capset syscall

We completely got rid of libcap dependency and ended up with more
straightforward code. So I was wondering if this is possible for
bpftool. The less unnecessary library dependencies - the better. The
less feature detection -- the better. That's my only line of thought
here :)

>
> Quentin
>
> PS: Not directly related but since we're talking of libcap, we
> recently discovered that the lib is apparently changing errno when it
> maybe shouldn't and plays badly with batch mode:
> https://stackoverflow.com/questions/71608181/bpf-xdp-bpftool-batch-file-returns-error-reading-batch-file-failed-opera

just another argument in favor of getting rid of extra layers of dependencies

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

* Re: [PATCH bpf-next 2/3] bpf/bpftool: add missing link types
  2022-04-01 16:05   ` Quentin Monnet
@ 2022-04-04 21:55     ` Andrii Nakryiko
  0 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2022-04-04 21:55 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Milan Landaverde, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin Lau, Song Liu, Yonghong Song,
	john fastabend, KP Singh, Dave Marchevsky, Stanislav Fomichev,
	Networking, open list

On Fri, Apr 1, 2022 at 9:05 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2022-03-31 11:45 UTC-0400 ~ Milan Landaverde <milan@mdaverde.com>
> > Will display the link type names in bpftool link show output
> >
> > Signed-off-by: Milan Landaverde <milan@mdaverde.com>
> > ---
> >  tools/bpf/bpftool/link.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> > index 97dec81950e5..9392ef390828 100644
> > --- a/tools/bpf/bpftool/link.c
> > +++ b/tools/bpf/bpftool/link.c
> > @@ -20,6 +20,8 @@ static const char * const link_type_name[] = {
> >       [BPF_LINK_TYPE_CGROUP]                  = "cgroup",
> >       [BPF_LINK_TYPE_ITER]                    = "iter",
> >       [BPF_LINK_TYPE_NETNS]                   = "netns",
> > +     [BPF_LINK_TYPE_XDP]                             = "xdp",

fixed indentation here and added KPROBE_MULTI while applying

> > +     [BPF_LINK_TYPE_PERF_EVENT]              = "perf_event",
>
> Since this goes into bpf-next, we should add BPF_LINK_TYPE_KPROBE_MULTI
> as well.
>
> Quentin

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

* Re: [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names
  2022-03-31 15:45 [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names Milan Landaverde
                   ` (2 preceding siblings ...)
  2022-03-31 15:45 ` [PATCH bpf-next 3/3] bpf/bpftool: handle libbpf_probe_prog_type errors Milan Landaverde
@ 2022-04-04 22:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 14+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-04-04 22:00 UTC (permalink / raw)
  To: Milan Landaverde
  Cc: bpf, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, quentin, davemarchevsky, sdf, netdev,
	linux-kernel

Hello:

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

On Thu, 31 Mar 2022 11:45:52 -0400 you wrote:
> With the addition of the syscall prog type we should now
> be able to see feature probe info for that prog type:
> 
>     $ bpftool feature probe kernel
>     ...
>     eBPF program_type syscall is available
>     ...
>     eBPF helpers supported for program type syscall:
>         ...
>         - bpf_sys_bpf
>         - bpf_sys_close
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/3] bpf/bpftool: add syscall prog type
    https://git.kernel.org/bpf/bpf-next/c/380341637ebb
  - [bpf-next,2/3] bpf/bpftool: add missing link types
    https://git.kernel.org/bpf/bpf-next/c/fff3dfab1786
  - [bpf-next,3/3] bpf/bpftool: handle libbpf_probe_prog_type errors
    https://git.kernel.org/bpf/bpf-next/c/7b53eaa656c3

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] 14+ messages in thread

end of thread, other threads:[~2022-04-04 22:46 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-31 15:45 [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names Milan Landaverde
2022-03-31 15:45 ` [PATCH bpf-next 1/3] bpf/bpftool: add syscall prog type Milan Landaverde
2022-04-01 16:04   ` Quentin Monnet
2022-04-01 18:40     ` Andrii Nakryiko
2022-04-01 21:20       ` Quentin Monnet
2022-03-31 15:45 ` [PATCH bpf-next 2/3] bpf/bpftool: add missing link types Milan Landaverde
2022-04-01 16:05   ` Quentin Monnet
2022-04-04 21:55     ` Andrii Nakryiko
2022-03-31 15:45 ` [PATCH bpf-next 3/3] bpf/bpftool: handle libbpf_probe_prog_type errors Milan Landaverde
2022-04-01 16:05   ` Quentin Monnet
2022-04-01 18:42     ` Andrii Nakryiko
2022-04-01 21:33       ` Quentin Monnet
2022-04-03 23:51         ` Andrii Nakryiko
2022-04-04 22:00 ` [PATCH bpf-next 0/3] bpf/bpftool: add program & link type names patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.