[PATCH bpf-next v2 0/5] xdp: Support multiple programs on a single interface through chain calls

[PATCH bpf-next v2 0/5] xdp: Support multiple programs on a single interface through chain calls

[Toke Høiland-Jørgensen]

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 the response to the previous iteration was pretty unanimous that this
should not be XDP-specific, this version takes a different approach: We add the
ability to inject chain call programs into the eBPF execution core itself. This
also turns out to be simpler, so that's good :)

The way this new approach works is 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 having the verifier inject instructions
into the program that performs the chain call lookup and tail call before each
BPF_EXIT instruction. Since this rewriting has to be performed at program load
time, a new flag has to be set to trigger the rewriting. Only programs loaded
with this flag set can have other programs attached to them for chain calls.

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.
However, rewriting the program reallocates the bpf_prog struct, which is
obviously not possible after the program has been attached to something.

One way around this could be a sysctl to force the flag one (for enforcing
system-wide support). Another could be to have the chain call support
itself built into the interpreter and JIT, which could conceivably be
re-run each time we attach a new chain call program. This would also allow
the JIT to inject direct calls to the next program instead of using the
tail call infrastructure, which presumably would be a performance win. The
drawback is, of course, that it would require modifying all the JITs.


# 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. I did not re-run the baseline tests from
before, so the two top values are the same

| Test case                        | Perf      | Overhead |
|----------------------------------+-----------+----------|
| Before patch (XDP DROP program)  | 31.0 Mpps |          |
| XDP tail call                    | 26.6 Mpps | 5.3 ns   |
|----------------------------------+-----------+----------|
| After patch (XDP DROP program)   | 31.2 Mpps |          |
| XDP chain call (wildcard return) | 26.8 Mpps | 5.3 ns   |
| XDP chain call (XDP_PASS return) | 27.2 Mpps | 4.7 ns   |

The difference between the wildcard and XDP_PASS cases for chain calls, is that
using the wildcard mode needs two tail call attempts where the first one fails,
(see the injected BPF code in patch 1) while XDP_PASS matches on the first tail
call.

# PATCH SET STRUCTURE
This series is structured as follows:

- Patch 1: Adds the code that injects the instructions into the programs
- 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-02

Changelog:

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.

---

Alan Maguire (1):
      bpf: Add support for setting chain call sequence for programs

Toke Høiland-Jørgensen (4):
      bpf: Support injecting chain calls into BPF programs on load
      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                           |    2 
 include/uapi/linux/bpf.h                      |   16 +
 kernel/bpf/core.c                             |   10 +
 kernel/bpf/syscall.c                          |   81 ++++++
 kernel/bpf/verifier.c                         |   76 ++++++
 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, 640 insertions(+), 2 deletions(-)
 create mode 100755 tools/testing/selftests/bpf/test_xdp_chain.sh
 create mode 100644 tools/testing/selftests/bpf/xdp_chain.c

[PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Toke Høiland-Jørgensen]

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

This adds support for injecting chain call logic into eBPF programs before
they return. The code injection is controlled by a flag at program load
time; if the flag is set, the verifier will add code to every BPF_EXIT
instruction that first does a lookup into a chain call structure to see if
it should call into another program before returning. The actual calls
reuse the tail call infrastructure.

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.
However, rewriting the program reallocates the bpf_prog struct, which is
obviously not possible after the program has been attached to something.

One way around this could be a sysctl to force the flag one (for enforcing
system-wide support). Another could be to have the chain call support
itself built into the interpreter and JIT, which could conceivably be
re-run each time we attach a new chain call program. This would also allow
the JIT to inject direct calls to the next program instead of using the
tail call infrastructure, which presumably would be a performance win. The
drawback is, of course, that it would require modifying all the JITs.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 include/linux/bpf.h      |    2 +
 include/uapi/linux/bpf.h |    6 ++++
 kernel/bpf/core.c        |   10 ++++++
 kernel/bpf/syscall.c     |    3 +-
 kernel/bpf/verifier.c    |   76 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5b9d22338606..753abfb78c13 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -383,6 +383,7 @@ struct bpf_prog_aux {
 	struct list_head ksym_lnode;
 	const struct bpf_prog_ops *ops;
 	struct bpf_map **used_maps;
+	struct bpf_array *chain_progs;
 	struct bpf_prog *prog;
 	struct user_struct *user;
 	u64 load_time; /* ns since boottime */
@@ -443,6 +444,7 @@ struct bpf_array {
 
 #define BPF_COMPLEXITY_LIMIT_INSNS      1000000 /* yes. 1M insns */
 #define MAX_TAIL_CALL_CNT 32
+#define BPF_NUM_CHAIN_SLOTS 8
 
 #define BPF_F_ACCESS_MASK	(BPF_F_RDONLY |		\
 				 BPF_F_RDONLY_PROG |	\
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 77c6be96d676..febe8934d19a 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 injection at program return. If set, the
+ * verifier will rewrite program returns to check for and jump to chain call
+ * programs configured with the BPF_PROG_CHAIN_* commands to the bpf syscall.
+ */
+#define BPF_F_INJECT_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..98f1ad920e48 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -255,6 +255,16 @@ void __bpf_prog_free(struct bpf_prog *fp)
 {
 	if (fp->aux) {
 		free_percpu(fp->aux->stats);
+		if (fp->aux->chain_progs) {
+			struct bpf_array *array = fp->aux->chain_progs;
+			int i;
+
+			for (i = 0; i < BPF_NUM_CHAIN_SLOTS; i++)
+				if (array->ptrs[i])
+					bpf_prog_put(array->ptrs[i]);
+
+			bpf_map_area_free(array);
+		}
 		kfree(fp->aux);
 	}
 	vfree(fp);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 82eabd4e38ad..c2a49df5f921 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_INJECT_CHAIN_CALLS))
 		return -EINVAL;
 
 	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ffc3e53f5300..dbc9bbf13300 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9154,6 +9154,79 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
 	return 0;
 }
 
+static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
+{
+	struct bpf_prog *prog = env->prog;
+	struct bpf_insn *insn = prog->insnsi;
+	int i, cnt, delta = 0, ret = -ENOMEM;
+	const int insn_cnt = prog->len;
+	struct bpf_array *prog_array;
+	struct bpf_prog *new_prog;
+	size_t array_size;
+
+	struct bpf_insn call_next[] = {
+		BPF_LD_IMM64(BPF_REG_2, 0),
+		/* Save real return value for later */
+		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
+		/* First try tail call with index ret+1 */
+		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
+		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
+		/* If that doesn't work, try with index 0 (wildcard) */
+		BPF_MOV64_IMM(BPF_REG_3, 0),
+		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
+		/* Restore saved return value and exit */
+		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
+		BPF_EXIT_INSN()
+	};
+
+	if (prog->aux->chain_progs)
+		return 0;
+
+	array_size = sizeof(*prog_array) + BPF_NUM_CHAIN_SLOTS * sizeof(void*);
+	prog_array = bpf_map_area_alloc(array_size, NUMA_NO_NODE);
+
+	if (!prog_array)
+		goto out_err;
+
+	prog_array->elem_size = sizeof(void*);
+	prog_array->map.max_entries = BPF_NUM_CHAIN_SLOTS;
+
+	call_next[0].imm = (u32)((u64) prog_array);
+	call_next[1].imm = ((u64) prog_array) >> 32;
+
+	for (i = 0; i < insn_cnt; i++, insn++) {
+		if (insn->code != (BPF_JMP | BPF_EXIT))
+			continue;
+
+		cnt = ARRAY_SIZE(call_next);
+
+		new_prog = bpf_patch_insn_data(env, i+delta, call_next, cnt);
+		if (!new_prog) {
+			goto out_err;
+		}
+
+		delta    += cnt - 1;
+		env->prog = prog = new_prog;
+		insn      = new_prog->insnsi + i + delta;
+	}
+
+	/* If we chain call into other programs, we cannot make any assumptions
+	 * since they can be replaced dynamically during runtime.
+	 */
+	prog->cb_access = 1;
+	env->prog->aux->stack_depth = MAX_BPF_STACK;
+	env->prog->aux->max_pkt_offset = MAX_PACKET_OFF;
+
+	prog->aux->chain_progs = prog_array;
+	return 0;
+
+out_err:
+	bpf_map_area_free(prog_array);
+	return ret;
+}
+
+
 static void free_states(struct bpf_verifier_env *env)
 {
 	struct bpf_verifier_state_list *sl, *sln;
@@ -9336,6 +9409,9 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
 	if (ret == 0)
 		ret = fixup_bpf_calls(env);
 
+	if (ret == 0 && (attr->prog_flags & BPF_F_INJECT_CHAIN_CALLS))
+		ret = bpf_inject_chain_calls(env);
+
 	/* do 32-bit optimization after insn patching has done so those patched
 	 * insns could be handled correctly.
 	 */

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1665 bytes --]

