[PATCH bpf-next v2 0/6] bpf: add netfilter program type

[PATCH bpf-next v2 0/6] bpf: add netfilter program type

[Florian Westphal]

Add minimal support to hook bpf programs to netfilter hooks, e.g.
PREROUTING or FORWARD.

For this the most relevant parts for registering a netfilter
hook via the in-kernel api are exposed to userspace via bpf_link.

The new program type is 'tracing style', i.e. there is no context
access rewrite done by verifier, the function argument (struct bpf_nf_ctx)
isn't stable.
There is no support for direct packet access, dynptr api should be used
instead.

With this its possible to build a small test program such as:

 #include "vmlinux.h"
extern int bpf_dynptr_from_skb(struct __sk_buff *skb, __u64 flags,
                               struct bpf_dynptr *ptr__uninit) __ksym;
extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, uint32_t offset,
                                   void *buffer, uint32_t buffer__sz) __ksym;
SEC("netfilter")
int nf_test(struct bpf_nf_ctx *ctx)
{
	struct nf_hook_state *state = ctx->state;
	struct sk_buff *skb = ctx->skb;
	const struct iphdr *iph, _iph;
	const struct tcphdr *th, _th;
	struct bpf_dynptr ptr;

	if (bpf_dynptr_from_skb(skb, 0, &ptr))
		return NF_DROP;

	iph = bpf_dynptr_slice(&ptr, 0, &_iph, sizeof(_iph));
	if (!iph)
		return NF_DROP;

	th = bpf_dynptr_slice(&ptr, iph->ihl << 2, &_th, sizeof(_th));
	if (!th)
		return NF_DROP;

	bpf_printk("accept %x:%d->%x:%d, hook %d ifin %d\n", iph->saddr, bpf_ntohs(th->source), iph->daddr, bpf_ntohs(th->dest), state->hook, state->in->ifindex);
        return NF_ACCEPT;
}

Then, tail /sys/kernel/tracing/trace_pipe.

Changes since v1:
1. Don't fail to link when CONFIG_NETFILTER=n (build bot)
2. Use test_progs instead of test_verifier (Alexei)

Changes since last RFC version:
1. extend 'bpftool link show' to print prio/hooknum etc
2. extend 'nft list hooks' so it can print the bpf program id
3. Add an extra patch to artificially restrict bpf progs with
   same priority.  Its fine from a technical pov but it will
   cause ordering issues (most recent one comes first).
   Can be removed later.
4. Add test_run support for netfilter prog type and a small
   extension to verifier tests to make sure we can't return
   verdicts like NF_STOLEN.
5. Alter the netfilter part of the bpf_link uapi struct:
   - add flags/reserved members.
  Not used here except returning errors when they are nonzero.
  Plan is to allow the bpf_link users to enable netfilter
  defrag or conntrack engine by setting feature flags at
  link create time in the future.

Let me know if there is anything missing that has to be addressed
before this can be merged.

Thanks!

Florian Westphal (6):
  bpf: add bpf_link support for BPF_NETFILTER programs
  bpf: minimal support for programs hooked into netfilter framework
  netfilter: nfnetlink hook: dump bpf prog id
  netfilter: disallow bpf hook attachment at same priority
  tools: bpftool: print netfilter link info
  bpf: add test_run support for netfilter program type

 include/linux/bpf.h                           |   3 +
 include/linux/bpf_types.h                     |   4 +
 include/linux/netfilter.h                     |   1 +
 include/net/netfilter/nf_bpf_link.h           |  15 ++
 include/uapi/linux/bpf.h                      |  15 ++
 include/uapi/linux/netfilter/nfnetlink_hook.h |  20 +-
 kernel/bpf/btf.c                              |   6 +
 kernel/bpf/syscall.c                          |   6 +
 kernel/bpf/verifier.c                         |   3 +
 net/bpf/test_run.c                            | 140 +++++++++++++
 net/core/filter.c                             |   1 +
 net/netfilter/Kconfig                         |   3 +
 net/netfilter/Makefile                        |   1 +
 net/netfilter/core.c                          |  12 ++
 net/netfilter/nf_bpf_link.c                   | 190 ++++++++++++++++++
 net/netfilter/nfnetlink_hook.c                |  81 ++++++--
 tools/bpf/bpftool/link.c                      |  24 +++
 tools/include/uapi/linux/bpf.h                |  15 ++
 tools/lib/bpf/libbpf.c                        |   1 +
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_netfilter_retcode.c    |  49 +++++
 21 files changed, 578 insertions(+), 14 deletions(-)
 create mode 100644 include/net/netfilter/nf_bpf_link.h
 create mode 100644 net/netfilter/nf_bpf_link.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_netfilter_retcode.c

-- 
2.39.2

[PATCH bpf-next v2 1/6] bpf: add bpf_link support for BPF_NETFILTER programs

[Florian Westphal]

Add bpf_link support skeleton.  To keep this reviewable, no bpf program
can be invoked yet, if a program is attached only a c-stub is called and
not the actual bpf program.

Defaults to 'y' if both netfilter and bpf syscall are enabled in kconfig.

Uapi example usage:
	union bpf_attr attr = { };

	attr.link_create.prog_fd = progfd;
	attr.link_create.attach_type = 0; /* unused */
	attr.link_create.netfilter.pf = PF_INET;
	attr.link_create.netfilter.hooknum = NF_INET_LOCAL_IN;
	attr.link_create.netfilter.priority = -128;

	err = bpf(BPF_LINK_CREATE, &attr, sizeof(attr));

... this would attach progfd to ipv4:input hook.

Such hook gets removed automatically if the calling program exits.

BPF_NETFILTER program invocation is added in followup change.

NF_HOOK_OP_BPF enum will eventually be read from nfnetlink_hook, it
allows to tell userspace which program is attached at the given hook
when user runs 'nft hook list' command rather than just the priority
and not-very-helpful 'this hook runs a bpf prog but I can't tell which
one'.

Will also be used to disallow registration of two bpf programs with
same priority in a followup patch.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: add stub so CONFIG_NETFILTER=n won't fail to link

 include/linux/netfilter.h           |   1 +
 include/net/netfilter/nf_bpf_link.h |  10 +++
 include/uapi/linux/bpf.h            |  15 ++++
 kernel/bpf/syscall.c                |   6 ++
 net/netfilter/Kconfig               |   3 +
 net/netfilter/Makefile              |   1 +
 net/netfilter/nf_bpf_link.c         | 121 ++++++++++++++++++++++++++++
 7 files changed, 157 insertions(+)
 create mode 100644 include/net/netfilter/nf_bpf_link.h
 create mode 100644 net/netfilter/nf_bpf_link.c

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index c8e03bcaecaa..0762444e3767 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -80,6 +80,7 @@ typedef unsigned int nf_hookfn(void *priv,
 enum nf_hook_ops_type {
 	NF_HOOK_OP_UNDEFINED,
 	NF_HOOK_OP_NF_TABLES,
+	NF_HOOK_OP_BPF,
 };
 
 struct nf_hook_ops {
diff --git a/include/net/netfilter/nf_bpf_link.h b/include/net/netfilter/nf_bpf_link.h
new file mode 100644
index 000000000000..eeaeaf3d15de
--- /dev/null
+++ b/include/net/netfilter/nf_bpf_link.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#if IS_ENABLED(CONFIG_NETFILTER_BPF_LINK)
+int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
+#else
+static inline int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+	return -EOPNOTSUPP;
+}
+#endif
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 3823100b7934..c93febc4c75f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -986,6 +986,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_LSM,
 	BPF_PROG_TYPE_SK_LOOKUP,
 	BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
+	BPF_PROG_TYPE_NETFILTER,
 };
 
 enum bpf_attach_type {
@@ -1050,6 +1051,7 @@ enum bpf_link_type {
 	BPF_LINK_TYPE_PERF_EVENT = 7,
 	BPF_LINK_TYPE_KPROBE_MULTI = 8,
 	BPF_LINK_TYPE_STRUCT_OPS = 9,
+	BPF_LINK_TYPE_NETFILTER = 10,
 
 	MAX_BPF_LINK_TYPE,
 };
@@ -1560,6 +1562,13 @@ union bpf_attr {
 				 */
 				__u64		cookie;
 			} tracing;
+			struct {
+				__u32		pf;
+				__u32		hooknum;
+				__s32		prio;
+				__u32		flags;
+				__u64		reserved[2];
+			} netfilter;
 		};
 	} link_create;
 
