bpf: program testing framework

net/bpf/test_run.c new file +16 -0

+	for (i = 0; i < repeat; i++) {
+		ret = bpf_test_run_one(prog, ctx);
+		if (need_resched()) {
+			if (signal_pending(current))
+				break;
+			time_spent += ktime_get_ns() - time_start;
+			cond_resched();
+			time_start = ktime_get_ns();
+		}
+	}
+	time_spent += ktime_get_ns() - time_start;
+	do_div(time_spent, repeat);
+	*time = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
+
+	return ret;
+}

net/bpf/test_run.c line 34

Quoted 8 lines · On Thu, 30 Mar 2017 21:45:38 -0700 Alexei Starovoitov <[email protected]> wrote:

static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time)

net/bpf/test_run.c new file +7 -0

+{
+	u64 time_start, time_spent = 0;
+	u32 ret = 0, i;
+
+	if (!repeat)
+		repeat = 1;
+	time_start = ktime_get_ns();

I've found that is useful to record the CPU cycles, as it is more useful for comparing between CPUs. The nanosec time measurement varies too much between CPUs and GHz. I do use nanosec measurements myself a lot, but that is mostly because it is easier to relate to pps rates. For eBPF code execution I think it is more useful to get a cycles cost count?

I've been using tsc[1] (rdtsc) to get the CPU cycles, I believe get_cycles() the more generic call, which have arch specific impl. (but can return 0 if no arch support).

The best solution would be to use the perf infrastructure and PMU counter to get both PMU cycles and instructions, as that also tell you about the pipeline efficiency like instructions per cycles. I only got this partly working in [1][2].

Quoted 18 lines · Jesper Dangaard Brouer

net/bpf/test_run.c line 34

Quoted 8 lines · On Thu, 30 Mar 2017 21:45:38 -0700 Alexei Starovoitov <[email protected]> wrote:

static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time)

net/bpf/test_run.c new file +7 -0

