[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220629165641.4nn7tf5imc7uklcn@muellerd-fedora-MJ0AC3F3>
Date: Wed, 29 Jun 2022 16:56:41 +0000
From: Daniel Müller <deso@...teo.net>
To: Quentin Monnet <quentin@...valent.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, bpf@...r.kernel.org,
netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next 2/2] bpftool: Use feature list in bash completion
On Wed, Jun 29, 2022 at 03:40:19PM +0100, Quentin Monnet wrote:
> Now that bpftool is able to produce a list of known program, map, attach
> types, let's use as much of this as we can in the bash completion file,
> so that we don't have to expand the list each time a new type is added
> to the kernel.
>
> Also update the relevant test script to remove some checks that are no
> longer needed.
>
> Signed-off-by: Quentin Monnet <quentin@...valent.com>
> ---
> tools/bpf/bpftool/bash-completion/bpftool | 21 ++++---------------
> .../selftests/bpf/test_bpftool_synctypes.py | 20 +++---------------
> 2 files changed, 7 insertions(+), 34 deletions(-)
>
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 9cef6516320b..ee177f83b179 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -703,15 +703,8 @@ _bpftool()
> return 0
> ;;
> type)
> - 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 ringbuf inode_storage task_storage \
> - bloom_filter'
> + local BPFTOOL_MAP_CREATE_TYPES="$(bpftool feature list map_types | \
> + grep -v '^unspec$')"
> COMPREPLY=( $( compgen -W "$BPFTOOL_MAP_CREATE_TYPES" -- "$cur" ) )
> return 0
> ;;
> @@ -1039,14 +1032,8 @@ _bpftool()
> return 0
> ;;
> attach|detach)
> - local BPFTOOL_CGROUP_ATTACH_TYPES='cgroup_inet_ingress cgroup_inet_egress \
> - cgroup_inet_sock_create cgroup_sock_ops cgroup_device cgroup_inet4_bind \
> - cgroup_inet6_bind cgroup_inet4_post_bind cgroup_inet6_post_bind \
> - cgroup_inet4_connect cgroup_inet6_connect cgroup_inet4_getpeername \
> - cgroup_inet6_getpeername cgroup_inet4_getsockname cgroup_inet6_getsockname \
> - cgroup_udp4_sendmsg cgroup_udp6_sendmsg cgroup_udp4_recvmsg \
> - cgroup_udp6_recvmsg cgroup_sysctl cgroup_getsockopt cgroup_setsockopt \
> - cgroup_inet_sock_release'
> + local BPFTOOL_CGROUP_ATTACH_TYPES="$(bpftool feature list attach_types | \
> + grep '^cgroup_')"
> local ATTACH_FLAGS='multi override'
> local PROG_TYPE='id pinned tag name'
> # Check for $prev = $command first
> diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> index e443e6542cb9..a6410bebe603 100755
> --- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> +++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py
> @@ -471,12 +471,6 @@ class BashcompExtractor(FileExtractor):
> 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.
> @@ -516,17 +510,12 @@ def main():
> 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 {BpfHeaderExtractor.filename} (bpf_map_type) and {MapFileExtractor.filename} (do_help() TYPE):')
> verify(source_map_types, man_map_types,
> f'Comparing {BpfHeaderExtractor.filename} (bpf_map_type) 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 {BpfHeaderExtractor.filename} (bpf_map_type) and {BashcompExtractor.filename} (BPFTOOL_MAP_CREATE_TYPES):')
>
> # Attach types (names)
>
> @@ -542,8 +531,10 @@ def main():
> 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_info = BashcompExtractor()
> bashcomp_prog_attach_types = bashcomp_info.get_prog_attach_types()
> + bashcomp_info.close()
>
> verify(source_prog_attach_types, help_prog_attach_types,
> f'Comparing {ProgFileExtractor.filename} (bpf_attach_type) and {ProgFileExtractor.filename} (do_help() ATTACH_TYPE):')
> @@ -568,17 +559,12 @@ def main():
> 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 {BpfHeaderExtractor.filename} (bpf_attach_type) and {CgroupFileExtractor.filename} (do_help() ATTACH_TYPE):')
> verify(source_cgroup_attach_types, man_cgroup_attach_types,
> f'Comparing {BpfHeaderExtractor.filename} (bpf_attach_type) 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 {BpfHeaderExtractor.filename} (bpf_attach_type) and {BashcompExtractor.filename} (BPFTOOL_CGROUP_ATTACH_TYPES):')
>
> # Options for remaining commands
>
That is a nice simplification. Looks good to me.
Acked-by: Daniel Müller <deso@...teo.net>
Powered by blists - more mailing lists