On Fri, Feb 23, 2024 at 1:36 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Sun, Feb 18, 2024 at 3:49 AM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
> >
> > Add selftests for the newly added bits iter.
> > - bits_iter_success
> > - The number of CPUs should be expected when iterating over a cpumask
> > - percpu data extracted from the percpu struct should be expected
> > - RCU lock is not required
> > - It is fine without calling bpf_iter_cpumask_next()
> > - It can work as expected when invalid arguments are passed
> >
> > - bits_iter_failure
> > - bpf_iter_bits_destroy() is required after calling
> > bpf_iter_bits_new()
> > - bpf_iter_bits_destroy() can only destroy an initialized iter
> > - bpf_iter_bits_next() must use an initialized iter
> >
> > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> > ---
> > tools/testing/selftests/bpf/config | 1 +
> > .../selftests/bpf/prog_tests/bits_iter.c | 180 ++++++++++++++++++
> > .../bpf/progs/test_bits_iter_failure.c | 53 ++++++
> > .../bpf/progs/test_bits_iter_success.c | 146 ++++++++++++++
> > 4 files changed, 380 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/bits_iter.c
> > create mode 100644 tools/testing/selftests/bpf/progs/test_bits_iter_failure.c
> > create mode 100644 tools/testing/selftests/bpf/progs/test_bits_iter_success.c
> >
> > diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> > index 01f241ea2c67..dd4b0935e35f 100644
> > --- a/tools/testing/selftests/bpf/config
> > +++ b/tools/testing/selftests/bpf/config
> > @@ -78,6 +78,7 @@ CONFIG_NF_CONNTRACK_MARK=y
> > CONFIG_NF_DEFRAG_IPV4=y
> > CONFIG_NF_DEFRAG_IPV6=y
> > CONFIG_NF_NAT=y
> > +CONFIG_PSI=y
> > CONFIG_RC_CORE=y
> > CONFIG_SECURITY=y
> > CONFIG_SECURITYFS=y
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bits_iter.c b/tools/testing/selftests/bpf/prog_tests/bits_iter.c
> > new file mode 100644
> > index 000000000000..778a7c942dba
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/bits_iter.c
> > @@ -0,0 +1,180 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2024 Yafang Shao <laoar.shao@xxxxxxxxx> */
> > +
> > +#define _GNU_SOURCE
> > +#include <sched.h>
> > +
> > +#include <test_progs.h>
> > +#include "test_bits_iter_success.skel.h"
> > +#include "test_bits_iter_failure.skel.h"
> > +#include "cgroup_helpers.h"
> > +
> > +static const char * const positive_testcases[] = {
> > + "cpumask_iter",
> > +};
> > +
> > +static const char * const negative_testcases[] = {
> > + "null_pointer",
> > + "zero_bit",
> > + "no_mem",
> > + "invalid_bits"
> > +};
> > +
> > +static int read_percpu_data(struct bpf_link *link, int nr_cpu_exp, int nr_running_exp)
> > +{
> > + int iter_fd, len, item, nr_running, psi_running, nr_cpus, err = -1;
> > + char buf[128];
> > + size_t left;
> > + char *p;
> > +
> > + iter_fd = bpf_iter_create(bpf_link__fd(link));
> > + if (!ASSERT_GE(iter_fd, 0, "iter_fd"))
> > + return -1;
> > +
> > + memset(buf, 0, sizeof(buf));
> > + left = ARRAY_SIZE(buf);
> > + p = buf;
> > + while ((len = read(iter_fd, p, left)) > 0) {
> > + p += len;
> > + left -= len;
> > + }
> > +
> > + item = sscanf(buf, "nr_running %u nr_cpus %u psi_running %u\n",
> > + &nr_running, &nr_cpus, &psi_running);
> > + if (!ASSERT_EQ(item, 3, "seq_format"))
> > + goto out;
> > + if (!ASSERT_EQ(nr_cpus, nr_cpu_exp, "nr_cpus"))
> > + goto out;
> > + if (!ASSERT_GE(nr_running, nr_running_exp, "nr_running"))
> > + goto out;
> > + if (!ASSERT_GE(psi_running, nr_running_exp, "psi_running"))
> > + goto out;
> > +
> > + err = 0;
> > +out:
> > + close(iter_fd);
> > + return err;
> > +}
>
> ..
> > +
> > + /* Case 1): Enable all possible CPUs */
> > + CPU_ZERO(&set);
> > + for (i = 0; i < nr_cpus; i++)
> > + CPU_SET(i, &set);
> > + err = sched_setaffinity(skel->bss->pid, sizeof(set), &set);
> > + if (!ASSERT_OK(err, "setaffinity_all_cpus"))
> > + goto free_link;
> > + err = read_percpu_data(link, nr_cpus, 1);
> > + if (!ASSERT_OK(err, "read_percpu_data"))
> > + goto free_link;
>
> The patch 1 looks good, but this test fails on s390.
>
> read_percpu_data:FAIL:nr_cpus unexpected nr_cpus: actual 0 != expected 2
> verify_iter_success:FAIL:read_percpu_data unexpected error: -1 (errno 95)
>
> Please see CI.
>
> So either add it to DENYLIST.s390x in the same commit or make it work.
>
> pw-bot: cr
The reason for the failure on s390x architecture is currently unclear.
One plausible explanation is that total_nr_cpus remains 0 when
executing the following code:
bpf_for_each(bits, cpu, p->cpus_ptr, total_nr_cpus)
This is despite setting total_nr_cpus to the value obtained from
libbpf_num_possible_cpus():
skel->bss->total_nr_cpus = libbpf_num_possible_cpus();
A potential workaround could involve using a hardcoded number of CPUs,
such as 8192, instead of relying on total_nr_cpus. This approach might
mitigate the issue temporarily.
--
Regards
Yafang
