| Message ID | d3bc620a491e4c626c20d80631063922cbe13e2b.1660254747.git.dxu@dxuuu.xyz (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 99799de2cba2d399acf65f49a986b3d5cf0732ab |
| Delegated to: | BPF |
| Headers | show |
| Series | Add more bpf_*_ct_lookup() selftests | expand |
On 8/11/22 2:55 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> > Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > --- > 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 88a2c0bdefec..544bf90ac2a7 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"; Hi Daniel Xu, this test starts failing recently in CI [0]: Warning: Extension CONNMARK revision 0 not supported, missing kernel module? iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: Invalid argument Warning: Extension CONNMARK revision 0 not supported, missing kernel module? iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: Invalid argument Warning: Extension CONNMARK revision 0 not supported, missing kernel module? iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: Invalid argument Warning: Extension CONNMARK revision 0 not supported, missing kernel module? iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: Invalid argument test_bpf_nf_ct:PASS:test_bpf_nf__open_and_load 0 nsec test_bpf_nf_ct:FAIL:iptables unexpected error: 1024 (errno 0) Could you help to take a look? Thanks. [0]: https://github.com/kernel-patches/bpf/actions/runs/3231598391/jobs/5291529292 > 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;
Hi Martin, On Tue, Oct 11, 2022 at 10:49:32PM -0700, Martin KaFai Lau wrote: > On 8/11/22 2:55 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> > > Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > --- > > 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 88a2c0bdefec..544bf90ac2a7 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"; > Hi Daniel Xu, this test starts failing recently in CI [0]: > > Warning: Extension CONNMARK revision 0 not supported, missing kernel module? > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > Invalid argument > > Warning: Extension CONNMARK revision 0 not supported, missing kernel module? > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > Invalid argument > > Warning: Extension CONNMARK revision 0 not supported, missing kernel module? > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > Invalid argument > > Warning: Extension CONNMARK revision 0 not supported, missing kernel module? > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > Invalid argument > > test_bpf_nf_ct:PASS:test_bpf_nf__open_and_load 0 nsec > test_bpf_nf_ct:FAIL:iptables unexpected error: 1024 (errno 0) > > Could you help to take a look? Thanks. > > [0]: https://github.com/kernel-patches/bpf/actions/runs/3231598391/jobs/5291529292 [...] Thanks for letting me know. I took a quick look and it seems that synproxy selftest is also failing: 2022-10-12T03:14:20.2007627Z test_synproxy:FAIL:iptables -t raw -I PREROUTING -i tmp1 -p tcp -m tcp --syn --dport 8080 -j CT --notrack unexpected error: 1024 (errno 2) Googling the "Could not fetch rule set generation id" yields a lot of hits. Most of the links are from downstream projects recommending user downgrade iptables (nftables) to iptables-legacy. So perhaps iptables/nftables suffered a regression somewhere. I'll take a closer look tonight / tomorrow morning. Thanks, Daniel
Daniel Xu <dxu@dxuuu.xyz> wrote: > > Warning: Extension CONNMARK revision 0 not supported, missing kernel module? > > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > > Invalid argument Martin, can you give result of modinfo xt_CONNMARK and modinfo nft_compat? I suspect your kernel lacks nf_tables support. > > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > > Invalid argument Probably a kernel without nftables support? > So perhaps iptables/nftables suffered a regression somewhere. I'll take > a closer look tonight / tomorrow morning. Possible but unlikely, all those tests pass for me.
On 10/12/22 3:09 PM, Daniel Xu wrote: > Hi Martin, > > On Tue, Oct 11, 2022 at 10:49:32PM -0700, Martin KaFai Lau wrote: >> On 8/11/22 2:55 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> >>> Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> >>> --- >>> 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 88a2c0bdefec..544bf90ac2a7 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"; >> Hi Daniel Xu, this test starts failing recently in CI [0]: >> >> Warning: Extension CONNMARK revision 0 not supported, missing kernel module? >> iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: >> Invalid argument >> >> Warning: Extension CONNMARK revision 0 not supported, missing kernel module? >> iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: >> Invalid argument >> >> Warning: Extension CONNMARK revision 0 not supported, missing kernel module? >> iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: >> Invalid argument >> >> Warning: Extension CONNMARK revision 0 not supported, missing kernel module? >> iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: >> Invalid argument >> >> test_bpf_nf_ct:PASS:test_bpf_nf__open_and_load 0 nsec >> test_bpf_nf_ct:FAIL:iptables unexpected error: 1024 (errno 0) >> >> Could you help to take a look? Thanks. >> >> [0]: https://github.com/kernel-patches/bpf/actions/runs/3231598391/jobs/5291529292 > > [...] > > Thanks for letting me know. I took a quick look and it seems that > synproxy selftest is also failing: > > 2022-10-12T03:14:20.2007627Z test_synproxy:FAIL:iptables -t raw -I PREROUTING -i tmp1 -p tcp -m tcp --syn --dport 8080 -j CT --notrack unexpected error: 1024 (errno 2) > > Googling the "Could not fetch rule set generation id" yields a lot of > hits. Most of the links are from downstream projects recommending user > downgrade iptables (nftables) to iptables-legacy. Thanks for looking into it! We have been debugging a bit today also. I also think iptables-legacy is the one to use. I posted a patch [0]. Let see how the CI goes. The rules that the selftest used is not a lot. I wonder what it takes to remove the iptables command usage from the selftest? [0]: https://lore.kernel.org/bpf/20221012221235.3529719-1-martin.lau@linux.dev/ > > So perhaps iptables/nftables suffered a regression somewhere. I'll take > a closer look tonight / tomorrow morning. > > Thanks, > Daniel
On Wed, Oct 12, 2022 at 03:20:01PM -0700, Martin KaFai Lau wrote: > On 10/12/22 3:09 PM, Daniel Xu wrote: > > Hi Martin, > > > > On Tue, Oct 11, 2022 at 10:49:32PM -0700, Martin KaFai Lau wrote: > > > On 8/11/22 2:55 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> > > > > Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > > > --- > > > > 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 88a2c0bdefec..544bf90ac2a7 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"; > > > Hi Daniel Xu, this test starts failing recently in CI [0]: > > > > > > Warning: Extension CONNMARK revision 0 not supported, missing kernel module? > > > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > > > Invalid argument > > > > > > Warning: Extension CONNMARK revision 0 not supported, missing kernel module? > > > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > > > Invalid argument > > > > > > Warning: Extension CONNMARK revision 0 not supported, missing kernel module? > > > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > > > Invalid argument > > > > > > Warning: Extension CONNMARK revision 0 not supported, missing kernel module? > > > iptables v1.8.8 (nf_tables): Could not fetch rule set generation id: > > > Invalid argument > > > > > > test_bpf_nf_ct:PASS:test_bpf_nf__open_and_load 0 nsec > > > test_bpf_nf_ct:FAIL:iptables unexpected error: 1024 (errno 0) > > > > > > Could you help to take a look? Thanks. > > > > > > [0]: https://github.com/kernel-patches/bpf/actions/runs/3231598391/jobs/5291529292 > > > > [...] > > > > Thanks for letting me know. I took a quick look and it seems that > > synproxy selftest is also failing: > > > > 2022-10-12T03:14:20.2007627Z test_synproxy:FAIL:iptables -t raw -I PREROUTING -i tmp1 -p tcp -m tcp --syn --dport 8080 -j CT --notrack unexpected error: 1024 (errno 2) > > > > Googling the "Could not fetch rule set generation id" yields a lot of > > hits. Most of the links are from downstream projects recommending user > > downgrade iptables (nftables) to iptables-legacy. > > Thanks for looking into it! We have been debugging a bit today also. I > also think iptables-legacy is the one to use. I posted a patch [0]. Let > see how the CI goes. > > The rules that the selftest used is not a lot. I wonder what it takes to > remove the iptables command usage from the selftest? At least the conntrack mark stuff, it would've been easier to write the selftests _without_ iptables. But I thought it was both good and necessary to test interop between BPF and netfilter. B/c that is what the user is doing (at least for me). However if it's causing maintenance trouble, I'll leave that call to you. Thanks, Daniel
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c index 88a2c0bdefec..544bf90ac2a7 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;