Re: [RFC PATCH v1 10/10] selftests/bpf: add file dynptr tests

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 10/3/25 21:02, Andrii Nakryiko wrote:
> On Fri, Oct 3, 2025 at 9:04 AM Mykyta Yatsenko
> <mykyta.yatsenko5@gmail.com> wrote:
>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>
>> Introducing selftests for validating file-backed dynptr works as
>> expected.
>>   * validate implementation supports dynptr slice and read operations
>>   * validate destructors should be paired with initializers
>>
>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>> ---
>>   .../selftests/bpf/prog_tests/file_reader.c    |  81 ++++++
>>   .../testing/selftests/bpf/progs/file_reader.c | 241 ++++++++++++++++++
>>   .../selftests/bpf/progs/file_reader_fail.c    |  57 +++++
>>   3 files changed, 379 insertions(+)
>>   create mode 100644 tools/testing/selftests/bpf/prog_tests/file_reader.c
>>   create mode 100644 tools/testing/selftests/bpf/progs/file_reader.c
>>   create mode 100644 tools/testing/selftests/bpf/progs/file_reader_fail.c
> Non-sleepable file dynptr can fail to read, so this test is a bit
> fragile. Let's have a sleepable test (fentry.s or something like
> that?)
This is sleepable, because it runs in task work callback.
>
> Plus, can you please add a test that validates that we do page in file
> contents even if it was not physically in memory? see madvise(addr,
> page_sz, MADV_PAGEOUT) in selftests
I tried to tell kernel to evict those pages via: posix_fadvise(fd, 0, 0, 
POSIX_FADV_DONTNEED);
I'll rework to madvise(addr, page_sz, MADV_PAGEOUT)
>
>> +int err;
>> +void *user_ptr;
>> +char buf[1024];
>> +char *user_buf;
>> +volatile const __u32 user_buf_sz;
>> +volatile const __s32 test_type = -1;
>> +
>> +static int process_vma(struct task_struct *task, struct vm_area_struct *vma, void *data);
>> +static int search_elf(struct file *file);
>> +static int validate_large_file_read(struct file *file);
>> +static int task_work_callback(struct bpf_map *map, void *key, void *value);
>> +
>> +SEC("raw_tp/sys_enter")
>> +int on_getpid(void *ctx)
>> +{
>> +       struct task_struct *task = bpf_get_current_task_btf();
>> +       struct elem *work;
>> +       int key = 0;
> this will be called for every syscall in the system, regardless of the
> process, so you probably need to filter this by process ID?
>
>> +
>> +       work = bpf_map_lookup_elem(&arrmap, &key);
>> +       if (!work) {
>> +               err = 1;
>> +               return 0;
>> +       }
>> +       bpf_task_work_schedule_signal(task, &work->tw, &arrmap, task_work_callback, NULL);
>> +       return 0;
>> +}
>> +
> [...]
>
>> +static long process_vma_unreleased_ref(struct task_struct *task, struct vm_area_struct *vma,
>> +                                      void *data)
>> +{
>> +       struct bpf_dynptr dynptr;
>> +
>> +       if (!vma->vm_file)
>> +               return 1;
>> +
>> +       err = bpf_dynptr_from_file(vma->vm_file, 0, &dynptr);
>> +       return err ? 1 : 0;
>> +}
>> +
>> +SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
>> +__failure __msg("Unreleased reference id=") int on_nanosleep_unreleased_ref(void *ctx)
> nit: keep annotations on separate line from the function itself
>
>> +{
>> +       struct task_struct *task = bpf_get_current_task_btf();
>> +
>> +       bpf_find_vma(task, (unsigned long)user_ptr, process_vma_unreleased_ref, NULL, 0);
>> +       return 0;
>> +}
>> +
>> +SEC("xdp")
>> +__failure __msg("Expected a dynptr of type file as arg #0")
>> +int xdp_wrong_dynptr_type(struct xdp_md *xdp)
>> +{
>> +       struct bpf_dynptr dynptr;
>> +
>> +       bpf_dynptr_from_xdp(xdp, 0, &dynptr);
>> +       bpf_dynptr_file_discard(&dynptr);
>> +       return 0;
>> +}
>> +
>> +SEC("xdp")
>> +__failure __msg("Expected an initialized dynptr as arg #0")
>> +int xdp_no_dynptr_type(struct xdp_md *xdp)
>> +{
>> +       struct bpf_dynptr dynptr;
>> +
>> +       bpf_dynptr_file_discard(&dynptr);
>> +       return 0;
>> +}
>> --
>> 2.51.0
>>