netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v3 0/6] bpf: add netfilter program type
@ 2023-04-18 13:10 Florian Westphal
  2023-04-18 13:10 ` [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-18 13:10 UTC (permalink / raw)
  To: bpf; +Cc: netdev, netfilter-devel, dxu, qde, 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 v2:
1. don't WARN when user calls 'bpftool loink detach' twice
   restrict attachment to ip+ip6 families, lets relax this
   later in case arp/bridge/netdev are needed too.
2. show netfilter links in 'bpftool net' output as well.

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.

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                   | 229 ++++++++++++++++++
 net/netfilter/nfnetlink_hook.c                |  81 ++++++-
 tools/bpf/bpftool/link.c                      |  83 +++++++
 tools/bpf/bpftool/main.h                      |   3 +
 tools/bpf/bpftool/net.c                       | 105 ++++++++
 tools/include/uapi/linux/bpf.h                |  15 ++
 tools/lib/bpf/libbpf.c                        |   2 +
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_netfilter_retcode.c    |  49 ++++
 23 files changed, 785 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs
  2023-04-18 13:10 [PATCH bpf-next v3 0/6] bpf: add netfilter program type Florian Westphal
@ 2023-04-18 13:10 ` Florian Westphal
  2023-04-18 18:35   ` Alexei Starovoitov
  2023-04-19  5:57   ` kernel test robot
  2023-04-18 13:10 ` [PATCH bpf-next v3 2/6] bpf: minimal support for programs hooked into netfilter framework Florian Westphal
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-18 13:10 UTC (permalink / raw)
  To: bpf; +Cc: netdev, netfilter-devel, dxu, qde, 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.

v2:
 - add cmpxchg to prevent double-unreg via 'bpftool link detach',
   (gives WARN splat from netfilter core)
 - restrict to ip/ip6 for now, lets lift restrictions later as
   more use cases pop up (arptables, ebtables, netdev ingress/egress
   etc).

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 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         | 160 ++++++++++++++++++++++++++++
 7 files changed, 196 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 4b20a7269bee..be97cc6398a4 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 bcf1a1920ddd..14f39c1e573e 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 || \
@@ -2462,6 +2463,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 */
@@ -4588,6 +4590,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:
@@ -4654,6 +4657,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 d0bf630482c1..441d1f134110 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..0f937c6bee6d
--- /dev/null
+++ b/net/netfilter/nf_bpf_link.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <linux/netfilter.h>
+
+#include <net/netfilter/nf_bpf_link.h>
+#include <uapi/linux/netfilter_ipv4.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;
+	bool dead;
+};
+
+static void bpf_nf_link_release(struct bpf_link *link)
+{
+	struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
+
+	if (nf_link->dead)
+		return;
+
+	/* prevent hook-not-found warning splat from netfilter core when .detach was
+	 * already called */
+	if (!cmpxchg(&nf_link->dead, false, true))
+		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,
+};
+
+static int bpf_nf_check_pf_and_hooks(const union bpf_attr *attr)
+{
+	switch (attr->link_create.netfilter.pf) {
+	case NFPROTO_IPV4:
+	case NFPROTO_IPV6:
+		if (attr->link_create.netfilter.hooknum >= NF_INET_NUMHOOKS)
+			return -EPROTO;
+		break;
+	default:
+		return -EAFNOSUPPORT;
+	}
+
+	if (attr->link_create.netfilter.flags)
+		return -EOPNOTSUPP;
+
+	/* make sure conntrack confirm is always last.
+	 *
+	 * In the future, if userspace can e.g. request defrag, then
+	 * "defrag_requested && prio before NF_IP_PRI_CONNTRACK_DEFRAG"
+	 * should fail.
+	 */
+	switch (attr->link_create.netfilter.prio) {
+	case NF_IP_PRI_FIRST: return -ERANGE; /* sabotage_in and other warts */
+	case NF_IP_PRI_LAST: return -ERANGE; /* e.g. conntrack confirm */
+	}
+
+	return 0;
+}
+
+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.reserved[0] | attr->link_create.netfilter.reserved[1])
+		return -EINVAL;
+
+	err = bpf_nf_check_pf_and_hooks(attr);
+	if (err)
+		return err;
+
+	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;
+	link->dead = false;
+
+	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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next v3 2/6] bpf: minimal support for programs hooked into netfilter framework
  2023-04-18 13:10 [PATCH bpf-next v3 0/6] bpf: add netfilter program type Florian Westphal
  2023-04-18 13:10 ` [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
@ 2023-04-18 13:10 ` Florian Westphal
  2023-04-19  2:11   ` Alexei Starovoitov
  2023-04-18 13:10 ` [PATCH bpf-next v3 3/6] netfilter: nfnetlink hook: dump bpf prog id Florian Westphal
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Florian Westphal @ 2023-04-18 13:10 UTC (permalink / raw)
  To: bpf; +Cc: netdev, netfilter-devel, dxu, qde, 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 027f9f8a3551..3556bb68e3ec 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,
 };
 
@@ -7800,6 +7804,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 1e05355facdc..fc7281d39e46 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13816,6 +13816,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 44fb997434ad..d9ce04ca22ce 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -11717,6 +11717,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 0f937c6bee6d..2d12c978e4e7 100644
--- a/net/netfilter/nf_bpf_link.c
+++ b/net/netfilter/nf_bpf_link.c
@@ -1,5 +1,6 @@
 // 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>
@@ -7,7 +8,13 @@
 
 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 {
@@ -158,3 +165,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next v3 3/6] netfilter: nfnetlink hook: dump bpf prog id
  2023-04-18 13:10 [PATCH bpf-next v3 0/6] bpf: add netfilter program type Florian Westphal
  2023-04-18 13:10 ` [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
  2023-04-18 13:10 ` [PATCH bpf-next v3 2/6] bpf: minimal support for programs hooked into netfilter framework Florian Westphal
@ 2023-04-18 13:10 ` Florian Westphal
  2023-04-20  9:54   ` Simon Horman
  2023-04-18 13:10 ` [PATCH bpf-next v3 4/6] netfilter: disallow bpf hook attachment at same priority Florian Westphal
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Florian Westphal @ 2023-04-18 13:10 UTC (permalink / raw)
  To: bpf; +Cc: netdev, netfilter-devel, dxu, qde, 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next v3 4/6] netfilter: disallow bpf hook attachment at same priority
  2023-04-18 13:10 [PATCH bpf-next v3 0/6] bpf: add netfilter program type Florian Westphal
                   ` (2 preceding siblings ...)
  2023-04-18 13:10 ` [PATCH bpf-next v3 3/6] netfilter: nfnetlink hook: dump bpf prog id Florian Westphal
@ 2023-04-18 13:10 ` Florian Westphal
  2023-04-18 13:10 ` [PATCH bpf-next v3 5/6] tools: bpftool: print netfilter link info Florian Westphal
  2023-04-18 13:10 ` [PATCH bpf-next v3 6/6] bpf: add test_run support for netfilter program type Florian Westphal
  5 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-18 13:10 UTC (permalink / raw)
  To: bpf; +Cc: netdev, netfilter-devel, dxu, qde, 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next v3 5/6] tools: bpftool: print netfilter link info
  2023-04-18 13:10 [PATCH bpf-next v3 0/6] bpf: add netfilter program type Florian Westphal
                   ` (3 preceding siblings ...)
  2023-04-18 13:10 ` [PATCH bpf-next v3 4/6] netfilter: disallow bpf hook attachment at same priority Florian Westphal
@ 2023-04-18 13:10 ` Florian Westphal
  2023-04-18 14:20   ` Quentin Monnet
  2023-04-18 13:10 ` [PATCH bpf-next v3 6/6] bpf: add test_run support for netfilter program type Florian Westphal
  5 siblings, 1 reply; 14+ messages in thread
From: Florian Westphal @ 2023-04-18 13:10 UTC (permalink / raw)
  To: bpf; +Cc: netdev, netfilter-devel, dxu, qde, Florian Westphal,
	Quentin Monnet

Dump protocol family, hook and priority value:
$ bpftool link
2: netfilter  prog 14
        ip input prio -128
        pids install(3264)
5: netfilter  prog 14
        ip6 forward prio 21
        pids a.out(3387)
9: netfilter  prog 14
        ip prerouting prio 123
        pids a.out(5700)
10: netfilter  prog 14
        ip input prio 21
        pids test2(5701)

v2: Quentin Monnet suggested to also add 'bpftool net' support:

$ bpftool net
xdp:

tc:

flow_dissector:

netfilter:

        ip prerouting prio 21 prog_id 14
        ip input prio -128 prog_id 14
        ip input prio 21 prog_id 14
        ip forward prio 21 prog_id 14
        ip output prio 21 prog_id 14
        ip postrouting prio 21 prog_id 14

'bpftool net' only dumps netfilter link type.  netfilter links are sorted by
protocol family, hook and priority.

Suggested-by: Quentin Monnet <quentin@isovalent.com>
Link: https://lore.kernel.org/bpf/eeeaac99-9053-90c2-aa33-cc1ecb1ae9ca@isovalent.com/
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tools/bpf/bpftool/link.c       |  83 ++++++++++++++++++++++++++
 tools/bpf/bpftool/main.h       |   3 +
 tools/bpf/bpftool/net.c        | 105 +++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h |  15 +++++
 tools/lib/bpf/libbpf.c         |   2 +
 5 files changed, 208 insertions(+)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index f985b79cca27..96a3c458a632 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -3,6 +3,8 @@
 
 #include <errno.h>
 #include <linux/err.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter_arp.h>
 #include <net/if.h>
 #include <stdio.h>
 #include <unistd.h>
@@ -135,6 +137,18 @@ static void show_iter_json(struct bpf_link_info *info, json_writer_t *wtr)
 	}
 }
 
