diff mbox series

[bpf-next,v1,12/15] net/netfilter: Add bpf_ct_kptr_get helper

Message ID 20220220134813.3411982-13-memxor@gmail.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series Introduce typed pointer support in BPF maps | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 91 this patch: 91
netdev/cc_maintainers warning 11 maintainers not CCed: pablo@netfilter.org kuba@kernel.org kpsingh@kernel.org john.fastabend@gmail.com kafai@fb.com songliubraving@fb.com coreteam@netfilter.org fw@strlen.de yhs@fb.com davem@davemloft.net kadlec@netfilter.org
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 92 this patch: 92
netdev/checkpatch warning WARNING: line length of 87 exceeds 80 columns WARNING: line length of 93 exceeds 80 columns WARNING: line length of 95 exceeds 80 columns WARNING: line length of 96 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 1 now: 0
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next success VM_Test

Commit Message

Kumar Kartikeya Dwivedi Feb. 20, 2022, 1:48 p.m. UTC
Require some more feedback on whether this is OK, before refactoring
netfilter functions to share code to increment reference and match the
tuple. Also probably need to work on allowing taking reference to struct
net * to save another lookup inside this function.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/net/netfilter/nf_conntrack_core.h |  17 +++
 net/netfilter/nf_conntrack_bpf.c          | 132 +++++++++++++++++-----
 net/netfilter/nf_conntrack_core.c         |  17 ---
 3 files changed, 119 insertions(+), 47 deletions(-)

Comments

