On Wed, Mar 5, 2025 at 5:57 PM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> On 3/6/2025 5:12 AM, Emil Tsalapatis wrote:
> > Add selftests for the bpf_cpumask_fill helper that sets a bpf_cpumask to
> > a bit pattern provided by a BPF program.
> >
> > Signed-off-by: Emil Tsalapatis (Meta) <emil@xxxxxxxxxxxxxxx>
> > ---
> > .../selftests/bpf/progs/cpumask_failure.c | 38 ++++++
> > .../selftests/bpf/progs/cpumask_success.c | 114 ++++++++++++++++++
> > 2 files changed, 152 insertions(+)
>
> My local build failed due to the missed declaration of
> "bpf_cpumask_populate" in cpumask_common.h. It reported the following error:
>
> progs/cpumask_success.c:788:8: error: call to undeclared function
> 'bpf_cpumask_populate'; ISO C99 and later do not support implicit fun
> ction declarations [-Wimplicit-function-declaration]
> 788 | ret = bpf_cpumask_populate((struct cpumask *)local,
> &toofewbits, sizeof(toofewbits));
>
> Don't know the reason why CI succeeded.
You need to upgrade pahole to make sure it emits kfuncs into vmlinux.h
>
> > diff --git a/tools/testing/selftests/bpf/progs/cpumask_failure.c b/tools/testing/selftests/bpf/progs/cpumask_failure.c
> > index b40b52548ffb..8a2fd596c8a3 100644
> > --- a/tools/testing/selftests/bpf/progs/cpumask_failure.c
> > +++ b/tools/testing/selftests/bpf/progs/cpumask_failure.c
> > @@ -222,3 +222,41 @@ int BPF_PROG(test_invalid_nested_array, struct task_struct *task, u64 clone_flag
> >
> > return 0;
> > }
> > +
> > +SEC("tp_btf/task_newtask")
> > +__failure __msg("type=scalar expected=fp")
> > +int BPF_PROG(test_populate_invalid_destination, struct task_struct *task, u64 clone_flags)
> > +{
> > + struct bpf_cpumask *invalid = (struct bpf_cpumask *)0x123456;
> > + u64 bits;
> > + int ret;
> > +
> > + ret = bpf_cpumask_populate((struct cpumask *)invalid, &bits, sizeof(bits));
> > + if (!ret)
> > + err = 2;
> > +
> > + return 0;
> > +}
> > +
> > +SEC("tp_btf/task_newtask")
> > +__failure __msg("leads to invalid memory access")
> > +int BPF_PROG(test_populate_invalid_source, struct task_struct *task, u64 clone_flags)
> > +{
> > + void *garbage = (void *)0x123456;
> > + struct bpf_cpumask *local;
> > + int ret;
> > +
> > + local = create_cpumask();
> > + if (!local) {
> > + err = 1;
> > + return 0;
> > + }
> > +
> > + ret = bpf_cpumask_populate((struct cpumask *)local, garbage, 8);
> > + if (!ret)
> > + err = 2;
> > +
> > + bpf_cpumask_release(local);
> > +
> > + return 0;
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/cpumask_success.c b/tools/testing/selftests/bpf/progs/cpumask_success.c
> > index 80ee469b0b60..5dc0fe9940dc 100644
> > --- a/tools/testing/selftests/bpf/progs/cpumask_success.c
> > +++ b/tools/testing/selftests/bpf/progs/cpumask_success.c
> > @@ -757,6 +757,7 @@ int BPF_PROG(test_refcount_null_tracking, struct task_struct *task, u64 clone_fl
> > mask1 = bpf_cpumask_create();
> > mask2 = bpf_cpumask_create();
> >
> > +
> > if (!mask1 || !mask2)
> > goto free_masks_return;
>
> An extra newline.
> >
> > @@ -770,3 +771,116 @@ int BPF_PROG(test_refcount_null_tracking, struct task_struct *task, u64 clone_fl
> > bpf_cpumask_release(mask2);
> > return 0;
> > }
> > +
> > +SEC("tp_btf/task_newtask")
> > +__success
>
> For tp_btf, bpf_prog_test_run() doesn't run the prog and it just returns
> directly, therefore, the prog below is not exercised at all. How about
> add test_populate_reject_small_mask into cpumask_success_testcases
> firstly, then switch these test cases to use __success() in a following
> patch ?
Good point. I'll revert and wait for respin.