+void netfilter_dump_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 +209,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:
+		netfilter_dump_json(info, json_wtr);
+		break;
+
 	default:
 		break;
 	}
@@ -263,6 +281,68 @@ static void show_iter_plain(struct bpf_link_info *info)
 	}
 }
 
+static const char *pf2name[] = {
+	[NFPROTO_INET] = "inet",
+	[NFPROTO_IPV4] = "ip",
+	[NFPROTO_ARP] = "arp",
+	[NFPROTO_NETDEV] = "netdev",
+	[NFPROTO_BRIDGE] = "bridge",
+	[NFPROTO_IPV6] = "ip6",
+};
+
+static const char *inethook2name[] = {
+	[NF_INET_PRE_ROUTING] = "prerouting",
+	[NF_INET_LOCAL_IN] = "input",
+	[NF_INET_FORWARD] = "forward",
+	[NF_INET_LOCAL_OUT] = "output",
+	[NF_INET_POST_ROUTING] = "postrouting",
+};
+
+static const char *arphook2name[] = {
+	[NF_ARP_IN] = "input",
+	[NF_ARP_OUT] = "output",
+};
+
+void netfilter_dump_plain(const struct bpf_link_info *info)
+{
+	const char *hookname = NULL, *pfname = NULL;
+	unsigned int hook = info->netfilter.hooknum;
+	unsigned int pf = info->netfilter.pf;
+
+	if (pf < ARRAY_SIZE(pf2name))
+		pfname = pf2name[pf];
+
+	switch (pf) {
+	case NFPROTO_BRIDGE: /* bridge shares numbers with enum nf_inet_hooks */
+	case NFPROTO_IPV4:
+	case NFPROTO_IPV6:
+	case NFPROTO_INET:
+		if (hook < ARRAY_SIZE(inethook2name))
+			hookname = inethook2name[hook];
+		break;
+	case NFPROTO_ARP:
+		if (hook < ARRAY_SIZE(arphook2name))
+			hookname = arphook2name[hook];
+	default:
+		break;
+	}
+
+	if (pfname)
+		printf("\n\t%s", pfname);
+	else
+		printf("\n\tpf: %d", pf);
+
+	if (hookname)
+		printf(" %s", hookname);
+	else
+		printf(", hook %u,", hook);
+
+	printf(" prio %d", info->netfilter.priority);
+
+	if (info->netfilter.flags)
+		printf(" flags 0x%x", info->netfilter.flags);
+}
+
 static int show_link_close_plain(int fd, struct bpf_link_info *info)
 {
 	struct bpf_prog_info prog_info;
@@ -301,6 +381,9 @@ 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:
+		netfilter_dump_plain(info);
+		break;
 	default:
 		break;
 	}
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 00d11ca6d3f2..63d9aa1012a9 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -264,4 +264,7 @@ static inline bool hashmap__empty(struct hashmap *map)
 	return map ? hashmap__size(map) == 0 : true;
 }
 
