Re: [PATCH bpf-next v2 20/20] tools/bpf: selftests: add bpf_iter selftests

[Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>]

On Sun, May 3, 2020 at 11:26 PM Yonghong Song <yhs@xxxxxx> wrote:
>
> The added test includes the following subtests:
>   - test verifier change for btf_id_or_null
>   - test load/create_iter/read for
>     ipv6_route/netlink/bpf_map/task/task_file
>   - test anon bpf iterator
>   - test anon bpf iterator reading one char at a time
>   - test file bpf iterator
>   - test overflow (single bpf program output not overflow)
>   - test overflow (single bpf program output overflows)
>
> Th ipv6_route tests the following verifier change
>   - access fields in the variable length array of the structure.
>
> The netlink load tests th following verifier change
>   - put a btf_id ptr value in a stack and accessible to
>     tracing/iter programs.
>
>   $ test_progs -n 2
>   #2/1 btf_id_or_null:OK
>   #2/2 ipv6_route:OK
>   #2/3 netlink:OK
>   #2/4 bpf_map:OK
>   #2/5 task:OK
>   #2/6 task_file:OK
>   #2/7 anon:OK
>   #2/8 anon-read-one-char:OK
>   #2/9 file:OK
>   #2/10 overflow:OK
>   #2/11 overflow-e2big:OK
>   #2 bpf_iter:OK
>   Summary: 1/11 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
> ---

Looks good overall. bpf_link__disconnect() is wrong, though, please
remove it. With that:

Acked-by: Andrii Nakryiko <andriin@xxxxxx>

>  .../selftests/bpf/prog_tests/bpf_iter.c       | 390 ++++++++++++++++++
>  .../selftests/bpf/progs/bpf_iter_test_kern1.c |   4 +
>  .../selftests/bpf/progs/bpf_iter_test_kern2.c |   4 +
>  .../selftests/bpf/progs/bpf_iter_test_kern3.c |  18 +
>  .../selftests/bpf/progs/bpf_iter_test_kern4.c |  48 +++
>  .../bpf/progs/bpf_iter_test_kern_common.h     |  22 +
>  6 files changed, 486 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_iter.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern1.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern2.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern3.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern_common.h
>

[...]

> +
> +free_link:
> +       bpf_link__disconnect(link);

bpf_link__disconnect() actually will make destroy() below not close
link. So no need for it. Same below in few places.

> +       bpf_link__destroy(link);
> +}
> +

[...]