@@ -6410,6 +6419,12 @@ struct bpf_link_info {
 		struct {
 			__u32 map_id;
 		} struct_ops;
+		struct {
+			__u32 pf;
+			__u32 hooknum;
+			__s32 priority;
+			__u32 flags;
+		} netfilter;
 	};
 } __attribute__((aligned(8)));
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6d575505f89c..a5583f196423 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -35,6 +35,7 @@
 #include <linux/rcupdate_trace.h>
 #include <linux/memcontrol.h>
 #include <linux/trace_events.h>
+#include <net/netfilter/nf_bpf_link.h>
 
 #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
 			  (map)->map_type == BPF_MAP_TYPE_CGROUP_ARRAY || \
@@ -2472,6 +2473,7 @@ static bool is_net_admin_prog_type(enum bpf_prog_type prog_type)
 	case BPF_PROG_TYPE_CGROUP_SYSCTL:
 	case BPF_PROG_TYPE_SOCK_OPS:
 	case BPF_PROG_TYPE_EXT: /* extends any prog */
+	case BPF_PROG_TYPE_NETFILTER:
 		return true;
 	case BPF_PROG_TYPE_CGROUP_SKB:
 		/* always unpriv */
@@ -4598,6 +4600,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 
 	switch (prog->type) {
 	case BPF_PROG_TYPE_EXT:
+	case BPF_PROG_TYPE_NETFILTER:
 		break;
 	case BPF_PROG_TYPE_PERF_EVENT:
 	case BPF_PROG_TYPE_TRACEPOINT:
@@ -4664,6 +4667,9 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 	case BPF_PROG_TYPE_XDP:
 		ret = bpf_xdp_link_attach(attr, prog);
 		break;
+	case BPF_PROG_TYPE_NETFILTER:
+		ret = bpf_nf_link_attach(attr, prog);
+		break;
 #endif
 	case BPF_PROG_TYPE_PERF_EVENT:
 	case BPF_PROG_TYPE_TRACEPOINT:
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 4d6737160857..bea06f62a30e 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -30,6 +30,9 @@ config NETFILTER_FAMILY_BRIDGE
 config NETFILTER_FAMILY_ARP
 	bool
 
+config NETFILTER_BPF_LINK
+	def_bool BPF_SYSCALL
+
 config NETFILTER_NETLINK_HOOK
 	tristate "Netfilter base hook dump support"
 	depends on NETFILTER_ADVANCED
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 5ffef1cd6143..d4958e7e7631 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -22,6 +22,7 @@ nf_conntrack-$(CONFIG_DEBUG_INFO_BTF) += nf_conntrack_bpf.o
 endif
 
 obj-$(CONFIG_NETFILTER) = netfilter.o
+obj-$(CONFIG_NETFILTER_BPF_LINK) += nf_bpf_link.o
 
 obj-$(CONFIG_NETFILTER_NETLINK) += nfnetlink.o
 obj-$(CONFIG_NETFILTER_NETLINK_ACCT) += nfnetlink_acct.o
diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c
new file mode 100644
index 000000000000..c68b2cb70fd4
--- /dev/null
+++ b/net/netfilter/nf_bpf_link.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <linux/netfilter.h>
+
+#include <net/netfilter/nf_bpf_link.h>
+
+static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb, const struct nf_hook_state *s)
+{
+	return NF_ACCEPT;
+}
+
+struct bpf_nf_link {
+	struct bpf_link link;
+	struct nf_hook_ops hook_ops;
+	struct net *net;
+};
+
+static void bpf_nf_link_release(struct bpf_link *link)
+{
+	struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
+
+	nf_unregister_net_hook(nf_link->net, &nf_link->hook_ops);
+}
+
+static void bpf_nf_link_dealloc(struct bpf_link *link)
+{
+	struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
+
+	kfree(nf_link);
+}
+
+static int bpf_nf_link_detach(struct bpf_link *link)
+{
+	bpf_nf_link_release(link);
+	return 0;
+}
+
+static void bpf_nf_link_show_info(const struct bpf_link *link,
+				  struct seq_file *seq)
+{
+	struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
+
+	seq_printf(seq, "pf:\t%u\thooknum:\t%u\tprio:\t%d\n",
+		   nf_link->hook_ops.pf, nf_link->hook_ops.hooknum,
+		   nf_link->hook_ops.priority);
+}
+
+static int bpf_nf_link_fill_link_info(const struct bpf_link *link,
+				      struct bpf_link_info *info)
+{
+	struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
+
+	info->netfilter.pf = nf_link->hook_ops.pf;
+	info->netfilter.hooknum = nf_link->hook_ops.hooknum;
+	info->netfilter.priority = nf_link->hook_ops.priority;
+	info->netfilter.flags = 0;
+
+	return 0;
+}
+
+static int bpf_nf_link_update(struct bpf_link *link, struct bpf_prog *new_prog,
+			      struct bpf_prog *old_prog)
+{
+	return -EOPNOTSUPP;
+}
+
+static const struct bpf_link_ops bpf_nf_link_lops = {
+	.release = bpf_nf_link_release,
+	.dealloc = bpf_nf_link_dealloc,
+	.detach = bpf_nf_link_detach,
+	.show_fdinfo = bpf_nf_link_show_info,
+	.fill_link_info = bpf_nf_link_fill_link_info,
+	.update_prog = bpf_nf_link_update,
+};
+
+int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+	struct net *net = current->nsproxy->net_ns;
+	struct bpf_link_primer link_primer;
+	struct bpf_nf_link *link;
+	int err;
+
+	if (attr->link_create.flags)
+		return -EINVAL;
+
+	if (attr->link_create.netfilter.flags)
+		return -EOPNOTSUPP;
+
+	if (attr->link_create.netfilter.reserved[0] | attr->link_create.netfilter.reserved[1])
+		return -EINVAL;
+
+	link = kzalloc(sizeof(*link), GFP_USER);
+	if (!link)
+		return -ENOMEM;
+
+	bpf_link_init(&link->link, BPF_LINK_TYPE_NETFILTER, &bpf_nf_link_lops, prog);
+
+	link->hook_ops.hook = nf_hook_run_bpf;
+	link->hook_ops.hook_ops_type = NF_HOOK_OP_BPF;
+	link->hook_ops.priv = prog;
+
+	link->hook_ops.pf = attr->link_create.netfilter.pf;
+	link->hook_ops.priority = attr->link_create.netfilter.prio;
+	link->hook_ops.hooknum = attr->link_create.netfilter.hooknum;
+
+	link->net = net;
+
+	err = bpf_link_prime(&link->link, &link_primer);
+	if (err) {
+		kfree(link);
+		return err;
+	}
+
+	err = nf_register_net_hook(net, &link->hook_ops);
+	if (err) {
+		bpf_link_cleanup(&link_primer);
+		return err;
+	}
+
+	return bpf_link_settle(&link_primer);
+}
-- 
2.39.2

[PATCH bpf-next v2 2/6] bpf: minimal support for programs hooked into netfilter framework

[Florian Westphal]

This adds minimal support for BPF_PROG_TYPE_NETFILTER bpf programs
that will be invoked via the NF_HOOK() points in the ip stack.

Invocation incurs an indirect call.  This is not a necessity: Its
possible to add 'DEFINE_BPF_DISPATCHER(nf_progs)' and handle the
program invocation with the same method already done for xdp progs.

