[RFC PATCH 2/3] selftests/bpf: add memcg eBPF struct ops test

Hui Zhu posted 3 patches 1 week, 6 days ago
[RFC PATCH 2/3] selftests/bpf: add memcg eBPF struct ops test
Posted by Hui Zhu 1 week, 6 days ago
From: Hui Zhu <zhuhui@kylinos.cn>

Add comprehensive selftest suite for memory controller eBPF
support. The tests validate the core functionality of the
memcg_ops struct ops implementation.

Test coverage includes:

1. test_memcg_ops_load: Validates that the eBPF object file
   can be successfully loaded by libbpf.

2. test_memcg_ops_attach: Tests attaching the memcg_ops struct
   ops to the kernel, verifying the basic attachment mechanism
   works correctly.

3. test_memcg_ops_double_attach: Validates that only one
   memcg_ops instance can be attached at a time. Attempts to
   attach a second program should fail, ensuring the
   single-handler design constraint is enforced.

The test suite includes:

- prog_tests/memcg_ops.c: Test entry point with individual
  test functions using standard BPF test framework helpers
  like ASSERT_OK_PTR and ASSERT_ERR_PTR
- progs/memcg_ops.bpf.c: Simple eBPF program implementing
  the struct ops interface

Uses standard test_progs framework macros for consistent error
reporting and handling. All tests properly clean up resources
in error paths.

Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 MAINTAINERS                                   |   1 +
 .../selftests/bpf/prog_tests/memcg_ops.c      | 117 ++++++++++++++++++
 tools/testing/selftests/bpf/progs/memcg_ops.c |  20 +++
 3 files changed, 138 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/memcg_ops.c
 create mode 100644 tools/testing/selftests/bpf/progs/memcg_ops.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 498d01c9a48e..dc3aa53d2346 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6356,6 +6356,7 @@ F:	mm/memcontrol_bpf.h
 F:	mm/page_counter.c
 F:	mm/swap_cgroup.c
 F:	samples/cgroup/*
+F:	tools/testing/selftests/bpf/*/memcg_ops.c
 F:	tools/testing/selftests/cgroup/memcg_protection.m
 F:	tools/testing/selftests/cgroup/test_hugetlb_memcg.c
 F:	tools/testing/selftests/cgroup/test_kmem.c
