[PATCH v2 bpf-next 0/2] net: netfilter: add kfunc helper to update ct timeout

[PATCH v2 bpf-next 0/2] net: netfilter: add kfunc helper to update ct timeout

[Lorenzo Bianconi]

Changes since v1:
- add bpf_ct_refresh_timeout kfunc selftest

Lorenzo Bianconi (2):
  net: netfilter: add kfunc helper to update ct timeout
  selftests/bpf: add selftest for bpf_ct_refresh_timeout kfunc

 include/net/netfilter/nf_conntrack.h          |  1 +
 net/netfilter/nf_conntrack_bpf.c              | 20 +++++++++++++++++
 net/netfilter/nf_conntrack_core.c             | 21 +++++++++++-------
 .../testing/selftests/bpf/prog_tests/bpf_nf.c | 10 +++++++++
 .../testing/selftests/bpf/progs/test_bpf_nf.c | 22 +++++++++++++++++++
 5 files changed, 66 insertions(+), 8 deletions(-)

-- 
2.35.3

[PATCH v2 bpf-next 1/2] net: netfilter: add kfunc helper to update ct timeout

[Lorenzo Bianconi]

Introduce bpf_ct_refresh_timeout kfunc helper in order to update time
nf_conn lifetime. Move timeout update logic in nf_ct_refresh_timeout
utility routine.

Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 include/net/netfilter/nf_conntrack.h |  1 +
 net/netfilter/nf_conntrack_bpf.c     | 20 ++++++++++++++++++++
 net/netfilter/nf_conntrack_core.c    | 21 +++++++++++++--------
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index 69e6c6a218be..02b7115b92d0 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -205,6 +205,7 @@ bool nf_ct_get_tuplepr(const struct sk_buff *skb, unsigned int nhoff,
 		       u_int16_t l3num, struct net *net,
 		       struct nf_conntrack_tuple *tuple);
 
+void nf_ct_refresh_timeout(struct nf_conn *ct, u32 extra_jiffies);
 void __nf_ct_refresh_acct(struct nf_conn *ct, enum ip_conntrack_info ctinfo,
 			  const struct sk_buff *skb,
 			  u32 extra_jiffies, bool do_acct);
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index bc4d5cd63a94..d6dcadf0e016 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -217,16 +217,36 @@ void bpf_ct_release(struct nf_conn *nfct)
 	nf_ct_put(nfct);
 }
 
+/* bpf_ct_refresh_timeout - Refresh nf_conn object
+ *
+ * Refresh timeout associated to the provided connection tracking entry.
+ * This must be invoked for referenced PTR_TO_BTF_ID.
+ *
+ * Parameters:
+ * @nf_conn      - Pointer to referenced nf_conn object, obtained using
+ *		   bpf_xdp_ct_lookup or bpf_skb_ct_lookup.
+ * @timeout      - delta time in msecs used to increase the ct entry lifetime.
+ */
+void bpf_ct_refresh_timeout(struct nf_conn *nfct, u32 timeout)
+{
+	if (!nfct)
+		return;
+
+	nf_ct_refresh_timeout(nfct, msecs_to_jiffies(timeout));
+}
+
 __diag_pop()
 
 BTF_SET_START(nf_ct_xdp_check_kfunc_ids)
 BTF_ID(func, bpf_xdp_ct_lookup)
 BTF_ID(func, bpf_ct_release)
+BTF_ID(func, bpf_ct_refresh_timeout);
 BTF_SET_END(nf_ct_xdp_check_kfunc_ids)
 
 BTF_SET_START(nf_ct_tc_check_kfunc_ids)
 BTF_ID(func, bpf_skb_ct_lookup)
 BTF_ID(func, bpf_ct_release)
+BTF_ID(func, bpf_ct_refresh_timeout);
 BTF_SET_END(nf_ct_tc_check_kfunc_ids)
 
 BTF_SET_START(nf_ct_acquire_kfunc_ids)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 0164e5f522e8..f43e743728bd 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2030,16 +2030,11 @@ void nf_conntrack_alter_reply(struct nf_conn *ct,
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_alter_reply);
 
