[PATCH v4 0/3] bpf: introduce helper for populating bpf_cpumask

[PATCH v4 0/3] bpf: introduce helper for populating bpf_cpumask

[Emil Tsalapatis]

Some BPF programs like scx schedulers have their own internal CPU mask types, 
mask types, which they must transform into struct bpf_cpumask instances
before passing them to scheduling-related kfuncs. There is currently no
way to efficiently populate the bitfield of a bpf_cpumask from BPF memory, 
and programs must use multiple bpf_cpumask_[set, clear] calls to do so. 
Introduce a kfunc helper to populate the bitfield of a bpf_cpumask from valid 
BPF memory with a single call.

Changelog :
-----------
v3->v4
v3: https://lore.kernel.org/bpf/20250305161327.203396-1-emil@etsalapatis.com/

	* Removed new tests from tools/selftests/bpf/prog_tests/cpumask.c because
they were being run twice.

Addressed feedback by Alexei Starovoitov:
	* Added missing return value in function kdoc
	* Added an additional patch fixing some missing kdoc fields in
	kernel/bpf/cpumask.c

Addressed feedback by Tejun Heo:
	* Renamed the kfunc to bpf_cpumask_populate to avoid confusion
	w/ bitmap_fill()

v2->v3
v2: https://lore.kernel.org/bpf/20250305021020.1004858-1-emil@etsalapatis.com/

Addressed feedback by Alexei Starovoitov:
	* Added back patch descriptions dropped from v1->v2
	* Elide the alignment check for archs with efficient
	  unaligned accesses

v1->v2
v1: https://lore.kernel.org/bpf/20250228003321.1409285-1-emil@etsalapatis.com/

Addressed feedback by Hou Tao:
	* Add check that the input buffer is aligned to sizeof(long)
	* Adjust input buffer size check to use bitmap_size()
	* Add selftest for checking the bit pattern of the bpf_cpumask
	* Moved all selftests into existing files

Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>

Emil Tsalapatis (3):
  bpf: add kfunc for populating cpumask bits
  selftests: bpf: add bpf_cpumask_fill selftests
  bpf: fix missing kdoc string fields in cpumask.c

 kernel/bpf/cpumask.c                          |  53 ++++++++
 .../selftests/bpf/progs/cpumask_failure.c     |  38 ++++++
 .../selftests/bpf/progs/cpumask_success.c     | 114 ++++++++++++++++++
 3 files changed, 205 insertions(+)

-- 
2.47.1

[PATCH v4 1/3] bpf: add kfunc for populating cpumask bits

[Emil Tsalapatis]

Add a helper kfunc that sets the bitmap of a bpf_cpumask from BPF memory.

Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
---
 kernel/bpf/cpumask.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
index cfa1c18e3a48..14080ca694b0 100644
--- a/kernel/bpf/cpumask.c
+++ b/kernel/bpf/cpumask.c
@@ -420,6 +420,38 @@ __bpf_kfunc u32 bpf_cpumask_weight(const struct cpumask *cpumask)
 	return cpumask_weight(cpumask);
 }
 
+/**
+ * bpf_cpumask_populate() - Populate the CPU mask from the contents of
+ * a BPF memory region.
+ *
+ * @cpumask: The cpumask being populated.
+ * @src: The BPF memory holding the bit pattern.
+ * @src__sz: Length of the BPF memory region in bytes.
+ *
+ * Return:
+ * * 0 if the struct cpumask * instance was populated successfully.
+ * * -EACCES if the memory region is too small to populate the cpumask.
+ * * -EINVAL if the memory region is not aligned to the size of a long
+ *   and the architecture does not support efficient unaligned accesses.
+ */
+__bpf_kfunc int bpf_cpumask_populate(struct cpumask *cpumask, void *src, size_t src__sz)
+{
+	unsigned long source = (unsigned long)src;
+
+	/* The memory region must be large enough to populate the entire CPU mask. */
+	if (src__sz < bitmap_size(nr_cpu_ids))
+		return -EACCES;
+
+	/* If avoiding unaligned accesses, the input region must be aligned to the nearest long. */
+	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
+		!IS_ALIGNED(source, sizeof(long)))
+		return -EINVAL;
+
+	bitmap_copy(cpumask_bits(cpumask), src, nr_cpu_ids);
+
+	return 0;
+}
+
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(cpumask_kfunc_btf_ids)
@@ -448,6 +480,7 @@ BTF_ID_FLAGS(func, bpf_cpumask_copy, KF_RCU)
 BTF_ID_FLAGS(func, bpf_cpumask_any_distribute, KF_RCU)
 BTF_ID_FLAGS(func, bpf_cpumask_any_and_distribute, KF_RCU)
 BTF_ID_FLAGS(func, bpf_cpumask_weight, KF_RCU)