+/* print netfilter bpf_link info */
+void netfilter_dump_plain(const struct bpf_link_info *info);
+void netfilter_dump_json(const struct bpf_link_info *info, json_writer_t *wtr);
 #endif
diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
index c40e44c938ae..61710cc63ef7 100644
--- a/tools/bpf/bpftool/net.c
+++ b/tools/bpf/bpftool/net.c
@@ -647,6 +647,107 @@ static int do_detach(int argc, char **argv)
 	return 0;
 }
 
+static int netfilter_link_compar(const void *a, const void *b)
+{
+	const struct bpf_link_info *nfa = a;
+	const struct bpf_link_info *nfb = b;
+	int delta;
+
+	delta = nfa->netfilter.pf - nfb->netfilter.pf;
+	if (delta)
+		return delta;
+
+	delta = nfa->netfilter.hooknum - nfb->netfilter.hooknum;
+	if (delta)
+		return delta;
+
+	if (nfa->netfilter.priority < nfb->netfilter.priority)
+		return -1;
+	if (nfa->netfilter.priority > nfb->netfilter.priority)
+		return 1;
+
+	return nfa->netfilter.flags - nfb->netfilter.flags;
+}
+
+static void show_link_netfilter(void)
+{
+	unsigned int nf_link_len = 0, nf_link_count = 0;
+	struct bpf_link_info *nf_link_info = NULL;
+	__u32 id = 0;
+
+	while (true) {
+		struct bpf_link_info info;
+		int fd, err;
+		__u32 len;
+
+		err = bpf_link_get_next_id(id, &id);
+		if (err) {
+			if (errno == ENOENT)
+				break;
+			p_err("can't get next link: %s (id %d)", strerror(errno), id);
+			break;
+		}
+
+		fd = bpf_link_get_fd_by_id(id);
+		if (fd < 0) {
+			p_err("can't get link by id (%u): %s", id, strerror(errno));
+			continue;
+		}
+
+		memset(&info, 0, sizeof(info));
+		len = sizeof(info);
+
+		err = bpf_link_get_info_by_fd(fd, &info, &len);
+
+		close(fd);
+
+		if (err) {
+			p_err("can't get link info for fd %d: %s", fd, strerror(errno));
+			continue;
+		}
+
+		if (info.type != BPF_LINK_TYPE_NETFILTER)
+			continue;
+
+		if (nf_link_count >= nf_link_len) {
+			struct bpf_link_info *expand;
+
+			if (nf_link_count > (INT_MAX / sizeof(info))) {
+				fprintf(stderr, "link count %d\n", nf_link_count);
+				break;
+			}
+
+			nf_link_len += 16;
+
+			expand = realloc(nf_link_info, nf_link_len * sizeof(info));
+			if (!expand) {
+				p_err("realloc: %s",  strerror(errno));
+				break;
+			}
+
+			nf_link_info = expand;
+		}
+
+		nf_link_info[nf_link_count] = info;
+		nf_link_count++;
+	}
+
+	qsort(nf_link_info, nf_link_count, sizeof(*nf_link_info), netfilter_link_compar);
+
+	for (id = 0; id < nf_link_count; id++) {
+		NET_START_OBJECT;
+		if (json_output)
+			netfilter_dump_json(&nf_link_info[id], json_wtr);
+		else
+			netfilter_dump_plain(&nf_link_info[id]);
+
+		NET_DUMP_UINT("id", " prog_id %u", nf_link_info[id].prog_id);
+		NET_END_OBJECT;
+	}
+
+	free(nf_link_info);
+}
+
 static int do_show(int argc, char **argv)
 {
 	struct bpf_attach_info attach_info = {};
@@ -701,6 +802,10 @@ static int do_show(int argc, char **argv)
 		NET_DUMP_UINT("id", "id %u", attach_info.flow_dissector_id);
 	NET_END_ARRAY("\n");
 
+	NET_START_ARRAY("netfilter", "%s:\n");
+	show_link_netfilter();
+	NET_END_ARRAY("\n");
+
 	NET_END_OBJECT;
 	if (json_output)
 		jsonw_end_array(json_wtr);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 4b20a7269bee..be97cc6398a4 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..ce0e3badc9db 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -130,6 +130,7 @@ static const char * const link_type_name[] = {
 	[BPF_LINK_TYPE_PERF_EVENT]		= "perf_event",
 	[BPF_LINK_TYPE_KPROBE_MULTI]		= "kprobe_multi",
 	[BPF_LINK_TYPE_STRUCT_OPS]		= "struct_ops",
+	[BPF_LINK_TYPE_NETFILTER]		= "netfilter",
 };
 
 static const char * const map_type_name[] = {
@@ -8641,6 +8642,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next v3 6/6] bpf: add test_run support for netfilter program type
  2023-04-18 13:10 [PATCH bpf-next v3 0/6] bpf: add netfilter program type Florian Westphal
                   ` (4 preceding siblings ...)
  2023-04-18 13:10 ` [PATCH bpf-next v3 5/6] tools: bpftool: print netfilter link info Florian Westphal
@ 2023-04-18 13:10 ` Florian Westphal
  5 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-18 13:10 UTC (permalink / raw)
  To: bpf; +Cc: netdev, netfilter-devel, dxu, qde, 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>
---
 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 18b592fde896..e53ceee1df37 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2264,6 +2264,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 f170e8a17974..1f6d785ad028 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>
@@ -1691,6 +1693,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 2d12c978e4e7..01292e654b6a 100644
--- a/net/netfilter/nf_bpf_link.c
+++ b/net/netfilter/nf_bpf_link.c
@@ -167,6 +167,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 25bc8958dbfe..491efd5f22ff 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_reg_equal.skel.h"
@@ -94,6 +95,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_reg_equal(void)            { RUN(verifier_reg_equal); }
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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next v3 5/6] tools: bpftool: print netfilter link info
  2023-04-18 13:10 ` [PATCH bpf-next v3 5/6] tools: bpftool: print netfilter link info Florian Westphal
@ 2023-04-18 14:20   ` Quentin Monnet
  2023-04-18 19:50     ` Florian Westphal
  0 siblings, 1 reply; 14+ messages in thread
From: Quentin Monnet @ 2023-04-18 14:20 UTC (permalink / raw)
  To: Florian Westphal, bpf; +Cc: netdev, netfilter-devel, dxu, qde

2023-04-18 15:10 UTC+0200 ~ Florian Westphal <fw@strlen.de>
> Dump protocol family, hook and priority value:
> $ bpftool link
> 2: netfilter  prog 14
>         ip input prio -128
>         pids install(3264)
> 5: netfilter  prog 14
>         ip6 forward prio 21
>         pids a.out(3387)
> 9: netfilter  prog 14
>         ip prerouting prio 123
>         pids a.out(5700)
> 10: netfilter  prog 14
>         ip input prio 21
>         pids test2(5701)
> 
> v2: Quentin Monnet suggested to also add 'bpftool net' support:
> 
> $ bpftool net
> xdp:
> 
> tc:
> 
> flow_dissector:
> 
> netfilter:
> 
>         ip prerouting prio 21 prog_id 14
>         ip input prio -128 prog_id 14
>         ip input prio 21 prog_id 14
>         ip forward prio 21 prog_id 14
>         ip output prio 21 prog_id 14
>         ip postrouting prio 21 prog_id 14
> 
> 'bpftool net' only dumps netfilter link type.  netfilter links are sorted by
> protocol family, hook and priority.
> 
> Suggested-by: Quentin Monnet <quentin@isovalent.com>
> Link: https://lore.kernel.org/bpf/eeeaac99-9053-90c2-aa33-cc1ecb1ae9ca@isovalent.com/
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  tools/bpf/bpftool/link.c       |  83 ++++++++++++++++++++++++++
>  tools/bpf/bpftool/main.h       |   3 +
>  tools/bpf/bpftool/net.c        | 105 +++++++++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h |  15 +++++
>  tools/lib/bpf/libbpf.c         |   2 +
>  5 files changed, 208 insertions(+)
> 

> diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
> index c40e44c938ae..61710cc63ef7 100644
> --- a/tools/bpf/bpftool/net.c
> +++ b/tools/bpf/bpftool/net.c
> @@ -647,6 +647,107 @@ static int do_detach(int argc, char **argv)

> +static void show_link_netfilter(void)
> +{
> +	unsigned int nf_link_len = 0, nf_link_count = 0;
> +	struct bpf_link_info *nf_link_info = NULL;
> +	__u32 id = 0;
> +
> +	while (true) {
> +		struct bpf_link_info info;
> +		int fd, err;
> +		__u32 len;
> +
> +		err = bpf_link_get_next_id(id, &id);
> +		if (err) {
> +			if (errno == ENOENT)
> +				break;
> +			p_err("can't get next link: %s (id %d)", strerror(errno), id);
> +			break;
> +		}
> +
> +		fd = bpf_link_get_fd_by_id(id);
> +		if (fd < 0) {
> +			p_err("can't get link by id (%u): %s", id, strerror(errno));
> +			continue;
> +		}
> +
> +		memset(&info, 0, sizeof(info));
> +		len = sizeof(info);
> +
> +		err = bpf_link_get_info_by_fd(fd, &info, &len);
> +
> +		close(fd);
> +
> +		if (err) {
> +			p_err("can't get link info for fd %d: %s", fd, strerror(errno));
> +			continue;
> +		}
> +
> +		if (info.type != BPF_LINK_TYPE_NETFILTER)
> +			continue;
> +
> +		if (nf_link_count >= nf_link_len) {
> +			struct bpf_link_info *expand;
> +
> +			if (nf_link_count > (INT_MAX / sizeof(info))) {
> +				fprintf(stderr, "link count %d\n", nf_link_count);

The only nit I have is that we could use p_err() here, and have a more
descriptive message (letting user know that we've reached a limit).

Looks all good otherwise. Thanks!

Reviewed-by: Quentin Monnet <quentin@isovalent.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs
  2023-04-18 13:10 ` [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
@ 2023-04-18 18:35   ` Alexei Starovoitov
  2023-04-18 19:48     ` Florian Westphal
  2023-04-19  5:57   ` kernel test robot
  1 sibling, 1 reply; 14+ messages in thread
From: Alexei Starovoitov @ 2023-04-18 18:35 UTC (permalink / raw)
  To: Florian Westphal; +Cc: bpf, netdev, netfilter-devel, dxu, qde

On Tue, Apr 18, 2023 at 03:10:33PM +0200, Florian Westphal wrote:
> @@ -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)));
...
> +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.reserved[0] | attr->link_create.netfilter.reserved[1])
> +		return -EINVAL;

Why add 'reserved' name that we cannot change later?
I think 'flags' is enough.

> +	link->hook_ops.pf = attr->link_create.netfilter.pf;
> +	link->hook_ops.priority = attr->link_create.netfilter.prio;

let's use the same name in both cases ? Either prio or priority. Both sound fine.

> +	link->hook_ops.hooknum = attr->link_create.netfilter.hooknum;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs
  2023-04-18 18:35   ` Alexei Starovoitov
@ 2023-04-18 19:48     ` Florian Westphal
  0 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-18 19:48 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Florian Westphal, bpf, netdev, netfilter-devel, dxu, qde

Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > +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.reserved[0] | attr->link_create.netfilter.reserved[1])
> > +		return -EINVAL;
> 
> Why add 'reserved' name that we cannot change later?
> I think 'flags' is enough.

OK, I'll zap this.

> > +	link->hook_ops.pf = attr->link_create.netfilter.pf;
> > +	link->hook_ops.priority = attr->link_create.netfilter.prio;
> 
> let's use the same name in both cases ? Either prio or priority. Both sound fine.

OK, I'll go with "priority" then because thats what its named in
nf_hook_ops structure.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next v3 5/6] tools: bpftool: print netfilter link info
  2023-04-18 14:20   ` Quentin Monnet
@ 2023-04-18 19:50     ` Florian Westphal
  0 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-18 19:50 UTC (permalink / raw)
  To: Quentin Monnet; +Cc: Florian Westphal, bpf, netdev, netfilter-devel, dxu, qde

