* [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates
2021-07-29 16:29 [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Quentin Monnet
@ 2021-07-29 16:29 ` Quentin Monnet
2021-07-30 18:45 ` Andrii Nakryiko
2021-07-29 16:29 ` [PATCH bpf-next 2/7] selftests/bpf: check consistency between bpftool source, doc, completion Quentin Monnet
` (6 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Quentin Monnet @ 2021-07-29 16:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Quentin Monnet
Bash completion for bpftool gets two minor improvements in this patch.
Move the detection of attach types for "bpftool cgroup attach" outside
of the "case/esac" bloc, where we cannot reuse our variable holding the
list of supported attach types as a pattern list. After the change, we
have only one list of cgroup attach types to update when new types are
added, instead of the former two lists.
Also rename the variables holding lists of names for program types, map
types, and attach types, to make them more unique. This can make it
slightly easier to point people to the relevant variables to update, but
the main objective here is to help run a script to check that bash
completion is up-to-date with bpftool's source code.
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/bpf/bpftool/bash-completion/bpftool | 57 +++++++++++++----------
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index cc33c5824a2f..b2e33a2d8524 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -404,8 +404,10 @@ _bpftool()
return 0
;;
5)
- COMPREPLY=( $( compgen -W 'msg_verdict stream_verdict \
- stream_parser flow_dissector' -- "$cur" ) )
+ local BPFTOOL_PROG_ATTACH_TYPES='msg_verdict \
+ stream_verdict stream_parser flow_dissector'
+ COMPREPLY=( $( compgen -W \
+ "$BPFTOOL_PROG_ATTACH_TYPES" -- "$cur" ) )
return 0
;;
6)
@@ -464,7 +466,7 @@ _bpftool()
case $prev in
type)
- COMPREPLY=( $( compgen -W "socket kprobe \
+ local BPFTOOL_PROG_LOAD_TYPES='socket kprobe \
kretprobe classifier flow_dissector \
action tracepoint raw_tracepoint \
xdp perf_event cgroup/skb cgroup/sock \
@@ -479,8 +481,9 @@ _bpftool()
cgroup/post_bind4 cgroup/post_bind6 \
cgroup/sysctl cgroup/getsockopt \
cgroup/setsockopt cgroup/sock_release struct_ops \
- fentry fexit freplace sk_lookup" -- \
- "$cur" ) )
+ fentry fexit freplace sk_lookup'
+ COMPREPLY=( $( compgen -W \
+ "$BPFTOOL_PROG_LOAD_TYPES" -- "$cur" ) )
return 0
;;
id)
@@ -698,15 +701,16 @@ _bpftool()
return 0
;;
type)
- COMPREPLY=( $( compgen -W 'hash array prog_array \
- perf_event_array percpu_hash percpu_array \
- stack_trace cgroup_array lru_hash \
+ local BPFTOOL_MAP_CREATE_TYPES='hash array \
+ prog_array perf_event_array percpu_hash \
+ percpu_array stack_trace cgroup_array lru_hash \
lru_percpu_hash lpm_trie array_of_maps \
hash_of_maps devmap devmap_hash sockmap cpumap \
xskmap sockhash cgroup_storage reuseport_sockarray \
percpu_cgroup_storage queue stack sk_storage \
- struct_ops inode_storage task_storage' -- \
- "$cur" ) )
+ struct_ops inode_storage task_storage'
+ COMPREPLY=( $( compgen -W \
+ "$BPFTOOL_MAP_CREATE_TYPES" -- "$cur" ) )
return 0
;;
key|value|flags|entries)
@@ -1017,34 +1021,37 @@ _bpftool()
return 0
;;
attach|detach)
- local ATTACH_TYPES='ingress egress sock_create sock_ops \
- device bind4 bind6 post_bind4 post_bind6 connect4 connect6 \
+ local BPFTOOL_CGROUP_ATTACH_TYPES='ingress egress \
+ sock_create sock_ops device \
+ bind4 bind6 post_bind4 post_bind6 connect4 connect6 \
getpeername4 getpeername6 getsockname4 getsockname6 \
sendmsg4 sendmsg6 recvmsg4 recvmsg6 sysctl getsockopt \
setsockopt sock_release'
local ATTACH_FLAGS='multi override'
local PROG_TYPE='id pinned tag name'
- case $prev in
- $command)
- _filedir
- return 0
- ;;
- ingress|egress|sock_create|sock_ops|device|bind4|bind6|\
- post_bind4|post_bind6|connect4|connect6|getpeername4|\
- getpeername6|getsockname4|getsockname6|sendmsg4|sendmsg6|\
- recvmsg4|recvmsg6|sysctl|getsockopt|setsockopt|sock_release)
+ # Check for $prev = $command first
+ if [ $prev = $command ]; then
+ _filedir
+ return 0
+ # Then check for attach type. This is done outside of the
+ # "case $prev in" to avoid writing the whole list of attach
+ # types again as pattern to match (where we cannot reuse
+ # our variable).
+ elif [[ $BPFTOOL_CGROUP_ATTACH_TYPES =~ $prev ]]; then
COMPREPLY=( $( compgen -W "$PROG_TYPE" -- \
"$cur" ) )
return 0
- ;;
+ fi
+ # case/esac for the other cases
+ case $prev in
id)
_bpftool_get_prog_ids
return 0
;;
*)
- if ! _bpftool_search_list "$ATTACH_TYPES"; then
- COMPREPLY=( $( compgen -W "$ATTACH_TYPES" -- \
- "$cur" ) )
+ if ! _bpftool_search_list "$BPFTOOL_CGROUP_ATTACH_TYPES"; then
+ COMPREPLY=( $( compgen -W \
+ "$BPFTOOL_CGROUP_ATTACH_TYPES" -- "$cur" ) )
elif [[ "$command" == "attach" ]]; then
# We have an attach type on the command line,
# but it is not the previous word, or
--
2.30.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates
2021-07-29 16:29 ` [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates Quentin Monnet
@ 2021-07-30 18:45 ` Andrii Nakryiko
2021-07-30 21:46 ` Quentin Monnet
0 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2021-07-30 18:45 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> Bash completion for bpftool gets two minor improvements in this patch.
>
> Move the detection of attach types for "bpftool cgroup attach" outside
> of the "case/esac" bloc, where we cannot reuse our variable holding the
> list of supported attach types as a pattern list. After the change, we
> have only one list of cgroup attach types to update when new types are
> added, instead of the former two lists.
>
> Also rename the variables holding lists of names for program types, map
> types, and attach types, to make them more unique. This can make it
> slightly easier to point people to the relevant variables to update, but
> the main objective here is to help run a script to check that bash
> completion is up-to-date with bpftool's source code.
>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
> tools/bpf/bpftool/bash-completion/bpftool | 57 +++++++++++++----------
> 1 file changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index cc33c5824a2f..b2e33a2d8524 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -404,8 +404,10 @@ _bpftool()
> return 0
> ;;
> 5)
> - COMPREPLY=( $( compgen -W 'msg_verdict stream_verdict \
> - stream_parser flow_dissector' -- "$cur" ) )
> + local BPFTOOL_PROG_ATTACH_TYPES='msg_verdict \
> + stream_verdict stream_parser flow_dissector'
> + COMPREPLY=( $( compgen -W \
> + "$BPFTOOL_PROG_ATTACH_TYPES" -- "$cur" ) )
> return 0
> ;;
> 6)
> @@ -464,7 +466,7 @@ _bpftool()
>
> case $prev in
> type)
> - COMPREPLY=( $( compgen -W "socket kprobe \
> + local BPFTOOL_PROG_LOAD_TYPES='socket kprobe \
> kretprobe classifier flow_dissector \
> action tracepoint raw_tracepoint \
> xdp perf_event cgroup/skb cgroup/sock \
> @@ -479,8 +481,9 @@ _bpftool()
> cgroup/post_bind4 cgroup/post_bind6 \
> cgroup/sysctl cgroup/getsockopt \
> cgroup/setsockopt cgroup/sock_release struct_ops \
> - fentry fexit freplace sk_lookup" -- \
> - "$cur" ) )
> + fentry fexit freplace sk_lookup'
> + COMPREPLY=( $( compgen -W \
> + "$BPFTOOL_PROG_LOAD_TYPES" -- "$cur" ) )
nit: this and similar COMPREPLY assignments now can be on a single line now, no?
> return 0
> ;;
> id)
> @@ -698,15 +701,16 @@ _bpftool()
> return 0
> ;;
> type)
> - COMPREPLY=( $( compgen -W 'hash array prog_array \
> - perf_event_array percpu_hash percpu_array \
> - stack_trace cgroup_array lru_hash \
> + local BPFTOOL_MAP_CREATE_TYPES='hash array \
> + prog_array perf_event_array percpu_hash \
> + percpu_array stack_trace cgroup_array lru_hash \
> lru_percpu_hash lpm_trie array_of_maps \
> hash_of_maps devmap devmap_hash sockmap cpumap \
> xskmap sockhash cgroup_storage reuseport_sockarray \
> percpu_cgroup_storage queue stack sk_storage \
> - struct_ops inode_storage task_storage' -- \
> - "$cur" ) )
> + struct_ops inode_storage task_storage'
> + COMPREPLY=( $( compgen -W \
> + "$BPFTOOL_MAP_CREATE_TYPES" -- "$cur" ) )
> return 0
> ;;
> key|value|flags|entries)
> @@ -1017,34 +1021,37 @@ _bpftool()
> return 0
> ;;
> attach|detach)
> - local ATTACH_TYPES='ingress egress sock_create sock_ops \
> - device bind4 bind6 post_bind4 post_bind6 connect4 connect6 \
> + local BPFTOOL_CGROUP_ATTACH_TYPES='ingress egress \
> + sock_create sock_ops device \
> + bind4 bind6 post_bind4 post_bind6 connect4 connect6 \
> getpeername4 getpeername6 getsockname4 getsockname6 \
> sendmsg4 sendmsg6 recvmsg4 recvmsg6 sysctl getsockopt \
> setsockopt sock_release'
> local ATTACH_FLAGS='multi override'
> local PROG_TYPE='id pinned tag name'
> - case $prev in
> - $command)
> - _filedir
> - return 0
> - ;;
> - ingress|egress|sock_create|sock_ops|device|bind4|bind6|\
> - post_bind4|post_bind6|connect4|connect6|getpeername4|\
> - getpeername6|getsockname4|getsockname6|sendmsg4|sendmsg6|\
> - recvmsg4|recvmsg6|sysctl|getsockopt|setsockopt|sock_release)
> + # Check for $prev = $command first
> + if [ $prev = $command ]; then
> + _filedir
> + return 0
> + # Then check for attach type. This is done outside of the
> + # "case $prev in" to avoid writing the whole list of attach
> + # types again as pattern to match (where we cannot reuse
> + # our variable).
> + elif [[ $BPFTOOL_CGROUP_ATTACH_TYPES =~ $prev ]]; then
> COMPREPLY=( $( compgen -W "$PROG_TYPE" -- \
> "$cur" ) )
> return 0
> - ;;
> + fi
> + # case/esac for the other cases
> + case $prev in
> id)
> _bpftool_get_prog_ids
> return 0
> ;;
> *)
> - if ! _bpftool_search_list "$ATTACH_TYPES"; then
> - COMPREPLY=( $( compgen -W "$ATTACH_TYPES" -- \
> - "$cur" ) )
> + if ! _bpftool_search_list "$BPFTOOL_CGROUP_ATTACH_TYPES"; then
> + COMPREPLY=( $( compgen -W \
> + "$BPFTOOL_CGROUP_ATTACH_TYPES" -- "$cur" ) )
> elif [[ "$command" == "attach" ]]; then
> # We have an attach type on the command line,
> # but it is not the previous word, or
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates
2021-07-30 18:45 ` Andrii Nakryiko
@ 2021-07-30 21:46 ` Quentin Monnet
2021-07-30 21:54 ` Andrii Nakryiko
0 siblings, 1 reply; 19+ messages in thread
From: Quentin Monnet @ 2021-07-30 21:46 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
2021-07-30 11:45 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> Bash completion for bpftool gets two minor improvements in this patch.
>>
>> Move the detection of attach types for "bpftool cgroup attach" outside
>> of the "case/esac" bloc, where we cannot reuse our variable holding the
>> list of supported attach types as a pattern list. After the change, we
>> have only one list of cgroup attach types to update when new types are
>> added, instead of the former two lists.
>>
>> Also rename the variables holding lists of names for program types, map
>> types, and attach types, to make them more unique. This can make it
>> slightly easier to point people to the relevant variables to update, but
>> the main objective here is to help run a script to check that bash
>> completion is up-to-date with bpftool's source code.
>>
>> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
>> ---
>> tools/bpf/bpftool/bash-completion/bpftool | 57 +++++++++++++----------
>> 1 file changed, 32 insertions(+), 25 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
>> index cc33c5824a2f..b2e33a2d8524 100644
>> --- a/tools/bpf/bpftool/bash-completion/bpftool
>> +++ b/tools/bpf/bpftool/bash-completion/bpftool
>> @@ -404,8 +404,10 @@ _bpftool()
>> return 0
>> ;;
>> 5)
>> - COMPREPLY=( $( compgen -W 'msg_verdict stream_verdict \
>> - stream_parser flow_dissector' -- "$cur" ) )
>> + local BPFTOOL_PROG_ATTACH_TYPES='msg_verdict \
>> + stream_verdict stream_parser flow_dissector'
>> + COMPREPLY=( $( compgen -W \
>> + "$BPFTOOL_PROG_ATTACH_TYPES" -- "$cur" ) )
>> return 0
>> ;;
>> 6)
>> @@ -464,7 +466,7 @@ _bpftool()
>>
>> case $prev in
>> type)
>> - COMPREPLY=( $( compgen -W "socket kprobe \
>> + local BPFTOOL_PROG_LOAD_TYPES='socket kprobe \
>> kretprobe classifier flow_dissector \
>> action tracepoint raw_tracepoint \
>> xdp perf_event cgroup/skb cgroup/sock \
>> @@ -479,8 +481,9 @@ _bpftool()
>> cgroup/post_bind4 cgroup/post_bind6 \
>> cgroup/sysctl cgroup/getsockopt \
>> cgroup/setsockopt cgroup/sock_release struct_ops \
>> - fentry fexit freplace sk_lookup" -- \
>> - "$cur" ) )
>> + fentry fexit freplace sk_lookup'
>> + COMPREPLY=( $( compgen -W \
>> + "$BPFTOOL_PROG_LOAD_TYPES" -- "$cur" ) )
>
> nit: this and similar COMPREPLY assignments now can be on a single line now, no?
It will go over 80 characters, but OK, it will probably be more readable
on a single line. I'll change for v2.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates
2021-07-30 21:46 ` Quentin Monnet
@ 2021-07-30 21:54 ` Andrii Nakryiko
0 siblings, 0 replies; 19+ messages in thread
From: Andrii Nakryiko @ 2021-07-30 21:54 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
On Fri, Jul 30, 2021 at 2:47 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2021-07-30 11:45 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> Bash completion for bpftool gets two minor improvements in this patch.
> >>
> >> Move the detection of attach types for "bpftool cgroup attach" outside
> >> of the "case/esac" bloc, where we cannot reuse our variable holding the
> >> list of supported attach types as a pattern list. After the change, we
> >> have only one list of cgroup attach types to update when new types are
> >> added, instead of the former two lists.
> >>
> >> Also rename the variables holding lists of names for program types, map
> >> types, and attach types, to make them more unique. This can make it
> >> slightly easier to point people to the relevant variables to update, but
> >> the main objective here is to help run a script to check that bash
> >> completion is up-to-date with bpftool's source code.
> >>
> >> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> >> ---
> >> tools/bpf/bpftool/bash-completion/bpftool | 57 +++++++++++++----------
> >> 1 file changed, 32 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> >> index cc33c5824a2f..b2e33a2d8524 100644
> >> --- a/tools/bpf/bpftool/bash-completion/bpftool
> >> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> >> @@ -404,8 +404,10 @@ _bpftool()
> >> return 0
> >> ;;
> >> 5)
> >> - COMPREPLY=( $( compgen -W 'msg_verdict stream_verdict \
> >> - stream_parser flow_dissector' -- "$cur" ) )
> >> + local BPFTOOL_PROG_ATTACH_TYPES='msg_verdict \
> >> + stream_verdict stream_parser flow_dissector'
> >> + COMPREPLY=( $( compgen -W \
> >> + "$BPFTOOL_PROG_ATTACH_TYPES" -- "$cur" ) )
> >> return 0
> >> ;;
> >> 6)
> >> @@ -464,7 +466,7 @@ _bpftool()
> >>
> >> case $prev in
> >> type)
> >> - COMPREPLY=( $( compgen -W "socket kprobe \
> >> + local BPFTOOL_PROG_LOAD_TYPES='socket kprobe \
> >> kretprobe classifier flow_dissector \
> >> action tracepoint raw_tracepoint \
> >> xdp perf_event cgroup/skb cgroup/sock \
> >> @@ -479,8 +481,9 @@ _bpftool()
> >> cgroup/post_bind4 cgroup/post_bind6 \
> >> cgroup/sysctl cgroup/getsockopt \
> >> cgroup/setsockopt cgroup/sock_release struct_ops \
> >> - fentry fexit freplace sk_lookup" -- \
> >> - "$cur" ) )
> >> + fentry fexit freplace sk_lookup'
> >> + COMPREPLY=( $( compgen -W \
> >> + "$BPFTOOL_PROG_LOAD_TYPES" -- "$cur" ) )
> >
> > nit: this and similar COMPREPLY assignments now can be on a single line now, no?
>
> It will go over 80 characters, but OK, it will probably be more readable
> on a single line. I'll change for v2.
80 character rule was lifted a while ago. 100 is totally fine. And in
some special cases readability beats even 100, IMO.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH bpf-next 2/7] selftests/bpf: check consistency between bpftool source, doc, completion
2021-07-29 16:29 [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates Quentin Monnet
@ 2021-07-29 16:29 ` Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 3/7] tools: bpftool: complete and synchronise attach or map types Quentin Monnet
` (5 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Quentin Monnet @ 2021-07-29 16:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Quentin Monnet
Whenever the eBPF subsystem gains new elements, such as new program or
map types, it is necessary to update bpftool if we want it able to
handle the new items.
In addition to the main arrays containing the names of these elements in
the source code, there are also multiple locations to update:
- The help message in the do_help() functions in bpftool's source code.
- The RST documentation files.
- The bash completion file.
This has led to omissions multiple times in the past. This patch
attempts to address this issue by adding consistency checks for all
these different locations. It also verifies that the bpf_prog_type,
bpf_map_type and bpf_attach_type enums from the UAPI BPF header have all
their members present in bpftool.
The script requires no argument to run, it reads and parses the
different files to check, and prints the mismatches, if any. It
currently reports a number of missing elements, which will be fixed in a
later patch:
$ ./test_bpftool_synctypes.py
Comparing [...]/linux/tools/bpf/bpftool/map.c (map_type_name) and [...]/linux/tools/bpf/bpftool/bash-completion/bpftool (BPFTOOL_MAP_CREATE_TYPES): {'ringbuf'}
Comparing BPF header (enum bpf_attach_type) and [...]/linux/tools/bpf/bpftool/common.c (attach_type_name): {'BPF_TRACE_ITER', 'BPF_XDP_DEVMAP', 'BPF_XDP', 'BPF_SK_REUSEPORT_SELECT', 'BPF_XDP_CPUMAP', 'BPF_SK_REUSEPORT_SELECT_OR_MIGRATE'}
Comparing [...]/linux/tools/bpf/bpftool/prog.c (attach_type_strings) and [...]/linux/tools/bpf/bpftool/prog.c (do_help() ATTACH_TYPE): {'skb_verdict'}
Comparing [...]/linux/tools/bpf/bpftool/prog.c (attach_type_strings) and [...]/linux/tools/bpf/bpftool/Documentation/bpftool-prog.rst (ATTACH_TYPE): {'skb_verdict'}
Comparing [...]/linux/tools/bpf/bpftool/prog.c (attach_type_strings) and [...]/linux/tools/bpf/bpftool/bash-completion/bpftool (BPFTOOL_PROG_ATTACH_TYPES): {'skb_verdict'}
Note that the script does NOT check for consistency between the list of
program types that bpftool claims it accepts and the actual list of
keywords that can be used. This is because bpftool does not "see" them,
they are ELF section names parsed by libbpf. It is not hard to parse the
section_defs[] array in libbpf, but some section names are associated
with program types that bpftool cannot load at the moment. For example,
some programs require a BTF target and an attach target that bpftool
cannot handle. The script may be extended to parse the array and check
only relevant values in the future.
In order to help maintain a good level of consistency for bpftool, the
script is added to the selftest suite for BPF.
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/testing/selftests/bpf/Makefile | 1 +
.../selftests/bpf/test_bpftool_synctypes.py | 486 ++++++++++++++++++
2 files changed, 487 insertions(+)
create mode 100755 tools/testing/selftests/bpf/test_bpftool_synctypes.py
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index f405b20c1e6c..d4955d056d19 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -72,6 +72,7 @@ TEST_PROGS := test_kmod.sh \
test_tc_edt.sh \
test_xdping.sh \
test_bpftool_build.sh \
+ test_bpftool_synctypes.py \
test_bpftool.sh \
test_bpftool_metadata.sh \
test_doc_build.sh \
diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
new file mode 100755
index 000000000000..b41464f46b3b
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
@@ -0,0 +1,486 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+#
+# Copyright (C) 2021 Isovalent, Inc.
+
+import argparse
+import re
+import os, sys
+
+LINUX_ROOT = os.path.abspath(os.path.join(__file__,
+ os.pardir, os.pardir, os.pardir, os.pardir, os.pardir))
+BPFTOOL_DIR = os.path.join(LINUX_ROOT, 'tools/bpf/bpftool')
+retval = 0
+
+class BlockParser(object):
+ """
+ A parser for extracting set of values from blocks such as enums.
+ @reader: a pointer to the open file to parse
+ """
+ def __init__(self, reader):
+ self.reader = reader
+
+ def search_block(self, start_marker):
+ """
+ Search for a given structure in a file.
+ @start_marker: regex marking the beginning of a structure to parse
+ """
+ offset = self.reader.tell()
+ array_start = re.search(start_marker, self.reader.read())
+ if array_start is None:
+ raise Exception('Failed to find start of block')
+ self.reader.seek(offset + array_start.start())
+
+ def parse(self, pattern, end_marker):
+ """
+ Parse a block and return a set of values. Values to extract must be
+ on separate lines in the file.
+ @pattern: pattern used to identify the values to extract
+ @end_marker: regex marking the end of the block to parse
+ """
+ entries = set()
+ while True:
+ line = self.reader.readline()
+ if not line or re.match(end_marker, line):
+ break
+ capture = pattern.search(line)
+ if capture and pattern.groups >= 1:
+ entries.add(capture.group(1))
+ return entries
+
+class ArrayParser(BlockParser):
+ """
+ A parser for extracting dicionaries of values from some BPF-related arrays.
+ @reader: a pointer to the open file to parse
+ @array_name: name of the array to parse
+ """
+ end_marker = re.compile('^};')
+
+ def __init__(self, reader, array_name):
+ self.array_name = array_name
+ self.start_marker = re.compile(f'(static )?const char \* const {self.array_name}\[.*\] = {{\n')
+ super().__init__(reader)
+
+ def search_block(self):
+ """
+ Search for the given array in a file.
+ """
+ super().search_block(self.start_marker);
+
+ def parse(self):
+ """
+ Parse a block and return data as a dictionary. Items to extract must be
+ on separate lines in the file.
+ """
+ pattern = re.compile('\[(BPF_\w*)\]\s*= "(.*)",?$')
+ entries = {}
+ while True:
+ line = self.reader.readline()
+ if line == '' or re.match(self.end_marker, line):
+ break
+ capture = pattern.search(line)
+ if capture:
+ entries[capture.group(1)] = capture.group(2)
+ return entries
+
+class InlineListParser(BlockParser):
+ """
+ A parser for extracting set of values from inline lists.
+ """
+ def parse(self, pattern, end_marker):
+ """
+ Parse a block and return a set of values. Multiple values to extract
+ can be on a same line in the file.
+ @pattern: pattern used to identify the values to extract
+ @end_marker: regex marking the end of the block to parse
+ """
+ entries = set()
+ while True:
+ line = self.reader.readline()
+ if not line:
+ break
+ entries.update(pattern.findall(line))
+ if re.search(end_marker, line):
+ break
+ return entries
+
+class FileExtractor(object):
+ """
+ A generic reader for extracting data from a given file. This class contains
+ several helper methods that wrap arround parser objects to extract values
+ from different structures.
+ This class does not offer a way to set a filename, which is expected to be
+ defined in children classes.
+ """
+ def __init__(self):
+ self.reader = open(self.filename, 'r')
+
+ def close(self):
+ """
+ Close the file used by the parser.
+ """
+ self.reader.close()
+
+ def reset_read(self):
+ """
+ Reset the file position indicator for this parser. This is useful when
+ parsing several structures in the file without respecting the order in
+ which those structures appear in the file.
+ """
+ self.reader.seek(0)
+
+ def get_types_from_array(self, array_name):
+ """
+ Search for and parse an array associating names to BPF_* enum members,
+ for example:
+
+ const char * const prog_type_name[] = {
+ [BPF_PROG_TYPE_UNSPEC] = "unspec",
+ [BPF_PROG_TYPE_SOCKET_FILTER] = "socket_filter",
+ [BPF_PROG_TYPE_KPROBE] = "kprobe",
+ };
+
+ Return a dictionary with the enum member names as keys and the
+ associated names as values, for example:
+
+ {'BPF_PROG_TYPE_UNSPEC': 'unspec',
+ 'BPF_PROG_TYPE_SOCKET_FILTER': 'socket_filter',
+ 'BPF_PROG_TYPE_KPROBE': 'kprobe'}
+
+ @array_name: name of the array to parse
+ """
+ array_parser = ArrayParser(self.reader, array_name)
+ array_parser.search_block()
+ return array_parser.parse()
+
+ def get_enum(self, enum_name):
+ """
+ Search for and parse an enum containing BPF_* members, for example:
+
+ enum bpf_prog_type {
+ BPF_PROG_TYPE_UNSPEC,
+ BPF_PROG_TYPE_SOCKET_FILTER,
+ BPF_PROG_TYPE_KPROBE,
+ };
+
+ Return a set containing all member names, for example:
+
+ {'BPF_PROG_TYPE_UNSPEC',
+ 'BPF_PROG_TYPE_SOCKET_FILTER',
+ 'BPF_PROG_TYPE_KPROBE'}
+
+ @enum_name: name of the enum to parse
+ """
+ start_marker = re.compile(f'enum {enum_name} {{\n')
+ pattern = re.compile('^\s*(BPF_\w+),?$')
+ end_marker = re.compile('^};')
+ parser = BlockParser(self.reader)
+ parser.search_block(start_marker)
+ return parser.parse(pattern, end_marker)
+
+ def __get_description_list(self, start_marker, pattern, end_marker):
+ parser = InlineListParser(self.reader)
+ parser.search_block(start_marker)
+ return parser.parse(pattern, end_marker)
+
+ def get_rst_list(self, block_name):
+ """
+ Search for and parse a list of type names from RST documentation, for
+ example:
+
+ | *TYPE* := {
+ | **socket** | **kprobe** |
+ | **kretprobe**
+ | }
+
+ Return a set containing all type names, for example:
+
+ {'socket', 'kprobe', 'kretprobe'}
+
+ @block_name: name of the blog to parse, 'TYPE' in the example
+ """
+ start_marker = re.compile(f'\*{block_name}\* := {{')
+ pattern = re.compile('\*\*([\w/]+)\*\*')
+ end_marker = re.compile('}\n')
+ return self.__get_description_list(start_marker, pattern, end_marker)
+
+ def get_help_list(self, block_name):
+ """
+ Search for and parse a list of type names from a help message in
+ bpftool, for example:
+
+ " TYPE := { socket | kprobe |\\n"
+ " kretprobe }\\n"
+
+ Return a set containing all type names, for example:
+
+ {'socket', 'kprobe', 'kretprobe'}
+
+ @block_name: name of the blog to parse, 'TYPE' in the example
+ """
+ start_marker = re.compile(f'"\s*{block_name} := {{')
+ pattern = re.compile('([\w/]+) [|}]')
+ end_marker = re.compile('}')
+ return self.__get_description_list(start_marker, pattern, end_marker)
+
+ def get_bashcomp_list(self, block_name):
+ """
+ Search for and parse a list of type names from a variable in bash
+ completion file, for example:
+
+ local BPFTOOL_PROG_LOAD_TYPES='socket kprobe \\
+ kretprobe'
+
+ Return a set containing all type names, for example:
+
+ {'socket', 'kprobe', 'kretprobe'}
+
+ @block_name: name of the blog to parse, 'TYPE' in the example
+ """
+ start_marker = re.compile(f'local {block_name}=\'')
+ pattern = re.compile('(?:.*=\')?([\w/]+)')
+ end_marker = re.compile('\'$')
+ return self.__get_description_list(start_marker, pattern, end_marker)
+
+class ProgFileExtractor(FileExtractor):
+ """
+ An extractor for bpftool's prog.c.
+ """
+ filename = os.path.join(BPFTOOL_DIR, 'prog.c')
+
+ def get_prog_types(self):
+ return self.get_types_from_array('prog_type_name')
+
+ def get_attach_types(self):
+ return self.get_types_from_array('attach_type_strings')
+
+ def get_prog_attach_help(self):
+ return self.get_help_list('ATTACH_TYPE')
+
+class MapFileExtractor(FileExtractor):
+ """
+ An extractor for bpftool's map.c.
+ """
+ filename = os.path.join(BPFTOOL_DIR, 'map.c')
+
+ def get_map_types(self):
+ return self.get_types_from_array('map_type_name')
+
+ def get_map_help(self):
+ return self.get_help_list('TYPE')
+
+class CgroupFileExtractor(FileExtractor):
+ """
+ An extractor for bpftool's cgroup.c.
+ """
+ filename = os.path.join(BPFTOOL_DIR, 'cgroup.c')
+
+ def get_prog_attach_help(self):
+ return self.get_help_list('ATTACH_TYPE')
+
+class CommonFileExtractor(FileExtractor):
+ """
+ An extractor for bpftool's common.c.
+ """
+ filename = os.path.join(BPFTOOL_DIR, 'common.c')
+
+ def __init__(self):
+ super().__init__()
+ self.attach_types = {}
+
+ def get_attach_types(self):
+ if not self.attach_types:
+ self.attach_types = self.get_types_from_array('attach_type_name')
+ return self.attach_types
+
+ def get_cgroup_attach_types(self):
+ if not self.attach_types:
+ self.get_attach_types()
+ cgroup_types = {}
+ for (key, value) in self.attach_types.items():
+ if key.find('BPF_CGROUP') != -1:
+ cgroup_types[key] = value
+ return cgroup_types
+
+class BpfHeaderExtractor(FileExtractor):
+ """
+ An extractor for the UAPI BPF header.
+ """
+ filename = os.path.join(LINUX_ROOT, 'tools/include/uapi/linux/bpf.h')
+
+ def get_prog_types(self):
+ return self.get_enum('bpf_prog_type')
+
+ def get_map_types(self):
+ return self.get_enum('bpf_map_type')
+
+ def get_attach_types(self):
+ return self.get_enum('bpf_attach_type')
+
+class ManProgExtractor(FileExtractor):
+ """
+ An extractor for bpftool-prog.rst.
+ """
+ filename = os.path.join(BPFTOOL_DIR, 'Documentation/bpftool-prog.rst')
+
+ def get_attach_types(self):
+ return self.get_rst_list('ATTACH_TYPE')
+
+class ManMapExtractor(FileExtractor):
+ """
+ An extractor for bpftool-map.rst.
+ """
+ filename = os.path.join(BPFTOOL_DIR, 'Documentation/bpftool-map.rst')
+
+ def get_map_types(self):
+ return self.get_rst_list('TYPE')
+
+class ManCgroupExtractor(FileExtractor):
+ """
+ An extractor for bpftool-cgroup.rst.
+ """
+ filename = os.path.join(BPFTOOL_DIR, 'Documentation/bpftool-cgroup.rst')
+
+ def get_attach_types(self):
+ return self.get_rst_list('ATTACH_TYPE')
+
+class BashcompExtractor(FileExtractor):
+ """
+ An extractor for bpftool's bash completion file.
+ """
+ filename = os.path.join(BPFTOOL_DIR, 'bash-completion/bpftool')
+
+ def get_prog_attach_types(self):
+ return self.get_bashcomp_list('BPFTOOL_PROG_ATTACH_TYPES')
+
+ def get_map_types(self):
+ return self.get_bashcomp_list('BPFTOOL_MAP_CREATE_TYPES')
+
+ def get_cgroup_attach_types(self):
+ return self.get_bashcomp_list('BPFTOOL_CGROUP_ATTACH_TYPES')
+
+def verify(first_set, second_set, message):
+ """
+ Print all values that differ between two sets.
+ @first_set: one set to compare
+ @second_set: another set to compare
+ @message: message to print for values belonging to only one of the sets
+ """
+ global retval
+ diff = first_set.symmetric_difference(second_set)
+ if diff:
+ print(message, diff)
+ retval = 1
+
+def main():
+ # No arguments supported at this time, but print usage for -h|--help
+ argParser = argparse.ArgumentParser(description="""
+ Verify that bpftool's code, help messages, documentation and bash completion
+ are all in sync on program types, map types and attach types. Also check that
+ bpftool is in sync with the UAPI BPF header.
+ """)
+ args = argParser.parse_args()
+
+ # Map types (enum)
+
+ bpf_info = BpfHeaderExtractor()
+ ref = bpf_info.get_map_types()
+
+ map_info = MapFileExtractor()
+ source_map_items = map_info.get_map_types()
+ map_types_enum = set(source_map_items.keys())
+
+ verify(ref, map_types_enum,
+ f'Comparing BPF header (enum bpf_map_type) and {MapFileExtractor.filename} (map_type_name):')
+
+ # Map types (names)
+
+ source_map_types = set(source_map_items.values())
+ source_map_types.discard('unspec')
+
+ help_map_types = map_info.get_map_help()
+ map_info.close()
+
+ man_map_info = ManMapExtractor()
+ man_map_types = man_map_info.get_map_types()
+ man_map_info.close()
+
+ bashcomp_info = BashcompExtractor()
+ bashcomp_map_types = bashcomp_info.get_map_types()
+
+ verify(source_map_types, help_map_types,
+ f'Comparing {MapFileExtractor.filename} (map_type_name) and {MapFileExtractor.filename} (do_help() TYPE):')
+ verify(source_map_types, man_map_types,
+ f'Comparing {MapFileExtractor.filename} (map_type_name) and {ManMapExtractor.filename} (TYPE):')
+ verify(source_map_types, bashcomp_map_types,
+ f'Comparing {MapFileExtractor.filename} (map_type_name) and {BashcompExtractor.filename} (BPFTOOL_MAP_CREATE_TYPES):')
+
+ # Program types (enum)
+
+ ref = bpf_info.get_prog_types()
+
+ prog_info = ProgFileExtractor()
+ prog_types = set(prog_info.get_prog_types().keys())
+
+ verify(ref, prog_types,
+ f'Comparing BPF header (enum bpf_prog_type) and {ProgFileExtractor.filename} (prog_type_name):')
+
+ # Attach types (enum)
+
+ ref = bpf_info.get_attach_types()
+ bpf_info.close()
+
+ common_info = CommonFileExtractor()
+ attach_types = common_info.get_attach_types()
+
+ verify(ref, attach_types,
+ f'Comparing BPF header (enum bpf_attach_type) and {CommonFileExtractor.filename} (attach_type_name):')
+
+ # Attach types (names)
+
+ source_prog_attach_types = set(prog_info.get_attach_types().values())
+
+ help_prog_attach_types = prog_info.get_prog_attach_help()
+ prog_info.close()
+
+ man_prog_info = ManProgExtractor()
+ man_prog_attach_types = man_prog_info.get_attach_types()
+ man_prog_info.close()
+
+ bashcomp_info.reset_read() # We stopped at map types, rewind
+ bashcomp_prog_attach_types = bashcomp_info.get_prog_attach_types()
+
+ verify(source_prog_attach_types, help_prog_attach_types,
+ f'Comparing {ProgFileExtractor.filename} (attach_type_strings) and {ProgFileExtractor.filename} (do_help() ATTACH_TYPE):')
+ verify(source_prog_attach_types, man_prog_attach_types,
+ f'Comparing {ProgFileExtractor.filename} (attach_type_strings) and {ManProgExtractor.filename} (ATTACH_TYPE):')
+ verify(source_prog_attach_types, bashcomp_prog_attach_types,
+ f'Comparing {ProgFileExtractor.filename} (attach_type_strings) and {BashcompExtractor.filename} (BPFTOOL_PROG_ATTACH_TYPES):')
+
+ # Cgroup attach types
+
+ source_cgroup_attach_types = set(common_info.get_cgroup_attach_types().values())
+ common_info.close()
+
+ cgroup_info = CgroupFileExtractor()
+ help_cgroup_attach_types = cgroup_info.get_prog_attach_help()
+ cgroup_info.close()
+
+ man_cgroup_info = ManCgroupExtractor()
+ man_cgroup_attach_types = man_cgroup_info.get_attach_types()
+ man_cgroup_info.close()
+
+ bashcomp_cgroup_attach_types = bashcomp_info.get_cgroup_attach_types()
+ bashcomp_info.close()
+
+ verify(source_cgroup_attach_types, help_cgroup_attach_types,
+ f'Comparing {CommonFileExtractor.filename} (attach_type_strings) and {CgroupFileExtractor.filename} (do_help() ATTACH_TYPE):')
+ verify(source_cgroup_attach_types, man_cgroup_attach_types,
+ f'Comparing {CommonFileExtractor.filename} (attach_type_strings) and {ManCgroupExtractor.filename} (ATTACH_TYPE):')
+ verify(source_cgroup_attach_types, bashcomp_cgroup_attach_types,
+ f'Comparing {CommonFileExtractor.filename} (attach_type_strings) and {BashcompExtractor.filename} (BPFTOOL_CGROUP_ATTACH_TYPES):')
+
+ sys.exit(retval)
+
+if __name__ == "__main__":
+ main()
--
2.30.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH bpf-next 3/7] tools: bpftool: complete and synchronise attach or map types
2021-07-29 16:29 [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 2/7] selftests/bpf: check consistency between bpftool source, doc, completion Quentin Monnet
@ 2021-07-29 16:29 ` Quentin Monnet
2021-07-30 18:52 ` Andrii Nakryiko
2021-07-29 16:29 ` [PATCH bpf-next 4/7] tools: bpftool: update and synchronise option list in doc and help msg Quentin Monnet
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Quentin Monnet @ 2021-07-29 16:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Quentin Monnet
Update bpftool's list of attach type names to tell it about the latest
attach types, or the "ringbuf" map. Also update the documentation, help
messages, and bash completion when relevant.
These missing items were reported by the newly added Python script used
to help maintain consistency in bpftool.
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 2 +-
tools/bpf/bpftool/bash-completion/bpftool | 5 +-
tools/bpf/bpftool/common.c | 76 ++++++++++---------
tools/bpf/bpftool/prog.c | 4 +-
4 files changed, 47 insertions(+), 40 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index fe1b38e7e887..abf5f4cd7d3e 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -48,7 +48,7 @@ PROG COMMANDS
| **struct_ops** | **fentry** | **fexit** | **freplace** | **sk_lookup**
| }
| *ATTACH_TYPE* := {
-| **msg_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
+| **msg_verdict** | **skb_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
| }
| *METRICs* := {
| **cycles** | **instructions** | **l1d_loads** | **llc_misses**
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index b2e33a2d8524..69d018474537 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -405,7 +405,8 @@ _bpftool()
;;
5)
local BPFTOOL_PROG_ATTACH_TYPES='msg_verdict \
- stream_verdict stream_parser flow_dissector'
+ skb_verdict stream_verdict stream_parser \
+ flow_dissector'
COMPREPLY=( $( compgen -W \
"$BPFTOOL_PROG_ATTACH_TYPES" -- "$cur" ) )
return 0
@@ -708,7 +709,7 @@ _bpftool()
hash_of_maps devmap devmap_hash sockmap cpumap \
xskmap sockhash cgroup_storage reuseport_sockarray \
percpu_cgroup_storage queue stack sk_storage \
- struct_ops inode_storage task_storage'
+ struct_ops inode_storage task_storage ringbuf'
COMPREPLY=( $( compgen -W \
"$BPFTOOL_MAP_CREATE_TYPES" -- "$cur" ) )
return 0
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 1828bba19020..b47797cac64f 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -31,42 +31,48 @@
#endif
const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE] = {
- [BPF_CGROUP_INET_INGRESS] = "ingress",
- [BPF_CGROUP_INET_EGRESS] = "egress",
- [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
- [BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
- [BPF_CGROUP_SOCK_OPS] = "sock_ops",
- [BPF_CGROUP_DEVICE] = "device",
- [BPF_CGROUP_INET4_BIND] = "bind4",
- [BPF_CGROUP_INET6_BIND] = "bind6",
- [BPF_CGROUP_INET4_CONNECT] = "connect4",
- [BPF_CGROUP_INET6_CONNECT] = "connect6",
- [BPF_CGROUP_INET4_POST_BIND] = "post_bind4",
- [BPF_CGROUP_INET6_POST_BIND] = "post_bind6",
- [BPF_CGROUP_INET4_GETPEERNAME] = "getpeername4",
- [BPF_CGROUP_INET6_GETPEERNAME] = "getpeername6",
- [BPF_CGROUP_INET4_GETSOCKNAME] = "getsockname4",
- [BPF_CGROUP_INET6_GETSOCKNAME] = "getsockname6",
- [BPF_CGROUP_UDP4_SENDMSG] = "sendmsg4",
- [BPF_CGROUP_UDP6_SENDMSG] = "sendmsg6",
- [BPF_CGROUP_SYSCTL] = "sysctl",
- [BPF_CGROUP_UDP4_RECVMSG] = "recvmsg4",
- [BPF_CGROUP_UDP6_RECVMSG] = "recvmsg6",
- [BPF_CGROUP_GETSOCKOPT] = "getsockopt",
- [BPF_CGROUP_SETSOCKOPT] = "setsockopt",
+ [BPF_CGROUP_INET_INGRESS] = "ingress",
+ [BPF_CGROUP_INET_EGRESS] = "egress",
+ [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
+ [BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
+ [BPF_CGROUP_SOCK_OPS] = "sock_ops",
+ [BPF_CGROUP_DEVICE] = "device",
+ [BPF_CGROUP_INET4_BIND] = "bind4",
+ [BPF_CGROUP_INET6_BIND] = "bind6",
+ [BPF_CGROUP_INET4_CONNECT] = "connect4",
+ [BPF_CGROUP_INET6_CONNECT] = "connect6",
+ [BPF_CGROUP_INET4_POST_BIND] = "post_bind4",
+ [BPF_CGROUP_INET6_POST_BIND] = "post_bind6",
+ [BPF_CGROUP_INET4_GETPEERNAME] = "getpeername4",
+ [BPF_CGROUP_INET6_GETPEERNAME] = "getpeername6",
+ [BPF_CGROUP_INET4_GETSOCKNAME] = "getsockname4",
+ [BPF_CGROUP_INET6_GETSOCKNAME] = "getsockname6",
+ [BPF_CGROUP_UDP4_SENDMSG] = "sendmsg4",
+ [BPF_CGROUP_UDP6_SENDMSG] = "sendmsg6",
+ [BPF_CGROUP_SYSCTL] = "sysctl",
+ [BPF_CGROUP_UDP4_RECVMSG] = "recvmsg4",
+ [BPF_CGROUP_UDP6_RECVMSG] = "recvmsg6",
+ [BPF_CGROUP_GETSOCKOPT] = "getsockopt",
+ [BPF_CGROUP_SETSOCKOPT] = "setsockopt",
- [BPF_SK_SKB_STREAM_PARSER] = "sk_skb_stream_parser",
- [BPF_SK_SKB_STREAM_VERDICT] = "sk_skb_stream_verdict",
- [BPF_SK_SKB_VERDICT] = "sk_skb_verdict",
- [BPF_SK_MSG_VERDICT] = "sk_msg_verdict",
- [BPF_LIRC_MODE2] = "lirc_mode2",
- [BPF_FLOW_DISSECTOR] = "flow_dissector",
- [BPF_TRACE_RAW_TP] = "raw_tp",
- [BPF_TRACE_FENTRY] = "fentry",
- [BPF_TRACE_FEXIT] = "fexit",
- [BPF_MODIFY_RETURN] = "mod_ret",
- [BPF_LSM_MAC] = "lsm_mac",
- [BPF_SK_LOOKUP] = "sk_lookup",
+ [BPF_SK_SKB_STREAM_PARSER] = "sk_skb_stream_parser",
+ [BPF_SK_SKB_STREAM_VERDICT] = "sk_skb_stream_verdict",
+ [BPF_SK_SKB_VERDICT] = "sk_skb_verdict",
+ [BPF_SK_MSG_VERDICT] = "sk_msg_verdict",
+ [BPF_LIRC_MODE2] = "lirc_mode2",
+ [BPF_FLOW_DISSECTOR] = "flow_dissector",
+ [BPF_TRACE_RAW_TP] = "raw_tp",
+ [BPF_TRACE_FENTRY] = "fentry",
+ [BPF_TRACE_FEXIT] = "fexit",
+ [BPF_MODIFY_RETURN] = "mod_ret",
+ [BPF_LSM_MAC] = "lsm_mac",
+ [BPF_SK_LOOKUP] = "sk_lookup",
+ [BPF_TRACE_ITER] = "trace_iter",
+ [BPF_XDP_DEVMAP] = "xdp_devmap",
+ [BPF_XDP_CPUMAP] = "xdp_cpumap",
+ [BPF_XDP] = "xdp",
+ [BPF_SK_REUSEPORT_SELECT] = "sk_skb_reuseport_select",
+ [BPF_SK_REUSEPORT_SELECT_OR_MIGRATE] = "sk_skb_reuseport_select_or_migrate",
};
void p_err(const char *fmt, ...)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index cc48726740ad..1ee87225543b 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -2245,8 +2245,8 @@ static int do_help(int argc, char **argv)
" cgroup/sendmsg6 | cgroup/recvmsg4 | cgroup/recvmsg6 |\n"
" cgroup/getsockopt | cgroup/setsockopt | cgroup/sock_release |\n"
" struct_ops | fentry | fexit | freplace | sk_lookup }\n"
- " ATTACH_TYPE := { msg_verdict | stream_verdict | stream_parser |\n"
- " flow_dissector }\n"
+ " ATTACH_TYPE := { msg_verdict | skb_verdict | stream_verdict |\n"
+ " stream_parser | flow_dissector }\n"
" METRIC := { cycles | instructions | l1d_loads | llc_misses | itlb_misses | dtlb_misses }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
--
2.30.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH bpf-next 3/7] tools: bpftool: complete and synchronise attach or map types
2021-07-29 16:29 ` [PATCH bpf-next 3/7] tools: bpftool: complete and synchronise attach or map types Quentin Monnet
@ 2021-07-30 18:52 ` Andrii Nakryiko
2021-07-30 21:47 ` Quentin Monnet
0 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2021-07-30 18:52 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> Update bpftool's list of attach type names to tell it about the latest
> attach types, or the "ringbuf" map. Also update the documentation, help
> messages, and bash completion when relevant.
>
> These missing items were reported by the newly added Python script used
> to help maintain consistency in bpftool.
>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
> .../bpftool/Documentation/bpftool-prog.rst | 2 +-
> tools/bpf/bpftool/bash-completion/bpftool | 5 +-
> tools/bpf/bpftool/common.c | 76 ++++++++++---------
> tools/bpf/bpftool/prog.c | 4 +-
> 4 files changed, 47 insertions(+), 40 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index fe1b38e7e887..abf5f4cd7d3e 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -48,7 +48,7 @@ PROG COMMANDS
> | **struct_ops** | **fentry** | **fexit** | **freplace** | **sk_lookup**
> | }
> | *ATTACH_TYPE* := {
> -| **msg_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
> +| **msg_verdict** | **skb_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
> | }
> | *METRICs* := {
> | **cycles** | **instructions** | **l1d_loads** | **llc_misses**
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index b2e33a2d8524..69d018474537 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -405,7 +405,8 @@ _bpftool()
> ;;
> 5)
> local BPFTOOL_PROG_ATTACH_TYPES='msg_verdict \
> - stream_verdict stream_parser flow_dissector'
> + skb_verdict stream_verdict stream_parser \
> + flow_dissector'
> COMPREPLY=( $( compgen -W \
> "$BPFTOOL_PROG_ATTACH_TYPES" -- "$cur" ) )
> return 0
> @@ -708,7 +709,7 @@ _bpftool()
> hash_of_maps devmap devmap_hash sockmap cpumap \
> xskmap sockhash cgroup_storage reuseport_sockarray \
> percpu_cgroup_storage queue stack sk_storage \
> - struct_ops inode_storage task_storage'
> + struct_ops inode_storage task_storage ringbuf'
> COMPREPLY=( $( compgen -W \
> "$BPFTOOL_MAP_CREATE_TYPES" -- "$cur" ) )
> return 0
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index 1828bba19020..b47797cac64f 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -31,42 +31,48 @@
> #endif
>
> const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE] = {
> - [BPF_CGROUP_INET_INGRESS] = "ingress",
> - [BPF_CGROUP_INET_EGRESS] = "egress",
> - [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
> - [BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
> - [BPF_CGROUP_SOCK_OPS] = "sock_ops",
> - [BPF_CGROUP_DEVICE] = "device",
> - [BPF_CGROUP_INET4_BIND] = "bind4",
> - [BPF_CGROUP_INET6_BIND] = "bind6",
> - [BPF_CGROUP_INET4_CONNECT] = "connect4",
> - [BPF_CGROUP_INET6_CONNECT] = "connect6",
> - [BPF_CGROUP_INET4_POST_BIND] = "post_bind4",
> - [BPF_CGROUP_INET6_POST_BIND] = "post_bind6",
> - [BPF_CGROUP_INET4_GETPEERNAME] = "getpeername4",
> - [BPF_CGROUP_INET6_GETPEERNAME] = "getpeername6",
> - [BPF_CGROUP_INET4_GETSOCKNAME] = "getsockname4",
> - [BPF_CGROUP_INET6_GETSOCKNAME] = "getsockname6",
> - [BPF_CGROUP_UDP4_SENDMSG] = "sendmsg4",
> - [BPF_CGROUP_UDP6_SENDMSG] = "sendmsg6",
> - [BPF_CGROUP_SYSCTL] = "sysctl",
> - [BPF_CGROUP_UDP4_RECVMSG] = "recvmsg4",
> - [BPF_CGROUP_UDP6_RECVMSG] = "recvmsg6",
> - [BPF_CGROUP_GETSOCKOPT] = "getsockopt",
> - [BPF_CGROUP_SETSOCKOPT] = "setsockopt",
> + [BPF_CGROUP_INET_INGRESS] = "ingress",
> + [BPF_CGROUP_INET_EGRESS] = "egress",
> + [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
> + [BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
> + [BPF_CGROUP_SOCK_OPS] = "sock_ops",
> + [BPF_CGROUP_DEVICE] = "device",
> + [BPF_CGROUP_INET4_BIND] = "bind4",
> + [BPF_CGROUP_INET6_BIND] = "bind6",
> + [BPF_CGROUP_INET4_CONNECT] = "connect4",
> + [BPF_CGROUP_INET6_CONNECT] = "connect6",
> + [BPF_CGROUP_INET4_POST_BIND] = "post_bind4",
> + [BPF_CGROUP_INET6_POST_BIND] = "post_bind6",
> + [BPF_CGROUP_INET4_GETPEERNAME] = "getpeername4",
> + [BPF_CGROUP_INET6_GETPEERNAME] = "getpeername6",
> + [BPF_CGROUP_INET4_GETSOCKNAME] = "getsockname4",
> + [BPF_CGROUP_INET6_GETSOCKNAME] = "getsockname6",
> + [BPF_CGROUP_UDP4_SENDMSG] = "sendmsg4",
> + [BPF_CGROUP_UDP6_SENDMSG] = "sendmsg6",
> + [BPF_CGROUP_SYSCTL] = "sysctl",
> + [BPF_CGROUP_UDP4_RECVMSG] = "recvmsg4",
> + [BPF_CGROUP_UDP6_RECVMSG] = "recvmsg6",
> + [BPF_CGROUP_GETSOCKOPT] = "getsockopt",
> + [BPF_CGROUP_SETSOCKOPT] = "setsockopt",
>
> - [BPF_SK_SKB_STREAM_PARSER] = "sk_skb_stream_parser",
> - [BPF_SK_SKB_STREAM_VERDICT] = "sk_skb_stream_verdict",
> - [BPF_SK_SKB_VERDICT] = "sk_skb_verdict",
> - [BPF_SK_MSG_VERDICT] = "sk_msg_verdict",
> - [BPF_LIRC_MODE2] = "lirc_mode2",
> - [BPF_FLOW_DISSECTOR] = "flow_dissector",
> - [BPF_TRACE_RAW_TP] = "raw_tp",
> - [BPF_TRACE_FENTRY] = "fentry",
> - [BPF_TRACE_FEXIT] = "fexit",
> - [BPF_MODIFY_RETURN] = "mod_ret",
> - [BPF_LSM_MAC] = "lsm_mac",
> - [BPF_SK_LOOKUP] = "sk_lookup",
> + [BPF_SK_SKB_STREAM_PARSER] = "sk_skb_stream_parser",
> + [BPF_SK_SKB_STREAM_VERDICT] = "sk_skb_stream_verdict",
> + [BPF_SK_SKB_VERDICT] = "sk_skb_verdict",
> + [BPF_SK_MSG_VERDICT] = "sk_msg_verdict",
> + [BPF_LIRC_MODE2] = "lirc_mode2",
> + [BPF_FLOW_DISSECTOR] = "flow_dissector",
> + [BPF_TRACE_RAW_TP] = "raw_tp",
> + [BPF_TRACE_FENTRY] = "fentry",
> + [BPF_TRACE_FEXIT] = "fexit",
> + [BPF_MODIFY_RETURN] = "mod_ret",
> + [BPF_LSM_MAC] = "lsm_mac",
> + [BPF_SK_LOOKUP] = "sk_lookup",
> + [BPF_TRACE_ITER] = "trace_iter",
> + [BPF_XDP_DEVMAP] = "xdp_devmap",
> + [BPF_XDP_CPUMAP] = "xdp_cpumap",
> + [BPF_XDP] = "xdp",
> + [BPF_SK_REUSEPORT_SELECT] = "sk_skb_reuseport_select",
> + [BPF_SK_REUSEPORT_SELECT_OR_MIGRATE] = "sk_skb_reuseport_select_or_migrate",
> };
>
you are ruining Git blaming abilities for purely aesthetic reasons,
which are not good enough reasons, IMO. Please don't do this, this
nice alignment is nice, but definitely not necessary. So whatever is
longer then the "default indentation", just add another tab or two and
be done with it. That way we can actually see what was added in this
patch, btw.
> void p_err(const char *fmt, ...)
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index cc48726740ad..1ee87225543b 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -2245,8 +2245,8 @@ static int do_help(int argc, char **argv)
> " cgroup/sendmsg6 | cgroup/recvmsg4 | cgroup/recvmsg6 |\n"
> " cgroup/getsockopt | cgroup/setsockopt | cgroup/sock_release |\n"
> " struct_ops | fentry | fexit | freplace | sk_lookup }\n"
> - " ATTACH_TYPE := { msg_verdict | stream_verdict | stream_parser |\n"
> - " flow_dissector }\n"
> + " ATTACH_TYPE := { msg_verdict | skb_verdict | stream_verdict |\n"
> + " stream_parser | flow_dissector }\n"
> " METRIC := { cycles | instructions | l1d_loads | llc_misses | itlb_misses | dtlb_misses }\n"
> " " HELP_SPEC_OPTIONS "\n"
> "",
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH bpf-next 3/7] tools: bpftool: complete and synchronise attach or map types
2021-07-30 18:52 ` Andrii Nakryiko
@ 2021-07-30 21:47 ` Quentin Monnet
0 siblings, 0 replies; 19+ messages in thread
From: Quentin Monnet @ 2021-07-30 21:47 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
2021-07-30 11:52 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> Update bpftool's list of attach type names to tell it about the latest
>> attach types, or the "ringbuf" map. Also update the documentation, help
>> messages, and bash completion when relevant.
>>
>> These missing items were reported by the newly added Python script used
>> to help maintain consistency in bpftool.
>>
>> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
>> ---
>> .../bpftool/Documentation/bpftool-prog.rst | 2 +-
>> tools/bpf/bpftool/bash-completion/bpftool | 5 +-
>> tools/bpf/bpftool/common.c | 76 ++++++++++---------
>> tools/bpf/bpftool/prog.c | 4 +-
>> 4 files changed, 47 insertions(+), 40 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
>> index 1828bba19020..b47797cac64f 100644
>> --- a/tools/bpf/bpftool/common.c
>> +++ b/tools/bpf/bpftool/common.c
>> @@ -31,42 +31,48 @@
>> #endif
>>
>> const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE] = {
>> - [BPF_CGROUP_INET_INGRESS] = "ingress",
>> - [BPF_CGROUP_INET_EGRESS] = "egress",
>> - [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
>> - [BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
>> - [BPF_CGROUP_SOCK_OPS] = "sock_ops",
>> - [BPF_CGROUP_DEVICE] = "device",
>> - [BPF_CGROUP_INET4_BIND] = "bind4",
>> - [BPF_CGROUP_INET6_BIND] = "bind6",
>> - [BPF_CGROUP_INET4_CONNECT] = "connect4",
>> - [BPF_CGROUP_INET6_CONNECT] = "connect6",
>> - [BPF_CGROUP_INET4_POST_BIND] = "post_bind4",
>> - [BPF_CGROUP_INET6_POST_BIND] = "post_bind6",
>> - [BPF_CGROUP_INET4_GETPEERNAME] = "getpeername4",
>> - [BPF_CGROUP_INET6_GETPEERNAME] = "getpeername6",
>> - [BPF_CGROUP_INET4_GETSOCKNAME] = "getsockname4",
>> - [BPF_CGROUP_INET6_GETSOCKNAME] = "getsockname6",
>> - [BPF_CGROUP_UDP4_SENDMSG] = "sendmsg4",
>> - [BPF_CGROUP_UDP6_SENDMSG] = "sendmsg6",
>> - [BPF_CGROUP_SYSCTL] = "sysctl",
>> - [BPF_CGROUP_UDP4_RECVMSG] = "recvmsg4",
>> - [BPF_CGROUP_UDP6_RECVMSG] = "recvmsg6",
>> - [BPF_CGROUP_GETSOCKOPT] = "getsockopt",
>> - [BPF_CGROUP_SETSOCKOPT] = "setsockopt",
>> + [BPF_CGROUP_INET_INGRESS] = "ingress",
>> + [BPF_CGROUP_INET_EGRESS] = "egress",
>> + [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
>> + [BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
>> + [BPF_CGROUP_SOCK_OPS] = "sock_ops",
>> + [BPF_CGROUP_DEVICE] = "device",
>> + [BPF_CGROUP_INET4_BIND] = "bind4",
>> + [BPF_CGROUP_INET6_BIND] = "bind6",
>> + [BPF_CGROUP_INET4_CONNECT] = "connect4",
>> + [BPF_CGROUP_INET6_CONNECT] = "connect6",
>> + [BPF_CGROUP_INET4_POST_BIND] = "post_bind4",
>> + [BPF_CGROUP_INET6_POST_BIND] = "post_bind6",
>> + [BPF_CGROUP_INET4_GETPEERNAME] = "getpeername4",
>> + [BPF_CGROUP_INET6_GETPEERNAME] = "getpeername6",
>> + [BPF_CGROUP_INET4_GETSOCKNAME] = "getsockname4",
>> + [BPF_CGROUP_INET6_GETSOCKNAME] = "getsockname6",
>> + [BPF_CGROUP_UDP4_SENDMSG] = "sendmsg4",
>> + [BPF_CGROUP_UDP6_SENDMSG] = "sendmsg6",
>> + [BPF_CGROUP_SYSCTL] = "sysctl",
>> + [BPF_CGROUP_UDP4_RECVMSG] = "recvmsg4",
>> + [BPF_CGROUP_UDP6_RECVMSG] = "recvmsg6",
>> + [BPF_CGROUP_GETSOCKOPT] = "getsockopt",
>> + [BPF_CGROUP_SETSOCKOPT] = "setsockopt",
>>
>> - [BPF_SK_SKB_STREAM_PARSER] = "sk_skb_stream_parser",
>> - [BPF_SK_SKB_STREAM_VERDICT] = "sk_skb_stream_verdict",
>> - [BPF_SK_SKB_VERDICT] = "sk_skb_verdict",
>> - [BPF_SK_MSG_VERDICT] = "sk_msg_verdict",
>> - [BPF_LIRC_MODE2] = "lirc_mode2",
>> - [BPF_FLOW_DISSECTOR] = "flow_dissector",
>> - [BPF_TRACE_RAW_TP] = "raw_tp",
>> - [BPF_TRACE_FENTRY] = "fentry",
>> - [BPF_TRACE_FEXIT] = "fexit",
>> - [BPF_MODIFY_RETURN] = "mod_ret",
>> - [BPF_LSM_MAC] = "lsm_mac",
>> - [BPF_SK_LOOKUP] = "sk_lookup",
>> + [BPF_SK_SKB_STREAM_PARSER] = "sk_skb_stream_parser",
>> + [BPF_SK_SKB_STREAM_VERDICT] = "sk_skb_stream_verdict",
>> + [BPF_SK_SKB_VERDICT] = "sk_skb_verdict",
>> + [BPF_SK_MSG_VERDICT] = "sk_msg_verdict",
>> + [BPF_LIRC_MODE2] = "lirc_mode2",
>> + [BPF_FLOW_DISSECTOR] = "flow_dissector",
>> + [BPF_TRACE_RAW_TP] = "raw_tp",
>> + [BPF_TRACE_FENTRY] = "fentry",
>> + [BPF_TRACE_FEXIT] = "fexit",
>> + [BPF_MODIFY_RETURN] = "mod_ret",
>> + [BPF_LSM_MAC] = "lsm_mac",
>> + [BPF_SK_LOOKUP] = "sk_lookup",
>> + [BPF_TRACE_ITER] = "trace_iter",
>> + [BPF_XDP_DEVMAP] = "xdp_devmap",
>> + [BPF_XDP_CPUMAP] = "xdp_cpumap",
>> + [BPF_XDP] = "xdp",
>> + [BPF_SK_REUSEPORT_SELECT] = "sk_skb_reuseport_select",
>> + [BPF_SK_REUSEPORT_SELECT_OR_MIGRATE] = "sk_skb_reuseport_select_or_migrate",
>> };
>>
>
> you are ruining Git blaming abilities for purely aesthetic reasons,
> which are not good enough reasons, IMO. Please don't do this, this
> nice alignment is nice, but definitely not necessary. So whatever is
> longer then the "default indentation", just add another tab or two and
> be done with it.
I think I've been asked to do the opposite in the past. And I wouldn't
be ruining much in the current case, the array has been moved from
another location and nearly all types were added here in the same
commit. But OK.
> That way we can actually see what was added in this
> patch, btw.
On this one I agree :)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH bpf-next 4/7] tools: bpftool: update and synchronise option list in doc and help msg
2021-07-29 16:29 [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Quentin Monnet
` (2 preceding siblings ...)
2021-07-29 16:29 ` [PATCH bpf-next 3/7] tools: bpftool: complete and synchronise attach or map types Quentin Monnet
@ 2021-07-29 16:29 ` Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 5/7] selftests/bpf: update bpftool's consistency script for checking options Quentin Monnet
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Quentin Monnet @ 2021-07-29 16:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Quentin Monnet
All bpftool commands support the options for JSON output and debug from
libbpf. In addition, some commands support additional options
corresponding to specific use cases.
The list of options described in the man pages for the different
commands are not always accurate. The messages for interactive help are
mostly limited to HELP_SPEC_OPTIONS, and are even less representative of
the actual set of options supported for the commands.
Let's update the lists:
- HELP_SPEC_OPTIONS is modified to contain the "default" options (JSON
and debug), and to be extensible (no ending curly bracket).
- All commands use HELP_SPEC_OPTIONS in their help message, and then
complete the list with their specific options.
- The lists of options in the man pages are updated.
- The formatting of the list for bpftool.rst is adjusted to match
formatting for the other man pages. This is for consistency, and also
because it will be helpful in a future patch to automatically check
that the files are synchronised.
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/bpf/bpftool/Documentation/bpftool-btf.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool-cgroup.rst | 3 ++-
tools/bpf/bpftool/Documentation/bpftool-feature.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool-gen.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool-iter.rst | 2 ++
tools/bpf/bpftool/Documentation/bpftool-link.rst | 3 ++-
tools/bpf/bpftool/Documentation/bpftool-map.rst | 3 ++-
tools/bpf/bpftool/Documentation/bpftool-net.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool-perf.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 3 ++-
.../bpf/bpftool/Documentation/bpftool-struct_ops.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool.rst | 12 ++++++------
tools/bpf/bpftool/btf.c | 2 +-
tools/bpf/bpftool/cgroup.c | 3 ++-
tools/bpf/bpftool/feature.c | 1 +
tools/bpf/bpftool/gen.c | 2 +-
tools/bpf/bpftool/iter.c | 2 ++
tools/bpf/bpftool/link.c | 3 ++-
tools/bpf/bpftool/main.c | 3 ++-
tools/bpf/bpftool/main.h | 3 +--
tools/bpf/bpftool/map.c | 5 +++--
tools/bpf/bpftool/net.c | 1 +
tools/bpf/bpftool/perf.c | 5 ++++-
tools/bpf/bpftool/prog.c | 3 ++-
tools/bpf/bpftool/struct_ops.c | 2 +-
25 files changed, 45 insertions(+), 28 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
index ff4d327a582e..1d37f3809842 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
@@ -12,7 +12,7 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **btf** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | {**-d** | **--debug** } }
*COMMANDS* := { **dump** | **help** }
diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
index baee8591ac76..3e4395eede4f 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -12,7 +12,8 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **cgroup** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
+ { **-f** | **--bpffs** } }
*COMMANDS* :=
{ **show** | **list** | **tree** | **attach** | **detach** | **help** }
diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
index dd3771bdbc57..ab9f57ee4c3a 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
@@ -12,7 +12,7 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **feature** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
*COMMANDS* := { **probe** | **help** }
diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
index 7cd6681137f3..709b93fe1da3 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
@@ -12,7 +12,7 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **gen** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
*COMMAND* := { **object** | **skeleton** | **help** }
diff --git a/tools/bpf/bpftool/Documentation/bpftool-iter.rst b/tools/bpf/bpftool/Documentation/bpftool-iter.rst
index 51f49bead619..471f363a725a 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-iter.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-iter.rst
@@ -12,6 +12,8 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **iter** *COMMAND*
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
+
*COMMANDS* := { **pin** | **help** }
ITER COMMANDS
diff --git a/tools/bpf/bpftool/Documentation/bpftool-link.rst b/tools/bpf/bpftool/Documentation/bpftool-link.rst
index 5f7db2a837cc..0de90f086238 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-link.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-link.rst
@@ -12,7 +12,8 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **link** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
+ { **-f** | **--bpffs** } | { **-n** | **--nomount** } }
*COMMANDS* := { **show** | **list** | **pin** | **help** }
diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 3d52256ba75f..d0c4abe08aba 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -12,7 +12,8 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **map** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
+ { **-f** | **--bpffs** } | { **-n** | **--nomount** } }
*COMMANDS* :=
{ **show** | **list** | **create** | **dump** | **update** | **lookup** | **getnext**
diff --git a/tools/bpf/bpftool/Documentation/bpftool-net.rst b/tools/bpf/bpftool/Documentation/bpftool-net.rst
index d8165d530937..1ae0375e8fea 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-net.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-net.rst
@@ -12,7 +12,7 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **net** *COMMAND*
- *OPTIONS* := { [{ **-j** | **--json** }] [{ **-p** | **--pretty** }] }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
*COMMANDS* :=
{ **show** | **list** | **attach** | **detach** | **help** }
diff --git a/tools/bpf/bpftool/Documentation/bpftool-perf.rst b/tools/bpf/bpftool/Documentation/bpftool-perf.rst
index e958ce91de72..ce52798a917d 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-perf.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-perf.rst
@@ -12,7 +12,7 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **perf** *COMMAND*
- *OPTIONS* := { [{ **-j** | **--json** }] [{ **-p** | **--pretty** }] }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
*COMMANDS* :=
{ **show** | **list** | **help** }
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index abf5f4cd7d3e..4b8412fe2c60 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -12,7 +12,8 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **prog** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
+ { **-f** | **--bpffs** } | { **-m** | **--mapcompat** } | { **-n** | **--nomount** } }
*COMMANDS* :=
{ **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load**
diff --git a/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst b/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
index 506e70ee78e9..02afc0fc14cb 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
@@ -12,7 +12,7 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **struct_ops** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
*COMMANDS* :=
{ **show** | **list** | **dump** | **register** | **unregister** | **help** }
diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
index e7d949334961..bb23f55bb05a 100644
--- a/tools/bpf/bpftool/Documentation/bpftool.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool.rst
@@ -18,15 +18,15 @@ SYNOPSIS
*OBJECT* := { **map** | **program** | **cgroup** | **perf** | **net** | **feature** }
- *OPTIONS* := { { **-V** | **--version** } | { **-h** | **--help** }
- | { **-j** | **--json** } [{ **-p** | **--pretty** }] }
+ *OPTIONS* := { { **-V** | **--version** } |
+ { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
*MAP-COMMANDS* :=
- { **show** | **list** | **create** | **dump** | **update** | **lookup** | **getnext**
- | **delete** | **pin** | **event_pipe** | **help** }
+ { **show** | **list** | **create** | **dump** | **update** | **lookup** | **getnext** |
+ **delete** | **pin** | **event_pipe** | **help** }
- *PROG-COMMANDS* := { **show** | **list** | **dump jited** | **dump xlated** | **pin**
- | **load** | **attach** | **detach** | **help** }
+ *PROG-COMMANDS* := { **show** | **list** | **dump jited** | **dump xlated** | **pin** |
+ **load** | **attach** | **detach** | **help** }
*CGROUP-COMMANDS* := { **show** | **list** | **attach** | **detach** | **help** }
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 385d5c955cf3..ad8374ab6620 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -985,7 +985,7 @@ static int do_help(int argc, char **argv)
" FORMAT := { raw | c }\n"
" " HELP_SPEC_MAP "\n"
" " HELP_SPEC_PROGRAM "\n"
- " " HELP_SPEC_OPTIONS "\n"
+ " " HELP_SPEC_OPTIONS " }\n"
"",
bin_name, "btf");
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index 6e53b1d393f4..c42f437a1015 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -501,7 +501,8 @@ static int do_help(int argc, char **argv)
HELP_SPEC_ATTACH_TYPES "\n"
" " HELP_SPEC_ATTACH_FLAGS "\n"
" " HELP_SPEC_PROGRAM "\n"
- " " HELP_SPEC_OPTIONS "\n"
+ " " HELP_SPEC_OPTIONS " |\n"
+ " {-f|--bpffs} }
"",
bin_name, argv[-2]);
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 40a88df275f9..7f36385aa9e2 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -1005,6 +1005,7 @@ static int do_help(int argc, char **argv)
" %1$s %2$s help\n"
"\n"
" COMPONENT := { kernel | dev NAME }\n"
+ " " HELP_SPEC_OPTIONS " }\n"
"",
bin_name, argv[-2]);
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 1d71ff8c52fa..d4225f7fbcee 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1026,7 +1026,7 @@ static int do_help(int argc, char **argv)
" %1$s %2$s skeleton FILE [name OBJECT_NAME]\n"
" %1$s %2$s help\n"
"\n"
- " " HELP_SPEC_OPTIONS "\n"
+ " " HELP_SPEC_OPTIONS " }\n"
"",
bin_name, "gen");
diff --git a/tools/bpf/bpftool/iter.c b/tools/bpf/bpftool/iter.c
index 3b1aad7535dd..84a9b01d956d 100644
--- a/tools/bpf/bpftool/iter.c
+++ b/tools/bpf/bpftool/iter.c
@@ -97,7 +97,9 @@ static int do_help(int argc, char **argv)
fprintf(stderr,
"Usage: %1$s %2$s pin OBJ PATH [map MAP]\n"
" %1$s %2$s help\n"
+ "\n"
" " HELP_SPEC_MAP "\n"
+ " " HELP_SPEC_OPTIONS " }\n"
"",
bin_name, "iter");
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index e77e1525d20a..8cc3e36f8cc6 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -401,7 +401,8 @@ static int do_help(int argc, char **argv)
" %1$s %2$s help\n"
"\n"
" " HELP_SPEC_LINK "\n"
- " " HELP_SPEC_OPTIONS "\n"
+ " " HELP_SPEC_OPTIONS " |\n"
+ " {-f|--bpffs} | {-n|--nomount} }\n"
"",
bin_name, argv[-2]);
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 3ddfd4843738..02eaaf065f65 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -64,7 +64,8 @@ static int do_help(int argc, char **argv)
" %s version\n"
"\n"
" OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter }\n"
- " " HELP_SPEC_OPTIONS "\n"
+ " " HELP_SPEC_OPTIONS " |\n"
+ " {-V|--version} }\n"
"",
bin_name, bin_name, bin_name);
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index c1cf29798b99..90caa42aac4c 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -57,8 +57,7 @@ static inline void *u64_to_ptr(__u64 ptr)
#define HELP_SPEC_PROGRAM \
"PROG := { id PROG_ID | pinned FILE | tag PROG_TAG | name PROG_NAME }"
#define HELP_SPEC_OPTIONS \
- "OPTIONS := { {-j|--json} [{-p|--pretty}] | {-f|--bpffs} |\n" \
- "\t {-m|--mapcompat} | {-n|--nomount} }"
+ "OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug}"
#define HELP_SPEC_MAP \
"MAP := { id MAP_ID | pinned FILE | name MAP_NAME }"
#define HELP_SPEC_LINK \
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 09ae0381205b..ab2a8e5e1809 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -1466,8 +1466,9 @@ static int do_help(int argc, char **argv)
" devmap | devmap_hash | sockmap | cpumap | xskmap | sockhash |\n"
" cgroup_storage | reuseport_sockarray | percpu_cgroup_storage |\n"
" queue | stack | sk_storage | struct_ops | ringbuf | inode_storage |\n"
- " task_storage }\n"
- " " HELP_SPEC_OPTIONS "\n"
+ " task_storage }\n"
+ " " HELP_SPEC_OPTIONS " |\n"
+ " {-f|--bpffs} | {-n|--nomount} }\n"
"",
bin_name, argv[-2]);
diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
index f836d115d7d6..649053704bd7 100644
--- a/tools/bpf/bpftool/net.c
+++ b/tools/bpf/bpftool/net.c
@@ -729,6 +729,7 @@ static int do_help(int argc, char **argv)
"\n"
" " HELP_SPEC_PROGRAM "\n"
" ATTACH_TYPE := { xdp | xdpgeneric | xdpdrv | xdpoffload }\n"
+ " " HELP_SPEC_OPTIONS " }\n"
"\n"
"Note: Only xdp and tc attachments are supported now.\n"
" For progs attached to cgroups, use \"bpftool cgroup\"\n"
diff --git a/tools/bpf/bpftool/perf.c b/tools/bpf/bpftool/perf.c
index ad23934819c7..50de087b0db7 100644
--- a/tools/bpf/bpftool/perf.c
+++ b/tools/bpf/bpftool/perf.c
@@ -231,7 +231,10 @@ static int do_show(int argc, char **argv)
static int do_help(int argc, char **argv)
{
fprintf(stderr,
- "Usage: %1$s %2$s { show | list | help }\n"
+ "Usage: %1$s %2$s { show | list }\n"
+ " %1$s %2$s help }\n"
+ "\n"
+ " " HELP_SPEC_OPTIONS " }\n"
"",
bin_name, argv[-2]);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 1ee87225543b..2f0f6c7b2fb9 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -2248,7 +2248,8 @@ static int do_help(int argc, char **argv)
" ATTACH_TYPE := { msg_verdict | skb_verdict | stream_verdict |\n"
" stream_parser | flow_dissector }\n"
" METRIC := { cycles | instructions | l1d_loads | llc_misses | itlb_misses | dtlb_misses }\n"
- " " HELP_SPEC_OPTIONS "\n"
+ " " HELP_SPEC_OPTIONS " |\n"
+ " {-f|--bpffs} | {-m|--mapcompat} | {-n|--nomount} }\n"
"",
bin_name, argv[-2]);
diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c
index b58b91f62ffb..ab2d2290569a 100644
--- a/tools/bpf/bpftool/struct_ops.c
+++ b/tools/bpf/bpftool/struct_ops.c
@@ -572,8 +572,8 @@ static int do_help(int argc, char **argv)
" %1$s %2$s unregister STRUCT_OPS_MAP\n"
" %1$s %2$s help\n"
"\n"
- " OPTIONS := { {-j|--json} [{-p|--pretty}] }\n"
" STRUCT_OPS_MAP := [ id STRUCT_OPS_MAP_ID | name STRUCT_OPS_MAP_NAME ]\n"
+ " " HELP_SPEC_OPTIONS " }\n"
"",
bin_name, argv[-2]);
--
2.30.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH bpf-next 5/7] selftests/bpf: update bpftool's consistency script for checking options
2021-07-29 16:29 [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Quentin Monnet
` (3 preceding siblings ...)
2021-07-29 16:29 ` [PATCH bpf-next 4/7] tools: bpftool: update and synchronise option list in doc and help msg Quentin Monnet
@ 2021-07-29 16:29 ` Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options Quentin Monnet
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Quentin Monnet @ 2021-07-29 16:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Quentin Monnet
Update the script responsible for checking that the different types used
at various places in bpftool are synchronised, and extend it to check
the consistency of options between the help messages in the source code
and the manual pages.
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
.../selftests/bpf/test_bpftool_synctypes.py | 122 ++++++++++++++++--
1 file changed, 111 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
index b41464f46b3b..be54b7335a76 100755
--- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
+++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
@@ -200,7 +200,7 @@ class FileExtractor(object):
@block_name: name of the blog to parse, 'TYPE' in the example
"""
start_marker = re.compile(f'\*{block_name}\* := {{')
- pattern = re.compile('\*\*([\w/]+)\*\*')
+ pattern = re.compile('\*\*([\w/-]+)\*\*')
end_marker = re.compile('}\n')
return self.__get_description_list(start_marker, pattern, end_marker)
@@ -223,6 +223,31 @@ class FileExtractor(object):
end_marker = re.compile('}')
return self.__get_description_list(start_marker, pattern, end_marker)
+ def get_help_list_macro(self, macro):
+ """
+ Search for and parse a list of values from a help message starting with
+ a macro in bpftool, for example:
+
+ " " HELP_SPEC_OPTIONS " |\\n"
+ " {-f|--bpffs} | {-m|--mapcompat} | {-n|--nomount} }\\n"
+
+ Return a set containing all item names, for example:
+
+ {'-f', '--bpffs', '-m', '--mapcompat', '-n', '--nomount'}
+
+ @macro: macro starting the block, 'HELP_SPEC_OPTIONS' in the example
+ """
+ start_marker = re.compile(f'"\s*{macro}\s*" [|}}]')
+ pattern = re.compile('([\w-]+) ?(?:\||}[ }\]])')
+ end_marker = re.compile('}\\\\n')
+ return self.__get_description_list(start_marker, pattern, end_marker)
+
+ def default_options(self):
+ """
+ Return the default options contained in HELP_SPEC_OPTIONS
+ """
+ return { '-j', '--json', '-p', '--pretty', '-d', '--debug' }
+
def get_bashcomp_list(self, block_name):
"""
Search for and parse a list of type names from a variable in bash
@@ -242,7 +267,16 @@ class FileExtractor(object):
end_marker = re.compile('\'$')
return self.__get_description_list(start_marker, pattern, end_marker)
-class ProgFileExtractor(FileExtractor):
+class SourceFileExtractor(FileExtractor):
+ """
+ An abstract extractor for a source file with usage message.
+ This class does not offer a way to set a filename, which is expected to be
+ defined in children classes.
+ """
+ def get_options(self):
+ return self.default_options().union(self.get_help_list_macro('HELP_SPEC_OPTIONS'))
+
+class ProgFileExtractor(SourceFileExtractor):
"""
An extractor for bpftool's prog.c.
"""
@@ -257,7 +291,7 @@ class ProgFileExtractor(FileExtractor):
def get_prog_attach_help(self):
return self.get_help_list('ATTACH_TYPE')
-class MapFileExtractor(FileExtractor):
+class MapFileExtractor(SourceFileExtractor):
"""
An extractor for bpftool's map.c.
"""
@@ -269,7 +303,7 @@ class MapFileExtractor(FileExtractor):
def get_map_help(self):
return self.get_help_list('TYPE')
-class CgroupFileExtractor(FileExtractor):
+class CgroupFileExtractor(SourceFileExtractor):
"""
An extractor for bpftool's cgroup.c.
"""
@@ -278,7 +312,7 @@ class CgroupFileExtractor(FileExtractor):
def get_prog_attach_help(self):
return self.get_help_list('ATTACH_TYPE')
-class CommonFileExtractor(FileExtractor):
+class CommonFileExtractor(SourceFileExtractor):
"""
An extractor for bpftool's common.c.
"""
@@ -302,6 +336,16 @@ class CommonFileExtractor(FileExtractor):
cgroup_types[key] = value
return cgroup_types
+class GenericSourceExtractor(SourceFileExtractor):
+ """
+ An extractor for generic source code files.
+ """
+ filename = ""
+
+ def __init__(self, filename):
+ self.filename = os.path.join(BPFTOOL_DIR, filename)
+ super().__init__()
+
class BpfHeaderExtractor(FileExtractor):
"""
An extractor for the UAPI BPF header.
@@ -317,7 +361,16 @@ class BpfHeaderExtractor(FileExtractor):
def get_attach_types(self):
return self.get_enum('bpf_attach_type')
-class ManProgExtractor(FileExtractor):
+class ManPageExtractor(FileExtractor):
+ """
+ An abstract extractor for an RST documentation page.
+ This class does not offer a way to set a filename, which is expected to be
+ defined in children classes.
+ """
+ def get_options(self):
+ return self.get_rst_list('OPTIONS')
+
+class ManProgExtractor(ManPageExtractor):
"""
An extractor for bpftool-prog.rst.
"""
@@ -326,7 +379,7 @@ class ManProgExtractor(FileExtractor):
def get_attach_types(self):
return self.get_rst_list('ATTACH_TYPE')
-class ManMapExtractor(FileExtractor):
+class ManMapExtractor(ManPageExtractor):
"""
An extractor for bpftool-map.rst.
"""
@@ -335,7 +388,7 @@ class ManMapExtractor(FileExtractor):
def get_map_types(self):
return self.get_rst_list('TYPE')
-class ManCgroupExtractor(FileExtractor):
+class ManCgroupExtractor(ManPageExtractor):
"""
An extractor for bpftool-cgroup.rst.
"""
@@ -344,6 +397,16 @@ class ManCgroupExtractor(FileExtractor):
def get_attach_types(self):
return self.get_rst_list('ATTACH_TYPE')
+class ManGenericExtractor(ManPageExtractor):
+ """
+ An extractor for generic RST documentation pages.
+ """
+ filename = ""
+
+ def __init__(self, filename):
+ self.filename = os.path.join(BPFTOOL_DIR, filename)
+ super().__init__()
+
class BashcompExtractor(FileExtractor):
"""
An extractor for bpftool's bash completion file.
@@ -375,9 +438,9 @@ def verify(first_set, second_set, message):
def main():
# No arguments supported at this time, but print usage for -h|--help
argParser = argparse.ArgumentParser(description="""
- Verify that bpftool's code, help messages, documentation and bash completion
- are all in sync on program types, map types and attach types. Also check that
- bpftool is in sync with the UAPI BPF header.
+ Verify that bpftool's code, help messages, documentation and bash
+ completion are all in sync on program types, map types, attach types, and
+ options. Also check that bpftool is in sync with the UAPI BPF header.
""")
args = argParser.parse_args()
@@ -399,9 +462,11 @@ def main():
source_map_types.discard('unspec')
help_map_types = map_info.get_map_help()
+ help_map_options = map_info.get_options()
map_info.close()
man_map_info = ManMapExtractor()
+ man_map_options = man_map_info.get_options()
man_map_types = man_map_info.get_map_types()
man_map_info.close()
@@ -412,6 +477,8 @@ def main():
f'Comparing {MapFileExtractor.filename} (map_type_name) and {MapFileExtractor.filename} (do_help() TYPE):')
verify(source_map_types, man_map_types,
f'Comparing {MapFileExtractor.filename} (map_type_name) and {ManMapExtractor.filename} (TYPE):')
+ verify(help_map_options, man_map_options,
+ f'Comparing {MapFileExtractor.filename} (do_help() OPTIONS) and {ManMapExtractor.filename} (OPTIONS):')
verify(source_map_types, bashcomp_map_types,
f'Comparing {MapFileExtractor.filename} (map_type_name) and {BashcompExtractor.filename} (BPFTOOL_MAP_CREATE_TYPES):')
@@ -441,9 +508,11 @@ def main():
source_prog_attach_types = set(prog_info.get_attach_types().values())
help_prog_attach_types = prog_info.get_prog_attach_help()
+ help_prog_options = prog_info.get_options()
prog_info.close()
man_prog_info = ManProgExtractor()
+ man_prog_options = man_prog_info.get_options()
man_prog_attach_types = man_prog_info.get_attach_types()
man_prog_info.close()
@@ -454,6 +523,8 @@ def main():
f'Comparing {ProgFileExtractor.filename} (attach_type_strings) and {ProgFileExtractor.filename} (do_help() ATTACH_TYPE):')
verify(source_prog_attach_types, man_prog_attach_types,
f'Comparing {ProgFileExtractor.filename} (attach_type_strings) and {ManProgExtractor.filename} (ATTACH_TYPE):')
+ verify(help_prog_options, man_prog_options,
+ f'Comparing {ProgFileExtractor.filename} (do_help() OPTIONS) and {ManProgExtractor.filename} (OPTIONS):')
verify(source_prog_attach_types, bashcomp_prog_attach_types,
f'Comparing {ProgFileExtractor.filename} (attach_type_strings) and {BashcompExtractor.filename} (BPFTOOL_PROG_ATTACH_TYPES):')
@@ -464,9 +535,11 @@ def main():
cgroup_info = CgroupFileExtractor()
help_cgroup_attach_types = cgroup_info.get_prog_attach_help()
+ help_cgroup_options = cgroup_info.get_options()
cgroup_info.close()
man_cgroup_info = ManCgroupExtractor()
+ man_cgroup_options = man_cgroup_info.get_options()
man_cgroup_attach_types = man_cgroup_info.get_attach_types()
man_cgroup_info.close()
@@ -477,9 +550,36 @@ def main():
f'Comparing {CommonFileExtractor.filename} (attach_type_strings) and {CgroupFileExtractor.filename} (do_help() ATTACH_TYPE):')
verify(source_cgroup_attach_types, man_cgroup_attach_types,
f'Comparing {CommonFileExtractor.filename} (attach_type_strings) and {ManCgroupExtractor.filename} (ATTACH_TYPE):')
+ verify(help_cgroup_options, man_cgroup_options,
+ f'Comparing {CgroupFileExtractor.filename} (do_help() OPTIONS) and {ManCgroupExtractor.filename} (OPTIONS):')
verify(source_cgroup_attach_types, bashcomp_cgroup_attach_types,
f'Comparing {CommonFileExtractor.filename} (attach_type_strings) and {BashcompExtractor.filename} (BPFTOOL_CGROUP_ATTACH_TYPES):')
+ # Options for remaining commands
+
+ for cmd in [ 'btf', 'feature', 'gen', 'iter', 'link', 'net', 'perf', 'struct_ops', ]:
+ source_info = GenericSourceExtractor(cmd + '.c')
+ help_cmd_options = source_info.get_options()
+ source_info.close()
+
+ man_cmd_info = ManGenericExtractor(os.path.join('Documentation', 'bpftool-' + cmd + '.rst'))
+ man_cmd_options = man_cmd_info.get_options()
+ man_cmd_info.close()
+
+ verify(help_cmd_options, man_cmd_options,
+ f'Comparing {source_info.filename} (do_help() OPTIONS) and {man_cmd_info.filename} (OPTIONS):')
+
+ source_main_info = GenericSourceExtractor('main.c')
+ help_main_options = source_main_info.get_options()
+ source_main_info.close()
+
+ man_main_info = ManGenericExtractor(os.path.join('Documentation', 'bpftool.rst'))
+ man_main_options = man_main_info.get_options()
+ man_main_info.close()
+
+ verify(help_main_options, man_main_options,
+ f'Comparing {source_main_info.filename} (do_help() OPTIONS) and {man_main_info.filename} (OPTIONS):')
+
sys.exit(retval)
if __name__ == "__main__":
--
2.30.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options
2021-07-29 16:29 [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Quentin Monnet
` (4 preceding siblings ...)
2021-07-29 16:29 ` [PATCH bpf-next 5/7] selftests/bpf: update bpftool's consistency script for checking options Quentin Monnet
@ 2021-07-29 16:29 ` Quentin Monnet
2021-07-30 18:59 ` Andrii Nakryiko
2021-07-29 16:29 ` [PATCH bpf-next 7/7] tools: bpftool: complete metrics list in "bpftool prog profile" doc Quentin Monnet
2021-07-30 19:06 ` [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Andrii Nakryiko
7 siblings, 1 reply; 19+ messages in thread
From: Quentin Monnet @ 2021-07-29 16:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Quentin Monnet
The -L|--use-loader option for using loader programs when loading, or
when generating a skeleton, did not have any documentation or bash
completion. Same thing goes for -B|--base-btf, used to pass a path to a
base BTF object for split BTF such as BTF for kernel modules.
This patch documents and adds bash completion for those options.
Fixes: 75fa1777694c ("tools/bpftool: Add bpftool support for split BTF")
Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
Note: The second example with base BTF in the BTF man page assumes that
dumping split BTF when objects are passed by id is supported. Support is
currently pending review in another PR.
---
.../bpf/bpftool/Documentation/bpftool-btf.rst | 48 ++++++++++++++++++-
.../bpf/bpftool/Documentation/bpftool-gen.rst | 9 +++-
.../bpftool/Documentation/bpftool-prog.rst | 30 +++++++++++-
tools/bpf/bpftool/bash-completion/bpftool | 8 ++--
tools/bpf/bpftool/btf.c | 3 +-
tools/bpf/bpftool/cgroup.c | 2 +-
tools/bpf/bpftool/gen.c | 3 +-
tools/bpf/bpftool/prog.c | 3 +-
8 files changed, 96 insertions(+), 10 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
index 1d37f3809842..88b28aa7431f 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
@@ -12,7 +12,8 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **btf** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | {**-d** | **--debug** } }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | {**-d** | **--debug** } |
+ { **-B** | **--base-btf** } }
*COMMANDS* := { **dump** | **help** }
@@ -73,6 +74,20 @@ OPTIONS
=======
.. include:: common_options.rst
+ -B, --base-btf *FILE*
+ Pass a base BTF object. Base BTF objects are typically used
+ with BTF objects for kernel modules. To avoid duplicating
+ all kernel symbols required by modules, BTF objects for
+ modules are "split", they are built incrementally on top of
+ the kernel (vmlinux) BTF object. So the base BTF reference
+ should usually point to the kernel BTF.
+
+ When the main BTF object to process (for example, the
+ module BTF to dump) is passed as a *FILE*, bpftool attempts
+ to autodetect the path for the base object, and passing
+ this option is optional. When the main BTF object is passed
+ through other handles, this option becomes necessary.
+
EXAMPLES
========
**# bpftool btf dump id 1226**
@@ -217,3 +232,34 @@ All the standard ways to specify map or program are supported:
**# bpftool btf dump prog tag b88e0a09b1d9759d**
**# bpftool btf dump prog pinned /sys/fs/bpf/prog_name**
+
+|
+| **# bpftool btf dump file /sys/kernel/btf/i2c_smbus**
+| (or)
+| **# I2C_SMBUS_ID=$(bpftool btf show -p | jq '.[] | select(.name=="i2c_smbus").id')**
+| **# bpftool btf dump id ${I2C_SMBUS_ID} -B /sys/kernel/btf/vmlinux**
+
+::
+
+ [104848] STRUCT 'i2c_smbus_alert' size=40 vlen=2
+ 'alert' type_id=393 bits_offset=0
+ 'ara' type_id=56050 bits_offset=256
+ [104849] STRUCT 'alert_data' size=12 vlen=3
+ 'addr' type_id=16 bits_offset=0
+ 'type' type_id=56053 bits_offset=32
+ 'data' type_id=7 bits_offset=64
+ [104850] PTR '(anon)' type_id=104848
+ [104851] PTR '(anon)' type_id=104849
+ [104852] FUNC 'i2c_register_spd' type_id=84745 linkage=static
+ [104853] FUNC 'smbalert_driver_init' type_id=1213 linkage=static
+ [104854] FUNC_PROTO '(anon)' ret_type_id=18 vlen=1
+ 'ara' type_id=56050
+ [104855] FUNC 'i2c_handle_smbus_alert' type_id=104854 linkage=static
+ [104856] FUNC 'smbalert_remove' type_id=104854 linkage=static
+ [104857] FUNC_PROTO '(anon)' ret_type_id=18 vlen=2
+ 'ara' type_id=56050
+ 'id' type_id=56056
+ [104858] FUNC 'smbalert_probe' type_id=104857 linkage=static
+ [104859] FUNC 'smbalert_work' type_id=9695 linkage=static
+ [104860] FUNC 'smbus_alert' type_id=71367 linkage=static
+ [104861] FUNC 'smbus_do_alert' type_id=84827 linkage=static
diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
index 709b93fe1da3..2ef2f2df0279 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
@@ -12,7 +12,8 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **gen** *COMMAND*
- *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
+ { **-L** | **--use-loader** } }
*COMMAND* := { **object** | **skeleton** | **help** }
@@ -152,6 +153,12 @@ OPTIONS
=======
.. include:: common_options.rst
+ -L, --use-loader
+ For skeletons, generate a "light" skeleton (also known as "loader"
+ skeleton). A light skeleton contains a loader eBPF program. It does
+ not use the majority of the libbpf infrastructure, and does not need
+ libelf.
+
EXAMPLES
========
**$ cat example1.bpf.c**
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 4b8412fe2c60..2ea5df30ff21 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -13,7 +13,8 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **prog** *COMMAND*
*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
- { **-f** | **--bpffs** } | { **-m** | **--mapcompat** } | { **-n** | **--nomount** } }
+ { **-f** | **--bpffs** } | { **-m** | **--mapcompat** } | { **-n** | **--nomount** } |
+ { **-L** | **--use-loader** } }
*COMMANDS* :=
{ **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load**
@@ -224,6 +225,20 @@ OPTIONS
Do not automatically attempt to mount any virtual file system
(such as tracefs or BPF virtual file system) when necessary.
+ -L, --use-loader
+ Load program as a "loader" program. This is useful to debug
+ the generation of such programs. When this option is in
+ use, bpftool attempts to load the programs from the object
+ file into the kernel, but does not pin them (therefore, the
+ *PATH* must not be provided).
+
+ When combined with the **-d**\ \|\ **--debug** option,
+ additional debug messages are generated, and the execution
+ of the loader program will use the **bpf_trace_printk**\ ()
+ helper to log each step of loading BTF, creating the maps,
+ and loading the programs (see **bpftool prog tracelog** as
+ a way to dump those messages).
+
EXAMPLES
========
**# bpftool prog show**
@@ -327,3 +342,16 @@ EXAMPLES
40176203 cycles (83.05%)
42518139 instructions # 1.06 insns per cycle (83.39%)
123 llc_misses # 2.89 LLC misses per million insns (83.15%)
+
+|
+| Output below is for the trace logs.
+| Run in separate terminals:
+| **# bpftool prog tracelog**
+| **# bpftool prog load -L -d file.o**
+
+::
+
+ bpftool-620059 [004] d... 2634685.517903: bpf_trace_printk: btf_load size 665 r=5
+ bpftool-620059 [004] d... 2634685.517912: bpf_trace_printk: map_create sample_map idx 0 type 2 value_size 4 value_btf_id 0 r=6
+ bpftool-620059 [004] d... 2634685.517997: bpf_trace_printk: prog_load sample insn_cnt 13 r=7
+ bpftool-620059 [004] d... 2634685.517999: bpf_trace_printk: close(5) = 0
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 69d018474537..d33b8f308a4c 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -260,7 +260,8 @@ _bpftool()
# Deal with options
if [[ ${words[cword]} == -* ]]; then
- local c='--version --json --pretty --bpffs --mapcompat --debug'
+ local c='--version --json --pretty --bpffs --mapcompat --debug \
+ --use-loader --base-btf'
COMPREPLY=( $( compgen -W "$c" -- "$cur" ) )
return 0
fi
@@ -278,7 +279,7 @@ _bpftool()
_sysfs_get_netdevs
return 0
;;
- file|pinned)
+ file|pinned|-B|--base-btf)
_filedir
return 0
;;
@@ -291,7 +292,8 @@ _bpftool()
# Remove all options so completions don't have to deal with them.
local i
for (( i=1; i < ${#words[@]}; )); do
- if [[ ${words[i]::1} == - ]]; then
+ if [[ ${words[i]::1} == - ]] &&
+ [[ ${words[i]} != "-B" ]] && [[ ${words[i]} != "--base-btf" ]]; then
words=( "${words[@]:0:i}" "${words[@]:i+1}" )
[[ $i -le $cword ]] && cword=$(( cword - 1 ))
else
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index ad8374ab6620..cee22041c8d0 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -985,7 +985,8 @@ static int do_help(int argc, char **argv)
" FORMAT := { raw | c }\n"
" " HELP_SPEC_MAP "\n"
" " HELP_SPEC_PROGRAM "\n"
- " " HELP_SPEC_OPTIONS " }\n"
+ " " HELP_SPEC_OPTIONS " |\n"
+ " {-B|--base-btf} }\n"
"",
bin_name, "btf");
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index c42f437a1015..3571a281c43f 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -502,7 +502,7 @@ static int do_help(int argc, char **argv)
" " HELP_SPEC_ATTACH_FLAGS "\n"
" " HELP_SPEC_PROGRAM "\n"
" " HELP_SPEC_OPTIONS " |\n"
- " {-f|--bpffs} }
+ " {-f|--bpffs} }\n"
"",
bin_name, argv[-2]);
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index d4225f7fbcee..d40d92bbf0e4 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1026,7 +1026,8 @@ static int do_help(int argc, char **argv)
" %1$s %2$s skeleton FILE [name OBJECT_NAME]\n"
" %1$s %2$s help\n"
"\n"
- " " HELP_SPEC_OPTIONS " }\n"
+ " " HELP_SPEC_OPTIONS " |\n"
+ " {-L|--use-loader} }\n"
"",
bin_name, "gen");
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 2f0f6c7b2fb9..22f8069cddbe 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -2249,7 +2249,8 @@ static int do_help(int argc, char **argv)
" stream_parser | flow_dissector }\n"
" METRIC := { cycles | instructions | l1d_loads | llc_misses | itlb_misses | dtlb_misses }\n"
" " HELP_SPEC_OPTIONS " |\n"
- " {-f|--bpffs} | {-m|--mapcompat} | {-n|--nomount} }\n"
+ " {-f|--bpffs} | {-m|--mapcompat} | {-n|--nomount} |\n"
+ " {-L|--use-loader} }\n"
"",
bin_name, argv[-2]);
--
2.30.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options
2021-07-29 16:29 ` [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options Quentin Monnet
@ 2021-07-30 18:59 ` Andrii Nakryiko
2021-07-30 21:48 ` Quentin Monnet
0 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2021-07-30 18:59 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> The -L|--use-loader option for using loader programs when loading, or
> when generating a skeleton, did not have any documentation or bash
> completion. Same thing goes for -B|--base-btf, used to pass a path to a
> base BTF object for split BTF such as BTF for kernel modules.
>
> This patch documents and adds bash completion for those options.
>
> Fixes: 75fa1777694c ("tools/bpftool: Add bpftool support for split BTF")
> Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
> Note: The second example with base BTF in the BTF man page assumes that
> dumping split BTF when objects are passed by id is supported. Support is
> currently pending review in another PR.
> ---
Not anymore :)
[...]
> @@ -73,6 +74,20 @@ OPTIONS
> =======
> .. include:: common_options.rst
>
> + -B, --base-btf *FILE*
> + Pass a base BTF object. Base BTF objects are typically used
> + with BTF objects for kernel modules. To avoid duplicating
> + all kernel symbols required by modules, BTF objects for
> + modules are "split", they are built incrementally on top of
> + the kernel (vmlinux) BTF object. So the base BTF reference
> + should usually point to the kernel BTF.
> +
> + When the main BTF object to process (for example, the
> + module BTF to dump) is passed as a *FILE*, bpftool attempts
> + to autodetect the path for the base object, and passing
> + this option is optional. When the main BTF object is passed
> + through other handles, this option becomes necessary.
> +
> EXAMPLES
> ========
> **# bpftool btf dump id 1226**
> @@ -217,3 +232,34 @@ All the standard ways to specify map or program are supported:
> **# bpftool btf dump prog tag b88e0a09b1d9759d**
>
> **# bpftool btf dump prog pinned /sys/fs/bpf/prog_name**
> +
> +|
> +| **# bpftool btf dump file /sys/kernel/btf/i2c_smbus**
> +| (or)
> +| **# I2C_SMBUS_ID=$(bpftool btf show -p | jq '.[] | select(.name=="i2c_smbus").id')**
> +| **# bpftool btf dump id ${I2C_SMBUS_ID} -B /sys/kernel/btf/vmlinux**
> +
> +::
> +
> + [104848] STRUCT 'i2c_smbus_alert' size=40 vlen=2
> + 'alert' type_id=393 bits_offset=0
> + 'ara' type_id=56050 bits_offset=256
> + [104849] STRUCT 'alert_data' size=12 vlen=3
> + 'addr' type_id=16 bits_offset=0
> + 'type' type_id=56053 bits_offset=32
> + 'data' type_id=7 bits_offset=64
> + [104850] PTR '(anon)' type_id=104848
> + [104851] PTR '(anon)' type_id=104849
> + [104852] FUNC 'i2c_register_spd' type_id=84745 linkage=static
> + [104853] FUNC 'smbalert_driver_init' type_id=1213 linkage=static
> + [104854] FUNC_PROTO '(anon)' ret_type_id=18 vlen=1
> + 'ara' type_id=56050
> + [104855] FUNC 'i2c_handle_smbus_alert' type_id=104854 linkage=static
> + [104856] FUNC 'smbalert_remove' type_id=104854 linkage=static
> + [104857] FUNC_PROTO '(anon)' ret_type_id=18 vlen=2
> + 'ara' type_id=56050
> + 'id' type_id=56056
> + [104858] FUNC 'smbalert_probe' type_id=104857 linkage=static
> + [104859] FUNC 'smbalert_work' type_id=9695 linkage=static
> + [104860] FUNC 'smbus_alert' type_id=71367 linkage=static
> + [104861] FUNC 'smbus_do_alert' type_id=84827 linkage=static
This reminded be that it would be awesome to support "format c"
use-case for dumping split BTF in a more sane way. I.e., instead of
dumping all types from base and split BTF, only dump necessary (used)
forward declarations from base BTF, and then full C dump of only new
types from the split (module) BTF. This will become more important as
people will start using module BTF more. It's an interesting add-on to
libbpf's btf_dumper functionality. Not sure how hard that would be,
but I'd imagine it shouldn't require much changes.
Just in case anyone wanted to challenge themselves with some more
algorithmic patch for libbpf (*wink wink*)...
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> index 709b93fe1da3..2ef2f2df0279 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> @@ -12,7 +12,8 @@ SYNOPSIS
>
> **bpftool** [*OPTIONS*] **gen** *COMMAND*
>
> - *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
> + *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
> + { **-L** | **--use-loader** } }
>
> *COMMAND* := { **object** | **skeleton** | **help** }
>
[...]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options
2021-07-30 18:59 ` Andrii Nakryiko
@ 2021-07-30 21:48 ` Quentin Monnet
2021-07-30 22:10 ` Andrii Nakryiko
0 siblings, 1 reply; 19+ messages in thread
From: Quentin Monnet @ 2021-07-30 21:48 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
2021-07-30 11:59 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> The -L|--use-loader option for using loader programs when loading, or
>> when generating a skeleton, did not have any documentation or bash
>> completion. Same thing goes for -B|--base-btf, used to pass a path to a
>> base BTF object for split BTF such as BTF for kernel modules.
>>
>> This patch documents and adds bash completion for those options.
>>
>> Fixes: 75fa1777694c ("tools/bpftool: Add bpftool support for split BTF")
>> Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")
>> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
>> ---
>> Note: The second example with base BTF in the BTF man page assumes that
>> dumping split BTF when objects are passed by id is supported. Support is
>> currently pending review in another PR.
>> ---
>
> Not anymore :)
>
> [...]
>
>> @@ -73,6 +74,20 @@ OPTIONS
>> =======
>> .. include:: common_options.rst
>>
>> + -B, --base-btf *FILE*
>> + Pass a base BTF object. Base BTF objects are typically used
>> + with BTF objects for kernel modules. To avoid duplicating
>> + all kernel symbols required by modules, BTF objects for
>> + modules are "split", they are built incrementally on top of
>> + the kernel (vmlinux) BTF object. So the base BTF reference
>> + should usually point to the kernel BTF.
>> +
>> + When the main BTF object to process (for example, the
>> + module BTF to dump) is passed as a *FILE*, bpftool attempts
>> + to autodetect the path for the base object, and passing
>> + this option is optional. When the main BTF object is passed
>> + through other handles, this option becomes necessary.
>> +
>> EXAMPLES
>> ========
>> **# bpftool btf dump id 1226**
>> @@ -217,3 +232,34 @@ All the standard ways to specify map or program are supported:
>> **# bpftool btf dump prog tag b88e0a09b1d9759d**
>>
>> **# bpftool btf dump prog pinned /sys/fs/bpf/prog_name**
>> +
>> +|
>> +| **# bpftool btf dump file /sys/kernel/btf/i2c_smbus**
>> +| (or)
>> +| **# I2C_SMBUS_ID=$(bpftool btf show -p | jq '.[] | select(.name=="i2c_smbus").id')**
>> +| **# bpftool btf dump id ${I2C_SMBUS_ID} -B /sys/kernel/btf/vmlinux**
>> +
>> +::
>> +
>> + [104848] STRUCT 'i2c_smbus_alert' size=40 vlen=2
>> + 'alert' type_id=393 bits_offset=0
>> + 'ara' type_id=56050 bits_offset=256
>> + [104849] STRUCT 'alert_data' size=12 vlen=3
>> + 'addr' type_id=16 bits_offset=0
>> + 'type' type_id=56053 bits_offset=32
>> + 'data' type_id=7 bits_offset=64
>> + [104850] PTR '(anon)' type_id=104848
>> + [104851] PTR '(anon)' type_id=104849
>> + [104852] FUNC 'i2c_register_spd' type_id=84745 linkage=static
>> + [104853] FUNC 'smbalert_driver_init' type_id=1213 linkage=static
>> + [104854] FUNC_PROTO '(anon)' ret_type_id=18 vlen=1
>> + 'ara' type_id=56050
>> + [104855] FUNC 'i2c_handle_smbus_alert' type_id=104854 linkage=static
>> + [104856] FUNC 'smbalert_remove' type_id=104854 linkage=static
>> + [104857] FUNC_PROTO '(anon)' ret_type_id=18 vlen=2
>> + 'ara' type_id=56050
>> + 'id' type_id=56056
>> + [104858] FUNC 'smbalert_probe' type_id=104857 linkage=static
>> + [104859] FUNC 'smbalert_work' type_id=9695 linkage=static
>> + [104860] FUNC 'smbus_alert' type_id=71367 linkage=static
>> + [104861] FUNC 'smbus_do_alert' type_id=84827 linkage=static
>
> This reminded be that it would be awesome to support "format c"
> use-case for dumping split BTF in a more sane way. I.e., instead of
> dumping all types from base and split BTF, only dump necessary (used)
> forward declarations from base BTF, and then full C dump of only new
> types from the split (module) BTF. This will become more important as
> people will start using module BTF more. It's an interesting add-on to
> libbpf's btf_dumper functionality. Not sure how hard that would be,
> but I'd imagine it shouldn't require much changes.
>
> Just in case anyone wanted to challenge themselves with some more
> algorithmic patch for libbpf (*wink wink*)...
If you're addressing this to me, I'm not particularly looking for such
challenge at the moment :). In fact I already noted a few things that I
would like to fix or improve for bpftool, I will append this one to the
list. I should maybe start thinking of a tracker of some sort to list
and share this.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options
2021-07-30 21:48 ` Quentin Monnet
@ 2021-07-30 22:10 ` Andrii Nakryiko
0 siblings, 0 replies; 19+ messages in thread
From: Andrii Nakryiko @ 2021-07-30 22:10 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking,
bpf, Alan Maguire
On Fri, Jul 30, 2021 at 2:48 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2021-07-30 11:59 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> The -L|--use-loader option for using loader programs when loading, or
> >> when generating a skeleton, did not have any documentation or bash
> >> completion. Same thing goes for -B|--base-btf, used to pass a path to a
> >> base BTF object for split BTF such as BTF for kernel modules.
> >>
> >> This patch documents and adds bash completion for those options.
> >>
> >> Fixes: 75fa1777694c ("tools/bpftool: Add bpftool support for split BTF")
> >> Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")
> >> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> >> ---
> >> Note: The second example with base BTF in the BTF man page assumes that
> >> dumping split BTF when objects are passed by id is supported. Support is
> >> currently pending review in another PR.
> >> ---
> >
> > Not anymore :)
> >
> > [...]
> >
> >> @@ -73,6 +74,20 @@ OPTIONS
> >> =======
> >> .. include:: common_options.rst
> >>
> >> + -B, --base-btf *FILE*
> >> + Pass a base BTF object. Base BTF objects are typically used
> >> + with BTF objects for kernel modules. To avoid duplicating
> >> + all kernel symbols required by modules, BTF objects for
> >> + modules are "split", they are built incrementally on top of
> >> + the kernel (vmlinux) BTF object. So the base BTF reference
> >> + should usually point to the kernel BTF.
> >> +
> >> + When the main BTF object to process (for example, the
> >> + module BTF to dump) is passed as a *FILE*, bpftool attempts
> >> + to autodetect the path for the base object, and passing
> >> + this option is optional. When the main BTF object is passed
> >> + through other handles, this option becomes necessary.
> >> +
> >> EXAMPLES
> >> ========
> >> **# bpftool btf dump id 1226**
> >> @@ -217,3 +232,34 @@ All the standard ways to specify map or program are supported:
> >> **# bpftool btf dump prog tag b88e0a09b1d9759d**
> >>
> >> **# bpftool btf dump prog pinned /sys/fs/bpf/prog_name**
> >> +
> >> +|
> >> +| **# bpftool btf dump file /sys/kernel/btf/i2c_smbus**
> >> +| (or)
> >> +| **# I2C_SMBUS_ID=$(bpftool btf show -p | jq '.[] | select(.name=="i2c_smbus").id')**
> >> +| **# bpftool btf dump id ${I2C_SMBUS_ID} -B /sys/kernel/btf/vmlinux**
> >> +
> >> +::
> >> +
> >> + [104848] STRUCT 'i2c_smbus_alert' size=40 vlen=2
> >> + 'alert' type_id=393 bits_offset=0
> >> + 'ara' type_id=56050 bits_offset=256
> >> + [104849] STRUCT 'alert_data' size=12 vlen=3
> >> + 'addr' type_id=16 bits_offset=0
> >> + 'type' type_id=56053 bits_offset=32
> >> + 'data' type_id=7 bits_offset=64
> >> + [104850] PTR '(anon)' type_id=104848
> >> + [104851] PTR '(anon)' type_id=104849
> >> + [104852] FUNC 'i2c_register_spd' type_id=84745 linkage=static
> >> + [104853] FUNC 'smbalert_driver_init' type_id=1213 linkage=static
> >> + [104854] FUNC_PROTO '(anon)' ret_type_id=18 vlen=1
> >> + 'ara' type_id=56050
> >> + [104855] FUNC 'i2c_handle_smbus_alert' type_id=104854 linkage=static
> >> + [104856] FUNC 'smbalert_remove' type_id=104854 linkage=static
> >> + [104857] FUNC_PROTO '(anon)' ret_type_id=18 vlen=2
> >> + 'ara' type_id=56050
> >> + 'id' type_id=56056
> >> + [104858] FUNC 'smbalert_probe' type_id=104857 linkage=static
> >> + [104859] FUNC 'smbalert_work' type_id=9695 linkage=static
> >> + [104860] FUNC 'smbus_alert' type_id=71367 linkage=static
> >> + [104861] FUNC 'smbus_do_alert' type_id=84827 linkage=static
> >
> > This reminded be that it would be awesome to support "format c"
> > use-case for dumping split BTF in a more sane way. I.e., instead of
> > dumping all types from base and split BTF, only dump necessary (used)
> > forward declarations from base BTF, and then full C dump of only new
> > types from the split (module) BTF. This will become more important as
> > people will start using module BTF more. It's an interesting add-on to
> > libbpf's btf_dumper functionality. Not sure how hard that would be,
> > but I'd imagine it shouldn't require much changes.
> >
> > Just in case anyone wanted to challenge themselves with some more
> > algorithmic patch for libbpf (*wink wink*)...
>
> If you're addressing this to me, I'm not particularly looking for such
> challenge at the moment :). In fact I already noted a few things that I
Just brain dumping a bit, too many small things to keep in mind (or to
keep track in various small TODO lists). Just thought maybe someone
following along would be interested. Alan would come closest to
knowing the internals of btf_dumper, so I'll CC him to try my luck ;)
No pressure.
> would like to fix or improve for bpftool, I will append this one to the
> list. I should maybe start thinking of a tracker of some sort to list
> and share this.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH bpf-next 7/7] tools: bpftool: complete metrics list in "bpftool prog profile" doc
2021-07-29 16:29 [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Quentin Monnet
` (5 preceding siblings ...)
2021-07-29 16:29 ` [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options Quentin Monnet
@ 2021-07-29 16:29 ` Quentin Monnet
2021-07-30 19:06 ` [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Andrii Nakryiko
7 siblings, 0 replies; 19+ messages in thread
From: Quentin Monnet @ 2021-07-29 16:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Quentin Monnet
Profiling programs with bpftool was extended some time ago to support
two new metrics, namely itlb_misses and dtlb_misses (misses for the
instruction/data translation lookaside buffer). Update the manual page
and bash completion accordingly.
Fixes: 450d060e8f75 ("bpftool: Add {i,d}tlb_misses support for bpftool profile")
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 3 ++-
tools/bpf/bpftool/bash-completion/bpftool | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 2ea5df30ff21..91608cb7e44a 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -53,7 +53,8 @@ PROG COMMANDS
| **msg_verdict** | **skb_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
| }
| *METRICs* := {
-| **cycles** | **instructions** | **l1d_loads** | **llc_misses**
+| **cycles** | **instructions** | **l1d_loads** | **llc_misses** |
+| **itlb_misses** | **dtlb_misses**
| }
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index d33b8f308a4c..17a5032b7325 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -345,7 +345,8 @@ _bpftool()
local PROG_TYPE='id pinned tag name'
local MAP_TYPE='id pinned name'
- local METRIC_TYPE='cycles instructions l1d_loads llc_misses'
+ local METRIC_TYPE='cycles instructions l1d_loads llc_misses \
+ itlb_misses dtlb_misses'
case $command in
show|list)
[[ $prev != "$command" ]] && return 0
--
2.30.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and
2021-07-29 16:29 [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Quentin Monnet
` (6 preceding siblings ...)
2021-07-29 16:29 ` [PATCH bpf-next 7/7] tools: bpftool: complete metrics list in "bpftool prog profile" doc Quentin Monnet
@ 2021-07-30 19:06 ` Andrii Nakryiko
2021-07-30 21:48 ` Quentin Monnet
7 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2021-07-30 19:06 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> To work with the different program types, map types, attach types etc.
> supported by eBPF, bpftool needs occasional updates to learn about the new
> features supported by the kernel. When such types translate into new
> keyword for the command line, updates are expected in several locations:
> typically, the help message displayed from bpftool itself, the manual page,
> and the bash completion file should be updated. The options used by the
> different commands for bpftool should also remain synchronised at those
> locations.
>
> Several omissions have occurred in the past, and a number of types are
> still missing today. This set is an attempt to improve the situation. It
> brings up-to-date the lists of types or options in bpftool, and also adds a
> Python script to the BPF selftests to automatically check that most of
> these lists remain synchronised.
>
> Quentin Monnet (7):
> tools: bpftool: slightly ease bash completion updates
> selftests/bpf: check consistency between bpftool source, doc,
> completion
> tools: bpftool: complete and synchronise attach or map types
> tools: bpftool: update and synchronise option list in doc and help msg
> selftests/bpf: update bpftool's consistency script for checking
> options
> tools: bpftool: document and add bash completion for -L, -B options
> tools: bpftool: complete metrics list in "bpftool prog profile" doc
>
> .../bpf/bpftool/Documentation/bpftool-btf.rst | 48 +-
> .../bpftool/Documentation/bpftool-cgroup.rst | 3 +-
> .../bpftool/Documentation/bpftool-feature.rst | 2 +-
> .../bpf/bpftool/Documentation/bpftool-gen.rst | 9 +-
> .../bpftool/Documentation/bpftool-iter.rst | 2 +
> .../bpftool/Documentation/bpftool-link.rst | 3 +-
> .../bpf/bpftool/Documentation/bpftool-map.rst | 3 +-
> .../bpf/bpftool/Documentation/bpftool-net.rst | 2 +-
> .../bpftool/Documentation/bpftool-perf.rst | 2 +-
> .../bpftool/Documentation/bpftool-prog.rst | 36 +-
> .../Documentation/bpftool-struct_ops.rst | 2 +-
> tools/bpf/bpftool/Documentation/bpftool.rst | 12 +-
> tools/bpf/bpftool/bash-completion/bpftool | 69 ++-
> tools/bpf/bpftool/btf.c | 3 +-
> tools/bpf/bpftool/cgroup.c | 3 +-
> tools/bpf/bpftool/common.c | 76 +--
> tools/bpf/bpftool/feature.c | 1 +
> tools/bpf/bpftool/gen.c | 3 +-
> tools/bpf/bpftool/iter.c | 2 +
> tools/bpf/bpftool/link.c | 3 +-
> tools/bpf/bpftool/main.c | 3 +-
> tools/bpf/bpftool/main.h | 3 +-
> tools/bpf/bpftool/map.c | 5 +-
> tools/bpf/bpftool/net.c | 1 +
> tools/bpf/bpftool/perf.c | 5 +-
> tools/bpf/bpftool/prog.c | 8 +-
> tools/bpf/bpftool/struct_ops.c | 2 +-
> tools/testing/selftests/bpf/Makefile | 1 +
> .../selftests/bpf/test_bpftool_synctypes.py | 586 ++++++++++++++++++
> 29 files changed, 802 insertions(+), 96 deletions(-)
> create mode 100755 tools/testing/selftests/bpf/test_bpftool_synctypes.py
>
> --
> 2.30.2
>
The patch set name ends abruptly at "synchronise and "... And what? I
need to know :)
Overall, it looks good, though I can't speak Python much, so I trust
the script works and we'll fix whatever is necessary as we go. I had
one small real nit about not re-formatting tons of existing lines for
no good reason, let's keep Git blame a bit more useful.
Also, it doesn't seem like you are actually calling a new script from
selftests/bpf/Makefile, right? That's good, because otherwise any UAPI
change in kernel header would require bpftool changes in the same
patch. But once this lands, we should probably run this in
kernel-patches CI ([0]) and, maybe, not sure, libbpf CI ([1]) as well.
So please follow up with that as well afterwards, that way you won't
be the only one nagging people about missed doc updates.
[0] https://github.com/kernel-patches/vmtest/tree/master/travis-ci/vmtest
[1] https://github.com/libbpf/libbpf/tree/master/travis-ci/vmtest
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and
2021-07-30 19:06 ` [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Andrii Nakryiko
@ 2021-07-30 21:48 ` Quentin Monnet
2021-07-30 21:58 ` Andrii Nakryiko
0 siblings, 1 reply; 19+ messages in thread
From: Quentin Monnet @ 2021-07-30 21:48 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
2021-07-30 12:06 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> To work with the different program types, map types, attach types etc.
>> supported by eBPF, bpftool needs occasional updates to learn about the new
>> features supported by the kernel. When such types translate into new
>> keyword for the command line, updates are expected in several locations:
>> typically, the help message displayed from bpftool itself, the manual page,
>> and the bash completion file should be updated. The options used by the
>> different commands for bpftool should also remain synchronised at those
>> locations.
>>
>> Several omissions have occurred in the past, and a number of types are
>> still missing today. This set is an attempt to improve the situation. It
>> brings up-to-date the lists of types or options in bpftool, and also adds a
>> Python script to the BPF selftests to automatically check that most of
>> these lists remain synchronised.
>>
>> Quentin Monnet (7):
>> tools: bpftool: slightly ease bash completion updates
>> selftests/bpf: check consistency between bpftool source, doc,
>> completion
>> tools: bpftool: complete and synchronise attach or map types
>> tools: bpftool: update and synchronise option list in doc and help msg
>> selftests/bpf: update bpftool's consistency script for checking
>> options
>> tools: bpftool: document and add bash completion for -L, -B options
>> tools: bpftool: complete metrics list in "bpftool prog profile" doc
>>
>> .../bpf/bpftool/Documentation/bpftool-btf.rst | 48 +-
>> .../bpftool/Documentation/bpftool-cgroup.rst | 3 +-
>> .../bpftool/Documentation/bpftool-feature.rst | 2 +-
>> .../bpf/bpftool/Documentation/bpftool-gen.rst | 9 +-
>> .../bpftool/Documentation/bpftool-iter.rst | 2 +
>> .../bpftool/Documentation/bpftool-link.rst | 3 +-
>> .../bpf/bpftool/Documentation/bpftool-map.rst | 3 +-
>> .../bpf/bpftool/Documentation/bpftool-net.rst | 2 +-
>> .../bpftool/Documentation/bpftool-perf.rst | 2 +-
>> .../bpftool/Documentation/bpftool-prog.rst | 36 +-
>> .../Documentation/bpftool-struct_ops.rst | 2 +-
>> tools/bpf/bpftool/Documentation/bpftool.rst | 12 +-
>> tools/bpf/bpftool/bash-completion/bpftool | 69 ++-
>> tools/bpf/bpftool/btf.c | 3 +-
>> tools/bpf/bpftool/cgroup.c | 3 +-
>> tools/bpf/bpftool/common.c | 76 +--
>> tools/bpf/bpftool/feature.c | 1 +
>> tools/bpf/bpftool/gen.c | 3 +-
>> tools/bpf/bpftool/iter.c | 2 +
>> tools/bpf/bpftool/link.c | 3 +-
>> tools/bpf/bpftool/main.c | 3 +-
>> tools/bpf/bpftool/main.h | 3 +-
>> tools/bpf/bpftool/map.c | 5 +-
>> tools/bpf/bpftool/net.c | 1 +
>> tools/bpf/bpftool/perf.c | 5 +-
>> tools/bpf/bpftool/prog.c | 8 +-
>> tools/bpf/bpftool/struct_ops.c | 2 +-
>> tools/testing/selftests/bpf/Makefile | 1 +
>> .../selftests/bpf/test_bpftool_synctypes.py | 586 ++++++++++++++++++
>> 29 files changed, 802 insertions(+), 96 deletions(-)
>> create mode 100755 tools/testing/selftests/bpf/test_bpftool_synctypes.py
>>
>> --
>> 2.30.2
>>
>
> The patch set name ends abruptly at "synchronise and "... And what? I
> need to know :)
"... and validate types and options" is the missing part. I noticed
after sending -_-. My editor wrapped the Subject: line, resulting in a
truncation. I'll fix for v2 to relieve readers from the suspense :).
>
> Overall, it looks good, though I can't speak Python much, so I trust
> the script works and we'll fix whatever is necessary as we go. I had
> one small real nit about not re-formatting tons of existing lines for
> no good reason, let's keep Git blame a bit more useful.
>
> Also, it doesn't seem like you are actually calling a new script from
> selftests/bpf/Makefile, right? That's good, because otherwise any UAPI
> change in kernel header would require bpftool changes in the same
> patch.
Hmm. Ha. Certainly I wouldn't do such a thing. Please don't look again
at patch 2, and let's focus on v2. 0:)
> But once this lands, we should probably run this in
> kernel-patches CI ([0]) and, maybe, not sure, libbpf CI ([1]) as well.
> So please follow up with that as well afterwards, that way you won't
> be the only one nagging people about missed doc updates.
>
> [0] https://github.com/kernel-patches/vmtest/tree/master/travis-ci/vmtest
> [1] https://github.com/libbpf/libbpf/tree/master/travis-ci/vmtest
>
What's the process to add them to the CI (did I miss some doc)? Should I
just go for a GitHub PR once the script is merged in bpf-next, or do you
have a tool to mirror the relevant scripts? Do we need to have the
Python script in the kernel repo if we don't run it as part of the
selftest suite, by the way?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and
2021-07-30 21:48 ` Quentin Monnet
@ 2021-07-30 21:58 ` Andrii Nakryiko
0 siblings, 0 replies; 19+ messages in thread
From: Andrii Nakryiko @ 2021-07-30 21:58 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf
On Fri, Jul 30, 2021 at 2:48 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2021-07-30 12:06 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> To work with the different program types, map types, attach types etc.
> >> supported by eBPF, bpftool needs occasional updates to learn about the new
> >> features supported by the kernel. When such types translate into new
> >> keyword for the command line, updates are expected in several locations:
> >> typically, the help message displayed from bpftool itself, the manual page,
> >> and the bash completion file should be updated. The options used by the
> >> different commands for bpftool should also remain synchronised at those
> >> locations.
> >>
> >> Several omissions have occurred in the past, and a number of types are
> >> still missing today. This set is an attempt to improve the situation. It
> >> brings up-to-date the lists of types or options in bpftool, and also adds a
> >> Python script to the BPF selftests to automatically check that most of
> >> these lists remain synchronised.
> >>
> >> Quentin Monnet (7):
> >> tools: bpftool: slightly ease bash completion updates
> >> selftests/bpf: check consistency between bpftool source, doc,
> >> completion
> >> tools: bpftool: complete and synchronise attach or map types
> >> tools: bpftool: update and synchronise option list in doc and help msg
> >> selftests/bpf: update bpftool's consistency script for checking
> >> options
> >> tools: bpftool: document and add bash completion for -L, -B options
> >> tools: bpftool: complete metrics list in "bpftool prog profile" doc
> >>
> >> .../bpf/bpftool/Documentation/bpftool-btf.rst | 48 +-
> >> .../bpftool/Documentation/bpftool-cgroup.rst | 3 +-
> >> .../bpftool/Documentation/bpftool-feature.rst | 2 +-
> >> .../bpf/bpftool/Documentation/bpftool-gen.rst | 9 +-
> >> .../bpftool/Documentation/bpftool-iter.rst | 2 +
> >> .../bpftool/Documentation/bpftool-link.rst | 3 +-
> >> .../bpf/bpftool/Documentation/bpftool-map.rst | 3 +-
> >> .../bpf/bpftool/Documentation/bpftool-net.rst | 2 +-
> >> .../bpftool/Documentation/bpftool-perf.rst | 2 +-
> >> .../bpftool/Documentation/bpftool-prog.rst | 36 +-
> >> .../Documentation/bpftool-struct_ops.rst | 2 +-
> >> tools/bpf/bpftool/Documentation/bpftool.rst | 12 +-
> >> tools/bpf/bpftool/bash-completion/bpftool | 69 ++-
> >> tools/bpf/bpftool/btf.c | 3 +-
> >> tools/bpf/bpftool/cgroup.c | 3 +-
> >> tools/bpf/bpftool/common.c | 76 +--
> >> tools/bpf/bpftool/feature.c | 1 +
> >> tools/bpf/bpftool/gen.c | 3 +-
> >> tools/bpf/bpftool/iter.c | 2 +
> >> tools/bpf/bpftool/link.c | 3 +-
> >> tools/bpf/bpftool/main.c | 3 +-
> >> tools/bpf/bpftool/main.h | 3 +-
> >> tools/bpf/bpftool/map.c | 5 +-
> >> tools/bpf/bpftool/net.c | 1 +
> >> tools/bpf/bpftool/perf.c | 5 +-
> >> tools/bpf/bpftool/prog.c | 8 +-
> >> tools/bpf/bpftool/struct_ops.c | 2 +-
> >> tools/testing/selftests/bpf/Makefile | 1 +
> >> .../selftests/bpf/test_bpftool_synctypes.py | 586 ++++++++++++++++++
> >> 29 files changed, 802 insertions(+), 96 deletions(-)
> >> create mode 100755 tools/testing/selftests/bpf/test_bpftool_synctypes.py
> >>
> >> --
> >> 2.30.2
> >>
> >
> > The patch set name ends abruptly at "synchronise and "... And what? I
> > need to know :)
>
> "... and validate types and options" is the missing part. I noticed
> after sending -_-. My editor wrapped the Subject: line, resulting in a
> truncation. I'll fix for v2 to relieve readers from the suspense :).
>
> >
> > Overall, it looks good, though I can't speak Python much, so I trust
> > the script works and we'll fix whatever is necessary as we go. I had
> > one small real nit about not re-formatting tons of existing lines for
> > no good reason, let's keep Git blame a bit more useful.
> >
> > Also, it doesn't seem like you are actually calling a new script from
> > selftests/bpf/Makefile, right? That's good, because otherwise any UAPI
> > change in kernel header would require bpftool changes in the same
> > patch.
>
> Hmm. Ha. Certainly I wouldn't do such a thing. Please don't look again
> at patch 2, and let's focus on v2. 0:)
You got it.
>
> > But once this lands, we should probably run this in
> > kernel-patches CI ([0]) and, maybe, not sure, libbpf CI ([1]) as well.
> > So please follow up with that as well afterwards, that way you won't
> > be the only one nagging people about missed doc updates.
> >
> > [0] https://github.com/kernel-patches/vmtest/tree/master/travis-ci/vmtest
> > [1] https://github.com/libbpf/libbpf/tree/master/travis-ci/vmtest
> >
>
> What's the process to add them to the CI (did I miss some doc)? Should I
> just go for a GitHub PR once the script is merged in bpf-next, or do you
> have a tool to mirror the relevant scripts? Do we need to have the
> Python script in the kernel repo if we don't run it as part of the
> selftest suite, by the way?
Just normal, nicely prepared and described PRs against respective repos.
^ permalink raw reply [flat|nested] 19+ messages in thread