-/* Refresh conntrack for this many jiffies and do accounting if do_acct is 1 */
-void __nf_ct_refresh_acct(struct nf_conn *ct,
-			  enum ip_conntrack_info ctinfo,
-			  const struct sk_buff *skb,
-			  u32 extra_jiffies,
-			  bool do_acct)
+void nf_ct_refresh_timeout(struct nf_conn *ct, u32 extra_jiffies)
 {
 	/* Only update if this is not a fixed timeout */
 	if (test_bit(IPS_FIXED_TIMEOUT_BIT, &ct->status))
-		goto acct;
+		return;
 
 	/* If not in hash table, timer will not be active yet */
 	if (nf_ct_is_confirmed(ct))
@@ -2047,7 +2042,17 @@ void __nf_ct_refresh_acct(struct nf_conn *ct,
 
 	if (READ_ONCE(ct->timeout) != extra_jiffies)
 		WRITE_ONCE(ct->timeout, extra_jiffies);
-acct:
+}
+
+/* Refresh conntrack for this many jiffies and do accounting if do_acct is 1 */
+void __nf_ct_refresh_acct(struct nf_conn *ct,
+			  enum ip_conntrack_info ctinfo,
+			  const struct sk_buff *skb,
+			  u32 extra_jiffies,
+			  bool do_acct)
+{
+	nf_ct_refresh_timeout(ct, extra_jiffies);
+
 	if (do_acct)
 		nf_ct_acct_update(ct, CTINFO2DIR(ctinfo), skb->len);
 }
-- 
2.35.3

[PATCH v2 bpf-next 2/2] selftests/bpf: add selftest for bpf_ct_refresh_timeout kfunc

[Lorenzo Bianconi]

Install a new ct entry in order to perform a successful lookup and
test bpf_ct_refresh_timeout kfunc helper.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 .../testing/selftests/bpf/prog_tests/bpf_nf.c | 10 +++++++++
 .../testing/selftests/bpf/progs/test_bpf_nf.c | 22 +++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
index dd30b1e3a67c..285687d2f7b3 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
@@ -18,6 +18,13 @@ void test_bpf_nf_ct(int mode)
 		.repeat = 1,
 	);
 
+	/* Flush previous nft ct entries */
+	ASSERT_OK(system("conntrack -F"), "flush ct entries");
+	/* Let's create a nft ct entry to perform lookup */
+	ASSERT_OK(system("conntrack -I -s 1.1.1.1 -d 2.2.2.2 --protonum 6  \
+			  --state ESTABLISHED --timeout 3600 --sport 12345 \
+			  --dport 1000 --zone 0"), "create ct entry");
+
 	skel = test_bpf_nf__open_and_load();
 	if (!ASSERT_OK_PTR(skel, "test_bpf_nf__open_and_load"))
 		return;
@@ -39,6 +46,9 @@ void test_bpf_nf_ct(int mode)
 	ASSERT_EQ(skel->bss->test_enonet_netns_id, -ENONET, "Test ENONET for bad but valid netns_id");
 	ASSERT_EQ(skel->bss->test_enoent_lookup, -ENOENT, "Test ENOENT for failed lookup");
 	ASSERT_EQ(skel->bss->test_eafnosupport, -EAFNOSUPPORT, "Test EAFNOSUPPORT for invalid len__tuple");
+	ASSERT_EQ(skel->bss->test_succ_lookup, 0, "Test for successful lookup");
+	ASSERT_EQ(skel->bss->test_delta_timeout, 10, "Test for ct timeout update");
+
 end:
 	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 f00a9731930e..3eb36679a0b5 100644
--- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c
+++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
 
 #define EAFNOSUPPORT 97
 #define EPROTO 71
@@ -8,6 +9,8 @@
 #define EINVAL 22
 #define ENOENT 2
 
+extern unsigned long CONFIG_HZ __kconfig;
+
 int test_einval_bpf_tuple = 0;
 int test_einval_reserved = 0;
 int test_einval_netns_id = 0;
@@ -16,6 +19,8 @@ int test_eproto_l4proto = 0;
 int test_enonet_netns_id = 0;
 int test_enoent_lookup = 0;
 int test_eafnosupport = 0;
+int test_succ_lookup = 0;
+u32 test_delta_timeout = 0;
 
 struct nf_conn;
 
