[PATCH bpf-next v4 5/5] selftests/bpf: Add test for open coded dmabuf_iter

[Song Liu]

On Thu, May 8, 2025 at 11:21 AM T.J. Mercier <tjmercier at google.com> wrote:
>
> Use the same test buffers as the traditional iterator and a new BPF map
> to verify the test buffers can be found with the open coded dmabuf
> iterator.

The way we split 4/5 and 5/5 makes the code tricker to follow. I guess
the motivation is to back port default iter along to older kernels. But I
think we can still make the code cleaner.

>
> Signed-off-by: T.J. Mercier <tjmercier at google.com>
> ---
[...]

>
> -static int create_udmabuf(void)
> +static int create_udmabuf(int map_fd)
>  {
>         struct udmabuf_create create;
>         int dev_udmabuf;
> +       bool f = false;
>
>         udmabuf_test_buffer_size = 10 * getpagesize();
>
> @@ -63,10 +64,10 @@ static int create_udmabuf(void)
>         if (!ASSERT_OK(ioctl(udmabuf, DMA_BUF_SET_NAME_B, udmabuf_test_buffer_name), "name"))
>                 return 1;
>
> -       return 0;
> +       return bpf_map_update_elem(map_fd, udmabuf_test_buffer_name, &f, BPF_ANY);

We don't really need this bpf_map_update_elem() inside
create_udmabuf(), right?

>  }
>
> -static int create_sys_heap_dmabuf(void)
> +static int create_sys_heap_dmabuf(int map_fd)
>  {
>         sysheap_test_buffer_size = 20 * getpagesize();
>
> @@ -77,6 +78,7 @@ static int create_sys_heap_dmabuf(void)
>                 .heap_flags = 0,
>         };
>         int heap_fd, ret;
> +       bool f = false;
>
>         if (!ASSERT_LE(sizeof(sysheap_test_buffer_name), DMA_BUF_NAME_LEN, "NAMETOOLONG"))
>                 return 1;
> @@ -95,18 +97,18 @@ static int create_sys_heap_dmabuf(void)
>         if (!ASSERT_OK(ioctl(sysheap_dmabuf, DMA_BUF_SET_NAME_B, sysheap_test_buffer_name), "name"))
>                 return 1;
>
> -       return 0;
> +       return bpf_map_update_elem(map_fd, sysheap_test_buffer_name, &f, BPF_ANY);

Same for this bpf_map_update_elem(), we can call this directly from
create_test_buffers().

>  }
>
> -static int create_test_buffers(void)
> +static int create_test_buffers(int map_fd)
>  {
>         int ret;
>
> -       ret = create_udmabuf();
> +       ret = create_udmabuf(map_fd);
>         if (ret)
>                 return ret;
>
> -       return create_sys_heap_dmabuf();
> +       return create_sys_heap_dmabuf(map_fd);

Personally, I would prefer we just merge all the logic of
create_udmabuf() and create_sys_heap_dmabuf()
into create_test_buffers().

>  }
>
[...]
> +
>  void test_dmabuf_iter(void)
>  {
>         struct dmabuf_iter *skel = NULL;
> +       int iter_fd, map_fd;
>         char buf[256];
> -       int iter_fd;
>
>         skel = dmabuf_iter__open_and_load();
>         if (!ASSERT_OK_PTR(skel, "dmabuf_iter__open_and_load"))
>                 return;
>
> -       if (!ASSERT_OK(create_test_buffers(), "create_buffers"))
> +       map_fd = bpf_map__fd(skel->maps.testbuf_hash);
> +       if (!ASSERT_OK_FD(map_fd, "map_fd"))
> +               goto destroy_skel;
> +
> +       if (!ASSERT_OK(create_test_buffers(map_fd), "create_buffers"))
>                 goto destroy;
>
>         if (!ASSERT_OK(dmabuf_iter__attach(skel), "skel_attach"))
> @@ -215,10 +246,13 @@ void test_dmabuf_iter(void)
>
>         if (test__start_subtest("default_iter"))
>                 subtest_dmabuf_iter_check_default_iter(skel);
> +       if (test__start_subtest("open_coded"))
> +               subtest_dmabuf_iter_check_open_coded(skel, map_fd);
>
>         close(iter_fd);
>
>  destroy:
>         destroy_test_buffers();
> +destroy_skel:
>         dmabuf_iter__destroy(skel);
>  }
[...]