+{
+	u64 time_start, time_spent = 0;
+	u32 ret = 0, i;
+
+	if (!repeat)
+		repeat = 1;
+	time_start = ktime_get_ns();  

I've found that is useful to record the CPU cycles, as it is more useful for comparing between CPUs. The nanosec time measurement varies too much between CPUs and GHz. I do use nanosec measurements myself a lot, but that is mostly because it is easier to relate to pps rates. For eBPF code execution I think it is more useful to get a cycles cost count?

for micro-benchmarking of an instruction or small primitives like spin_lock and irq_save/restore, yes. Cycles are more interesting to look at. Here it's the whole program which in case of networking likely does at least a few map lookups.
Also this duration field is more of sanity test then actual metric.

Quoted 8 lines

I've been using tsc[1] (rdtsc) to get the CPU cycles, I believe get_cycles() the more generic call, which have arch specific impl. (but can return 0 if no arch support).

The best solution would be to use the perf infrastructure and PMU counter to get both PMU cycles and instructions, as that also tell you about the pipeline efficiency like instructions per cycles. I only got this partly working in [1][2].

to use get_cycles() or perf_event_create_kernel_counter() the current simple loop would become kthread pinned to cpu and so on. imo it's an overkill.
The only reason 'duration' being reported is a sanity test with user space measurements.
What this command allows to do is:

$ time ./my_bpf_benchmark
The reported time should match the kernel reported 'duration'.
The tiny difference will come from resched. That's sanity part.
Now we can also do
$ perf record ./my_bpf_benchmark

and get all perf goodness for free without adding any kernel code. I want this test_run command to stay execution only. All pmu and performance metrics should stay on perf side.
In case of performance optimization of bpf programs we're trying to improve perf by changing the way program is written, hence we need perf to point out which line of C code is costly. Second is improving performance by changing JIT, map implementations and so on. Here we also want full perf tool power.

Unfortunately there is an issue with perf today, since as soon as my_bpf_benchmark exits, bpf prog is unloaded and ksym is gone, so 'perf report' cannot associate addresses back to source code. We discussed a solution with Arnaldo. So that's orthogonal work in progress which is needed regardless of this test_run command.

User space can also pin itself to cpu instead of asking kernel to do it and run the same program on multiple cpus in parallel testing interaction between concurrent map accesses and so on. So by keeping test_run command as execution only primitive we allow user space to do all the fancy tricks and measurements.

add support for BPF_PROG_TEST_RUN command to libbpf.a

Signed-off-by Alexei Starovoitov <[email protected]>

Acked-by Daniel Borkmann <[email protected]>

Acked-by Martin KaFai Lau <[email protected]>

---
  tools/include/uapi/linux/bpf.h | 24 ++++++++++++++++++++++++
  tools/lib/bpf/bpf.c            | 24 ++++++++++++++++++++++++
  tools/lib/bpf/bpf.h            |  4 +++-
  3 files changed, 51 insertions(+), 1 deletion(-)

tools/include/uapi/linux/bpf.h +24 -0

index 1ea08ce35567..a1d95386f562 100644
@@ -81,6 +81,7 @@ enum bpf_cmd {
  	BPF_OBJ_GET,
  	BPF_PROG_ATTACH,
  	BPF_PROG_DETACH,
+	BPF_PROG_TEST_RUN,
  };
 
  enum bpf_map_type {

@@ -189,6 +190,17 @@ union bpf_attr {
  		__u32		attach_type;
  		__u32		attach_flags;
  	};
+
+	struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
+		__u32		prog_fd;
+		__u32		retval;
+		__u32		data_size_in;
+		__u32		data_size_out;
+		__aligned_u64	data_in;
+		__aligned_u64	data_out;
+		__u32		repeat;
+		__u32		duration;
+	} test;
  } __attribute__((aligned(8)));
 
  /* BPF helper function descriptions:

@@ -459,6 +471,18 @@ union bpf_attr {
   *     Return:
   *       > 0 length of the string including the trailing NUL on success
   *       < 0 error
+ *
+ * u64 bpf_bpf_get_socket_cookie(skb)
+ *     Get the cookie for the socket stored inside sk_buff.
+ *     @skb: pointer to skb
+ *     Return: 8 Bytes non-decreasing number on success or 0 if the socket
+ *     field is missing inside sk_buff
+ *
+ * u32 bpf_get_socket_uid(skb)
+ *     Get the owner uid of the socket stored inside sk_buff.
+ *     @skb: pointer to skb
+ *     Return: uid of the socket owner on success or 0 if the socket pointer
+ *     inside sk_buff is NULL
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\

tools/lib/bpf/bpf.c +24 -0

index 9b58d20e8c93..f84c398c11f4 100644
@@ -209,3 +209,27 @@ int bpf_prog_detach(int target_fd, enum bpf_attach_type type)
 
  	return sys_bpf(BPF_PROG_DETACH, &attr, sizeof(attr));
  }
+
+int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
+		      void *data_out, __u32 *size_out, __u32 *retval,
+		      __u32 *duration)
+{
+	union bpf_attr attr;
+	int ret;
+
+	bzero(&attr, sizeof(attr));
+	attr.test.prog_fd = prog_fd;
+	attr.test.data_in = ptr_to_u64(data);
+	attr.test.data_out = ptr_to_u64(data_out);
+	attr.test.data_size_in = size;
+	attr.test.repeat = repeat;
+
+	ret = sys_bpf(BPF_PROG_TEST_RUN, &attr, sizeof(attr));
+	if (size_out)
+		*size_out = attr.test.data_size_out;
+	if (retval)
+		*retval = attr.test.retval;
+	if (duration)
+		*duration = attr.test.duration;
+	return ret;
+}

tools/lib/bpf/bpf.h +3 -1

index 93f021932623..edb4daeff7a5 100644
@@ -47,6 +47,8 @@ int bpf_obj_get(const char *pathname);
  int bpf_prog_attach(int prog_fd, int attachable_fd, enum bpf_attach_type type,
  		    unsigned int flags);
  int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
-
+int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
+		      void *data_out, __u32 *size_out, __u32 *retval,
+		      __u32 *duration);
 
  #endif

expose bpf_program__set_type() to set program type

Signed-off-by Alexei Starovoitov <[email protected]>

Acked-by Daniel Borkmann <[email protected]>

Acked-by Martin KaFai Lau <[email protected]>

---

Hi Alexei,

Please see the patch I sent. Since we export bpf_program__set_type(), bpf_program__set_xxx() should be built based on it.

expose bpf_program__set_type() to set program type

Signed-off-by Alexei Starovoitov <[email protected]>

Acked-by Daniel Borkmann <[email protected]>

Acked-by Martin KaFai Lau <[email protected]>

---
  tools/lib/bpf/libbpf.c | 3 +--
  tools/lib/bpf/libbpf.h | 2 ++
  2 files changed, 3 insertions(+), 2 deletions(-)

tools/lib/bpf/libbpf.c +0 -0

index ac6eb863b2a4..1a2c07eb7795 100644

tools/lib/bpf/libbpf.h +1 -0

index b30394f9947a..32c7252f734e 100644
@@ -25,6 +25,7 @@
  #include <stdint.h>
  #include <stdbool.h>
  #include <sys/types.h>  // for size_t
+#include <linux/bpf.h>
 
  enum libbpf_errno {
  	__LIBBPF_ERRNO__START = 4000,

Quoted 21 lines · Alexei Starovoitov

expose bpf_program__set_type() to set program type

Signed-off-by Alexei Starovoitov <[email protected]>

Acked-by Daniel Borkmann <[email protected]>

Acked-by Martin KaFai Lau <[email protected]>

---
  tools/lib/bpf/libbpf.c | 3 +--
  tools/lib/bpf/libbpf.h | 2 ++
  2 files changed, 3 insertions(+), 2 deletions(-)

tools/lib/bpf/libbpf.c +1 -1

index ac6eb863b2a4..1a2c07eb7795 100644
@@ -1618,8 +1618,7 @@ int bpf_program__nth_fd(struct bpf_program
 *prog, int n)
      return fd;
  }
  -static void bpf_program__set_type(struct bpf_program *prog,
-                  enum bpf_prog_type type)
+void bpf_program__set_type(struct bpf_program *prog, enum

tools/lib/bpf/libbpf.c int bpf_program__nth_fd(struct bpf_program *prog, int n) · line 1624

Quoted 4 lines

tools/lib/bpf/libbpf.c +0 -0

 bpf_prog_type type)
  {
      prog->type = type;
  }

Since it become a public interface, we need to check if prog is a NULL pointer and check if the value of type is okay, and let it return an errno.

Quoted 12 lines

tools/lib/bpf/libbpf.h +1 -0

index b30394f9947a..32c7252f734e 100644
@@ -25,6 +25,7 @@
  #include <stdint.h>
  #include <stdbool.h>
  #include <sys/types.h>  // for size_t
+#include <linux/bpf.h>
    enum libbpf_errno {
      __LIBBPF_ERRNO__START = 4000,

tools/lib/bpf/libbpf.h int bpf_program__set_sched_cls(struct bpf_program · line 192

Quoted 7 lines

tools/lib/bpf/libbpf.h +1 -0

@@ -185,6 +186,7 @@ int bpf_program__set_sched_cls(struct bpf_program
 *prog);
  int bpf_program__set_sched_act(struct bpf_program *prog);
  int bpf_program__set_xdp(struct bpf_program *prog);
  int bpf_program__set_perf_event(struct bpf_program *prog);
+void bpf_program__set_type(struct bpf_program *prog, enum
 bpf_prog_type type);
 

The above bpf_program__set_xxx become redundancy. It should be generated using macro as static inline functions.

Quoted 2 lines

  bool bpf_program__is_socket_filter(struct bpf_program *prog);
  bool bpf_program__is_tracepoint(struct bpf_program *prog);

bpf_program__is_xxx should be updated like bpf_program__set_xxx, since enum bpf_prog_type is not a problem now.

Quoted 24 lines · Wangnan (F)

Quoted 21 lines · Alexei Starovoitov

expose bpf_program__set_type() to set program type

Signed-off-by Alexei Starovoitov <[email protected]>

Acked-by Daniel Borkmann <[email protected]>

Acked-by Martin KaFai Lau <[email protected]>

---
  tools/lib/bpf/libbpf.c | 3 +--
  tools/lib/bpf/libbpf.h | 2 ++
  2 files changed, 3 insertions(+), 2 deletions(-)

tools/lib/bpf/libbpf.c +1 -1

index ac6eb863b2a4..1a2c07eb7795 100644
@@ -1618,8 +1618,7 @@ int bpf_program__nth_fd(struct bpf_program
 *prog, int n)
      return fd;
  }
  -static void bpf_program__set_type(struct bpf_program *prog,
-                  enum bpf_prog_type type)
+void bpf_program__set_type(struct bpf_program *prog, enum

Quoted 8 lines

tools/lib/bpf/libbpf.c int bpf_program__nth_fd(struct bpf_program *prog, int n) · line 1624

Quoted 4 lines

tools/lib/bpf/libbpf.c +0 -0

 bpf_prog_type type)
  {
      prog->type = type;
  }

Since it become a public interface, we need to check if prog is a NULL pointer and check if the value of type is okay, and let it return an errno.

I strongly disagree with such defensive programming. It's a cause of bugs for users of this library.
I think you're trying to mimic c++ setters/getters, but c++ never checks 'this != null', since passing null into setter is a bug of the user of the library and not the library. The setters also should have 'void' return type when setters cannot fail. That is exactly the case here.
If, in the future, we decide that this libbpf shouldn't support all bpf program types then you'd need to change the prototype of this function to return error code and change all places where this function is called to check for error code.
It may or may not be the right approach.
For example today the only user of bpf_program__set*() methods is perf/util/bpf-loader.c and it calls bpf_program__set_kprobe() and bpf_program__set_tracepoint() _without_ checking the return value which is _correct_ thing to do. Instead the current prototype of 'int bpf_program__set_tracepoint(struct bpf_program *prog); is not correct and I suggest you to fix it.

You also need to do other cleanup. Like in bpf_object__elf_finish() you have:

        if (!obj_elf_valid(obj))
                return;

        if (obj->efile.elf) {

which is redundant. It's another example where mistakes creep in due to defensive programming.

Another bug in bpf_object__close() which does:

        if (!obj)
                return;

again defensive programming strikes, since
you're not checking IS_ERR(obj) and that's what bpf_object__open() returns, so most users of the library (who don't read the source code and just using it based on .h) will do

obj = bpf_object__open(...);
bpf_object__close(obj);

and current 'if (!obj)' won't help and it will segfault. I hit this issue will developing this patch set.

Quoted 23 lines

Quoted 12 lines

tools/lib/bpf/libbpf.h +1 -0

index b30394f9947a..32c7252f734e 100644
@@ -25,6 +25,7 @@
  #include <stdint.h>
  #include <stdbool.h>
  #include <sys/types.h>  // for size_t
+#include <linux/bpf.h>
    enum libbpf_errno {
      __LIBBPF_ERRNO__START = 4000,

tools/lib/bpf/libbpf.h int bpf_program__set_sched_cls(struct bpf_program · line 192

Quoted 7 lines

tools/lib/bpf/libbpf.h +1 -0

@@ -185,6 +186,7 @@ int bpf_program__set_sched_cls(struct bpf_program
 *prog);
  int bpf_program__set_sched_act(struct bpf_program *prog);
  int bpf_program__set_xdp(struct bpf_program *prog);
  int bpf_program__set_perf_event(struct bpf_program *prog);
+void bpf_program__set_type(struct bpf_program *prog, enum
 bpf_prog_type type);
 

The above bpf_program__set_xxx become redundancy. It should be generated using macro as static inline functions.

Quoted 5 lines

Quoted 2 lines

  bool bpf_program__is_socket_filter(struct bpf_program *prog);
  bool bpf_program__is_tracepoint(struct bpf_program *prog);

bpf_program__is_xxx should be updated like bpf_program__set_xxx, since enum bpf_prog_type is not a problem now.

All of these suggestions is a cleanup for your code that you need to do yourself. I actually suggest you kill all bpf_program__is*() and all but one bpf_program__set*() functions.
The current user perf/util/bpf-loader.c should be converted to using bpf_program__set_type() with _void_ return code that I'm introducing here.

Overall, I think, tools/lib/bpf/ is a nice library and it can be used by many projects, but I suggest to stop making excuses based on your proprietary usage of it.

Also please cc me in the future on changes to the library. It still has my copyrights, though a lot has changed, since last time I looked at it and it's my fault for not pay attention earlier.

OK. Then let your patch be merged first then let me do the code cleanup.

Development and testing of networking bpf programs is quite cumbersome. Especially tricky are XDP programs that attach to real netdevices and program development feels like working on the car engine while the car is in motion.
Another problem is ongoing changes to upstream llvm core that can introduce an optimization that verifier will not recognize. llvm bpf backend tests have no ability to run the programs. To improve this situation introduce BPF_PROG_TEST_RUN command to test and performance benchmark bpf programs.
It achieves several goals:
- development of xdp and skb based bpf programs can be done in a canned environment with unit tests
- program performance optimizations can be benchmarked outside of networking core (without driver and skb costs)
- continuous testing of upstream changes is finally practical

Patches 4,5,6 add C based test cases of various complexity to cover some sched_cls and xdp features. More tests will be added in the future. The tests were run on centos7 only.

For now the framework supports only skb and xdp programs. In the future it can be extended to socket_filter and tracing program types.

More details are in individual patches.

v1->v2:
- rename bpf_program_test_run->bpf_prog_test_run
- add missing #include <linux/bpf.h> since libbpf.h shouldn't depend on prior includes
- reordered patches 3 and 4 to keep bisect clean