Hello, this small series aims to integrate test_dev_cgroup in test_progs so it could be run automatically in CI. The new version brings a few differences with the current one: - test now uses directly syscalls instead of wrapping commandline tools into system() calls - test_progs manipulates /dev/null (eg: redirecting test logs into it), so disabling access to it in the bpf program confuses the tests. To fix this, the first commit modifies the bpf program to allow access to char devices 1:3 (/dev/null), and disable access to char devices 1:5 (/dev/zero) - once test is converted, add a small subtest to also check for device type interpretation (char or block) - paths used in mknod tests are now in /dev instead of /tmp: due to the CI runner organisation and mountpoints manipulations, trying to create nodes in /tmp leads to errors unrelated to the test (ie, mknod calls refused by kernel, not the bpf program). I don't understand exactly the root cause at the deepest point (all I see in CI is an -ENXIO error on mknod when trying to create the node in tmp, and I can not make sense out of it neither replicate it locally), so I would gladly take inputs from anyone more educated than me about this.
The new test_progs part has been tested in a local qemu environment as well as in upstream CI:
./test_progs -a cgroup_dev 47/1 cgroup_dev/deny-mknod:OK 47/2 cgroup_dev/allow-mknod:OK 47/3 cgroup_dev/deny-mknod-wrong-type:OK 47/4 cgroup_dev/allow-read:OK 47/5 cgroup_dev/allow-write:OK 47/6 cgroup_dev/deny-read:OK 47/7 cgroup_dev/deny-write:OK 47 cgroup_dev:OK Summary: 1/7 PASSED, 0 SKIPPED, 0 FAILED
--- Alexis Lothoré (eBPF Foundation) (3): selftests/bpf: do not disable /dev/null device access in cgroup dev test selftests/bpf: convert test_dev_cgroup to test_progs selftests/bpf: add wrong type test to cgroup dev
tools/testing/selftests/bpf/.gitignore | 1 - tools/testing/selftests/bpf/Makefile | 2 - .../testing/selftests/bpf/prog_tests/cgroup_dev.c | 123 +++++++++++++++++++++ tools/testing/selftests/bpf/progs/dev_cgroup.c | 4 +- tools/testing/selftests/bpf/test_dev_cgroup.c | 85 -------------- 5 files changed, 125 insertions(+), 90 deletions(-) --- base-commit: c90e2d4a7738a24fbb3657092dbd1ca20c040ed1 change-id: 20240723-convert_dev_cgroup-6464b0d37f1a
Best regards,
test_dev_cgroup currently loads a small bpf program allowing any access on urandom and zero devices, disabling access to any other device. It makes migrating this test to test_progs impossible, since this one manipulates extensively /dev/null.
Allow /dev/null manipulation in dev_cgroup program to make its usage in test_progs framework possible. Update test_dev_cgroup.c as well to match this change while it has not been removed.
Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- tools/testing/selftests/bpf/progs/dev_cgroup.c | 4 ++-- tools/testing/selftests/bpf/test_dev_cgroup.c | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/dev_cgroup.c b/tools/testing/selftests/bpf/progs/dev_cgroup.c index 79b54a4fa244..c1dfbd2b56fc 100644 --- a/tools/testing/selftests/bpf/progs/dev_cgroup.c +++ b/tools/testing/selftests/bpf/progs/dev_cgroup.c @@ -41,14 +41,14 @@ int bpf_prog1(struct bpf_cgroup_dev_ctx *ctx) bpf_trace_printk(fmt, sizeof(fmt), ctx->major, ctx->minor); #endif
- /* Allow access to /dev/zero and /dev/random. + /* Allow access to /dev/null and /dev/urandom. * Forbid everything else. */ if (ctx->major != 1 || type != BPF_DEVCG_DEV_CHAR) return 0;
switch (ctx->minor) { - case 5: /* 1:5 /dev/zero */ + case 3: /* 1:3 /dev/null */ case 9: /* 1:9 /dev/urandom */ return 1; } diff --git a/tools/testing/selftests/bpf/test_dev_cgroup.c b/tools/testing/selftests/bpf/test_dev_cgroup.c index adeaf63cb6fa..33f544f0005a 100644 --- a/tools/testing/selftests/bpf/test_dev_cgroup.c +++ b/tools/testing/selftests/bpf/test_dev_cgroup.c @@ -54,25 +54,25 @@ int main(int argc, char **argv) goto err; }
- /* All operations with /dev/zero and and /dev/urandom are allowed, + /* All operations with /dev/null and /dev/urandom are allowed, * everything else is forbidden. */ - assert(system("rm -f /tmp/test_dev_cgroup_null") == 0); - assert(system("mknod /tmp/test_dev_cgroup_null c 1 3")); - assert(system("rm -f /tmp/test_dev_cgroup_null") == 0); - - /* /dev/zero is whitelisted */ assert(system("rm -f /tmp/test_dev_cgroup_zero") == 0); - assert(system("mknod /tmp/test_dev_cgroup_zero c 1 5") == 0); + assert(system("mknod /tmp/test_dev_cgroup_zero c 1 5")); assert(system("rm -f /tmp/test_dev_cgroup_zero") == 0);
- assert(system("dd if=/dev/urandom of=/dev/zero count=64") == 0); + /* /dev/null is whitelisted */ + assert(system("rm -f /tmp/test_dev_cgroup_null") == 0); + assert(system("mknod /tmp/test_dev_cgroup_null c 1 3") == 0); + assert(system("rm -f /tmp/test_dev_cgroup_null") == 0); + + assert(system("dd if=/dev/urandom of=/dev/null count=64") == 0);
/* src is allowed, target is forbidden */ assert(system("dd if=/dev/urandom of=/dev/full count=64"));
/* src is forbidden, target is allowed */ - assert(system("dd if=/dev/random of=/dev/zero count=64")); + assert(system("dd if=/dev/random of=/dev/null count=64"));
error = 0; printf("test_dev_cgroup:PASS\n");
test_dev_cgroup is defined as a standalone test program, and so is not executed in CI.
Convert it to test_progs framework so it is tested automatically in CI, and remove the old test. In order to be able to run it in test_progs, /dev/null must remain usable, so change the new test to test operations on devices 1:3 as valid, and operations on devices 1:5 (/dev/zero) as invalid.
Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- tools/testing/selftests/bpf/.gitignore | 1 - tools/testing/selftests/bpf/Makefile | 2 - .../testing/selftests/bpf/prog_tests/cgroup_dev.c | 120 +++++++++++++++++++++ tools/testing/selftests/bpf/test_dev_cgroup.c | 85 --------------- 4 files changed, 120 insertions(+), 88 deletions(-)
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore index 4e4aae8aa7ec..8f14d8faeb0b 100644 --- a/tools/testing/selftests/bpf/.gitignore +++ b/tools/testing/selftests/bpf/.gitignore @@ -9,7 +9,6 @@ test_lpm_map test_tag FEATURE-DUMP.libbpf fixdep -test_dev_cgroup /test_progs /test_progs-no_alu32 /test_progs-bpf_gcc diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index aeada478e37a..2a9ba2246f80 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -69,7 +69,6 @@ endif
# Order correspond to 'make run_tests' order TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \ - test_dev_cgroup \ test_sock test_sockmap get_cgroup_id_user \ test_cgroup_storage \ test_tcpnotify_user test_sysctl \ @@ -295,7 +294,6 @@ JSON_WRITER := $(OUTPUT)/json_writer.o CAP_HELPERS := $(OUTPUT)/cap_helpers.o NETWORK_HELPERS := $(OUTPUT)/network_helpers.o
-$(OUTPUT)/test_dev_cgroup: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(OUTPUT)/test_skb_cgroup_id_user: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(OUTPUT)/test_sock: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(OUTPUT)/test_sockmap: $(CGROUP_HELPERS) $(TESTING_HELPERS) diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c b/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c new file mode 100644 index 000000000000..5112b99213ad --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <sys/stat.h> +#include <sys/sysmacros.h> +#include "test_progs.h" +#include "cgroup_helpers.h" +#include "dev_cgroup.skel.h" + +#define TEST_CGROUP "/test-bpf-based-device-cgroup/" +#define TEST_BUFFER_SIZE 64 + +static void test_mknod(const char *path, mode_t mode, int dev_major, + int dev_minor, int should_fail) +{ + int ret; + + unlink(path); + ret = mknod(path, mode, makedev(dev_major, dev_minor)); + if (should_fail) + ASSERT_ERR(ret, "mknod"); + else + ASSERT_OK(ret, "mknod"); + unlink(path); +} + +static void test_read(const char *path, int should_fail) +{ + char buf[TEST_BUFFER_SIZE]; + int ret, fd; + + fd = open(path, O_RDONLY); + + /* A bare open on unauthorized device should fail */ + if (should_fail) { + ASSERT_ERR(fd, "open file for read"); + if (fd) + close(fd); + return; + } + + if (!ASSERT_OK_FD(fd, "open file for read")) + return; + + ret = read(fd, buf, TEST_BUFFER_SIZE); + if (should_fail) + ASSERT_ERR(ret, "read"); + else + ASSERT_EQ(ret, TEST_BUFFER_SIZE, "read"); + + close(fd); +} + +static void test_write(const char *path, int should_fail) +{ + char buf[] = "some random test data"; + int ret, fd; + + fd = open(path, O_WRONLY); + + /* A bare open on unauthorized device should fail */ + if (should_fail) { + ASSERT_ERR(fd, "open file for write"); + if (fd) + close(fd); + return; + } + + if (!ASSERT_OK_FD(fd, "open file for write")) + return; + + ret = write(fd, buf, sizeof(buf)); + if (should_fail) + ASSERT_ERR(ret, "write"); + else + ASSERT_EQ(ret, sizeof(buf), "write"); + + close(fd); +} + +void test_cgroup_dev(void) +{ + struct dev_cgroup *skel; + int cgroup_fd; + + cgroup_fd = cgroup_setup_and_join(TEST_CGROUP); + if (!ASSERT_OK_FD(cgroup_fd, "cgroup switch")) + return; + + skel = dev_cgroup__open_and_load(); + if (!ASSERT_OK_PTR(skel, "load program")) + goto cleanup_cgroup; + + if (!ASSERT_OK(bpf_prog_attach(bpf_program__fd(skel->progs.bpf_prog1), + cgroup_fd, BPF_CGROUP_DEVICE, 0), + "attach_program")) + goto cleanup_progs; + + if (test__start_subtest("deny-mknod")) + test_mknod("/dev/test_dev_cgroup_zero", S_IFCHR, 1, 5, 1); + + if (test__start_subtest("allow-mknod")) + test_mknod("/dev/test_dev_cgroup_null", S_IFCHR, 1, 3, 0); + + if (test__start_subtest("allow-read")) + test_read("/dev/urandom", 0); + + if (test__start_subtest("allow-write")) + test_write("/dev/null", 0); + + if (test__start_subtest("deny-read")) + test_read("/dev/random", 1); + + if (test__start_subtest("deny-write")) + test_write("/dev/zero", 1); + +cleanup_progs: + dev_cgroup__destroy(skel); +cleanup_cgroup: + cleanup_cgroup_environment(); +} diff --git a/tools/testing/selftests/bpf/test_dev_cgroup.c b/tools/testing/selftests/bpf/test_dev_cgroup.c deleted file mode 100644 index 33f544f0005a..000000000000 --- a/tools/testing/selftests/bpf/test_dev_cgroup.c +++ /dev/null @@ -1,85 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2017 Facebook - */ - -#include <stdio.h> -#include <stdlib.h> -#include <string.h> -#include <errno.h> -#include <assert.h> -#include <sys/time.h> - -#include <linux/bpf.h> -#include <bpf/bpf.h> -#include <bpf/libbpf.h> - -#include "cgroup_helpers.h" -#include "testing_helpers.h" - -#define DEV_CGROUP_PROG "./dev_cgroup.bpf.o" - -#define TEST_CGROUP "/test-bpf-based-device-cgroup/" - -int main(int argc, char **argv) -{ - struct bpf_object *obj; - int error = EXIT_FAILURE; - int prog_fd, cgroup_fd; - __u32 prog_cnt; - - /* Use libbpf 1.0 API mode */ - libbpf_set_strict_mode(LIBBPF_STRICT_ALL); - - if (bpf_prog_test_load(DEV_CGROUP_PROG, BPF_PROG_TYPE_CGROUP_DEVICE, - &obj, &prog_fd)) { - printf("Failed to load DEV_CGROUP program\n"); - goto out; - } - - cgroup_fd = cgroup_setup_and_join(TEST_CGROUP); - if (cgroup_fd < 0) { - printf("Failed to create test cgroup\n"); - goto out; - } - - /* Attach bpf program */ - if (bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_DEVICE, 0)) { - printf("Failed to attach DEV_CGROUP program"); - goto err; - } - - if (bpf_prog_query(cgroup_fd, BPF_CGROUP_DEVICE, 0, NULL, NULL, - &prog_cnt)) { - printf("Failed to query attached programs"); - goto err; - } - - /* All operations with /dev/null and /dev/urandom are allowed, - * everything else is forbidden. - */ - assert(system("rm -f /tmp/test_dev_cgroup_zero") == 0); - assert(system("mknod /tmp/test_dev_cgroup_zero c 1 5")); - assert(system("rm -f /tmp/test_dev_cgroup_zero") == 0); - - /* /dev/null is whitelisted */ - assert(system("rm -f /tmp/test_dev_cgroup_null") == 0); - assert(system("mknod /tmp/test_dev_cgroup_null c 1 3") == 0); - assert(system("rm -f /tmp/test_dev_cgroup_null") == 0); - - assert(system("dd if=/dev/urandom of=/dev/null count=64") == 0); - - /* src is allowed, target is forbidden */ - assert(system("dd if=/dev/urandom of=/dev/full count=64")); - - /* src is forbidden, target is allowed */ - assert(system("dd if=/dev/random of=/dev/null count=64")); - - error = 0; - printf("test_dev_cgroup:PASS\n"); - -err: - cleanup_cgroup_environment(); - -out: - return error; -}
On 07/25, Alexis Lothoré (eBPF Foundation) wrote:
test_dev_cgroup is defined as a standalone test program, and so is not executed in CI.
Convert it to test_progs framework so it is tested automatically in CI, and remove the old test. In order to be able to run it in test_progs, /dev/null must remain usable, so change the new test to test operations on devices 1:3 as valid, and operations on devices 1:5 (/dev/zero) as invalid.
Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com
tools/testing/selftests/bpf/.gitignore | 1 - tools/testing/selftests/bpf/Makefile | 2 - .../testing/selftests/bpf/prog_tests/cgroup_dev.c | 120 +++++++++++++++++++++ tools/testing/selftests/bpf/test_dev_cgroup.c | 85 --------------- 4 files changed, 120 insertions(+), 88 deletions(-)
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore index 4e4aae8aa7ec..8f14d8faeb0b 100644 --- a/tools/testing/selftests/bpf/.gitignore +++ b/tools/testing/selftests/bpf/.gitignore @@ -9,7 +9,6 @@ test_lpm_map test_tag FEATURE-DUMP.libbpf fixdep -test_dev_cgroup /test_progs /test_progs-no_alu32 /test_progs-bpf_gcc diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index aeada478e37a..2a9ba2246f80 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -69,7 +69,6 @@ endif # Order correspond to 'make run_tests' order TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
- test_dev_cgroup \ test_sock test_sockmap get_cgroup_id_user \ test_cgroup_storage \ test_tcpnotify_user test_sysctl \
@@ -295,7 +294,6 @@ JSON_WRITER := $(OUTPUT)/json_writer.o CAP_HELPERS := $(OUTPUT)/cap_helpers.o NETWORK_HELPERS := $(OUTPUT)/network_helpers.o -$(OUTPUT)/test_dev_cgroup: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(OUTPUT)/test_skb_cgroup_id_user: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(OUTPUT)/test_sock: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(OUTPUT)/test_sockmap: $(CGROUP_HELPERS) $(TESTING_HELPERS) diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c b/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c new file mode 100644 index 000000000000..5112b99213ad --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: GPL-2.0
+#include <sys/stat.h> +#include <sys/sysmacros.h> +#include "test_progs.h" +#include "cgroup_helpers.h" +#include "dev_cgroup.skel.h"
+#define TEST_CGROUP "/test-bpf-based-device-cgroup/" +#define TEST_BUFFER_SIZE 64
+static void test_mknod(const char *path, mode_t mode, int dev_major,
int dev_minor, int should_fail)+{
- int ret;
- unlink(path);
- ret = mknod(path, mode, makedev(dev_major, dev_minor));
[..]
- if (should_fail)
ASSERT_ERR(ret, "mknod");- else
ASSERT_OK(ret, "mknod");
Optional: might be easier to use something like expected_ret instead of should_fail and then do:
ASSERT_EQ(ret, expected_ret)
I see this part being copy-pasted in a bunch of places below.
- unlink(path);
+}
+static void test_read(const char *path, int should_fail) +{
- char buf[TEST_BUFFER_SIZE];
- int ret, fd;
- fd = open(path, O_RDONLY);
- /* A bare open on unauthorized device should fail */
- if (should_fail) {
ASSERT_ERR(fd, "open file for read");
[..]
if (fd)close(fd);
nit: should this be 'if (fd >= 0)'? I'm assuming the intention is to avoid close(-1)?
return;- }
- if (!ASSERT_OK_FD(fd, "open file for read"))
return;- ret = read(fd, buf, TEST_BUFFER_SIZE);
- if (should_fail)
ASSERT_ERR(ret, "read");- else
ASSERT_EQ(ret, TEST_BUFFER_SIZE, "read");- close(fd);
+}
+static void test_write(const char *path, int should_fail) +{
- char buf[] = "some random test data";
- int ret, fd;
- fd = open(path, O_WRONLY);
- /* A bare open on unauthorized device should fail */
- if (should_fail) {
ASSERT_ERR(fd, "open file for write");if (fd)close(fd);
Same 'if (fd >= 0)'
Hello Stanislas, thanks for the review
On 7/27/24 00:48, Stanislav Fomichev wrote:
On 07/25, Alexis Lothoré (eBPF Foundation) wrote:
[...]
- if (should_fail)
ASSERT_ERR(ret, "mknod");- else
ASSERT_OK(ret, "mknod");Optional: might be easier to use something like expected_ret instead of should_fail and then do:
ASSERT_EQ(ret, expected_ret)
Yes, you are right. I initially went with a version relying on system() to perform the mknods/dd calls, which could return different errors codes so I used this should_fail. But while debugging some issues in CI with this series, I realized that the needed commands are basic enough to be replaced with direct library calls and I forgot to update this part, which can now assert an exact return value. I will update this accordingly.
I see this part being copy-pasted in a bunch of places below.
- unlink(path);
+}
+static void test_read(const char *path, int should_fail) +{
- char buf[TEST_BUFFER_SIZE];
- int ret, fd;
- fd = open(path, O_RDONLY);
- /* A bare open on unauthorized device should fail */
- if (should_fail) {
ASSERT_ERR(fd, "open file for read");[..]
if (fd)close(fd);nit: should this be 'if (fd >= 0)'? I'm assuming the intention is to avoid close(-1)?
Right as well, I'll fix it (here and below) in v2
Thanks,
Alexis
Current cgroup_dev test mostly tests that device operation is accepted or refused base on passed major/minor (and so, any operation performed during test involves only char device)
Add a small subtest ensuring that the device type passed to bpf program allows it to take decisions as well.
Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- tools/testing/selftests/bpf/prog_tests/cgroup_dev.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c b/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c index 5112b99213ad..f0b6e5d9604b 100644 --- a/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_dev.c @@ -101,6 +101,9 @@ void test_cgroup_dev(void) if (test__start_subtest("allow-mknod")) test_mknod("/dev/test_dev_cgroup_null", S_IFCHR, 1, 3, 0);
+ if (test__start_subtest("deny-mknod-wrong-type")) + test_mknod("/dev/test_dev_cgroup_null_block", S_IFBLK, 1, 3, 1); + if (test__start_subtest("allow-read")) test_read("/dev/urandom", 0);
On 07/25, Alexis Lothoré (eBPF Foundation) wrote:
Hello, this small series aims to integrate test_dev_cgroup in test_progs so it could be run automatically in CI. The new version brings a few differences with the current one:
- test now uses directly syscalls instead of wrapping commandline tools into system() calls
- test_progs manipulates /dev/null (eg: redirecting test logs into it), so disabling access to it in the bpf program confuses the tests. To fix this, the first commit modifies the bpf program to allow access to char devices 1:3 (/dev/null), and disable access to char devices 1:5 (/dev/zero)
- once test is converted, add a small subtest to also check for device type interpretation (char or block)
- paths used in mknod tests are now in /dev instead of /tmp: due to the CI runner organisation and mountpoints manipulations, trying to create nodes in /tmp leads to errors unrelated to the test (ie, mknod calls refused by kernel, not the bpf program). I don't understand exactly the root cause at the deepest point (all I see in CI is an -ENXIO error on mknod when trying to create the node in tmp, and I can not make sense out of it neither replicate it locally), so I would gladly take inputs from anyone more educated than me about this.
The new test_progs part has been tested in a local qemu environment as well as in upstream CI:
./test_progs -a cgroup_dev 47/1 cgroup_dev/deny-mknod:OK 47/2 cgroup_dev/allow-mknod:OK 47/3 cgroup_dev/deny-mknod-wrong-type:OK 47/4 cgroup_dev/allow-read:OK 47/5 cgroup_dev/allow-write:OK 47/6 cgroup_dev/deny-read:OK 47/7 cgroup_dev/deny-write:OK 47 cgroup_dev:OK Summary: 1/7 PASSED, 0 SKIPPED, 0 FAILED
Alexis Lothoré (eBPF Foundation) (3): selftests/bpf: do not disable /dev/null device access in cgroup dev test selftests/bpf: convert test_dev_cgroup to test_progs selftests/bpf: add wrong type test to cgroup dev
tools/testing/selftests/bpf/.gitignore | 1 - tools/testing/selftests/bpf/Makefile | 2 - .../testing/selftests/bpf/prog_tests/cgroup_dev.c | 123 +++++++++++++++++++++ tools/testing/selftests/bpf/progs/dev_cgroup.c | 4 +- tools/testing/selftests/bpf/test_dev_cgroup.c | 85 -------------- 5 files changed, 125 insertions(+), 90 deletions(-)
base-commit: c90e2d4a7738a24fbb3657092dbd1ca20c040ed1 change-id: 20240723-convert_dev_cgroup-6464b0d37f1a
Best regards,
Alexis Lothoré, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Going forward, can you pls use [PATCH bpf-next] as a subject (or bpf when targeting bpf tree)? I'm not sure whether patchworks picks up plain [PATCH] messages..
On 7/27/24 00:49, Stanislav Fomichev wrote:
On 07/25, Alexis Lothoré (eBPF Foundation) wrote:
Hello, this small series aims to integrate test_dev_cgroup in test_progs so it could be run automatically in CI. The new version brings a few differences with the current one:
- test now uses directly syscalls instead of wrapping commandline tools into system() calls
- test_progs manipulates /dev/null (eg: redirecting test logs into it), so disabling access to it in the bpf program confuses the tests. To fix this, the first commit modifies the bpf program to allow access to char devices 1:3 (/dev/null), and disable access to char devices 1:5 (/dev/zero)
- once test is converted, add a small subtest to also check for device type interpretation (char or block)
- paths used in mknod tests are now in /dev instead of /tmp: due to the CI runner organisation and mountpoints manipulations, trying to create nodes in /tmp leads to errors unrelated to the test (ie, mknod calls refused by kernel, not the bpf program). I don't understand exactly the root cause at the deepest point (all I see in CI is an -ENXIO error on mknod when trying to create the node in tmp, and I can not make sense out of it neither replicate it locally), so I would gladly take inputs from anyone more educated than me about this.
[...]
Going forward, can you pls use [PATCH bpf-next] as a subject (or bpf when targeting bpf tree)? I'm not sure whether patchworks picks up plain [PATCH] messages..
Yes, my bad, I realized some time after sending that I may have missed some proper patch prefix. I have just checked on patchwork and see this series and the one I have sent before, so I guess there is no need to resend those, but I'll make sure to apply the relevant prefix for next series.
Thanks,
Alexis
linux-kselftest-mirror@lists.linaro.org
-
Alexis Lothoré -
Alexis Lothoré (eBPF Foundation)
-
Stanislav Fomichev