Netdev 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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
  0 siblings, 0 replies; 24+ 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] 24+ 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
  1 sibling, 0 replies; 24+ 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] 24+ messages in thread

end of thread, back to index

Thread overview: 24+ 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-16  2:28               ` Alexei Starovoitov
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

Netdev Archive on lore.kernel.org

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

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

Example config snippet for mirrors

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


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