Hi "Toke,

I love your patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/bpf-Support-injecting-chain-calls-into-BPF-programs-on-load/20191005-035650
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: i386-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

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

All errors (new ones prefixed by >>):

   kernel/bpf/core.c: In function '__bpf_prog_free':
>> kernel/bpf/core.c:266:4: error: implicit declaration of function 'bpf_map_area_free'; did you mean 'bpf_prog_free'? [-Werror=implicit-function-declaration]
       bpf_map_area_free(array);
       ^~~~~~~~~~~~~~~~~
       bpf_prog_free
   cc1: some warnings being treated as errors

vim +266 kernel/bpf/core.c

   253	
   254	void __bpf_prog_free(struct bpf_prog *fp)
   255	{
   256		if (fp->aux) {
   257			free_percpu(fp->aux->stats);
   258			if (fp->aux->chain_progs) {
   259				struct bpf_array *array = fp->aux->chain_progs;
   260				int i;
   261	
   262				for (i = 0; i < BPF_NUM_CHAIN_SLOTS; i++)
   263					if (array->ptrs[i])
   264						bpf_prog_put(array->ptrs[i]);
   265	
 > 266				bpf_map_area_free(array);
   267			}
   268			kfree(fp->aux);
   269		}
   270		vfree(fp);
   271	}
   272	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28158 bytes --]

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3932 bytes --]

Hi "Toke,

I love your patch! Perhaps something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/bpf-Support-injecting-chain-calls-into-BPF-programs-on-load/20191005-035650
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=xtensa 

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

All warnings (new ones prefixed by >>):

   kernel/bpf/verifier.c: In function 'bpf_inject_chain_calls':
>> kernel/bpf/verifier.c:9195:27: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     call_next[0].imm = (u32)((u64) prog_array);
                              ^
   kernel/bpf/verifier.c:9196:22: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     call_next[1].imm = ((u64) prog_array) >> 32;
                         ^

vim +9195 kernel/bpf/verifier.c

  9156	
  9157	static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
  9158	{
  9159		struct bpf_prog *prog = env->prog;
  9160		struct bpf_insn *insn = prog->insnsi;
  9161		int i, cnt, delta = 0, ret = -ENOMEM;
  9162		const int insn_cnt = prog->len;
  9163		struct bpf_array *prog_array;
  9164		struct bpf_prog *new_prog;
  9165		size_t array_size;
  9166	
  9167		struct bpf_insn call_next[] = {
  9168			BPF_LD_IMM64(BPF_REG_2, 0),
  9169			/* Save real return value for later */
  9170			BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
  9171			/* First try tail call with index ret+1 */
  9172			BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
  9173			BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
  9174			BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
  9175			/* If that doesn't work, try with index 0 (wildcard) */
  9176			BPF_MOV64_IMM(BPF_REG_3, 0),
  9177			BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
  9178			/* Restore saved return value and exit */
  9179			BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
  9180			BPF_EXIT_INSN()
  9181		};
  9182	
  9183		if (prog->aux->chain_progs)
  9184			return 0;
  9185	
  9186		array_size = sizeof(*prog_array) + BPF_NUM_CHAIN_SLOTS * sizeof(void*);
  9187		prog_array = bpf_map_area_alloc(array_size, NUMA_NO_NODE);
  9188	
  9189		if (!prog_array)
  9190			goto out_err;
  9191	
  9192		prog_array->elem_size = sizeof(void*);
  9193		prog_array->map.max_entries = BPF_NUM_CHAIN_SLOTS;
  9194	
> 9195		call_next[0].imm = (u32)((u64) prog_array);
  9196		call_next[1].imm = ((u64) prog_array) >> 32;
  9197	
  9198		for (i = 0; i < insn_cnt; i++, insn++) {
  9199			if (insn->code != (BPF_JMP | BPF_EXIT))
  9200				continue;
  9201	
  9202			cnt = ARRAY_SIZE(call_next);
  9203	
  9204			new_prog = bpf_patch_insn_data(env, i+delta, call_next, cnt);
  9205			if (!new_prog) {
  9206				goto out_err;
  9207			}
  9208	
  9209			delta    += cnt - 1;
  9210			env->prog = prog = new_prog;
  9211			insn      = new_prog->insnsi + i + delta;
  9212		}
  9213	
  9214		/* If we chain call into other programs, we cannot make any assumptions
  9215		 * since they can be replaced dynamically during runtime.
  9216		 */
  9217		prog->cb_access = 1;
  9218		env->prog->aux->stack_depth = MAX_BPF_STACK;
  9219		env->prog->aux->max_pkt_offset = MAX_PACKET_OFF;
  9220	
  9221		prog->aux->chain_progs = prog_array;
  9222		return 0;
  9223	
  9224	out_err:
  9225		bpf_map_area_free(prog_array);
  9226		return ret;
  9227	}
  9228	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59593 bytes --]

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Jakub Kicinski]

On Fri, 04 Oct 2019 19:22:41 +0200, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
> 
> This adds support for injecting chain call logic into eBPF programs before
> they return. The code injection is controlled by a flag at program load
> time; if the flag is set, the verifier will add code to every BPF_EXIT
> instruction that first does a lookup into a chain call structure to see if
> it should call into another program before returning. The actual calls
> reuse the tail call infrastructure.
> 
> 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.
> However, rewriting the program reallocates the bpf_prog struct, which is
> obviously not possible after the program has been attached to something.

Very exciting stuff :)

Forgive my ignorance, isn't synchronize_rcu() enough to ensure old
image is no longer used?

For simple rewrites which don't require much context like the one here
it'd be cool to do it after loading..

