[PATCH 0/2] perf tools: Fix perf build with dynamic libbpf

[PATCH 0/2] perf tools: Fix perf build with dynamic libbpf

[Jiri Olsa]

hi,
sending fixes for building perf with LIBBPF_DYNAMIC=1.

We hit the window where perf uses libbpf functions, that did not
make it to the official libbpf release yet and it's breaking perf
build with dynamicly linked libbpf. Fixing this with adding weak
stub functions for possibly missing libbpf function.

thanks,
jirka

---
Jiri Olsa (2):
      perf tools: Add more weak libbpf functions
      perf tools: Add weak variants for the deprecated libbpf functions

 tools/perf/util/bpf-event.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

[PATCH 1/2] perf tools: Add more weak libbpf functions

[Jiri Olsa]

We hit the window where perf uses libbpf functions, that did not
make it to the official libbpf release yet and it's breaking perf
build with dynamicly linked libbpf.

Fixing this by providing the new interface as weak functions which
calls the original libbpf functions. Fortunatelly the changes were
just renames.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/bpf-event.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index 4d3b4cdce176..ceb96360fd12 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -33,6 +33,33 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
        return err ? ERR_PTR(err) : btf;
 }
 
+struct bpf_program * __weak
+bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
+{
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+	return bpf_program__next(prev, obj);
+#pragma GCC diagnostic pop
+}
+
+struct bpf_map * __weak
+bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
+{
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+	return bpf_map__next(prev, obj);
+#pragma GCC diagnostic pop
+}
+
+const void * __weak
+btf__raw_data(const struct btf *btf_ro, __u32 *size)
+{
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+	return btf__get_raw_data(btf_ro, size);
+#pragma GCC diagnostic pop
+}
+
 static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
 {
 	int ret = 0;
-- 
2.31.1

Re: [PATCH 1/2] perf tools: Add more weak libbpf functions

[Ian Rogers]

On Tue, Nov 9, 2021 at 6:07 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> We hit the window where perf uses libbpf functions, that did not
> make it to the official libbpf release yet and it's breaking perf
> build with dynamicly linked libbpf.
>
> Fixing this by providing the new interface as weak functions which
> calls the original libbpf functions. Fortunatelly the changes were
> just renames.

Could we just provide these functions behind a libbpf version #if ?
Weak symbols break things in subtle ways, under certain circumstances
the weak symbol is preferred over the strong due to lazy object file
resolution:
https://maskray.me/blog/2021-06-20-symbol-processing#archive-processing
This bit me last week, but in general you get away with it as the lazy
object file will get processed in an archive exposing the strong
symbol. With an #if you either get a linker error for 2 definitions or
0 definitions, and it's clear what is broken.

In the past we had problems due to constant propagation from weak
const variables, where #if was the solution:
https://lore.kernel.org/lkml/20191001003623.255186-1-irogers@google.com/

There was some recent conversation on libbpf version for pahole here:
https://lore.kernel.org/bpf/CAP-5=fUc3LtU0WYg-Py9Jf+9picaWHJdSw=sdOMA54uY3p1pdw@mail.gmail.com/T/
https://lore.kernel.org/bpf/20211021183330.460681-1-irogers@google.com/

Thanks,
Ian

> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/bpf-event.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index 4d3b4cdce176..ceb96360fd12 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -33,6 +33,33 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
>         return err ? ERR_PTR(err) : btf;
>  }
>
> +struct bpf_program * __weak
> +bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
> +{
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> +       return bpf_program__next(prev, obj);
> +#pragma GCC diagnostic pop
> +}
> +
> +struct bpf_map * __weak
> +bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
> +{
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> +       return bpf_map__next(prev, obj);
> +#pragma GCC diagnostic pop
> +}
> +
> +const void * __weak
> +btf__raw_data(const struct btf *btf_ro, __u32 *size)
> +{
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> +       return btf__get_raw_data(btf_ro, size);
> +#pragma GCC diagnostic pop
> +}
> +
>  static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
>  {
>         int ret = 0;
> --
> 2.31.1
>

Re: [PATCH 1/2] perf tools: Add more weak libbpf functions

[Andrii Nakryiko]

On Tue, Nov 9, 2021 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
>
> On Tue, Nov 9, 2021 at 6:07 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > We hit the window where perf uses libbpf functions, that did not
> > make it to the official libbpf release yet and it's breaking perf
> > build with dynamicly linked libbpf.
> >
> > Fixing this by providing the new interface as weak functions which
> > calls the original libbpf functions. Fortunatelly the changes were
> > just renames.
>
> Could we just provide these functions behind a libbpf version #if ?
> Weak symbols break things in subtle ways, under certain circumstances
> the weak symbol is preferred over the strong due to lazy object file
> resolution:
> https://maskray.me/blog/2021-06-20-symbol-processing#archive-processing
> This bit me last week, but in general you get away with it as the lazy
> object file will get processed in an archive exposing the strong
> symbol. With an #if you either get a linker error for 2 definitions or
> 0 definitions, and it's clear what is broken.
>
> In the past we had problems due to constant propagation from weak
> const variables, where #if was the solution:
> https://lore.kernel.org/lkml/20191001003623.255186-1-irogers@google.com/
>
> There was some recent conversation on libbpf version for pahole here:
> https://lore.kernel.org/bpf/CAP-5=fUc3LtU0WYg-Py9Jf+9picaWHJdSw=sdOMA54uY3p1pdw@mail.gmail.com/T/
> https://lore.kernel.org/bpf/20211021183330.460681-1-irogers@google.com/
>
> Thanks,
> Ian
>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/bpf-event.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> > index 4d3b4cdce176..ceb96360fd12 100644
> > --- a/tools/perf/util/bpf-event.c
> > +++ b/tools/perf/util/bpf-event.c
> > @@ -33,6 +33,33 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
> >         return err ? ERR_PTR(err) : btf;
> >  }
> >
> > +struct bpf_program * __weak
> > +bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
> > +{
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > +       return bpf_program__next(prev, obj);
> > +#pragma GCC diagnostic pop
> > +}
> > +
> > +struct bpf_map * __weak
> > +bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
> > +{
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > +       return bpf_map__next(prev, obj);
> > +#pragma GCC diagnostic pop
> > +}
> > +
> > +const void * __weak
> > +btf__raw_data(const struct btf *btf_ro, __u32 *size)
> > +{
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > +       return btf__get_raw_data(btf_ro, size);

you can still use old variants for the time being, if you want. Were
new APIs used accidentally? Libbpf maintains a guarantee that if some
API is deprecated in favor of the new one, there will be at least one
full libbpf release where both APIs are available and not marked as
deprecated.


> > +#pragma GCC diagnostic pop
> > +}
> > +
> >  static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
> >  {
> >         int ret = 0;
> > --
> > 2.31.1
> >

Re: [PATCH 1/2] perf tools: Add more weak libbpf functions

[Jiri Olsa]

On Tue, Nov 09, 2021 at 03:33:04PM -0800, Andrii Nakryiko wrote:
> On Tue, Nov 9, 2021 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
> >
> > On Tue, Nov 9, 2021 at 6:07 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > We hit the window where perf uses libbpf functions, that did not
> > > make it to the official libbpf release yet and it's breaking perf
> > > build with dynamicly linked libbpf.
> > >
> > > Fixing this by providing the new interface as weak functions which
> > > calls the original libbpf functions. Fortunatelly the changes were
> > > just renames.
> >
> > Could we just provide these functions behind a libbpf version #if ?
> > Weak symbols break things in subtle ways, under certain circumstances
> > the weak symbol is preferred over the strong due to lazy object file
> > resolution:
> > https://maskray.me/blog/2021-06-20-symbol-processing#archive-processing
> > This bit me last week, but in general you get away with it as the lazy
> > object file will get processed in an archive exposing the strong
> > symbol. With an #if you either get a linker error for 2 definitions or
> > 0 definitions, and it's clear what is broken.
> >
> > In the past we had problems due to constant propagation from weak
> > const variables, where #if was the solution:
> > https://lore.kernel.org/lkml/20191001003623.255186-1-irogers@google.com/
> >
> > There was some recent conversation on libbpf version for pahole here:
> > https://lore.kernel.org/bpf/CAP-5=fUc3LtU0WYg-Py9Jf+9picaWHJdSw=sdOMA54uY3p1pdw@mail.gmail.com/T/
> > https://lore.kernel.org/bpf/20211021183330.460681-1-irogers@google.com/
> >
> > Thanks,
> > Ian
> >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  tools/perf/util/bpf-event.c | 27 +++++++++++++++++++++++++++
> > >  1 file changed, 27 insertions(+)
> > >
> > > diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> > > index 4d3b4cdce176..ceb96360fd12 100644
> > > --- a/tools/perf/util/bpf-event.c
> > > +++ b/tools/perf/util/bpf-event.c
> > > @@ -33,6 +33,33 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
> > >         return err ? ERR_PTR(err) : btf;
> > >  }
> > >
> > > +struct bpf_program * __weak
> > > +bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
> > > +{
> > > +#pragma GCC diagnostic push
> > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > +       return bpf_program__next(prev, obj);
> > > +#pragma GCC diagnostic pop
> > > +}
> > > +
> > > +struct bpf_map * __weak
> > > +bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
> > > +{
> > > +#pragma GCC diagnostic push
> > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > +       return bpf_map__next(prev, obj);
> > > +#pragma GCC diagnostic pop
> > > +}
> > > +
> > > +const void * __weak
> > > +btf__raw_data(const struct btf *btf_ro, __u32 *size)
> > > +{
> > > +#pragma GCC diagnostic push
> > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > +       return btf__get_raw_data(btf_ro, size);
> 
> you can still use old variants for the time being, if you want. Were
> new APIs used accidentally? Libbpf maintains a guarantee that if some
> API is deprecated in favor of the new one, there will be at least one
> full libbpf release where both APIs are available and not marked as
> deprecated.

we could use old api instead of btf__raw_data, we could just revert
the perf change

but bpf_object__next_program and bpf_object__next_map are used through
macros like bpf_object__for_each_map or bpf_object__for_each_program,
so we'd need to define 'old versions' of them

jirka

> 
> 
> > > +#pragma GCC diagnostic pop
> > > +}
> > > +
> > >  static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
> > >  {
> > >         int ret = 0;
> > > --
> > > 2.31.1
> > >
> 

Re: [PATCH 1/2] perf tools: Add more weak libbpf functions

[Andrii Nakryiko]

On Wed, Nov 10, 2021 at 12:45 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Tue, Nov 09, 2021 at 03:33:04PM -0800, Andrii Nakryiko wrote:
> > On Tue, Nov 9, 2021 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
> > >
> > > On Tue, Nov 9, 2021 at 6:07 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > > >
> > > > We hit the window where perf uses libbpf functions, that did not
> > > > make it to the official libbpf release yet and it's breaking perf
> > > > build with dynamicly linked libbpf.
> > > >
> > > > Fixing this by providing the new interface as weak functions which
> > > > calls the original libbpf functions. Fortunatelly the changes were
> > > > just renames.
> > >
> > > Could we just provide these functions behind a libbpf version #if ?
> > > Weak symbols break things in subtle ways, under certain circumstances
> > > the weak symbol is preferred over the strong due to lazy object file
> > > resolution:
> > > https://maskray.me/blog/2021-06-20-symbol-processing#archive-processing
> > > This bit me last week, but in general you get away with it as the lazy
> > > object file will get processed in an archive exposing the strong
> > > symbol. With an #if you either get a linker error for 2 definitions or
> > > 0 definitions, and it's clear what is broken.
> > >
> > > In the past we had problems due to constant propagation from weak
> > > const variables, where #if was the solution:
> > > https://lore.kernel.org/lkml/20191001003623.255186-1-irogers@google.com/
> > >
> > > There was some recent conversation on libbpf version for pahole here:
> > > https://lore.kernel.org/bpf/CAP-5=fUc3LtU0WYg-Py9Jf+9picaWHJdSw=sdOMA54uY3p1pdw@mail.gmail.com/T/
> > > https://lore.kernel.org/bpf/20211021183330.460681-1-irogers@google.com/
> > >
> > > Thanks,
> > > Ian
> > >
> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > > ---
> > > >  tools/perf/util/bpf-event.c | 27 +++++++++++++++++++++++++++
> > > >  1 file changed, 27 insertions(+)
> > > >
> > > > diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> > > > index 4d3b4cdce176..ceb96360fd12 100644
> > > > --- a/tools/perf/util/bpf-event.c
> > > > +++ b/tools/perf/util/bpf-event.c
> > > > @@ -33,6 +33,33 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
> > > >         return err ? ERR_PTR(err) : btf;
> > > >  }
> > > >
> > > > +struct bpf_program * __weak
> > > > +bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
> > > > +{
> > > > +#pragma GCC diagnostic push
> > > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > > +       return bpf_program__next(prev, obj);
> > > > +#pragma GCC diagnostic pop
> > > > +}
> > > > +
> > > > +struct bpf_map * __weak
> > > > +bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
> > > > +{
> > > > +#pragma GCC diagnostic push
> > > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > > +       return bpf_map__next(prev, obj);
> > > > +#pragma GCC diagnostic pop
> > > > +}
> > > > +
> > > > +const void * __weak
> > > > +btf__raw_data(const struct btf *btf_ro, __u32 *size)
> > > > +{
> > > > +#pragma GCC diagnostic push
> > > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > > +       return btf__get_raw_data(btf_ro, size);
> >
> > you can still use old variants for the time being, if you want. Were
> > new APIs used accidentally? Libbpf maintains a guarantee that if some
> > API is deprecated in favor of the new one, there will be at least one
> > full libbpf release where both APIs are available and not marked as
> > deprecated.
>
> we could use old api instead of btf__raw_data, we could just revert
> the perf change
>
> but bpf_object__next_program and bpf_object__next_map are used through
> macros like bpf_object__for_each_map or bpf_object__for_each_program,
> so we'd need to define 'old versions' of them

There is nothing magical about bpf_object__for_each_map(). If it's
causing problems, just implement your own iteration logic. You'll be
suffering like this because you are trying to support both shared
library mode and static library mode with libbpf. I'm sorry for your
pain, but you are trying to compile against the latest unreleased
headers, yet work properly with older released libbpf shared library.
It's painful and you know what I think about using shared libraries,
right?

>
> jirka
>
> >
> >
> > > > +#pragma GCC diagnostic pop
> > > > +}
> > > > +
> > > >  static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
> > > >  {
> > > >         int ret = 0;
> > > > --
> > > > 2.31.1
> > > >
> >
>

Re: [PATCH 1/2] perf tools: Add more weak libbpf functions

[Jiri Olsa]

On Wed, Nov 10, 2021 at 02:37:53PM -0800, Andrii Nakryiko wrote:
> On Wed, Nov 10, 2021 at 12:45 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Tue, Nov 09, 2021 at 03:33:04PM -0800, Andrii Nakryiko wrote:
> > > On Tue, Nov 9, 2021 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
> > > >
> > > > On Tue, Nov 9, 2021 at 6:07 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > > > >
> > > > > We hit the window where perf uses libbpf functions, that did not
> > > > > make it to the official libbpf release yet and it's breaking perf
> > > > > build with dynamicly linked libbpf.
> > > > >
> > > > > Fixing this by providing the new interface as weak functions which
> > > > > calls the original libbpf functions. Fortunatelly the changes were
> > > > > just renames.
> > > >
> > > > Could we just provide these functions behind a libbpf version #if ?
> > > > Weak symbols break things in subtle ways, under certain circumstances
> > > > the weak symbol is preferred over the strong due to lazy object file
> > > > resolution:
> > > > https://maskray.me/blog/2021-06-20-symbol-processing#archive-processing
> > > > This bit me last week, but in general you get away with it as the lazy
> > > > object file will get processed in an archive exposing the strong
> > > > symbol. With an #if you either get a linker error for 2 definitions or
> > > > 0 definitions, and it's clear what is broken.
> > > >
> > > > In the past we had problems due to constant propagation from weak
> > > > const variables, where #if was the solution:
> > > > https://lore.kernel.org/lkml/20191001003623.255186-1-irogers@google.com/
> > > >
> > > > There was some recent conversation on libbpf version for pahole here:
> > > > https://lore.kernel.org/bpf/CAP-5=fUc3LtU0WYg-Py9Jf+9picaWHJdSw=sdOMA54uY3p1pdw@mail.gmail.com/T/
> > > > https://lore.kernel.org/bpf/20211021183330.460681-1-irogers@google.com/
> > > >
> > > > Thanks,
> > > > Ian
> > > >
> > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > > > ---
> > > > >  tools/perf/util/bpf-event.c | 27 +++++++++++++++++++++++++++
> > > > >  1 file changed, 27 insertions(+)
> > > > >
> > > > > diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> > > > > index 4d3b4cdce176..ceb96360fd12 100644
> > > > > --- a/tools/perf/util/bpf-event.c
> > > > > +++ b/tools/perf/util/bpf-event.c
> > > > > @@ -33,6 +33,33 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
> > > > >         return err ? ERR_PTR(err) : btf;
> > > > >  }
> > > > >
> > > > > +struct bpf_program * __weak
> > > > > +bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
> > > > > +{
> > > > > +#pragma GCC diagnostic push
> > > > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > > > +       return bpf_program__next(prev, obj);
> > > > > +#pragma GCC diagnostic pop
> > > > > +}
> > > > > +
> > > > > +struct bpf_map * __weak
> > > > > +bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
> > > > > +{
> > > > > +#pragma GCC diagnostic push
> > > > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > > > +       return bpf_map__next(prev, obj);
> > > > > +#pragma GCC diagnostic pop
> > > > > +}
> > > > > +
> > > > > +const void * __weak
> > > > > +btf__raw_data(const struct btf *btf_ro, __u32 *size)
> > > > > +{
> > > > > +#pragma GCC diagnostic push
> > > > > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > > > +       return btf__get_raw_data(btf_ro, size);
> > >
> > > you can still use old variants for the time being, if you want. Were
> > > new APIs used accidentally? Libbpf maintains a guarantee that if some
> > > API is deprecated in favor of the new one, there will be at least one
> > > full libbpf release where both APIs are available and not marked as
> > > deprecated.
> >
> > we could use old api instead of btf__raw_data, we could just revert
> > the perf change
> >
> > but bpf_object__next_program and bpf_object__next_map are used through
> > macros like bpf_object__for_each_map or bpf_object__for_each_program,
> > so we'd need to define 'old versions' of them
> 
> There is nothing magical about bpf_object__for_each_map(). If it's
> causing problems, just implement your own iteration logic. You'll be

ok

> suffering like this because you are trying to support both shared
> library mode and static library mode with libbpf. I'm sorry for your
> pain, but you are trying to compile against the latest unreleased
> headers, yet work properly with older released libbpf shared library.
> It's painful and you know what I think about using shared libraries,
> right?

you are not a fan.. ;-)

