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

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

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.

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

>  /*
>   * Get bpf_prog_info in continuous memory
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index b3938b3f8fc9..059d168452d7 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -423,6 +423,7 @@ LIBBPF_0.6.0 {
>  LIBBPF_0.7.0 {
>         global:
>                 bpf_btf_load;
> +               bpf_probe_bounded_loops;
>                 bpf_program__log_buf;
>                 bpf_program__log_level;
>                 bpf_program__set_log_buf;
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index 4bdec69523a7..e5bd691059e4 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -356,3 +356,23 @@ bool bpf_probe_large_insn_limit(__u32 ifindex)
>
>         return errno != E2BIG && errno != EINVAL;
>  }
> +
> +/*
> + * Probe for bounded loop support introduced in commit 2589726d12a1
> + * ("bpf: introduce bounded loops").
> + */
> +bool bpf_probe_bounded_loops(__u32 ifindex)
> +{
> +       struct bpf_insn insns[4] = {
> +               BPF_MOV64_IMM(BPF_REG_0, 10),
> +               BPF_ALU64_IMM(BPF_SUB, BPF_REG_0, 1),
> +               BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, -2),
> +               BPF_EXIT_INSN()
> +       };
> +
> +       errno = 0;
> +       probe_load(BPF_PROG_TYPE_SOCKET_FILTER, insns, ARRAY_SIZE(insns), NULL,
> +                  0, ifindex);
> +
> +       return !errno;
> +}
> --
> 2.25.1