> One way around this could be a sysctl to force the flag one (for enforcing
> system-wide support). Another could be to have the chain call support
> itself built into the interpreter and JIT, which could conceivably be
> re-run each time we attach a new chain call program. This would also allow
> the JIT to inject direct calls to the next program instead of using the
> tail call infrastructure, which presumably would be a performance win. The
> drawback is, of course, that it would require modifying all the JITs.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 5b9d22338606..753abfb78c13 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -383,6 +383,7 @@ struct bpf_prog_aux {
>  	struct list_head ksym_lnode;
>  	const struct bpf_prog_ops *ops;
>  	struct bpf_map **used_maps;
> +	struct bpf_array *chain_progs;
>  	struct bpf_prog *prog;
>  	struct user_struct *user;
>  	u64 load_time; /* ns since boottime */
> @@ -443,6 +444,7 @@ struct bpf_array {
>  
>  #define BPF_COMPLEXITY_LIMIT_INSNS      1000000 /* yes. 1M insns */
>  #define MAX_TAIL_CALL_CNT 32
> +#define BPF_NUM_CHAIN_SLOTS 8

This could be user arg? Also the behaviour of mapping could be user
controlled? Perhaps even users could pass the snippet to map the return
code to the location, one day?

>  #define BPF_F_ACCESS_MASK	(BPF_F_RDONLY |		\
>  				 BPF_F_RDONLY_PROG |	\
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 77c6be96d676..febe8934d19a 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 injection at program return. If set, the
> + * verifier will rewrite program returns to check for and jump to chain call
> + * programs configured with the BPF_PROG_CHAIN_* commands to the bpf syscall.
> + */
> +#define BPF_F_INJECT_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..98f1ad920e48 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -255,6 +255,16 @@ void __bpf_prog_free(struct bpf_prog *fp)
>  {
>  	if (fp->aux) {
>  		free_percpu(fp->aux->stats);
> +		if (fp->aux->chain_progs) {
> +			struct bpf_array *array = fp->aux->chain_progs;
> +			int i;
> +
> +			for (i = 0; i < BPF_NUM_CHAIN_SLOTS; i++)
> +				if (array->ptrs[i])
> +					bpf_prog_put(array->ptrs[i]);
> +
> +			bpf_map_area_free(array);
> +		}
>  		kfree(fp->aux);
>  	}
>  	vfree(fp);
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 82eabd4e38ad..c2a49df5f921 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_INJECT_CHAIN_CALLS))
>  		return -EINVAL;
>  
>  	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index ffc3e53f5300..dbc9bbf13300 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -9154,6 +9154,79 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
>  	return 0;
>  }
>  
> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
> +{
> +	struct bpf_prog *prog = env->prog;
> +	struct bpf_insn *insn = prog->insnsi;
> +	int i, cnt, delta = 0, ret = -ENOMEM;
> +	const int insn_cnt = prog->len;
> +	struct bpf_array *prog_array;
> +	struct bpf_prog *new_prog;
> +	size_t array_size;
> +
> +	struct bpf_insn call_next[] = {
> +		BPF_LD_IMM64(BPF_REG_2, 0),
> +		/* Save real return value for later */
> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
> +		/* First try tail call with index ret+1 */
> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),

Don't we need to check against the max here, and spectre-proofing here?

> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> +		/* If that doesn't work, try with index 0 (wildcard) */
> +		BPF_MOV64_IMM(BPF_REG_3, 0),
> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> +		/* Restore saved return value and exit */
> +		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
> +		BPF_EXIT_INSN()
> +	};

> +	if (prog->aux->chain_progs)
> +		return 0;

Not sure why this check is here?

> +	array_size = sizeof(*prog_array) + BPF_NUM_CHAIN_SLOTS * sizeof(void*);
> +	prog_array = bpf_map_area_alloc(array_size, NUMA_NO_NODE);
> +
> +	if (!prog_array)
> +		goto out_err;
> +
> +	prog_array->elem_size = sizeof(void*);
> +	prog_array->map.max_entries = BPF_NUM_CHAIN_SLOTS;
> +
> +	call_next[0].imm = (u32)((u64) prog_array);
> +	call_next[1].imm = ((u64) prog_array) >> 32;
> +
> +	for (i = 0; i < insn_cnt; i++, insn++) {
> +		if (insn->code != (BPF_JMP | BPF_EXIT))
> +			continue;

Mm.. don't subprogs also exit with JMP | EXIT?  This should only apply
to main prog, no?

> +		cnt = ARRAY_SIZE(call_next);
> +
> +		new_prog = bpf_patch_insn_data(env, i+delta, call_next, cnt);
> +		if (!new_prog) {
> +			goto out_err;
> +		}
> +
> +		delta    += cnt - 1;
> +		env->prog = prog = new_prog;
> +		insn      = new_prog->insnsi + i + delta;
> +	}
> +
> +	/* If we chain call into other programs, we cannot make any assumptions
> +	 * since they can be replaced dynamically during runtime.
> +	 */
> +	prog->cb_access = 1;
> +	env->prog->aux->stack_depth = MAX_BPF_STACK;
> +	env->prog->aux->max_pkt_offset = MAX_PACKET_OFF;

Some refactoring & reuse of the normal tail call code could be nice?
Same for the hand allocation of the prog array actually :(

> +	prog->aux->chain_progs = prog_array;
> +	return 0;
> +
> +out_err:
> +	bpf_map_area_free(prog_array);
> +	return ret;
> +}
> +
> +
>  static void free_states(struct bpf_verifier_env *env)
>  {
>  	struct bpf_verifier_state_list *sl, *sln;
> @@ -9336,6 +9409,9 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
>  	if (ret == 0)
>  		ret = fixup_bpf_calls(env);
>  
> +	if (ret == 0 && (attr->prog_flags & BPF_F_INJECT_CHAIN_CALLS))
> +		ret = bpf_inject_chain_calls(env);
> +
>  	/* do 32-bit optimization after insn patching has done so those patched
>  	 * insns could be handled correctly.
>  	 */
> 

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Toke Høiland-Jørgensen]

Jakub Kicinski <jakub.kicinski@netronome.com> writes:

> On Fri, 04 Oct 2019 19:22:41 +0200, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> 
>> This adds support for injecting chain call logic into eBPF programs before
>> they return. The code injection is controlled by a flag at program load
>> time; if the flag is set, the verifier will add code to every BPF_EXIT
>> instruction that first does a lookup into a chain call structure to see if
>> it should call into another program before returning. The actual calls
>> reuse the tail call infrastructure.
>> 
>> 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.
>> However, rewriting the program reallocates the bpf_prog struct, which is
>> obviously not possible after the program has been attached to something.
>
> Very exciting stuff :)
>
> Forgive my ignorance, isn't synchronize_rcu() enough to ensure old
> image is no longer used?

Because it's reallocating the 'struct bpf_prog*'. Which is what we pass
into the driver on XDP attach. So we'd have to track down all the uses
and replace the pointer.

At least, that's how I read the code; am I missing something?

> For simple rewrites which don't require much context like the one here
> it'd be cool to do it after loading..

I think re-jit'ing is probably doable, which is why I kinda want the
support in the JIT. We could also conceivably just stick in some NOP
instructions in the eBPF byte code on load and then replace them
dynamically later?

