2020-05-27 05:04:20

by Daniel Xu

[permalink] [raw]
Subject: [PATCH bpf-next] libbpf: Export bpf_object__load_vmlinux_btf

Right now the libbpf model encourages loading the entire object at once.
In this model, libbpf handles loading BTF from vmlinux for us. However,
it can be useful to selectively load certain maps and programs inside an
object without loading everything else.

In the latter model, there was perviously no way to load BTF on-demand.
This commit exports the bpf_object__load_vmlinux_btf such that we are
able to load BTF on demand.

Signed-off-by: Daniel Xu <[email protected]>
---
tools/lib/bpf/libbpf.c | 2 +-
tools/lib/bpf/libbpf.h | 1 +
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5d60de6fd818..399094b1f580 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2477,7 +2477,7 @@ static inline bool libbpf_prog_needs_vmlinux_btf(struct bpf_program *prog)
return false;
}

-static int bpf_object__load_vmlinux_btf(struct bpf_object *obj)
+int bpf_object__load_vmlinux_btf(struct bpf_object *obj)
{
struct bpf_program *prog;
int err;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 1e2e399a5f2c..6cbd678eb124 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -147,6 +147,7 @@ LIBBPF_API unsigned int bpf_object__kversion(const struct bpf_object *obj);
struct btf;
LIBBPF_API struct btf *bpf_object__btf(const struct bpf_object *obj);
LIBBPF_API int bpf_object__btf_fd(const struct bpf_object *obj);
+LIBBPF_API int bpf_object__load_vmlinux_btf(struct bpf_object *obj);

LIBBPF_API struct bpf_program *
bpf_object__find_program_by_title(const struct bpf_object *obj,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 381a7342ecfc..56415e671c70 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -261,6 +261,7 @@ LIBBPF_0.0.9 {
bpf_iter_create;
bpf_link_get_fd_by_id;
bpf_link_get_next_id;
+ bpf_object__load_vmlinux_btf;
bpf_program__attach_iter;
perf_buffer__consume;
} LIBBPF_0.0.8;
--
2.26.2


2020-05-27 09:54:34

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH bpf-next] libbpf: Export bpf_object__load_vmlinux_btf

On Tue, May 26, 2020 at 7:09 PM Daniel Xu <[email protected]> wrote:
>
> Right now the libbpf model encourages loading the entire object at once.
> In this model, libbpf handles loading BTF from vmlinux for us. However,
> it can be useful to selectively load certain maps and programs inside an
> object without loading everything else.

There is no way to selectively load or not load a map. All maps are
created, unless they are reusing map FD or pinned instances. See
below, I'd like to understand the use case better.

>
> In the latter model, there was perviously no way to load BTF on-demand.
> This commit exports the bpf_object__load_vmlinux_btf such that we are
> able to load BTF on demand.
>

Let's start with the real problem, not a solution. Do you have
specific use case where you need bpf_object__load_vmlinux_btf()? It
might not do anything if none of BPF programs in the object requires
BTF, because it's very much tightly coupled with loading bpf_object as
a whole model. I'd like to understand what you are after with this,
before exposing internal implementation details as an API.

> Signed-off-by: Daniel Xu <[email protected]>
> ---
> tools/lib/bpf/libbpf.c | 2 +-
> tools/lib/bpf/libbpf.h | 1 +
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 3 insertions(+), 1 deletion(-)
>

[...]

2020-05-27 19:18:36

by Daniel Xu

[permalink] [raw]
Subject: Re: [PATCH bpf-next] libbpf: Export bpf_object__load_vmlinux_btf

Hi Andrii,

On Tue May 26, 2020 at 3:09 PM PST, Andrii Nakryiko wrote:
> On Tue, May 26, 2020 at 7:09 PM Daniel Xu <[email protected]> wrote:
> >
> > Right now the libbpf model encourages loading the entire object at once.
> > In this model, libbpf handles loading BTF from vmlinux for us. However,
> > it can be useful to selectively load certain maps and programs inside an
> > object without loading everything else.
>
> There is no way to selectively load or not load a map. All maps are
> created, unless they are reusing map FD or pinned instances. See
> below, I'd like to understand the use case better.
>
> >
> > In the latter model, there was perviously no way to load BTF on-demand.
> > This commit exports the bpf_object__load_vmlinux_btf such that we are
> > able to load BTF on demand.
> >
>
> Let's start with the real problem, not a solution. Do you have
> specific use case where you need bpf_object__load_vmlinux_btf()? It
> might not do anything if none of BPF programs in the object requires
> BTF, because it's very much tightly coupled with loading bpf_object as
> a whole model. I'd like to understand what you are after with this,
> before exposing internal implementation details as an API.

If I try loading a program through the following sequence:

bpf_object__open_file()
bpf_object__find_program_by_name()
bpf_program__load()

And the program require BTF (tp_btf), I get an unavoidable (to the best
of my knowledge) segfault in the following code path:

bpf_program__load()
libbpf_find_attach_btf_id() <-- [0]
__find_vmlinx_btf_id()
find_btf_by_prefix_kind()
btf__find_by_name_kind() <-- boom (btf->nr_types)

because [0] passes prog->obj->btf_vmlinux which is still null. So the
solution I'm proposing is exporting bpf_object__load_vmlinux_btf() and
calling that on struct bpf_object before performing prog loads.

[...]

Thanks,
Daniel

2020-05-27 21:35:46

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH bpf-next] libbpf: Export bpf_object__load_vmlinux_btf

