From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, "Alexei Starovoitov" <ast@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Martin KaFai Lau" <kafai@fb.com>,
"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
"Jesper Dangaard Brouer" <brouer@redhat.com>,
"Toke Høiland-Jørgensen" <toke@redhat.com>,
Networking <netdev@vger.kernel.org>
Subject: Re: [PATCH bpf-next v6 9/9] bpf: selftests: Add selftests for module kfunc support
Date: Sat, 2 Oct 2021 03:46:55 +0530 [thread overview]
Message-ID: <20211001221655.4sqtw5vbbdilsttx@apollo.localdomain> (raw)
In-Reply-To: <CAEf4BzYXFU+o-AKj_JP3_2VzAYHRtkyzO5Wu0BD7W=n9UHxe6w@mail.gmail.com>
On Sat, Oct 02, 2021 at 03:43:05AM IST, Andrii Nakryiko wrote:
> On Wed, Sep 29, 2021 at 11:30 PM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > This adds selftests that tests the success and failure path for modules
> > kfuncs (in presence of invalid kfunc calls) for both libbpf and
> > gen_loader. It also adds a prog_test kfunc_btf_id_list so that we can
> > add module BTF ID set from bpf_testmod.
> >
> > This also introduces a couple of test cases to verifier selftests for
> > validating whether we get an error or not depending on if invalid kfunc
> > call remains after elimination of unreachable instructions.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> > include/linux/btf.h | 2 +
> > kernel/bpf/btf.c | 2 +
> > net/bpf/test_run.c | 5 +-
> > tools/testing/selftests/bpf/Makefile | 8 ++--
> > .../selftests/bpf/bpf_testmod/bpf_testmod.c | 23 +++++++++-
> > .../selftests/bpf/prog_tests/ksyms_module.c | 29 ++++++------
> > .../bpf/prog_tests/ksyms_module_libbpf.c | 28 +++++++++++
> > .../selftests/bpf/progs/test_ksyms_module.c | 46 ++++++++++++++-----
> > tools/testing/selftests/bpf/verifier/calls.c | 23 ++++++++++
> > 9 files changed, 135 insertions(+), 31 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/ksyms_module_libbpf.c
> >
>
> [...]
>
> > @@ -243,7 +244,9 @@ BTF_SET_END(test_sk_kfunc_ids)
> >
> > bool bpf_prog_test_check_kfunc_call(u32 kfunc_id, struct module *owner)
> > {
> > - return btf_id_set_contains(&test_sk_kfunc_ids, kfunc_id);
> > + if (btf_id_set_contains(&test_sk_kfunc_ids, kfunc_id))
> > + return true;
> > + return __bpf_prog_test_check_kfunc_call(kfunc_id, owner);
> > }
> >
> > static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index e1ce73be7a5b..df461699932d 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -174,6 +174,7 @@ $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_tes
> > $(Q)$(RM) bpf_testmod/bpf_testmod.ko # force re-compilation
> > $(Q)$(MAKE) $(submake_extras) -C bpf_testmod
> > $(Q)cp bpf_testmod/bpf_testmod.ko $@
> > + $(Q)$(RESOLVE_BTFIDS) -b $(VMLINUX_BTF) bpf_testmod.ko
>
> This should be done by kernel Makefiles, which are used to build
> bpf_testmod.ko. If this is not happening, something is wrong and let's
> try to figure out what.
>
> >
> > $(OUTPUT)/test_stub.o: test_stub.c $(BPFOBJ)
> > $(call msg,CC,,$@)
> > @@ -315,8 +316,9 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \
> > linked_vars.skel.h linked_maps.skel.h
> >
> > LSKELS := kfunc_call_test.c fentry_test.c fexit_test.c fexit_sleep.c \
> > - test_ksyms_module.c test_ringbuf.c atomics.c trace_printk.c \
> > - trace_vprintk.c
> > + test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c
> > +# Generate both light skeleton and libbpf skeleton for these
> > +LSKELS_EXTRA := test_ksyms_module.c
> > SKEL_BLACKLIST += $$(LSKELS)
> >
>
> [...]
>
> > +#define X_0(x)
> > +#define X_1(x) x X_0(x)
> > +#define X_2(x) x X_1(x)
> > +#define X_3(x) x X_2(x)
> > +#define X_4(x) x X_3(x)
> > +#define X_5(x) x X_4(x)
> > +#define X_6(x) x X_5(x)
> > +#define X_7(x) x X_6(x)
> > +#define X_8(x) x X_7(x)
> > +#define X_9(x) x X_8(x)
> > +#define X_10(x) x X_9(x)
> > +#define REPEAT_256(Y) X_2(X_10(X_10(Y))) X_5(X_10(Y)) X_6(Y)
>
> this is impressive, I can even sort of read it :)
>
> > +
> > extern const int bpf_testmod_ksym_percpu __ksym;
> > +extern void bpf_testmod_test_mod_kfunc(int i) __ksym;
> > +extern void bpf_testmod_invalid_mod_kfunc(void) __ksym __weak;
> >
> > -int out_mod_ksym_global = 0;
> > -bool triggered = false;
> > +int out_bpf_testmod_ksym = 0;
> > +const volatile int x = 0;
> >
> > -SEC("raw_tp/sys_enter")
> > -int handler(const void *ctx)
> > +SEC("tc")
>
> Did you switch to tc because kfuncs are not allowed from raw_tp
> programs? Or is there some other reason?
>
Yeah, I was only adding .check_kfunc_call to it because of the tests, I figured
I'd just use a tc prog since other kfunc tests also use that, and because
there's no other user of kfuncs for raw_tp yet.
> > +int load(struct __sk_buff *skb)
> > {
> > - int *val;
> > - __u32 cpu;
> > -
> > - val = (int *)bpf_this_cpu_ptr(&bpf_testmod_ksym_percpu);
> > - out_mod_ksym_global = *val;
> > - triggered = true;
> > + /* This will be kept by clang, but removed by verifier. Since it is
> > + * marked as __weak, libbpf and gen_loader don't error out if BTF ID
> > + * is not found for it, instead imm and off is set to 0 for it.
> > + */
> > + if (x)
> > + bpf_testmod_invalid_mod_kfunc();
> > + bpf_testmod_test_mod_kfunc(42);
> > + out_bpf_testmod_ksym = *(int *)bpf_this_cpu_ptr(&bpf_testmod_ksym_percpu);
> > + return 0;
> > +}
> >
>
> [...]
--
Kartikeya
prev parent reply other threads:[~2021-10-01 22:17 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-30 6:29 [PATCH bpf-next v6 0/9] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
2021-09-30 6:29 ` [PATCH bpf-next v6 1/9] bpf: Introduce BPF support for kernel module function calls Kumar Kartikeya Dwivedi
2021-09-30 6:29 ` [PATCH bpf-next v6 2/9] bpf: Be conservative while processing invalid kfunc calls Kumar Kartikeya Dwivedi
2021-09-30 6:29 ` [PATCH bpf-next v6 3/9] bpf: btf: Introduce helpers for dynamic BTF set registration Kumar Kartikeya Dwivedi
2021-10-01 21:09 ` Andrii Nakryiko
2021-09-30 6:29 ` [PATCH bpf-next v6 4/9] tools: Allow specifying base BTF file in resolve_btfids Kumar Kartikeya Dwivedi
2021-10-01 21:11 ` Andrii Nakryiko
2021-09-30 6:29 ` [PATCH bpf-next v6 5/9] bpf: Enable TCP congestion control kfunc from modules Kumar Kartikeya Dwivedi
2021-10-01 21:14 ` Andrii Nakryiko
2021-09-30 6:29 ` [PATCH bpf-next v6 6/9] libbpf: Support kernel module function calls Kumar Kartikeya Dwivedi
2021-10-01 21:56 ` Andrii Nakryiko
2021-09-30 6:29 ` [PATCH bpf-next v6 7/9] libbpf: Resolve invalid weak kfunc calls with imm = 0, off = 0 Kumar Kartikeya Dwivedi
2021-09-30 6:29 ` [PATCH bpf-next v6 8/9] libbpf: Update gen_loader to emit BTF_KIND_FUNC relocations Kumar Kartikeya Dwivedi
2021-10-01 19:56 ` Alexei Starovoitov
2021-09-30 6:29 ` [PATCH bpf-next v6 9/9] bpf: selftests: Add selftests for module kfunc support Kumar Kartikeya Dwivedi
2021-10-01 22:13 ` Andrii Nakryiko
2021-10-01 22:16 ` Kumar Kartikeya Dwivedi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211001221655.4sqtw5vbbdilsttx@apollo.localdomain \
--to=memxor@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brouer@redhat.com \
--cc=daniel@iogearbox.net \
--cc=kafai@fb.com \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=toke@redhat.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).