This isn't done here to keep the size of this chunk down.

Verifier restricts verdicts to either DROP or ACCEPT.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/bpf_types.h           |  4 ++
 include/net/netfilter/nf_bpf_link.h |  5 +++
 kernel/bpf/btf.c                    |  6 +++
 kernel/bpf/verifier.c               |  3 ++
 net/core/filter.c                   |  1 +
 net/netfilter/nf_bpf_link.c         | 70 ++++++++++++++++++++++++++++-
 6 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index d4ee3ccd3753..39a999abb0ce 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -79,6 +79,10 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LSM, lsm,
 #endif
 BPF_PROG_TYPE(BPF_PROG_TYPE_SYSCALL, bpf_syscall,
 	      void *, void *)
+#ifdef CONFIG_NETFILTER
+BPF_PROG_TYPE(BPF_PROG_TYPE_NETFILTER, netfilter,
+	      struct bpf_nf_ctx, struct bpf_nf_ctx)
+#endif
 
 BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
diff --git a/include/net/netfilter/nf_bpf_link.h b/include/net/netfilter/nf_bpf_link.h
index eeaeaf3d15de..6c984b0ea838 100644
--- a/include/net/netfilter/nf_bpf_link.h
+++ b/include/net/netfilter/nf_bpf_link.h
@@ -1,5 +1,10 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
+struct bpf_nf_ctx {
+	const struct nf_hook_state *state;
+	struct sk_buff *skb;
+};
+
 #if IS_ENABLED(CONFIG_NETFILTER_BPF_LINK)
 int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
 #else
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 913b9d717a4a..b9924e0f2402 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -25,6 +25,9 @@
 #include <linux/bsearch.h>
 #include <linux/kobject.h>
 #include <linux/sysfs.h>
+
+#include <net/netfilter/nf_bpf_link.h>
+
 #include <net/sock.h>
 #include "../tools/lib/bpf/relo_core.h"
 
@@ -212,6 +215,7 @@ enum btf_kfunc_hook {
 	BTF_KFUNC_HOOK_SK_SKB,
 	BTF_KFUNC_HOOK_SOCKET_FILTER,
 	BTF_KFUNC_HOOK_LWT,
+	BTF_KFUNC_HOOK_NETFILTER,
 	BTF_KFUNC_HOOK_MAX,
 };
 
@@ -7857,6 +7861,8 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
 	case BPF_PROG_TYPE_LWT_XMIT:
 	case BPF_PROG_TYPE_LWT_SEG6LOCAL:
 		return BTF_KFUNC_HOOK_LWT;
+	case BPF_PROG_TYPE_NETFILTER:
+		return BTF_KFUNC_HOOK_NETFILTER;
 	default:
 		return BTF_KFUNC_HOOK_MAX;
 	}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d6db6de3e9ea..f04c7c2d6908 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13799,6 +13799,9 @@ static int check_return_code(struct bpf_verifier_env *env)
 		}
 		break;
 
