| Message ID | 20201005163934.331875-1-lrizzo@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] use valid btf in bpf_program__set_attach_target(prog, 0, ...); | expand |
On Mon, Oct 5, 2020 at 9:40 AM Luigi Rizzo <lrizzo@google.com> wrote: > > bpf_program__set_attach_target() will always fail with fd=0 (attach to a > kernel symbol) because obj->btf_vmlinux is NULL and there is no way to > set it. > > Fix this by using libbpf_find_vmlinux_btf_id() > > (on a side note: it is unclear whether btf_vmlinux is meant to be > just temporary storage for use in bpf_object__load_xattr(), or > a property of bpf_object, in which case it could be initialuzed > opportunistically, and properly released in bpf_object__close() ). It's more of a former. vmlinux BTF shouldn't be needed past bpf_object's load phase, so there is no need to keep a few megabytes of memory laying around needlessly. > > Signed-off-by: Luigi Rizzo <lrizzo@google.com> > --- > tools/lib/bpf/libbpf.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index a4f55f8a460d..33bf102259dd 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -10353,9 +10353,8 @@ int bpf_program__set_attach_target(struct bpf_program *prog, > btf_id = libbpf_find_prog_btf_id(attach_func_name, > attach_prog_fd); > else > - btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux, > - attach_func_name, > - prog->expected_attach_type); > + btf_id = libbpf_find_vmlinux_btf_id(attach_func_name, > + prog->expected_attach_type); It's a bit inefficient, if you need to do this for a few programs, but it's ok as a fix. We'll need to unify this internal vmlinux BTF caching at some point. Acked-by: Andrii Nakryiko <andriin@fb.com> > > if (btf_id < 0) > return btf_id; > -- > 2.28.0.806.g8561365e88-goog >
On 10/5/20 9:21 PM, Andrii Nakryiko wrote: > On Mon, Oct 5, 2020 at 9:40 AM Luigi Rizzo <lrizzo@google.com> wrote: >> >> bpf_program__set_attach_target() will always fail with fd=0 (attach to a >> kernel symbol) because obj->btf_vmlinux is NULL and there is no way to >> set it. >> >> Fix this by using libbpf_find_vmlinux_btf_id() >> >> (on a side note: it is unclear whether btf_vmlinux is meant to be >> just temporary storage for use in bpf_object__load_xattr(), or >> a property of bpf_object, in which case it could be initialuzed >> opportunistically, and properly released in bpf_object__close() ). > > It's more of a former. vmlinux BTF shouldn't be needed past > bpf_object's load phase, so there is no need to keep a few megabytes > of memory laying around needlessly. Could you send a v3 with updated commit message wrt side note and prepend e.g. 'bpf, libbpf: ' into subject prefix? Please also carry Andrii's ACK forward. >> Signed-off-by: Luigi Rizzo <lrizzo@google.com> >> --- >> tools/lib/bpf/libbpf.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c >> index a4f55f8a460d..33bf102259dd 100644 >> --- a/tools/lib/bpf/libbpf.c >> +++ b/tools/lib/bpf/libbpf.c >> @@ -10353,9 +10353,8 @@ int bpf_program__set_attach_target(struct bpf_program *prog, >> btf_id = libbpf_find_prog_btf_id(attach_func_name, >> attach_prog_fd); >> else >> - btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux, >> - attach_func_name, >> - prog->expected_attach_type); >> + btf_id = libbpf_find_vmlinux_btf_id(attach_func_name, >> + prog->expected_attach_type); > > It's a bit inefficient, if you need to do this for a few programs, but > it's ok as a fix. We'll need to unify this internal vmlinux BTF > caching at some point. > > Acked-by: Andrii Nakryiko <andriin@fb.com> > > >> >> if (btf_id < 0) >> return btf_id; >> -- >> 2.28.0.806.g8561365e88-goog >>
On Tue, Oct 6, 2020 at 12:33 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > ... > Could you send a v3 with updated commit message wrt side note and prepend > e.g. 'bpf, libbpf: ' into subject prefix? > > Please also carry Andrii's ACK forward. done (note the subject change in v3) thanks luigi
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index a4f55f8a460d..33bf102259dd 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -10353,9 +10353,8 @@ int bpf_program__set_attach_target(struct bpf_program *prog, btf_id = libbpf_find_prog_btf_id(attach_func_name, attach_prog_fd); else - btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux, - attach_func_name, - prog->expected_attach_type); + btf_id = libbpf_find_vmlinux_btf_id(attach_func_name, + prog->expected_attach_type); if (btf_id < 0) return btf_id;
bpf_program__set_attach_target() will always fail with fd=0 (attach to a kernel symbol) because obj->btf_vmlinux is NULL and there is no way to set it. Fix this by using libbpf_find_vmlinux_btf_id() (on a side note: it is unclear whether btf_vmlinux is meant to be just temporary storage for use in bpf_object__load_xattr(), or a property of bpf_object, in which case it could be initialuzed opportunistically, and properly released in bpf_object__close() ). Signed-off-by: Luigi Rizzo <lrizzo@google.com> --- tools/lib/bpf/libbpf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)