Date:   Fri, 21 Aug 2020 20:18:32 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next 00/16] Add libbpf full support for BPF-to-BPF calls

On Fri, Aug 21, 2020 at 4:00 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Thu, Aug 20, 2020 at 04:12:34PM -0700, Andrii Nakryiko wrote:
> > Currently, libbpf supports a limited form of BPF-to-BPF subprogram calls. The
> > restriction is that entry-point BPF program should use *all* of defined
> > sub-programs in BPF .o file. If any of the subprograms is not used, such
> > entry-point BPF program will be rejected by verifier as containing unreachable
> > dead code. This is not a big limitation for cases with single entry-point BPF
> > programs, but is quite a havy restriction for multi-programs that use only
> > partially overlapping set of subprograms.
> >
> > This patch sets removes all such restrictions and adds complete support for
> > using BPF sub-program calls on BPF side. This is achieved through libbpf
> > tracking subprograms individually and detecting which subprograms are used by
> > any given entry-point BPF program, and subsequently only appending and
> > relocating code for just those used subprograms.
> >
> > In addition, libbpf now also supports multiple entry-point BPF programs within
> > the same ELF section. This allows to structure code so that there are few
> > variants of BPF programs of the same type and attaching to the same target
> > (e.g., for tracepoints and kprobes) without the need to worry about ELF
> > section name clashes.
> >
> > This patch set opens way for more wider adoption of BPF subprogram calls,
> > especially for real-world production use-cases with complicated net of
> > subprograms. This will allow to further scale BPF verification process through
> > good use of global functions, which can be verified independently. This is
> > also important prerequisite for static linking which allows static BPF
> > libraries to not worry about naming clashes for section names, as well as use
> > static non-inlined functions (subprograms) without worries of verifier
> > rejecting program due to dead code.
> >
> > Patch set is structured as follows:
> > - patches 1-5 contain various smaller improvements to logging and selftests;
> > - patched 6-11 contain all the libbpf changes necessary to support multi-prog
> >   sections and bpf2bpf subcalls;
> > - patch 12 adds dedicated selftests validating all combinations of possible
> >   sub-calls (within and across sections, static vs global functions);
> > - patch 13 deprecated bpf_program__title() in favor of
> >   bpf_program__section_name(). The intent was to also deprecate
> >   bpf_object__find_program_by_title() as it's now non-sensical with multiple
> >   programs per section. But there were too many selftests uses of this and
> >   I didn't want to delay this patches further and make it even bigger, so left
> >   it for a follow up cleanup;
> > - patches 14-15 remove uses for title-related APIs from bpftool and
> >   bpf_program__title() use from selftests;
> > - patch 16 is converting fexit_bpf2bpf to have explicit subtest (it does
> >   contain 4 subtests, which are not handled as sub-tests).
>
> I've applied the first 5 patches. Cleanup of 'elf:' logs is nice.
> Thanks for doing it.
> The rest of the patches look fine as well, but minimalistic selftest is
> a bit concerning for such major update to libbpf.
> Please consider expanding the tests.

That test is not that minimalistic, actually. It tests all
combinations of bpf2bpf calls (global/static * same/other section),
plus with only subsets of functions used by entry-point BPF programs.
Similarly, fentry_bpf2bpf tests have also pretty complicated patterns,
as well as test_pkt_access.c.

> May be cloudflare's test_cls_redirect.c can be adopted for this purpose?
> test_xdp_noinline.c can also be extended by doing two copies of
> balancer_ingress(). One to process ipv4 another ipv6.

I'll take a look at those, if they are using sub-program calls (not
__always_inline), they are already testing this. I'll see if I can
un-inline some more functions, though.

> Then it will make libbpf to do plenty of intersting call adjustments
> and function munipulations for three programs in "xdp-test" section
> that use different sets of sub-programs.
> test_l4lb_noinline.c can be another candidate.
> The selftest that is part of this set is nice for targeted debugging, but would
> be great to see production bpf prog adopting this exciting libbpf feature.

Sure, I'll also see if strobemeta examples can be modified minimally
to allow non-inlined functions.

Powered by blists - more mailing lists