@@ -31,6 +36,7 @@ struct nf_conn *bpf_xdp_ct_lookup(struct xdp_md *, struct bpf_sock_tuple *, u32,
 struct nf_conn *bpf_skb_ct_lookup(struct __sk_buff *, struct bpf_sock_tuple *, u32,
 				  struct bpf_ct_opts___local *, u32) __ksym;
 void bpf_ct_release(struct nf_conn *) __ksym;
+void bpf_ct_refresh_timeout(struct nf_conn *, u32) __ksym;
 
 static __always_inline void
 nf_ct_test(struct nf_conn *(*func)(void *, struct bpf_sock_tuple *, u32,
@@ -99,6 +105,22 @@ nf_ct_test(struct nf_conn *(*func)(void *, struct bpf_sock_tuple *, u32,
 		bpf_ct_release(ct);
 	else
 		test_eafnosupport = opts_def.error;
+
+	bpf_tuple.ipv4.saddr = 0x01010101; /* src IP 1.1.1.1 */
+	bpf_tuple.ipv4.daddr = 0x02020202; /* dst IP 2.2.2.2 */
+	bpf_tuple.ipv4.sport = bpf_htons(12345); /* src port */
+	bpf_tuple.ipv4.dport = bpf_htons(1000);  /* dst port */
+	ct = func(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def,
+		  sizeof(opts_def));
+	if (ct) {
+		/* update ct entry timeout */
+		bpf_ct_refresh_timeout(ct, 10000);
+		test_delta_timeout = ct->timeout - bpf_jiffies64();
+		test_delta_timeout /= CONFIG_HZ;
+		bpf_ct_release(ct);
+	} else {
+		test_succ_lookup = opts_def.error;
+	}
 }
 
 SEC("xdp")
-- 
2.35.3

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add selftest for bpf_ct_refresh_timeout kfunc

[Alexei Starovoitov]

On Thu, May 12, 2022 at 9:34 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> Install a new ct entry in order to perform a successful lookup and
> test bpf_ct_refresh_timeout kfunc helper.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

CI is failing:
test_bpf_nf_ct:FAIL:flush ct entries unexpected error: 32512 (errno 2)
test_bpf_nf_ct:FAIL:create ct entry unexpected error: 32512 (errno 2)

Please follow the links from patchwork for details.

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add selftest for bpf_ct_refresh_timeout kfunc

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

> On Thu, May 12, 2022 at 9:34 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > Install a new ct entry in order to perform a successful lookup and
> > test bpf_ct_refresh_timeout kfunc helper.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> CI is failing:
> test_bpf_nf_ct:FAIL:flush ct entries unexpected error: 32512 (errno 2)
> test_bpf_nf_ct:FAIL:create ct entry unexpected error: 32512 (errno 2)
> 
> Please follow the links from patchwork for details.

Hi Alexei,

tests failed because conntrack is not installed on the system:

2022-05-14T00:12:09.0799053Z sh: line 1: conntrack: command not found

Is it ok to just skip the test if conntrack is not installed on the system
or do you prefer to directly send netlink messages to ct in order to add a
new ct entry?

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add selftest for bpf_ct_refresh_timeout kfunc

[Alexei Starovoitov]

On Sat, May 14, 2022 at 3:40 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> > On Thu, May 12, 2022 at 9:34 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > >
> > > Install a new ct entry in order to perform a successful lookup and
> > > test bpf_ct_refresh_timeout kfunc helper.
> > >
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> >
> > CI is failing:
> > test_bpf_nf_ct:FAIL:flush ct entries unexpected error: 32512 (errno 2)
> > test_bpf_nf_ct:FAIL:create ct entry unexpected error: 32512 (errno 2)
> >
> > Please follow the links from patchwork for details.
>
> Hi Alexei,
>
> tests failed because conntrack is not installed on the system:
>
> 2022-05-14T00:12:09.0799053Z sh: line 1: conntrack: command not found
>
> Is it ok to just skip the test if conntrack is not installed on the system
> or do you prefer to directly send netlink messages to ct in order to add a
> new ct entry?

It will take a long time to update x86 and s390 images.
Maybe we should add a kfunc that creates a ct entry?

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add selftest for bpf_ct_refresh_timeout kfunc

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

> On Sat, May 14, 2022 at 3:40 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > > On Thu, May 12, 2022 at 9:34 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > > >
> > > > Install a new ct entry in order to perform a successful lookup and
> > > > test bpf_ct_refresh_timeout kfunc helper.
> > > >
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > >
> > > CI is failing:
> > > test_bpf_nf_ct:FAIL:flush ct entries unexpected error: 32512 (errno 2)
> > > test_bpf_nf_ct:FAIL:create ct entry unexpected error: 32512 (errno 2)
> > >
> > > Please follow the links from patchwork for details.
> >
> > Hi Alexei,
> >
> > tests failed because conntrack is not installed on the system:
> >
> > 2022-05-14T00:12:09.0799053Z sh: line 1: conntrack: command not found
> >
> > Is it ok to just skip the test if conntrack is not installed on the system
> > or do you prefer to directly send netlink messages to ct in order to add a
> > new ct entry?
> 
> It will take a long time to update x86 and s390 images.
> Maybe we should add a kfunc that creates a ct entry?

ack, I added the support for it. I will post it soon.

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]