MIME-Version: 1.0
References: <20230116010115.490713-1-irogers@google.com> <CAP-5=fVUgc8xtBzGi66YRUxZHyXvW2kiMjGz39dywaLxrO4Hpg@mail.gmail.com>
 <Y8mAuDvs566zwG67@kernel.org> <Y8myfqy5EMit3Kr/@krava> <CAP-5=fUugnKd=pGpZve7tKThhM5b0AqGMnuiELF+fZQw-xJz9w@mail.gmail.com>
 <ZAmRjk1x4p4TrFb0@gentoo.org> <CAEf4BzZJvEnWdTKVSgdBDr_KEgkW5HLHc7N-HRkmb-drCbg2uw@mail.gmail.com>
In-Reply-To: <CAEf4BzZJvEnWdTKVSgdBDr_KEgkW5HLHc7N-HRkmb-drCbg2uw@mail.gmail.com>
From:   Ian Rogers <irogers@google.com>
Date:   Thu, 9 Mar 2023 19:26:35 -0800
Message-ID: <CAP-5=fWvMFXOaKA0bKaKdzYjqOxTCXGapJy-4x34hJyZxqD-Dw@mail.gmail.com>
Subject: Re: [PATCH v2 0/3] Assume libbpf 1.0+
To:     Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc:     Guilherme Amadio <amadio@gentoo.org>,
        Jiri Olsa <olsajiri@gmail.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Andres Freund <andres@anarazel.de>,
        Quentin Monnet <quentin@isovalent.com>,
        Roberto Sassu <roberto.sassu@huawei.com>,
        Christy Lee <christylee@fb.com>,
        Andrii Nakryiko <andrii@kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
        bpf@vger.kernel.org, Michael Petlan <mpetlan@redhat.com>,
        Ben Hutchings <ben@decadent.org.uk>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Mar 9, 2023 at 9:25=E2=80=AFAM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Mar 8, 2023 at 11:58=E2=80=AFPM Guilherme Amadio <amadio@gentoo.o=
rg> wrote:
> >
> > On Wed, Mar 08, 2023 at 06:13:34PM -0800, Ian Rogers wrote:
> > > On Thu, Jan 19, 2023 at 1:13=E2=80=AFPM Jiri Olsa <olsajiri@gmail.com=
> wrote:
> > > >
> > > > On Thu, Jan 19, 2023 at 02:41:12PM -0300, Arnaldo Carvalho de Melo =
wrote:
> > > > > Em Thu, Jan 19, 2023 at 09:11:03AM -0800, Ian Rogers escreveu:
> > > > > > On Sun, Jan 15, 2023 at 5:01 PM Ian Rogers <irogers@google.com>=
 wrote:
> > > > > > > libbpf 1.0 was a major change in API. Perf has partially supp=
orted
> > > > > > > older libbpf's but an implementation may be:
> > > > > > > ..
> > > > > > >        pr_err("%s: not support, update libbpf\n", __func__);
> > > > > > >        return -ENOTSUP;
> > > > > > > ..
> > > > > > >
> > > > > > > Rather than build a binary that would fail at runtime it is
> > > > > > > preferrential just to build libbpf statically and link agains=
t
> > > > > > > that. The static version is in the kernel tools tree and newe=
r than
> > > > > > > 1.0.
> > > > > > >
> > > > > > > These patches change the libbpf test to only pass when at lea=
st
> > > > > > > version 1.0 is installed, then remove the conditional build a=
nd
> > > > > > > feature logic.
> > > > > > >
> > > > > > > The issue is discussed here:
> > > > > > > https://lore.kernel.org/lkml/20230106151320.619514-1-irogers@=
google.com/
> > > > > > > perf bpf:
> > > > > > >
> > > > > > > A variant of this fix was added to Linux 6.2 in:
> > > > > > > "perf bpf: Avoid build breakage with libbpf < 0.8.0 + LIBBPF_=
DYNAMIC=3D1"
> > > > > > > https://lore.kernel.org/lkml/Y71+eh00Ju7WeEFX@kernel.org/
> > > > > > > This change goes further in removing logic that is now no lon=
ger
> > > > > > > necessary.
> > > > > > >
> > > > > > > v2. Rebase now that breakage fix patch is in linus/master.
> > > > > >
> > > > > > I missed the:
> > > > > > Acked/Tested-by: Jiri Olsa <jolsa@kernel.org>
> > > > > > I believe we are waiting for package maintainer input.
> > > > >
> > > > > Yes, as fedora:37 still is at libbpf 0.8.0 :-\
> > > >
> > > > rawhide (f38) is already on 1.1.0 ... I'll check how bad it'd be to=
 move
