This patch set adds the ability to point to a custom BTF for the
purposes of BPF CO-RE relocations. This is useful for using BPF CO-RE
on old kernels that don't yet natively support kernel (vmlinux) BTF
and thus libbpf needs application's help in locating kernel BTF
generated separately from the kernel itself. This was already possible
to do through bpf_object__load's attribute struct, but that makes it
inconvenient to use with BPF skeleton, which only allows to specify
bpf_object_open_opts during the open step. Thus, add the ability to
override vmlinux BTF at open time.
Patch #1 adds libbpf changes.
Patch #2 fixes pre-existing memory leak detected during the code review.
Patch #3 switches existing selftests to using open_opts for custom BTF.
Changelog:
----------
v3: https://lore.kernel.org/bpf/CAEf4BzY2cdT44bfbMus=gei27ViqGE1BtGo6XrErSsOCnqtVJg@mail.gmail.com/T/#m877eed1d4cf0a1d3352d3f3d6c5ff158be45c542
v3->v4:
--- Follow Andrii's suggestion to modify cover letter description.--- Delete function bpf_object__load_override_btf.--- Follow Dan's suggestion to add fixes tag and modify commit msg to patch #2.--- Add pathch #3 to switch existing selftests to using open_opts.
--- Load the BTF specified by btf_custom_path to btf_vmlinux_override
instead of btf_bmlinux.
--- Fix the memory leak that may be introduced by the second version
of the patch.
--- Add a new patch to fix the possible memory leak caused by
obj->kconfig.
v1: https://lore.kernel.org/bpf/CAEf4BzaGjEC4t1OefDo11pj2-HfNy0BLhs_G2UREjRNTmb2u=A@mail.gmail.com/t/#m4d9f7c6761fbd2b436b5dfe491cd864b70225804
v1->v2:
-- Change custom_btf_path to btf_custom_path.
-- If the length of btf_custom_path of bpf_obj_open_opts is too long,
return ERR_PTR(-ENAMETOOLONG).
-- Add `custom BTF is in addition to vmlinux BTF`
with btf_custom_path field.
Shuyi Cheng (3):
libbpf: Introduce 'btf_custom_path' to 'bpf_obj_open_opts'
libbpf: Fix the possible memory leak on error
selftests/bpf: switches existing selftests to using open_opts for custom BTF
tools/lib/bpf/libbpf.c | 42 +++++++++++++++++-----
tools/lib/bpf/libbpf.h | 9 ++++-
.../selftests/bpf/prog_tests/core_autosize.c | 22 ++++++------
.../testing/selftests/bpf/prog_tests/core_reloc.c | 28 +++++++--------
4 files changed, 66 insertions(+), 35 deletions(-)
--
1.8.3.1
btf_custom_path allows developers to load custom BTF, and subsequent
CO-RE will use custom BTF for relocation.
Learn from Andrii's comments in [0], add the btf_custom_path parameter
to bpf_obj_open_opts, you can directly use the skeleton's
<objname>_bpf__open_opts function to pass in the btf_custom_path
parameter.
Prior to this, there was also a developer who provided a patch with
similar functions. It is a pity that the follow-up did not continue to
advance. See [1].
[0]https://lore.kernel.org/bpf/CAEf4BzbJZLjNoiK8_VfeVg_Vrg=9iYFv+po-38SMe=UzwDKJ=Q@mail.gmail.com/#t
[1]https://yhbt.net/lore/all/CAEf4Bzbgw49w2PtowsrzKQNcxD4fZRE6AKByX-5-dMo-+oWHHA@mail.gmail.com/
Signed-off-by: Shuyi Cheng <redacted>
---
tools/lib/bpf/libbpf.c | 36 ++++++++++++++++++++++++++++++------
tools/lib/bpf/libbpf.h | 9 ++++++++-
2 files changed, 38 insertions(+), 7 deletions(-)
@@ -498,6 +498,13 @@ struct bpf_object {*itatloadtime.*/structbtf*btf_vmlinux;+/* Path to the custom BTF to be used for BPF CO-RE relocations.+*ThiscustomBTFcompletelyreplacestheuseofvmlinuxBTF+*forthepurposeofCO-RErelocations.+*NOTE:anyotherBPFfeature(e.g.,fentry/fexitprograms,+*struct_ops,etc)willneedactualkernelBTFat/sys/kernel/btf/vmlinux.+*/+char*btf_custom_path;/* vmlinux BTF override for CO-RE relocations */structbtf*btf_vmlinux_override;/* Lazily initialized kernel module BTFs */
@@ -2645,10 +2652,6 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)structbpf_program*prog;inti;-/* CO-RE relocations need kernel BTF */-if(obj->btf_ext&&obj->btf_ext->core_relo_info.len)-returntrue;-/* Support for typed ksyms needs kernel BTF */for(i=0;i<obj->nr_extern;i++){conststructextern_desc*ext;
@@ -2665,6 +2668,13 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)returntrue;}+/* CO-RE relocations need kernel BTF, only when btf_custom_path+*isnotspecified+*/+if(obj->btf_ext&&obj->btf_ext->core_relo_info.len+&&!obj->btf_custom_path)+returntrue;+returnfalse;}
@@ -94,8 +94,15 @@ struct bpf_object_open_opts {*systemKconfigforCONFIG_xxxexterns.*/constchar*kconfig;+/* Path to the custom BTF to be used for BPF CO-RE relocations.+*ThiscustomBTFcompletelyreplacestheuseofvmlinuxBTF+*forthepurposeofCO-RErelocations.+*NOTE:anyotherBPFfeature(e.g.,fentry/fexitprograms,+*struct_ops,etc)willneedactualkernelBTFat/sys/kernel/btf/vmlinux.+*/+char*btf_custom_path;};-#define bpf_object_open_opts__last_field kconfig+#define bpf_object_open_opts__last_field btf_custom_pathLIBBPF_APIstructbpf_object*bpf_object__open(constchar*path);LIBBPF_APIstructbpf_object*
If the strdup() fails then we need to call bpf_object__close(obj) to
avoid a resource leak.
Fixes: 166750b ("libbpf: Support libbpf-provided extern variables")
Cc: Dan Carpenter <redacted>
Signed-off-by: Shuyi Cheng <redacted>
---
tools/lib/bpf/libbpf.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
@@ -125,9 +125,11 @@ void test_core_autosize(void)fd=-1;/* open and load BPF program with custom BTF as the kernel BTF */-skel=test_core_autosize__open();+open_opts.btf_custom_path=btf_file;+open_opts.sz=sizeof(structbpf_object_open_opts);+skel=test_core_autosize__open_opts(&open_opts);if(!ASSERT_OK_PTR(skel,"skel_open"))-return;+gotocleanup;/* disable handle_signed() for now */prog=bpf_object__find_program_by_name(skel->obj,"handle_signed");
@@ -204,13 +204,13 @@ void test_core_autosize(void)skel=NULL;/* now re-load with handle_signed() enabled, it should fail loading */-skel=test_core_autosize__open();+open_opts.btf_custom_path=btf_file;+open_opts.sz=sizeof(structbpf_object_open_opts);+skel=test_core_autosize__open_opts(&opts);if(!ASSERT_OK_PTR(skel,"skel_open"))-return;+gotocleanup;-load_attr.obj=skel->obj;-load_attr.target_btf_path=btf_file;-err=bpf_object__load_xattr(&load_attr);+err=bpf_object__load(skel);if(!ASSERT_ERR(err,"bad_prog_load"))gotocleanup;
On Tue, Jul 13, 2021 at 5:43 AM Shuyi Cheng
[off-list ref] wrote:
This patch set adds the ability to point to a custom BTF for the
purposes of BPF CO-RE relocations. This is useful for using BPF CO-RE
on old kernels that don't yet natively support kernel (vmlinux) BTF
and thus libbpf needs application's help in locating kernel BTF
generated separately from the kernel itself. This was already possible
to do through bpf_object__load's attribute struct, but that makes it
inconvenient to use with BPF skeleton, which only allows to specify
bpf_object_open_opts during the open step. Thus, add the ability to
override vmlinux BTF at open time.
Patch #1 adds libbpf changes.
Patch #2 fixes pre-existing memory leak detected during the code review.
Patch #3 switches existing selftests to using open_opts for custom BTF.
LGTM with some minor things I'll adjust while applying (which I'll
point out in respective patches). So no need to re-send anything.
Thanks.
--- Follow Andrii's suggestion to modify cover letter description.--- Delete function bpf_object__load_override_btf.--- Follow Dan's suggestion to add fixes tag and modify commit msg to patch #2.--- Add pathch #3 to switch existing selftests to using open_opts.
--- Load the BTF specified by btf_custom_path to btf_vmlinux_override
instead of btf_bmlinux.
--- Fix the memory leak that may be introduced by the second version
of the patch.
--- Add a new patch to fix the possible memory leak caused by
obj->kconfig.
v1: https://lore.kernel.org/bpf/CAEf4BzaGjEC4t1OefDo11pj2-HfNy0BLhs_G2UREjRNTmb2u=A@mail.gmail.com/t/#m4d9f7c6761fbd2b436b5dfe491cd864b70225804
v1->v2:
-- Change custom_btf_path to btf_custom_path.
-- If the length of btf_custom_path of bpf_obj_open_opts is too long,
return ERR_PTR(-ENAMETOOLONG).
-- Add `custom BTF is in addition to vmlinux BTF`
with btf_custom_path field.
Shuyi Cheng (3):
libbpf: Introduce 'btf_custom_path' to 'bpf_obj_open_opts'
libbpf: Fix the possible memory leak on error
selftests/bpf: switches existing selftests to using open_opts for custom BTF
tools/lib/bpf/libbpf.c | 42 +++++++++++++++++-----
tools/lib/bpf/libbpf.h | 9 ++++-
.../selftests/bpf/prog_tests/core_autosize.c | 22 ++++++------
.../testing/selftests/bpf/prog_tests/core_reloc.c | 28 +++++++--------
4 files changed, 66 insertions(+), 35 deletions(-)
--
1.8.3.1
On Tue, Jul 13, 2021 at 5:43 AM Shuyi Cheng
[off-list ref] wrote:
quoted hunk
btf_custom_path allows developers to load custom BTF, and subsequent
CO-RE will use custom BTF for relocation.
Learn from Andrii's comments in [0], add the btf_custom_path parameter
to bpf_obj_open_opts, you can directly use the skeleton's
<objname>_bpf__open_opts function to pass in the btf_custom_path
parameter.
Prior to this, there was also a developer who provided a patch with
similar functions. It is a pity that the follow-up did not continue to
advance. See [1].
[0]https://lore.kernel.org/bpf/CAEf4BzbJZLjNoiK8_VfeVg_Vrg=9iYFv+po-38SMe=UzwDKJ=Q@mail.gmail.com/#t
[1]https://yhbt.net/lore/all/CAEf4Bzbgw49w2PtowsrzKQNcxD4fZRE6AKByX-5-dMo-+oWHHA@mail.gmail.com/
Signed-off-by: Shuyi Cheng <redacted>
---
tools/lib/bpf/libbpf.c | 36 ++++++++++++++++++++++++++++++------
tools/lib/bpf/libbpf.h | 9 ++++++++-
2 files changed, 38 insertions(+), 7 deletions(-)
@@ -498,6 +498,13 @@ struct bpf_object {*itatloadtime.*/structbtf*btf_vmlinux;+/* Path to the custom BTF to be used for BPF CO-RE relocations.+*ThiscustomBTFcompletelyreplacestheuseofvmlinuxBTF+*forthepurposeofCO-RErelocations.+*NOTE:anyotherBPFfeature(e.g.,fentry/fexitprograms,+*struct_ops,etc)willneedactualkernelBTFat/sys/kernel/btf/vmlinux.+*/
this comment completely duplicates the one from bpf_object_open_opts,
I'll remove or shorten it
@@ -125,9 +125,11 @@ void test_core_autosize(void)fd=-1;/* open and load BPF program with custom BTF as the kernel BTF */-skel=test_core_autosize__open();+open_opts.btf_custom_path=btf_file;+open_opts.sz=sizeof(structbpf_object_open_opts);+skel=test_core_autosize__open_opts(&open_opts);if(!ASSERT_OK_PTR(skel,"skel_open"))-return;+gotocleanup;/* disable handle_signed() for now */prog=bpf_object__find_program_by_name(skel->obj,"handle_signed");
@@ -204,13 +204,13 @@ void test_core_autosize(void)skel=NULL;/* now re-load with handle_signed() enabled, it should fail loading */-skel=test_core_autosize__open();+open_opts.btf_custom_path=btf_file;+open_opts.sz=sizeof(structbpf_object_open_opts);
For opts structs libbpf provides DECLARE_LIBBPF_OPTS macro for their
initialization which zeroes the struct out and sets its sz
automatically. So I'll switch to using that instead. All the rest
looks good.
On Tue, Jul 13, 2021 at 5:43 AM Shuyi Cheng
[off-list ref] wrote:
This patch mainly replaces the bpf_object_load_attr of
the core_autosize.c and core_reloc.c files with bpf_object_open_opts.
Signed-off-by: Shuyi Cheng <redacted>
---
.../selftests/bpf/prog_tests/core_autosize.c | 22 ++++++++---------
.../testing/selftests/bpf/prog_tests/core_reloc.c | 28 ++++++++++------------
2 files changed, 24 insertions(+), 26 deletions(-)
So I applied this, but it's obvious you haven't bothered even
*building* selftests, because it had at least one compilation warning
and one compilation *error*, not building test_progs at all. I've
noted stuff I fixed (and still remember) below. I understand it might
be your first kernel contribution, but it's not acceptable to submit
patches that don't build. Next time please be more thorough.
[...]
This was reporting a valid warning about dropping const modifier. For
good reason, becyase btf_custom_path in open_opts should have been
`const char *`, I fixed that.
On Tue, Jul 13, 2021 at 5:43 AM Shuyi Cheng
[off-list ref] wrote:
quoted
btf_custom_path allows developers to load custom BTF, and subsequent
CO-RE will use custom BTF for relocation.
Learn from Andrii's comments in [0], add the btf_custom_path parameter
to bpf_obj_open_opts, you can directly use the skeleton's
<objname>_bpf__open_opts function to pass in the btf_custom_path
parameter.
Prior to this, there was also a developer who provided a patch with
similar functions. It is a pity that the follow-up did not continue to
advance. See [1].
[0]https://lore.kernel.org/bpf/CAEf4BzbJZLjNoiK8_VfeVg_Vrg=9iYFv+po-38SMe=UzwDKJ=Q@mail.gmail.com/#t
[1]https://yhbt.net/lore/all/CAEf4Bzbgw49w2PtowsrzKQNcxD4fZRE6AKByX-5-dMo-+oWHHA@mail.gmail.com/
Signed-off-by: Shuyi Cheng <redacted>
---
tools/lib/bpf/libbpf.c | 36 ++++++++++++++++++++++++++++++------
tools/lib/bpf/libbpf.h | 9 ++++++++-
2 files changed, 38 insertions(+), 7 deletions(-)
@@ -498,6 +498,13 @@ struct bpf_object {*itatloadtime.*/structbtf*btf_vmlinux;+/* Path to the custom BTF to be used for BPF CO-RE relocations.+*ThiscustomBTFcompletelyreplacestheuseofvmlinuxBTF+*forthepurposeofCO-RErelocations.+*NOTE:anyotherBPFfeature(e.g.,fentry/fexitprograms,+*struct_ops,etc)willneedactualkernelBTFat/sys/kernel/btf/vmlinux.+*/
this comment completely duplicates the one from bpf_object_open_opts,
I'll remove or shorten it
On Tue, Jul 13, 2021 at 5:43 AM Shuyi Cheng
[off-list ref] wrote:
quoted
This patch mainly replaces the bpf_object_load_attr of
the core_autosize.c and core_reloc.c files with bpf_object_open_opts.
Signed-off-by: Shuyi Cheng <redacted>
---
.../selftests/bpf/prog_tests/core_autosize.c | 22 ++++++++---------
.../testing/selftests/bpf/prog_tests/core_reloc.c | 28 ++++++++++------------
2 files changed, 24 insertions(+), 26 deletions(-)
So I applied this, but it's obvious you haven't bothered even
*building* selftests, because it had at least one compilation warning
and one compilation *error*, not building test_progs at all. I've
noted stuff I fixed (and still remember) below. I understand it might
be your first kernel contribution, but it's not acceptable to submit
patches that don't build. Next time please be more thorough.
I'm very sorry, it was my fault. Although I learned a lot from libbpf,
there is still a lot to learn and improve. Thank you very much for your
advice and the very powerful libbpf.
regards,
Shuyi
This was reporting a valid warning about dropping const modifier. For
good reason, becyase btf_custom_path in open_opts should have been
`const char *`, I fixed that.