diff --git a/tools/testing/selftests/bpf/prog_tests/memcg_ops.c b/tools/testing/selftests/bpf/prog_tests/memcg_ops.c
new file mode 100644
index 000000000000..3f989bcfb8c4
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/memcg_ops.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Memory controller eBPF struct ops test
+ */
+
+#include <test_progs.h>
+#include <bpf/btf.h>
+
+void test_memcg_ops_load(void)
+{
+	struct bpf_object *obj;
+	int err;
+
+	obj = bpf_object__open_file("memcg_ops.bpf.o", NULL);
+	err = libbpf_get_error(obj);
+	if (CHECK_FAIL(err)) {
+		obj = NULL;
+		goto out;
+	}
+
+	err = bpf_object__load(obj);
+	if (CHECK_FAIL(err))
+		goto out;
+
+out:
+	if (obj)
+		bpf_object__close(obj);
+}
+
+void test_memcg_ops_attach(void)
+{
+	struct bpf_object *obj;
+	struct bpf_map *map;
+	struct bpf_link *link = NULL;
+	int err;
+
+	obj = bpf_object__open_file("memcg_ops.bpf.o", NULL);
+	err = libbpf_get_error(obj);
+	if (CHECK_FAIL(err)) {
+		obj = NULL;
+		goto out;
+	}
+
+	err = bpf_object__load(obj);
+	if (CHECK_FAIL(err))
+		goto out;
+
+	map = bpf_object__find_map_by_name(obj, "mcg_ops");
+	if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name"))
+		goto out;
+
+	link = bpf_map__attach_struct_ops(map);
+	if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops"))
+		goto out;
+
+out:
+	if (link)
+		bpf_link__destroy(link);
+	if (obj)
+		bpf_object__close(obj);
+}
+
+void test_memcg_ops_double_attach(void)
+{
+	struct bpf_object *obj, *obj2;
+	struct bpf_map *map, *map2;
+	struct bpf_link *link = NULL, *link2 = NULL;
+	int err;
+
+	obj = bpf_object__open_file("memcg_ops.bpf.o", NULL);
+	err = libbpf_get_error(obj);
+	if (CHECK_FAIL(err)) {
+		obj = NULL;
+		goto out;
+	}
+
+	err = bpf_object__load(obj);
+	if (CHECK_FAIL(err))
+		goto out;
+
+	map = bpf_object__find_map_by_name(obj, "mcg_ops");
+	if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name"))
+		goto out;
+
+	link = bpf_map__attach_struct_ops(map);
+	if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops"))
+		goto out;
+
+	obj2 = bpf_object__open_file("memcg_ops.bpf.o", NULL);
+	err = libbpf_get_error(obj2);
+	if (CHECK_FAIL(err)) {
+		obj2 = NULL;
+		goto out;
+	}
+
+	err = bpf_object__load(obj2);
+	if (CHECK_FAIL(err))
+		goto out;
+
+	map2 = bpf_object__find_map_by_name(obj2, "mcg_ops");
+	if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name"))
+		goto out;
+
+	link2 = bpf_map__attach_struct_ops(map2);
+	if (!ASSERT_ERR_PTR(link2, "bpf_map__attach_struct_ops")) {
+		bpf_link__destroy(link2);
+		goto out;
+	}
+
+out:
+	if (link)
+		bpf_link__destroy(link);
+	if (obj)
+		bpf_object__close(obj);
+	if (obj2)
+		bpf_object__close(obj2);
+}
diff --git a/tools/testing/selftests/bpf/progs/memcg_ops.c b/tools/testing/selftests/bpf/progs/memcg_ops.c
new file mode 100644
index 000000000000..a21fbe859fd3
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/memcg_ops.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "vmlinux.h"
+
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+SEC("struct_ops/try_charge_memcg")
+int BPF_PROG(test_try_charge_memcg,
+	     struct try_charge_memcg *tcm)
+{
+	return 0;
+}
+
+SEC(".struct_ops")
+struct memcg_ops mcg_ops = {
+	.try_charge_memcg = (void *)test_try_charge_memcg,
+};
-- 
2.43.0
Re: [RFC PATCH 2/3] selftests/bpf: add memcg eBPF struct ops test
Posted by bot+bpf-ci@kernel.org 1 week, 6 days ago
> diff --git a/tools/testing/selftests/bpf/prog_tests/memcg_ops.c b/tools/testing/selftests/bpf/prog_tests/memcg_ops.c
> new file mode 100644
> index 000000000..3f989bcfb
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/memcg_ops.c

[ ... ]

> +void test_memcg_ops_double_attach(void)
> +{
> +	struct bpf_object *obj, *obj2;
> +	struct bpf_map *map, *map2;
> +	struct bpf_link *link = NULL, *link2 = NULL;
> +	int err;
> +
> +	obj = bpf_object__open_file("memcg_ops.bpf.o", NULL);
> +	err = libbpf_get_error(obj);
> +	if (CHECK_FAIL(err)) {
> +		obj = NULL;
> +		goto out;
> +	}
> +
> +	err = bpf_object__load(obj);
> +	if (CHECK_FAIL(err))
> +		goto out;
> +
> +	map = bpf_object__find_map_by_name(obj, "mcg_ops");
> +	if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name"))
> +		goto out;
> +
> +	link = bpf_map__attach_struct_ops(map);
> +	if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops"))
> +		goto out;
> +
> +	obj2 = bpf_object__open_file("memcg_ops.bpf.o", NULL);
> +	err = libbpf_get_error(obj2);
> +	if (CHECK_FAIL(err)) {
> +		obj2 = NULL;
> +		goto out;
> +	}
> +
> +	err = bpf_object__load(obj2);
> +	if (CHECK_FAIL(err))
> +		goto out;
> +
> +	map2 = bpf_object__find_map_by_name(obj2, "mcg_ops");
> +	if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name"))
                         ^^^

In test_memcg_ops_double_attach(), the NULL check is validating the wrong
variable. The code retrieves map2 from obj2, but then checks map (from the
first object) instead of map2. This means if bpf_object__find_map_by_name()
returns NULL for map2, the test won't catch it and will proceed to use the
potentially NULL map2 pointer in the subsequent attach call.

Should this be checking map2 instead?

> +		goto out;
> +
> +	link2 = bpf_map__attach_struct_ops(map2);
> +	if (!ASSERT_ERR_PTR(link2, "bpf_map__attach_struct_ops")) {
> +		bpf_link__destroy(link2);
> +		goto out;
> +	}
> +
> +out:
> +	if (link)
> +		bpf_link__destroy(link);
> +	if (obj)
> +		bpf_object__close(obj);
> +	if (obj2)
> +		bpf_object__close(obj2);
> +}


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19486904708