| Message ID | 1626180159-112996-4-git-send-email-chengshuyi@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add btf_custom_path in bpf_obj_open_opts | expand |
On Tue, Jul 13, 2021 at 5:43 AM Shuyi Cheng <chengshuyi@linux.alibaba.com> 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 <chengshuyi@linux.alibaba.com> > --- > .../selftests/bpf/prog_tests/core_autosize.c | 22 ++++++++--------- > .../testing/selftests/bpf/prog_tests/core_reloc.c | 28 ++++++++++------------ > 2 files changed, 24 insertions(+), 26 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/core_autosize.c b/tools/testing/selftests/bpf/prog_tests/core_autosize.c > index 981c251..d163342 100644 > --- a/tools/testing/selftests/bpf/prog_tests/core_autosize.c > +++ b/tools/testing/selftests/bpf/prog_tests/core_autosize.c > @@ -54,7 +54,7 @@ void test_core_autosize(void) > int err, fd = -1, zero = 0; > int char_id, short_id, int_id, long_long_id, void_ptr_id, id; > struct test_core_autosize* skel = NULL; > - struct bpf_object_load_attr load_attr = {}; > + struct bpf_object_open_opts open_opts = {}; > struct bpf_program *prog; > struct bpf_map *bss_map; > struct btf *btf = NULL; > @@ -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(struct bpf_object_open_opts); > + skel = test_core_autosize__open_opts(&open_opts); > if (!ASSERT_OK_PTR(skel, "skel_open")) > - return; > + goto cleanup; > > /* disable handle_signed() for now */ > prog = bpf_object__find_program_by_name(skel->obj, "handle_signed"); > @@ -135,9 +137,7 @@ void test_core_autosize(void) > goto cleanup; > bpf_program__set_autoload(prog, false); > > - load_attr.obj = skel->obj; > - load_attr.target_btf_path = btf_file; > - err = bpf_object__load_xattr(&load_attr); > + err = bpf_object__load(skel->obj); > if (!ASSERT_OK(err, "prog_load")) > goto cleanup; > > @@ -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(struct bpf_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. > + skel = test_core_autosize__open_opts(&opts); > if (!ASSERT_OK_PTR(skel, "skel_open")) > - return; > + goto cleanup; > > - 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")) > goto cleanup; > [...]
On Tue, Jul 13, 2021 at 5:43 AM Shuyi Cheng <chengshuyi@linux.alibaba.com> 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 <chengshuyi@linux.alibaba.com> > --- > .../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. [...] > > - load_attr.obj = skel->obj; > - load_attr.target_btf_path = btf_file; > - err = bpf_object__load_xattr(&load_attr); > + err = bpf_object__load(skel); This didn't compile outright, because it should have been test_core_autosize__load(skel). > if (!ASSERT_ERR(err, "bad_prog_load")) > goto cleanup; > > diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c > index d02e064..10eb2407 100644 > --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c > +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c > @@ -816,7 +816,7 @@ static size_t roundup_page(size_t sz) > void test_core_reloc(void) > { > const size_t mmap_sz = roundup_page(sizeof(struct data)); > - struct bpf_object_load_attr load_attr = {}; > + struct bpf_object_open_opts open_opts = {}; > struct core_reloc_test_case *test_case; > const char *tp_name, *probe_name; > int err, i, equal; > @@ -846,9 +846,17 @@ void test_core_reloc(void) > continue; > } > > - obj = bpf_object__open_file(test_case->bpf_obj_file, NULL); > + if (test_case->btf_src_file) { > + err = access(test_case->btf_src_file, R_OK); > + if (!ASSERT_OK(err, "btf_src_file")) > + goto cleanup; > + } > + > + open_opts.btf_custom_path = test_case->btf_src_file; 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. > + open_opts.sz = sizeof(struct bpf_object_open_opts); > + obj = bpf_object__open_file(test_case->bpf_obj_file, &open_opts); > if (!ASSERT_OK_PTR(obj, "obj_open")) > - continue; > + goto cleanup; > > probe_name = "raw_tracepoint/sys_enter"; > tp_name = "sys_enter"; [...]
On 7/17/21 4:27 AM, Andrii Nakryiko wrote: > On Tue, Jul 13, 2021 at 5:43 AM Shuyi Cheng > <chengshuyi@linux.alibaba.com> 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 <chengshuyi@linux.alibaba.com> >> --- >> .../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 > [...] > >> >> - load_attr.obj = skel->obj; >> - load_attr.target_btf_path = btf_file; >> - err = bpf_object__load_xattr(&load_attr); >> + err = bpf_object__load(skel); > > This didn't compile outright, because it should have been > test_core_autosize__load(skel). > >> if (!ASSERT_ERR(err, "bad_prog_load")) >> goto cleanup; >> >> diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c >> index d02e064..10eb2407 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c >> +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c >> @@ -816,7 +816,7 @@ static size_t roundup_page(size_t sz) >> void test_core_reloc(void) >> { >> const size_t mmap_sz = roundup_page(sizeof(struct data)); >> - struct bpf_object_load_attr load_attr = {}; >> + struct bpf_object_open_opts open_opts = {}; >> struct core_reloc_test_case *test_case; >> const char *tp_name, *probe_name; >> int err, i, equal; >> @@ -846,9 +846,17 @@ void test_core_reloc(void) >> continue; >> } >> >> - obj = bpf_object__open_file(test_case->bpf_obj_file, NULL); >> + if (test_case->btf_src_file) { >> + err = access(test_case->btf_src_file, R_OK); >> + if (!ASSERT_OK(err, "btf_src_file")) >> + goto cleanup; >> + } >> + >> + open_opts.btf_custom_path = test_case->btf_src_file; > > 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. > >> + open_opts.sz = sizeof(struct bpf_object_open_opts); >> + obj = bpf_object__open_file(test_case->bpf_obj_file, &open_opts); >> if (!ASSERT_OK_PTR(obj, "obj_open")) >> - continue; >> + goto cleanup; >> >> probe_name = "raw_tracepoint/sys_enter"; >> tp_name = "sys_enter"; > > [...] >
diff --git a/tools/testing/selftests/bpf/prog_tests/core_autosize.c b/tools/testing/selftests/bpf/prog_tests/core_autosize.c index 981c251..d163342 100644 --- a/tools/testing/selftests/bpf/prog_tests/core_autosize.c +++ b/tools/testing/selftests/bpf/prog_tests/core_autosize.c @@ -54,7 +54,7 @@ void test_core_autosize(void) int err, fd = -1, zero = 0; int char_id, short_id, int_id, long_long_id, void_ptr_id, id; struct test_core_autosize* skel = NULL; - struct bpf_object_load_attr load_attr = {}; + struct bpf_object_open_opts open_opts = {}; struct bpf_program *prog; struct bpf_map *bss_map; struct btf *btf = NULL; @@ -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(struct bpf_object_open_opts); + skel = test_core_autosize__open_opts(&open_opts); if (!ASSERT_OK_PTR(skel, "skel_open")) - return; + goto cleanup; /* disable handle_signed() for now */ prog = bpf_object__find_program_by_name(skel->obj, "handle_signed"); @@ -135,9 +137,7 @@ void test_core_autosize(void) goto cleanup; bpf_program__set_autoload(prog, false); - load_attr.obj = skel->obj; - load_attr.target_btf_path = btf_file; - err = bpf_object__load_xattr(&load_attr); + err = bpf_object__load(skel->obj); if (!ASSERT_OK(err, "prog_load")) goto cleanup; @@ -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(struct bpf_object_open_opts); + skel = test_core_autosize__open_opts(&opts); if (!ASSERT_OK_PTR(skel, "skel_open")) - return; + goto cleanup; - 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")) goto cleanup; diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c index d02e064..10eb2407 100644 --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c @@ -816,7 +816,7 @@ static size_t roundup_page(size_t sz) void test_core_reloc(void) { const size_t mmap_sz = roundup_page(sizeof(struct data)); - struct bpf_object_load_attr load_attr = {}; + struct bpf_object_open_opts open_opts = {}; struct core_reloc_test_case *test_case; const char *tp_name, *probe_name; int err, i, equal; @@ -846,9 +846,17 @@ void test_core_reloc(void) continue; } - obj = bpf_object__open_file(test_case->bpf_obj_file, NULL); + if (test_case->btf_src_file) { + err = access(test_case->btf_src_file, R_OK); + if (!ASSERT_OK(err, "btf_src_file")) + goto cleanup; + } + + open_opts.btf_custom_path = test_case->btf_src_file; + open_opts.sz = sizeof(struct bpf_object_open_opts); + obj = bpf_object__open_file(test_case->bpf_obj_file, &open_opts); if (!ASSERT_OK_PTR(obj, "obj_open")) - continue; + goto cleanup; probe_name = "raw_tracepoint/sys_enter"; tp_name = "sys_enter"; @@ -861,18 +869,8 @@ void test_core_reloc(void) if (CHECK(!prog, "find_probe", "prog '%s' not found\n", probe_name)) goto cleanup; - - - if (test_case->btf_src_file) { - err = access(test_case->btf_src_file, R_OK); - if (!ASSERT_OK(err, "btf_src_file")) - goto cleanup; - } - - load_attr.obj = obj; - load_attr.log_level = 0; - load_attr.target_btf_path = test_case->btf_src_file; - err = bpf_object__load_xattr(&load_attr); + + err = bpf_object__load(obj); if (err) { if (!test_case->fails) ASSERT_OK(err, "obj_load");
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 <chengshuyi@linux.alibaba.com> --- .../selftests/bpf/prog_tests/core_autosize.c | 22 ++++++++--------- .../testing/selftests/bpf/prog_tests/core_reloc.c | 28 ++++++++++------------ 2 files changed, 24 insertions(+), 26 deletions(-)