On Wed, Oct 20, 2021 at 3:30 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Wed, Oct 20, 2021 at 2:49 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> >
> > On Wed, Oct 20, 2021 at 2:27 PM Andrii Nakryiko
> > <andrii.nakryiko@xxxxxxxxx> wrote:
> > >
> > > On Wed, Oct 20, 2021 at 2:23 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> > > >
> > > > On Wed, Oct 20, 2021 at 2:12 PM Andrii Nakryiko
> > > > <andrii.nakryiko@xxxxxxxxx> wrote:
> > > > >
> > > > > On Thu, Oct 14, 2021 at 2:20 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> > > > > >
> > > > > > BTF_KIND_TAG is present in libbtf 6.0 but not libbtf in 5.15rc4. Make
> > > > > > the code requiring it conditionally compiled in.
> > > > > >
> > > > > > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> > > > > > ---
> > > > > > btf_encoder.c | 7 +++++++
> > > > > > lib/bpf | 2 +-
> > > > > > 2 files changed, 8 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/btf_encoder.c b/btf_encoder.c
> > > > > > index c341f95..400d64b 100644
> > > > > > --- a/btf_encoder.c
> > > > > > +++ b/btf_encoder.c
> > > > > > @@ -141,7 +141,9 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
> > > > > > [BTF_KIND_VAR] = "VAR",
> > > > > > [BTF_KIND_DATASEC] = "DATASEC",
> > > > > > [BTF_KIND_FLOAT] = "FLOAT",
> > > > > > +#ifdef BTF_KIND_TAG /* BTF_KIND_TAG was added in 6.0 */
> > > > > > [BTF_KIND_TAG] = "TAG",
> > > > > > +#endif
> > > > > > };
> > > > > >
> > > > > > static const char *btf__printable_name(const struct btf *btf, uint32_t offset)
> > > > > > @@ -648,6 +650,7 @@ static int32_t btf_encoder__add_datasec(struct btf_encoder *encoder, const char
> > > > > > static int32_t btf_encoder__add_tag(struct btf_encoder *encoder, const char *value, uint32_t type,
> > > > > > int component_idx)
> > > > > > {
> > > > > > +#ifdef BTF_KIND_TAG /* Proxy for libbtf 6.0 */
> > > > >
> > > > > How will this work when libbpf is loaded dynamically? I believe pahole
> > > > > has this mode as well.
> > > >
> > > > Well it won't have a compilation error because BTF_KIND_TAG isn't
> > >
> > > Great, you traded compile-time error for runtime linking error, I hope
> > > that trade off makes sense to Arnaldo.
> > >
> > > > undefined :-) Tbh, I'm not sure but it seems that you'd be limited to
> > > > features in the version of libbpf you compiled against.
> > >
> > > I've been consistently advocating for statically linking against
> > > libbpf exactly to control what APIs and features are supported. But
> > > people stubbornly want dynamic linking. I hope added complexity and
> > > feature detection makes sense in practice for pahole.
> > >
> > > >
> > > > > Also, note that libbpf now provides LIBBPF_MAJOR_VERSION and
> > > > > LIBBPF_MINOR_VERSION macros, starting from 0.5, so no need for
> > > > > guessing the version
> > > >
> > > > This was moved to a header file in:
> > > > https://lore.kernel.org/bpf/CAADnVQJ2qd095mvj3z9u9BXQYCe2OTDn4=Gsu9nv1tjFHc2yqQ@xxxxxxxxxxxxxx/T/
> > > >
> > > > But that header doesn't appear any more:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/tree/tools/lib/bpf
> > > >
> > > > Is that a bug?
> > >
> > > You should be checking here:
> > >
> > > https://github.com/libbpf/libbpf/blob/master/src/libbpf_version.h
> >
> > We don't currently mirror this or bpf-next, but presumably the
>
> Sorry, who's "we" and what's the use case we are talking about here?
> pahole itself is using libbpf from Github mirror and that's what all
> distros either are already doing or strongly encouraged to start
> doing.
I work for Google. When I spoke with Arnaldo it seemed uncommon that a
distro would be tracking bpf-next. There's a policy of a single
library version within Google and a different version for pahole has
some issues for us.
> > released version of libbpf is that in the Linus' tree [1]? There are
> > some things like traceevent that are planned for removal. It seems
> > like a bug that these trees are missing libbpf_version.h.
>
> I misremembered versions, LIBBPF_MAJOR_VERSION/LIBBPF_MINOR_VERSION
> are available starting from v0.6 (unreleased yet), not v0.5. It's a
> pretty recent change, so might have not made it to the tip tree. But
> Github repo does have it, it's synced from bpf-next directly.
Ok, do you suggest something like:
#if defined(LIBBPF_MAJOR_VERSION)
#if LIBBPF_MAJOR_VERSION > 5
..
#endif
#endif
rather than #ifdef BTF_KIND_TAG ? I couldn't see similar examples to
cargo cult from, so there's a likelihood that this could become a
pattern others copy.
Thanks,
Ian
> >
> > Thanks,
> > Ian
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf
> > > >
> > > > Thanks,
> > > > Ian
> > > >
> > > > > > struct btf *btf = encoder->btf;
> > > > > > const struct btf_type *t;
> > > > > > int32_t id;
> > > > > > @@ -663,6 +666,10 @@ static int32_t btf_encoder__add_tag(struct btf_encoder *encoder, const char *val
> > > > > > }
> > > > > >
> > > > > > return id;
> > > > > > +#else
> > > > > > + fprintf(stderr, "error: unable to encode BTF_KIND_TAG due to old libbtf\n");
> > > > > > + return -ENOTSUP;
> > > > > > +#endif
> > > > > > }
> > > > > >
> > > > > > /*
> > > > > > diff --git a/lib/bpf b/lib/bpf
> > > > > > index 980777c..986962f 160000
> > > > > > --- a/lib/bpf
> > > > > > +++ b/lib/bpf
> > > > > > @@ -1 +1 @@
> > > > > > -Subproject commit 980777cc16db75d5628a537c892aefc2640bb242
> > > > > > +Subproject commit 986962fade5dfa89c2890f3854eb040d2a64ab38
> > > > > > --
> > > > > > 2.33.0.1079.g6e70778dc9-goog
> > > > > >