>> One way around this could be a sysctl to force the flag one (for enforcing
>> system-wide support). Another could be to have the chain call support
>> itself built into the interpreter and JIT, which could conceivably be
>> re-run each time we attach a new chain call program. This would also allow
>> the JIT to inject direct calls to the next program instead of using the
>> tail call infrastructure, which presumably would be a performance win. The
>> drawback is, of course, that it would require modifying all the JITs.
>> 
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 5b9d22338606..753abfb78c13 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -383,6 +383,7 @@ struct bpf_prog_aux {
>>  	struct list_head ksym_lnode;
>>  	const struct bpf_prog_ops *ops;
>>  	struct bpf_map **used_maps;
>> +	struct bpf_array *chain_progs;
>>  	struct bpf_prog *prog;
>>  	struct user_struct *user;
>>  	u64 load_time; /* ns since boottime */
>> @@ -443,6 +444,7 @@ struct bpf_array {
>>  
>>  #define BPF_COMPLEXITY_LIMIT_INSNS      1000000 /* yes. 1M insns */
>>  #define MAX_TAIL_CALL_CNT 32
>> +#define BPF_NUM_CHAIN_SLOTS 8
>
> This could be user arg? Also the behaviour of mapping could be user
> controlled? Perhaps even users could pass the snippet to map the return
> code to the location, one day?
>
>>  #define BPF_F_ACCESS_MASK	(BPF_F_RDONLY |		\
>>  				 BPF_F_RDONLY_PROG |	\
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 77c6be96d676..febe8934d19a 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 injection at program return. If set, the
>> + * verifier will rewrite program returns to check for and jump to chain call
>> + * programs configured with the BPF_PROG_CHAIN_* commands to the bpf syscall.
>> + */
>> +#define BPF_F_INJECT_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..98f1ad920e48 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -255,6 +255,16 @@ void __bpf_prog_free(struct bpf_prog *fp)
>>  {
>>  	if (fp->aux) {
>>  		free_percpu(fp->aux->stats);
>> +		if (fp->aux->chain_progs) {
>> +			struct bpf_array *array = fp->aux->chain_progs;
>> +			int i;
>> +
>> +			for (i = 0; i < BPF_NUM_CHAIN_SLOTS; i++)
>> +				if (array->ptrs[i])
>> +					bpf_prog_put(array->ptrs[i]);
>> +
>> +			bpf_map_area_free(array);
>> +		}
>>  		kfree(fp->aux);
>>  	}
>>  	vfree(fp);
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 82eabd4e38ad..c2a49df5f921 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_INJECT_CHAIN_CALLS))
>>  		return -EINVAL;
>>  
>>  	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index ffc3e53f5300..dbc9bbf13300 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -9154,6 +9154,79 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
>>  	return 0;
>>  }
>>  
>> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
>> +{
>> +	struct bpf_prog *prog = env->prog;
>> +	struct bpf_insn *insn = prog->insnsi;
>> +	int i, cnt, delta = 0, ret = -ENOMEM;
>> +	const int insn_cnt = prog->len;
>> +	struct bpf_array *prog_array;
>> +	struct bpf_prog *new_prog;
>> +	size_t array_size;
>> +
>> +	struct bpf_insn call_next[] = {
>> +		BPF_LD_IMM64(BPF_REG_2, 0),
>> +		/* Save real return value for later */
>> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
>> +		/* First try tail call with index ret+1 */
>> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
>
> Don't we need to check against the max here, and spectre-proofing
> here?

No, I don't think so. This is just setting up the arguments for the
BPF_TAIL_CALL instruction below. The JIT will do its thing with that and
emit the range check and the retpoline stuff...

>> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
>> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
>> +		/* If that doesn't work, try with index 0 (wildcard) */
>> +		BPF_MOV64_IMM(BPF_REG_3, 0),
>> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
>> +		/* Restore saved return value and exit */
>> +		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
>> +		BPF_EXIT_INSN()
>> +	};
>
>> +	if (prog->aux->chain_progs)
>> +		return 0;
>
> Not sure why this check is here?

In preparation for being able to call this function multiple times when
attaching the chain call program :)

>> +	array_size = sizeof(*prog_array) + BPF_NUM_CHAIN_SLOTS * sizeof(void*);
>> +	prog_array = bpf_map_area_alloc(array_size, NUMA_NO_NODE);
>> +
>> +	if (!prog_array)
>> +		goto out_err;
>> +
>> +	prog_array->elem_size = sizeof(void*);
>> +	prog_array->map.max_entries = BPF_NUM_CHAIN_SLOTS;
>> +
>> +	call_next[0].imm = (u32)((u64) prog_array);
>> +	call_next[1].imm = ((u64) prog_array) >> 32;
>> +
>> +	for (i = 0; i < insn_cnt; i++, insn++) {
>> +		if (insn->code != (BPF_JMP | BPF_EXIT))
>> +			continue;
>
> Mm.. don't subprogs also exit with JMP | EXIT?  This should only apply
> to main prog, no?

Hmm, no idea? If it does, you're right of course. Guess I'll try to
figure that out...

>> +		cnt = ARRAY_SIZE(call_next);
>> +
>> +		new_prog = bpf_patch_insn_data(env, i+delta, call_next, cnt);
>> +		if (!new_prog) {
>> +			goto out_err;
>> +		}
>> +
>> +		delta    += cnt - 1;
>> +		env->prog = prog = new_prog;
>> +		insn      = new_prog->insnsi + i + delta;
>> +	}
>> +
>> +	/* If we chain call into other programs, we cannot make any assumptions
>> +	 * since they can be replaced dynamically during runtime.
>> +	 */
>> +	prog->cb_access = 1;
>> +	env->prog->aux->stack_depth = MAX_BPF_STACK;
>> +	env->prog->aux->max_pkt_offset = MAX_PACKET_OFF;
>
> Some refactoring & reuse of the normal tail call code could be nice?
> Same for the hand allocation of the prog array actually :(

See above; this is actually just setting up the arguments to reuse the
tail call logic. I'll try to make that clearer...

Most other places that do these kinds of smallish code injections seem
to be hand-coding the instructions; what else would I do?

-Toke

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Jakub Kicinski]

On Sat, 05 Oct 2019 12:29:14 +0200, Toke Høiland-Jørgensen wrote:
> >> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
> >> +{
> >> +	struct bpf_prog *prog = env->prog;
> >> +	struct bpf_insn *insn = prog->insnsi;
> >> +	int i, cnt, delta = 0, ret = -ENOMEM;
> >> +	const int insn_cnt = prog->len;
> >> +	struct bpf_array *prog_array;
> >> +	struct bpf_prog *new_prog;
> >> +	size_t array_size;
> >> +
> >> +	struct bpf_insn call_next[] = {
> >> +		BPF_LD_IMM64(BPF_REG_2, 0),
> >> +		/* Save real return value for later */
> >> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
> >> +		/* First try tail call with index ret+1 */
> >> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),  
> >
> > Don't we need to check against the max here, and spectre-proofing
> > here?  
> 
> No, I don't think so. This is just setting up the arguments for the
> BPF_TAIL_CALL instruction below. The JIT will do its thing with that and
> emit the range check and the retpoline stuff...

Sorry, wrong CPU bug, I meant Meltdown :)

https://elixir.bootlin.com/linux/v5.4-rc1/source/kernel/bpf/verifier.c#L9029

> >> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> >> +		/* If that doesn't work, try with index 0 (wildcard) */
> >> +		BPF_MOV64_IMM(BPF_REG_3, 0),
> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> >> +		/* Restore saved return value and exit */
> >> +		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
> >> +		BPF_EXIT_INSN()
> >> +	};  

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Toke Høiland-Jørgensen]

Jakub Kicinski <jakub.kicinski@netronome.com> writes:

> On Sat, 05 Oct 2019 12:29:14 +0200, Toke Høiland-Jørgensen wrote:
>> >> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
>> >> +{
>> >> +	struct bpf_prog *prog = env->prog;
>> >> +	struct bpf_insn *insn = prog->insnsi;
>> >> +	int i, cnt, delta = 0, ret = -ENOMEM;
>> >> +	const int insn_cnt = prog->len;
>> >> +	struct bpf_array *prog_array;
>> >> +	struct bpf_prog *new_prog;
>> >> +	size_t array_size;
>> >> +
>> >> +	struct bpf_insn call_next[] = {
>> >> +		BPF_LD_IMM64(BPF_REG_2, 0),
>> >> +		/* Save real return value for later */
>> >> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
>> >> +		/* First try tail call with index ret+1 */
>> >> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),  
>> >
>> > Don't we need to check against the max here, and spectre-proofing
>> > here?  
>> 
>> No, I don't think so. This is just setting up the arguments for the
>> BPF_TAIL_CALL instruction below. The JIT will do its thing with that and
>> emit the range check and the retpoline stuff...
>
> Sorry, wrong CPU bug, I meant Meltdown :)
>
> https://elixir.bootlin.com/linux/v5.4-rc1/source/kernel/bpf/verifier.c#L9029

Ah, right. Well, it only adds those extra instructions if
bpf_map_ptr_unpriv() returns true. So I figured that since we're
injecting a pointer here that is not from a userspace map, it was not
needed. Though I must admit I didn't look too closely at exactly which
conditions would make bpf_map_ptr_unpriv() return true... :)

-Toke

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Toke Høiland-Jørgensen]

Jakub Kicinski <jakub.kicinski@netronome.com> writes:

>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 5b9d22338606..753abfb78c13 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -383,6 +383,7 @@ struct bpf_prog_aux {
>>  	struct list_head ksym_lnode;
>>  	const struct bpf_prog_ops *ops;
>>  	struct bpf_map **used_maps;
>> +	struct bpf_array *chain_progs;
>>  	struct bpf_prog *prog;
>>  	struct user_struct *user;
>>  	u64 load_time; /* ns since boottime */
>> @@ -443,6 +444,7 @@ struct bpf_array {
>>  
>>  #define BPF_COMPLEXITY_LIMIT_INSNS      1000000 /* yes. 1M insns */
>>  #define MAX_TAIL_CALL_CNT 32
>> +#define BPF_NUM_CHAIN_SLOTS 8
>
> This could be user arg? Also the behaviour of mapping could be user
> controlled? Perhaps even users could pass the snippet to map the
> return code to the location, one day?

(Forgot to reply to this point).

Yeah, we could make it user-configurable. Or just dynamically increase
the size of the array if we run out. Or do something different with
linked list, as I alluded to in the other reply :)

-Toke

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Alexei Starovoitov]

On Fri, Oct 04, 2019 at 07:22:41PM +0200, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
> 
> This adds support for injecting chain call logic into eBPF programs before
> they return. The code injection is controlled by a flag at program load
> time; if the flag is set, the verifier will add code to every BPF_EXIT
> instruction that first does a lookup into a chain call structure to see if
> it should call into another program before returning. The actual calls
> reuse the tail call infrastructure.
> 
> 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.
> However, rewriting the program reallocates the bpf_prog struct, which is
> obviously not possible after the program has been attached to something.
> 
> One way around this could be a sysctl to force the flag one (for enforcing
> system-wide support). Another could be to have the chain call support
> itself built into the interpreter and JIT, which could conceivably be
> re-run each time we attach a new chain call program. This would also allow
> the JIT to inject direct calls to the next program instead of using the
> tail call infrastructure, which presumably would be a performance win. The
> drawback is, of course, that it would require modifying all the JITs.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
...
>  
> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
> +{
> +	struct bpf_prog *prog = env->prog;
> +	struct bpf_insn *insn = prog->insnsi;
> +	int i, cnt, delta = 0, ret = -ENOMEM;
> +	const int insn_cnt = prog->len;
> +	struct bpf_array *prog_array;
> +	struct bpf_prog *new_prog;
> +	size_t array_size;
> +
> +	struct bpf_insn call_next[] = {
> +		BPF_LD_IMM64(BPF_REG_2, 0),
> +		/* Save real return value for later */
> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
> +		/* First try tail call with index ret+1 */
> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> +		/* If that doesn't work, try with index 0 (wildcard) */
> +		BPF_MOV64_IMM(BPF_REG_3, 0),
> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> +		/* Restore saved return value and exit */
> +		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
> +		BPF_EXIT_INSN()
> +	};

How did you test it?
With the only test from patch 5?
+int xdp_drop_prog(struct xdp_md *ctx)
+{
+       return XDP_DROP;
+}

Please try different program with more than one instruction.
And then look at above asm and think how it can be changed to
get valid R1 all the way to each bpf_exit insn.
Do you see amount of headaches this approach has?

The way you explained the use case of XDP-based firewall plus XDP-based
IPS/IDS it's about "knows nothing" admin that has to deal with more than
one XDP application on an unfamiliar server.
This is the case of debugging.
The admin would probably want to see all values xdp prog returns, right?
The possible answer is to add a tracepoint to bpf_prog_run_xdp().
Most drivers have XDP_DROP stats. So some visibility into drops
is already available.
Dumping all packets that xdp prog is dropping into user space via another
xdp application is imo pointless. User space won't be able to process
this rate of packets. Human admin won't be able to "grep" through millions
of packets either.
xdp-firewall prog is dropping the packets for some reason.
That reason is what admin is looking for!
The admin wants to see inside the program.
The actual content of the packet is like bread crumbs.
The authors of xdp firewall can find packet dumps useful,
but poor admin who was tasked to debug unknown xdp application will
not find anything useful in those packets.
I think what you're advocating for is better xdp debugging.
Let's work on that. Let's focus on designing good debugging facility.
This chaining feature isn't necessary for that and looks unlikely to converge.

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Toke Høiland-Jørgensen]

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

> On Fri, Oct 04, 2019 at 07:22:41PM +0200, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> 
>> This adds support for injecting chain call logic into eBPF programs before
>> they return. The code injection is controlled by a flag at program load
>> time; if the flag is set, the verifier will add code to every BPF_EXIT
>> instruction that first does a lookup into a chain call structure to see if
>> it should call into another program before returning. The actual calls
>> reuse the tail call infrastructure.
>> 
>> 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.
>> However, rewriting the program reallocates the bpf_prog struct, which is
>> obviously not possible after the program has been attached to something.
>> 
>> One way around this could be a sysctl to force the flag one (for enforcing
>> system-wide support). Another could be to have the chain call support
>> itself built into the interpreter and JIT, which could conceivably be
>> re-run each time we attach a new chain call program. This would also allow
>> the JIT to inject direct calls to the next program instead of using the
>> tail call infrastructure, which presumably would be a performance win. The
>> drawback is, of course, that it would require modifying all the JITs.
>> 
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ...
>>  
>> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
>> +{
>> +	struct bpf_prog *prog = env->prog;
>> +	struct bpf_insn *insn = prog->insnsi;
>> +	int i, cnt, delta = 0, ret = -ENOMEM;
>> +	const int insn_cnt = prog->len;
>> +	struct bpf_array *prog_array;
>> +	struct bpf_prog *new_prog;
>> +	size_t array_size;
>> +
>> +	struct bpf_insn call_next[] = {
>> +		BPF_LD_IMM64(BPF_REG_2, 0),
>> +		/* Save real return value for later */
>> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
>> +		/* First try tail call with index ret+1 */
>> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
>> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
>> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
>> +		/* If that doesn't work, try with index 0 (wildcard) */
>> +		BPF_MOV64_IMM(BPF_REG_3, 0),
>> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
>> +		/* Restore saved return value and exit */
>> +		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
>> +		BPF_EXIT_INSN()
>> +	};
>
> How did you test it?
> With the only test from patch 5?
> +int xdp_drop_prog(struct xdp_md *ctx)
> +{
> +       return XDP_DROP;
> +}
>
> Please try different program with more than one instruction.
> And then look at above asm and think how it can be changed to
> get valid R1 all the way to each bpf_exit insn.
> Do you see amount of headaches this approach has?

Ah yes, that's a good point. It seems that I totally overlooked that
issue, somehow...

> The way you explained the use case of XDP-based firewall plus XDP-based
> IPS/IDS it's about "knows nothing" admin that has to deal with more than
> one XDP application on an unfamiliar server.
> This is the case of debugging.

This is not about debugging. The primary use case is about deploying
multiple, independently developed, XDP-enabled applications on the same
server.

Basically, we want the admin to be able to do:

# yum install MyIDS
# yum install MyXDPFirewall

and then have both of those *just work* in XDP mode, on the same
interface.

I originally started solving this in an XDP-specific way (v1 of this
patch set), but the reactions to that was pretty unanimous that this
could be useful as a general eBPF feature. Do you agree with this?

-Toke

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Daniel Borkmann]

