* [PATCH bpf-next v2 0/2] Add more bpf_*_ct_lookup() selftests @ 2022-08-09 16:34 Daniel Xu 2022-08-09 16:34 ` [PATCH bpf-next v2 1/2] selftests/bpf: Add existing connection bpf_*_ct_lookup() test Daniel Xu 2022-08-09 16:34 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add connmark read test Daniel Xu 0 siblings, 2 replies; 5+ messages in thread From: Daniel Xu @ 2022-08-09 16:34 UTC (permalink / raw) To: bpf, ast, daniel, andrii, memxor; +Cc: Daniel Xu, linux-kernel This patchset adds more bpf_*_ct_lookup() selftests. The goal is to test interaction with netfilter subsystem as well as reading from `struct nf_conn`. The first is important when migrating legacy systems towards bpf. The latter is important in general to take full advantage of connection tracking. I'll follow this patchset up with support for writing to `struct nf_conn`. This change will require two changes to BPF CI kconfig: * CONFIG_NF_CONNTRACK_MARK=y * CONFIG_NETFILTER_XT_CONNMARK=y I can put up the PR if this patchset looks good. Past discussion: - v1: https://lore.kernel.org/bpf/cover.1659209738.git.dxu@dxuuu.xyz/ Changes since v1: - Reword commit message / cover letter to not mention connmark writing Daniel Xu (2): selftests/bpf: Add existing connection bpf_*_ct_lookup() test selftests/bpf: Add connmark read test .../testing/selftests/bpf/prog_tests/bpf_nf.c | 60 +++++++++++++++++++ .../testing/selftests/bpf/progs/test_bpf_nf.c | 21 +++++++ 2 files changed, 81 insertions(+) -- 2.37.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH bpf-next v2 1/2] selftests/bpf: Add existing connection bpf_*_ct_lookup() test 2022-08-09 16:34 [PATCH bpf-next v2 0/2] Add more bpf_*_ct_lookup() selftests Daniel Xu @ 2022-08-09 16:34 ` Daniel Xu 2022-08-09 16:34 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add connmark read test Daniel Xu 1 sibling, 0 replies; 5+ messages in thread From: Daniel Xu @ 2022-08-09 16:34 UTC (permalink / raw) To: bpf, ast, daniel, andrii, memxor; +Cc: Daniel Xu, linux-kernel Add a test where we do a conntrack lookup on an existing connection. This is nice because it's a more realistic test than artifically creating a ct entry and looking it up afterwards. Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> --- .../testing/selftests/bpf/prog_tests/bpf_nf.c | 59 +++++++++++++++++++ .../testing/selftests/bpf/progs/test_bpf_nf.c | 18 ++++++ 2 files changed, 77 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c index 7a74a1579076..317978cac029 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c @@ -24,10 +24,34 @@ enum { TEST_TC_BPF, }; +#define TIMEOUT_MS 3000 + +static int connect_to_server(int srv_fd) +{ + int fd = -1; + + fd = socket(AF_INET, SOCK_STREAM, 0); + if (!ASSERT_GE(fd, 0, "socket")) + goto out; + + if (CHECK_FAIL(connect_fd_to_fd(fd, srv_fd, TIMEOUT_MS))) { + close(fd); + fd = -1; + } +out: + return fd; +} + static void test_bpf_nf_ct(int mode) { + const char *iptables = "iptables -t raw %s PREROUTING -j CT"; + int srv_fd = -1, client_fd = -1, srv_client_fd = -1; + struct sockaddr_in peer_addr = {}; struct test_bpf_nf *skel; int prog_fd, err; + socklen_t len; + u16 srv_port; + char cmd[64]; LIBBPF_OPTS(bpf_test_run_opts, topts, .data_in = &pkt_v4, .data_size_in = sizeof(pkt_v4), @@ -38,6 +62,32 @@ static void test_bpf_nf_ct(int mode) if (!ASSERT_OK_PTR(skel, "test_bpf_nf__open_and_load")) return; + /* Enable connection tracking */ + snprintf(cmd, sizeof(cmd), iptables, "-A"); + if (!ASSERT_OK(system(cmd), "iptables")) + goto end; + + srv_port = (mode == TEST_XDP) ? 5005 : 5006; + srv_fd = start_server(AF_INET, SOCK_STREAM, "127.0.0.1", srv_port, TIMEOUT_MS); + if (!ASSERT_GE(srv_fd, 0, "start_server")) + goto end; + + client_fd = connect_to_server(srv_fd); + if (!ASSERT_GE(client_fd, 0, "connect_to_server")) + goto end; + + len = sizeof(peer_addr); + srv_client_fd = accept(srv_fd, (struct sockaddr *)&peer_addr, &len); + if (!ASSERT_GE(srv_client_fd, 0, "accept")) + goto end; + if (!ASSERT_EQ(len, sizeof(struct sockaddr_in), "sockaddr len")) + goto end; + + skel->bss->saddr = peer_addr.sin_addr.s_addr; + skel->bss->sport = peer_addr.sin_port; + skel->bss->daddr = peer_addr.sin_addr.s_addr; + skel->bss->dport = htons(srv_port); + if (mode == TEST_XDP) prog_fd = bpf_program__fd(skel->progs.nf_xdp_ct_test); else @@ -63,7 +113,16 @@ static void test_bpf_nf_ct(int mode) ASSERT_LE(skel->bss->test_delta_timeout, 10, "Test for max ct timeout update"); /* expected status is IPS_SEEN_REPLY */ ASSERT_EQ(skel->bss->test_status, 2, "Test for ct status update "); + ASSERT_EQ(skel->data->test_exist_lookup, 0, "Test existing connection lookup"); end: + if (srv_client_fd != -1) + close(srv_client_fd); + if (client_fd != -1) + close(client_fd); + if (srv_fd != -1) + close(srv_fd); + snprintf(cmd, sizeof(cmd), iptables, "-D"); + system(cmd); test_bpf_nf__destroy(skel); } diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c index 196cd8dfe42a..84e0fd479794 100644 --- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c +++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c @@ -23,6 +23,11 @@ int test_insert_entry = -EAFNOSUPPORT; int test_succ_lookup = -ENOENT; u32 test_delta_timeout = 0; u32 test_status = 0; +__be32 saddr = 0; +__be16 sport = 0; +__be32 daddr = 0; +__be16 dport = 0; +int test_exist_lookup = -ENOENT; struct nf_conn; @@ -160,6 +165,19 @@ nf_ct_test(struct nf_conn *(*lookup_fn)(void *, struct bpf_sock_tuple *, u32, } test_alloc_entry = 0; } + + bpf_tuple.ipv4.saddr = saddr; + bpf_tuple.ipv4.daddr = daddr; + bpf_tuple.ipv4.sport = sport; + bpf_tuple.ipv4.dport = dport; + ct = lookup_fn(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def, + sizeof(opts_def)); + if (ct) { + test_exist_lookup = 0; + bpf_ct_release(ct); + } else { + test_exist_lookup = opts_def.error; + } } SEC("xdp") -- 2.37.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH bpf-next v2 2/2] selftests/bpf: Add connmark read test 2022-08-09 16:34 [PATCH bpf-next v2 0/2] Add more bpf_*_ct_lookup() selftests Daniel Xu 2022-08-09 16:34 ` [PATCH bpf-next v2 1/2] selftests/bpf: Add existing connection bpf_*_ct_lookup() test Daniel Xu @ 2022-08-09 16:34 ` Daniel Xu 2022-08-09 21:14 ` Daniel Borkmann 1 sibling, 1 reply; 5+ messages in thread From: Daniel Xu @ 2022-08-09 16:34 UTC (permalink / raw) To: bpf, ast, daniel, andrii, memxor; +Cc: Daniel Xu, linux-kernel Test that the prog can read from the connection mark. This test is nice because it ensures progs can interact with netfilter subsystem correctly. Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> --- tools/testing/selftests/bpf/prog_tests/bpf_nf.c | 3 ++- tools/testing/selftests/bpf/progs/test_bpf_nf.c | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c index 317978cac029..7232f6dcd252 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c @@ -44,7 +44,7 @@ static int connect_to_server(int srv_fd) static void test_bpf_nf_ct(int mode) { - const char *iptables = "iptables -t raw %s PREROUTING -j CT"; + const char *iptables = "iptables -t raw %s PREROUTING -j CONNMARK --set-mark 42/0"; int srv_fd = -1, client_fd = -1, srv_client_fd = -1; struct sockaddr_in peer_addr = {}; struct test_bpf_nf *skel; @@ -114,6 +114,7 @@ static void test_bpf_nf_ct(int mode) /* expected status is IPS_SEEN_REPLY */ ASSERT_EQ(skel->bss->test_status, 2, "Test for ct status update "); ASSERT_EQ(skel->data->test_exist_lookup, 0, "Test existing connection lookup"); + ASSERT_EQ(skel->bss->test_exist_lookup_mark, 43, "Test existing connection lookup ctmark"); end: if (srv_client_fd != -1) close(srv_client_fd); diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c index 84e0fd479794..2722441850cc 100644 --- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c +++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c @@ -28,6 +28,7 @@ __be16 sport = 0; __be32 daddr = 0; __be16 dport = 0; int test_exist_lookup = -ENOENT; +u32 test_exist_lookup_mark = 0; struct nf_conn; @@ -174,6 +175,8 @@ nf_ct_test(struct nf_conn *(*lookup_fn)(void *, struct bpf_sock_tuple *, u32, sizeof(opts_def)); if (ct) { test_exist_lookup = 0; + if (ct->mark == 42) + test_exist_lookup_mark = 43; bpf_ct_release(ct); } else { test_exist_lookup = opts_def.error; -- 2.37.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add connmark read test 2022-08-09 16:34 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add connmark read test Daniel Xu @ 2022-08-09 21:14 ` Daniel Borkmann 2022-08-10 0:20 ` Daniel Xu 0 siblings, 1 reply; 5+ messages in thread From: Daniel Borkmann @ 2022-08-09 21:14 UTC (permalink / raw) To: Daniel Xu, bpf, ast, andrii, memxor; +Cc: linux-kernel Hi Daniel, On 8/9/22 6:34 PM, Daniel Xu wrote: > Test that the prog can read from the connection mark. This test is nice > because it ensures progs can interact with netfilter subsystem > correctly. > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > --- > tools/testing/selftests/bpf/prog_tests/bpf_nf.c | 3 ++- > tools/testing/selftests/bpf/progs/test_bpf_nf.c | 3 +++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c > index 317978cac029..7232f6dcd252 100644 > --- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c > @@ -44,7 +44,7 @@ static int connect_to_server(int srv_fd) > > static void test_bpf_nf_ct(int mode) > { > - const char *iptables = "iptables -t raw %s PREROUTING -j CT"; > + const char *iptables = "iptables -t raw %s PREROUTING -j CONNMARK --set-mark 42/0"; > int srv_fd = -1, client_fd = -1, srv_client_fd = -1; > struct sockaddr_in peer_addr = {}; > struct test_bpf_nf *skel; > @@ -114,6 +114,7 @@ static void test_bpf_nf_ct(int mode) > /* expected status is IPS_SEEN_REPLY */ > ASSERT_EQ(skel->bss->test_status, 2, "Test for ct status update "); > ASSERT_EQ(skel->data->test_exist_lookup, 0, "Test existing connection lookup"); > + ASSERT_EQ(skel->bss->test_exist_lookup_mark, 43, "Test existing connection lookup ctmark"); > end: > if (srv_client_fd != -1) > close(srv_client_fd); > diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c > index 84e0fd479794..2722441850cc 100644 > --- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c > +++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c > @@ -28,6 +28,7 @@ __be16 sport = 0; > __be32 daddr = 0; > __be16 dport = 0; > int test_exist_lookup = -ENOENT; > +u32 test_exist_lookup_mark = 0; > > struct nf_conn; > > @@ -174,6 +175,8 @@ nf_ct_test(struct nf_conn *(*lookup_fn)(void *, struct bpf_sock_tuple *, u32, > sizeof(opts_def)); > if (ct) { > test_exist_lookup = 0; > + if (ct->mark == 42) > + test_exist_lookup_mark = 43; Looks like CI failed here: [...] progs/test_bpf_nf.c:178:11: error: no member named 'mark' in 'struct nf_conn' if (ct->mark == 42) ~~ ^ 1 error generated. make: *** [Makefile:521: /tmp/runner/work/bpf/bpf/tools/testing/selftests/bpf/test_bpf_nf.o] Error 1 make: *** Waiting for unfinished jobs.... Error: Process completed with exit code 2. Likely due to missing CONFIG_NF_CONNTRACK_MARK for the CI instance. > bpf_ct_release(ct); > } else { > test_exist_lookup = opts_def.error; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add connmark read test 2022-08-09 21:14 ` Daniel Borkmann @ 2022-08-10 0:20 ` Daniel Xu 0 siblings, 0 replies; 5+ messages in thread From: Daniel Xu @ 2022-08-10 0:20 UTC (permalink / raw) To: Daniel Borkmann, bpf@vger.kernel.org, Alexei Starovoitov, Andrii Nakryiko, Kumar Kartikeya Dwivedi Cc: linux-kernel Hi Daniel, On Tue, Aug 9, 2022, at 3:14 PM, Daniel Borkmann wrote: > Hi Daniel, > > On 8/9/22 6:34 PM, Daniel Xu wrote: >> Test that the prog can read from the connection mark. This test is nice >> because it ensures progs can interact with netfilter subsystem >> correctly. >> >> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> >> --- >> tools/testing/selftests/bpf/prog_tests/bpf_nf.c | 3 ++- >> tools/testing/selftests/bpf/progs/test_bpf_nf.c | 3 +++ >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c >> index 317978cac029..7232f6dcd252 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c >> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c >> @@ -44,7 +44,7 @@ static int connect_to_server(int srv_fd) >> >> static void test_bpf_nf_ct(int mode) >> { >> - const char *iptables = "iptables -t raw %s PREROUTING -j CT"; >> + const char *iptables = "iptables -t raw %s PREROUTING -j CONNMARK --set-mark 42/0"; >> int srv_fd = -1, client_fd = -1, srv_client_fd = -1; >> struct sockaddr_in peer_addr = {}; >> struct test_bpf_nf *skel; >> @@ -114,6 +114,7 @@ static void test_bpf_nf_ct(int mode) >> /* expected status is IPS_SEEN_REPLY */ >> ASSERT_EQ(skel->bss->test_status, 2, "Test for ct status update "); >> ASSERT_EQ(skel->data->test_exist_lookup, 0, "Test existing connection lookup"); >> + ASSERT_EQ(skel->bss->test_exist_lookup_mark, 43, "Test existing connection lookup ctmark"); >> end: >> if (srv_client_fd != -1) >> close(srv_client_fd); >> diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c >> index 84e0fd479794..2722441850cc 100644 >> --- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c >> +++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c >> @@ -28,6 +28,7 @@ __be16 sport = 0; >> __be32 daddr = 0; >> __be16 dport = 0; >> int test_exist_lookup = -ENOENT; >> +u32 test_exist_lookup_mark = 0; >> >> struct nf_conn; >> >> @@ -174,6 +175,8 @@ nf_ct_test(struct nf_conn *(*lookup_fn)(void *, struct bpf_sock_tuple *, u32, >> sizeof(opts_def)); >> if (ct) { >> test_exist_lookup = 0; >> + if (ct->mark == 42) >> + test_exist_lookup_mark = 43; > > Looks like CI failed here: > > [...] > progs/test_bpf_nf.c:178:11: error: no member named 'mark' in 'struct > nf_conn' > if (ct->mark == 42) > ~~ ^ > 1 error generated. > make: *** [Makefile:521: > /tmp/runner/work/bpf/bpf/tools/testing/selftests/bpf/test_bpf_nf.o] > Error 1 > make: *** Waiting for unfinished jobs.... > Error: Process completed with exit code 2. > > Likely due to missing CONFIG_NF_CONNTRACK_MARK for the CI instance. Originally (as stated in the cover letter) I thought the CI kconfig was hosted somewhere else. Looking closer I see the kconfigs are checked into the selftest tree. I think the following should fix the CI. I'll send out a v3 tomorrow morning: diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config index fabf0c014349..3fc46f9cfb22 100644 --- a/tools/testing/selftests/bpf/config +++ b/tools/testing/selftests/bpf/config @@ -50,9 +50,11 @@ CONFIG_NET_SCHED=y CONFIG_NETDEVSIM=m CONFIG_NETFILTER=y CONFIG_NETFILTER_SYNPROXY=y +CONFIG_NETFILTER_XT_CONNMARK=y CONFIG_NETFILTER_XT_MATCH_STATE=y CONFIG_NETFILTER_XT_TARGET_CT=y CONFIG_NF_CONNTRACK=y +CONFIG_NF_CONNTRACK_MARK=y CONFIG_NF_DEFRAG_IPV4=y CONFIG_NF_DEFRAG_IPV6=y CONFIG_RC_CORE=y [...] Thanks, Daniel ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-08-10 0:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-08-09 16:34 [PATCH bpf-next v2 0/2] Add more bpf_*_ct_lookup() selftests Daniel Xu 2022-08-09 16:34 ` [PATCH bpf-next v2 1/2] selftests/bpf: Add existing connection bpf_*_ct_lookup() test Daniel Xu 2022-08-09 16:34 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add connmark read test Daniel Xu 2022-08-09 21:14 ` Daniel Borkmann 2022-08-10 0:20 ` Daniel Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox