On 12/20, Quentin Monnet wrote:
> 2018-12-20 09:47 UTC-0800 ~ Stanislav Fomichev <s...@fomichev.me>
> > On 12/20, Quentin Monnet wrote:
> > > Add new probes for eBPF map types, to detect what are the ones available
> > > on the system. Try creating one map of each type, and see if the kernel
> > > complains.
> > >
> > > Sample output:
> > >
> > > # bpftool feature probe kernel
> > > ...
> > > Scanning eBPF map types...
> > > eBPF map_type hash is available
> > > eBPF map_type array is available
> > > eBPF map_type prog_array is available
> > > ...
> > >
> > > # bpftool --json --pretty feature probe kernel
> > > {
> > > ...
> > > "map_types": {
> > > "have_hash_map_type": true,
> > > "have_array_map_type": true,
> > > "have_prog_array_map_type": true,
> > > ...
> > > }
> > > }
> > >
> > > v2:
> > > - Move probes from bpftool to libbpf.
> > > - Remove C-style macros output from this patch.
> > >
> > > Signed-off-by: Quentin Monnet <quentin.mon...@netronome.com>
> > > ---
> > > tools/bpf/bpftool/feature.c | 26 +++++++++++++++
> > > tools/bpf/bpftool/main.h | 3 ++
> > > tools/bpf/bpftool/map.c | 4 ++-
> > > tools/lib/bpf/libbpf.h | 1 +
> > > tools/lib/bpf/libbpf.map | 1 +
> > > tools/lib/bpf/libbpf_probes.c | 59 +++++++++++++++++++++++++++++++++++
> > > 6 files changed, 93 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> > > index 3ba0a0a5904c..3c44953ded5a 100644
> > > --- a/tools/bpf/bpftool/feature.c
> > > +++ b/tools/bpf/bpftool/feature.c
> > > @@ -398,6 +398,26 @@ probe_prog_type(enum bpf_prog_type prog_type, int
> > > kernel_version,
> > > print_bool_feature(feat_name, plain_desc, res);
> > > }
> > > +static void probe_map_type(enum bpf_map_type map_type)
> > > +{
> > > + const char *plain_comment = "eBPF map_type ";
> > > + char feat_name[128], plain_desc[128];
> > > + size_t maxlen;
> > > + bool res;
> > > +
> > > + res = bpf_probe_map_type(map_type, 0);
> > > +
> > > + maxlen = sizeof(plain_desc) - strlen(plain_comment) - 1;
> > > + if (strlen(map_type_name[map_type]) > maxlen) {
> > > + p_info("map type name too long");
> > > + return;
> > > + }
> > > +
> > > + sprintf(feat_name, "have_%s_map_type", map_type_name[map_type]);
> > > + sprintf(plain_desc, "%s%s", plain_comment, map_type_name[map_type]);
> > > + print_bool_feature(feat_name, plain_desc, res);
> > > +}
> > > +
> > > static int do_probe(int argc, char **argv)
> > > {
> > > enum probe_component target = COMPONENT_UNSPEC;
> > > @@ -468,6 +488,12 @@ static int do_probe(int argc, char **argv)
> > > for (i = BPF_PROG_TYPE_UNSPEC + 1; i <
> > > ARRAY_SIZE(prog_type_name); i++)
> > > probe_prog_type(i, kernel_version, supported_types);
> > > + print_end_then_start_section("map_types",
> > > + "Scanning eBPF map types...");
> > > +
> > > + for (i = BPF_MAP_TYPE_UNSPEC + 1; i < map_type_name_size; i++)
> > > + probe_map_type(i);
> > > +
> > > exit_close_json:
> > > if (json_output) {
> > > /* End current "section" of probes */
> > > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > > index 5cfc6601de9b..d7dd84d3c660 100644
> > > --- a/tools/bpf/bpftool/main.h
> > > +++ b/tools/bpf/bpftool/main.h
> > > @@ -75,6 +75,9 @@ static const char * const prog_type_name[] = {
> > > [BPF_PROG_TYPE_FLOW_DISSECTOR] = "flow_dissector",
> > > };
> > > +extern const char * const map_type_name[];
> > > +extern const size_t map_type_name_size;
> > > +
> > > enum bpf_obj_type {
> > > BPF_OBJ_UNKNOWN,
> > > BPF_OBJ_PROG,
> > > diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> > > index 2037e3dc864b..b73985589929 100644
> > > --- a/tools/bpf/bpftool/map.c
> > > +++ b/tools/bpf/bpftool/map.c
> > > @@ -21,7 +21,7 @@
> > > #include "json_writer.h"
> > > #include "main.h"
> > > -static const char * const map_type_name[] = {
> > > +const char * const map_type_name[] = {
> > > [BPF_MAP_TYPE_UNSPEC] = "unspec",
> > > [BPF_MAP_TYPE_HASH] = "hash",
> > > [BPF_MAP_TYPE_ARRAY] = "array",
> > > @@ -48,6 +48,8 @@ static const char * const map_type_name[] = {
> > > [BPF_MAP_TYPE_STACK] = "stack",
> > > };
> > > +const size_t map_type_name_size = ARRAY_SIZE(map_type_name);
> > > +
> > > static bool map_is_per_cpu(__u32 type)
> > > {
> > > return type == BPF_MAP_TYPE_PERCPU_HASH ||
> > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > > index f4bb2764ca9a..202c1ee5c579 100644
> > > --- a/tools/lib/bpf/libbpf.h
> > > +++ b/tools/lib/bpf/libbpf.h
> > > @@ -360,6 +360,7 @@ bpf_prog_linfo__lfind(const struct bpf_prog_linfo
> > > *prog_linfo,
> > > */
> > > LIBBPF_API bool bpf_probe_prog_type(enum bpf_prog_type prog_type,
> > > int kernel_version, __u32 ifindex);
> > > +LIBBPF_API bool bpf_probe_map_type(enum bpf_map_type map_type, __u32
> > > ifindex);
> > > #ifdef __cplusplus
> > > } /* extern "C" */
> > > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > > index 6355e4c80a86..c08f4c726e8e 100644
> > > --- a/tools/lib/bpf/libbpf.map
> > > +++ b/tools/lib/bpf/libbpf.map
> > > @@ -56,6 +56,7 @@ LIBBPF_0.0.1 {
> > > bpf_object__unpin_maps;
> > > bpf_object__unpin_programs;
> > > bpf_perf_event_read_simple;
> > > + bpf_probe_map_type;
> > > bpf_probe_prog_type;
> > > bpf_prog_attach;
> > > bpf_prog_detach;
> > > diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> > > index 2c5e0cdc9f2f..796fe1e66169 100644
> > > --- a/tools/lib/bpf/libbpf_probes.c
> > > +++ b/tools/lib/bpf/libbpf_probes.c
> > > @@ -53,3 +53,62 @@ bool bpf_probe_prog_type(enum bpf_prog_type prog_type,
> > > int kernel_version,
> > > return errno != EINVAL && errno != EOPNOTSUPP;
> > > }
> > > +
> > > +bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
> > > +{
> > > + int key_size, value_size, max_entries, map_flags;
> > > + struct bpf_create_map_attr attr = {};
> > > + int fd = -1, fd_inner;
> > > +
> > > + key_size = sizeof(__u32);
> > > + value_size = sizeof(__u32);
> > > + max_entries = 1;
> > > + map_flags = 0;
> > > +
> > > + if (map_type == BPF_MAP_TYPE_LPM_TRIE) {
> > How about using switch here? So it yells and complains when the new item
> > is added to the bpf_map_type enum.
> > > + key_size = sizeof(__u64);
> > > + value_size = sizeof(__u64);
> > > + map_flags = BPF_F_NO_PREALLOC;
> > > + } else if (map_type == BPF_MAP_TYPE_STACK_TRACE) {
> > > + value_size = sizeof(__u64);
> > > + } else if (map_type == BPF_MAP_TYPE_CGROUP_STORAGE ||
> > > + map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
> > > + key_size = sizeof(struct bpf_cgroup_storage_key);
> > > + value_size = sizeof(__u64);
> > > + max_entries = 0;
> > > + } else if (map_type == BPF_MAP_TYPE_QUEUE ||
> > > + map_type == BPF_MAP_TYPE_STACK) {
> > > + key_size = 0;
> > > + }
>
> Hmm so I had a switch in the first place and it complained even with a
> "default" label because -Wno-switch-enum is used for compiling. So you would
> have me use a switch with all existing map types as explicit labels, even if
> we do not use them? To limit the risk of forgetting to update for new map
> types that would need specific parameters for the probe, is that correct?
Correct. You'd have to list all enum items to pass -Wno-switch-enum. It
looks more rigid, it makes sure we never forget to update the probes
when adding new map types.