Quentin Monnet <quentin@isovalent.com> wrote:
> > +			if (nf_link_count > (INT_MAX / sizeof(info))) {
> > +				fprintf(stderr, "link count %d\n", nf_link_count);
> 
> The only nit I have is that we could use p_err() here, and have a more
> descriptive message (letting user know that we've reached a limit).

Drats, I'll replace this and will retain your RvB tag in v4, thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next v3 2/6] bpf: minimal support for programs hooked into netfilter framework
  2023-04-18 13:10 ` [PATCH bpf-next v3 2/6] bpf: minimal support for programs hooked into netfilter framework Florian Westphal
@ 2023-04-19  2:11   ` Alexei Starovoitov
  0 siblings, 0 replies; 14+ messages in thread
From: Alexei Starovoitov @ 2023-04-19  2:11 UTC (permalink / raw)
  To: Florian Westphal; +Cc: bpf, netdev, netfilter-devel, dxu, qde

On Tue, Apr 18, 2023 at 03:10:34PM +0200, Florian Westphal wrote:
> 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 027f9f8a3551..3556bb68e3ec 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,
>  };
>  
> @@ -7800,6 +7804,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 1e05355facdc..fc7281d39e46 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13816,6 +13816,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 44fb997434ad..d9ce04ca22ce 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -11717,6 +11717,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 0f937c6bee6d..2d12c978e4e7 100644
> --- a/net/netfilter/nf_bpf_link.c
> +++ b/net/netfilter/nf_bpf_link.c
> @@ -1,5 +1,6 @@
>  // 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>
> @@ -7,7 +8,13 @@
>  
>  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 {
> @@ -158,3 +165,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;

This can be improved. Instead of run-time search use pre-processed
btf_tracing_ids[] approach.
Add sk_buff and nf_hook_state to BTF_TRACING_TYPE_xxx
and take btf ids from array.

It can be a follow up,
but since you're respinning anyway please add a selftest for ctx->skb, ctx->state.
The patch 6 only validates the return codes.

> +}
> +
> +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);