+BTF_ID_FLAGS(func, bpf_cpumask_populate, KF_RCU)
 BTF_KFUNCS_END(cpumask_kfunc_btf_ids)
 
 static const struct btf_kfunc_id_set cpumask_kfunc_set = {
-- 
2.47.1

[PATCH v4 2/3] selftests: bpf: add bpf_cpumask_fill selftests

[Emil Tsalapatis]

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@etsalapatis.com>
---
 .../selftests/bpf/progs/cpumask_failure.c     |  38 ++++++
 .../selftests/bpf/progs/cpumask_success.c     | 114 ++++++++++++++++++
 2 files changed, 152 insertions(+)

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;
 
@@ -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
+int BPF_PROG(test_populate_reject_small_mask, struct task_struct *task, u64 clone_flags)
+{
+	struct bpf_cpumask *local;
+	u8 toofewbits;
+	int ret;
+
+	local = create_cpumask();
+	if (!local)
+		return 0;
+
+	/* The kfunc should prevent this operation */
+	ret = bpf_cpumask_populate((struct cpumask *)local, &toofewbits, sizeof(toofewbits));
+	if (ret != -EACCES)
+		err = 2;
+
+	bpf_cpumask_release(local);
+
+	return 0;
+}
+
+/* Mask is guaranteed to be large enough for bpf_cpumask_t. */
+#define CPUMASK_TEST_MASKLEN (sizeof(cpumask_t))
+
+/* Add an extra word for the test_populate_reject_unaligned test. */
+u64 bits[CPUMASK_TEST_MASKLEN / 8 + 1];
+extern bool CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS __kconfig __weak;
+
+SEC("tp_btf/task_newtask")
+__success
+int BPF_PROG(test_populate_reject_unaligned, struct task_struct *task, u64 clone_flags)
+{
+	struct bpf_cpumask *mask;
+	char *src;
+	int ret;
+
+	/* Skip if unaligned accesses are fine for this arch.  */
+	if (CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+		return 0;
+
+	mask = bpf_cpumask_create();
+	if (!mask) {
+		err = 1;
+		return 0;
+	}
+
+	/* Misalign the source array by a byte. */
+	src = &((char *)bits)[1];
+
+	ret = bpf_cpumask_populate((struct cpumask *)mask, src, CPUMASK_TEST_MASKLEN);
+	if (ret != -EINVAL)
+		err = 2;
+
+	bpf_cpumask_release(mask);
+
+	return 0;
+}
+
+
+SEC("tp_btf/task_newtask")
+__success
+int BPF_PROG(test_populate, struct task_struct *task, u64 clone_flags)
+{
+	struct bpf_cpumask *mask;
+	bool bit;
+	int ret;
+	int i;
+
+	/* Set only odd bits. */
+	__builtin_memset(bits, 0xaa, CPUMASK_TEST_MASKLEN);
+
+	mask = bpf_cpumask_create();
+	if (!mask) {
+		err = 1;
+		return 0;
+	}
+
+	/* Pass the entire bits array, the kfunc will only copy the valid bits. */
+	ret = bpf_cpumask_populate((struct cpumask *)mask, bits, CPUMASK_TEST_MASKLEN);
+	if (ret) {
+		err = 2;
+		goto out;
+	}
+
+	/*
+	 * Test is there to appease the verifier. We cannot directly
+	 * access NR_CPUS, the upper bound for nr_cpus, so we infer
+	 * it from the size of cpumask_t.
+	 */
+	if (nr_cpus < 0 || nr_cpus >= CPUMASK_TEST_MASKLEN * 8) {
+		err = 3;
+		goto out;
+	}
+
+	bpf_for(i, 0, nr_cpus) {
+		/* Odd-numbered bits should be set, even ones unset. */
+		bit = bpf_cpumask_test_cpu(i, (const struct cpumask *)mask);
+		if (bit == (i % 2 != 0))
+			continue;
+
+		err = 4;
+		break;
+	}
+
+out:
+	bpf_cpumask_release(mask);
+
+	return 0;
+}
+
+#undef CPUMASK_TEST_MASKLEN
-- 
2.47.1

[PATCH v4 3/3] bpf: fix missing kdoc string fields in cpumask.c

[Emil Tsalapatis]

Some bpf_cpumask-related kfuncs have kdoc strings that are missing
return values. Add a the missing descriptions for the return values.

Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
---
 kernel/bpf/cpumask.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
index 14080ca694b0..c6c1ec6d2b2c 100644
--- a/kernel/bpf/cpumask.c
+++ b/kernel/bpf/cpumask.c
@@ -45,6 +45,10 @@ __bpf_kfunc_start_defs();
  *
  * bpf_cpumask_create() allocates memory using the BPF memory allocator, and
  * will not block. It may return NULL if no memory is available.
+ *
+ * Return:
+ * * A pointer to a new struct bpf_cpumask instance on success.
+ * * NULL if the BPF memory allocator is out of memory.
  */
 __bpf_kfunc struct bpf_cpumask *bpf_cpumask_create(void)
 {
@@ -71,6 +75,10 @@ __bpf_kfunc struct bpf_cpumask *bpf_cpumask_create(void)
  * Acquires a reference to a BPF cpumask. The cpumask returned by this function
  * must either be embedded in a map as a kptr, or freed with
  * bpf_cpumask_release().
+ *
+ * Return:
+ * * The struct bpf_cpumask pointer passed to the function.
+ *
  */
 __bpf_kfunc struct bpf_cpumask *bpf_cpumask_acquire(struct bpf_cpumask *cpumask)
 {
@@ -106,6 +114,9 @@ CFI_NOSEAL(bpf_cpumask_release_dtor);
  *
  * Find the index of the first nonzero bit of the cpumask. A struct bpf_cpumask
  * pointer may be safely passed to this function.
+ *
+ * Return:
+ * * The index of the first nonzero bit in the struct cpumask.
  */
 __bpf_kfunc u32 bpf_cpumask_first(const struct cpumask *cpumask)
 {
@@ -119,6 +130,9 @@ __bpf_kfunc u32 bpf_cpumask_first(const struct cpumask *cpumask)
  *
  * Find the index of the first unset bit of the cpumask. A struct bpf_cpumask
  * pointer may be safely passed to this function.
+ *
+ * Return:
+ * * The index of the first zero bit in the struct cpumask.
  */
 __bpf_kfunc u32 bpf_cpumask_first_zero(const struct cpumask *cpumask)
 {
@@ -133,6 +147,9 @@ __bpf_kfunc u32 bpf_cpumask_first_zero(const struct cpumask *cpumask)
  *
  * Find the index of the first nonzero bit of the AND of two cpumasks.
  * struct bpf_cpumask pointers may be safely passed to @src1 and @src2.
+ *
+ * Return:
+ * * The index of the first bit that is nonzero in both cpumask instances.
  */
 __bpf_kfunc u32 bpf_cpumask_first_and(const struct cpumask *src1,
 				      const struct cpumask *src2)
@@ -414,6 +431,9 @@ __bpf_kfunc u32 bpf_cpumask_any_and_distribute(const struct cpumask *src1,
  * @cpumask: The cpumask being queried.
  *
  * Count the number of set bits in the given cpumask.
+ *
+ * Return:
+ * * The number of bits set in the mask.
  */
 __bpf_kfunc u32 bpf_cpumask_weight(const struct cpumask *cpumask)
 {
-- 
2.47.1

Re: [PATCH v4 0/3] bpf: introduce helper for populating bpf_cpumask

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Wed,  5 Mar 2025 16:12:32 -0500 you wrote:
> Some BPF programs like scx schedulers have their own internal CPU mask types,
> mask types, which they must transform into struct bpf_cpumask instances
> before passing them to scheduling-related kfuncs. There is currently no
> way to efficiently populate the bitfield of a bpf_cpumask from BPF memory,
> and programs must use multiple bpf_cpumask_[set, clear] calls to do so.
> Introduce a kfunc helper to populate the bitfield of a bpf_cpumask from valid
> BPF memory with a single call.
> 
> [...]

Here is the summary with links:
  - [v4,1/3] bpf: add kfunc for populating cpumask bits
    https://git.kernel.org/bpf/bpf-next/c/a6db20f88a63
  - [v4,2/3] selftests: bpf: add bpf_cpumask_fill selftests
    https://git.kernel.org/bpf/bpf-next/c/66be130f8bf9
  - [v4,3/3] bpf: fix missing kdoc string fields in cpumask.c
    https://git.kernel.org/bpf/bpf-next/c/f3839e9749b1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v4 1/3] bpf: add kfunc for populating cpumask bits

[Hou Tao]

On 3/6/2025 5:12 AM, Emil Tsalapatis wrote:
> Add a helper kfunc that sets the bitmap of a bpf_cpumask from BPF memory.
>
> Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>

Acked-by: Hou Tao <houtao1@huawei.com>

Re: [PATCH v4 2/3] selftests: bpf: add bpf_cpumask_fill selftests

[Hou Tao]

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@etsalapatis.com>
> ---
>  .../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.

> 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 ?
> +int BPF_PROG(test_populate_reject_small_mask, struct task_struct *task, u64 clone_flags)
> +{
> +	struct bpf_cpumask *local;
> +	u8 toofewbits;
> +	int ret;
> +
> +	local = create_cpumask();
> +	if (!local)
> +		return 0;
> +
> +	/* The kfunc should prevent this operation */
> +	ret = bpf_cpumask_populate((struct cpumask *)local, &toofewbits, sizeof(toofewbits));
> +	if (ret != -EACCES)
> +		err = 2;
> +
> +	bpf_cpumask_release(local);
> +
> +	return 0;
> +}
> +
> +/* Mask is guaranteed to be large enough for bpf_cpumask_t. */
> +#define CPUMASK_TEST_MASKLEN (sizeof(cpumask_t))
> +
> +/* Add an extra word for the test_populate_reject_unaligned test. */
> +u64 bits[CPUMASK_TEST_MASKLEN / 8 + 1];
> +extern bool CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS __kconfig __weak;
> +
> +SEC("tp_btf/task_newtask")
> +__success

Same for test_populate_reject_unaligned.
> +int BPF_PROG(test_populate_reject_unaligned, struct task_struct *task, u64 clone_flags)
> +{
> +	struct bpf_cpumask *mask;
> +	char *src;
> +	int ret;
> +
> +	/* Skip if unaligned accesses are fine for this arch.  */
> +	if (CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> +		return 0;
> +
> +	mask = bpf_cpumask_create();
> +	if (!mask) {
> +		err = 1;
> +		return 0;
> +	}
> +
> +	/* Misalign the source array by a byte. */
> +	src = &((char *)bits)[1];
> +
> +	ret = bpf_cpumask_populate((struct cpumask *)mask, src, CPUMASK_TEST_MASKLEN);
> +	if (ret != -EINVAL)
> +		err = 2;
> +
> +	bpf_cpumask_release(mask);
> +
> +	return 0;
> +}
> +
> +
> +SEC("tp_btf/task_newtask")
> +__success
> +int BPF_PROG(test_populate, struct task_struct *task, u64 clone_flags)
> +{
> +	struct bpf_cpumask *mask;
> +	bool bit;
> +	int ret;
> +	int i;
> +
> +	/* Set only odd bits. */
> +	__builtin_memset(bits, 0xaa, CPUMASK_TEST_MASKLEN);
> +
> +	mask = bpf_cpumask_create();
> +	if (!mask) {
> +		err = 1;
> +		return 0;
> +	}
> +
> +	/* Pass the entire bits array, the kfunc will only copy the valid bits. */
> +	ret = bpf_cpumask_populate((struct cpumask *)mask, bits, CPUMASK_TEST_MASKLEN);
> +	if (ret) {
> +		err = 2;
> +		goto out;
> +	}
> +
> +	/*
> +	 * Test is there to appease the verifier. We cannot directly
> +	 * access NR_CPUS, the upper bound for nr_cpus, so we infer
> +	 * it from the size of cpumask_t.
> +	 */
> +	if (nr_cpus < 0 || nr_cpus >= CPUMASK_TEST_MASKLEN * 8) {
> +		err = 3;
> +		goto out;
> +	}
> +
> +	bpf_for(i, 0, nr_cpus) {
> +		/* Odd-numbered bits should be set, even ones unset. */
> +		bit = bpf_cpumask_test_cpu(i, (const struct cpumask *)mask);
> +		if (bit == (i % 2 != 0))
> +			continue;
> +
> +		err = 4;
> +		break;
> +	}
> +
> +out:
> +	bpf_cpumask_release(mask);
> +
> +	return 0;
> +}
> +
> +#undef CPUMASK_TEST_MASKLEN

Re: [PATCH v4 2/3] selftests: bpf: add bpf_cpumask_fill selftests

[Alexei Starovoitov]

On Wed, Mar 5, 2025 at 5:57 PM Hou Tao <houtao@huaweicloud.com> 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@etsalapatis.com>
> > ---
> >  .../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.

Re: [PATCH v4 2/3] selftests: bpf: add bpf_cpumask_fill selftests

[Emil Tsalapatis]

Hi,

thank you for the feedback. I will address it in a v5.

On Wed, Mar 5, 2025 at 8:57 PM Hou Tao <houtao@huaweicloud.com> 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@etsalapatis.com>
> > ---
> >  .../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.
>

Based on Alexei's email systems with recent pahole versions handle
this fine (at least the CI and my local setup),
I will still add the definition in cpumask_common.h for uniformity
since all the other kfuncs have one.

> > 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 ?

Sorry about that, I had the selftests properly hooked into
prog_tests/cpumask.c until v3 but saw duplicate entries in the
selftest log
and thought it was being run twice. I will add them back in.

Is __success() a different annotation? AFAICT __success is enough as
long as err is set to nonzero on an error path, and all
error paths are set like that in the selftests. In that case,
shouldn't adding the new tests cpumask_success_testcases be
enough to properly run the tests?


> > +int BPF_PROG(test_populate_reject_small_mask, struct task_struct *task, u64 clone_flags)
> > +{
> > +     struct bpf_cpumask *local;
> > +     u8 toofewbits;
> > +     int ret;
> > +
> > +     local = create_cpumask();
> > +     if (!local)
> > +             return 0;
> > +
> > +     /* The kfunc should prevent this operation */
> > +     ret = bpf_cpumask_populate((struct cpumask *)local, &toofewbits, sizeof(toofewbits));
> > +     if (ret != -EACCES)
> > +             err = 2;
> > +
> > +     bpf_cpumask_release(local);
> > +
> > +     return 0;
> > +}
> > +
> > +/* Mask is guaranteed to be large enough for bpf_cpumask_t. */
> > +#define CPUMASK_TEST_MASKLEN (sizeof(cpumask_t))
> > +
> > +/* Add an extra word for the test_populate_reject_unaligned test. */
> > +u64 bits[CPUMASK_TEST_MASKLEN / 8 + 1];
> > +extern bool CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS __kconfig __weak;
> > +
> > +SEC("tp_btf/task_newtask")
> > +__success
>
> Same for test_populate_reject_unaligned.
> > +int BPF_PROG(test_populate_reject_unaligned, struct task_struct *task, u64 clone_flags)
> > +{
> > +     struct bpf_cpumask *mask;
> > +     char *src;
> > +     int ret;
> > +
> > +     /* Skip if unaligned accesses are fine for this arch.  */
> > +     if (CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> > +             return 0;
> > +
> > +     mask = bpf_cpumask_create();
> > +     if (!mask) {
> > +             err = 1;
> > +             return 0;
> > +     }
> > +
> > +     /* Misalign the source array by a byte. */
> > +     src = &((char *)bits)[1];
> > +
> > +     ret = bpf_cpumask_populate((struct cpumask *)mask, src, CPUMASK_TEST_MASKLEN);
> > +     if (ret != -EINVAL)
> > +             err = 2;
> > +
> > +     bpf_cpumask_release(mask);
> > +
> > +     return 0;
> > +}
> > +
> > +
> > +SEC("tp_btf/task_newtask")
> > +__success
> > +int BPF_PROG(test_populate, struct task_struct *task, u64 clone_flags)
> > +{
> > +     struct bpf_cpumask *mask;
> > +     bool bit;
> > +     int ret;
> > +     int i;
> > +
> > +     /* Set only odd bits. */
> > +     __builtin_memset(bits, 0xaa, CPUMASK_TEST_MASKLEN);
> > +
> > +     mask = bpf_cpumask_create();
> > +     if (!mask) {
> > +             err = 1;
> > +             return 0;
> > +     }
> > +
> > +     /* Pass the entire bits array, the kfunc will only copy the valid bits. */
> > +     ret = bpf_cpumask_populate((struct cpumask *)mask, bits, CPUMASK_TEST_MASKLEN);
> > +     if (ret) {
> > +             err = 2;
> > +             goto out;
> > +     }
> > +
> > +     /*
> > +      * Test is there to appease the verifier. We cannot directly
> > +      * access NR_CPUS, the upper bound for nr_cpus, so we infer
> > +      * it from the size of cpumask_t.
> > +      */
> > +     if (nr_cpus < 0 || nr_cpus >= CPUMASK_TEST_MASKLEN * 8) {
> > +             err = 3;
> > +             goto out;
> > +     }
> > +
> > +     bpf_for(i, 0, nr_cpus) {
> > +             /* Odd-numbered bits should be set, even ones unset. */
> > +             bit = bpf_cpumask_test_cpu(i, (const struct cpumask *)mask);
> > +             if (bit == (i % 2 != 0))
> > +                     continue;
> > +
> > +             err = 4;
> > +             break;
> > +     }
> > +
> > +out:
> > +     bpf_cpumask_release(mask);
> > +
> > +     return 0;
> > +}
> > +
> > +#undef CPUMASK_TEST_MASKLEN
>

Re: [PATCH v4 2/3] selftests: bpf: add bpf_cpumask_fill selftests

[Hou Tao]

Hi,

On 3/6/2025 10:36 AM, Emil Tsalapatis wrote:
> Hi,
>
> thank you for the feedback. I will address it in a v5.
>
> On Wed, Mar 5, 2025 at 8:57 PM Hou Tao <houtao@huaweicloud.com> 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.

Just find out, the name of bpf_cpumask_fill() also needs update.
>>>
>>> Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
>>> ---
>>>  .../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.
>>
> Based on Alexei's email systems with recent pahole versions handle
> this fine (at least the CI and my local setup),
> I will still add the definition in cpumask_common.h for uniformity
> since all the other kfuncs have one.

I see. Thanks for that.
>
>>> 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;
>>> +}
>>> +

SNIP
>> 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 ?
> Sorry about that, I had the selftests properly hooked into
> prog_tests/cpumask.c until v3 but saw duplicate entries in the
> selftest log
> and thought it was being run twice. I will add them back in.
>
> Is __success() a different annotation? AFAICT __success is enough as
> long as err is set to nonzero on an error path, and all
> error paths are set like that in the selftests. In that case,
> shouldn't adding the new tests cpumask_success_testcases be
> enough to properly run the tests?

Yes. __success() annotation is a bit different. It uses
bpf_prog_test_run() to run the bpf prog directly instead of trigger the
running of prog through an external event. I think adding new tests in
cpumask_success_testcases will be enough. However, there is one success
test test_refcount_null_tracking in cpumask_success.c which uses
__success annotation, and it is still buggy. I think it would be better
to switch all test cases to use __success annotation because the
annotation provides much clarity.
>
>
>>> +int BPF_PROG(test_populate_reject_small_mask, struct task_struct *task, u64 clone_flags)
>>> +{
>>> +     struct bpf_cpumask *local;
>>> +     u8 toofewbits;
>>> +     int ret;
>>> +
>>> +     local = create_cpumask();
>>> +     if (!local)
>>> +             return 0;
>>> +
>>> +     /* The kfunc should prevent this operation */
>>> +     ret = bpf_cpumask_populate((struct cpumask *)local, &toofewbits, sizeof(toofewbits));
>>> +     if (ret != -EACCES)
>>> +             err = 2;
>>> +
>>> +     bpf_cpumask_release(local);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +/* Mask is guaranteed to be large enough for bpf_cpumask_t. */
>>> +#define CPUMASK_TEST_MASKLEN (sizeof(cpumask_t))
>>> +
>>> +/* Add an extra word for the test_populate_reject_unaligned test. */
>>> +u64 bits[CPUMASK_TEST_MASKLEN / 8 + 1];
>>> +extern bool CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS __kconfig __weak;
>>> +
>>> +SEC("tp_btf/task_newtask")
>>> +__success
>> Same for test_populate_reject_unaligned.
>>> +int BPF_PROG(test_populate_reject_unaligned, struct task_struct *task, u64 clone_flags)
>>> +{
>>> +     struct bpf_cpumask *mask;
>>> +     char *src;
>>> +     int ret;
>>> +
>>> +     /* Skip if unaligned accesses are fine for this arch.  */
>>> +     if (CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
>>> +             return 0;
>>> +
>>> +     mask = bpf_cpumask_create();
>>> +     if (!mask) {
>>> +             err = 1;
>>> +             return 0;
>>> +     }
>>> +
>>> +     /* Misalign the source array by a byte. */
>>> +     src = &((char *)bits)[1];
>>> +
>>> +     ret = bpf_cpumask_populate((struct cpumask *)mask, src, CPUMASK_TEST_MASKLEN);
>>> +     if (ret != -EINVAL)
>>> +             err = 2;
>>> +
>>> +     bpf_cpumask_release(mask);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +
>>> +SEC("tp_btf/task_newtask")
>>> +__success
>>> +int BPF_PROG(test_populate, struct task_struct *task, u64 clone_flags)
>>> +{
>>> +     struct bpf_cpumask *mask;
>>> +     bool bit;
>>> +     int ret;
>>> +     int i;
>>> +
>>> +     /* Set only odd bits. */
>>> +     __builtin_memset(bits, 0xaa, CPUMASK_TEST_MASKLEN);
>>> +
>>> +     mask = bpf_cpumask_create();
>>> +     if (!mask) {
>>> +             err = 1;
>>> +             return 0;
>>> +     }
>>> +
>>> +     /* Pass the entire bits array, the kfunc will only copy the valid bits. */
>>> +     ret = bpf_cpumask_populate((struct cpumask *)mask, bits, CPUMASK_TEST_MASKLEN);
>>> +     if (ret) {
>>> +             err = 2;
>>> +             goto out;
>>> +     }
>>> +
>>> +     /*
>>> +      * Test is there to appease the verifier. We cannot directly
>>> +      * access NR_CPUS, the upper bound for nr_cpus, so we infer
>>> +      * it from the size of cpumask_t.
>>> +      */
>>> +     if (nr_cpus < 0 || nr_cpus >= CPUMASK_TEST_MASKLEN * 8) {
>>> +             err = 3;
>>> +             goto out;
>>> +     }
>>> +
>>> +     bpf_for(i, 0, nr_cpus) {
>>> +             /* Odd-numbered bits should be set, even ones unset. */
>>> +             bit = bpf_cpumask_test_cpu(i, (const struct cpumask *)mask);
>>> +             if (bit == (i % 2 != 0))
>>> +                     continue;
>>> +
>>> +             err = 4;
>>> +             break;
>>> +     }
>>> +
>>> +out:
>>> +     bpf_cpumask_release(mask);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +#undef CPUMASK_TEST_MASKLEN