2020-04-23 22:35 UTC-0700 ~ Andrii Nakryiko <andriin@xxxxxx>
> Move attach_type_strings into main.h for access in non-cgroup code.
> bpf_attach_type is used for non-cgroup attach types quite widely now. So also
> complete missing string translations for non-cgroup attach types.
>
> Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
> ---
> tools/bpf/bpftool/cgroup.c | 28 +++-------------------------
> tools/bpf/bpftool/main.h | 32 ++++++++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index 62c6a1d7cd18..d1fd9c9f2690 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -31,35 +31,13 @@
>
> static unsigned int query_flags;
>
> -static const char * const attach_type_strings[] = {
> - [BPF_CGROUP_INET_INGRESS] = "ingress",
> - [BPF_CGROUP_INET_EGRESS] = "egress",
> - [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
> - [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_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",
> - [__MAX_BPF_ATTACH_TYPE] = NULL,
So you removed the "[__MAX_BPF_ATTACH_TYPE] = NULL" from the new array,
if I understand correctly this is because all attach type enum members
are now in the new attach_type_name[] so we're safe by looping until we
reach __MAX_BPF_ATTACH_TYPE. Sounds good in theory but...
> -};
> -
> static enum bpf_attach_type parse_attach_type(const char *str)
> {
> enum bpf_attach_type type;
>
> for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
> - if (attach_type_strings[type] &&
> - is_prefix(str, attach_type_strings[type]))
> + if (attach_type_name[type] &&
> + is_prefix(str, attach_type_name[type]))
> return type;
> }
... I'm concerned the "attach_type_name[type]" here could segfault if we
add a new attach type to the kernel, but don't report it immediately to
bpftool's array.
Is there any drawback with keeping the "[__MAX_BPF_ATTACH_TYPE] = NULL"?
Or change here to loop on ARRAY_SIZE(), as you do in your own patch for
link?