On Mon, Oct 07, 2019 at 12:11:31PM +0200, Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > On Fri, Oct 04, 2019 at 07:22:41PM +0200, Toke Høiland-Jørgensen wrote:
> >> From: Toke Høiland-Jørgensen <toke@redhat.com>
> >> 
> >> This adds support for injecting chain call logic into eBPF programs before
> >> they return. The code injection is controlled by a flag at program load
> >> time; if the flag is set, the verifier will add code to every BPF_EXIT
> >> instruction that first does a lookup into a chain call structure to see if
> >> it should call into another program before returning. The actual calls
> >> reuse the tail call infrastructure.
> >> 
> >> 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.
> >> However, rewriting the program reallocates the bpf_prog struct, which is
> >> obviously not possible after the program has been attached to something.
> >> 
> >> One way around this could be a sysctl to force the flag one (for enforcing
> >> system-wide support). Another could be to have the chain call support
> >> itself built into the interpreter and JIT, which could conceivably be
> >> re-run each time we attach a new chain call program. This would also allow
> >> the JIT to inject direct calls to the next program instead of using the
> >> tail call infrastructure, which presumably would be a performance win. The
> >> drawback is, of course, that it would require modifying all the JITs.
> >> 
> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > ...
> >>  
> >> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
> >> +{
> >> +	struct bpf_prog *prog = env->prog;
> >> +	struct bpf_insn *insn = prog->insnsi;
> >> +	int i, cnt, delta = 0, ret = -ENOMEM;
> >> +	const int insn_cnt = prog->len;
> >> +	struct bpf_array *prog_array;
> >> +	struct bpf_prog *new_prog;
> >> +	size_t array_size;
> >> +
> >> +	struct bpf_insn call_next[] = {
> >> +		BPF_LD_IMM64(BPF_REG_2, 0),
> >> +		/* Save real return value for later */
> >> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
> >> +		/* First try tail call with index ret+1 */
> >> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
> >> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> >> +		/* If that doesn't work, try with index 0 (wildcard) */
> >> +		BPF_MOV64_IMM(BPF_REG_3, 0),
> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> >> +		/* Restore saved return value and exit */
> >> +		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
> >> +		BPF_EXIT_INSN()
> >> +	};
> >
> > How did you test it?
> > With the only test from patch 5?
> > +int xdp_drop_prog(struct xdp_md *ctx)
> > +{
> > +       return XDP_DROP;
> > +}
> >
> > Please try different program with more than one instruction.
> > And then look at above asm and think how it can be changed to
> > get valid R1 all the way to each bpf_exit insn.
> > Do you see amount of headaches this approach has?
> 
> Ah yes, that's a good point. It seems that I totally overlooked that
> issue, somehow...
> 
> > The way you explained the use case of XDP-based firewall plus XDP-based
> > IPS/IDS it's about "knows nothing" admin that has to deal with more than
> > one XDP application on an unfamiliar server.
> > This is the case of debugging.
> 
> This is not about debugging. The primary use case is about deploying
> multiple, independently developed, XDP-enabled applications on the same
> server.
> 
> Basically, we want the admin to be able to do:
> 
> # yum install MyIDS
> # yum install MyXDPFirewall
> 
> and then have both of those *just work* in XDP mode, on the same
> interface.

How is the user space loader side handled in this situation, meaning,
what are your plans on this regard?

Reason I'm asking is that those independently developed, XDP-enabled
applications today might on startup simply forcefully remove what is
currently installed on XDP layer at device X, and then override it
with their own program, meaning both of MyIDS and MyXDPFirewall would
remove each other's programs on start.

This will still require some sort of cooperation, think of something
like systemd service files or the like where the former would then
act as the loader to link these together in the background (perhaps
also allowing to specify some sort of a dependency between well-known
ones). How would an admin ad-hoc insert his xdpdump program in between,
meaning what tooling do you have in mind here?

And how would daemons update their own installed programs at runtime?
Right now it's simply atomic update of whatever is currently installed,
but with chained progs, they would need to send it to whatever central
daemon is managing all these instead of just calling bpf() and do the
attaching by themselves, or is the expectation that the application
would need to iterate its own chain via BPF_PROG_CHAIN_GET and resetup
everything by itself?

Thanks,
Daniel

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Toke Høiland-Jørgensen]

Daniel Borkmann <daniel@iogearbox.net> writes:

