| Message ID | 20210326122438.211242-1-yauheni.kaliuta@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [v2,1/4] selftests/bpf: test_progs/sockopt_sk: Convert to use BPF skeleton | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Mar 26, 2021 at 5:24 AM Yauheni Kaliuta <yauheni.kaliuta@redhat.com> 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 <yauheni.kaliuta@redhat.com> > --- It's a nice cleanup. See some suggestion to further clean it up. But even with this: Acked-by: Andrii Nakryiko <andrii@kernel.org> > .../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 >
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; + + 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))) + 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()); -close_bpf_object: - bpf_object__close(obj); +cleanup: + sockopt_sk__destroy(skel); } void test_sockopt_sk(void)
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 <yauheni.kaliuta@redhat.com> --- .../selftests/bpf/prog_tests/sockopt_sk.c | 72 ++++++------------- 1 file changed, 23 insertions(+), 49 deletions(-)