| Message ID | 160417035105.2823.2453428685023319711.stgit@localhost.localdomain |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | selftests/bpf: Migrate test_tcpbpf_user to be a part of test_progs framework | expand |
| Context | Check | Description |
|---|---|---|
| jkicinski/cover_letter | success | Link |
| jkicinski/fixes_present | success | Link |
| jkicinski/patch_count | success | Link |
| jkicinski/tree_selection | success | Clearly marked for bpf-next |
| jkicinski/subject_prefix | success | Link |
| jkicinski/source_inline | success | Was 0 now: 0 |
| jkicinski/verify_signedoff | success | Link |
| jkicinski/module_param | success | Was 0 now: 0 |
| jkicinski/build_32bit | fail | Errors and warnings before: 4 this patch: 4 |
| jkicinski/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/verify_fixes | success | Link |
| jkicinski/checkpatch | fail | Link |
| jkicinski/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/header_inline | success | Link |
| jkicinski/stable | success | Stable not CCed |
On Sat, Oct 31, 2020 at 11:52:31AM -0700, Alexander Duyck wrote: > From: Alexander Duyck <alexanderduyck@fb.com> > > Update tcpbpf_user.c to make use of the BPF skeleton. Doing this we can > simplify test_tcpbpf_user and reduce the overhead involved in setting up > the test. > > In addition we can clean up the remaining bits such as the one remaining > CHECK_FAIL at the end of test_tcpbpf_user so that the function only makes > use of CHECK as needed. > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: Alexander Duyck <alexanderduyck@fb.com> Acked-by: Martin KaFai Lau <kafai@fb.com> > --- > .../testing/selftests/bpf/prog_tests/tcpbpf_user.c | 48 ++++++++------------ > 1 file changed, 18 insertions(+), 30 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c > index d96f4084d2f5..c7a61b0d616a 100644 > --- a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c > +++ b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c > @@ -4,6 +4,7 @@ > #include <network_helpers.h> > > #include "test_tcpbpf.h" > +#include "test_tcpbpf_kern.skel.h" > > #define LO_ADDR6 "::1" > #define CG_NAME "/tcpbpf-user-test" > @@ -133,44 +134,31 @@ static void run_test(int map_fd, int sock_map_fd) > > void test_tcpbpf_user(void) > { > - const char *file = "test_tcpbpf_kern.o"; > - int prog_fd, map_fd, sock_map_fd; > - int error = EXIT_FAILURE; > - struct bpf_object *obj; > + struct test_tcpbpf_kern *skel; > + int map_fd, sock_map_fd; > int cg_fd = -1; > - int rv; > - > - cg_fd = test__join_cgroup(CG_NAME); > - if (cg_fd < 0) > - goto err; > > - if (bpf_prog_load(file, BPF_PROG_TYPE_SOCK_OPS, &obj, &prog_fd)) { > - fprintf(stderr, "FAILED: load_bpf_file failed for: %s\n", file); > - goto err; > - } > + skel = test_tcpbpf_kern__open_and_load(); > + if (CHECK(!skel, "open and load skel", "failed")) > + return; > > - rv = bpf_prog_attach(prog_fd, cg_fd, BPF_CGROUP_SOCK_OPS, 0); > - if (rv) { > - fprintf(stderr, "FAILED: bpf_prog_attach: %d (%s)\n", > - errno, strerror(errno)); > - goto err; > - } > + cg_fd = test__join_cgroup(CG_NAME); > + if (CHECK(cg_fd < 0, "test__join_cgroup(" CG_NAME ")", > + "cg_fd:%d errno:%d", cg_fd, errno)) > + goto cleanup_skel; > > - map_fd = bpf_find_map(__func__, obj, "global_map"); > - if (map_fd < 0) > - goto err; > + map_fd = bpf_map__fd(skel->maps.global_map); > + sock_map_fd = bpf_map__fd(skel->maps.sockopt_results); > > - sock_map_fd = bpf_find_map(__func__, obj, "sockopt_results"); > - if (sock_map_fd < 0) > - goto err; > + skel->links.bpf_testcb = bpf_program__attach_cgroup(skel->progs.bpf_testcb, cg_fd); > + if (ASSERT_OK_PTR(skel->links.bpf_testcb, "attach_cgroup(bpf_testcb)")) > + goto cleanup_namespace; > > run_test(map_fd, sock_map_fd); > > - error = 0; > -err: > - bpf_prog_detach(cg_fd, BPF_CGROUP_SOCK_OPS); > +cleanup_namespace: nit. may be "cleanup_cgroup" instead? or only have one jump label to handle failure since "cg_fd != -1" has been tested already. > if (cg_fd != -1) > close(cg_fd); > - > - CHECK_FAIL(error); > +cleanup_skel: > + test_tcpbpf_kern__destroy(skel); > } > >
On Mon, Nov 2, 2020 at 4:55 PM Martin KaFai Lau <kafai@fb.com> wrote: > > On Sat, Oct 31, 2020 at 11:52:31AM -0700, Alexander Duyck wrote: > > From: Alexander Duyck <alexanderduyck@fb.com> > > > > Update tcpbpf_user.c to make use of the BPF skeleton. Doing this we can > > simplify test_tcpbpf_user and reduce the overhead involved in setting up > > the test. > > > > In addition we can clean up the remaining bits such as the one remaining > > CHECK_FAIL at the end of test_tcpbpf_user so that the function only makes > > use of CHECK as needed. > > > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > Signed-off-by: Alexander Duyck <alexanderduyck@fb.com> > Acked-by: Martin KaFai Lau <kafai@fb.com> > > > --- > > .../testing/selftests/bpf/prog_tests/tcpbpf_user.c | 48 ++++++++------------ > > 1 file changed, 18 insertions(+), 30 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c > > index d96f4084d2f5..c7a61b0d616a 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c > > +++ b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c > > @@ -4,6 +4,7 @@ > > #include <network_helpers.h> > > > > #include "test_tcpbpf.h" > > +#include "test_tcpbpf_kern.skel.h" > > > > #define LO_ADDR6 "::1" > > #define CG_NAME "/tcpbpf-user-test" > > @@ -133,44 +134,31 @@ static void run_test(int map_fd, int sock_map_fd) > > > > void test_tcpbpf_user(void) > > { > > - const char *file = "test_tcpbpf_kern.o"; > > - int prog_fd, map_fd, sock_map_fd; > > - int error = EXIT_FAILURE; > > - struct bpf_object *obj; > > + struct test_tcpbpf_kern *skel; > > + int map_fd, sock_map_fd; > > int cg_fd = -1; > > - int rv; > > - > > - cg_fd = test__join_cgroup(CG_NAME); > > - if (cg_fd < 0) > > - goto err; > > > > - if (bpf_prog_load(file, BPF_PROG_TYPE_SOCK_OPS, &obj, &prog_fd)) { > > - fprintf(stderr, "FAILED: load_bpf_file failed for: %s\n", file); > > - goto err; > > - } > > + skel = test_tcpbpf_kern__open_and_load(); > > + if (CHECK(!skel, "open and load skel", "failed")) > > + return; > > > > - rv = bpf_prog_attach(prog_fd, cg_fd, BPF_CGROUP_SOCK_OPS, 0); > > - if (rv) { > > - fprintf(stderr, "FAILED: bpf_prog_attach: %d (%s)\n", > > - errno, strerror(errno)); > > - goto err; > > - } > > + cg_fd = test__join_cgroup(CG_NAME); > > + if (CHECK(cg_fd < 0, "test__join_cgroup(" CG_NAME ")", > > + "cg_fd:%d errno:%d", cg_fd, errno)) > > + goto cleanup_skel; > > > > - map_fd = bpf_find_map(__func__, obj, "global_map"); > > - if (map_fd < 0) > > - goto err; > > + map_fd = bpf_map__fd(skel->maps.global_map); > > + sock_map_fd = bpf_map__fd(skel->maps.sockopt_results); > > > > - sock_map_fd = bpf_find_map(__func__, obj, "sockopt_results"); > > - if (sock_map_fd < 0) > > - goto err; > > + skel->links.bpf_testcb = bpf_program__attach_cgroup(skel->progs.bpf_testcb, cg_fd); > > + if (ASSERT_OK_PTR(skel->links.bpf_testcb, "attach_cgroup(bpf_testcb)")) > > + goto cleanup_namespace; > > > > run_test(map_fd, sock_map_fd); > > > > - error = 0; > > -err: > > - bpf_prog_detach(cg_fd, BPF_CGROUP_SOCK_OPS); > > +cleanup_namespace: > nit. > > may be "cleanup_cgroup" instead? > > or only have one jump label to handle failure since "cg_fd != -1" has been > tested already. Good point. I can go through and just drop the second label and simplify this. Will fix for v3.
diff --git a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c index d96f4084d2f5..c7a61b0d616a 100644 --- a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c +++ b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c @@ -4,6 +4,7 @@ #include <network_helpers.h> #include "test_tcpbpf.h" +#include "test_tcpbpf_kern.skel.h" #define LO_ADDR6 "::1" #define CG_NAME "/tcpbpf-user-test" @@ -133,44 +134,31 @@ static void run_test(int map_fd, int sock_map_fd) void test_tcpbpf_user(void) { - const char *file = "test_tcpbpf_kern.o"; - int prog_fd, map_fd, sock_map_fd; - int error = EXIT_FAILURE; - struct bpf_object *obj; + struct test_tcpbpf_kern *skel; + int map_fd, sock_map_fd; int cg_fd = -1; - int rv; - - cg_fd = test__join_cgroup(CG_NAME); - if (cg_fd < 0) - goto err; - if (bpf_prog_load(file, BPF_PROG_TYPE_SOCK_OPS, &obj, &prog_fd)) { - fprintf(stderr, "FAILED: load_bpf_file failed for: %s\n", file); - goto err; - } + skel = test_tcpbpf_kern__open_and_load(); + if (CHECK(!skel, "open and load skel", "failed")) + return; - rv = bpf_prog_attach(prog_fd, cg_fd, BPF_CGROUP_SOCK_OPS, 0); - if (rv) { - fprintf(stderr, "FAILED: bpf_prog_attach: %d (%s)\n", - errno, strerror(errno)); - goto err; - } + cg_fd = test__join_cgroup(CG_NAME); + if (CHECK(cg_fd < 0, "test__join_cgroup(" CG_NAME ")", + "cg_fd:%d errno:%d", cg_fd, errno)) + goto cleanup_skel; - map_fd = bpf_find_map(__func__, obj, "global_map"); - if (map_fd < 0) - goto err; + map_fd = bpf_map__fd(skel->maps.global_map); + sock_map_fd = bpf_map__fd(skel->maps.sockopt_results); - sock_map_fd = bpf_find_map(__func__, obj, "sockopt_results"); - if (sock_map_fd < 0) - goto err; + skel->links.bpf_testcb = bpf_program__attach_cgroup(skel->progs.bpf_testcb, cg_fd); + if (ASSERT_OK_PTR(skel->links.bpf_testcb, "attach_cgroup(bpf_testcb)")) + goto cleanup_namespace; run_test(map_fd, sock_map_fd); - error = 0; -err: - bpf_prog_detach(cg_fd, BPF_CGROUP_SOCK_OPS); +cleanup_namespace: if (cg_fd != -1) close(cg_fd); - - CHECK_FAIL(error); +cleanup_skel: + test_tcpbpf_kern__destroy(skel); }