Re: [PATCH bpf-next 3/3] selftests/bpf: Add unit tests for global functions

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Mon, Dec 14, 2020 at 11:53 AM Dmitrii Banshchikov <me@ubique.spb.ru> wrote:
>
> test_global_func9  - check valid scenarios for struct pointers
> test_global_func10 - check that the smaller struct cannot be passed as a
>                      the larger one
> test_global_func11 - check that CTX pointer cannot be passed as a struct
>                      pointer
> test_global_func12 - check access to a null pointer
> test_global_func13 - check access to an arbitrary pointer value
>
> Signed-off-by: Dmitrii Banshchikov <me@ubique.spb.ru>
> ---
>  .../bpf/prog_tests/test_global_funcs.c        |  5 ++
>  .../selftests/bpf/progs/test_global_func10.c  | 29 +++++++++
>  .../selftests/bpf/progs/test_global_func11.c  | 19 ++++++
>  .../selftests/bpf/progs/test_global_func12.c  | 21 +++++++
>  .../selftests/bpf/progs/test_global_func13.c  | 24 ++++++++
>  .../selftests/bpf/progs/test_global_func9.c   | 59 +++++++++++++++++++
>  6 files changed, 157 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_global_func10.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_global_func11.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_global_func12.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_global_func13.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_global_func9.c
>

[...]

> +
> +SEC("cgroup_skb/ingress")
> +int test_cls(struct __sk_buff *skb)
> +{
> +       int result = 0;
> +
> +       {
> +               const struct S s = {.x = skb->len };
> +
> +               result |= foo(&s);
> +       }
> +
> +       {
> +               const __u32 key = 1;
> +               const struct S *s = bpf_map_lookup_elem(&map, &key);
> +
> +               result |= foo(s);
> +       }

Can you please also add a test with passing a pointer to a global
variable as an input parameter?

Also, none of these tests seem to validate that correct data is read
and returned from foo. So it would be nice to have a dedicated
selftest (not part of test_global_func) that would pass some input
parameters (easiest to do with global variables) and see that the
subprogram returns it correctly.

> +
> +       {
> +               const struct C c = {.x = skb->len, .y = skb->family };
> +
> +               result |= foo((const struct S *)&c);
> +       }
> +
> +       {
> +               result |= foo(NULL);
> +       }
> +
> +       return result ? 1 : 0;
> +}
> --
> 2.25.1
>