> On Mon, Oct 07, 2019 at 12:11:31PM +0200, Toke Høiland-Jørgensen wrote:
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> > On Fri, Oct 04, 2019 at 07:22:41PM +0200, Toke Høiland-Jørgensen wrote:
>> >> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> >> 
>> >> This adds support for injecting chain call logic into eBPF programs before
>> >> they return. The code injection is controlled by a flag at program load
>> >> time; if the flag is set, the verifier will add code to every BPF_EXIT
>> >> instruction that first does a lookup into a chain call structure to see if
>> >> it should call into another program before returning. The actual calls
>> >> reuse the tail call infrastructure.
>> >> 
>> >> 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.
>> >> However, rewriting the program reallocates the bpf_prog struct, which is
>> >> obviously not possible after the program has been attached to something.
>> >> 
>> >> One way around this could be a sysctl to force the flag one (for enforcing
>> >> system-wide support). Another could be to have the chain call support
>> >> itself built into the interpreter and JIT, which could conceivably be
>> >> re-run each time we attach a new chain call program. This would also allow
>> >> the JIT to inject direct calls to the next program instead of using the
>> >> tail call infrastructure, which presumably would be a performance win. The
>> >> drawback is, of course, that it would require modifying all the JITs.
>> >> 
>> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> > ...
>> >>  
>> >> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
>> >> +{
>> >> +	struct bpf_prog *prog = env->prog;
>> >> +	struct bpf_insn *insn = prog->insnsi;
>> >> +	int i, cnt, delta = 0, ret = -ENOMEM;
>> >> +	const int insn_cnt = prog->len;
>> >> +	struct bpf_array *prog_array;
>> >> +	struct bpf_prog *new_prog;
>> >> +	size_t array_size;
>> >> +
>> >> +	struct bpf_insn call_next[] = {
>> >> +		BPF_LD_IMM64(BPF_REG_2, 0),
>> >> +		/* Save real return value for later */
>> >> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
>> >> +		/* First try tail call with index ret+1 */
>> >> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
>> >> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
>> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
>> >> +		/* If that doesn't work, try with index 0 (wildcard) */
>> >> +		BPF_MOV64_IMM(BPF_REG_3, 0),
>> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
>> >> +		/* Restore saved return value and exit */
>> >> +		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
>> >> +		BPF_EXIT_INSN()
>> >> +	};
>> >
>> > How did you test it?
>> > With the only test from patch 5?
>> > +int xdp_drop_prog(struct xdp_md *ctx)
>> > +{
>> > +       return XDP_DROP;
>> > +}
>> >
>> > Please try different program with more than one instruction.
>> > And then look at above asm and think how it can be changed to
>> > get valid R1 all the way to each bpf_exit insn.
>> > Do you see amount of headaches this approach has?
>> 
>> Ah yes, that's a good point. It seems that I totally overlooked that
>> issue, somehow...
>> 
>> > The way you explained the use case of XDP-based firewall plus XDP-based
>> > IPS/IDS it's about "knows nothing" admin that has to deal with more than
>> > one XDP application on an unfamiliar server.
>> > This is the case of debugging.
>> 
>> This is not about debugging. The primary use case is about deploying
>> multiple, independently developed, XDP-enabled applications on the same
>> server.
>> 
>> Basically, we want the admin to be able to do:
>> 
>> # yum install MyIDS
>> # yum install MyXDPFirewall
>> 
>> and then have both of those *just work* in XDP mode, on the same
>> interface.
>
> How is the user space loader side handled in this situation, meaning,
> what are your plans on this regard?

I am planning to write a loader that supports this which can be used
stand-alone or as a library (either a standalone library, or as part of
libbpf). Applications can then use the library functions to load itself,
or ship an eBPF binary and have the user load it as needed (depending on
what makes sense for its use case).

> Reason I'm asking is that those independently developed, XDP-enabled
> applications today might on startup simply forcefully remove what is
> currently installed on XDP layer at device X, and then override it
> with their own program, meaning both of MyIDS and MyXDPFirewall would
> remove each other's programs on start.

Yes, they could. I'm hoping to establish sufficiently strong conventions
that they won't :)

> This will still require some sort of cooperation, think of something
> like systemd service files or the like where the former would then act
> as the loader to link these together in the background (perhaps also
> allowing to specify some sort of a dependency between well-known
> ones).

I am imagining something like a manifest file where an application can
specify which return codes it will return that it makes sense to chain
on. E.g., a firewall could say "chain after me when I return XDP_PASS".
In general, chaining on XDP_PASS will probably be the most common.

> How would an admin ad-hoc insert his xdpdump program in between,
> meaning what tooling do you have in mind here?

xdpdump would do something like this:

xdpdump_attach(after=my_firewall, action=drop) {
  my_prog_fd = load_prog(xdpdump.so)
  other_prog_fd = find_prog_fd(name=my_firewall)
  existing_prog_id = bpf_prog_chain_get(other_prog_fd, action)
  if (existing_prog_id) {
     existing_prog_fd = bpf_get_fd_by_id(existing_prog_id);
     bpf_prog_chain_add(my_prog_fd, action, existing_prog_fd);
  }
  bpf_prog_chain_add(other_prog_fd, action, my_prog_fd);
}

xdpdump_detach() {
  bpf_prog_chain_add(other_prog_fd, action, existing_prog_fd);
}

> And how would daemons update their own installed programs at runtime?

Like above; you attach the chain actions to your new prog first, then
atomically replace it with the old one.

> Right now it's simply atomic update of whatever is currently
> installed, but with chained progs, they would need to send it to
> whatever central daemon is managing all these instead of just calling
> bpf() and do the attaching by themselves, or is the expectation that
> the application would need to iterate its own chain via
> BPF_PROG_CHAIN_GET and resetup everything by itself?

I'm expecting each application to "play nice" like above, with the help
of a library. In particular, I want to avoid having a userspace daemon
that needs to keep state. If the kernel keeps the state, different
userspace applications can cooperatively insert and remove themselves
using that state. Similar to how they today can install themselves on
*different* interfaces, but still have to avoid replacing each other...

-Toke

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Alexei Starovoitov]