On Wed, May 27, 2020 at 10:12 AM Daniel Xu <[email protected]> wrote:
>
> Hi Andrii,
>
> On Tue May 26, 2020 at 3:09 PM PST, Andrii Nakryiko wrote:
> > On Tue, May 26, 2020 at 7:09 PM Daniel Xu <[email protected]> wrote:
> > >
> > > Right now the libbpf model encourages loading the entire object at once.
> > > In this model, libbpf handles loading BTF from vmlinux for us. However,
> > > it can be useful to selectively load certain maps and programs inside an
> > > object without loading everything else.
> >
> > There is no way to selectively load or not load a map. All maps are
> > created, unless they are reusing map FD or pinned instances. See
> > below, I'd like to understand the use case better.
> >
> > >
> > > In the latter model, there was perviously no way to load BTF on-demand.
> > > This commit exports the bpf_object__load_vmlinux_btf such that we are
> > > able to load BTF on demand.
> > >
> >
> > Let's start with the real problem, not a solution. Do you have
> > specific use case where you need bpf_object__load_vmlinux_btf()? It
> > might not do anything if none of BPF programs in the object requires
> > BTF, because it's very much tightly coupled with loading bpf_object as
> > a whole model. I'd like to understand what you are after with this,
> > before exposing internal implementation details as an API.
>
> If I try loading a program through the following sequence:
>
> bpf_object__open_file()
> bpf_object__find_program_by_name()
> bpf_program__load()
>

bpf_program__load() is just broken and shouldn't have been ever
exposed. It **might** work for trivial BPF programs not using maps,
Kconfig and global variables, etc, but more by accident. I think the
right fix for your use-case is to allow more control of which programs
are auto-loaded. There was a patch by Eric Sage previously adding
bpf_program__set_autoload(), but it never landed. We should actually
do that approach instead.

> And the program require BTF (tp_btf), I get an unavoidable (to the best
> of my knowledge) segfault in the following code path:
>
> bpf_program__load()
> libbpf_find_attach_btf_id() <-- [0]
> __find_vmlinx_btf_id()
> find_btf_by_prefix_kind()
> btf__find_by_name_kind() <-- boom (btf->nr_types)
>
> because [0] passes prog->obj->btf_vmlinux which is still null. So the
> solution I'm proposing is exporting bpf_object__load_vmlinux_btf() and
> calling that on struct bpf_object before performing prog loads.
>
> [...]
>
> Thanks,
> Daniel