kernel test robot Feb. 21, 2022, 4:35 a.m. UTC | #1
Hi Kumar,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20220217]
[cannot apply to bpf-next/master bpf/master linus/master v5.17-rc4 v5.17-rc3 v5.17-rc2 v5.17-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kumar-Kartikeya-Dwivedi/Introduce-typed-pointer-support-in-BPF-maps/20220220-215105
base:    3c30cf91b5ecc7272b3d2942ae0505dd8320b81c
config: s390-defconfig (https://download.01.org/0day-ci/archive/20220221/202202211228.CO4wFX0Q-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/79e35d4e4ee33a7692f0612065012307a361cd56
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kumar-Kartikeya-Dwivedi/Introduce-typed-pointer-support-in-BPF-maps/20220220-215105
        git checkout 79e35d4e4ee33a7692f0612065012307a361cd56
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=s390 SHELL=/bin/bash net/netfilter/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   net/netfilter/nf_conntrack_bpf.c: In function 'bpf_ct_kptr_get':
>> net/netfilter/nf_conntrack_bpf.c:226:21: warning: variable 'net' set but not used [-Wunused-but-set-variable]
     226 |         struct net *net;
         |                     ^~~
   net/netfilter/nf_conntrack_bpf.c: At top level:
   net/netfilter/nf_conntrack_bpf.c:314:5: warning: no previous prototype for 'register_nf_conntrack_bpf' [-Wmissing-prototypes]
     314 | int register_nf_conntrack_bpf(void)
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~


vim +/net +226 net/netfilter/nf_conntrack_bpf.c

   219	
   220	/* TODO: Just a PoC, need to reuse code in __nf_conntrack_find_get for this */
   221	struct nf_conn *bpf_ct_kptr_get(struct nf_conn **ptr, struct bpf_sock_tuple *bpf_tuple,
   222					u32 tuple__sz, u8 protonum, u8 direction)
   223	{
   224		struct nf_conntrack_tuple tuple;
   225		struct nf_conn *nfct;
 > 226		struct net *net;
   227		u64 *nfct_p;
   228		int ret;
   229	
   230		WARN_ON_ONCE(!rcu_read_lock_held());
   231	
   232		if ((protonum != IPPROTO_TCP && protonum != IPPROTO_UDP) ||
   233		    (direction != IP_CT_DIR_ORIGINAL && direction != IP_CT_DIR_REPLY))
   234			return NULL;
   235	
   236		/* ptr is actually pointer to u64 having address, hence recast u64 load
   237		 * to native pointer width.
   238		 */
   239		nfct_p = (u64 *)ptr;
   240		nfct = (struct nf_conn *)READ_ONCE(*nfct_p);
   241		if (!nfct || unlikely(!refcount_inc_not_zero(&nfct->ct_general.use)))
   242			return NULL;
   243	
   244		memset(&tuple, 0, sizeof(tuple));
   245		ret = bpf_fill_nf_tuple(&tuple, bpf_tuple, tuple__sz);
   246		if (ret < 0)
   247			goto end;
   248		tuple.dst.protonum = protonum;
   249	
   250		/* XXX: Need to allow passing in struct net *, or take netns_id, this is non-sense */
   251		net = nf_ct_net(nfct);
   252		if (!nf_ct_key_equal(&nfct->tuplehash[direction], &tuple,
   253				     &nf_ct_zone_dflt, nf_ct_net(nfct)))
   254			goto end;
   255		return nfct;
   256	end:
   257		nf_ct_put(nfct);
   258		return NULL;
   259	}
   260	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index 13807ea94cd2..09389769dce3 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -51,6 +51,23 @@  nf_conntrack_find_get(struct net *net,
 
 int __nf_conntrack_confirm(struct sk_buff *skb);
 
+static inline bool
+nf_ct_key_equal(struct nf_conntrack_tuple_hash *h,
+		const struct nf_conntrack_tuple *tuple,
+		const struct nf_conntrack_zone *zone,
+		const struct net *net)
+{
+	struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h);
+
+	/* A conntrack can be recreated with the equal tuple,
+	 * so we need to check that the conntrack is confirmed
+	 */
+	return nf_ct_tuple_equal(tuple, &h->tuple) &&
+	       nf_ct_zone_equal(ct, zone, NF_CT_DIRECTION(h)) &&
+	       nf_ct_is_confirmed(ct) &&
+	       net_eq(net, nf_ct_net(ct));
+}
+
 /* Confirm a connection: returns NF_DROP if packet must be dropped. */
 static inline int nf_conntrack_confirm(struct sk_buff *skb)
 {
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index 8ad3f52579f3..26211a5ec0c4 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -52,6 +52,30 @@  enum {
 	NF_BPF_CT_OPTS_SZ = 12,
 };
 
+static int bpf_fill_nf_tuple(struct nf_conntrack_tuple *tuple,
+			     struct bpf_sock_tuple *bpf_tuple, u32 tuple_len)
+{
+	switch (tuple_len) {
+	case sizeof(bpf_tuple->ipv4):
+		tuple->src.l3num = AF_INET;
+		tuple->src.u3.ip = bpf_tuple->ipv4.saddr;
+		tuple->src.u.tcp.port = bpf_tuple->ipv4.sport;
+		tuple->dst.u3.ip = bpf_tuple->ipv4.daddr;
+		tuple->dst.u.tcp.port = bpf_tuple->ipv4.dport;
+		break;
+	case sizeof(bpf_tuple->ipv6):
+		tuple->src.l3num = AF_INET6;
+		memcpy(tuple->src.u3.ip6, bpf_tuple->ipv6.saddr, sizeof(bpf_tuple->ipv6.saddr));
+		tuple->src.u.tcp.port = bpf_tuple->ipv6.sport;
+		memcpy(tuple->dst.u3.ip6, bpf_tuple->ipv6.daddr, sizeof(bpf_tuple->ipv6.daddr));
+		tuple->dst.u.tcp.port = bpf_tuple->ipv6.dport;
+		break;
+	default:
+		return -EAFNOSUPPORT;
+	}
+	return 0;
+}
+
 static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
 					  struct bpf_sock_tuple *bpf_tuple,
 					  u32 tuple_len, u8 protonum,
@@ -59,6 +83,7 @@  static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
 {
 	struct nf_conntrack_tuple_hash *hash;
 	struct nf_conntrack_tuple tuple;
+	int ret;
 
 	if (unlikely(protonum != IPPROTO_TCP && protonum != IPPROTO_UDP))
 		return ERR_PTR(-EPROTO);
@@ -66,25 +91,9 @@  static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
 		return ERR_PTR(-EINVAL);
 
 	memset(&tuple, 0, sizeof(tuple));
-	switch (tuple_len) {
-	case sizeof(bpf_tuple->ipv4):
-		tuple.src.l3num = AF_INET;
-		tuple.src.u3.ip = bpf_tuple->ipv4.saddr;
-		tuple.src.u.tcp.port = bpf_tuple->ipv4.sport;
-		tuple.dst.u3.ip = bpf_tuple->ipv4.daddr;
-		tuple.dst.u.tcp.port = bpf_tuple->ipv4.dport;
-		break;
-	case sizeof(bpf_tuple->ipv6):
-		tuple.src.l3num = AF_INET6;
-		memcpy(tuple.src.u3.ip6, bpf_tuple->ipv6.saddr, sizeof(bpf_tuple->ipv6.saddr));
-		tuple.src.u.tcp.port = bpf_tuple->ipv6.sport;
-		memcpy(tuple.dst.u3.ip6, bpf_tuple->ipv6.daddr, sizeof(bpf_tuple->ipv6.daddr));
-		tuple.dst.u.tcp.port = bpf_tuple->ipv6.dport;
-		break;
-	default:
-		return ERR_PTR(-EAFNOSUPPORT);
-	}
-
+	ret = bpf_fill_nf_tuple(&tuple, bpf_tuple, tuple_len);
+	if (ret < 0)
+		return ERR_PTR(ret);
 	tuple.dst.protonum = protonum;
 
 	if (netns_id >= 0) {
@@ -208,50 +217,113 @@  void bpf_ct_release(struct nf_conn *nfct)
 	nf_ct_put(nfct);
 }
 
+/* TODO: Just a PoC, need to reuse code in __nf_conntrack_find_get for this */
+struct nf_conn *bpf_ct_kptr_get(struct nf_conn **ptr, struct bpf_sock_tuple *bpf_tuple,
+				u32 tuple__sz, u8 protonum, u8 direction)
+{
+	struct nf_conntrack_tuple tuple;
+	struct nf_conn *nfct;
+	struct net *net;
+	u64 *nfct_p;
+	int ret;
+
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	if ((protonum != IPPROTO_TCP && protonum != IPPROTO_UDP) ||
+	    (direction != IP_CT_DIR_ORIGINAL && direction != IP_CT_DIR_REPLY))
+		return NULL;
+
+	/* ptr is actually pointer to u64 having address, hence recast u64 load
+	 * to native pointer width.
+	 */
+	nfct_p = (u64 *)ptr;
+	nfct = (struct nf_conn *)READ_ONCE(*nfct_p);
+	if (!nfct || unlikely(!refcount_inc_not_zero(&nfct->ct_general.use)))
+		return NULL;
+
+	memset(&tuple, 0, sizeof(tuple));
+	ret = bpf_fill_nf_tuple(&tuple, bpf_tuple, tuple__sz);
+	if (ret < 0)
+		goto end;
+	tuple.dst.protonum = protonum;
+
+	/* XXX: Need to allow passing in struct net *, or take netns_id, this is non-sense */
+	net = nf_ct_net(nfct);
+	if (!nf_ct_key_equal(&nfct->tuplehash[direction], &tuple,
+			     &nf_ct_zone_dflt, nf_ct_net(nfct)))
+		goto end;
+	return nfct;
+end:
+	nf_ct_put(nfct);
+	return NULL;
+}
+
 __diag_pop()
 
 BTF_SET_START(nf_ct_xdp_check_kfunc_ids)
 BTF_ID(func, bpf_xdp_ct_lookup)
+BTF_ID(func, bpf_ct_kptr_get)
 BTF_ID(func, bpf_ct_release)
 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_kptr_get)
 BTF_ID(func, bpf_ct_release)
 BTF_SET_END(nf_ct_tc_check_kfunc_ids)
 
 BTF_SET_START(nf_ct_acquire_kfunc_ids)
 BTF_ID(func, bpf_xdp_ct_lookup)
 BTF_ID(func, bpf_skb_ct_lookup)
+BTF_ID(func, bpf_ct_kptr_get)
 BTF_SET_END(nf_ct_acquire_kfunc_ids)
 
 BTF_SET_START(nf_ct_release_kfunc_ids)
 BTF_ID(func, bpf_ct_release)
 BTF_SET_END(nf_ct_release_kfunc_ids)
 
+BTF_SET_START(nf_ct_kptr_acquire_kfunc_ids)
+BTF_ID(func, bpf_ct_kptr_get)
+BTF_SET_END(nf_ct_kptr_acquire_kfunc_ids)
+
 /* Both sets are identical */
 #define nf_ct_ret_null_kfunc_ids nf_ct_acquire_kfunc_ids
 
 static const struct btf_kfunc_id_set nf_conntrack_xdp_kfunc_set = {
-	.owner        = THIS_MODULE,
-	.check_set    = &nf_ct_xdp_check_kfunc_ids,
-	.acquire_set  = &nf_ct_acquire_kfunc_ids,
-	.release_set  = &nf_ct_release_kfunc_ids,
-	.ret_null_set = &nf_ct_ret_null_kfunc_ids,
+	.owner            = THIS_MODULE,
+	.check_set        = &nf_ct_xdp_check_kfunc_ids,
+	.acquire_set      = &nf_ct_acquire_kfunc_ids,
+	.release_set      = &nf_ct_release_kfunc_ids,
+	.ret_null_set     = &nf_ct_ret_null_kfunc_ids,
+	.kptr_acquire_set = &nf_ct_kptr_acquire_kfunc_ids,
 };
 
 static const struct btf_kfunc_id_set nf_conntrack_tc_kfunc_set = {
-	.owner        = THIS_MODULE,
-	.check_set    = &nf_ct_tc_check_kfunc_ids,
-	.acquire_set  = &nf_ct_acquire_kfunc_ids,
-	.release_set  = &nf_ct_release_kfunc_ids,
-	.ret_null_set = &nf_ct_ret_null_kfunc_ids,
+	.owner            = THIS_MODULE,
+	.check_set        = &nf_ct_tc_check_kfunc_ids,
+	.acquire_set      = &nf_ct_acquire_kfunc_ids,
+	.release_set      = &nf_ct_release_kfunc_ids,
+	.ret_null_set     = &nf_ct_ret_null_kfunc_ids,
+	.kptr_acquire_set = &nf_ct_kptr_acquire_kfunc_ids,
 };
 
+BTF_ID_LIST(nf_conntrack_dtor_kfunc_ids)
+BTF_ID(struct, nf_conn)
+BTF_ID(func, bpf_ct_release)
+
 int register_nf_conntrack_bpf(void)
 {
+	const struct btf_id_dtor_kfunc nf_conntrack_dtor_kfunc[] = {
+		{
+			.btf_id       = nf_conntrack_dtor_kfunc_ids[0],
+			.kfunc_btf_id = nf_conntrack_dtor_kfunc_ids[1],
+		}
+	};
 	int ret;
 
-	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &nf_conntrack_xdp_kfunc_set);
+	ret = register_btf_id_dtor_kfuncs(nf_conntrack_dtor_kfunc,
+					  ARRAY_SIZE(nf_conntrack_dtor_kfunc),
+					  THIS_MODULE);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &nf_conntrack_xdp_kfunc_set);
 	return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &nf_conntrack_tc_kfunc_set);
 }
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 9b7f9c966f73..0aae98f60769 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -710,23 +710,6 @@  bool nf_ct_delete(struct nf_conn *ct, u32 portid, int report)
 }
 EXPORT_SYMBOL_GPL(nf_ct_delete);
 
-static inline bool
-nf_ct_key_equal(struct nf_conntrack_tuple_hash *h,
-		const struct nf_conntrack_tuple *tuple,
-		const struct nf_conntrack_zone *zone,
-		const struct net *net)
-{
-	struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h);
-
-	/* A conntrack can be recreated with the equal tuple,
-	 * so we need to check that the conntrack is confirmed
-	 */
-	return nf_ct_tuple_equal(tuple, &h->tuple) &&
-	       nf_ct_zone_equal(ct, zone, NF_CT_DIRECTION(h)) &&
-	       nf_ct_is_confirmed(ct) &&
-	       net_eq(net, nf_ct_net(ct));
-}
-
 static inline bool
 nf_ct_match(const struct nf_conn *ct1, const struct nf_conn *ct2)
 {