+	case BPF_PROG_TYPE_NETFILTER:
+		range = tnum_range(NF_DROP, NF_ACCEPT);
+		break;
 	case BPF_PROG_TYPE_EXT:
 		/* freplace program can return anything as its return value
 		 * depends on the to-be-replaced kernel func or bpf program.
diff --git a/net/core/filter.c b/net/core/filter.c
index 727c5269867d..dfc8dfaf631b 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -11712,6 +11712,7 @@ static int __init bpf_kfunc_init(void)
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_IN, &bpf_kfunc_set_skb);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_XMIT, &bpf_kfunc_set_skb);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_SEG6LOCAL, &bpf_kfunc_set_skb);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_NETFILTER, &bpf_kfunc_set_skb);
 	return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &bpf_kfunc_set_xdp);
 }
 late_initcall(bpf_kfunc_init);
diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c
index c68b2cb70fd4..4b22a31d6df5 100644
--- a/net/netfilter/nf_bpf_link.c
+++ b/net/netfilter/nf_bpf_link.c
@@ -1,12 +1,19 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/bpf.h>
+#include <linux/filter.h>
 #include <linux/netfilter.h>
 
 #include <net/netfilter/nf_bpf_link.h>
 
 static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb, const struct nf_hook_state *s)
 {
-	return NF_ACCEPT;
+	const struct bpf_prog *prog = bpf_prog;
+	struct bpf_nf_ctx ctx = {
+		.state = s,
+		.skb = skb,
+	};
+
+	return bpf_prog_run(prog, &ctx);
 }
 
 struct bpf_nf_link {
@@ -119,3 +126,64 @@ int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 
 	return bpf_link_settle(&link_primer);
 }
+
+const struct bpf_prog_ops netfilter_prog_ops = {
+};
+
+static bool nf_ptr_to_btf_id(struct bpf_insn_access_aux *info, const char *name)
+{
+	struct btf *btf;
+	s32 type_id;
+
+	btf = bpf_get_btf_vmlinux();
+	if (IS_ERR_OR_NULL(btf))
+		return false;
+
+	type_id = btf_find_by_name_kind(btf, name, BTF_KIND_STRUCT);
+	if (WARN_ON_ONCE(type_id < 0))
+		return false;
+
+	info->btf = btf;
+	info->btf_id = type_id;
+	info->reg_type = PTR_TO_BTF_ID | PTR_TRUSTED;
+	return true;
+}
+
+static bool nf_is_valid_access(int off, int size, enum bpf_access_type type,
+			       const struct bpf_prog *prog,
+			       struct bpf_insn_access_aux *info)
+{
+	if (off < 0 || off >= sizeof(struct bpf_nf_ctx))
+		return false;
+
+	if (type == BPF_WRITE)
+		return false;
+
+	switch (off) {
+	case bpf_ctx_range(struct bpf_nf_ctx, skb):
+		if (size != sizeof_field(struct bpf_nf_ctx, skb))
+			return false;
+
+		return nf_ptr_to_btf_id(info, "sk_buff");
+	case bpf_ctx_range(struct bpf_nf_ctx, state):
+		if (size != sizeof_field(struct bpf_nf_ctx, state))
+			return false;
+
+		return nf_ptr_to_btf_id(info, "nf_hook_state");
+	default:
+		return false;
+	}
+
+	return false;
+}
+
+static const struct bpf_func_proto *
+bpf_nf_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+	return bpf_base_func_proto(func_id);
+}
+
+const struct bpf_verifier_ops netfilter_verifier_ops = {
+	.is_valid_access	= nf_is_valid_access,
+	.get_func_proto		= bpf_nf_func_proto,
+};
-- 
2.39.2

[PATCH bpf-next v2 3/6] netfilter: nfnetlink hook: dump bpf prog id

[Florian Westphal]

This allows userspace ("nft list hooks") to show which bpf program
is attached to which hook.

Without this, user only knows bpf prog is attached at prio
x, y, z at INPUT and FORWARD, but can't tell which program is where.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/uapi/linux/netfilter/nfnetlink_hook.h | 20 ++++-
 net/netfilter/nfnetlink_hook.c                | 81 ++++++++++++++++---
 2 files changed, 87 insertions(+), 14 deletions(-)

diff --git a/include/uapi/linux/netfilter/nfnetlink_hook.h b/include/uapi/linux/netfilter/nfnetlink_hook.h
index bbcd285b22e1..63b7dbddf0b1 100644
--- a/include/uapi/linux/netfilter/nfnetlink_hook.h
+++ b/include/uapi/linux/netfilter/nfnetlink_hook.h
@@ -32,8 +32,12 @@ enum nfnl_hook_attributes {
 /**
  * enum nfnl_hook_chain_info_attributes - chain description
  *
- * NFNLA_HOOK_INFO_DESC: nft chain and table name (enum nft_table_attributes) (NLA_NESTED)
+ * NFNLA_HOOK_INFO_DESC: nft chain and table name (NLA_NESTED)
  * NFNLA_HOOK_INFO_TYPE: chain type (enum nfnl_hook_chaintype) (NLA_U32)
+ *
+ * NFNLA_HOOK_INFO_DESC depends on NFNLA_HOOK_INFO_TYPE value:
+ *   NFNL_HOOK_TYPE_NFTABLES: enum nft_table_attributes
+ *   NFNL_HOOK_TYPE_BPF: enum nfnl_hook_bpf_info_attributes
  */
 enum nfnl_hook_chain_info_attributes {
 	NFNLA_HOOK_INFO_UNSPEC,
@@ -56,9 +60,23 @@ enum nfnl_hook_chain_desc_attributes {
  * enum nfnl_hook_chaintype - chain type
  *
  * @NFNL_HOOK_TYPE_NFTABLES nf_tables base chain
+ * @NFNL_HOOK_TYPE_BPF bpf program
  */
 enum nfnl_hook_chaintype {
 	NFNL_HOOK_TYPE_NFTABLES = 0x1,
+	NFNL_HOOK_TYPE_BPF,
+};
+
+/**
+ * enum nfnl_hook_bpf_info_attributes - bpf prog description
+ *
+ * NFNLA_BPF_INFO_ID: bpf program id (NLA_U32)
+ */
+enum nfnl_hook_bpf_attributes {
+	NFNLA_HOOK_BPF_UNSPEC,
+	NFNLA_HOOK_BPF_ID,
+	__NFNLA_HOOK_BPF_MAX,
 };
+#define NFNLA_HOOK_BPF_MAX (__NFNLA_HOOK_BPF_MAX - 1)
 
 #endif /* _NFNL_HOOK_H */
diff --git a/net/netfilter/nfnetlink_hook.c b/net/netfilter/nfnetlink_hook.c
index 8120aadf6a0f..ade8ee1988b1 100644
--- a/net/netfilter/nfnetlink_hook.c
+++ b/net/netfilter/nfnetlink_hook.c
@@ -5,6 +5,7 @@
  * Author: Florian Westphal <fw@strlen.de>
  */
 
+#include <linux/bpf.h>
 #include <linux/module.h>
 #include <linux/kallsyms.h>
 #include <linux/kernel.h>
@@ -57,35 +58,76 @@ struct nfnl_dump_hook_data {
 	u8 hook;
 };
 
+static struct nlattr *nfnl_start_info_type(struct sk_buff *nlskb, enum nfnl_hook_chaintype t)
+{
+	struct nlattr *nest = nla_nest_start(nlskb, NFNLA_HOOK_CHAIN_INFO);
+	int ret;
+
+	if (!nest)
+		return NULL;
+
+	ret = nla_put_be32(nlskb, NFNLA_HOOK_INFO_TYPE, htonl(t));
+	if (ret == 0)
+		return nest;
+
+	nla_nest_cancel(nlskb, nest);
+	return NULL;
+}
+
+static int nfnl_hook_put_bpf_prog_info(struct sk_buff *nlskb,
+				       const struct nfnl_dump_hook_data *ctx,
+				       unsigned int seq,
+				       const struct bpf_prog *prog)
+{
+	struct nlattr *nest, *nest2;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_NETFILTER_BPF_LINK))
+		return 0;
+
+	if (WARN_ON_ONCE(!prog))
+		return 0;
+
+	nest = nfnl_start_info_type(nlskb, NFNL_HOOK_TYPE_BPF);
+	if (!nest)
+		return -EMSGSIZE;
+
+	nest2 = nla_nest_start(nlskb, NFNLA_HOOK_INFO_DESC);
+	if (!nest2)
+		goto cancel_nest;
+
+	ret = nla_put_be32(nlskb, NFNLA_HOOK_BPF_ID, htonl(prog->aux->id));
+	if (ret)
+		goto cancel_nest;
+
+	nla_nest_end(nlskb, nest2);
+	nla_nest_end(nlskb, nest);
+	return 0;
+
+cancel_nest:
+	nla_nest_cancel(nlskb, nest);
+	return -EMSGSIZE;
+}
+
 static int nfnl_hook_put_nft_chain_info(struct sk_buff *nlskb,
 					const struct nfnl_dump_hook_data *ctx,
 					unsigned int seq,
-					const struct nf_hook_ops *ops)
+					struct nft_chain *chain)
 {
 	struct net *net = sock_net(nlskb->sk);
 	struct nlattr *nest, *nest2;
-	struct nft_chain *chain;
 	int ret = 0;
 
-	if (ops->hook_ops_type != NF_HOOK_OP_NF_TABLES)
-		return 0;
-
-	chain = ops->priv;
 	if (WARN_ON_ONCE(!chain))
 		return 0;
 
 	if (!nft_is_active(net, chain))
 		return 0;
 
-	nest = nla_nest_start(nlskb, NFNLA_HOOK_CHAIN_INFO);
+	nest = nfnl_start_info_type(nlskb, NFNL_HOOK_TYPE_NFTABLES);
 	if (!nest)
 		return -EMSGSIZE;
 
-	ret = nla_put_be32(nlskb, NFNLA_HOOK_INFO_TYPE,
-			   htonl(NFNL_HOOK_TYPE_NFTABLES));
-	if (ret)
-		goto cancel_nest;
-
 	nest2 = nla_nest_start(nlskb, NFNLA_HOOK_INFO_DESC);
 	if (!nest2)
 		goto cancel_nest;
@@ -171,7 +213,20 @@ static int nfnl_hook_dump_one(struct sk_buff *nlskb,
 	if (ret)
 		goto nla_put_failure;
 
-	ret = nfnl_hook_put_nft_chain_info(nlskb, ctx, seq, ops);
+	switch (ops->hook_ops_type) {
+	case NF_HOOK_OP_NF_TABLES:
+		ret = nfnl_hook_put_nft_chain_info(nlskb, ctx, seq, ops->priv);
+		break;
+	case NF_HOOK_OP_BPF:
+		ret = nfnl_hook_put_bpf_prog_info(nlskb, ctx, seq, ops->priv);
+		break;
+	case NF_HOOK_OP_UNDEFINED:
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		break;
+	}
+
 	if (ret)
 		goto nla_put_failure;
 
-- 
2.39.2

[PATCH bpf-next v2 4/6] netfilter: disallow bpf hook attachment at same priority

[Florian Westphal]

This is just to avoid ordering issues between multiple bpf programs,
this could be removed later in case it turns out to be too cautious.

bpf prog could still be shared with non-bpf hook, otherwise we'd have to
make conntrack hook registration fail just because a bpf program has
same priority.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 358220b58521..f0783e42108b 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -119,6 +119,18 @@ nf_hook_entries_grow(const struct nf_hook_entries *old,
 		for (i = 0; i < old_entries; i++) {
 			if (orig_ops[i] != &dummy_ops)
 				alloc_entries++;
+
+			/* Restrict BPF hook type to force a unique priority, not
+			 * shared at attach time.
+			 *
+			 * This is mainly to avoid ordering issues between two
+			 * different bpf programs, this doesn't prevent a normal
+			 * hook at same priority as a bpf one (we don't want to
+			 * prevent defrag, conntrack, iptables etc from attaching).
+			 */
+			if (reg->priority == orig_ops[i]->priority &&
+			    reg->hook_ops_type == NF_HOOK_OP_BPF)
+				return ERR_PTR(-EBUSY);
 		}
 	}
 
-- 
2.39.2

[PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info

[Florian Westphal]

Dump protocol family, hook and priority value:
$ bpftool link
2: type 10  prog 20
        pf: 2, hook 1, prio -128

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tools/bpf/bpftool/link.c       | 24 ++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 15 +++++++++++++++
 tools/lib/bpf/libbpf.c         |  1 +
 3 files changed, 40 insertions(+)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index f985b79cca27..a2ea85d1ebbf 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -135,6 +135,18 @@ static void show_iter_json(struct bpf_link_info *info, json_writer_t *wtr)
 	}
 }
 
+static void show_netfilter_json(const struct bpf_link_info *info, json_writer_t *wtr)
+{
+	jsonw_uint_field(json_wtr, "pf",
+			 info->netfilter.pf);
+	jsonw_uint_field(json_wtr, "hook",
+			 info->netfilter.hooknum);
+	jsonw_int_field(json_wtr, "prio",
+			 info->netfilter.priority);
+	jsonw_uint_field(json_wtr, "flags",
+			 info->netfilter.flags);
+}
+
 static int get_prog_info(int prog_id, struct bpf_prog_info *info)
 {
 	__u32 len = sizeof(*info);
@@ -195,6 +207,10 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
 				 info->netns.netns_ino);
 		show_link_attach_type_json(info->netns.attach_type, json_wtr);
 		break;
+	case BPF_LINK_TYPE_NETFILTER:
+		show_netfilter_json(info, json_wtr);
+		break;
+
 	default:
 		break;
 	}
