Re: [PATCH v2 1/4] selftests/bpf: test_progs/sockopt_sk: Convert to use BPF skeleton
From: Andrii Nakryiko <hidden>
Date: 2021-03-28 04:59:57
On Fri, Mar 26, 2021 at 5:24 AM Yauheni Kaliuta [off-list ref] wrote:
Switch the test to use BPF skeleton to save some boilerplate and make it easy to access bpf program bss segment. The latter will be used to pass PAGE_SIZE from userspace since there is no convenient way for bpf program to get it from inside of the kernel. Signed-off-by: Yauheni Kaliuta <redacted> ---
It's a nice cleanup. See some suggestion to further clean it up. But even with this: Acked-by: Andrii Nakryiko <andrii@kernel.org>
quoted hunk ↗ jump to hunk
.../selftests/bpf/prog_tests/sockopt_sk.c | 72 ++++++------------- 1 file changed, 23 insertions(+), 49 deletions(-)diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c index d5b44b135c00..114c1a622ffa 100644 --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c@@ -3,6 +3,7 @@ #include "cgroup_helpers.h" #include <linux/tcp.h> +#include "sockopt_sk.skel.h" #ifndef SOL_TCP #define SOL_TCP IPPROTO_TCP@@ -191,60 +192,33 @@ static int getsetsockopt(void) return -1; } -static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title) -{ - enum bpf_attach_type attach_type; - enum bpf_prog_type prog_type; - struct bpf_program *prog; - int err; - - err = libbpf_prog_type_by_name(title, &prog_type, &attach_type); - if (err) { - log_err("Failed to deduct types for %s BPF program", title); - return -1; - } - - prog = bpf_object__find_program_by_title(obj, title); - if (!prog) { - log_err("Failed to find %s BPF program", title); - return -1; - } - - err = bpf_prog_attach(bpf_program__fd(prog), cgroup_fd, - attach_type, 0); - if (err) { - log_err("Failed to attach %s BPF program", title); - return -1; - } - - return 0; -} - static void run_test(int cgroup_fd) { - struct bpf_prog_load_attr attr = { - .file = "./sockopt_sk.o", - }; - struct bpf_object *obj; - int ignored; - int err; - - err = bpf_prog_load_xattr(&attr, &obj, &ignored); - if (CHECK_FAIL(err)) - return; - - err = prog_attach(obj, cgroup_fd, "cgroup/getsockopt"); - if (CHECK_FAIL(err)) - goto close_bpf_object; - - err = prog_attach(obj, cgroup_fd, "cgroup/setsockopt"); - if (CHECK_FAIL(err)) - goto close_bpf_object; + struct sockopt_sk *skel; + int duration = 0; + + skel = sockopt_sk__open_and_load(); + if (CHECK(!skel, "skel_load", "sockopt_sk skeleton failed\n")) + goto cleanup;
if (!ASSERT_OK_PTR(skel, "skel_load)) is still less boilerplate :)
+ + skel->links._setsockopt = + bpf_program__attach_cgroup(skel->progs._setsockopt, cgroup_fd); + if (CHECK(IS_ERR(skel->links._setsockopt), + "setsockopt_attach", "err: %ld\n", + PTR_ERR(skel->links._setsockopt)))
you could save some more boilerplate if you did:
if (!ASSERT_OK_PTR(skel->links._getsockopt), "getsockopt_link")
goto cleanup;
+ goto cleanup;
+
+ skel->links._getsockopt =
+ bpf_program__attach_cgroup(skel->progs._getsockopt, cgroup_fd);
+ if (CHECK(IS_ERR(skel->links._getsockopt),
+ "getsockopt_attach", "err: %ld\n",
+ PTR_ERR(skel->links._getsockopt)))
+ goto cleanup;
CHECK_FAIL(getsetsockopt());please switch this to ASSERT_OK() as well. Once you don't use CHECK/CHECK_FAIL, you also won't need `int duration`, btw.
-close_bpf_object: - bpf_object__close(obj); +cleanup: + sockopt_sk__destroy(skel); } void test_sockopt_sk(void) -- 2.29.2