| Message ID | 20210920231617.3141867-3-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | libbpf: add legacy uprobe support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 10 maintainers not CCed: kpsingh@kernel.org john.fastabend@gmail.com yhs@fb.com naveen.n.rao@linux.vnet.ibm.com linux-kselftest@vger.kernel.org shuah@kernel.org jolsa@kernel.org songliubraving@fb.com netdev@vger.kernel.org kafai@fb.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 39 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next | success | VM_Test |
On Mon, Sep 20, 2021 at 4:18 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > Make sure to not use ref_ctr_off feature when running on old kernels > that don't support this feature. This allows to test libbpf's legacy > kprobe and uprobe logic on old kernels. > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > --- > .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c > index bf307bb9e446..cbd6b6175d5c 100644 > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c > @@ -14,6 +14,12 @@ void test_attach_probe(void) > struct test_attach_probe* skel; > size_t uprobe_offset; > ssize_t base_addr, ref_ctr_offset; > + bool legacy; > + > + /* check is new-style kprobe/uprobe API is supported */ > + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0; > + > + legacy = true; What is the idea of the above? One of them is a leftover?
On Tue, Sep 21, 2021 at 1:34 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Mon, Sep 20, 2021 at 4:18 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > > > Make sure to not use ref_ctr_off feature when running on old kernels > > that don't support this feature. This allows to test libbpf's legacy > > kprobe and uprobe logic on old kernels. > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > --- > > .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c > > index bf307bb9e446..cbd6b6175d5c 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c > > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c > > @@ -14,6 +14,12 @@ void test_attach_probe(void) > > struct test_attach_probe* skel; > > size_t uprobe_offset; > > ssize_t base_addr, ref_ctr_offset; > > + bool legacy; > > + > > + /* check is new-style kprobe/uprobe API is supported */ > > + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0; > > + > > + legacy = true; > > What is the idea of the above? > One of them is a leftover? Oh, sorry, `legacy = true` was me locally testing, forgot to remove that. This will be properly tested in libbpf CI where we have 4.9 kernel, I was just trying to simulate this locally on modern kernel. I'll re-submit with this removed.
On Tue, Sep 21, 2021 at 1:47 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Sep 21, 2021 at 1:34 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Mon, Sep 20, 2021 at 4:18 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > > > > > Make sure to not use ref_ctr_off feature when running on old kernels > > > that don't support this feature. This allows to test libbpf's legacy > > > kprobe and uprobe logic on old kernels. > > > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > > --- > > > .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++---- > > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c > > > index bf307bb9e446..cbd6b6175d5c 100644 > > > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c > > > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c > > > @@ -14,6 +14,12 @@ void test_attach_probe(void) > > > struct test_attach_probe* skel; > > > size_t uprobe_offset; > > > ssize_t base_addr, ref_ctr_offset; > > > + bool legacy; > > > + > > > + /* check is new-style kprobe/uprobe API is supported */ > > > + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0; > > > + > > > + legacy = true; > > > > What is the idea of the above? > > One of them is a leftover? > > Oh, sorry, `legacy = true` was me locally testing, forgot to remove > that. This will be properly tested in libbpf CI where we have 4.9 > kernel, I was just trying to simulate this locally on modern kernel. > I'll re-submit with this removed. Got it. Could you explain how access() works to detect it?
On Tue, Sep 21, 2021 at 1:48 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Tue, Sep 21, 2021 at 1:47 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Tue, Sep 21, 2021 at 1:34 PM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > > > > > > On Mon, Sep 20, 2021 at 4:18 PM Andrii Nakryiko <andrii@kernel.org> wrote: > > > > > > > > Make sure to not use ref_ctr_off feature when running on old kernels > > > > that don't support this feature. This allows to test libbpf's legacy > > > > kprobe and uprobe logic on old kernels. > > > > > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > > > --- > > > > .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++---- > > > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c > > > > index bf307bb9e446..cbd6b6175d5c 100644 > > > > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c > > > > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c > > > > @@ -14,6 +14,12 @@ void test_attach_probe(void) > > > > struct test_attach_probe* skel; > > > > size_t uprobe_offset; > > > > ssize_t base_addr, ref_ctr_offset; > > > > + bool legacy; > > > > + > > > > + /* check is new-style kprobe/uprobe API is supported */ > > > > + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0; > > > > + > > > > + legacy = true; > > > > > > What is the idea of the above? > > > One of them is a leftover? > > > > Oh, sorry, `legacy = true` was me locally testing, forgot to remove > > that. This will be properly tested in libbpf CI where we have 4.9 > > kernel, I was just trying to simulate this locally on modern kernel. > > I'll re-submit with this removed. > > Got it. > Could you explain how access() works to detect it? Yeah, I'll expand the comment with this as well. The gist is that if /sys/bus/event_source/devices/kprobe/type exists in the system, then new FD-based kprobe attachment through perf system is supported, because that file exposes a magic number that's passed to perf_event_open() to create a kprobe event. So if access() is successful, then it's a new enough kernel. Having said that, this is a kprobe-specific file, while uprobe has its own (under .../uprobe/type). Given we actually care about uprobe features for this legacy check, I'll switch to an uprobe-specific one. But the idea is the same.
diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c index bf307bb9e446..cbd6b6175d5c 100644 --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c @@ -14,6 +14,12 @@ void test_attach_probe(void) struct test_attach_probe* skel; size_t uprobe_offset; ssize_t base_addr, ref_ctr_offset; + bool legacy; + + /* check is new-style kprobe/uprobe API is supported */ + legacy = access("/sys/bus/event_source/devices/kprobe/type", F_OK) != 0; + + legacy = true; base_addr = get_base_addr(); if (CHECK(base_addr < 0, "get_base_addr", @@ -45,10 +51,11 @@ void test_attach_probe(void) goto cleanup; skel->links.handle_kretprobe = kretprobe_link; - ASSERT_EQ(uprobe_ref_ctr, 0, "uprobe_ref_ctr_before"); + if (!legacy) + ASSERT_EQ(uprobe_ref_ctr, 0, "uprobe_ref_ctr_before"); uprobe_opts.retprobe = false; - uprobe_opts.ref_ctr_offset = ref_ctr_offset; + uprobe_opts.ref_ctr_offset = legacy ? 0 : ref_ctr_offset; uprobe_link = bpf_program__attach_uprobe_opts(skel->progs.handle_uprobe, 0 /* self pid */, "/proc/self/exe", @@ -58,11 +65,12 @@ void test_attach_probe(void) goto cleanup; skel->links.handle_uprobe = uprobe_link; - ASSERT_GT(uprobe_ref_ctr, 0, "uprobe_ref_ctr_after"); + if (!legacy) + ASSERT_GT(uprobe_ref_ctr, 0, "uprobe_ref_ctr_after"); /* if uprobe uses ref_ctr, uretprobe has to use ref_ctr as well */ uprobe_opts.retprobe = true; - uprobe_opts.ref_ctr_offset = ref_ctr_offset; + uprobe_opts.ref_ctr_offset = legacy ? 0 : ref_ctr_offset; uretprobe_link = bpf_program__attach_uprobe_opts(skel->progs.handle_uretprobe, -1 /* any pid */, "/proc/self/exe",
Make sure to not use ref_ctr_off feature when running on old kernels that don't support this feature. This allows to test libbpf's legacy kprobe and uprobe logic on old kernels. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- .../selftests/bpf/prog_tests/attach_probe.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)