[PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Ian Rogers]

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@google.com>
---
 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 */
 	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

Re: [PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Arnaldo Carvalho de Melo]

Em Thu, Oct 14, 2021 at 02:20:49PM -0700, Ian Rogers escreveu:
> BTF_KIND_TAG is present in libbtf 6.0 but not libbtf in 5.15rc4. Make
> the code requiring it conditionally compiled in.

Thanks, applied.

I just removed the part updating lib/bpf, as I have updated it recently:

⬢[acme@toolbox pahole]$ git show cc6c7d473d51832490aa7b743a0ed7f7f9e05592
commit cc6c7d473d51832490aa7b743a0ed7f7f9e05592
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Thu Oct 14 16:27:07 2021 -0300

    Update libbpf to get API to combine BTF
    
    I.e. the one in:
    
     13ebb60ab66799ab libbpf: Add API that copies all BTF types from one BTF object to another
    
    This will be used to paralellize the BTF encoding phase.
    
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/lib/bpf b/lib/bpf
index 980777cc16db75d5..92c1e61a605410b1 160000
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit 980777cc16db75d5628a537c892aefc2640bb242
+Subproject commit 92c1e61a605410b16d6330fdd4a7a4e03add86d4
⬢[acme@toolbox pahole]$ git log --oneline -3
cad8b8b840d621cd (HEAD -> master) btf_encoder: Make BTF_KIND_TAG conditional
a9c99e98815f06bd dwarves: Introduce conf_load->thread_exit() callback
cc6c7d473d518324 Update libbpf to get API to combine BTF
⬢[acme@toolbox pahole]$

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  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 */
>  	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

Re: [PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Andrii Nakryiko]

On Thu, Oct 14, 2021 at 2:20 PM Ian Rogers <irogers@google.com> 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@google.com>
> ---
>  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.

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

>         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
>

Re: [PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Ian Rogers]

On Wed, Oct 20, 2021 at 2:12 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Oct 14, 2021 at 2:20 PM Ian Rogers <irogers@google.com> 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@google.com>
> > ---
> >  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
undefined :-) Tbh, I'm not sure but it seems that you'd be limited to
features in the version of libbpf you compiled against.

> 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@mail.gmail.com/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?

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
> >

Re: [PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Andrii Nakryiko]

On Wed, Oct 20, 2021 at 2:23 PM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Oct 20, 2021 at 2:12 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Oct 14, 2021 at 2:20 PM Ian Rogers <irogers@google.com> 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@google.com>
> > > ---
> > >  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@mail.gmail.com/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

>
> 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
> > >

Re: [PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Ian Rogers]

On Wed, Oct 20, 2021 at 2:27 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 20, 2021 at 2:23 PM Ian Rogers <irogers@google.com> wrote:
> >
> > On Wed, Oct 20, 2021 at 2:12 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Thu, Oct 14, 2021 at 2:20 PM Ian Rogers <irogers@google.com> 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@google.com>
> > > > ---
> > > >  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@mail.gmail.com/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
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.

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
> > > >

Re: [PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Andrii Nakryiko]

On Wed, Oct 20, 2021 at 2:49 PM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Oct 20, 2021 at 2:27 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Oct 20, 2021 at 2:23 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > On Wed, Oct 20, 2021 at 2:12 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Thu, Oct 14, 2021 at 2:20 PM Ian Rogers <irogers@google.com> 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@google.com>
> > > > > ---
> > > > >  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@mail.gmail.com/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.

> 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.

>
> 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
> > > > >

Re: [PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Ian Rogers]

On Wed, Oct 20, 2021 at 3:30 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 20, 2021 at 2:49 PM Ian Rogers <irogers@google.com> wrote:
> >
> > On Wed, Oct 20, 2021 at 2:27 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Wed, Oct 20, 2021 at 2:23 PM Ian Rogers <irogers@google.com> wrote:
> > > >
> > > > On Wed, Oct 20, 2021 at 2:12 PM Andrii Nakryiko
> > > > <andrii.nakryiko@gmail.com> wrote:
> > > > >
> > > > > On Thu, Oct 14, 2021 at 2:20 PM Ian Rogers <irogers@google.com> 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@google.com>
> > > > > > ---
> > > > > >  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@mail.gmail.com/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
> > > > > >

Re: [PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Andrii Nakryiko]

On Wed, Oct 20, 2021 at 4:46 PM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Oct 20, 2021 at 3:30 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Oct 20, 2021 at 2:49 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > On Wed, Oct 20, 2021 at 2:27 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Wed, Oct 20, 2021 at 2:23 PM Ian Rogers <irogers@google.com> wrote:
> > > > >
> > > > > On Wed, Oct 20, 2021 at 2:12 PM Andrii Nakryiko
> > > > > <andrii.nakryiko@gmail.com> wrote:
> > > > > >
> > > > > > On Thu, Oct 14, 2021 at 2:20 PM Ian Rogers <irogers@google.com> 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@google.com>
> > > > > > > ---
> > > > > > >  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@mail.gmail.com/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.

I'm not following. Distros don't track bpf-next for libbpf. They track
https://github.com/libbpf/libbpf as an official libbpf repo. Not sure
what you mean by "different version for pahole"? Use pahole version
that statically links against its own (submodule) libbpf.

>
> > > 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.

That's more reliable than checking BTF_KIND_TAG for sure. BTF_KIND_TAG
comes from kernel UAPI headers, it doesn't directly correlate with
libbpf version, unless you make sure that you only use UAPI headers
that libbpf uses for its own build.

But then, if you use pahole in the mode that dynamically links against
libbpf, then this whole LIBBPF_MAJOR_VERSION business is bogus,
because it only checks what version of libbpf pahole was compiled
against, not which libbpf version will it dynamically link against.


Regardless, it's probably a good idea to add libbpf version APIs that
could be queried at runtime, e.g., int libbpf_major_version(), int
libbpf_minor_version(), etc. That would be usable even with shared
library.

>
> 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
> > > > > > >

Re: [PATCH] btf_encoder: Make BTF_KIND_TAG conditional

[Arnaldo Carvalho de Melo]

Em Wed, Oct 20, 2021 at 02:27:37PM -0700, Andrii Nakryiko escreveu:
> On Wed, Oct 20, 2021 at 2:23 PM Ian Rogers <irogers@google.com> wrote:
> > On Wed, Oct 20, 2021 at 2:12 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > > On Thu, Oct 14, 2021 at 2:20 PM Ian Rogers <irogers@google.com> wrote:
> > > > @@ -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.

This situation is tricky to handle, yeah :-\
 
> > 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.

It is a pain, but fedora also have this policy.
 
> > > 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@mail.gmail.com/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

Ian, would be so kind as to follow up on this so that we get this
situation improved?

- Arnaldo