Messages in this thread | | | Subject | Re: [PATCH bpf-next 6/6] selftests: bpf: test sockmap update from BPF | From | Yonghong Song <> | Date | Wed, 19 Aug 2020 13:45:31 -0700 |
| |
On 8/19/20 2:24 AM, Lorenz Bauer wrote: > Add a test which copies a socket from a sockmap into another sockmap > or sockhash. This excercises bpf_map_update_elem support from BPF > context. Compare the socket cookies from source and destination to > ensure that the copy succeeded. > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> > --- > .../selftests/bpf/prog_tests/sockmap_basic.c | 76 +++++++++++++++++++ > .../selftests/bpf/progs/test_sockmap_copy.c | 48 ++++++++++++ > 2 files changed, 124 insertions(+) > create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_copy.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > index 96e7b7f84c65..d30cabc00e9e 100644 > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > @@ -4,6 +4,7 @@ > > #include "test_progs.h" > #include "test_skmsg_load_helpers.skel.h" > +#include "test_sockmap_copy.skel.h" > > #define TCP_REPAIR 19 /* TCP sock is under repair right now */ > > @@ -101,6 +102,77 @@ static void test_skmsg_helpers(enum bpf_map_type map_type) > test_skmsg_load_helpers__destroy(skel); > } > > +static void test_sockmap_copy(enum bpf_map_type map_type) > +{ > + struct bpf_prog_test_run_attr attr; > + struct test_sockmap_copy *skel; > + __u64 src_cookie, dst_cookie; > + int err, prog, s, src, dst; > + const __u32 zero = 0; > + char dummy[14] = {0}; > + > + s = connected_socket_v4();
Maybe change variable name to "sk" for better clarity?
> + if (CHECK_FAIL(s == -1)) > + return; > + > + skel = test_sockmap_copy__open_and_load(); > + if (CHECK_FAIL(!skel)) { > + close(s); > + perror("test_sockmap_copy__open_and_load"); > + return; > + }
Could you use CHECK instead of CHECK_FAIL? With CHECK, you can print additional information without perror.
> + > + prog = bpf_program__fd(skel->progs.copy_sock_map); > + src = bpf_map__fd(skel->maps.src); > + if (map_type == BPF_MAP_TYPE_SOCKMAP) > + dst = bpf_map__fd(skel->maps.dst_sock_map); > + else > + dst = bpf_map__fd(skel->maps.dst_sock_hash); > + > + err = bpf_map_update_elem(src, &zero, &s, BPF_NOEXIST);
The map defined in bpf program is __u64 and here "s" is int. Any potential issues?
> + if (CHECK_FAIL(err)) { > + perror("bpf_map_update"); > + goto out; > + } > + > + err = bpf_map_lookup_elem(src, &zero, &src_cookie); > + if (CHECK_FAIL(err)) { > + perror("bpf_map_lookup_elem(src)"); > + goto out; > + } > + > + attr = (struct bpf_prog_test_run_attr){ > + .prog_fd = prog, > + .repeat = 1, > + .data_in = dummy, > + .data_size_in = sizeof(dummy), > + }; > + > + err = bpf_prog_test_run_xattr(&attr); > + if (err) {
You can use CHECK macro here.
> + test__fail(); > + perror("bpf_prog_test_run"); > + goto out; > + } else if (!attr.retval) { > + PRINT_FAIL("bpf_prog_test_run: program returned %u\n", > + attr.retval); > + goto out; > + } > + > + err = bpf_map_lookup_elem(dst, &zero, &dst_cookie); > + if (CHECK_FAIL(err)) { > + perror("bpf_map_lookup_elem(dst)"); > + goto out; > + } > + > + if (dst_cookie != src_cookie) > + PRINT_FAIL("cookie %llu != %llu\n", dst_cookie, src_cookie);
Just replace the whole if statement with a CHECK macro.
> + > +out: > + close(s); > + test_sockmap_copy__destroy(skel); > +} > + > void test_sockmap_basic(void) > { > if (test__start_subtest("sockmap create_update_free")) > @@ -111,4 +183,8 @@ void test_sockmap_basic(void) > test_skmsg_helpers(BPF_MAP_TYPE_SOCKMAP); > if (test__start_subtest("sockhash sk_msg load helpers")) > test_skmsg_helpers(BPF_MAP_TYPE_SOCKHASH); > + if (test__start_subtest("sockmap copy")) > + test_sockmap_copy(BPF_MAP_TYPE_SOCKMAP); > + if (test__start_subtest("sockhash copy")) > + test_sockmap_copy(BPF_MAP_TYPE_SOCKHASH); > } > diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_copy.c b/tools/testing/selftests/bpf/progs/test_sockmap_copy.c > new file mode 100644 > index 000000000000..9d0c9f28cab2 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_sockmap_copy.c > @@ -0,0 +1,48 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2020 Cloudflare > +#include "vmlinux.h" > +#include <bpf/bpf_helpers.h> > + > +struct { > + __uint(type, BPF_MAP_TYPE_SOCKMAP); > + __uint(max_entries, 1); > + __type(key, __u32); > + __type(value, __u64); > +} src SEC(".maps"); > + > +struct { > + __uint(type, BPF_MAP_TYPE_SOCKMAP); > + __uint(max_entries, 1); > + __type(key, __u32); > + __type(value, __u64); > +} dst_sock_map SEC(".maps"); > + > +struct { > + __uint(type, BPF_MAP_TYPE_SOCKHASH); > + __uint(max_entries, 1); > + __type(key, __u32); > + __type(value, __u64); > +} dst_sock_hash SEC(".maps"); > + > +SEC("classifier/copy_sock_map") > +int copy_sock_map(void *ctx) > +{ > + struct bpf_sock *sk; > + bool failed = false; > + __u32 key = 0; > + > + sk = bpf_map_lookup_elem(&src, &key); > + if (!sk) > + return SK_DROP; > + > + if (bpf_map_update_elem(&dst_sock_map, &key, sk, 0)) > + failed = true; > + > + if (bpf_map_update_elem(&dst_sock_hash, &key, sk, 0)) > + failed = true; > + > + bpf_sk_release(sk); > + return failed ? SK_DROP : SK_PASS; > +} > + > +char _license[] SEC("license") = "GPL"; >
| |