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

On Thu, May 8, 2025 at 11:21 AM T.J. Mercier tjmercier@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@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); }

[...]