As a next step we probably need to generalize tc_cls_act_func_proto
to take trusted ptr_to_btf skb instead of ctx.
netfilter progs should be able to use bpf_skb_load_bytes and friends.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs
  2023-04-18 13:10 ` [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
  2023-04-18 18:35   ` Alexei Starovoitov
@ 2023-04-19  5:57   ` kernel test robot
  1 sibling, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-04-19  5:57 UTC (permalink / raw)
  To: Florian Westphal, bpf
  Cc: oe-kbuild-all, netdev, netfilter-devel, dxu, qde,
	Florian Westphal

Hi Florian,

kernel test robot noticed the following build errors:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/bpf-add-bpf_link-support-for-BPF_NETFILTER-programs/20230418-211350
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20230418131038.18054-2-fw%40strlen.de
patch subject: [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs
config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20230419/202304191332.9uIkDScp-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.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/intel-lab-lkp/linux/commit/13cdccc8c376b453ae0ca2280ec22baa6210b450
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Florian-Westphal/bpf-add-bpf_link-support-for-BPF_NETFILTER-programs/20230418-211350
        git checkout 13cdccc8c376b453ae0ca2280ec22baa6210b450
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304191332.9uIkDScp-lkp@intel.com/

All errors (new ones prefixed by >>):

   arm-linux-gnueabi-ld: net/netfilter/nf_bpf_link.o: in function `bpf_nf_link_release':