@@ -301,6 +317,14 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
 		printf("\n\tnetns_ino %u  ", info->netns.netns_ino);
 		show_link_attach_type_plain(info->netns.attach_type);
 		break;
+	case BPF_LINK_TYPE_NETFILTER:
+		printf("\n\tpf: %d, hook %u, prio %d",
+		       info->netfilter.pf,
+		       info->netfilter.hooknum,
+		       info->netfilter.priority);
+		if (info->netfilter.flags)
+			printf(" flags 0x%x", info->netfilter.flags);
+		break;
 	default:
 		break;
 	}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 3823100b7934..c93febc4c75f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -986,6 +986,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_LSM,
 	BPF_PROG_TYPE_SK_LOOKUP,
 	BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
+	BPF_PROG_TYPE_NETFILTER,
 };
 
 enum bpf_attach_type {
@@ -1050,6 +1051,7 @@ enum bpf_link_type {
 	BPF_LINK_TYPE_PERF_EVENT = 7,
 	BPF_LINK_TYPE_KPROBE_MULTI = 8,
 	BPF_LINK_TYPE_STRUCT_OPS = 9,
+	BPF_LINK_TYPE_NETFILTER = 10,
 
 	MAX_BPF_LINK_TYPE,
 };
@@ -1560,6 +1562,13 @@ union bpf_attr {
 				 */
 				__u64		cookie;
 			} tracing;
+			struct {
+				__u32		pf;
+				__u32		hooknum;
+				__s32		prio;
+				__u32		flags;
+				__u64		reserved[2];
+			} netfilter;
 		};
 	} link_create;
 
@@ -6410,6 +6419,12 @@ struct bpf_link_info {
 		struct {
 			__u32 map_id;
 		} struct_ops;
+		struct {
+			__u32 pf;
+			__u32 hooknum;
+			__s32 priority;
+			__u32 flags;
+		} netfilter;
 	};
 } __attribute__((aligned(8)));
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 49cd304ae3bc..ae27451002ae 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8641,6 +8641,7 @@ static const struct bpf_sec_def section_defs[] = {
 	SEC_DEF("struct_ops+",		STRUCT_OPS, 0, SEC_NONE),
 	SEC_DEF("struct_ops.s+",	STRUCT_OPS, 0, SEC_SLEEPABLE),
 	SEC_DEF("sk_lookup",		SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE),
+	SEC_DEF("netfilter",		NETFILTER, 0, SEC_NONE),
 };
 
 static size_t custom_sec_def_cnt;
-- 
2.39.2

[PATCH bpf-next v2 6/6] bpf: add test_run support for netfilter program type

[Florian Westphal]

also extend prog_tests with a small retval test: values other
than accept or drop (0, 1) will cause issues.

NF_QUEUE could be implemented later if we can guarantee that attachment
of such programs can be rejected if they get attached to a pf/hook that
doesn't support async reinjection.

NF_STOLEN could be implemented via trusted helpers that can guarantee
that the skb will eventually be free'd.

$ ./test_progs --allow=verifier_netfilter_retcode
 #278/1   verifier_netfilter_retcode/bpf_exit with invalid return code. test1:OK
 #278/2   verifier_netfilter_retcode/bpf_exit with valid return code. test2:OK
 #278/3   verifier_netfilter_retcode/bpf_exit with valid return code. test3:OK
 #278/4   verifier_netfilter_retcode/bpf_exit with invalid return code. test4:OK
 #278     verifier_netfilter_retcode:OK

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: use progs/, not old verifier tests
     avoid unused variable warning
 include/linux/bpf.h                           |   3 +
 net/bpf/test_run.c                            | 140 ++++++++++++++++++
 net/netfilter/nf_bpf_link.c                   |   1 +
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_netfilter_retcode.c    |  49 ++++++
 5 files changed, 195 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_netfilter_retcode.c

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2c6095bd7d69..12bdd6d4bb20 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2234,6 +2234,9 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
 int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
 				const union bpf_attr *kattr,
 				union bpf_attr __user *uattr);
+int bpf_prog_test_run_nf(struct bpf_prog *prog,
+			 const union bpf_attr *kattr,
+			 union bpf_attr __user *uattr);
 bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 		    const struct bpf_prog *prog,
 		    struct bpf_insn_access_aux *info);
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 68bdfc041a7b..2d925d3ef2e3 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -19,7 +19,9 @@
 #include <linux/error-injection.h>
 #include <linux/smp.h>
 #include <linux/sock_diag.h>
+#include <linux/netfilter.h>
 #include <net/xdp.h>
+#include <net/netfilter/nf_bpf_link.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/bpf_test_run.h>
@@ -1696,6 +1698,144 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
 	return err;
 }
 
