bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests
@ 2022-08-11 21:55 Daniel Xu
  2022-08-11 21:55 ` [PATCH bpf-next v4 1/3] selftests/bpf: Add existing connection bpf_*_ct_lookup() test Daniel Xu
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Daniel Xu @ 2022-08-11 21:55 UTC (permalink / raw)
  To: bpf, ast, daniel, andrii, memxor
  Cc: Daniel Xu, pablo, fw, netfilter-devel, netdev, 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`.

Past discussion:
- v3: https://lore.kernel.org/bpf/cover.1660173222.git.dxu@dxuuu.xyz/
- v2: https://lore.kernel.org/bpf/cover.1660062725.git.dxu@dxuuu.xyz/
- v1: https://lore.kernel.org/bpf/cover.1659209738.git.dxu@dxuuu.xyz/

Changes since v3:
- Remove deprecated CHECK_FAIL() usage
- cc netfilter folks

Changes since v2:
- Add bpf-ci kconfig changes

Changes since v1:
- Reword commit message / cover letter to not mention connmark writing

Daniel Xu (3):
  selftests/bpf: Add existing connection bpf_*_ct_lookup() test
  selftests/bpf: Add connmark read test
  selftests/bpf: Update CI kconfig

 tools/testing/selftests/bpf/config            |  2 +
 .../testing/selftests/bpf/prog_tests/bpf_nf.c | 60 +++++++++++++++++++
 .../testing/selftests/bpf/progs/test_bpf_nf.c | 21 +++++++
 3 files changed, 83 insertions(+)