>> nf_bpf_link.c:(.text+0x174): undefined reference to `__bad_cmpxchg'
   arm-linux-gnueabi-ld: net/netfilter/nf_bpf_link.o: in function `bpf_nf_link_detach':
   nf_bpf_link.c:(.text+0x200): undefined reference to `__bad_cmpxchg'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next v3 3/6] netfilter: nfnetlink hook: dump bpf prog id
  2023-04-18 13:10 ` [PATCH bpf-next v3 3/6] netfilter: nfnetlink hook: dump bpf prog id Florian Westphal
@ 2023-04-20  9:54   ` Simon Horman
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Horman @ 2023-04-20  9:54 UTC (permalink / raw)
  To: Florian Westphal; +Cc: bpf, netdev, netfilter-devel, dxu, qde

On Tue, Apr 18, 2023 at 03:10:35PM +0200, Florian Westphal wrote:
> 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>

Hi Florian,

as it seems there will be a v4, some kdoc nits from my side.

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

nit: -,.s/NFLA/@NFLA/

> + *
> + * 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

nit: s/@NFNL_HOOK_TYPE_NFTABLES/@NFNL_HOOK_TYPE_NFTABLES:/
     s/@NFNL_HOOK_TYPE_BPF/@NFNL_HOOK_TYPE_BPF:/

>   */
>  enum nfnl_hook_chaintype {
>  	NFNL_HOOK_TYPE_NFTABLES = 0x1,
> +	NFNL_HOOK_TYPE_BPF,
> +};
> +
> +/**
> + * enum nfnl_hook_bpf_info_attributes - bpf prog description

nit: s/nfnl_hook_bpf_info_attributes/nfnl_hook_bpf_attributes/

> + *
> + * NFNLA_BPF_INFO_ID: bpf program id (NLA_U32)

nit: s/NFNLA_BPF_INFO_ID:/NFNLA_HOOK_BPF_ID:/

> + */
> +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)

...

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-04-20  9:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-18 13:10 [PATCH bpf-next v3 0/6] bpf: add netfilter program type Florian Westphal
2023-04-18 13:10 ` [PATCH bpf-next v3 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
2023-04-18 18:35   ` Alexei Starovoitov
2023-04-18 19:48     ` Florian Westphal
2023-04-19  5:57   ` kernel test robot
2023-04-18 13:10 ` [PATCH bpf-next v3 2/6] bpf: minimal support for programs hooked into netfilter framework Florian Westphal
2023-04-19  2:11   ` Alexei Starovoitov
2023-04-18 13:10 ` [PATCH bpf-next v3 3/6] netfilter: nfnetlink hook: dump bpf prog id Florian Westphal
2023-04-20  9:54   ` Simon Horman
2023-04-18 13:10 ` [PATCH bpf-next v3 4/6] netfilter: disallow bpf hook attachment at same priority Florian Westphal
2023-04-18 13:10 ` [PATCH bpf-next v3 5/6] tools: bpftool: print netfilter link info Florian Westphal
2023-04-18 14:20   ` Quentin Monnet
2023-04-18 19:50     ` Florian Westphal
2023-04-18 13:10 ` [PATCH bpf-next v3 6/6] bpf: add test_run support for netfilter program type Florian Westphal

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).