+static int verify_and_copy_hook_state(struct nf_hook_state *state,
+				      const struct nf_hook_state *user,
+				      struct net_device *dev)
+{
+	if (user->in || user->out)
+		return -EINVAL;
+
+	if (user->net || user->sk || user->okfn)
+		return -EINVAL;
+
+	switch (user->pf) {
+	case NFPROTO_IPV4:
+	case NFPROTO_IPV6:
+		switch (state->hook) {
+		case NF_INET_PRE_ROUTING:
+			state->in = dev;
+			break;
+		case NF_INET_LOCAL_IN:
+			state->in = dev;
+			break;
+		case NF_INET_FORWARD:
+			state->in = dev;
+			state->out = dev;
+			break;
+		case NF_INET_LOCAL_OUT:
+			state->out = dev;
+			break;
+		case NF_INET_POST_ROUTING:
+			state->out = dev;
+			break;
+		}
+
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	state->pf = user->pf;
+	state->hook = user->hook;
+
+	return 0;
+}
+
+int bpf_prog_test_run_nf(struct bpf_prog *prog,
+			 const union bpf_attr *kattr,
+			 union bpf_attr __user *uattr)
+{
+	struct net *net = current->nsproxy->net_ns;
+	struct net_device *dev = net->loopback_dev;
+	struct nf_hook_state *user_ctx, hook_state = {
+		.pf = NFPROTO_IPV4,
+		.hook = NF_INET_PRE_ROUTING,
+	};
+	u32 size = kattr->test.data_size_in;
+	u32 repeat = kattr->test.repeat;
+	struct bpf_nf_ctx ctx = {
+		.state = &hook_state,
+	};
+	struct sk_buff *skb = NULL;
+	u32 retval, duration;
+	void *data;
+	int ret;
+
+	if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
+		return -EINVAL;
+
+	if (size < ETH_HLEN + sizeof(struct iphdr))
+		return -EINVAL;
+
+	data = bpf_test_init(kattr, kattr->test.data_size_in, size,
+			     NET_SKB_PAD + NET_IP_ALIGN,
+			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	if (!repeat)
+		repeat = 1;
+
+	user_ctx = bpf_ctx_init(kattr, sizeof(struct nf_hook_state));
+	if (IS_ERR(user_ctx)) {
+		kfree(data);
+		return PTR_ERR(user_ctx);
+	}
+
+	if (user_ctx) {
+		ret = verify_and_copy_hook_state(&hook_state, user_ctx, dev);
+		if (ret)
+			goto out;
+	}
+
+	skb = slab_build_skb(data);
+	if (!skb) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	data = NULL; /* data released via kfree_skb */
+
+	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
+	__skb_put(skb, size);
+
+	skb->protocol = eth_type_trans(skb, dev);
+
+	skb_reset_network_header(skb);
+
+	ret = -EINVAL;
+
+	switch (skb->protocol) {
+	case htons(ETH_P_IP):
+		if (hook_state.pf == NFPROTO_IPV4)
+			break;
+		goto out;
+	case htons(ETH_P_IPV6):
+		if (size < ETH_HLEN + sizeof(struct ipv6hdr))
+			goto out;
+		if (hook_state.pf == NFPROTO_IPV6)
+			break;
+		goto out;
+	default:
+		ret = -EPROTO;
+		goto out;
+	}
+
+	ctx.skb = skb;
+
+	ret = bpf_test_run(prog, &ctx, repeat, &retval, &duration, false);
+	if (ret)
+		goto out;
+
+	ret = bpf_test_finish(kattr, uattr, NULL, NULL, 0, retval, duration);
+
+out:
+	kfree(user_ctx);
+	kfree_skb(skb);
+	kfree(data);
+	return ret;
+}
+
 static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
 	.owner = THIS_MODULE,
 	.set   = &test_sk_check_kfunc_ids,
diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c
index 4b22a31d6df5..c27fd569adf1 100644
--- a/net/netfilter/nf_bpf_link.c
+++ b/net/netfilter/nf_bpf_link.c
@@ -128,6 +128,7 @@ int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 }
 
 const struct bpf_prog_ops netfilter_prog_ops = {
+	.test_run = bpf_prog_test_run_nf,
 };
 
 static bool nf_ptr_to_btf_id(struct bpf_insn_access_aux *info, const char *name)
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 73dff693d411..e3b042e274f3 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -29,6 +29,7 @@
 #include "verifier_map_ret_val.skel.h"
 #include "verifier_masking.skel.h"
 #include "verifier_meta_access.skel.h"
+#include "verifier_netfilter_retcode.skel.h"
 #include "verifier_raw_stack.skel.h"
 #include "verifier_raw_tp_writable.skel.h"
 #include "verifier_ringbuf.skel.h"
@@ -93,6 +94,7 @@ void test_verifier_map_ptr(void)              { RUN(verifier_map_ptr); }
 void test_verifier_map_ret_val(void)          { RUN(verifier_map_ret_val); }
 void test_verifier_masking(void)              { RUN(verifier_masking); }
 void test_verifier_meta_access(void)          { RUN(verifier_meta_access); }
+void test_verifier_netfilter_retcode(void)    { RUN(verifier_netfilter_retcode); }
 void test_verifier_raw_stack(void)            { RUN(verifier_raw_stack); }
 void test_verifier_raw_tp_writable(void)      { RUN(verifier_raw_tp_writable); }
 void test_verifier_ringbuf(void)              { RUN(verifier_ringbuf); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_netfilter_retcode.c b/tools/testing/selftests/bpf/progs/verifier_netfilter_retcode.c
new file mode 100644
index 000000000000..353ae6da00e1
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_netfilter_retcode.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+SEC("netfilter")
+__description("bpf_exit with invalid return code. test1")
+__failure __msg("R0 is not a known value")
+__naked void with_invalid_return_code_test1(void)
+{
+	asm volatile ("					\
+	r0 = *(u64*)(r1 + 0);				\
+	exit;						\
+"	::: __clobber_all);
+}
+
+SEC("netfilter")
+__description("bpf_exit with valid return code. test2")
+__success
+__naked void with_valid_return_code_test2(void)
+{
+	asm volatile ("					\
+	r0 = 0;						\
+	exit;						\
+"	::: __clobber_all);
+}
+
+SEC("netfilter")
+__description("bpf_exit with valid return code. test3")
+__success
+__naked void with_valid_return_code_test3(void)
+{
+	asm volatile ("					\
+	r0 = 1;						\
+	exit;						\
+"	::: __clobber_all);
+}
+
+SEC("netfilter")
+__description("bpf_exit with invalid return code. test4")
+__failure __msg("R0 has value (0x2; 0x0)")
+__naked void with_invalid_return_code_test4(void)
+{
+	asm volatile ("					\
+	r0 = 2;						\
+	exit;						\
+"	::: __clobber_all);
+}
-- 
2.39.2

Re: [PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info

[Quentin Monnet]

Hi Florian,

On Thu, 13 Apr 2023 at 14:36, Florian Westphal <fw@strlen.de> wrote:
>
> Dump protocol family, hook and priority value:
> $ bpftool link
> 2: type 10  prog 20

Could you please update link_type_name in libbpf (libbpf.c) so that we
display "netfilter" here instead of "type 10"?

>         pf: 2, hook 1, prio -128
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  tools/bpf/bpftool/link.c       | 24 ++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h | 15 +++++++++++++++
>  tools/lib/bpf/libbpf.c         |  1 +
>  3 files changed, 40 insertions(+)
>
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index f985b79cca27..a2ea85d1ebbf 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
> @@ -135,6 +135,18 @@ static void show_iter_json(struct bpf_link_info *info, json_writer_t *wtr)
>         }
>  }
>
> +static void show_netfilter_json(const struct bpf_link_info *info, json_writer_t *wtr)
> +{
> +       jsonw_uint_field(json_wtr, "pf",
> +                        info->netfilter.pf);
> +       jsonw_uint_field(json_wtr, "hook",
> +                        info->netfilter.hooknum);
> +       jsonw_int_field(json_wtr, "prio",
> +                        info->netfilter.priority);
> +       jsonw_uint_field(json_wtr, "flags",
> +                        info->netfilter.flags);
> +}
> +
>  static int get_prog_info(int prog_id, struct bpf_prog_info *info)
>  {
>         __u32 len = sizeof(*info);
> @@ -195,6 +207,10 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
>                                  info->netns.netns_ino);
>                 show_link_attach_type_json(info->netns.attach_type, json_wtr);
>                 break;
> +       case BPF_LINK_TYPE_NETFILTER:
> +               show_netfilter_json(info, json_wtr);
> +               break;
> +
>         default:
>                 break;
>         }
> @@ -301,6 +317,14 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
>                 printf("\n\tnetns_ino %u  ", info->netns.netns_ino);
>                 show_link_attach_type_plain(info->netns.attach_type);
>                 break;
> +       case BPF_LINK_TYPE_NETFILTER:
> +               printf("\n\tpf: %d, hook %u, prio %d",
> +                      info->netfilter.pf,
> +                      info->netfilter.hooknum,
> +                      info->netfilter.priority);
> +               if (info->netfilter.flags)
> +                       printf(" flags 0x%x", info->netfilter.flags);
> +               break;
>         default:
>                 break;
>         }
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 3823100b7934..c93febc4c75f 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -986,6 +986,7 @@ enum bpf_prog_type {
>         BPF_PROG_TYPE_LSM,
>         BPF_PROG_TYPE_SK_LOOKUP,
>         BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
> +       BPF_PROG_TYPE_NETFILTER,

If netfilter programs could be loaded with bpftool, we'd need to
update bpftool's docs. But I don't think this is the case, right? We
don't currently have a way to pass the pf, hooknum, priority and flags
necessary to load the program with "bpftool prog load" so it would
fail?

Have you considered listing netfilter programs in the output of
"bpftool net" as well? Given that they're related to networking, it
would maybe make sense to have them listed alongside XDP, TC, and flow
dissector programs?

The patch looks good to me otherwise.

Thanks,
Quentin

Re: [PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info

[Florian Westphal]

Quentin Monnet <quentin@isovalent.com> wrote:
> On Thu, 13 Apr 2023 at 14:36, Florian Westphal <fw@strlen.de> wrote:
> >
> > Dump protocol family, hook and priority value:
> > $ bpftool link
> > 2: type 10  prog 20
> 
> Could you please update link_type_name in libbpf (libbpf.c) so that we
> display "netfilter" here instead of "type 10"?

Done.

> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > index 3823100b7934..c93febc4c75f 100644
> > --- a/tools/include/uapi/linux/bpf.h
> > +++ b/tools/include/uapi/linux/bpf.h
> > @@ -986,6 +986,7 @@ enum bpf_prog_type {
> >         BPF_PROG_TYPE_LSM,
> >         BPF_PROG_TYPE_SK_LOOKUP,
> >         BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
> > +       BPF_PROG_TYPE_NETFILTER,
> 
> If netfilter programs could be loaded with bpftool, we'd need to
> update bpftool's docs. But I don't think this is the case, right?

bpftool prog load nftest.o /sys/fs/bpf/nftest

will work, but the program isn't attached anywhere.

> don't currently have a way to pass the pf, hooknum, priority and flags
> necessary to load the program with "bpftool prog load" so it would
> fail?

I don't know how to make it work to actually attach it, because
the hook is unregistered when the link fd is closed.

So either bpftool would have to fork and auto-daemon (maybe
unexpected...) or wait/block until CTRL-C.

This also needs new libbpf api AFAICS because existing bpf_link
are specific to the program type, so I'd have to add something like:

struct bpf_link *
bpf_program__attach_netfilter(const struct bpf_program *prog,
			      const struct bpf_netfilter_opts *opts)

Advice welcome.

> Have you considered listing netfilter programs in the output of
> "bpftool net" as well? Given that they're related to networking, it
> would maybe make sense to have them listed alongside XDP, TC, and flow
> dissector programs?

I could print the same output that 'bpf link' already shows.

Not sure on the real distinction between those two here.

When should I use 'bpftool link' and when 'bpftool net', and what info
and features should either of these provide for netfilter programs?

Re: [PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info

[Quentin Monnet]

2023-04-14 12:41 UTC+0200 ~ Florian Westphal <fw@strlen.de>
> Quentin Monnet <quentin@isovalent.com> wrote:
>> On Thu, 13 Apr 2023 at 14:36, Florian Westphal <fw@strlen.de> wrote:
>>>
>>> Dump protocol family, hook and priority value:
>>> $ bpftool link
>>> 2: type 10  prog 20
>>
>> Could you please update link_type_name in libbpf (libbpf.c) so that we
>> display "netfilter" here instead of "type 10"?
> 
> Done.

Thanks!

I'm just thinking we could also maybe print something nicer for the pf
and the hook, "NF_INET_LOCAL_IN" would be more user-friendly than "hook 1"?

> 
>>> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
>>> index 3823100b7934..c93febc4c75f 100644
>>> --- a/tools/include/uapi/linux/bpf.h
>>> +++ b/tools/include/uapi/linux/bpf.h
>>> @@ -986,6 +986,7 @@ enum bpf_prog_type {
>>>         BPF_PROG_TYPE_LSM,
>>>         BPF_PROG_TYPE_SK_LOOKUP,
>>>         BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
>>> +       BPF_PROG_TYPE_NETFILTER,
>>
>> If netfilter programs could be loaded with bpftool, we'd need to
>> update bpftool's docs. But I don't think this is the case, right?
> 
> bpftool prog load nftest.o /sys/fs/bpf/nftest
> 
> will work, but the program isn't attached anywhere.

Let's maybe not document it, then. It may still be useful to check
whether a program load, but users would definitely expect the program to
remain loaded after bpftool invocation has completed. Or alternatively,
we could document, but print a warning.

> 
>> don't currently have a way to pass the pf, hooknum, priority and flags
>> necessary to load the program with "bpftool prog load" so it would
>> fail?
> 
> I don't know how to make it work to actually attach it, because
> the hook is unregistered when the link fd is closed.
> 
> So either bpftool would have to fork and auto-daemon (maybe
> unexpected...) or wait/block until CTRL-C.
> 
> This also needs new libbpf api AFAICS because existing bpf_link
> are specific to the program type, so I'd have to add something like:
> 
> struct bpf_link *
> bpf_program__attach_netfilter(const struct bpf_program *prog,
> 			      const struct bpf_netfilter_opts *opts)
> 
> Advice welcome.

OK, yes we'd need something like this if we wanted to load and attach
from bpftool. If you already have the tooling elsewhere, it's maybe not
necessary to add it here. Depends if you want users to be able to attach
netfilter programs with bpftool or even libbpf.

There are other program types that are not supported for
loading/attaching with bpftool (the bpftool-prog man page is not always
correct in that regard, I think).

I'd say let's keep this out of the current patchset anyway. If we have a
use case for attaching via libbpf/bpftool we can do this as a follow-up.

> 
>> Have you considered listing netfilter programs in the output of
>> "bpftool net" as well? Given that they're related to networking, it
>> would maybe make sense to have them listed alongside XDP, TC, and flow
>> dissector programs?
> 
> I could print the same output that 'bpf link' already shows.
> 
> Not sure on the real distinction between those two here.

There would probably be some overlap (to say the least), yes.

> 
> When should I use 'bpftool link' and when 'bpftool net', and what info
> and features should either of these provide for netfilter programs?

That's a good question. I thought I'd check how we handle it for XDP for
"bpftool net" vs. "bpftool link", but I realised this link type (and
some others) were never added to the switch/case you update in
bpftool/link.c, and we're not printing any particular information about
them beyond type and associated program id. Conversely, I'd have to
check whether we print XDP programs using links in "bpftool net". Maybe
some things to improve here. Anyway.

The way I see it, "bpftool net" should provide a more structured
overview of the different programs affecting networking, in particular
for JSON. The idea would be to display all BPF programs that can affect
packet processing. See what we have for XDP for example:


    # bpftool net -p
    [{
            "xdp": [{
                    "devname": "eni88np1",
                    "ifindex": 12,
                    "multi_attachments": [{
                            "mode": "driver",
                            "id": 1238
                        },{
                            "mode": "offload",
                            "id": 1239
                        }
                    ]
                }
            ],
            "tc": [{
                    "devname": "eni88np1",
                    "ifindex": 12,
                    "kind": "clsact/ingress",
                    "name": "sample_ret0.o:[.text]",
                    "id": 1241
                },{
                    "devname": "eni88np1",
                    "ifindex": 12,
                    "kind": "clsact/ingress",
                    "name": "sample_ret0.o:[.text]",
                    "id": 1240
                }
            ],
            "flow_dissector": [
                "id": 1434
            ]
        }
    ]

This gives us all the info about XDP programs at once, grouped by device
when relevant. By contrast, listing them in "bpftool link" would likely
only show one at a time, in an uncorrelated manner. Similarly, we could
have netfilter sorted by pf then hook in "bpftool net". If there's more
relevant info that we get from program info and not from the netfilter
link, this would also be a good place to have it (but not sure there's
any info we're missing from "bpftool link"?).

But given that the info will be close, or identical, if not for the JSON
structure, I don't mean to impose this to you - it's also OK to just
skip "bpftool net" for now if you prefer.

Quentin

Re: [PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info

[Florian Westphal]

Quentin Monnet <quentin@isovalent.com> wrote:
> 2023-04-14 12:41 UTC+0200 ~ Florian Westphal <fw@strlen.de>
> > Quentin Monnet <quentin@isovalent.com> wrote:
> >> On Thu, 13 Apr 2023 at 14:36, Florian Westphal <fw@strlen.de> wrote:
> >>>
> >>> Dump protocol family, hook and priority value:
> >>> $ bpftool link
> >>> 2: type 10  prog 20
> >>
> >> Could you please update link_type_name in libbpf (libbpf.c) so that we
> >> display "netfilter" here instead of "type 10"?
> > 
> > Done.
> 
> Thanks!
> 
> I'm just thinking we could also maybe print something nicer for the pf
> and the hook, "NF_INET_LOCAL_IN" would be more user-friendly than "hook 1"?

Done.  I've also made the first patch more restrictive wrt. allowed
attachment points and priorities.

Better safe than sorry, we can be more liberal later if there are
use-cases.

v3 will be coming next week.

> > I don't know how to make it work to actually attach it, because
> > the hook is unregistered when the link fd is closed.
> > 
> > So either bpftool would have to fork and auto-daemon (maybe
> > unexpected...) or wait/block until CTRL-C.
> > 
> > This also needs new libbpf api AFAICS because existing bpf_link
> > are specific to the program type, so I'd have to add something like:
> > 
> > struct bpf_link *
> > bpf_program__attach_netfilter(const struct bpf_program *prog,
> > 			      const struct bpf_netfilter_opts *opts)
> > 
> > Advice welcome.
> 
> OK, yes we'd need something like this if we wanted to load and attach
> from bpftool. If you already have the tooling elsewhere, it's maybe not
> necessary to add it here. Depends if you want users to be able to attach
> netfilter programs with bpftool or even libbpf.

[..]

> I'd say let's keep this out of the current patchset anyway. If we have a
> use case for attaching via libbpf/bpftool we can do this as a follow-up.

Sounds good to me.

Quentin Deslandes or Daniel Xu might want/need libbpf support for their
projects.

> The way I see it, "bpftool net" should provide a more structured
> overview of the different programs affecting networking, in particular
> for JSON. The idea would be to display all BPF programs that can affect
> packet processing. See what we have for XDP for example:
> 
> 
>     # bpftool net -p
>     [{
>             "xdp": [{
>                     "devname": "eni88np1",
>                     "ifindex": 12,
>                     "multi_attachments": [{
>                             "mode": "driver",
>                             "id": 1238
>                         },{
>                             "mode": "offload",
>                             "id": 1239
>                         }
>                     ]
>                 }
>             ],
>             "tc": [{
>                     "devname": "eni88np1",
>                     "ifindex": 12,
>                     "kind": "clsact/ingress",
>                     "name": "sample_ret0.o:[.text]",
>                     "id": 1241
>                 },{
>                     "devname": "eni88np1",
>                     "ifindex": 12,
>                     "kind": "clsact/ingress",
>                     "name": "sample_ret0.o:[.text]",
>                     "id": 1240
>                 }
>             ],
>             "flow_dissector": [
>                 "id": 1434
>             ]
>         }
>     ]
> 
> This gives us all the info about XDP programs at once, grouped by device
> when relevant. By contrast, listing them in "bpftool link" would likely
> only show one at a time, in an uncorrelated manner. Similarly, we could
> have netfilter sorted by pf then hook in "bpftool net". If there's more
> relevant info that we get from program info and not from the netfilter
> link, this would also be a good place to have it (but not sure there's
> any info we're missing from "bpftool link"?).

Currently 'bpftool link' shows everything wrt. netfilter bpf programs.

> But given that the info will be close, or identical, if not for the JSON
> structure, I don't mean to impose this to you - it's also OK to just
> skip "bpftool net" for now if you prefer.

I'll probably make 'bpftool net' and 'bpftool link' print identical
netfilter output, I'll check this on Monday (to make sure the formatting
doesn't seem out of place).

Its kinda silly to not have anything netfilter related in 'bpftool
net', this thing isn't named 'linkfilter' after all 8-)

Re: [PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info

[Quentin Monnet]

2023-04-14 16:49 UTC+0200 ~ Florian Westphal <fw@strlen.de>
> Quentin Monnet <quentin@isovalent.com> wrote:
>> 2023-04-14 12:41 UTC+0200 ~ Florian Westphal <fw@strlen.de>
>>> Quentin Monnet <quentin@isovalent.com> wrote:
>>>> On Thu, 13 Apr 2023 at 14:36, Florian Westphal <fw@strlen.de> wrote:
>>>>>
>>>>> Dump protocol family, hook and priority value:
>>>>> $ bpftool link
>>>>> 2: type 10  prog 20
>>>>
>>>> Could you please update link_type_name in libbpf (libbpf.c) so that we
>>>> display "netfilter" here instead of "type 10"?
>>>
>>> Done.
>>
>> Thanks!
>>
>> I'm just thinking we could also maybe print something nicer for the pf
>> and the hook, "NF_INET_LOCAL_IN" would be more user-friendly than "hook 1"?
> 
> Done.  I've also made the first patch more restrictive wrt. allowed
> attachment points and priorities.
> 
> Better safe than sorry, we can be more liberal later if there are
> use-cases.
> 
> v3 will be coming next week.
> 
>>> I don't know how to make it work to actually attach it, because
>>> the hook is unregistered when the link fd is closed.
>>>
>>> So either bpftool would have to fork and auto-daemon (maybe
>>> unexpected...) or wait/block until CTRL-C.
>>>
>>> This also needs new libbpf api AFAICS because existing bpf_link
>>> are specific to the program type, so I'd have to add something like:
>>>
>>> struct bpf_link *
>>> bpf_program__attach_netfilter(const struct bpf_program *prog,
>>> 			      const struct bpf_netfilter_opts *opts)
>>>
>>> Advice welcome.
>>
>> OK, yes we'd need something like this if we wanted to load and attach
>> from bpftool. If you already have the tooling elsewhere, it's maybe not
>> necessary to add it here. Depends if you want users to be able to attach
>> netfilter programs with bpftool or even libbpf.
> 
> [..]
> 
>> I'd say let's keep this out of the current patchset anyway. If we have a
>> use case for attaching via libbpf/bpftool we can do this as a follow-up.
> 
> Sounds good to me.
> 
> Quentin Deslandes or Daniel Xu might want/need libbpf support for their
> projects.
> 
>> The way I see it, "bpftool net" should provide a more structured
>> overview of the different programs affecting networking, in particular
>> for JSON. The idea would be to display all BPF programs that can affect
>> packet processing. See what we have for XDP for example:
>>
>>
>>     # bpftool net -p
>>     [{
>>             "xdp": [{
>>                     "devname": "eni88np1",
>>                     "ifindex": 12,
>>                     "multi_attachments": [{
>>                             "mode": "driver",
>>                             "id": 1238
>>                         },{
>>                             "mode": "offload",
>>                             "id": 1239
>>                         }
>>                     ]
>>                 }
>>             ],
>>             "tc": [{
>>                     "devname": "eni88np1",
>>                     "ifindex": 12,
>>                     "kind": "clsact/ingress",
>>                     "name": "sample_ret0.o:[.text]",
>>                     "id": 1241
>>                 },{
>>                     "devname": "eni88np1",
>>                     "ifindex": 12,
>>                     "kind": "clsact/ingress",
>>                     "name": "sample_ret0.o:[.text]",
>>                     "id": 1240
>>                 }
>>             ],
>>             "flow_dissector": [
>>                 "id": 1434
>>             ]
>>         }
>>     ]
>>
>> This gives us all the info about XDP programs at once, grouped by device
>> when relevant. By contrast, listing them in "bpftool link" would likely
>> only show one at a time, in an uncorrelated manner. Similarly, we could
>> have netfilter sorted by pf then hook in "bpftool net". If there's more
>> relevant info that we get from program info and not from the netfilter
>> link, this would also be a good place to have it (but not sure there's
>> any info we're missing from "bpftool link"?).
> 
> Currently 'bpftool link' shows everything wrt. netfilter bpf programs.
> 
>> But given that the info will be close, or identical, if not for the JSON
>> structure, I don't mean to impose this to you - it's also OK to just
>> skip "bpftool net" for now if you prefer.
> 
> I'll probably make 'bpftool net' and 'bpftool link' print identical
> netfilter output, I'll check this on Monday (to make sure the formatting
> doesn't seem out of place).
> 
> Its kinda silly to not have anything netfilter related in 'bpftool
> net', this thing isn't named 'linkfilter' after all 8-)

Sounds all good to me :) Thanks!