On Mon, Oct 07, 2019 at 12:11:31PM +0200, Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> 
> > On Fri, Oct 04, 2019 at 07:22:41PM +0200, Toke Høiland-Jørgensen wrote:
> >> From: Toke Høiland-Jørgensen <toke@redhat.com>
> >> 
> >> This adds support for injecting chain call logic into eBPF programs before
> >> they return. The code injection is controlled by a flag at program load
> >> time; if the flag is set, the verifier will add code to every BPF_EXIT
> >> instruction that first does a lookup into a chain call structure to see if
> >> it should call into another program before returning. The actual calls
> >> reuse the tail call infrastructure.
> >> 
> >> 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.
> >> However, rewriting the program reallocates the bpf_prog struct, which is
> >> obviously not possible after the program has been attached to something.
> >> 
> >> One way around this could be a sysctl to force the flag one (for enforcing
> >> system-wide support). Another could be to have the chain call support
> >> itself built into the interpreter and JIT, which could conceivably be
> >> re-run each time we attach a new chain call program. This would also allow
> >> the JIT to inject direct calls to the next program instead of using the
> >> tail call infrastructure, which presumably would be a performance win. The
> >> drawback is, of course, that it would require modifying all the JITs.
> >> 
> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > ...
> >>  
> >> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
> >> +{
> >> +	struct bpf_prog *prog = env->prog;
> >> +	struct bpf_insn *insn = prog->insnsi;
> >> +	int i, cnt, delta = 0, ret = -ENOMEM;
> >> +	const int insn_cnt = prog->len;
> >> +	struct bpf_array *prog_array;
> >> +	struct bpf_prog *new_prog;
> >> +	size_t array_size;
> >> +
> >> +	struct bpf_insn call_next[] = {
> >> +		BPF_LD_IMM64(BPF_REG_2, 0),
> >> +		/* Save real return value for later */
> >> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
> >> +		/* First try tail call with index ret+1 */
> >> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
> >> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> >> +		/* If that doesn't work, try with index 0 (wildcard) */
> >> +		BPF_MOV64_IMM(BPF_REG_3, 0),
> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
> >> +		/* Restore saved return value and exit */
> >> +		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
> >> +		BPF_EXIT_INSN()
> >> +	};
> >
> > How did you test it?
> > With the only test from patch 5?
> > +int xdp_drop_prog(struct xdp_md *ctx)
> > +{
> > +       return XDP_DROP;
> > +}
> >
> > Please try different program with more than one instruction.
> > And then look at above asm and think how it can be changed to
> > get valid R1 all the way to each bpf_exit insn.
> > Do you see amount of headaches this approach has?
> 
> Ah yes, that's a good point. It seems that I totally overlooked that
> issue, somehow...
> 
> > The way you explained the use case of XDP-based firewall plus XDP-based
> > IPS/IDS it's about "knows nothing" admin that has to deal with more than
> > one XDP application on an unfamiliar server.
> > This is the case of debugging.
> 
> This is not about debugging. The primary use case is about deploying
> multiple, independently developed, XDP-enabled applications on the same
> server.
> 
> Basically, we want the admin to be able to do:
> 
> # yum install MyIDS
> # yum install MyXDPFirewall
> 
> and then have both of those *just work* in XDP mode, on the same
> interface.
> 
> I originally started solving this in an XDP-specific way (v1 of this
> patch set), but the reactions to that was pretty unanimous that this
> could be useful as a general eBPF feature. Do you agree with this?

Chaining in general is useful, but
yum install ids
yum install firewall
is not.

Say, xdp doesn't exist. Such ids and firewall will be using iptables.
And they will collide and conflict all over it.
The problem of mixing unrelated ids and firewall is not new.

Re: [PATCH bpf-next v2 1/5] bpf: Support injecting chain calls into BPF programs on load

[Toke Høiland-Jørgensen]

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

> On Mon, Oct 07, 2019 at 12:11:31PM +0200, Toke Høiland-Jørgensen wrote:
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> 
>> > On Fri, Oct 04, 2019 at 07:22:41PM +0200, Toke Høiland-Jørgensen wrote:
>> >> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> >> 
>> >> This adds support for injecting chain call logic into eBPF programs before
>> >> they return. The code injection is controlled by a flag at program load
>> >> time; if the flag is set, the verifier will add code to every BPF_EXIT
>> >> instruction that first does a lookup into a chain call structure to see if
>> >> it should call into another program before returning. The actual calls
>> >> reuse the tail call infrastructure.
>> >> 
>> >> 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.
>> >> However, rewriting the program reallocates the bpf_prog struct, which is
>> >> obviously not possible after the program has been attached to something.
>> >> 
>> >> One way around this could be a sysctl to force the flag one (for enforcing
>> >> system-wide support). Another could be to have the chain call support
>> >> itself built into the interpreter and JIT, which could conceivably be
>> >> re-run each time we attach a new chain call program. This would also allow
>> >> the JIT to inject direct calls to the next program instead of using the
>> >> tail call infrastructure, which presumably would be a performance win. The
>> >> drawback is, of course, that it would require modifying all the JITs.
>> >> 
>> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> > ...
>> >>  
>> >> +static int bpf_inject_chain_calls(struct bpf_verifier_env *env)
>> >> +{
>> >> +	struct bpf_prog *prog = env->prog;
>> >> +	struct bpf_insn *insn = prog->insnsi;
>> >> +	int i, cnt, delta = 0, ret = -ENOMEM;
>> >> +	const int insn_cnt = prog->len;
>> >> +	struct bpf_array *prog_array;
>> >> +	struct bpf_prog *new_prog;
>> >> +	size_t array_size;
>> >> +
>> >> +	struct bpf_insn call_next[] = {
>> >> +		BPF_LD_IMM64(BPF_REG_2, 0),
>> >> +		/* Save real return value for later */
>> >> +		BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
>> >> +		/* First try tail call with index ret+1 */
>> >> +		BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
>> >> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 1),
>> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
>> >> +		/* If that doesn't work, try with index 0 (wildcard) */
>> >> +		BPF_MOV64_IMM(BPF_REG_3, 0),
>> >> +		BPF_RAW_INSN(BPF_JMP | BPF_TAIL_CALL, 0, 0, 0, 0),
>> >> +		/* Restore saved return value and exit */
>> >> +		BPF_MOV64_REG(BPF_REG_0, BPF_REG_6),
>> >> +		BPF_EXIT_INSN()
>> >> +	};
>> >
>> > How did you test it?
>> > With the only test from patch 5?
>> > +int xdp_drop_prog(struct xdp_md *ctx)
>> > +{
>> > +       return XDP_DROP;
>> > +}
>> >
>> > Please try different program with more than one instruction.
>> > And then look at above asm and think how it can be changed to
>> > get valid R1 all the way to each bpf_exit insn.
>> > Do you see amount of headaches this approach has?
>> 
>> Ah yes, that's a good point. It seems that I totally overlooked that
>> issue, somehow...
>> 
>> > The way you explained the use case of XDP-based firewall plus XDP-based
>> > IPS/IDS it's about "knows nothing" admin that has to deal with more than
>> > one XDP application on an unfamiliar server.
>> > This is the case of debugging.
>> 
>> This is not about debugging. The primary use case is about deploying
>> multiple, independently developed, XDP-enabled applications on the same
>> server.
>> 
>> Basically, we want the admin to be able to do:
>> 
>> # yum install MyIDS
>> # yum install MyXDPFirewall
>> 
>> and then have both of those *just work* in XDP mode, on the same
>> interface.
>> 
>> I originally started solving this in an XDP-specific way (v1 of this
>> patch set), but the reactions to that was pretty unanimous that this
>> could be useful as a general eBPF feature. Do you agree with this?
>
> Chaining in general is useful, but
> yum install ids
> yum install firewall
> is not.
>
> Say, xdp doesn't exist. Such ids and firewall will be using iptables.
> And they will collide and conflict all over it.

Yeah, and conventions have evolved to handle these conflicts (for
iptables: create your own chain and put all your rules there; for
connmark, masking off different bits so each application can have its
own mark, etc). As I explained in my reply to Daniel, this is the
minimum kernel infrastructure required to enable userspace applications
to play nice with each other...

-Toke

[PATCH bpf-next v2 2/5] bpf: Add support for setting chain call sequence for programs

[Toke Høiland-Jørgensen]

From: Alan Maguire <alan.maguire@oracle.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 febe8934d19a..b5dbc49fa1a3 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 c2a49df5f921..054b1f7c83f8 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2112,6 +2112,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_array *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 no chain_progs array is set, that's because the chain call flag
+	 * was not set on program load, and so we can't support chain calls.
+	 */
+	if (!prog->aux->chain_progs)
+		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->ptrs + index, next_prog);
+		if (old_prog)
+			bpf_prog_put(old_prog);
+		ret = 0;
+		break;
+	case BPF_PROG_CHAIN_DEL:
+		old_prog = xchg(array->ptrs + 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->ptrs + 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,
@@ -2884,6 +2957,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);

Re: [PATCH bpf-next v2 2/5] bpf: Add support for setting chain call sequence for programs

[Jakub Kicinski]

On Fri, 04 Oct 2019 19:22:42 +0200, Toke Høiland-Jørgensen wrote:
> From: Alan Maguire <alan.maguire@oracle.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>

It'd be good to explain why not just allocate a full prog array (or 
in fact get one from the user), instead of having a hidden one which
requires new command to interact with?

Re: [PATCH bpf-next v2 2/5] bpf: Add support for setting chain call sequence for programs

[Toke Høiland-Jørgensen]

Jakub Kicinski <jakub.kicinski@netronome.com> writes:

> On Fri, 04 Oct 2019 19:22:42 +0200, Toke Høiland-Jørgensen wrote:
>> From: Alan Maguire <alan.maguire@oracle.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>
>
> It'd be good to explain why not just allocate a full prog array (or 
> in fact get one from the user), instead of having a hidden one which
> requires new command to interact with?

Because I consider the reuse of the prog array to be an implementation
detail that we may want to change later. Whereas if we expose it to
userspace it becomes API.

For instance, if we do end up wanting to have support directly in the
JIT for this, we could make the next progs just a linked list that the
JIT will walk and emit direct call instructions for each, instead of
doing the index-lookup.

-Toke

[PATCH bpf-next v2 3/5] tools: Update bpf.h header for program chain calls

[Toke Høiland-Jørgensen]

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..b5dbc49fa1a3 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 injection at program return. If set, the
+ * verifier will rewrite program returns to check for and jump to chain call
+ * programs configured with the BPF_PROG_CHAIN_* commands to the bpf syscall.
+ */
+#define BPF_F_INJECT_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

[PATCH bpf-next v2 4/5] libbpf: Add syscall wrappers for BPF_PROG_CHAIN_* commands

[Toke Høiland-Jørgensen]

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;

[PATCH bpf-next v2 5/5] selftests: Add tests for XDP chain calls

[Toke Høiland-Jørgensen]

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..8a2cd5ded163
--- /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_INJECT_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;
+}