Re: [RFC PATCH bpf-next 10/12] selftests/bpf: Add test for bpf_list_{front,back}

[Martin KaFai Lau <martin.lau@linux.dev>]

On 4/21/25 8:08 PM, Kumar Kartikeya Dwivedi wrote:
> On Sat, 19 Apr 2025 at 00:48, Martin KaFai Lau <martin.lau@linux.dev> wrote:
>>
>> From: Martin KaFai Lau <martin.lau@kernel.org>
>>
>> This patch adds a test for the new bpf_list_{front,back} kfunc.
>>
>> It also adds a test to ensure the non-owning node pointer cannot
>> be used after unlock.
>>
>> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
>> ---
>>   .../selftests/bpf/prog_tests/linked_list.c    |   2 +
>>   .../selftests/bpf/progs/linked_list_peek.c    | 104 ++++++++++++++++++
>>   2 files changed, 106 insertions(+)
>>   create mode 100644 tools/testing/selftests/bpf/progs/linked_list_peek.c
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/linked_list.c b/tools/testing/selftests/bpf/prog_tests/linked_list.c
>> index 77d07e0a4a55..559f45239a83 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/linked_list.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/linked_list.c
>> @@ -7,6 +7,7 @@
>>
>>   #include "linked_list.skel.h"
>>   #include "linked_list_fail.skel.h"
>> +#include "linked_list_peek.skel.h"
>>
>>   static char log_buf[1024 * 1024];
>>
>> @@ -804,4 +805,5 @@ void test_linked_list(void)
>>          test_linked_list_success(LIST_IN_LIST, false);
>>          test_linked_list_success(LIST_IN_LIST, true);
>>          test_linked_list_success(TEST_ALL, false);
>> +       RUN_TESTS(linked_list_peek);
>>   }
>> diff --git a/tools/testing/selftests/bpf/progs/linked_list_peek.c b/tools/testing/selftests/bpf/progs/linked_list_peek.c
>> new file mode 100644
>> index 000000000000..26c978091e5b
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/linked_list_peek.c
>> @@ -0,0 +1,104 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
>> +
>> +#include <vmlinux.h>
>> +#include <bpf/bpf_helpers.h>
>> +#include "bpf_misc.h"
>> +#include "bpf_experimental.h"
>> +
>> +struct node_data {
>> +       struct bpf_list_node l;
>> +       int key;
>> +};
>> +
>> +#define private(name) SEC(".data." #name) __hidden __attribute__((aligned(8)))
>> +private(A) struct bpf_spin_lock glock;
>> +private(A) struct bpf_list_head ghead __contains(node_data, l);
>> +
>> +#define list_entry(ptr, type, member) container_of(ptr, type, member)
>> +#define NR_NODES 32
>> +
>> +int zero = 0;
>> +
>> +SEC("syscall")
>> +__failure __msg("invalid mem access 'scalar'")
>> +long list_peek_unlock_scalar_node(void *ctx)
>> +{
>> +       struct bpf_list_node *l_n;
>> +       struct node_data *n;
>> +
>> +       bpf_spin_lock(&glock);
>> +       l_n = bpf_list_front(&ghead);
>> +       bpf_spin_unlock(&glock);
>> +
>> +       if (l_n) {
>> +               n = list_entry(l_n, struct node_data, l);
>> +               if (n->key == 0)
>> +                       return __LINE__;
>> +       }
>> +
>> +       return 0;
>> +}
> 
> Would be good to have tests explicitly asserting the type is
> non-owning ref (even though we indirectly do that by touching it after
> unlock, relying on invalidation logic.).

I will try to address the test suggestions in patch 6 and 10.

Thanks for the review!