Re: [PATCH bpf-next 2/3] selftests/bpf: add libbpf feature-probing API selftests

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Thu, Dec 16, 2021 at 4:42 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Dec 16, 2021 at 4:21 PM Dave Marchevsky <davemarchevsky@fb.com> wrote:
> >
> > On 12/16/21 2:04 AM, Andrii Nakryiko wrote:
> > > Add selftests for prog/map/prog+helper feature probing APIs. Prog and
> > > map selftests are designed in such a way that they will always test all
> > > the possible prog/map types, based on running kernel's vmlinux BTF enum
> > > definition. This way we'll always be sure that when adding new BPF
> > > program types or map types, libbpf will be always updated accordingly to
> > > be able to feature-detect them.
> > >
> > > BPF prog_helper selftest will have to be manually extended with
> > > interesting and important prog+helper combinations, it's easy, but can't
> > > be completely automated.
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > ---
> >
> > [...]
> >
> > > +     for (e = btf_enum(t), i = 0, n = btf_vlen(t); i < n; e++, i++) {
> > > +             const char *prog_type_name = btf__str_by_offset(btf, e->name_off);
> > > +             enum bpf_prog_type prog_type = (enum bpf_prog_type)e->val;
> > > +             int res;
> > > +
> > > +             if (prog_type == BPF_PROG_TYPE_UNSPEC)
> > > +                     continue;
> > > +
> > > +             if (!test__start_subtest(prog_type_name))
> > > +                     continue;
> > > +
> > > +             res = libbpf_probe_bpf_prog_type(prog_type, NULL);
> > > +             ASSERT_EQ(res, 1, prog_type_name);
> > > +     }
> >
> > I like how easy BTF makes this.
> > Maybe worth trying to probe one-past-the-end of enum to confirm it fails as
> > expected?
> >
>
> Yeah, sure, not a bad idea, I'll add in v2.

Couldn't do it :( Because selftest is using running kernel's BTF to
find out maximum BPF map type. But that maximum on a slightly outdated
kernel could be something that libbpf actually knows about and
supports (because it was compiled against the latest kernel UAPI
headers). So at run time libbpf returns 0 (correct answer, not
map/prog is not supported), but not the expected -EOPNOTSUPP. And I
don't want to hardcode the maximum enum values (like
BPF_MAP_TYPE_BLOOM_FILTER, as of now), because every time we add new
map we'll need to fix selftests in the same patch, which is a bit
annoying, I think.

If we had equivalents of __BPF_FUNC_MAX_ID for bpf_map_type and
bpf_prog_type, we could have just used those constants, so maybe we
should do that instead?

Regardless, I'm sending selftests as they are right now and we can
follow up with UAPI additions and selftests improvements separately.

>
> > Regardless,
> >
> > Acked-by: Dave Marchevsky <davemarchevsky@fb.com>
> >
> > > +cleanup:
> > > +     btf__free(btf);
> > > +}
> >
> > [...]