thanks,
jirka

Re: [PATCH 1/2] perf tools: Add more weak libbpf functions

[Jiri Olsa]

On Tue, Nov 09, 2021 at 10:49:53AM -0800, Ian Rogers wrote:
> On Tue, Nov 9, 2021 at 6:07 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > We hit the window where perf uses libbpf functions, that did not
> > make it to the official libbpf release yet and it's breaking perf
> > build with dynamicly linked libbpf.
> >
> > Fixing this by providing the new interface as weak functions which
> > calls the original libbpf functions. Fortunatelly the changes were
> > just renames.
> 
> Could we just provide these functions behind a libbpf version #if ?
> Weak symbols break things in subtle ways, under certain circumstances
> the weak symbol is preferred over the strong due to lazy object file
> resolution:
> https://maskray.me/blog/2021-06-20-symbol-processing#archive-processing
> This bit me last week, but in general you get away with it as the lazy
> object file will get processed in an archive exposing the strong
> symbol. With an #if you either get a linker error for 2 definitions or
> 0 definitions, and it's clear what is broken.

hum, I see 2 problems..

usinf #if works nicely for btf__raw_data because it's used directly,
but bpf_object__next_program and bpf_object__next_map are used
through macros:

   #define bpf_object__for_each_map(pos, obj)              \
        for ((pos) = bpf_object__next_map((obj), NULL); \
             (pos) != NULL;                             \
             (pos) = bpf_object__next_map((obj), (pos)))

   #define bpf_object__for_each_program(pos, obj)                  \
        for ((pos) = bpf_object__next_program((obj), NULL);     \
             (pos) != NULL;                                     \
             (pos) = bpf_object__next_program((obj), (pos)))

