On Tue, Apr 26, 2022 at 09:05:44AM IST, Alexei Starovoitov wrote:
> On Mon, Apr 25, 2022 at 03:19:00AM +0530, Kumar Kartikeya Dwivedi wrote:
> > Reuse bpf_prog_test functions to test the support for PTR_TO_BTF_ID in
> > BPF map case, including some tests that verify implementation sanity and
> > corner cases.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> > ---
> > net/bpf/test_run.c | 45 +-
> > tools/testing/selftests/bpf/test_verifier.c | 55 +-
> > .../testing/selftests/bpf/verifier/map_kptr.c | 469 ++++++++++++++++++
> > 3 files changed, 562 insertions(+), 7 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/verifier/map_kptr.c
> >
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index e7b9c2636d10..29fe32821e7e 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -584,6 +584,12 @@ noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p)
> > {
> > }
> >
> > +noinline struct prog_test_ref_kfunc *
> > +bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b)
> > +{
> > + return &prog_test_struct;
> > +}
> > +
> > struct prog_test_pass1 {
> > int x0;
> > struct {
> > @@ -669,6 +675,7 @@ BTF_ID(func, bpf_kfunc_call_test3)
> > BTF_ID(func, bpf_kfunc_call_test_acquire)
> > BTF_ID(func, bpf_kfunc_call_test_release)
> > BTF_ID(func, bpf_kfunc_call_memb_release)
> > +BTF_ID(func, bpf_kfunc_call_test_kptr_get)
> > BTF_ID(func, bpf_kfunc_call_test_pass_ctx)
> > BTF_ID(func, bpf_kfunc_call_test_pass1)
> > BTF_ID(func, bpf_kfunc_call_test_pass2)
> > @@ -682,6 +689,7 @@ BTF_SET_END(test_sk_check_kfunc_ids)
> >
> > BTF_SET_START(test_sk_acquire_kfunc_ids)
> > BTF_ID(func, bpf_kfunc_call_test_acquire)
> > +BTF_ID(func, bpf_kfunc_call_test_kptr_get)
> > BTF_SET_END(test_sk_acquire_kfunc_ids)
> >
> > BTF_SET_START(test_sk_release_kfunc_ids)
> > @@ -691,8 +699,13 @@ BTF_SET_END(test_sk_release_kfunc_ids)
> >
> > BTF_SET_START(test_sk_ret_null_kfunc_ids)
> > BTF_ID(func, bpf_kfunc_call_test_acquire)
> > +BTF_ID(func, bpf_kfunc_call_test_kptr_get)
> > BTF_SET_END(test_sk_ret_null_kfunc_ids)
> >
> > +BTF_SET_START(test_sk_kptr_acquire_kfunc_ids)
> > +BTF_ID(func, bpf_kfunc_call_test_kptr_get)
> > +BTF_SET_END(test_sk_kptr_acquire_kfunc_ids)
> > +
> > static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size,
> > u32 size, u32 headroom, u32 tailroom)
> > {
> > @@ -1579,14 +1592,36 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
> >
> > static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
> > .owner = THIS_MODULE,
> > - .check_set = &test_sk_check_kfunc_ids,
> > - .acquire_set = &test_sk_acquire_kfunc_ids,
> > - .release_set = &test_sk_release_kfunc_ids,
> > - .ret_null_set = &test_sk_ret_null_kfunc_ids,
> > + .check_set = &test_sk_check_kfunc_ids,
> > + .acquire_set = &test_sk_acquire_kfunc_ids,
> > + .release_set = &test_sk_release_kfunc_ids,
> > + .ret_null_set = &test_sk_ret_null_kfunc_ids,
> > + .kptr_acquire_set = &test_sk_kptr_acquire_kfunc_ids
> > };
>
> This hunk probably should have been in the previous patch,
> but since it's not affecting bisect I left it as-is.
>
> > +BTF_ID_LIST(bpf_prog_test_dtor_kfunc_ids)
> > +BTF_ID(struct, prog_test_ref_kfunc)
> > +BTF_ID(func, bpf_kfunc_call_test_release)
> > +BTF_ID(struct, prog_test_member)
> > +BTF_ID(func, bpf_kfunc_call_memb_release)
>
> dtor of prog_test_member doesn't seem to be used ?
>
It is necessary to be able to embed prog_test_member struct as a referenced kptr
in a map value, so it is still called by the map implementation's free path. But
since this is just used for verifier tests related to type matching when
non-zero offset is involved, both acquire and release are dummy functions.
> Please improve dtor and kptr_get test methods for
> struct prog_test_ref_kfunc and prog_test_member to do the real refcnting.
> Empty methods are not testing things fully.
I will leave out prog_test_member since it is not meant to be used at runtime,
but I will add refcount_t to prog_test_ref_kfunc, and include tests for it.
--
Kartikeya