-- 
2.37.1


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH bpf-next v4 1/3] selftests/bpf: Add existing connection bpf_*_ct_lookup() test
  2022-08-11 21:55 [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests Daniel Xu
@ 2022-08-11 21:55 ` Daniel Xu
  2022-08-11 21:55 ` [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test Daniel Xu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Daniel Xu @ 2022-08-11 21:55 UTC (permalink / raw)
  To: bpf, ast, daniel, andrii, memxor
  Cc: Daniel Xu, pablo, fw, netfilter-devel, netdev, 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>
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../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..88a2c0bdefec 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 (!ASSERT_EQ(connect_fd_to_fd(fd, srv_fd, TIMEOUT_MS), 0, "connect_fd_to_fd")) {
+		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] 10+ messages in thread
* [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test
  2022-08-11 21:55 [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests Daniel Xu
  2022-08-11 21:55 ` [PATCH bpf-next v4 1/3] selftests/bpf: Add existing connection bpf_*_ct_lookup() test Daniel Xu
@ 2022-08-11 21:55 ` Daniel Xu
  2022-10-12  5:49   ` Martin KaFai Lau
  2022-08-11 21:55 ` [PATCH bpf-next v4 3/3] selftests/bpf: Update CI kconfig Daniel Xu
  2022-08-15 19:00 ` [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests patchwork-bot+netdevbpf
  3 siblings, 1 reply; 10+ messages in thread
From: Daniel Xu @ 2022-08-11 21:55 UTC (permalink / raw)
  To: bpf, ast, daniel, andrii, memxor
  Cc: Daniel Xu, pablo, fw, netfilter-devel, netdev, 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>
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";
 	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] 10+ messages in thread
* [PATCH bpf-next v4 3/3] selftests/bpf: Update CI kconfig
  2022-08-11 21:55 [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests Daniel Xu
  2022-08-11 21:55 ` [PATCH bpf-next v4 1/3] selftests/bpf: Add existing connection bpf_*_ct_lookup() test Daniel Xu
  2022-08-11 21:55 ` [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test Daniel Xu
@ 2022-08-11 21:55 ` Daniel Xu
  2022-08-15 19:00 ` [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests patchwork-bot+netdevbpf
  3 siblings, 0 replies; 10+ messages in thread
From: Daniel Xu @ 2022-08-11 21:55 UTC (permalink / raw)
  To: bpf, ast, daniel, andrii, memxor
  Cc: Daniel Xu, pablo, fw, netfilter-devel, netdev, linux-kernel

The previous selftest changes require two kconfig changes in bpf-ci.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 tools/testing/selftests/bpf/config | 2 ++
 1 file changed, 2 insertions(+)

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
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests
  2022-08-11 21:55 [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests Daniel Xu
                   ` (2 preceding siblings ...)
  2022-08-11 21:55 ` [PATCH bpf-next v4 3/3] selftests/bpf: Update CI kconfig Daniel Xu
@ 2022-08-15 19:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-08-15 19:00 UTC (permalink / raw)
  To: Daniel Xu
  Cc: bpf, ast, daniel, andrii, memxor, pablo, fw, netfilter-devel,
	netdev, linux-kernel

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Thu, 11 Aug 2022 15:55:24 -0600 you wrote:
> 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`.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v4,1/3] selftests/bpf: Add existing connection bpf_*_ct_lookup() test
    https://git.kernel.org/bpf/bpf-next/c/e81fbd4c1ba7
  - [bpf-next,v4,2/3] selftests/bpf: Add connmark read test
    https://git.kernel.org/bpf/bpf-next/c/99799de2cba2
  - [bpf-next,v4,3/3] selftests/bpf: Update CI kconfig
    https://git.kernel.org/bpf/bpf-next/c/8308bf207ce6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test
  2022-08-11 21:55 ` [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test Daniel Xu
@ 2022-10-12  5:49   ` Martin KaFai Lau
  2022-10-12 22:09     ` Daniel Xu
  0 siblings, 1 reply; 10+ messages in thread
From: Martin KaFai Lau @ 2022-10-12  5:49 UTC (permalink / raw)
  To: Daniel Xu
  Cc: pablo, fw, netfilter-devel, netdev, linux-kernel, andrii, daniel,
	ast, bpf, memxor

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;


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test
  2022-10-12  5:49   ` Martin KaFai Lau
@ 2022-10-12 22:09     ` Daniel Xu
  2022-10-12 22:18       ` Florian Westphal
  2022-10-12 22:20       ` Martin KaFai Lau
  0 siblings, 2 replies; 10+ messages in thread
From: Daniel Xu @ 2022-10-12 22:09 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: pablo, fw, netfilter-devel, netdev, linux-kernel, andrii, daniel,
	ast, bpf, memxor

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test
  2022-10-12 22:09     ` Daniel Xu
@ 2022-10-12 22:18       ` Florian Westphal
  2022-10-12 22:20       ` Martin KaFai Lau
  1 sibling, 0 replies; 10+ messages in thread
From: Florian Westphal @ 2022-10-12 22:18 UTC (permalink / raw)
  To: Daniel Xu
  Cc: Martin KaFai Lau, pablo, fw, netfilter-devel, netdev,
	linux-kernel, andrii, daniel, ast, bpf, memxor

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test
  2022-10-12 22:09     ` Daniel Xu
  2022-10-12 22:18       ` Florian Westphal
@ 2022-10-12 22:20       ` Martin KaFai Lau
  2022-10-13 17:34         ` Daniel Xu
  1 sibling, 1 reply; 10+ messages in thread
From: Martin KaFai Lau @ 2022-10-12 22:20 UTC (permalink / raw)
  To: Daniel Xu
  Cc: pablo, fw, netfilter-devel, netdev, linux-kernel, andrii, daniel,
	ast, bpf, memxor

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


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test
  2022-10-12 22:20       ` Martin KaFai Lau
@ 2022-10-13 17:34         ` Daniel Xu
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Xu @ 2022-10-13 17:34 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: pablo, fw, netfilter-devel, netdev, linux-kernel, andrii, daniel,
	ast, bpf, memxor

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-10-13 17:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-11 21:55 [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests Daniel Xu
2022-08-11 21:55 ` [PATCH bpf-next v4 1/3] selftests/bpf: Add existing connection bpf_*_ct_lookup() test Daniel Xu
2022-08-11 21:55 ` [PATCH bpf-next v4 2/3] selftests/bpf: Add connmark read test Daniel Xu
2022-10-12  5:49   ` Martin KaFai Lau
2022-10-12 22:09     ` Daniel Xu
2022-10-12 22:18       ` Florian Westphal
2022-10-12 22:20       ` Martin KaFai Lau
2022-10-13 17:34         ` Daniel Xu
2022-08-11 21:55 ` [PATCH bpf-next v4 3/3] selftests/bpf: Update CI kconfig Daniel Xu
2022-08-15 19:00 ` [PATCH bpf-next v4 0/3] Add more bpf_*_ct_lookup() selftests patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).