Re: [PATCH bpf-next 1/2] libbpf: Probe for bounded loop support

[Paul Chaignon <paul@isovalent.com>]

On Fri, Dec 17, 2021 at 08:12:23AM -0800, Andrii Nakryiko wrote:
> On Fri, Dec 17, 2021 at 4:12 AM Paul Chaignon <paul@isovalent.com> wrote:
> >
> > This patch introduces a new probe to check whether the verifier supports
> > bounded loops as introduced in commit 2589726d12a1 ("bpf: introduce
> > bounded loops"). This patch will allow BPF users such as Cilium to probe
> > for loop support on startup and only unconditionally unroll loops on
> > older kernels.
> >
> > Reviewed-by: Quentin Monnet <quentin@isovalent.com>
> > Signed-off-by: Paul Chaignon <paul@isovalent.com>
> > ---
> >  tools/lib/bpf/libbpf.h        |  1 +
> >  tools/lib/bpf/libbpf.map      |  1 +
> >  tools/lib/bpf/libbpf_probes.c | 20 ++++++++++++++++++++
> >  3 files changed, 22 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 42b2f36fd9f0..3621aaaff67c 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -1058,6 +1058,7 @@ LIBBPF_API bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex);
> >  LIBBPF_API bool bpf_probe_helper(enum bpf_func_id id,
> >                                  enum bpf_prog_type prog_type, __u32 ifindex);
> >  LIBBPF_API bool bpf_probe_large_insn_limit(__u32 ifindex);
> > +LIBBPF_API bool bpf_probe_bounded_loops(__u32 ifindex);
> >
> 
> Nope, see [0], I'm removing bpf_probe_large_insn_limit, so no new
> ad-hoc feature probing APIs, please. There has to be some system to
> this. If you want to add it to bpftool, go ahead, but keep it inside
> bpftool code only. In practice I'd use CO-RE feature detection from
> the BPF program side to pick the best implementation. Worst case, I'd
> add two BPF program implementations and picked one or the other
> (bpf_program__set_autoload(false) to disable one of them) after doing
> feature detection from the process, not relying on shelling out to
> bpftool.

Thanks for the pointer, I wasn't aware of that ongoing work.

For CO-RE feature detection, do you have in mind a bpf_core_field_exists
call to check one of the bpf_func_state fields introduced in the same
commit as bounded loop support, or is there some other CO-RE magic I'm
not aware of?

In any case, I don't think we can assume BTF support in Cilium yet
(soon, hopefully). I'll probably resend as a bpftool-only patch.

> 
>   [0] https://patchwork.kernel.org/project/netdevbpf/patch/20211216070442.1492204-2-andrii@kernel.org/

[...]