BPF Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls
@ 2019-10-07 17:20 Toke Høiland-Jørgensen
  2019-10-07 17:20 ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Toke Høiland-Jørgensen
                   ` (5 more replies)
  0 siblings, 6 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-07 17:20 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

This series adds support for executing multiple XDP programs on a single
interface in sequence, through the use of chain calls, as discussed at the Linux
Plumbers Conference last month:

https://linuxplumbersconf.org/event/4/contributions/460/

# HIGH-LEVEL IDEA

Since Alexei pointed out some issues with trying to rewrite the eBPF byte code,
let's try a third approach: We add the ability to chain call programs into the
eBPF execution core itself, but without rewriting the eBPF byte code.

As in the previous version, the bpf() syscall gets a couple of new commands
which takes a pair of BPF program fds and a return code. It will then attach the
second program to the first one in a structured keyed by return code. When a
program chain is thus established, the former program will tail call to the
latter at the end of its execution.

The actual tail calling is achieved by adding a new flag to struct bpf_prog and
having BPF_PROG_RUN run the chain call logic if that flag is set. This means
that if the feature is *not* used, the overhead is a single conditional branch
(which means I couldn't measure a performance difference, as can be seen in the
results below).

For this version I kept the load-time flag from the previous version, to avoid
having to remove the read-only memory protection from the bpf prog. Only
programs loaded with this flag set can have other programs attached to them for
chain calls.

As before, it shouldn't be necessary to set the flag on program load time, but
rather we should enable the feature when a chain call program is first loaded.
We could conceivably just remove the RO property from the first page of struct
bpf_prog and set the flag as needed.

# PERFORMANCE

I performed a simple performance test to get an initial feel for the overhead of
the chain call mechanism. This test consists of running only two programs in
sequence: One that returns XDP_PASS and another that returns XDP_DROP. I then
measure the drop PPS performance and compare it to a baseline of just a single
program that only returns XDP_DROP.

For comparison, a test case that uses regular eBPF tail calls to sequence two
programs together is also included.

| Test case                        | Perf      | Overhead |
|----------------------------------+-----------+----------|
| Before patch (XDP DROP program)  | 31.5 Mpps |          |
| After patch (XDP DROP program)   | 32.0 Mpps |          |
| XDP chain call (XDP_PASS return) | 28.5 Mpps | 3.8 ns   |
| XDP chain call (wildcard return) | 28.1 Mpps | 4.3 ns   |

I consider the "Before patch" and "After patch" to be identical; the .5 Mpps
difference is within the regular test variance I see between runs. Likewise,
there is probably no significant difference between hooking the XDP_PASS return
code and using the wildcard slot.

# PATCH SET STRUCTURE
This series is structured as follows:

- Patch 1: Adds the call chain looping logic
- Patch 2: Adds the new commands added to the bpf() syscall
- Patch 3-4: Tools/ update and libbpf syscall wrappers
- Patch 5: Selftest  with example user space code (a bit hacky still)

The whole series is also available in my git repo on kernel.org:
https://git.kernel.org/pub/scm/linux/kernel/git/toke/linux.git/log/?h=xdp-multiprog-03

Changelog:

v3:
  - Keep the UAPI from v2, but change the implementation to hook into
    BPF_PROG_RUN instead of trying to inject instructions into the eBPF program
    itself (since that had problems as Alexei pointed out).
v2:
  - Completely new approach that integrates chain calls into the core eBPF
    runtime instead of doing the map XDP-specific thing with a new map from v1.

---

Toke Høiland-Jørgensen (5):
      bpf: Support chain calling multiple BPF programs after each other
      bpf: Add support for setting chain call sequence for programs
      tools: Update bpf.h header for program chain calls
      libbpf: Add syscall wrappers for BPF_PROG_CHAIN_* commands
      selftests: Add tests for XDP chain calls


 include/linux/bpf.h                           |    3 
 include/linux/filter.h                        |   34 +++
 include/uapi/linux/bpf.h                      |   16 +
 kernel/bpf/core.c                             |    6 
 kernel/bpf/syscall.c                          |   82 ++++++-
 tools/include/uapi/linux/bpf.h                |   16 +
 tools/lib/bpf/bpf.c                           |   34 +++
 tools/lib/bpf/bpf.h                           |    4 
 tools/lib/bpf/libbpf.map                      |    3 
 tools/testing/selftests/bpf/.gitignore        |    1 
 tools/testing/selftests/bpf/Makefile          |    3 
 tools/testing/selftests/bpf/progs/xdp_dummy.c |    6 
 tools/testing/selftests/bpf/test_xdp_chain.sh |   77 ++++++
 tools/testing/selftests/bpf/xdp_chain.c       |  313 +++++++++++++++++++++++++
 14 files changed, 594 insertions(+), 4 deletions(-)
 create mode 100755 tools/testing/selftests/bpf/test_xdp_chain.sh
 create mode 100644 tools/testing/selftests/bpf/xdp_chain.c


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

* [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-07 17:20 [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
@ 2019-10-07 17:20 ` Toke Høiland-Jørgensen
  2019-10-07 20:42   ` Alexei Starovoitov
  2019-10-07 17:20 ` [PATCH bpf-next v3 2/5] bpf: Add support for setting chain call sequence for programs Toke Høiland-Jørgensen
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-07 17:20 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

From: Toke Høiland-Jørgensen <toke@redhat.com>

This adds support for wrapping eBPF program dispatch in chain calling
logic. The code injection is controlled by a flag at program load time; if
the flag is set, the BPF program will carry a flag bit that changes the
program dispatch logic to wrap it in a chain call loop.

Ideally, it shouldn't be necessary to set the flag on program load time,
but rather inject the calls when a chain call program is first loaded. The
allocation logic sets the whole of struct bpf_prog to be read-only memory,
so it can't immediately be modified, but conceivably we could just unlock
the first page of the struct and flip the bit when a chain call program is
first attached.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 include/linux/bpf.h      |    3 +++
 include/linux/filter.h   |   34 ++++++++++++++++++++++++++++++++--
 include/uapi/linux/bpf.h |    6 ++++++
 kernel/bpf/core.c        |    6 ++++++
 kernel/bpf/syscall.c     |    4 +++-
 5 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5b9d22338606..13e5f38cf5c6 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -365,6 +365,8 @@ struct bpf_prog_stats {
 	struct u64_stats_sync syncp;
 };
 
