Date:   Wed, 19 Aug 2020 12:30:58 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v2 bpf-next 3/5] selftests/bpf: add CO-RE relo test for TYPE_ID_LOCAL/TYPE_ID_TARGET

On Wed, Aug 19, 2020 at 9:43 AM Yonghong Song <yhs@...com> wrote:
>
>
>
> On 8/18/20 10:28 PM, Andrii Nakryiko wrote:
> > Add tests for BTF type ID relocations. To allow testing this, enhance
> > core_relo.c test runner to allow dynamic initialization of test inputs.
> > If __builtin_btf_type_id() is not supported by Clang, skip tests.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@...com>
> > ---
> >   .../selftests/bpf/prog_tests/core_reloc.c     | 168 +++++++++++++++++-
> >   .../bpf/progs/btf__core_reloc_type_id.c       |   3 +
> >   ...tf__core_reloc_type_id___missing_targets.c |   3 +
> >   .../selftests/bpf/progs/core_reloc_types.h    |  40 +++++
> >   .../bpf/progs/test_core_reloc_type_based.c    |  14 --
> >   .../bpf/progs/test_core_reloc_type_id.c       | 107 +++++++++++
> >   6 files changed, 316 insertions(+), 19 deletions(-)
> >   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_id.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_id___missing_targets.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/test_core_reloc_type_id.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> > index b775ce0ede41..ad550510ef69 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> > @@ -3,6 +3,9 @@
> >   #include "progs/core_reloc_types.h"
> >   #include <sys/mman.h>
> >   #include <sys/syscall.h>
> > +#include <bpf/btf.h>
> > +
> > +static int duration = 0;
> >
> >   #define STRUCT_TO_CHAR_PTR(struct_name) (const char *)&(struct struct_name)
> >
> [...]
> > +
> > +typedef struct a_struct named_struct_typedef;
> > +
> > +typedef int (*func_proto_typedef)(long);
> > +
> > +typedef char arr_typedef[20];
> > +
> > +struct core_reloc_type_id_output {
> > +     int local_anon_struct;
> > +     int local_anon_union;
> > +     int local_anon_enum;
> > +     int local_anon_func_proto_ptr;
> > +     int local_anon_void_ptr;
> > +     int local_anon_arr;
> > +
> > +     int local_struct;
> > +     int local_union;
> > +     int local_enum;
> > +     int local_int;
> > +     int local_struct_typedef;
> > +     int local_func_proto_typedef;
> > +     int local_arr_typedef;
> > +
> > +     int targ_struct;
> > +     int targ_union;
> > +     int targ_enum;
> > +     int targ_int;
> > +     int targ_struct_typedef;
> > +     int targ_func_proto_typedef;
> > +     int targ_arr_typedef;
> > +};
> > +
> > +/* preserve types even if Clang doesn't support built-in */
> > +struct a_struct t1 = {};
> > +union a_union t2 = {};
> > +enum an_enum t3 = 0;
> > +named_struct_typedef t4 = {};
> > +func_proto_typedef t5 = 0;
> > +arr_typedef t6 = {};
> > +
> > +SEC("raw_tracepoint/sys_enter")
> > +int test_core_type_id(void *ctx)
> > +{
> > +#if __has_builtin(__builtin_btf_type_id)
>
> __builtin_btf_type_id is introduced in llvm11 but has issues for the
> following case:
>     - struct t { ... } is defined
>     - typedef struct t __t is defined
> both "struct t" and "__t" are used in __builtin_btf_type_id in the same
> function. This is a corner case but it will make the test failure with
> llvm11.
>
> I suggest to test builtin __builtin_preserve_type_info here with a
> comment to explain why. This will available test failure with llvm11.
>

ok, makes sense, will add comment and switch

>
> > +     struct core_reloc_type_id_output *out = (void *)&data.out;
> > +
> > +     out->local_anon_struct = bpf_core_type_id_local(struct { int marker_field; });
> > +     out->local_anon_union = bpf_core_type_id_local(union { int marker_field; });
> > +     out->local_anon_enum = bpf_core_type_id_local(enum { MARKER_ENUM_VAL = 123 });
> > +     out->local_anon_func_proto_ptr = bpf_core_type_id_local(_Bool(*)(int));
> > +     out->local_anon_void_ptr = bpf_core_type_id_local(void *);
> > +     out->local_anon_arr = bpf_core_type_id_local(_Bool[47]);
> > +
> > +     out->local_struct = bpf_core_type_id_local(struct a_struct);
> > +     out->local_union = bpf_core_type_id_local(union a_union);
> > +     out->local_enum = bpf_core_type_id_local(enum an_enum);
> > +     out->local_int = bpf_core_type_id_local(int);
> > +     out->local_struct_typedef = bpf_core_type_id_local(named_struct_typedef);
> > +     out->local_func_proto_typedef = bpf_core_type_id_local(func_proto_typedef);
> > +     out->local_arr_typedef = bpf_core_type_id_local(arr_typedef);
> > +
> > +     out->targ_struct = bpf_core_type_id_kernel(struct a_struct);
> > +     out->targ_union = bpf_core_type_id_kernel(union a_union);
> > +     out->targ_enum = bpf_core_type_id_kernel(enum an_enum);
> > +     out->targ_int = bpf_core_type_id_kernel(int);
> > +     out->targ_struct_typedef = bpf_core_type_id_kernel(named_struct_typedef);
> > +     out->targ_func_proto_typedef = bpf_core_type_id_kernel(func_proto_typedef);
> > +     out->targ_arr_typedef = bpf_core_type_id_kernel(arr_typedef);
> > +#else
> > +     data.skip = true;
> > +#endif
> > +
> > +     return 0;
> > +}
> > +
>
> empty line at the end of file?

Powered by blists - more mailing lists