we would need to provide 'old version' macro as well


another issue is more disturbing.. compiling with LIBBPF_DYNAMIC=1
still seems to take the in-kernel bpf headers, because we use 

  -I$(KTREE)/tools/lib

so any include with '<bpf/...> will pick up the kernel version
and not the one in /usr/include, perhaps the order of '-I...'
could help, I need to check

jirka

> 
> In the past we had problems due to constant propagation from weak
> const variables, where #if was the solution:
> https://lore.kernel.org/lkml/20191001003623.255186-1-irogers@google.com/
> 
> There was some recent conversation on libbpf version for pahole here:
> https://lore.kernel.org/bpf/CAP-5=fUc3LtU0WYg-Py9Jf+9picaWHJdSw=sdOMA54uY3p1pdw@mail.gmail.com/T/
> https://lore.kernel.org/bpf/20211021183330.460681-1-irogers@google.com/
> 
> Thanks,
> Ian
> 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/bpf-event.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> > index 4d3b4cdce176..ceb96360fd12 100644
> > --- a/tools/perf/util/bpf-event.c
> > +++ b/tools/perf/util/bpf-event.c
> > @@ -33,6 +33,33 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
> >         return err ? ERR_PTR(err) : btf;
> >  }
> >
> > +struct bpf_program * __weak
> > +bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
> > +{
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > +       return bpf_program__next(prev, obj);
> > +#pragma GCC diagnostic pop
> > +}
> > +
> > +struct bpf_map * __weak
> > +bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
> > +{
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > +       return bpf_map__next(prev, obj);
> > +#pragma GCC diagnostic pop
> > +}
> > +
> > +const void * __weak
> > +btf__raw_data(const struct btf *btf_ro, __u32 *size)
> > +{
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > +       return btf__get_raw_data(btf_ro, size);
> > +#pragma GCC diagnostic pop
> > +}
> > +
> >  static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
> >  {
> >         int ret = 0;
> > --
> > 2.31.1
> >
> 

Re: [PATCH 1/2] perf tools: Add more weak libbpf functions

[Arnaldo Carvalho de Melo]

Em Tue, Nov 09, 2021 at 10:49:53AM -0800, Ian Rogers escreveu:
> On Tue, Nov 9, 2021 at 6:07 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > We hit the window where perf uses libbpf functions, that did not
> > make it to the official libbpf release yet and it's breaking perf
> > build with dynamicly linked libbpf.
> >
> > Fixing this by providing the new interface as weak functions which
> > calls the original libbpf functions. Fortunatelly the changes were
> > just renames.
> 
> Could we just provide these functions behind a libbpf version #if ?
> Weak symbols break things in subtle ways, under certain circumstances
> the weak symbol is preferred over the strong due to lazy object file
> resolution:
> https://maskray.me/blog/2021-06-20-symbol-processing#archive-processing
> This bit me last week, but in general you get away with it as the lazy
> object file will get processed in an archive exposing the strong
> symbol. With an #if you either get a linker error for 2 definitions or
> 0 definitions, and it's clear what is broken.
> 
> In the past we had problems due to constant propagation from weak
> const variables, where #if was the solution:
> https://lore.kernel.org/lkml/20191001003623.255186-1-irogers@google.com/
> 
> There was some recent conversation on libbpf version for pahole here:
> https://lore.kernel.org/bpf/CAP-5=fUc3LtU0WYg-Py9Jf+9picaWHJdSw=sdOMA54uY3p1pdw@mail.gmail.com/T/
> https://lore.kernel.org/bpf/20211021183330.460681-1-irogers@google.com/

Valid concerns and we should improve the situation, but I'm going with
Jiri's stopgap solution so as to have everything buildind for v5.16,
which window is closing.

We can improve this in followup patches.

Thanks,

- Arnaldo

[PATCH 2/2] perf tools: Add weak variants for the deprecated libbpf functions

[Jiri Olsa]

Adding weak functions for deprecated libbpf functions, so that
we don't get build fail with libbpf version where they are
finally removed.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/bpf-event.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index ceb96360fd12..476427f3e804 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -22,6 +22,20 @@
 #include "record.h"
 #include "util/synthetic-events.h"
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wredundant-decls"
+int btf__get_from_id(__u32 id, struct btf **btf);
+struct bpf_program *bpf_program__next(struct bpf_program *prog,
+				      const struct bpf_object *obj);
+struct bpf_map *bpf_map__next(const struct bpf_map *map, const struct bpf_object *obj);
+const void *btf__get_raw_data(const struct btf *btf, __u32 *size);
+#pragma GCC diagnostic pop
+
+int __weak btf__get_from_id(__u32 id __maybe_unused, struct btf **btf __maybe_unused)
+{
+	return -ENOTSUP;
+}
+
 struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
 {
        struct btf *btf;
@@ -33,6 +47,13 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
        return err ? ERR_PTR(err) : btf;
 }
 
+struct bpf_program * __weak
+bpf_program__next(struct bpf_program *prog __maybe_unused,
+		  const struct bpf_object *obj __maybe_unused)
+{
+	return NULL;
+}
+
 struct bpf_program * __weak
 bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
 {
@@ -42,6 +63,12 @@ bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
 #pragma GCC diagnostic pop
 }
 
+struct bpf_map * __weak bpf_map__next(const struct bpf_map *map __maybe_unused,
+				      const struct bpf_object *obj __maybe_unused)
+{
+	return NULL;
+}
+
 struct bpf_map * __weak
 bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
 {
@@ -51,6 +78,12 @@ bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
 #pragma GCC diagnostic pop
 }
 
+const void * __weak btf__get_raw_data(const struct btf *btf __maybe_unused,
+				      __u32 *size __maybe_unused)
+{
+	return NULL;
+}
+
 const void * __weak
 btf__raw_data(const struct btf *btf_ro, __u32 *size)
 {
-- 
2.31.1