diff mbox series

[bpf-next,1/3] bpf/bpftool: add syscall prog type

Message ID 20220331154555.422506-2-milan@mdaverde.com (mailing list archive)
State Accepted
Commit 380341637ebb41f56031a0fd0779e85413a1da59
Delegated to: BPF
Headers show
Series bpf/bpftool: add program & link type names | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 12 of 12 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-PR fail merge-conflict

Commit Message

Milan Landaverde March 31, 2022, 3:45 p.m. UTC
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(+)

Comments

Quentin Monnet April 1, 2022, 4:04 p.m. UTC | #1
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
Andrii Nakryiko April 1, 2022, 6:40 p.m. UTC | #2
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
Quentin Monnet April 1, 2022, 9:20 p.m. UTC | #3
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.
diff mbox series

Patch

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);