From: Kui-Feng Lee <sinquersw@gmail.com>
To: Eduard Zingerman <eddyz87@gmail.com>,
Kui-Feng Lee <thinker.li@gmail.com>,
bpf@vger.kernel.org, ast@kernel.org, martin.lau@linux.dev,
song@kernel.org, kernel-team@meta.com, andrii@kernel.org
Cc: kuifeng@meta.com
Subject: Re: [PATCH bpf-next v5 7/9] selftests/bpf: Test kptr arrays and kptrs in nested struct fields.
Date: Fri, 10 May 2024 14:59:38 -0700 [thread overview]
Message-ID: <d2b9a943-ca26-404d-899a-c7651ce18a42@gmail.com> (raw)
In-Reply-To: <d8f2fa21a9af5bfcb2acb1addecea435285c40e6.camel@gmail.com>
On 5/10/24 03:03, Eduard Zingerman wrote:
> On Thu, 2024-05-09 at 18:13 -0700, Kui-Feng Lee wrote:
>> Make sure that BPF programs can declare global kptr arrays and kptr fields
>> in struct types that is the type of a global variable or the type of a
>> nested descendant field in a global variable.
>>
>> An array with only one element is special case, that it treats the element
>> like a non-array kptr field. Nested arrays are also tested to ensure they
>> are handled properly.
>>
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>> ---
>> .../selftests/bpf/prog_tests/cpumask.c | 5 +
>> .../selftests/bpf/progs/cpumask_success.c | 133 ++++++++++++++++++
>> 2 files changed, 138 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/cpumask.c b/tools/testing/selftests/bpf/prog_tests/cpumask.c
>> index ecf89df78109..2570bd4b0cb2 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/cpumask.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/cpumask.c
>> @@ -18,6 +18,11 @@ static const char * const cpumask_success_testcases[] = {
>> "test_insert_leave",
>> "test_insert_remove_release",
>> "test_global_mask_rcu",
>> + "test_global_mask_array_one_rcu",
>> + "test_global_mask_array_rcu",
>> + "test_global_mask_array_l2_rcu",
>> + "test_global_mask_nested_rcu",
>> + "test_global_mask_nested_deep_rcu",
>> "test_cpumask_weight",
>> };
>>
>> diff --git a/tools/testing/selftests/bpf/progs/cpumask_success.c b/tools/testing/selftests/bpf/progs/cpumask_success.c
>> index 7a1e64c6c065..0b6383fa9958 100644
>> --- a/tools/testing/selftests/bpf/progs/cpumask_success.c
>> +++ b/tools/testing/selftests/bpf/progs/cpumask_success.c
>> @@ -12,6 +12,25 @@ char _license[] SEC("license") = "GPL";
>>
>> int pid, nr_cpus;
>>
>> +struct kptr_nested {
>> + struct bpf_cpumask __kptr * mask;
>> +};
>> +
>> +struct kptr_nested_mid {
>> + int dummy;
>> + struct kptr_nested m;
>> +};
>> +
>> +struct kptr_nested_deep {
>> + struct kptr_nested_mid ptrs[2];
>> +};
>
> For the sake of completeness, would it be possible to create a test
> case where there are several struct arrays following each other?
> E.g. as below:
>
> struct foo {
> ... __kptr *a;
> ... __kptr *b;
> }
>
> struct bar {
> ... __kptr *c;
> }
>
> struct {
> struct foo foos[3];
> struct bar bars[2];
> }
>
> Just to check that offset is propagated correctly.
Sure!
>
> Also, in the tests below you check that a pointer to some object could
> be put into an array at different indexes. Tbh, I find it not very
> interesting if we want to check that offsets are correct.
> Would it be possible to create an array of object kptrs,
> put specific references at specific indexes and somehow check which
> object ended up where? (not necessarily 'bpf_cpumask').
Do you mean checking index in the way like the following code?
if (array[0] != ref0 || array[1] != ref1 || array[2] != ref2 ....)
return err;
>
>> +
>> +private(MASK) static struct bpf_cpumask __kptr * global_mask_array[2];
>> +private(MASK) static struct bpf_cpumask __kptr * global_mask_array_l2[2][1];
>> +private(MASK) static struct bpf_cpumask __kptr * global_mask_array_one[1];
>> +private(MASK) static struct kptr_nested global_mask_nested[2];
>> +private(MASK) static struct kptr_nested_deep global_mask_nested_deep;
>> +
>> static bool is_test_task(void)
>> {
>> int cur_pid = bpf_get_current_pid_tgid() >> 32;
>> @@ -460,6 +479,120 @@ int BPF_PROG(test_global_mask_rcu, struct task_struct *task, u64 clone_flags)
>> return 0;
>> }
>>
>> +SEC("tp_btf/task_newtask")
>> +int BPF_PROG(test_global_mask_array_one_rcu, struct task_struct *task, u64 clone_flags)
>> +{
>> + struct bpf_cpumask *local, *prev;
>> +
>> + if (!is_test_task())
>> + return 0;
>> +
>> + /* Kptr arrays with one element are special cased, being treated
>> + * just like a single pointer.
>> + */
>> +
>> + local = create_cpumask();
>> + if (!local)
>> + return 0;
>> +
>> + prev = bpf_kptr_xchg(&global_mask_array_one[0], local);
>> + if (prev) {
>> + bpf_cpumask_release(prev);
>> + err = 3;
>> + return 0;
>> + }
>> +
>> + bpf_rcu_read_lock();
>> + local = global_mask_array_one[0];
>> + if (!local) {
>> + err = 4;
>> + bpf_rcu_read_unlock();
>> + return 0;
>> + }
>> +
>> + bpf_rcu_read_unlock();
>> +
>> + return 0;
>> +}
>> +
>> +static int _global_mask_array_rcu(struct bpf_cpumask **mask0,
>> + struct bpf_cpumask **mask1)
>> +{
>> + struct bpf_cpumask *local;
>> +
>> + if (!is_test_task())
>> + return 0;
>> +
>> + /* Check if two kptrs in the array work and independently */
>> +
>> + local = create_cpumask();
>> + if (!local)
>> + return 0;
>> +
>> + bpf_rcu_read_lock();
>> +
>> + local = bpf_kptr_xchg(mask0, local);
>> + if (local) {
>> + err = 1;
>> + goto err_exit;
>> + }
>> +
>> + /* [<mask 0>, NULL] */
>> + if (!*mask0 || *mask1) {
>> + err = 2;
>> + goto err_exit;
>> + }
>> +
>> + local = create_cpumask();
>> + if (!local) {
>> + err = 9;
>> + goto err_exit;
>> + }
>> +
>> + local = bpf_kptr_xchg(mask1, local);
>> + if (local) {
>> + err = 10;
>> + goto err_exit;
>> + }
>> +
>> + /* [<mask 0>, <mask 1>] */
>> + if (!*mask0 || !*mask1 || *mask0 == *mask1) {
>> + err = 11;
>> + goto err_exit;
>> + }
>> +
>> +err_exit:
>> + if (local)
>> + bpf_cpumask_release(local);
>> + bpf_rcu_read_unlock();
>> + return 0;
>> +}
>> +
>> +SEC("tp_btf/task_newtask")
>> +int BPF_PROG(test_global_mask_array_rcu, struct task_struct *task, u64 clone_flags)
>> +{
>> + return _global_mask_array_rcu(&global_mask_array[0], &global_mask_array[1]);
>> +}
>> +
>> +SEC("tp_btf/task_newtask")
>> +int BPF_PROG(test_global_mask_array_l2_rcu, struct task_struct *task, u64 clone_flags)
>> +{
>> + return _global_mask_array_rcu(&global_mask_array_l2[0][0], &global_mask_array_l2[1][0]);
>> +}
>> +
>> +SEC("tp_btf/task_newtask")
>> +int BPF_PROG(test_global_mask_nested_rcu, struct task_struct *task, u64 clone_flags)
>> +{
>> + return _global_mask_array_rcu(&global_mask_nested[0].mask, &global_mask_nested[1].mask);
>> +}
>> +
>> +SEC("tp_btf/task_newtask")
>> +int BPF_PROG(test_global_mask_nested_deep_rcu, struct task_struct *task, u64 clone_flags)
>> +{
>> + return _global_mask_array_rcu(&global_mask_nested_deep.ptrs[0].m.mask,
>> + &global_mask_nested_deep.ptrs[1].m.mask);
>> +}
>> +
>> SEC("tp_btf/task_newtask")
>> int BPF_PROG(test_cpumask_weight, struct task_struct *task, u64 clone_flags)
>> {
>
next prev parent reply other threads:[~2024-05-10 21:59 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 1:13 [PATCH bpf-next v5 0/9] Enable BPF programs to declare arrays of kptr, bpf_rb_root, and bpf_list_head Kui-Feng Lee
2024-05-10 1:13 ` [PATCH bpf-next v5 1/9] bpf: Remove unnecessary checks on the offset of btf_field Kui-Feng Lee
2024-05-10 1:13 ` [PATCH bpf-next v5 2/9] bpf: Remove unnecessary call to btf_field_type_size() Kui-Feng Lee
2024-05-10 1:13 ` [PATCH bpf-next v5 3/9] bpf: refactor btf_find_struct_field() and btf_find_datasec_var() Kui-Feng Lee
2024-05-10 1:13 ` [PATCH bpf-next v5 4/9] bpf: create repeated fields for arrays Kui-Feng Lee
2024-05-10 1:13 ` [PATCH bpf-next v5 5/9] bpf: look into the types of the fields of a struct type recursively Kui-Feng Lee
2024-05-10 1:13 ` [PATCH bpf-next v5 6/9] bpf: limit the number of levels of a nested struct type Kui-Feng Lee
2024-05-10 2:37 ` Eduard Zingerman
2024-05-10 1:13 ` [PATCH bpf-next v5 7/9] selftests/bpf: Test kptr arrays and kptrs in nested struct fields Kui-Feng Lee
2024-05-10 10:03 ` Eduard Zingerman
2024-05-10 21:59 ` Kui-Feng Lee [this message]
2024-05-10 22:08 ` Eduard Zingerman
2024-05-10 22:25 ` Kui-Feng Lee
2024-05-10 22:31 ` Eduard Zingerman
2024-05-10 22:53 ` Kui-Feng Lee
2024-05-10 22:57 ` Eduard Zingerman
2024-05-10 23:04 ` Kui-Feng Lee
2024-05-10 23:17 ` Eduard Zingerman
2024-05-10 23:29 ` Eduard Zingerman
2024-05-20 15:55 ` Kui-Feng Lee
2024-05-10 1:13 ` [PATCH bpf-next v5 8/9] selftests/bpf: Test global bpf_rb_root arrays and fields in nested struct types Kui-Feng Lee
2024-05-10 1:13 ` [PATCH bpf-next v5 9/9] selftests/bpf: Test global bpf_list_head arrays Kui-Feng Lee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d2b9a943-ca26-404d-899a-c7651ce18a42@gmail.com \
--to=sinquersw@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=eddyz87@gmail.com \
--cc=kernel-team@meta.com \
--cc=kuifeng@meta.com \
--cc=martin.lau@linux.dev \
--cc=song@kernel.org \
--cc=thinker.li@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).