> > > > f37 to 1.x, but I had to do bulk update of like 10 other dependent =
packages
> > > > for f38, so not sure how bad it'd be for f37
> > > >
> > > > jirka
> > >
> > > +Guilherme
> > >
> > > We were looking for maintainer input on these changes, but there is n=
o
> > > update in over a month. Here is the original lore link:
> > > https://lore.kernel.org/lkml/CAP-5=3DfVUgc8xtBzGi66YRUxZHyXvW2kiMjGz3=
9dywaLxrO4Hpg@mail.gmail.com/
> > > Should these changes land in perf-tools-next targeting Linux 6.4?
> >
> > Gentoo has libbpf-1.1 already available, so requiring >libbpf-1.0 is no=
t
> > a problem. We (Gentoo) just need to make sure to stabilize libbpf-1.x b=
efore
> > stabilizing newer versions of perf, as the stable libbpf is 0.8.1 at th=
e moment.
> >
>
> libbpf v0.8 is basically all the 1.0 APIs, except by default 1.0
> semantics is not enforced, unless libbpf_set_strict_mode() is enabled.
>
> So, if 0.8 is a restriction, perf can stay on 0.8, use all the same
> APIs that are in 1.0 (except newer one added later, but I'm not sure
> perf needs any of the newer additions), and just stick to setting
> libbpf_set_strict_mode() unconditionally.

Thanks Andrii,

The default perf build is to build against tools/lib/bpf and
statically link libbpf in. This means by default we have the latest
libbpf 1.2. If any perf code has a dependency on 0.8 (we don't support
earlier) we need to #ifdef for it. Currently we have 7 feature tests
for libbpf, but perhaps there is some cruft that's carried forward.
The features are:
 - btf__load_from_kernel_by_id
 - bpf_prog_load
 - bpf_object__next_program
 - bpf_object__next_map
 - bpf_program__set_insns
 - btf__raw_data
 - bpf_map_create

The not present implementations look like:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/p=
erf/util/bpf-loader.c?h=3Dperf-tools#n36
```
int bpf_program__set_insns(struct bpf_program *prog __maybe_unused,
  struct bpf_insn *new_insns __maybe_unused, size_t new_insn_cnt __maybe_un=
used)
{
pr_err("%s: not support, update libbpf\n", __func__);
return -ENOTSUP;
}

int libbpf_register_prog_handler(const char *sec __maybe_unused,
                                 enum bpf_prog_type prog_type __maybe_unuse=
d,
                                 enum bpf_attach_type exp_attach_type
__maybe_unused,
                                 const struct libbpf_prog_handler_opts
*opts __maybe_unused)
{
pr_err("%s: not support, update libbpf\n", __func__);
return -ENOTSUP;
}
```
This will basically mean that while you dynamically linked with libbpf
0.8 you are in all likelihood not going to get proper BPF support.
These changes up the version requirement to 1.0 and get rid entirely
of the feature tests - so no runtime failing implementations. If the
build determines at build time libbpf 1.0+ isn't present then it still
executes, switching from dynamic libbpf to the default static libbpf
that is at 1.2. As mentioned in this thread, distributions like Debian
use the default static linking of libbpf.

I'm not keen to hold on to the feature tests for the complexity that
they hold and their needlessly (as you can always statically link)
broken at runtime behavior. We could but my opinion is, let's not :-)

Thanks,
Ian

> > Best regards,
> > -Guilherme
> >