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(); + 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; + } + + 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); + 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) { + 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); + +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";
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";
On Wed, 19 Aug 2020 at 21:46, Yonghong Song yhs@fb.com wrote:
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?
Yup!
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.
I avoid CHECK because it requires `duration`, which doesn't make sense for most things that I call CHECK_FAIL on here. So either it outputs 0 nsec (which is bogus) or it outputs the value from the last bpf_prog_test_run call (which is also bogus). How do other tests handle this? Just ignore it?
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?
Hm, good point. This is a quirk of the sockmap API, I need to dig into this a bit.
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.
See above, re duration.
+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";
On 8/20/20 4:58 AM, Lorenz Bauer wrote:
On Wed, 19 Aug 2020 at 21:46, Yonghong Song yhs@fb.com wrote:
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?
Yup!
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.
I avoid CHECK because it requires `duration`, which doesn't make sense for most things that I call CHECK_FAIL on here. So either it outputs 0 nsec (which is bogus) or it outputs the value from the last bpf_prog_test_run call (which is also bogus). How do other tests handle this? Just ignore it?
Just ignore it. You can define a static variable duration in the beginning of file and then use CHECK in the rest of file.
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);
[...]
On Thu, 20 Aug 2020 at 15:49, Yonghong Song yhs@fb.com wrote:
On 8/20/20 4:58 AM, Lorenz Bauer wrote:
On Wed, 19 Aug 2020 at 21:46, Yonghong Song yhs@fb.com wrote:
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?
Yup!
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.
I avoid CHECK because it requires `duration`, which doesn't make sense for most things that I call CHECK_FAIL on here. So either it outputs 0 nsec (which is bogus) or it outputs the value from the last bpf_prog_test_run call (which is also bogus). How do other tests handle this? Just ignore it?
Just ignore it. You can define a static variable duration in the beginning of file and then use CHECK in the rest of file.
Ok, will do in v3!
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);
[...]
linux-kselftest-mirror@lists.linaro.org