+#define BPF_NUM_CHAIN_SLOTS 8
+
 struct bpf_prog_aux {
 	atomic_t refcnt;
 	u32 used_map_cnt;
@@ -383,6 +385,7 @@ struct bpf_prog_aux {
 	struct list_head ksym_lnode;
 	const struct bpf_prog_ops *ops;
 	struct bpf_map **used_maps;
+	struct bpf_prog *chain_progs[BPF_NUM_CHAIN_SLOTS];
 	struct bpf_prog *prog;
 	struct user_struct *user;
 	u64 load_time; /* ns since boottime */
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 2ce57645f3cd..3d1e4991e61d 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -21,6 +21,7 @@
 #include <linux/kallsyms.h>
 #include <linux/if_vlan.h>
 #include <linux/vmalloc.h>
+#include <linux/nospec.h>
 
 #include <net/sch_generic.h>
 
@@ -528,6 +529,7 @@ struct bpf_prog {
 				is_func:1,	/* program is a bpf function */
 				kprobe_override:1, /* Do we override a kprobe? */
 				has_callchain_buf:1, /* callchain buffer allocated? */
+				chain_calls:1, /* should this use the chain_call wrapper */
 				enforce_expected_attach_type:1; /* Enforce expected_attach_type checking at attach time */
 	enum bpf_prog_type	type;		/* Type of BPF program */
 	enum bpf_attach_type	expected_attach_type; /* For some prog types */
@@ -551,6 +553,30 @@ struct sk_filter {
 	struct bpf_prog	*prog;
 };
 
+#define BPF_MAX_CHAIN_CALLS 32
+static __always_inline unsigned int do_chain_calls(const struct bpf_prog *prog,
+						   const void *ctx)
+{
+	int i = BPF_MAX_CHAIN_CALLS;
+	int idx;
+	u32 ret;
+
+	do {
+		ret = (*(prog)->bpf_func)(ctx, prog->insnsi);
+
+		if (ret + 1 >= BPF_NUM_CHAIN_SLOTS) {
+			prog = prog->aux->chain_progs[0];
+			continue;
+		}
+		idx = ret + 1;
+		idx = array_index_nospec(idx, BPF_NUM_CHAIN_SLOTS);
+
+		prog = prog->aux->chain_progs[idx] ?: prog->aux->chain_progs[0];
+	} while (prog && --i);
+
+	return ret;
+}
+
 DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
 
 #define BPF_PROG_RUN(prog, ctx)	({				\
@@ -559,14 +585,18 @@ DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
 	if (static_branch_unlikely(&bpf_stats_enabled_key)) {	\
 		struct bpf_prog_stats *stats;			\
 		u64 start = sched_clock();			\
-		ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
+		ret = prog->chain_calls ?			\
+			do_chain_calls(prog, ctx) :			\
+			 (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
 		stats = this_cpu_ptr(prog->aux->stats);		\
 		u64_stats_update_begin(&stats->syncp);		\
 		stats->cnt++;					\
 		stats->nsecs += sched_clock() - start;		\
 		u64_stats_update_end(&stats->syncp);		\
 	} else {						\
-		ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
+		ret = prog->chain_calls ?				\
+			do_chain_calls(prog, ctx) :			\
+			 (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
 	}							\
 	ret; })
 
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 77c6be96d676..1ce80a227be3 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -288,6 +288,12 @@ enum bpf_attach_type {
 /* The verifier internal test flag. Behavior is undefined */
 #define BPF_F_TEST_STATE_FREQ	(1U << 3)
 
+/* Whether to enable chain call logic at program execution. If set, the program
+ * execution logic will check for and jump to chain call programs configured
+ * with the BPF_PROG_CHAIN_* commands to the bpf syscall.
+ */
+#define BPF_F_CHAIN_CALLS	(1U << 4)
+
 /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
  * two extensions:
  *
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 66088a9e9b9e..5dfe3585bc5d 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -254,6 +254,12 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
 void __bpf_prog_free(struct bpf_prog *fp)
 {
 	if (fp->aux) {
+		int i;
+
+		for (i = 0; i < BPF_NUM_CHAIN_SLOTS; i++)
+			if (fp->aux->chain_progs[i])
+				bpf_prog_put(fp->aux->chain_progs[i]);
+
 		free_percpu(fp->aux->stats);
 		kfree(fp->aux);
 	}
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 82eabd4e38ad..b8a203a05881 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1630,7 +1630,8 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
 	if (attr->prog_flags & ~(BPF_F_STRICT_ALIGNMENT |
 				 BPF_F_ANY_ALIGNMENT |
 				 BPF_F_TEST_STATE_FREQ |
-				 BPF_F_TEST_RND_HI32))
+				 BPF_F_TEST_RND_HI32 |
+				 BPF_F_CHAIN_CALLS))
 		return -EINVAL;
 
 	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
@@ -1665,6 +1666,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
 		return -ENOMEM;
 
 	prog->expected_attach_type = attr->expected_attach_type;
+	prog->chain_calls = !!(attr->prog_flags & BPF_F_CHAIN_CALLS);
 
 	prog->aux->offload_requested = !!attr->prog_ifindex;
 


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

* [PATCH bpf-next v3 2/5] bpf: Add support for setting chain call sequence for programs
  2019-10-07 17:20 [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
  2019-10-07 17:20 ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Toke Høiland-Jørgensen
@ 2019-10-07 17:20 ` Toke Høiland-Jørgensen
  2019-10-07 20:38   ` Daniel Borkmann
  2019-10-07 17:20 ` [PATCH bpf-next v3 3/5] tools: Update bpf.h header for program chain calls Toke Høiland-Jørgensen
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-07 17:20 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

From: Toke Høiland-Jørgensen <toke@redhat.com>

This adds support for setting and deleting bpf chain call programs through
a couple of new commands in the bpf() syscall. The CHAIN_ADD and CHAIN_DEL
commands take two eBPF program fds and a return code, and install the
'next' program to be chain called after the 'prev' program if that program
returns 'retcode'. A retcode of -1 means "wildcard", so that the program
will be executed regardless of the previous program's return code.


The syscall command names are based on Alexei's prog_chain example[0],
which Alan helpfully rebased on current bpf-next. However, the logic and
program storage is obviously adapted to the execution logic in the previous
commit.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/ast/bpf.git/commit/?h=prog_chain&id=f54f45d00f91e083f6aec2abe35b6f0be52ae85b&context=15

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 include/uapi/linux/bpf.h |   10 ++++++
 kernel/bpf/syscall.c     |   78 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1ce80a227be3..b03c23963af8 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -107,6 +107,9 @@ enum bpf_cmd {
 	BPF_MAP_LOOKUP_AND_DELETE_ELEM,
 	BPF_MAP_FREEZE,
 	BPF_BTF_GET_NEXT_ID,
+	BPF_PROG_CHAIN_ADD,
+	BPF_PROG_CHAIN_DEL,
+	BPF_PROG_CHAIN_GET,
 };
 
 enum bpf_map_type {
@@ -516,6 +519,13 @@ union bpf_attr {
 		__u64		probe_offset;	/* output: probe_offset */
 		__u64		probe_addr;	/* output: probe_addr */
 	} task_fd_query;
+
+	struct { /* anonymous struct used by BPF_PROG_CHAIN_* commands */
+		__u32		prev_prog_fd;
+		__u32		next_prog_fd;
+		__u32		retcode;
+		__u32		next_prog_id;   /* output: prog_id */
+	};
 } __attribute__((aligned(8)));
 
 /* The description below is an attempt at providing documentation to eBPF
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b8a203a05881..be8112e08a88 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2113,6 +2113,79 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
 	return ret;
 }
 
+#define BPF_PROG_CHAIN_LAST_FIELD next_prog_id
+
+static int bpf_prog_chain(int cmd, const union bpf_attr *attr,
+			  union bpf_attr __user *uattr)
+{
+	struct bpf_prog *prog, *next_prog, *old_prog;
+	struct bpf_prog **array;
+	int ret = -EOPNOTSUPP;
+	u32 index, prog_id;
+
+	if (CHECK_ATTR(BPF_PROG_CHAIN))
+		return -EINVAL;
+
+	/* Index 0 is wildcard, encoded as ~0 by userspace */
+	if (attr->retcode == ((u32) ~0))
+		index = 0;
+	else
+		index = attr->retcode + 1;
+
+	if (index >= BPF_NUM_CHAIN_SLOTS)
+		return -E2BIG;
+
+	prog = bpf_prog_get(attr->prev_prog_fd);
+	if (IS_ERR(prog))
+		return PTR_ERR(prog);
+
+	/* If the chain_calls bit is not set, that's because the chain call flag
+	 * was not set on program load, and so we can't support chain calls.
+	 */
+	if (!prog->chain_calls)
+		goto out;
+
+	array = prog->aux->chain_progs;
+
+	switch (cmd) {
+	case BPF_PROG_CHAIN_ADD:
+		next_prog = bpf_prog_get(attr->next_prog_fd);
+		if (IS_ERR(next_prog)) {
+			ret = PTR_ERR(next_prog);
+			break;
+		}
+		old_prog = xchg(array + index, next_prog);
+		if (old_prog)
+			bpf_prog_put(old_prog);
+		ret = 0;
+		break;
+	case BPF_PROG_CHAIN_DEL:
+		old_prog = xchg(array + index, NULL);
+		if (old_prog) {
+			bpf_prog_put(old_prog);
+			ret = 0;
+		} else {
+			ret = -ENOENT;
+		}
+		break;
+	case BPF_PROG_CHAIN_GET:
+		old_prog = READ_ONCE(array[index]);
+		if (old_prog) {
+			prog_id = old_prog->aux->id;
+			if (put_user(prog_id, &uattr->next_prog_id))
+				ret = -EFAULT;
+			else
+				ret = 0;
+		} else
+			ret = -ENOENT;
+		break;
+	}
+
+out:
+	bpf_prog_put(prog);
+	return ret;
+}
+
 #define BPF_OBJ_GET_NEXT_ID_LAST_FIELD next_id
 
 static int bpf_obj_get_next_id(const union bpf_attr *attr,
@@ -2885,6 +2958,11 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
 	case BPF_PROG_TEST_RUN:
 		err = bpf_prog_test_run(&attr, uattr);
 		break;
+	case BPF_PROG_CHAIN_ADD:
+	case BPF_PROG_CHAIN_DEL:
+	case BPF_PROG_CHAIN_GET:
+		err = bpf_prog_chain(cmd, &attr, uattr);
+		break;
 	case BPF_PROG_GET_NEXT_ID:
 		err = bpf_obj_get_next_id(&attr, uattr,
 					  &prog_idr, &prog_idr_lock);


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

* [PATCH bpf-next v3 3/5] tools: Update bpf.h header for program chain calls
  2019-10-07 17:20 [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
  2019-10-07 17:20 ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Toke Høiland-Jørgensen
  2019-10-07 17:20 ` [PATCH bpf-next v3 2/5] bpf: Add support for setting chain call sequence for programs Toke Høiland-Jørgensen
@ 2019-10-07 17:20 ` Toke Høiland-Jørgensen
  2019-10-07 17:20 ` [PATCH bpf-next v3 4/5] libbpf: Add syscall wrappers for BPF_PROG_CHAIN_* commands Toke Høiland-Jørgensen
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-07 17:20 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

From: Toke Høiland-Jørgensen <toke@redhat.com>

This updates the bpf.h UAPI header in tools/ to add the bpf chain
call-related definitions.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/include/uapi/linux/bpf.h |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 77c6be96d676..b03c23963af8 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -107,6 +107,9 @@ enum bpf_cmd {
 	BPF_MAP_LOOKUP_AND_DELETE_ELEM,
 	BPF_MAP_FREEZE,
 	BPF_BTF_GET_NEXT_ID,
+	BPF_PROG_CHAIN_ADD,
+	BPF_PROG_CHAIN_DEL,
+	BPF_PROG_CHAIN_GET,
 };
 
 enum bpf_map_type {
@@ -288,6 +291,12 @@ enum bpf_attach_type {
 /* The verifier internal test flag. Behavior is undefined */
 #define BPF_F_TEST_STATE_FREQ	(1U << 3)
 
+/* Whether to enable chain call logic at program execution. If set, the program
+ * execution logic will check for and jump to chain call programs configured
+ * with the BPF_PROG_CHAIN_* commands to the bpf syscall.
+ */
+#define BPF_F_CHAIN_CALLS	(1U << 4)
+
 /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
  * two extensions:
  *
@@ -510,6 +519,13 @@ union bpf_attr {
 		__u64		probe_offset;	/* output: probe_offset */
 		__u64		probe_addr;	/* output: probe_addr */
 	} task_fd_query;
+
+	struct { /* anonymous struct used by BPF_PROG_CHAIN_* commands */
+		__u32		prev_prog_fd;
+		__u32		next_prog_fd;
+		__u32		retcode;
+		__u32		next_prog_id;   /* output: prog_id */
+	};
 } __attribute__((aligned(8)));
 
 /* The description below is an attempt at providing documentation to eBPF


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

* [PATCH bpf-next v3 4/5] libbpf: Add syscall wrappers for BPF_PROG_CHAIN_* commands
  2019-10-07 17:20 [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
                   ` (2 preceding siblings ...)
  2019-10-07 17:20 ` [PATCH bpf-next v3 3/5] tools: Update bpf.h header for program chain calls Toke Høiland-Jørgensen
@ 2019-10-07 17:20 ` Toke Høiland-Jørgensen
  2019-10-07 17:20 ` [PATCH bpf-next v3 5/5] selftests: Add tests for XDP chain calls Toke Høiland-Jørgensen
  2019-10-07 18:58 ` [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through " John Fastabend
  5 siblings, 0 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-07 17:20 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

From: Toke Høiland-Jørgensen <toke@redhat.com>

This adds simple syscall wrappers for the new BPF_PROG_CHAIN_* commands to
libbpf.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/lib/bpf/bpf.c      |   34 ++++++++++++++++++++++++++++++++++
 tools/lib/bpf/bpf.h      |    4 ++++
 tools/lib/bpf/libbpf.map |    3 +++
 3 files changed, 41 insertions(+)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index cbb933532981..23246fa169e7 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -703,3 +703,37 @@ int bpf_task_fd_query(int pid, int fd, __u32 flags, char *buf, __u32 *buf_len,
 
 	return err;
 }
+
+int bpf_prog_chain_add(int prev_prog_fd, __u32 retcode, int next_prog_fd) {
+	union bpf_attr attr = {};
+
+	attr.prev_prog_fd = prev_prog_fd;
+	attr.next_prog_fd = next_prog_fd;
+	attr.retcode = retcode;
+
+	return sys_bpf(BPF_PROG_CHAIN_ADD, &attr, sizeof(attr));
+}
+
+int bpf_prog_chain_del(int prev_prog_fd, __u32 retcode) {
+	union bpf_attr attr = {};
+
+	attr.prev_prog_fd = prev_prog_fd;
+	attr.retcode = retcode;
+
+	return sys_bpf(BPF_PROG_CHAIN_DEL, &attr, sizeof(attr));
+}
+
+int bpf_prog_chain_get(int prev_prog_fd, __u32 retcode, __u32 *prog_id) {
+	union bpf_attr attr = {};
+	int err;
+
+	attr.prev_prog_fd = prev_prog_fd;
+	attr.retcode = retcode;
+
+	err = sys_bpf(BPF_PROG_CHAIN_GET, &attr, sizeof(attr));
+
+	if (!err)
+		*prog_id = attr.next_prog_id;
+
+	return err;
+}
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 0db01334740f..0300cb8c8bed 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -171,6 +171,10 @@ LIBBPF_API int bpf_task_fd_query(int pid, int fd, __u32 flags, char *buf,
 				 __u32 *buf_len, __u32 *prog_id, __u32 *fd_type,
 				 __u64 *probe_offset, __u64 *probe_addr);
 
+LIBBPF_API int bpf_prog_chain_add(int prev_prog_fd, __u32 retcode, int next_prog_fd);
+LIBBPF_API int bpf_prog_chain_del(int prev_prog_fd, __u32 retcode);
+LIBBPF_API int bpf_prog_chain_get(int prev_prog_fd, __u32 retcode, __u32 *prog_id);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 8d10ca03d78d..9c483c554054 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -192,4 +192,7 @@ LIBBPF_0.0.5 {
 } LIBBPF_0.0.4;
 
 LIBBPF_0.0.6 {
+		bpf_prog_chain_add;
+		bpf_prog_chain_del;
+		bpf_prog_chain_get;
 } LIBBPF_0.0.5;


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

* [PATCH bpf-next v3 5/5] selftests: Add tests for XDP chain calls
  2019-10-07 17:20 [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
                   ` (3 preceding siblings ...)
  2019-10-07 17:20 ` [PATCH bpf-next v3 4/5] libbpf: Add syscall wrappers for BPF_PROG_CHAIN_* commands Toke Høiland-Jørgensen
@ 2019-10-07 17:20 ` Toke Høiland-Jørgensen
  2019-10-07 18:58 ` [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through " John Fastabend
  5 siblings, 0 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-07 17:20 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

From: Toke Høiland-Jørgensen <toke@redhat.com>

This adds new self tests for the XDP chain call functionality.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/testing/selftests/bpf/.gitignore        |    1 
 tools/testing/selftests/bpf/Makefile          |    3 
 tools/testing/selftests/bpf/progs/xdp_dummy.c |    6 
 tools/testing/selftests/bpf/test_xdp_chain.sh |   77 ++++++
 tools/testing/selftests/bpf/xdp_chain.c       |  313 +++++++++++++++++++++++++
 5 files changed, 399 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/bpf/test_xdp_chain.sh
 create mode 100644 tools/testing/selftests/bpf/xdp_chain.c

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 7470327edcfe..e9d2d765cc8f 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -39,3 +39,4 @@ libbpf.so.*
 test_hashmap
 test_btf_dump
 xdping
+xdp_chain
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 6889c19a628c..97e8f6ae4a15 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -29,7 +29,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 	test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
 	test_cgroup_storage test_select_reuseport test_section_names \
 	test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
-	test_btf_dump test_cgroup_attach xdping
+	test_btf_dump test_cgroup_attach xdping xdp_chain
 
 BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
 TEST_GEN_FILES = $(BPF_OBJ_FILES)
@@ -71,6 +71,7 @@ TEST_PROGS := test_kmod.sh \
 	test_tc_tunnel.sh \
 	test_tc_edt.sh \
 	test_xdping.sh \
+	test_xdp_chain.sh \
 	test_bpftool_build.sh
 
 TEST_PROGS_EXTENDED := with_addr.sh \
diff --git a/tools/testing/selftests/bpf/progs/xdp_dummy.c b/tools/testing/selftests/bpf/progs/xdp_dummy.c
index 43b0ef1001ed..454a1f0763a1 100644
--- a/tools/testing/selftests/bpf/progs/xdp_dummy.c
+++ b/tools/testing/selftests/bpf/progs/xdp_dummy.c
@@ -10,4 +10,10 @@ int xdp_dummy_prog(struct xdp_md *ctx)
 	return XDP_PASS;
 }
 
+SEC("xdp_drop")
+int xdp_drop_prog(struct xdp_md *ctx)
+{
+	return XDP_DROP;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_xdp_chain.sh b/tools/testing/selftests/bpf/test_xdp_chain.sh
new file mode 100755
index 000000000000..3997655d4e45
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_xdp_chain.sh
@@ -0,0 +1,77 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# xdp_chain tests
+#   Here we setup and teardown configuration required to run
+#   xdp_chain, exercising its options.
+#
+#   Setup is similar to xdping tests.
+#
+# Topology:
+# ---------
+#     root namespace   |     tc_ns0 namespace
+#                      |
+#      ----------      |     ----------
+#      |  veth1  | --------- |  veth0  |
+#      ----------    peer    ----------
+#
+# Device Configuration
+# --------------------
+# Root namespace with BPF
+# Device names and addresses:
+#	veth1 IP: 10.1.1.200
+#
+# Namespace tc_ns0 with BPF
+# Device names and addresses:
+#       veth0 IPv4: 10.1.1.100
+#	xdp_chain binary run inside this
+#
+
+readonly TARGET_IP="10.1.1.100"
+readonly TARGET_NS="xdp_ns0"
+
+readonly LOCAL_IP="10.1.1.200"
+
+setup()
+{
+	ip netns add $TARGET_NS
+	ip link add veth0 type veth peer name veth1
+	ip link set veth0 netns $TARGET_NS
+	ip netns exec $TARGET_NS ip addr add ${TARGET_IP}/24 dev veth0
+	ip addr add ${LOCAL_IP}/24 dev veth1
+	ip netns exec $TARGET_NS ip link set veth0 up
+	ip link set veth1 up
+}
+
+cleanup()
+{
+	set +e
+	ip netns delete $TARGET_NS 2>/dev/null
+	ip link del veth1 2>/dev/null
+}
+
+die()
+{
+        echo "$@" >&2
+        exit 1
+}
+
+test()
+{
+	args="$1"
+
+	ip netns exec $TARGET_NS ./xdp_chain $args || die "XDP chain test error"
+}
+
+set -e
+
+server_pid=0
+
+trap cleanup EXIT
+
+setup
+
+test "-I veth0 -S $LOCAL_IP"
+
+echo "OK. All tests passed"
+exit 0
diff --git a/tools/testing/selftests/bpf/xdp_chain.c b/tools/testing/selftests/bpf/xdp_chain.c
new file mode 100644
index 000000000000..4b3fa26224fa
--- /dev/null
+++ b/tools/testing/selftests/bpf/xdp_chain.c
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. */
+
+#include <linux/bpf.h>
+#include <linux/if_link.h>
+#include <arpa/inet.h>
+#include <assert.h>
+#include <errno.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <libgen.h>
+#include <sys/resource.h>
+#include <net/if.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netdb.h>
+
+#include "bpf/bpf.h"
+#include "bpf/libbpf.h"
+
+static int ifindex;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+static char *dest = NULL, *ifname = NULL;
+
+static void cleanup(int sig)
+{
+	int ret;
+
+	fprintf(stderr, "  Cleaning up\n");
+	if ((ret = bpf_set_link_xdp_fd(ifindex, -1, xdp_flags)))
+		fprintf(stderr, "Warning: Unable to clear XDP prog: %s\n",
+			strerror(-ret));
+	if (sig)
+		exit(1);
+}
+
+static void show_usage(const char *prog)
+{
+	fprintf(stderr,
+		"usage: %s [OPTS] -I interface destination\n\n"
+		"OPTS:\n"
+		"    -I interface		interface name\n"
+		"    -N			Run in driver mode\n"
+		"    -S			Run in skb mode\n"
+		"    -p pin_path		path to pin chain call map\n"
+		"    -x			Exit after setup\n"
+		"    -c			Cleanup and exit\n"
+		"    -v			Verbose eBPF logging\n",
+		prog);
+}
+
+static int run_ping(bool should_fail, const char *msg)
+{
+	char cmd[256];
+	bool success;
+	int ret;
+
+	snprintf(cmd, sizeof(cmd), "ping -c 1 -W 1 -I %s %s >/dev/null", ifname, dest);
+
+	printf("  %s: ", msg);
+
+	ret = system(cmd);
+
+	success = (!!ret == should_fail);
+	printf(success ? "PASS\n" : "FAIL\n");
+
+	return !success;
+}
+
+struct bpf_program {
+	/* Index in elf obj file, for relocation use. */
+	int idx;
+	char *name;
+	int prog_ifindex;
+	char *section_name;
+	/* section_name with / replaced by _; makes recursive pinning
+	 * in bpf_object__pin_programs easier
+	 */
+	char *pin_name;
+	struct bpf_insn *insns;
+	size_t insns_cnt, main_prog_cnt;
+	enum bpf_prog_type type;
+
+	struct reloc_desc {
+		enum {
+			RELO_LD64,
+			RELO_CALL,
+			RELO_DATA,
+		} type;
+		int insn_idx;
+		union {
+			int map_idx;
+			int text_off;
+		};
+	} *reloc_desc;
+	int nr_reloc;
+	int log_level;
+
+	struct {
+		int nr;
+		int *fds;
+	} instances;
+	bpf_program_prep_t preprocessor;
+
+	struct bpf_object *obj;
+	void *priv;
+	bpf_program_clear_priv_t clear_priv;
+
+	enum bpf_attach_type expected_attach_type;
+	void *func_info;
+	__u32 func_info_rec_size;
+	__u32 func_info_cnt;
+
+	struct bpf_capabilities *caps;
+
+	void *line_info;
+	__u32 line_info_rec_size;
+	__u32 line_info_cnt;
+	__u32 prog_flags;
+};
+
+static int printfunc(enum libbpf_print_level level, const char *format, va_list args)
+{
+	return vfprintf(stderr, format, args);
+}
+
+int main(int argc, char **argv)
+{
+	__u32 mode_flags = XDP_FLAGS_DRV_MODE | XDP_FLAGS_SKB_MODE;
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	bool setup_only = false, cleanup_only = false;
+	struct bpf_program *pass_prog, *drop_prog, *prog;
+	int pass_prog_fd = -1, drop_prog_fd = -1;
+	const char *filename = "xdp_dummy.o";
+	int opt, ret = 1, log_level = 0;
+	const char *optstr = "I:NSxcv";
+	struct bpf_object *obj;
+	u32 prog_id;
+
+	struct bpf_object_open_attr open_attr = {
+						 .file = filename,
+						 .prog_type = BPF_PROG_TYPE_XDP,
+	};
+
+	while ((opt = getopt(argc, argv, optstr)) != -1) {
+		switch (opt) {
+		case 'I':
+			ifname = optarg;
+			ifindex = if_nametoindex(ifname);
+			if (!ifindex) {
+				fprintf(stderr, "Could not get interface %s\n",
+					ifname);
+				return 1;
+			}
+			break;
+		case 'N':
+			xdp_flags |= XDP_FLAGS_DRV_MODE;
+			break;
+		case 'S':
+			xdp_flags |= XDP_FLAGS_SKB_MODE;
+			break;
+		case 'x':
+			setup_only = true;
+			break;
+		case 'v':
+			log_level = 7;
+			break;
+		case 'c':
+			cleanup_only = true;
+			break;
+		default:
+			show_usage(basename(argv[0]));
+			return 1;
+		}
+	}
+
+	if (!ifname) {
+		show_usage(basename(argv[0]));
+		return 1;
+	}
+
+	if (cleanup_only) {
+		cleanup(0);
+		return 0;
+	}
+
+	if (!setup_only && optind == argc) {
+		show_usage(basename(argv[0]));
+		return 1;
+	}
+	dest = argv[optind];
+
+	if ((xdp_flags & mode_flags) == mode_flags) {
+		fprintf(stderr, "-N or -S can be specified, not both.\n");
+		show_usage(basename(argv[0]));
+		return 1;
+	}
+
+	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
+		perror("setrlimit(RLIMIT_MEMLOCK)");
+		return 1;
+	}
+
+	if (log_level)
+		libbpf_set_print(printfunc);
+
+	obj = bpf_object__open_xattr(&open_attr);
+
+	bpf_object__for_each_program(prog, obj) {
+		bpf_program__set_type(prog, BPF_PROG_TYPE_XDP);
+		prog->prog_flags = BPF_F_CHAIN_CALLS;
+		prog->log_level = log_level;
+		if ((ret = bpf_program__load(prog, "GPL", 0))) {
+			fprintf(stderr, "unable to load program: %s\n", strerror(-ret));
+			return 1;
+		}
+	}
+	pass_prog = bpf_object__find_program_by_title(obj, "xdp_dummy");
+	drop_prog = bpf_object__find_program_by_title(obj, "xdp_drop");
+
+	if (!pass_prog || !drop_prog) {
+		fprintf(stderr, "could not find xdp programs\n");
+		return 1;
+	}
+	pass_prog_fd = bpf_program__fd(pass_prog);
+	drop_prog_fd = bpf_program__fd(drop_prog);
+	if (pass_prog_fd < 0 || drop_prog_fd < 0) {
+		fprintf(stderr, "could not find xdp programs\n");
+		goto done;
+	}
+
+
+#define RUN_PING(should_fail, err) if ((ret = run_ping(should_fail, err))) goto done;
+
+	if (!setup_only) {
+		RUN_PING(false, "Pre-setup ping test");
+
+		signal(SIGINT, cleanup);
+		signal(SIGTERM, cleanup);
+	}
+
+	if ((ret = bpf_set_link_xdp_fd(ifindex, pass_prog_fd, xdp_flags)) < 0) {
+		fprintf(stderr, "Link set xdp fd failed for %s: %s\n", ifname,
+			strerror(-ret));
+		goto done;
+	}
+
+	if (!setup_only) {
+		sleep(1);
+		RUN_PING(false, "Empty map test");
+	}
+
+	if (bpf_prog_chain_add(pass_prog_fd, -1, drop_prog_fd)) {
+		fprintf(stderr, "unable to add chain prog wildcard: %s (%d)\n", strerror(errno), errno);
+		goto done;
+	}
+
+	if (bpf_prog_chain_get(pass_prog_fd, -1, &prog_id)) {
+		fprintf(stderr, "unable to get chain prog wildcard: %s (%d)\n", strerror(errno), errno);
+		goto done;
+	}
+	printf("Next program attached with ID: %u\n", prog_id);
+
+	if (setup_only) {
+		printf("Setup done; exiting.\n");
+		ret = 0;
+		goto done;
+	}
+
+	sleep(1);
+
+	RUN_PING(true, "Wildcard act test");
+
+	if (bpf_prog_chain_del(pass_prog_fd, -1)) {
+		fprintf(stderr, "unable to delete chain prog: %s\n", strerror(errno));
+		goto done;
+	}
+	sleep(1);
+
+	RUN_PING(false, "Post-delete map test");
+
+	if (bpf_prog_chain_add(pass_prog_fd, XDP_PASS, drop_prog_fd)) {
+		fprintf(stderr, "unable to add chain prog PASS: %s\n", strerror(errno));
+		goto done;
+	}
+	sleep(1);
+
+	RUN_PING(true, "Pass act test");
+
+
+	if ((ret = bpf_set_link_xdp_fd(ifindex, -1, xdp_flags)) < 0) {
+		fprintf(stderr, "Link clear xdp fd failed for %s: '%s'\n", ifname, strerror(-ret));
+		goto done;
+	}
+	sleep(1);
+
+	RUN_PING(false, "Post-delete prog test");
+
+
+done:
+	if (!setup_only)
+		cleanup(ret);
+
+	if (pass_prog_fd > 0)
+		close(pass_prog_fd);
+	if (drop_prog_fd > 0)
+		close(drop_prog_fd);
+
+	return ret;
+}


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

* RE: [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls
  2019-10-07 17:20 [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
                   ` (4 preceding siblings ...)
  2019-10-07 17:20 ` [PATCH bpf-next v3 5/5] selftests: Add tests for XDP chain calls Toke Høiland-Jørgensen
@ 2019-10-07 18:58 ` " John Fastabend
  2019-10-08  8:42   ` Toke Høiland-Jørgensen
  5 siblings, 1 reply; 29+ messages in thread
From: John Fastabend @ 2019-10-07 18:58 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

Toke Høiland-Jørgensen wrote:
> This series adds support for executing multiple XDP programs on a single
> interface in sequence, through the use of chain calls, as discussed at the Linux
> Plumbers Conference last month:
> 
> https://linuxplumbersconf.org/event/4/contributions/460/
> 

Can we add RFC to the title if we are just iterating through idea-space here.

> # HIGH-LEVEL IDEA
> 
> Since Alexei pointed out some issues with trying to rewrite the eBPF byte code,
> let's try a third approach: We add the ability to chain call programs into the
> eBPF execution core itself, but without rewriting the eBPF byte code.
> 
> As in the previous version, the bpf() syscall gets a couple of new commands
> which takes a pair of BPF program fds and a return code. It will then attach the
> second program to the first one in a structured keyed by return code. When a
> program chain is thus established, the former program will tail call to the
> latter at the end of its execution.
> 
> The actual tail calling is achieved by adding a new flag to struct bpf_prog and
> having BPF_PROG_RUN run the chain call logic if that flag is set. This means
> that if the feature is *not* used, the overhead is a single conditional branch
> (which means I couldn't measure a performance difference, as can be seen in the
> results below).
> 

I still believe user space should be able to link these multiple programs
together as Ed and I were suggesting in the last series. It seems much cleaner
to handle this with calls and linker steps vs adding something on the side to
handle this. Also by doing it by linking your control program can be arbitrary
complex. For example not just taking the output of one program and jumping
to another but doing arbitrary more complex/interesting things. Taking the
input from multiple programs to pick next call for example.

Maybe I missed a point but it seems like the main complaint is tail calls and
regular calls don't mix well. We want to fix this regardless so I don't think
that should be a blocker on using a linking step in user space.

> For this version I kept the load-time flag from the previous version, to avoid
> having to remove the read-only memory protection from the bpf prog. Only
> programs loaded with this flag set can have other programs attached to them for
> chain calls.
> 
> As before, it shouldn't be necessary to set the flag on program load time, but
> rather we should enable the feature when a chain call program is first loaded.
> We could conceivably just remove the RO property from the first page of struct
> bpf_prog and set the flag as needed.
> 
> # PERFORMANCE
> 
> I performed a simple performance test to get an initial feel for the overhead of
> the chain call mechanism. This test consists of running only two programs in
> sequence: One that returns XDP_PASS and another that returns XDP_DROP. I then
> measure the drop PPS performance and compare it to a baseline of just a single
> program that only returns XDP_DROP.
> 
> For comparison, a test case that uses regular eBPF tail calls to sequence two
> programs together is also included.
> 
> | Test case                        | Perf      | Overhead |
> |----------------------------------+-----------+----------|
> | Before patch (XDP DROP program)  | 31.5 Mpps |          |
> | After patch (XDP DROP program)   | 32.0 Mpps |          |
> | XDP chain call (XDP_PASS return) | 28.5 Mpps | 3.8 ns   |
> | XDP chain call (wildcard return) | 28.1 Mpps | 4.3 ns   |
> 
> I consider the "Before patch" and "After patch" to be identical; the .5 Mpps
> difference is within the regular test variance I see between runs. Likewise,
> there is probably no significant difference between hooking the XDP_PASS return
> code and using the wildcard slot.
> 
> # PATCH SET STRUCTURE
> This series is structured as follows:
> 
> - Patch 1: Adds the call chain looping logic
> - Patch 2: Adds the new commands added to the bpf() syscall
> - Patch 3-4: Tools/ update and libbpf syscall wrappers
> - Patch 5: Selftest  with example user space code (a bit hacky still)
> 
> The whole series is also available in my git repo on kernel.org:
> https://git.kernel.org/pub/scm/linux/kernel/git/toke/linux.git/log/?h=xdp-multiprog-03
> 
> Changelog:
> 
> v3:
>   - Keep the UAPI from v2, but change the implementation to hook into
>     BPF_PROG_RUN instead of trying to inject instructions into the eBPF program
>     itself (since that had problems as Alexei pointed out).
> v2:
>   - Completely new approach that integrates chain calls into the core eBPF
>     runtime instead of doing the map XDP-specific thing with a new map from v1.
> 
> ---
> 
> Toke Høiland-Jørgensen (5):
>       bpf: Support chain calling multiple BPF programs after each other
>       bpf: Add support for setting chain call sequence for programs
>       tools: Update bpf.h header for program chain calls
>       libbpf: Add syscall wrappers for BPF_PROG_CHAIN_* commands
>       selftests: Add tests for XDP chain calls
> 
> 
>  include/linux/bpf.h                           |    3 
>  include/linux/filter.h                        |   34 +++
>  include/uapi/linux/bpf.h                      |   16 +
>  kernel/bpf/core.c                             |    6 
>  kernel/bpf/syscall.c                          |   82 ++++++-
>  tools/include/uapi/linux/bpf.h                |   16 +
>  tools/lib/bpf/bpf.c                           |   34 +++
>  tools/lib/bpf/bpf.h                           |    4 
>  tools/lib/bpf/libbpf.map                      |    3 
>  tools/testing/selftests/bpf/.gitignore        |    1 
>  tools/testing/selftests/bpf/Makefile          |    3 
>  tools/testing/selftests/bpf/progs/xdp_dummy.c |    6 
>  tools/testing/selftests/bpf/test_xdp_chain.sh |   77 ++++++
>  tools/testing/selftests/bpf/xdp_chain.c       |  313 +++++++++++++++++++++++++
>  14 files changed, 594 insertions(+), 4 deletions(-)
>  create mode 100755 tools/testing/selftests/bpf/test_xdp_chain.sh
>  create mode 100644 tools/testing/selftests/bpf/xdp_chain.c
> 



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

* Re: [PATCH bpf-next v3 2/5] bpf: Add support for setting chain call sequence for programs
  2019-10-07 17:20 ` [PATCH bpf-next v3 2/5] bpf: Add support for setting chain call sequence for programs Toke Høiland-Jørgensen
@ 2019-10-07 20:38   ` Daniel Borkmann
  2019-10-08  8:09     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 29+ messages in thread
From: Daniel Borkmann @ 2019-10-07 20:38 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

On Mon, Oct 07, 2019 at 07:20:37PM +0200, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
> 
> This adds support for setting and deleting bpf chain call programs through
> a couple of new commands in the bpf() syscall. The CHAIN_ADD and CHAIN_DEL
> commands take two eBPF program fds and a return code, and install the
> 'next' program to be chain called after the 'prev' program if that program
> returns 'retcode'. A retcode of -1 means "wildcard", so that the program
> will be executed regardless of the previous program's return code.
> 
> 
> The syscall command names are based on Alexei's prog_chain example[0],
> which Alan helpfully rebased on current bpf-next. However, the logic and
> program storage is obviously adapted to the execution logic in the previous
> commit.
> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/ast/bpf.git/commit/?h=prog_chain&id=f54f45d00f91e083f6aec2abe35b6f0be52ae85b&context=15
> 
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
>  include/uapi/linux/bpf.h |   10 ++++++
>  kernel/bpf/syscall.c     |   78 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 1ce80a227be3..b03c23963af8 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -107,6 +107,9 @@ enum bpf_cmd {
>  	BPF_MAP_LOOKUP_AND_DELETE_ELEM,
>  	BPF_MAP_FREEZE,
>  	BPF_BTF_GET_NEXT_ID,
> +	BPF_PROG_CHAIN_ADD,
> +	BPF_PROG_CHAIN_DEL,
> +	BPF_PROG_CHAIN_GET,
>  };
>  
>  enum bpf_map_type {
> @@ -516,6 +519,13 @@ union bpf_attr {
>  		__u64		probe_offset;	/* output: probe_offset */
>  		__u64		probe_addr;	/* output: probe_addr */
>  	} task_fd_query;
> +
> +	struct { /* anonymous struct used by BPF_PROG_CHAIN_* commands */
> +		__u32		prev_prog_fd;
> +		__u32		next_prog_fd;
> +		__u32		retcode;
> +		__u32		next_prog_id;   /* output: prog_id */
> +	};
>  } __attribute__((aligned(8)));
>  
>  /* The description below is an attempt at providing documentation to eBPF
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index b8a203a05881..be8112e08a88 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2113,6 +2113,79 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
>  	return ret;
>  }
>  
> +#define BPF_PROG_CHAIN_LAST_FIELD next_prog_id
> +
> +static int bpf_prog_chain(int cmd, const union bpf_attr *attr,
> +			  union bpf_attr __user *uattr)
> +{
> +	struct bpf_prog *prog, *next_prog, *old_prog;
> +	struct bpf_prog **array;
> +	int ret = -EOPNOTSUPP;
> +	u32 index, prog_id;
> +
> +	if (CHECK_ATTR(BPF_PROG_CHAIN))
> +		return -EINVAL;
> +
> +	/* Index 0 is wildcard, encoded as ~0 by userspace */
> +	if (attr->retcode == ((u32) ~0))
> +		index = 0;
> +	else
> +		index = attr->retcode + 1;
> +
> +	if (index >= BPF_NUM_CHAIN_SLOTS)
> +		return -E2BIG;
> +
> +	prog = bpf_prog_get(attr->prev_prog_fd);
> +	if (IS_ERR(prog))
> +		return PTR_ERR(prog);
> +
> +	/* If the chain_calls bit is not set, that's because the chain call flag
> +	 * was not set on program load, and so we can't support chain calls.
> +	 */
> +	if (!prog->chain_calls)
> +		goto out;
> +
> +	array = prog->aux->chain_progs;
> +
> +	switch (cmd) {
> +	case BPF_PROG_CHAIN_ADD:
> +		next_prog = bpf_prog_get(attr->next_prog_fd);
> +		if (IS_ERR(next_prog)) {
> +			ret = PTR_ERR(next_prog);
> +			break;
> +		}
> +		old_prog = xchg(array + index, next_prog);
> +		if (old_prog)
> +			bpf_prog_put(old_prog);
> +		ret = 0;
> +		break;

How are circular dependencies resolved here? Seems the situation is
not prevented, so progs unloaded via XDP won't get the __bpf_prog_free()
call where they then drop the references of all the other progs in the
chain.

> +	case BPF_PROG_CHAIN_DEL:
> +		old_prog = xchg(array + index, NULL);
> +		if (old_prog) {
> +			bpf_prog_put(old_prog);
> +			ret = 0;
> +		} else {
> +			ret = -ENOENT;
> +		}
> +		break;
> +	case BPF_PROG_CHAIN_GET:
> +		old_prog = READ_ONCE(array[index]);
> +		if (old_prog) {
> +			prog_id = old_prog->aux->id;
> +			if (put_user(prog_id, &uattr->next_prog_id))
> +				ret = -EFAULT;
> +			else
> +				ret = 0;
> +		} else
> +			ret = -ENOENT;
> +		break;
> +	}
> +
> +out:
> +	bpf_prog_put(prog);
> +	return ret;
> +}
> +
>  #define BPF_OBJ_GET_NEXT_ID_LAST_FIELD next_id
>  
>  static int bpf_obj_get_next_id(const union bpf_attr *attr,
> @@ -2885,6 +2958,11 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
>  	case BPF_PROG_TEST_RUN:
>  		err = bpf_prog_test_run(&attr, uattr);
>  		break;
> +	case BPF_PROG_CHAIN_ADD:
> +	case BPF_PROG_CHAIN_DEL:
> +	case BPF_PROG_CHAIN_GET:
> +		err = bpf_prog_chain(cmd, &attr, uattr);
> +		break;
>  	case BPF_PROG_GET_NEXT_ID:
>  		err = bpf_obj_get_next_id(&attr, uattr,
>  					  &prog_idr, &prog_idr_lock);
> 

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-07 17:20 ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Toke Høiland-Jørgensen
@ 2019-10-07 20:42   ` Alexei Starovoitov
  2019-10-08  8:07     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 29+ messages in thread
From: Alexei Starovoitov @ 2019-10-07 20:42 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

On Mon, Oct 07, 2019 at 07:20:36PM +0200, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
> 
> This adds support for wrapping eBPF program dispatch in chain calling
> logic. The code injection is controlled by a flag at program load time; if
> the flag is set, the BPF program will carry a flag bit that changes the
> program dispatch logic to wrap it in a chain call loop.
> 
> Ideally, it shouldn't be necessary to set the flag on program load time,
> but rather inject the calls when a chain call program is first loaded. The
> allocation logic sets the whole of struct bpf_prog to be read-only memory,
> so it can't immediately be modified, but conceivably we could just unlock
> the first page of the struct and flip the bit when a chain call program is
> first attached.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
>  include/linux/bpf.h      |    3 +++
>  include/linux/filter.h   |   34 ++++++++++++++++++++++++++++++++--
>  include/uapi/linux/bpf.h |    6 ++++++
>  kernel/bpf/core.c        |    6 ++++++
>  kernel/bpf/syscall.c     |    4 +++-
>  5 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 5b9d22338606..13e5f38cf5c6 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -365,6 +365,8 @@ struct bpf_prog_stats {
>  	struct u64_stats_sync syncp;
>  };
>  
> +#define BPF_NUM_CHAIN_SLOTS 8
> +
>  struct bpf_prog_aux {
>  	atomic_t refcnt;
>  	u32 used_map_cnt;
> @@ -383,6 +385,7 @@ struct bpf_prog_aux {
>  	struct list_head ksym_lnode;
>  	const struct bpf_prog_ops *ops;
>  	struct bpf_map **used_maps;
> +	struct bpf_prog *chain_progs[BPF_NUM_CHAIN_SLOTS];
>  	struct bpf_prog *prog;
>  	struct user_struct *user;
>  	u64 load_time; /* ns since boottime */
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 2ce57645f3cd..3d1e4991e61d 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -21,6 +21,7 @@
>  #include <linux/kallsyms.h>
>  #include <linux/if_vlan.h>
>  #include <linux/vmalloc.h>
> +#include <linux/nospec.h>
>  
>  #include <net/sch_generic.h>
>  
> @@ -528,6 +529,7 @@ struct bpf_prog {
>  				is_func:1,	/* program is a bpf function */
>  				kprobe_override:1, /* Do we override a kprobe? */
>  				has_callchain_buf:1, /* callchain buffer allocated? */
> +				chain_calls:1, /* should this use the chain_call wrapper */
>  				enforce_expected_attach_type:1; /* Enforce expected_attach_type checking at attach time */
>  	enum bpf_prog_type	type;		/* Type of BPF program */
>  	enum bpf_attach_type	expected_attach_type; /* For some prog types */
> @@ -551,6 +553,30 @@ struct sk_filter {
>  	struct bpf_prog	*prog;
>  };
>  
> +#define BPF_MAX_CHAIN_CALLS 32
> +static __always_inline unsigned int do_chain_calls(const struct bpf_prog *prog,
> +						   const void *ctx)
> +{
> +	int i = BPF_MAX_CHAIN_CALLS;
> +	int idx;
> +	u32 ret;
> +
> +	do {
> +		ret = (*(prog)->bpf_func)(ctx, prog->insnsi);

This breaks program stats.

> +
> +		if (ret + 1 >= BPF_NUM_CHAIN_SLOTS) {
> +			prog = prog->aux->chain_progs[0];
> +			continue;
> +		}
> +		idx = ret + 1;
> +		idx = array_index_nospec(idx, BPF_NUM_CHAIN_SLOTS);
> +
> +		prog = prog->aux->chain_progs[idx] ?: prog->aux->chain_progs[0];
> +	} while (prog && --i);
> +
> +	return ret;
> +}
> +
>  DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
>  
>  #define BPF_PROG_RUN(prog, ctx)	({				\
> @@ -559,14 +585,18 @@ DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
>  	if (static_branch_unlikely(&bpf_stats_enabled_key)) {	\
>  		struct bpf_prog_stats *stats;			\
>  		u64 start = sched_clock();			\
> -		ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
> +		ret = prog->chain_calls ?			\
> +			do_chain_calls(prog, ctx) :			\
> +			 (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\

I thought you agreed on 'no performance regressions' rule?

>  		stats = this_cpu_ptr(prog->aux->stats);		\
>  		u64_stats_update_begin(&stats->syncp);		\
>  		stats->cnt++;					\
>  		stats->nsecs += sched_clock() - start;		\
>  		u64_stats_update_end(&stats->syncp);		\
>  	} else {						\
> -		ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
> +		ret = prog->chain_calls ?				\
> +			do_chain_calls(prog, ctx) :			\
> +			 (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
>  	}							\
>  	ret; })
>  
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 77c6be96d676..1ce80a227be3 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -288,6 +288,12 @@ enum bpf_attach_type {
>  /* The verifier internal test flag. Behavior is undefined */
>  #define BPF_F_TEST_STATE_FREQ	(1U << 3)
>  
> +/* Whether to enable chain call logic at program execution. If set, the program
> + * execution logic will check for and jump to chain call programs configured
> + * with the BPF_PROG_CHAIN_* commands to the bpf syscall.
> + */
> +#define BPF_F_CHAIN_CALLS	(1U << 4)
> +
>  /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
>   * two extensions:
>   *
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 66088a9e9b9e..5dfe3585bc5d 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -254,6 +254,12 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
>  void __bpf_prog_free(struct bpf_prog *fp)
>  {
>  	if (fp->aux) {
> +		int i;
> +
> +		for (i = 0; i < BPF_NUM_CHAIN_SLOTS; i++)
> +			if (fp->aux->chain_progs[i])
> +				bpf_prog_put(fp->aux->chain_progs[i]);
> +
>  		free_percpu(fp->aux->stats);
>  		kfree(fp->aux);
>  	}
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 82eabd4e38ad..b8a203a05881 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1630,7 +1630,8 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
>  	if (attr->prog_flags & ~(BPF_F_STRICT_ALIGNMENT |
>  				 BPF_F_ANY_ALIGNMENT |
>  				 BPF_F_TEST_STATE_FREQ |
> -				 BPF_F_TEST_RND_HI32))
> +				 BPF_F_TEST_RND_HI32 |
> +				 BPF_F_CHAIN_CALLS))
>  		return -EINVAL;
>  
>  	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
> @@ -1665,6 +1666,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
>  		return -ENOMEM;
>  
>  	prog->expected_attach_type = attr->expected_attach_type;
> +	prog->chain_calls = !!(attr->prog_flags & BPF_F_CHAIN_CALLS);
>  
>  	prog->aux->offload_requested = !!attr->prog_ifindex;
>  
> 

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-07 20:42   ` Alexei Starovoitov
@ 2019-10-08  8:07     ` Toke Høiland-Jørgensen
  2019-10-09  1:51       ` Alexei Starovoitov
  0 siblings, 1 reply; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-08  8:07 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Mon, Oct 07, 2019 at 07:20:36PM +0200, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> 
>> This adds support for wrapping eBPF program dispatch in chain calling
>> logic. The code injection is controlled by a flag at program load time; if
>> the flag is set, the BPF program will carry a flag bit that changes the
>> program dispatch logic to wrap it in a chain call loop.
>> 
>> Ideally, it shouldn't be necessary to set the flag on program load time,
>> but rather inject the calls when a chain call program is first loaded. The
>> allocation logic sets the whole of struct bpf_prog to be read-only memory,
>> so it can't immediately be modified, but conceivably we could just unlock
>> the first page of the struct and flip the bit when a chain call program is
>> first attached.
>> 
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> ---
>>  include/linux/bpf.h      |    3 +++
>>  include/linux/filter.h   |   34 ++++++++++++++++++++++++++++++++--
>>  include/uapi/linux/bpf.h |    6 ++++++
>>  kernel/bpf/core.c        |    6 ++++++
>>  kernel/bpf/syscall.c     |    4 +++-
>>  5 files changed, 50 insertions(+), 3 deletions(-)
>> 
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 5b9d22338606..13e5f38cf5c6 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -365,6 +365,8 @@ struct bpf_prog_stats {
>>  	struct u64_stats_sync syncp;
>>  };
>>  
>> +#define BPF_NUM_CHAIN_SLOTS 8
>> +
>>  struct bpf_prog_aux {
>>  	atomic_t refcnt;
>>  	u32 used_map_cnt;
>> @@ -383,6 +385,7 @@ struct bpf_prog_aux {
>>  	struct list_head ksym_lnode;
>>  	const struct bpf_prog_ops *ops;
>>  	struct bpf_map **used_maps;
>> +	struct bpf_prog *chain_progs[BPF_NUM_CHAIN_SLOTS];
>>  	struct bpf_prog *prog;
>>  	struct user_struct *user;
>>  	u64 load_time; /* ns since boottime */
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index 2ce57645f3cd..3d1e4991e61d 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -21,6 +21,7 @@
>>  #include <linux/kallsyms.h>
>>  #include <linux/if_vlan.h>
>>  #include <linux/vmalloc.h>
>> +#include <linux/nospec.h>
>>  
>>  #include <net/sch_generic.h>
>>  
>> @@ -528,6 +529,7 @@ struct bpf_prog {
>>  				is_func:1,	/* program is a bpf function */
>>  				kprobe_override:1, /* Do we override a kprobe? */
>>  				has_callchain_buf:1, /* callchain buffer allocated? */
>> +				chain_calls:1, /* should this use the chain_call wrapper */
>>  				enforce_expected_attach_type:1; /* Enforce expected_attach_type checking at attach time */
>>  	enum bpf_prog_type	type;		/* Type of BPF program */
>>  	enum bpf_attach_type	expected_attach_type; /* For some prog types */
>> @@ -551,6 +553,30 @@ struct sk_filter {
>>  	struct bpf_prog	*prog;
>>  };
>>  
>> +#define BPF_MAX_CHAIN_CALLS 32
>> +static __always_inline unsigned int do_chain_calls(const struct bpf_prog *prog,
>> +						   const void *ctx)
>> +{
>> +	int i = BPF_MAX_CHAIN_CALLS;
>> +	int idx;
>> +	u32 ret;
>> +
>> +	do {
>> +		ret = (*(prog)->bpf_func)(ctx, prog->insnsi);
>
> This breaks program stats.

Oh, right, silly me. Will fix.

>> +
>> +		if (ret + 1 >= BPF_NUM_CHAIN_SLOTS) {
>> +			prog = prog->aux->chain_progs[0];
>> +			continue;
>> +		}
>> +		idx = ret + 1;
>> +		idx = array_index_nospec(idx, BPF_NUM_CHAIN_SLOTS);
>> +
>> +		prog = prog->aux->chain_progs[idx] ?: prog->aux->chain_progs[0];
>> +	} while (prog && --i);
>> +
>> +	return ret;
>> +}
>> +
>>  DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
>>  
>>  #define BPF_PROG_RUN(prog, ctx)	({				\
>> @@ -559,14 +585,18 @@ DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
>>  	if (static_branch_unlikely(&bpf_stats_enabled_key)) {	\
>>  		struct bpf_prog_stats *stats;			\
>>  		u64 start = sched_clock();			\
>> -		ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
>> +		ret = prog->chain_calls ?			\
>> +			do_chain_calls(prog, ctx) :			\
>> +			 (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
>
> I thought you agreed on 'no performance regressions' rule?

As I wrote in the cover letter I could not measurable a performance
impact from this, even with the simplest possible XDP program (where
program setup time has the largest impact).

This was the performance before/after patch (also in the cover letter):

Before patch (XDP DROP program):  31.5 Mpps
After patch (XDP DROP program):   32.0 Mpps

So actually this *increases* performance ;)
(Or rather, the difference is within the measurement uncertainty on my
system).

-Toke

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

* Re: [PATCH bpf-next v3 2/5] bpf: Add support for setting chain call sequence for programs
  2019-10-07 20:38   ` Daniel Borkmann
@ 2019-10-08  8:09     ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-08  8:09 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

Daniel Borkmann <daniel@iogearbox.net> writes:

> On Mon, Oct 07, 2019 at 07:20:37PM +0200, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> 
>> This adds support for setting and deleting bpf chain call programs through
>> a couple of new commands in the bpf() syscall. The CHAIN_ADD and CHAIN_DEL
>> commands take two eBPF program fds and a return code, and install the
>> 'next' program to be chain called after the 'prev' program if that program
>> returns 'retcode'. A retcode of -1 means "wildcard", so that the program
>> will be executed regardless of the previous program's return code.
>> 
>> 
>> The syscall command names are based on Alexei's prog_chain example[0],
>> which Alan helpfully rebased on current bpf-next. However, the logic and
>> program storage is obviously adapted to the execution logic in the previous
>> commit.
>> 
>> [0] https://git.kernel.org/pub/scm/linux/kernel/git/ast/bpf.git/commit/?h=prog_chain&id=f54f45d00f91e083f6aec2abe35b6f0be52ae85b&context=15
>> 
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> ---
>>  include/uapi/linux/bpf.h |   10 ++++++
>>  kernel/bpf/syscall.c     |   78 ++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 88 insertions(+)
>> 
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 1ce80a227be3..b03c23963af8 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -107,6 +107,9 @@ enum bpf_cmd {
>>  	BPF_MAP_LOOKUP_AND_DELETE_ELEM,
>>  	BPF_MAP_FREEZE,
>>  	BPF_BTF_GET_NEXT_ID,
>> +	BPF_PROG_CHAIN_ADD,
>> +	BPF_PROG_CHAIN_DEL,
>> +	BPF_PROG_CHAIN_GET,
>>  };
>>  
>>  enum bpf_map_type {
>> @@ -516,6 +519,13 @@ union bpf_attr {
>>  		__u64		probe_offset;	/* output: probe_offset */
>>  		__u64		probe_addr;	/* output: probe_addr */
>>  	} task_fd_query;
>> +
>> +	struct { /* anonymous struct used by BPF_PROG_CHAIN_* commands */
>> +		__u32		prev_prog_fd;
>> +		__u32		next_prog_fd;
>> +		__u32		retcode;
>> +		__u32		next_prog_id;   /* output: prog_id */
>> +	};
>>  } __attribute__((aligned(8)));
>>  
>>  /* The description below is an attempt at providing documentation to eBPF
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index b8a203a05881..be8112e08a88 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -2113,6 +2113,79 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
>>  	return ret;
>>  }
>>  
>> +#define BPF_PROG_CHAIN_LAST_FIELD next_prog_id
>> +
>> +static int bpf_prog_chain(int cmd, const union bpf_attr *attr,
>> +			  union bpf_attr __user *uattr)
>> +{
>> +	struct bpf_prog *prog, *next_prog, *old_prog;
>> +	struct bpf_prog **array;
>> +	int ret = -EOPNOTSUPP;
>> +	u32 index, prog_id;
>> +
>> +	if (CHECK_ATTR(BPF_PROG_CHAIN))
>> +		return -EINVAL;
>> +
>> +	/* Index 0 is wildcard, encoded as ~0 by userspace */
>> +	if (attr->retcode == ((u32) ~0))
>> +		index = 0;
>> +	else
>> +		index = attr->retcode + 1;
>> +
>> +	if (index >= BPF_NUM_CHAIN_SLOTS)
>> +		return -E2BIG;
>> +
>> +	prog = bpf_prog_get(attr->prev_prog_fd);
>> +	if (IS_ERR(prog))
>> +		return PTR_ERR(prog);
>> +
>> +	/* If the chain_calls bit is not set, that's because the chain call flag
>> +	 * was not set on program load, and so we can't support chain calls.
>> +	 */
>> +	if (!prog->chain_calls)
>> +		goto out;
>> +
>> +	array = prog->aux->chain_progs;
>> +
>> +	switch (cmd) {
>> +	case BPF_PROG_CHAIN_ADD:
>> +		next_prog = bpf_prog_get(attr->next_prog_fd);
>> +		if (IS_ERR(next_prog)) {
>> +			ret = PTR_ERR(next_prog);
>> +			break;
>> +		}
>> +		old_prog = xchg(array + index, next_prog);
>> +		if (old_prog)
>> +			bpf_prog_put(old_prog);
>> +		ret = 0;
>> +		break;
>
> How are circular dependencies resolved here? Seems the situation is
> not prevented, so progs unloaded via XDP won't get the __bpf_prog_free()
> call where they then drop the references of all the other progs in the
> chain.

Yeah, that's true. My plan was to just walk the "call graph" on insert
and reject any circular inserts. Just haven't gotten around to adding
that yet; will fix that in the next version.

-Toke

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

* RE: [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls
  2019-10-07 18:58 ` [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through " John Fastabend
@ 2019-10-08  8:42   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-08  8:42 UTC (permalink / raw)
  To: John Fastabend, Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

John Fastabend <john.fastabend@gmail.com> writes:

> Toke Høiland-Jørgensen wrote:
>> This series adds support for executing multiple XDP programs on a single
>> interface in sequence, through the use of chain calls, as discussed at the Linux
>> Plumbers Conference last month:
>> 
>> https://linuxplumbersconf.org/event/4/contributions/460/
>> 
>
> Can we add RFC to the title if we are just iterating through
> idea-space here.

I don't view this as "just iterating through idea-space". I'll admit
that I may have overestimated the extent to which we were all on the
same page on this after LPC, but I do view these submissions as serious
proposals that are making progress... :)

>> # HIGH-LEVEL IDEA
>> 
>> Since Alexei pointed out some issues with trying to rewrite the eBPF byte code,
>> let's try a third approach: We add the ability to chain call programs into the
>> eBPF execution core itself, but without rewriting the eBPF byte code.
>> 
>> As in the previous version, the bpf() syscall gets a couple of new commands
>> which takes a pair of BPF program fds and a return code. It will then attach the
>> second program to the first one in a structured keyed by return code. When a
>> program chain is thus established, the former program will tail call to the
>> latter at the end of its execution.
>> 
>> The actual tail calling is achieved by adding a new flag to struct bpf_prog and
>> having BPF_PROG_RUN run the chain call logic if that flag is set. This means
>> that if the feature is *not* used, the overhead is a single conditional branch
>> (which means I couldn't measure a performance difference, as can be seen in the
>> results below).
>> 
>
> I still believe user space should be able to link these multiple
> programs together as Ed and I were suggesting in the last series.

I expect that userspace probably could (I mean, after all, eBPF is
within spitting distance of a full almost-Turing-complete executing
environment so userspace can conceivably do pretty much anything).

However, I don't believe that doing it in userspace is the best
solution. I view it as a tradeoff: How much complexity do we have to add
to the kernel to make things easier for userspace. And given that we can
do this without negatively impacting performance, and with a reasonable
cost in terms of complexity (both of which I think this series
demonstrates that yes, we can), I think this makes sense to put in the
kernel.

> Also by doing it by linking your control program can be arbitrary
> complex. For example not just taking the output of one program and
> jumping to another but doing arbitrary more complex/interesting
> things. Taking the input from multiple programs to pick next call for
> example.

I expect there will indeed be more complex use cases where combining
multiple programs in arbitrary complex ways would make a lot of sense,
and doing that by linking in userspace is probably a good fit for that.
But for the simple use case of running multiple programs after one
another, I think it is overkill, and something that is better to do in
the kernel.

-Toke

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-08  8:07     ` Toke Høiland-Jørgensen
@ 2019-10-09  1:51       ` Alexei Starovoitov
  2019-10-09  8:03         ` Toke Høiland-Jørgensen
  2019-10-09 10:19         ` Jesper Dangaard Brouer
  0 siblings, 2 replies; 29+ messages in thread
From: Alexei Starovoitov @ 2019-10-09  1:51 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

On Tue, Oct 08, 2019 at 10:07:46AM +0200, Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> 
> > On Mon, Oct 07, 2019 at 07:20:36PM +0200, Toke Høiland-Jørgensen wrote:
> >> From: Toke Høiland-Jørgensen <toke@redhat.com>
> >> 
> >> This adds support for wrapping eBPF program dispatch in chain calling
> >> logic. The code injection is controlled by a flag at program load time; if
> >> the flag is set, the BPF program will carry a flag bit that changes the
> >> program dispatch logic to wrap it in a chain call loop.
> >> 
> >> Ideally, it shouldn't be necessary to set the flag on program load time,
> >> but rather inject the calls when a chain call program is first loaded. The
> >> allocation logic sets the whole of struct bpf_prog to be read-only memory,
> >> so it can't immediately be modified, but conceivably we could just unlock
> >> the first page of the struct and flip the bit when a chain call program is
> >> first attached.
> >> 
> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> >> ---
> >>  include/linux/bpf.h      |    3 +++
> >>  include/linux/filter.h   |   34 ++++++++++++++++++++++++++++++++--
> >>  include/uapi/linux/bpf.h |    6 ++++++
> >>  kernel/bpf/core.c        |    6 ++++++
> >>  kernel/bpf/syscall.c     |    4 +++-
> >>  5 files changed, 50 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> >> index 5b9d22338606..13e5f38cf5c6 100644
> >> --- a/include/linux/bpf.h
> >> +++ b/include/linux/bpf.h
> >> @@ -365,6 +365,8 @@ struct bpf_prog_stats {
> >>  	struct u64_stats_sync syncp;
> >>  };
> >>  
> >> +#define BPF_NUM_CHAIN_SLOTS 8
> >> +
> >>  struct bpf_prog_aux {
> >>  	atomic_t refcnt;
> >>  	u32 used_map_cnt;
> >> @@ -383,6 +385,7 @@ struct bpf_prog_aux {
> >>  	struct list_head ksym_lnode;
> >>  	const struct bpf_prog_ops *ops;
> >>  	struct bpf_map **used_maps;
> >> +	struct bpf_prog *chain_progs[BPF_NUM_CHAIN_SLOTS];
> >>  	struct bpf_prog *prog;
> >>  	struct user_struct *user;
> >>  	u64 load_time; /* ns since boottime */
> >> diff --git a/include/linux/filter.h b/include/linux/filter.h
> >> index 2ce57645f3cd..3d1e4991e61d 100644
> >> --- a/include/linux/filter.h
> >> +++ b/include/linux/filter.h
> >> @@ -21,6 +21,7 @@
> >>  #include <linux/kallsyms.h>
> >>  #include <linux/if_vlan.h>
> >>  #include <linux/vmalloc.h>
> >> +#include <linux/nospec.h>
> >>  
> >>  #include <net/sch_generic.h>
> >>  
> >> @@ -528,6 +529,7 @@ struct bpf_prog {
> >>  				is_func:1,	/* program is a bpf function */
> >>  				kprobe_override:1, /* Do we override a kprobe? */
> >>  				has_callchain_buf:1, /* callchain buffer allocated? */
> >> +				chain_calls:1, /* should this use the chain_call wrapper */
> >>  				enforce_expected_attach_type:1; /* Enforce expected_attach_type checking at attach time */
> >>  	enum bpf_prog_type	type;		/* Type of BPF program */
> >>  	enum bpf_attach_type	expected_attach_type; /* For some prog types */
> >> @@ -551,6 +553,30 @@ struct sk_filter {
> >>  	struct bpf_prog	*prog;
> >>  };
> >>  
> >> +#define BPF_MAX_CHAIN_CALLS 32
> >> +static __always_inline unsigned int do_chain_calls(const struct bpf_prog *prog,
> >> +						   const void *ctx)
> >> +{
> >> +	int i = BPF_MAX_CHAIN_CALLS;
> >> +	int idx;
> >> +	u32 ret;
> >> +
> >> +	do {
> >> +		ret = (*(prog)->bpf_func)(ctx, prog->insnsi);
> >
> > This breaks program stats.
> 
> Oh, right, silly me. Will fix.
> 
> >> +
> >> +		if (ret + 1 >= BPF_NUM_CHAIN_SLOTS) {
> >> +			prog = prog->aux->chain_progs[0];
> >> +			continue;
> >> +		}
> >> +		idx = ret + 1;
> >> +		idx = array_index_nospec(idx, BPF_NUM_CHAIN_SLOTS);
> >> +
> >> +		prog = prog->aux->chain_progs[idx] ?: prog->aux->chain_progs[0];
> >> +	} while (prog && --i);
> >> +
> >> +	return ret;
> >> +}
> >> +
> >>  DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> >>  
> >>  #define BPF_PROG_RUN(prog, ctx)	({				\
> >> @@ -559,14 +585,18 @@ DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> >>  	if (static_branch_unlikely(&bpf_stats_enabled_key)) {	\
> >>  		struct bpf_prog_stats *stats;			\
> >>  		u64 start = sched_clock();			\
> >> -		ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
> >> +		ret = prog->chain_calls ?			\
> >> +			do_chain_calls(prog, ctx) :			\
> >> +			 (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
> >
> > I thought you agreed on 'no performance regressions' rule?
> 
> As I wrote in the cover letter I could not measurable a performance
> impact from this, even with the simplest possible XDP program (where
> program setup time has the largest impact).
> 
> This was the performance before/after patch (also in the cover letter):
> 
> Before patch (XDP DROP program):  31.5 Mpps
> After patch (XDP DROP program):   32.0 Mpps
> 
> So actually this *increases* performance ;)
> (Or rather, the difference is within the measurement uncertainty on my
> system).

I have hard time believing such numbers.
If I wasn't clear before: Nack to such hack in BPF_PROG_RUN.
Please implement proper indirect calls and jumps.
Apps have to cooperate with each other regardless
whereas above is a narrow solution to one problem.


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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-09  1:51       ` Alexei Starovoitov
@ 2019-10-09  8:03         ` Toke Høiland-Jørgensen
  2019-10-10  4:41           ` Alexei Starovoitov
  2019-10-09 10:19         ` Jesper Dangaard Brouer
  1 sibling, 1 reply; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-09  8:03 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> Please implement proper indirect calls and jumps.

I am still not convinced this will actually solve our problem; but OK, I
can give it a shot.

However, I don't actually have a clear picture of what exactly is
missing to add this support. Could you please provide a pointer or two?

-Toke

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-09  1:51       ` Alexei Starovoitov
  2019-10-09  8:03         ` Toke Høiland-Jørgensen
@ 2019-10-09 10:19         ` Jesper Dangaard Brouer
  2019-10-09 17:57           ` Alexei Starovoitov
  1 sibling, 1 reply; 29+ messages in thread
From: Jesper Dangaard Brouer @ 2019-10-09 10:19 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Toke Høiland-Jørgensen, Daniel Borkmann,
	Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire, David Miller,
	netdev, bpf, brouer

On Tue, 8 Oct 2019 18:51:19 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Tue, Oct 08, 2019 at 10:07:46AM +0200, Toke Høiland-Jørgensen wrote:
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> >   
> > > On Mon, Oct 07, 2019 at 07:20:36PM +0200, Toke Høiland-Jørgensen wrote:  
> > >> From: Toke Høiland-Jørgensen <toke@redhat.com>
> > >> 
> > >> This adds support for wrapping eBPF program dispatch in chain calling
> > >> logic. The code injection is controlled by a flag at program load time; if
> > >> the flag is set, the BPF program will carry a flag bit that changes the
> > >> program dispatch logic to wrap it in a chain call loop.
[...]
> > >> diff --git a/include/linux/filter.h b/include/linux/filter.h
> > >> index 2ce57645f3cd..3d1e4991e61d 100644
> > >> --- a/include/linux/filter.h
> > >> +++ b/include/linux/filter.h
[...]
> > >>  #define BPF_PROG_RUN(prog, ctx)	({				\
> > >> @@ -559,14 +585,18 @@ DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> > >>  	if (static_branch_unlikely(&bpf_stats_enabled_key)) {	\
> > >>  		struct bpf_prog_stats *stats;			\
> > >>  		u64 start = sched_clock();			\
> > >> -		ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
> > >> +		ret = prog->chain_calls ?			\
> > >> +			do_chain_calls(prog, ctx) :			\
> > >> +			 (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\  
> > >
> > > I thought you agreed on 'no performance regressions' rule?  
> > 
> > As I wrote in the cover letter I could not measurable a performance
> > impact from this, even with the simplest possible XDP program (where
> > program setup time has the largest impact).
> > 
> > This was the performance before/after patch (also in the cover letter):
> > 
> > Before patch (XDP DROP program):  31.5 Mpps
> > After patch (XDP DROP program):   32.0 Mpps
> > 
> > So actually this *increases* performance ;)
> > (Or rather, the difference is within the measurement uncertainty on my
> > system).  
> 
> I have hard time believing such numbers.

Don't look at this as +/- 500Kpps.  Instead you have to realize that the
performance difference in ns (nano-seconds) is only 0.5 ns (0.496 ns).

 (1/31.5*1000)-(1/32.0*1000) = 0.4960 ns

This "half-a-nanosec" is below the measurement uncertainty of any
system. My system have approx 2-3 ns measurement variance, which I'm
proud of.  At these speeds (32Mpps) this e.g. 3 ns variance would
result in -2.8 Mpps (29.2Mpps).


The change Toke did in BPF_PROG_RUN does not introduce any measurable
performance change, as least on high-end Intel CPUs.  This DOES satisfy
'no performance regressions' rule.  You can dislike the solution for
other reasons ;-)

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-09 10:19         ` Jesper Dangaard Brouer
@ 2019-10-09 17:57           ` Alexei Starovoitov
  0 siblings, 0 replies; 29+ messages in thread
From: Alexei Starovoitov @ 2019-10-09 17:57 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Toke Høiland-Jørgensen, Daniel Borkmann,
	Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire, David Miller,
	Network Development, bpf

On Wed, Oct 9, 2019 at 3:20 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
>
> The change Toke did in BPF_PROG_RUN does not introduce any measurable
> performance change, as least on high-end Intel CPUs.  This DOES satisfy
> 'no performance regressions' rule.

It adds extra load and branch in critical path of every program.
Including classic socket filters.
Not being able to measure the overhead in a microbenchmark
doesn't mean that overhead is not added.
Will 10 such branches be measurable?
What if they're not?
Then everyone will say: "It's not measurable in my setup, hence
I'm adding these branches".
tcp stack is even harder to measure. Yet, Eric will rightfully nack patches
that add such things when alternative solution is possible.

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-09  8:03         ` Toke Høiland-Jørgensen
@ 2019-10-10  4:41           ` Alexei Starovoitov
  2019-10-14 12:35             ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 29+ messages in thread
From: Alexei Starovoitov @ 2019-10-10  4:41 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

On Wed, Oct 09, 2019 at 10:03:43AM +0200, Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> 
> > Please implement proper indirect calls and jumps.
> 
> I am still not convinced this will actually solve our problem; but OK, I
> can give it a shot.

If you're not convinced let's talk about it first.

Indirect calls is a building block for debugpoints.
Let's not call them tracepoints, because Linus banned any discusion
that includes that name.
The debugpoints is a way for BPF program to insert points in its
code to let external facility to do tracing and debugging.

void (*debugpoint1)(struct xdp_buff *, int code);
void (*debugpoint2)(struct xdp_buff *);
void (*debugpoint3)(int len);

int bpf_prog(struct xdp_buff *ctx)
{
    // let's parse the packet
    if (debugpoint3)
        debugpoint3(ctx->data_end - ctx->data);

    if (condition) {
        // deciding to drop this packet
        if (debugpoint1)
            debugpoint1(ctx, XDP_DROP);
        return XDP_DROP;
    }
    if (some other condition) {
        // lets pass it to the stack
        if (debugpoint2)
            debugpoint2(ctx);
        return XDP_PASS;
    }
}

In normal operation nothing is being called.
The execution cost to the program is load plus branch.
But since program is annotated with BTF the external tool,
like bpftool, can load another program and populate
debugpointN pointer in the original program to trace its
execution.
Essentially it's live debugging (tracing) of cooperative
bpf programs that added debugpoints to their code.

Obviously indirect calls can be used for a ton of other things
including proper chaing of progs, but I'm convinced that
you don't need chaining to solve your problem.
You need debugging.
If you disagree please explain _your_ problem again.
Saying that fb katran is a use case for chaining is, hrm, not correct.


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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-10  4:41           ` Alexei Starovoitov
@ 2019-10-14 12:35             ` Toke Høiland-Jørgensen
  2019-10-14 17:08               ` John Fastabend
  2019-10-16  2:28               ` Alexei Starovoitov
  0 siblings, 2 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-14 12:35 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Wed, Oct 09, 2019 at 10:03:43AM +0200, Toke Høiland-Jørgensen wrote:
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> 
>> > Please implement proper indirect calls and jumps.
>> 
>> I am still not convinced this will actually solve our problem; but OK, I
>> can give it a shot.
>
> If you're not convinced let's talk about it first.
>
> Indirect calls is a building block for debugpoints.
> Let's not call them tracepoints, because Linus banned any discusion
> that includes that name.
> The debugpoints is a way for BPF program to insert points in its
> code to let external facility to do tracing and debugging.
>
> void (*debugpoint1)(struct xdp_buff *, int code);
> void (*debugpoint2)(struct xdp_buff *);
> void (*debugpoint3)(int len);

So how would these work? Similar to global variables (i.e., the loader
creates a single-entry PROG_ARRAY map for each one)? Presumably with
some BTF to validate the argument types?

So what would it take to actually support this? It doesn't quite sound
trivial to add?

> Essentially it's live debugging (tracing) of cooperative bpf programs
> that added debugpoints to their code.

Yup, certainly not disputing that this would be useful for debugging;
although it'll probably be a while before its use becomes widespread
enough that it'll be a reliable tool for people deploying XDP programs...

> Obviously indirect calls can be used for a ton of other things
> including proper chaing of progs, but I'm convinced that
> you don't need chaining to solve your problem.
> You need debugging.

Debugging is certainly also an area that I want to improve. However, I
think that focusing on debugging as the driver for chaining programs was
a mistake on my part; rudimentary debugging (using a tool such as
xdpdump) is something that falls out of program chaining, but it's not
the main driver for it.

> If you disagree please explain _your_ problem again.
> Saying that fb katran is a use case for chaining is, hrm, not correct.

I never said Katran was the driver for this. I just used Katran as one
of the "prior art" examples for my "how are people solving running
multiple programs on the same interface" survey.

What I want to achieve is simply the ability to run multiple independent
XDP programs on the same interface, without having to put any
constraints on the programs themselves. I'm not disputing that this is
*possible* to do completely in userspace, I just don't believe the
resulting solution will be very good. Proper kernel support for indirect
calls (or just "tail calls that return") may change that; but in any
case I think I need to go write some userspace code to have some more
concrete examples to discuss from. So we can come back to the
particulars once I've done that :)

-Toke

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-14 12:35             ` Toke Høiland-Jørgensen
@ 2019-10-14 17:08               ` John Fastabend
  2019-10-14 18:48                 ` Toke Høiland-Jørgensen
  2019-10-16  2:28               ` Alexei Starovoitov
  1 sibling, 1 reply; 29+ messages in thread
From: John Fastabend @ 2019-10-14 17:08 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> 
> > On Wed, Oct 09, 2019 at 10:03:43AM +0200, Toke Høiland-Jørgensen wrote:
> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> >> 
> >> > Please implement proper indirect calls and jumps.
> >> 
> >> I am still not convinced this will actually solve our problem; but OK, I
> >> can give it a shot.
> >
> > If you're not convinced let's talk about it first.
> >
> > Indirect calls is a building block for debugpoints.
> > Let's not call them tracepoints, because Linus banned any discusion
> > that includes that name.
> > The debugpoints is a way for BPF program to insert points in its
> > code to let external facility to do tracing and debugging.
> >
> > void (*debugpoint1)(struct xdp_buff *, int code);
> > void (*debugpoint2)(struct xdp_buff *);
> > void (*debugpoint3)(int len);

I was considering some basic static linking from libbpf side. Something
like,

  bpf_object__link_programs(struct bpf_object *obj1, struct bpf_object *obj2);

This way you could just 'link' in debugpoint{1,2,3} from libbpf before
loading? This would be useful on my side for adding/removing features
and handling different kernel versions. So more generally useful IMO.

We can manage this now but its a bit ugly the above seems nicer to me.
Also not quite as daunting as getting llvm-lld working although that
would also be worth while.

> 
> So how would these work? Similar to global variables (i.e., the loader
> creates a single-entry PROG_ARRAY map for each one)? Presumably with
> some BTF to validate the argument types?
> 
> So what would it take to actually support this? It doesn't quite sound
> trivial to add?
> 
> > Essentially it's live debugging (tracing) of cooperative bpf programs
> > that added debugpoints to their code.
> 
> Yup, certainly not disputing that this would be useful for debugging;
> although it'll probably be a while before its use becomes widespread
> enough that it'll be a reliable tool for people deploying XDP programs...
> 

I guess linking would be a bit different than tracing. Both seem
useful.

> > Obviously indirect calls can be used for a ton of other things
> > including proper chaing of progs, but I'm convinced that
> > you don't need chaining to solve your problem.
> > You need debugging.
> 
> Debugging is certainly also an area that I want to improve. However, I
> think that focusing on debugging as the driver for chaining programs was
> a mistake on my part; rudimentary debugging (using a tool such as
> xdpdump) is something that falls out of program chaining, but it's not
> the main driver for it.
> 
> > If you disagree please explain _your_ problem again.
> > Saying that fb katran is a use case for chaining is, hrm, not correct.
> 
> I never said Katran was the driver for this. I just used Katran as one
> of the "prior art" examples for my "how are people solving running
> multiple programs on the same interface" survey.
> 
> What I want to achieve is simply the ability to run multiple independent
> XDP programs on the same interface, without having to put any
> constraints on the programs themselves. I'm not disputing that this is
> *possible* to do completely in userspace, I just don't believe the
> resulting solution will be very good. Proper kernel support for indirect
> calls (or just "tail calls that return") may change that; but in any
> case I think I need to go write some userspace code to have some more
> concrete examples to discuss from. So we can come back to the
> particulars once I've done that :)

I was imaging that because you have to develop some sort of coordination
by using linking you could enforce call signatures which would allow
you to drop in any XDP program at a call site as long as it matches the
signature.

> 
> -Toke



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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-14 17:08               ` John Fastabend
@ 2019-10-14 18:48                 ` Toke Høiland-Jørgensen
  2019-10-15 16:30                   ` Edward Cree
  0 siblings, 1 reply; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-14 18:48 UTC (permalink / raw)
  To: John Fastabend, Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

John Fastabend <john.fastabend@gmail.com> writes:

> Toke Høiland-Jørgensen wrote:
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> 
>> > On Wed, Oct 09, 2019 at 10:03:43AM +0200, Toke Høiland-Jørgensen wrote:
>> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> >> 
>> >> > Please implement proper indirect calls and jumps.
>> >> 
>> >> I am still not convinced this will actually solve our problem; but OK, I
>> >> can give it a shot.
>> >
>> > If you're not convinced let's talk about it first.
>> >
>> > Indirect calls is a building block for debugpoints.
>> > Let's not call them tracepoints, because Linus banned any discusion
>> > that includes that name.
>> > The debugpoints is a way for BPF program to insert points in its
>> > code to let external facility to do tracing and debugging.
>> >
>> > void (*debugpoint1)(struct xdp_buff *, int code);
>> > void (*debugpoint2)(struct xdp_buff *);
>> > void (*debugpoint3)(int len);
>
> I was considering some basic static linking from libbpf side. Something
> like,
>
>   bpf_object__link_programs(struct bpf_object *obj1, struct bpf_object *obj2);
>
> This way you could just 'link' in debugpoint{1,2,3} from libbpf before
> loading? This would be useful on my side for adding/removing features
> and handling different kernel versions. So more generally useful IMO.

So that will end up with a single monolithic BPF program being loaded
(from the kernel PoV), right? That won't do; we want to be able to go
back to the component programs, and manipulate them as separate kernel
objects.

-Toke

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-14 18:48                 ` Toke Høiland-Jørgensen
@ 2019-10-15 16:30                   ` Edward Cree
  2019-10-15 16:42                     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 29+ messages in thread
From: Edward Cree @ 2019-10-15 16:30 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, John Fastabend, Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

On 14/10/2019 19:48, Toke Høiland-Jørgensen wrote:
> So that will end up with a single monolithic BPF program being loaded
> (from the kernel PoV), right? That won't do; we want to be able to go
> back to the component programs, and manipulate them as separate kernel
> objects.
Why's that?  (Since it also applies to the static-linking PoC I'm
 putting together.)  What do you gain by having the components be
 kernel-visible?
(Bad analogy time: the kernel doesn't care about the .o files you
 linked together to make a userspace executable; any debugging you
 do on that relies on other userspace infrastructure — loading
 symbol tables into a debugger — to interpret things.)

-Ed

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-15 16:30                   ` Edward Cree
@ 2019-10-15 16:42                     ` Toke Høiland-Jørgensen
  2019-10-15 18:33                       ` Edward Cree
  0 siblings, 1 reply; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-15 16:42 UTC (permalink / raw)
  To: Edward Cree, John Fastabend, Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

Edward Cree <ecree@solarflare.com> writes:

> On 14/10/2019 19:48, Toke Høiland-Jørgensen wrote:
>> So that will end up with a single monolithic BPF program being loaded
>> (from the kernel PoV), right? That won't do; we want to be able to go
>> back to the component programs, and manipulate them as separate kernel
>> objects.
> Why's that? (Since it also applies to the static-linking PoC I'm
> putting together.) What do you gain by having the components be
> kernel-visible?

Because then userspace will have to keep state to be able to answer
questions like "show me the list of programs that are currently loaded
(and their call chain)", or do operations like "insert this program into
the call chain at position X".

We already keep all this state in the kernel, so replicating it all in
userspace means both a lot of extra work to implement that
functionality, and having to deal with the inevitable fallout when the
userspace and kernel space state get out of sync...

-Toke

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-15 16:42                     ` Toke Høiland-Jørgensen
@ 2019-10-15 18:33                       ` Edward Cree
  2019-10-17 12:11                         ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 29+ messages in thread
From: Edward Cree @ 2019-10-15 18:33 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, John Fastabend, Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

On 15/10/2019 17:42, Toke Høiland-Jørgensen wrote:
> Edward Cree <ecree@solarflare.com> writes:
>> On 14/10/2019 19:48, Toke Høiland-Jørgensen wrote:
>>> So that will end up with a single monolithic BPF program being loaded
>>> (from the kernel PoV), right? That won't do; we want to be able to go
>>> back to the component programs, and manipulate them as separate kernel
>>> objects.
>> Why's that? (Since it also applies to the static-linking PoC I'm
>> putting together.) What do you gain by having the components be
>> kernel-visible?
> Because then userspace will have to keep state to be able to answer
> questions like "show me the list of programs that are currently loaded
> (and their call chain)", or do operations like "insert this program into
> the call chain at position X".
Userspace keeps state for stuff all the time.  We call them "daemons" ;)
Now you might have arguments for why putting a given piece of state in
 userspace is a bad idea — there's a reason why not everything is a
 microkernel — but those arguments need to be made.

> We already keep all this state in the kernel,
The kernel keeps the state of "current (monolithic) BPF program loaded
 (against each hook)".  Prior to this patch series, the kernel does
 *not* keep any state on what that BPF program was made of (except in
 the sense of BTF debuginfos, which a linker could combine appropriately).

So if we _don't_ add your chained-programs functionality into the kernel,
 and then _do_ implement userspace linking, then there isn't any
 duplicated functionality or even duplicated state — the userland state
 is "what are my components and what's the linker invocation that glues
 them together", the kernel state is "here is one monolithic BPF blob,
 along with a BTF blob to debug it".  The kernel knows nothing of the
 former, and userspace doesn't store (but knows how to recreate) the
 latter.

(That said, proper dynamic linking is better than static linking OR chain
 calls, because it gives us the full flexibility of linking while giving
 you your 'subprogs as kernel objects & kernel state'.  But I think we'll
 need to prototype things with static linking first so that we can be
 sure of the linker semantics we want, before we try to put a new dynamic
 linker in the kernel.)

-Ed

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-14 12:35             ` Toke Høiland-Jørgensen
  2019-10-14 17:08               ` John Fastabend
@ 2019-10-16  2:28               ` Alexei Starovoitov
  2019-10-16  8:27                 ` Jesper Dangaard Brouer
  2019-10-16 13:51                 ` Toke Høiland-Jørgensen
  1 sibling, 2 replies; 29+ messages in thread
From: Alexei Starovoitov @ 2019-10-16  2:28 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

On Mon, Oct 14, 2019 at 02:35:45PM +0200, Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> 
> > On Wed, Oct 09, 2019 at 10:03:43AM +0200, Toke Høiland-Jørgensen wrote:
> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> >> 
> >> > Please implement proper indirect calls and jumps.
> >> 
> >> I am still not convinced this will actually solve our problem; but OK, I
> >> can give it a shot.
> >
> > If you're not convinced let's talk about it first.
> >
> > Indirect calls is a building block for debugpoints.
> > Let's not call them tracepoints, because Linus banned any discusion
> > that includes that name.
> > The debugpoints is a way for BPF program to insert points in its
> > code to let external facility to do tracing and debugging.
> >
> > void (*debugpoint1)(struct xdp_buff *, int code);
> > void (*debugpoint2)(struct xdp_buff *);
> > void (*debugpoint3)(int len);
> 
> So how would these work? Similar to global variables (i.e., the loader
> creates a single-entry PROG_ARRAY map for each one)? Presumably with
> some BTF to validate the argument types?
> 
> So what would it take to actually support this? It doesn't quite sound
> trivial to add?

Depends on definition of 'trivial' :)
The kernel has a luxury of waiting until clean solution is implemented
instead of resorting to hacks.

> > Essentially it's live debugging (tracing) of cooperative bpf programs
> > that added debugpoints to their code.
> 
> Yup, certainly not disputing that this would be useful for debugging;
> although it'll probably be a while before its use becomes widespread
> enough that it'll be a reliable tool for people deploying XDP programs...

same for any new api.

> > Obviously indirect calls can be used for a ton of other things
> > including proper chaing of progs, but I'm convinced that
> > you don't need chaining to solve your problem.
> > You need debugging.
> 
> Debugging is certainly also an area that I want to improve. However, I
> think that focusing on debugging as the driver for chaining programs was
> a mistake on my part; rudimentary debugging (using a tool such as
> xdpdump) is something that falls out of program chaining, but it's not
> the main driver for it.

xdpdump can be done already the way I suggested without adding new kernel
code and it will work on old-ish kernels. Aside from xdp itself
the other requirement is to have get_fd_by_id sys_bpf command.

> > If you disagree please explain _your_ problem again.
> > Saying that fb katran is a use case for chaining is, hrm, not correct.
> 
> I never said Katran was the driver for this. I just used Katran as one
> of the "prior art" examples for my "how are people solving running
> multiple programs on the same interface" survey.

and they solved it. that's the point.

> What I want to achieve is simply the ability to run multiple independent
> XDP programs on the same interface, without having to put any
> constraints on the programs themselves. I'm not disputing that this is
> *possible* to do completely in userspace, I just don't believe the
> resulting solution will be very good.

What makes me uneasy about the whole push for program chaining
is that tc cls_bpf supported multiple independent programs from day one.
Yet it doesn't help to run two firewalls hooked into tc ingress.
Similarly cgroup-bpf had a ton discussions on proper multi-prog api.
Everyone was eventually convinced that it's flexible and generic.
Yet people who started to use it complain that it's missing features
to make it truly usable in production.
Tracing is the only bit where multi-prog works.
Because kernel always runs all programs there.
If we could use PROG_RUN_ARRAY for XDP that could have been a solution.
But we cannot. Return codes matter for XDP.


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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-16  2:28               ` Alexei Starovoitov
@ 2019-10-16  8:27                 ` Jesper Dangaard Brouer
  2019-10-16 10:35                   ` Daniel Borkmann
  2019-10-16 13:51                 ` Toke Høiland-Jørgensen
  1 sibling, 1 reply; 29+ messages in thread
From: Jesper Dangaard Brouer @ 2019-10-16  8:27 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Toke Høiland-Jørgensen, Daniel Borkmann,
	Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire, David Miller,
	netdev, bpf, brouer

On Tue, 15 Oct 2019 19:28:51 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Mon, Oct 14, 2019 at 02:35:45PM +0200, Toke Høiland-Jørgensen wrote:
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> >   
> > > On Wed, Oct 09, 2019 at 10:03:43AM +0200, Toke Høiland-Jørgensen wrote:  
> > >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > >>   
> > >> > Please implement proper indirect calls and jumps.  
> > >> 
> > >> I am still not convinced this will actually solve our problem; but OK, I
> > >> can give it a shot.  
> > >
> > > If you're not convinced let's talk about it first.
> > >
> > > Indirect calls is a building block for debugpoints.
> > > Let's not call them tracepoints, because Linus banned any discusion
> > > that includes that name.
> > > The debugpoints is a way for BPF program to insert points in its
> > > code to let external facility to do tracing and debugging.
> > >
> > > void (*debugpoint1)(struct xdp_buff *, int code);
> > > void (*debugpoint2)(struct xdp_buff *);
> > > void (*debugpoint3)(int len);  
> > 
> > So how would these work? Similar to global variables (i.e., the loader
> > creates a single-entry PROG_ARRAY map for each one)? Presumably with
> > some BTF to validate the argument types?
> > 
> > So what would it take to actually support this? It doesn't quite sound
> > trivial to add?  
> 
> Depends on definition of 'trivial' :)
> The kernel has a luxury of waiting until clean solution is implemented
> instead of resorting to hacks.
> 
> > > Essentially it's live debugging (tracing) of cooperative bpf programs
> > > that added debugpoints to their code.  
> > 
> > Yup, certainly not disputing that this would be useful for debugging;
> > although it'll probably be a while before its use becomes widespread
> > enough that it'll be a reliable tool for people deploying XDP programs...  
> 
> same for any new api.
> 
> > > Obviously indirect calls can be used for a ton of other things
> > > including proper chaing of progs, but I'm convinced that
> > > you don't need chaining to solve your problem.
> > > You need debugging.  
> > 
> > Debugging is certainly also an area that I want to improve. However, I
> > think that focusing on debugging as the driver for chaining programs was
> > a mistake on my part; rudimentary debugging (using a tool such as
> > xdpdump) is something that falls out of program chaining, but it's not
> > the main driver for it.  
> 
> xdpdump can be done already the way I suggested without adding new
> kernel code and it will work on old-ish kernels. Aside from xdp itself
> the other requirement is to have get_fd_by_id sys_bpf command.

You only demonstrated we can hook in xdpdump BEFORE an existing XDP
program without modifying the XDP program.  I'm much more interested in
running xdpdump AFTER an existing XDP program (without modifying it),
and very importantly I want to know the XDP-return codes in my xdpdump. 

That said, with your proposal of "proper indirect calls for BPF", then
the xdpdump AFTER will be easy to implement.

Maybe we should not focus on the xdpdump use-case, because it might be
better to solve by simply adding a tracepoint, that have access to the
xdp_buff.


> > > If you disagree please explain _your_ problem again.
> > > Saying that fb katran is a use case for chaining is, hrm, not correct.  
> > 
> > I never said Katran was the driver for this. I just used Katran as one
> > of the "prior art" examples for my "how are people solving running
> > multiple programs on the same interface" survey.  
> 
> and they solved it. that's the point.
> 
> > What I want to achieve is simply the ability to run multiple independent
> > XDP programs on the same interface, without having to put any
> > constraints on the programs themselves. I'm not disputing that this is
> > *possible* to do completely in userspace, I just don't believe the
> > resulting solution will be very good.  
> 
> What makes me uneasy about the whole push for program chaining
> is that tc cls_bpf supported multiple independent programs from day one.
> Yet it doesn't help to run two firewalls hooked into tc ingress.

I do understand your concerns.

Let me explain why I believe TC cls_bpf multiple independent programs
have not seen much usage.

First of all the TC-tool is notorious difficult to use and configure (I
admit, I struggle with this myself every single time). (The TC layer
have some amazing features, like hash based lookup, that never get used
due to this).

Second, the multiple "independent programs", are actually not
independent, because the current running program must return
TC_ACT_UNSPEC to allow next bpf-prog to run.  Thus, it is not really
usable.


> Similarly cgroup-bpf had a ton discussions on proper multi-prog api.
> Everyone was eventually convinced that it's flexible and generic.
> Yet people who started to use it complain that it's missing features
> to make it truly usable in production.

I've not looked at the cgroup-bpf multi-prog API, I guess we should to
understand why this failed.

> Tracing is the only bit where multi-prog works.
> Because kernel always runs all programs there.

This is important insight ("kernel always runs all programs").  A key
part of Toke's design with chain-calling, is that the kernel always
runs all the XDP/BPF-progs in the chain. Regardless of the XDP return
value.  The next program in the chain, need info about previous
BPF-prog return value, but it can choose to override this.

> If we could use PROG_RUN_ARRAY for XDP that could have been a solution.
> But we cannot. Return codes matter for XDP.

The proposal from Toke, is to allow next-chain BPF-program can override
the prev BPF-prog return value.  This part of the design, which I must
admit is also the only option due to tail-calls.  But I do think it
makes sense, because even if XDP_DROP is returned, then I can install
another XDP-prog that does XDP_REDIRECT out another interface to an
analyzer box, or into an AF_XDP based dump tool.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-16  8:27                 ` Jesper Dangaard Brouer
@ 2019-10-16 10:35                   ` Daniel Borkmann
  2019-10-16 11:16                     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 29+ messages in thread
From: Daniel Borkmann @ 2019-10-16 10:35 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Alexei Starovoitov, Toke Høiland-Jørgensen,
	Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	Marek Majkowski, Lorenz Bauer, Alan Maguire, David Miller,
	netdev, bpf

On Wed, Oct 16, 2019 at 10:27:12AM +0200, Jesper Dangaard Brouer wrote:
> On Tue, 15 Oct 2019 19:28:51 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > On Mon, Oct 14, 2019 at 02:35:45PM +0200, Toke Høiland-Jørgensen wrote:
> > > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > > > On Wed, Oct 09, 2019 at 10:03:43AM +0200, Toke Høiland-Jørgensen wrote:  
> > > >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
[...]
> > > > If you disagree please explain _your_ problem again.
> > > > Saying that fb katran is a use case for chaining is, hrm, not correct.  
> > > 
> > > I never said Katran was the driver for this. I just used Katran as one
> > > of the "prior art" examples for my "how are people solving running
> > > multiple programs on the same interface" survey.  
> > 
> > and they solved it. that's the point.
> > 
> > > What I want to achieve is simply the ability to run multiple independent
> > > XDP programs on the same interface, without having to put any
> > > constraints on the programs themselves. I'm not disputing that this is
> > > *possible* to do completely in userspace, I just don't believe the
> > > resulting solution will be very good.  
> > 
> > What makes me uneasy about the whole push for program chaining
> > is that tc cls_bpf supported multiple independent programs from day one.
> > Yet it doesn't help to run two firewalls hooked into tc ingress.
> 
> I do understand your concerns.
> 
> Let me explain why I believe TC cls_bpf multiple independent programs
> have not seen much usage.
> 
> First of all the TC-tool is notorious difficult to use and configure (I
> admit, I struggle with this myself every single time). (The TC layer
> have some amazing features, like hash based lookup, that never get used
> due to this).

We do use cls_bpf heavily in Cilium, but I don't necessarily agree on
the notorious difficult to use aspect (at least for tc + BPF): i) this
is abstracted away from the /user/ entirely to the point that this is an
implementation detail he doesn't need to know about, ii) these days most
access to these hooks is done programmatically, if this is a worry, then
lets simply add a cls_bpf pendant for APIs like bpf_set_link_xdp_fd() we
have in libbpf where you only pass in ifindex, direction (ingress/egress)
and priority of the program so that underneath it sets up cls_act qdisc
with a cls_bpf instance that makes the whole thing foolproof, e.g.:

  int bpf_set_link_tc_fd(int ifindex, int fd, enum bpf_tc_dir dir,
                         __u32 priority, __u32 flags);

The flags could be similar to XDP: 0 or xxx_FLAGS_UPDATE_IF_NOEXIST and
xxx_FLAGS_HW_MODE. The problem that might be easy to miss via tc cmdline
tool is that when you don't specify explicit prio/handle upon tc replace,
then it auto-allocates one and keeps adding new programs instead of
replacing the old ones, but this quirk can be solved via API like above.

> Second, the multiple "independent programs", are actually not
> independent, because the current running program must return
> TC_ACT_UNSPEC to allow next bpf-prog to run.  Thus, it is not really
> usable.

I'd argue that unless the only thing you do in your debugging program is
to introspect (read-only) the packet at the current point, you'd run into
a similar coordination issue, meaning, the "independent programs" works
for simple cases where you only have ACCEPT and DROP policy, such that
you could run through all the programs and have precedence on DROP.

But once you have conflicting policies with regards to how these programs
mangle and redirect packets, how would you handle these? I'd argue it's
a non-trivial task to outsource if /admins/ are supposed to do manual
order adjustments and more importantly to troubleshoot issues due to
them. Potentially debugging hooks would make that easier to avoid
recompilation, but it's more of a developer task.

Often times orchestration tools i) assume they just own the data path
to reduce complexity in an already complex system and ii) also keep
'refreshing' their setup. One random example for the latter is k8s'
kube-proxy that reinstalls its iptables rules every x sec, in order to
make sure there was no manual messing around and to keep the data path
eventually consistent with the daemon view (if they got borked). How
would you make the loader aware of daemons automatically refreshing/
reconfiguring their BPF progs in the situation where admins changed
the pipeline, adding similar handle as tc so whoever does the 'chain'
assembly know which one to update?

> > Similarly cgroup-bpf had a ton discussions on proper multi-prog api.
> > Everyone was eventually convinced that it's flexible and generic.
> > Yet people who started to use it complain that it's missing features
> > to make it truly usable in production.
> 
> I've not looked at the cgroup-bpf multi-prog API, I guess we should to
> understand why this failed.
> 
> > Tracing is the only bit where multi-prog works.
> > Because kernel always runs all programs there.
> 
> This is important insight ("kernel always runs all programs").  A key
> part of Toke's design with chain-calling, is that the kernel always
> runs all the XDP/BPF-progs in the chain. Regardless of the XDP return
> value.  The next program in the chain, need info about previous
> BPF-prog return value, but it can choose to override this.
> 
> > If we could use PROG_RUN_ARRAY for XDP that could have been a solution.
> > But we cannot. Return codes matter for XDP.
> 
> The proposal from Toke, is to allow next-chain BPF-program can override
> the prev BPF-prog return value.  This part of the design, which I must
> admit is also the only option due to tail-calls.  But I do think it
> makes sense, because even if XDP_DROP is returned, then I can install
> another XDP-prog that does XDP_REDIRECT out another interface to an
> analyzer box, or into an AF_XDP based dump tool.

Thanks,
Daniel

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-16 10:35                   ` Daniel Borkmann
@ 2019-10-16 11:16                     ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-16 11:16 UTC (permalink / raw)
  To: Daniel Borkmann, Jesper Dangaard Brouer
  Cc: Alexei Starovoitov, Alexei Starovoitov, Martin KaFai Lau,
	Song Liu, Yonghong Song, Marek Majkowski, Lorenz Bauer,
	Alan Maguire, David Miller, netdev, bpf

Daniel Borkmann <daniel@iogearbox.net> writes:
> We do use cls_bpf heavily in Cilium, but I don't necessarily agree on
> the notorious difficult to use aspect (at least for tc + BPF): i) this
> is abstracted away from the /user/ entirely to the point that this is an
> implementation detail he doesn't need to know about, ii) these days most
> access to these hooks is done programmatically, if this is a worry, then
> lets simply add a cls_bpf pendant for APIs like bpf_set_link_xdp_fd() we
> have in libbpf where you only pass in ifindex, direction (ingress/egress)
> and priority of the program so that underneath it sets up cls_act qdisc
> with a cls_bpf instance that makes the whole thing foolproof, e.g.:
>
>   int bpf_set_link_tc_fd(int ifindex, int fd, enum bpf_tc_dir dir,
>                          __u32 priority, __u32 flags);

Basically, what I'm trying to achieve with XDP chain calls is to be able
to do something similar to this, but for XDP programs. Just with the
added ability to also select on return code...

>> Second, the multiple "independent programs", are actually not
>> independent, because the current running program must return
>> TC_ACT_UNSPEC to allow next bpf-prog to run.  Thus, it is not really
>> usable.
>
> I'd argue that unless the only thing you do in your debugging program is
> to introspect (read-only) the packet at the current point, you'd run into
> a similar coordination issue, meaning, the "independent programs" works
> for simple cases where you only have ACCEPT and DROP policy, such that
> you could run through all the programs and have precedence on DROP.
>
> But once you have conflicting policies with regards to how these programs
> mangle and redirect packets, how would you handle these?

I imagine that in most relevant cases this can be handled by convention;
the most obvious convention being "chain call on XDP_PASS". But still
allowing the admin to override this if they know what they are doing.

> I'd argue it's a non-trivial task to outsource if /admins/ are
> supposed to do manual order adjustments and more importantly to
> troubleshoot issues due to them. Potentially debugging hooks would
> make that easier to avoid recompilation, but it's more of a developer
> task.

Sure, in the general case this could become arbitrarily complex; but I
think that the feature can still be useful.

> Often times orchestration tools i) assume they just own the data path
> to reduce complexity in an already complex system and ii) also keep
> 'refreshing' their setup. One random example for the latter is k8s'
> kube-proxy that reinstalls its iptables rules every x sec, in order to
> make sure there was no manual messing around and to keep the data path
> eventually consistent with the daemon view (if they got borked).

This is actually the reason I want the kernel state to be the source of
truth (instead of keeping state in a userspace daemon). If the kernel
keeps the state it can enforce consistency, whereas a userspace daemon
has to be able to deal with things in the kernel changing underneath
it...

> How would you make the loader aware of daemons automatically
> refreshing/ reconfiguring their BPF progs in the situation where
> admins changed the pipeline, adding similar handle as tc so whoever
> does the 'chain' assembly know which one to update?

My idea for this was to implement atomic updates in the form of a
"cmpxchg" type of operation. I.e., we'd add a parameter to the syscall
where userspace could say "I want to install this program in place of
this one", and if that "old program" value doesn't match the current
state of the kernel, it can be rejected, atomically.

-Toke

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-16  2:28               ` Alexei Starovoitov
  2019-10-16  8:27                 ` Jesper Dangaard Brouer
@ 2019-10-16 13:51                 ` Toke Høiland-Jørgensen
  1 sibling, 0 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-16 13:51 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Mon, Oct 14, 2019 at 02:35:45PM +0200, Toke Høiland-Jørgensen wrote:
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> 
>> > On Wed, Oct 09, 2019 at 10:03:43AM +0200, Toke Høiland-Jørgensen wrote:
>> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> >> 
>> >> > Please implement proper indirect calls and jumps.
>> >> 
>> >> I am still not convinced this will actually solve our problem; but OK, I
>> >> can give it a shot.
>> >
>> > If you're not convinced let's talk about it first.
>> >
>> > Indirect calls is a building block for debugpoints.
>> > Let's not call them tracepoints, because Linus banned any discusion
>> > that includes that name.
>> > The debugpoints is a way for BPF program to insert points in its
>> > code to let external facility to do tracing and debugging.
>> >
>> > void (*debugpoint1)(struct xdp_buff *, int code);
>> > void (*debugpoint2)(struct xdp_buff *);
>> > void (*debugpoint3)(int len);
>> 
>> So how would these work? Similar to global variables (i.e., the loader
>> creates a single-entry PROG_ARRAY map for each one)? Presumably with
>> some BTF to validate the argument types?
>> 
>> So what would it take to actually support this? It doesn't quite sound
>> trivial to add?
>
> Depends on definition of 'trivial' :)

Well, I don't know... :)

> The kernel has a luxury of waiting until clean solution is implemented
> instead of resorting to hacks.

It would be helpful if you could give an opinion on what specific
features are missing in the kernel to support these indirect calls. A
few high-level sentences is fine (e.g., "the verifier needs to be able
to do X, and llvm/libbpf needs to have support for Y")... I'm trying to
gauge whether this is something it would even make sense for me to poke
into, or if I'm better off waiting for someone who actually knows what
they are doing to work on this :)

>> > If you disagree please explain _your_ problem again.
>> > Saying that fb katran is a use case for chaining is, hrm, not correct.
>> 
>> I never said Katran was the driver for this. I just used Katran as one
>> of the "prior art" examples for my "how are people solving running
>> multiple programs on the same interface" survey.
>
> and they solved it. that's the point.

Yes, in a way that's specific to Katran. This whole thing actually
started out as an effort to make something that's (a) generically useful
so everyone doesn't have to re-invent their own way of doing it, and (b)
interoperable in the case where there is no direct coordination between
the program authors.

The ability to chain a program per return code came out of (a), and the
need to not have to modify all programs involved came out of (b).

>> What I want to achieve is simply the ability to run multiple independent
>> XDP programs on the same interface, without having to put any
>> constraints on the programs themselves. I'm not disputing that this is
>> *possible* to do completely in userspace, I just don't believe the
>> resulting solution will be very good.
>
> What makes me uneasy about the whole push for program chaining
> is that tc cls_bpf supported multiple independent programs from day one.
> Yet it doesn't help to run two firewalls hooked into tc ingress.
> Similarly cgroup-bpf had a ton discussions on proper multi-prog api.
> Everyone was eventually convinced that it's flexible and generic.
> Yet people who started to use it complain that it's missing features
> to make it truly usable in production.

I'll go look at the cgroup-bpf API as well... Do you have any references
to those complaints?

-Toke

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

* Re: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other
  2019-10-15 18:33                       ` Edward Cree
@ 2019-10-17 12:11                         ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-17 12:11 UTC (permalink / raw)
  To: Edward Cree, John Fastabend, Alexei Starovoitov
  Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Marek Majkowski, Lorenz Bauer, Alan Maguire,
	Jesper Dangaard Brouer, David Miller, netdev, bpf

Edward Cree <ecree@solarflare.com> writes:

> On 15/10/2019 17:42, Toke Høiland-Jørgensen wrote:
>> Edward Cree <ecree@solarflare.com> writes:
>>> On 14/10/2019 19:48, Toke Høiland-Jørgensen wrote:
>>>> So that will end up with a single monolithic BPF program being loaded
>>>> (from the kernel PoV), right? That won't do; we want to be able to go
>>>> back to the component programs, and manipulate them as separate kernel
>>>> objects.
>>> Why's that? (Since it also applies to the static-linking PoC I'm
>>> putting together.) What do you gain by having the components be
>>> kernel-visible?
>> Because then userspace will have to keep state to be able to answer
>> questions like "show me the list of programs that are currently loaded
>> (and their call chain)", or do operations like "insert this program into
>> the call chain at position X".
> Userspace keeps state for stuff all the time.  We call them "daemons" ;)
> Now you might have arguments for why putting a given piece of state in
>  userspace is a bad idea — there's a reason why not everything is a
>  microkernel — but those arguments need to be made.
>
>> We already keep all this state in the kernel,
> The kernel keeps the state of "current (monolithic) BPF program loaded
>  (against each hook)".  Prior to this patch series, the kernel does
>  *not* keep any state on what that BPF program was made of (except in
>  the sense of BTF debuginfos, which a linker could combine appropriately).
>
> So if we _don't_ add your chained-programs functionality into the kernel,
>  and then _do_ implement userspace linking, then there isn't any
>  duplicated functionality or even duplicated state — the userland state
>  is "what are my components and what's the linker invocation that glues
>  them together", the kernel state is "here is one monolithic BPF blob,
>  along with a BTF blob to debug it".  The kernel knows nothing of the
>  former, and userspace doesn't store (but knows how to recreate) the
>  latter.

I think there's a conceptual disconnect here in how we view what an XDP
program is. In my mind, an XDP program is a stand-alone entity tied to a
particular application; not a library function that can just be inserted
into another program. Thus, what you're proposing sounds to me like the
equivalent of saying "we don't want to do process management in the
kernel; the init process should just link in all the programs userspace
wants to run". Which is technically possible; but that doesn't make it a
good idea.

Setting aside that for a moment; the reason I don't think this belongs
in userspace is that putting it there would carry a complexity cost that
is higher than having it in the kernel. Specifically, if we do implement
an 'xdpd' daemon to handle all this, that would mean that we:

- Introduce a new, separate code base that we'll have to write, support
  and manage updates to.

- Add a new dependency to using XDP (now you not only need the kernel
  and libraries, you'll also need the daemon).

- Have to duplicate or wrap functionality currently found in the kernel;
  at least:
  
    - Keeping track of which XDP programs are loaded and attached to
      each interface (as well as the "new state" of their attachment
      order).

    - Some kind of interface with the verifier; if an app does
      xdpd_rpc_load(prog), how is the verifier result going to get back
      to the caller?

- Have to deal with state synchronisation issues (how does xdpd handle
  kernel state changing from underneath it?).


While these are issues that are (probably) all solvable, I think the
cost of solving them is far higher than putting the support into the
kernel. Which is why I think kernel support is the best solution :)

-Toke

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

end of thread, back to index

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-07 17:20 [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Toke Høiland-Jørgensen
2019-10-07 20:42   ` Alexei Starovoitov
2019-10-08  8:07     ` Toke Høiland-Jørgensen
2019-10-09  1:51       ` Alexei Starovoitov
2019-10-09  8:03         ` Toke Høiland-Jørgensen
2019-10-10  4:41           ` Alexei Starovoitov
2019-10-14 12:35             ` Toke Høiland-Jørgensen
2019-10-14 17:08               ` John Fastabend
2019-10-14 18:48                 ` Toke Høiland-Jørgensen
2019-10-15 16:30                   ` Edward Cree
2019-10-15 16:42                     ` Toke Høiland-Jørgensen
2019-10-15 18:33                       ` Edward Cree
2019-10-17 12:11                         ` Toke Høiland-Jørgensen
2019-10-16  2:28               ` Alexei Starovoitov
2019-10-16  8:27                 ` Jesper Dangaard Brouer
2019-10-16 10:35                   ` Daniel Borkmann
2019-10-16 11:16                     ` Toke Høiland-Jørgensen
2019-10-16 13:51                 ` Toke Høiland-Jørgensen
2019-10-09 10:19         ` Jesper Dangaard Brouer
2019-10-09 17:57           ` Alexei Starovoitov
2019-10-07 17:20 ` [PATCH bpf-next v3 2/5] bpf: Add support for setting chain call sequence for programs Toke Høiland-Jørgensen
2019-10-07 20:38   ` Daniel Borkmann
2019-10-08  8:09     ` Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 3/5] tools: Update bpf.h header for program chain calls Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 4/5] libbpf: Add syscall wrappers for BPF_PROG_CHAIN_* commands Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 5/5] selftests: Add tests for XDP chain calls Toke Høiland-Jørgensen
2019-10-07 18:58 ` [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through " John Fastabend
2019-10-08  8:42   ` Toke Høiland-Jørgensen

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org bpf@archiver.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/ public-inbox