[PATCH 1/4] samples/bpf: Fix build breakage with map_perf_test_user.c

[PATCH 1/4] samples/bpf: Fix build breakage with map_perf_test_user.c

[Naveen N. Rao]

Building BPF samples is failing with the below error:

samples/bpf/map_perf_test_user.c: In function ‘main’:
samples/bpf/map_perf_test_user.c:134:9: error: variable ‘r’ has
initializer but incomplete type
  struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
         ^
samples/bpf/map_perf_test_user.c:134:21: error: ‘RLIM_INFINITY’
undeclared (first use in this function)
  struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
                     ^
samples/bpf/map_perf_test_user.c:134:21: note: each undeclared
identifier is reported only once for each function it appears in
samples/bpf/map_perf_test_user.c:134:9: warning: excess elements in
struct initializer [enabled by default]
  struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
         ^
samples/bpf/map_perf_test_user.c:134:9: warning: (near initialization
for ‘r’) [enabled by default]
samples/bpf/map_perf_test_user.c:134:9: warning: excess elements in
struct initializer [enabled by default]
samples/bpf/map_perf_test_user.c:134:9: warning: (near initialization
for ‘r’) [enabled by default]
samples/bpf/map_perf_test_user.c:134:16: error: storage size of ‘r’
isn’t known
  struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
                ^
samples/bpf/map_perf_test_user.c:139:2: warning: implicit declaration of
function ‘setrlimit’ [-Wimplicit-function-declaration]
  setrlimit(RLIMIT_MEMLOCK, &r);
  ^
samples/bpf/map_perf_test_user.c:139:12: error: ‘RLIMIT_MEMLOCK’
undeclared (first use in this function)
  setrlimit(RLIMIT_MEMLOCK, &r);
            ^
samples/bpf/map_perf_test_user.c:134:16: warning: unused variable ‘r’
[-Wunused-variable]
  struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
                ^
make[2]: *** [samples/bpf/map_perf_test_user.o] Error 1

Fix this by including the necessary header file.

Cc: Alexei Starovoitov <ast@fb.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 samples/bpf/map_perf_test_user.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/samples/bpf/map_perf_test_user.c b/samples/bpf/map_perf_test_user.c
index 95af56e..3147377 100644
--- a/samples/bpf/map_perf_test_user.c
+++ b/samples/bpf/map_perf_test_user.c
@@ -17,6 +17,7 @@
 #include <linux/bpf.h>
 #include <string.h>
 #include <time.h>
+#include <sys/resource.h>
 #include "libbpf.h"
 #include "bpf_load.h"
 
-- 
2.7.4

[PATCH 2/4] samples/bpf: Use llc in PATH, rather than a hardcoded value

[Naveen N. Rao]

While at it, fix some typos in the comment.

Cc: Alexei Starovoitov <ast@fb.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 samples/bpf/Makefile | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 502c9fc..88bc5a0 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -76,16 +76,13 @@ HOSTLOADLIBES_offwaketime += -lelf
 HOSTLOADLIBES_spintest += -lelf
 HOSTLOADLIBES_map_perf_test += -lelf -lrt
 
-# point this to your LLVM backend with bpf support
-LLC=$(srctree)/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
-
-# asm/sysreg.h inline assmbly used by it is incompatible with llvm.
-# But, ehere is not easy way to fix it, so just exclude it since it is
+# asm/sysreg.h - inline assembly used by it is incompatible with llvm.
+# But, there is no easy way to fix it, so just exclude it since it is
 # useless for BPF samples.
 $(obj)/%.o: $(src)/%.c
 	clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
 		-D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
-		-O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
+		-O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=obj -o $@
 	clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
 		-D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
-		-O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=asm -o $@.s
+		-O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o $@.s
-- 
2.7.4

[PATCH 3/4] samples/bpf: Simplify building BPF samples

[Naveen N. Rao]

Make BPF samples build depend on CONFIG_SAMPLE_BPF. We still don't add a
Kconfig option since that will add a dependency on llvm for allyesconfig
builds which may not be desirable.

Those who need to build the BPF samples can now just do:

make CONFIG_SAMPLE_BPF=y

or:

export CONFIG_SAMPLE_BPF=y
make

Cc: Alexei Starovoitov <ast@fb.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 samples/Makefile     |  2 +-
 samples/bpf/Makefile | 39 ++++++++++++++++++++-------------------
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/samples/Makefile b/samples/Makefile
index 48001d7..3c77fc8 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -2,4 +2,4 @@
 
 obj-$(CONFIG_SAMPLES)	+= kobject/ kprobes/ trace_events/ livepatch/ \
 			   hw_breakpoint/ kfifo/ kdb/ hidraw/ rpmsg/ seccomp/ \
-			   configfs/
+			   configfs/ bpf/
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 88bc5a0..bc5b675 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -2,23 +2,23 @@
 obj- := dummy.o
 
 # List of programs to build
-hostprogs-y := test_verifier test_maps
-hostprogs-y += sock_example
-hostprogs-y += fds_example
-hostprogs-y += sockex1
-hostprogs-y += sockex2
-hostprogs-y += sockex3
-hostprogs-y += tracex1
-hostprogs-y += tracex2
-hostprogs-y += tracex3
-hostprogs-y += tracex4
-hostprogs-y += tracex5
-hostprogs-y += tracex6
-hostprogs-y += trace_output
-hostprogs-y += lathist
-hostprogs-y += offwaketime
-hostprogs-y += spintest
-hostprogs-y += map_perf_test
+hostprogs-$(CONFIG_SAMPLE_BPF) := test_verifier test_maps
+hostprogs-$(CONFIG_SAMPLE_BPF) += sock_example
+hostprogs-$(CONFIG_SAMPLE_BPF) += fds_example
+hostprogs-$(CONFIG_SAMPLE_BPF) += sockex1
+hostprogs-$(CONFIG_SAMPLE_BPF) += sockex2
+hostprogs-$(CONFIG_SAMPLE_BPF) += sockex3
+hostprogs-$(CONFIG_SAMPLE_BPF) += tracex1
+hostprogs-$(CONFIG_SAMPLE_BPF) += tracex2
+hostprogs-$(CONFIG_SAMPLE_BPF) += tracex3
+hostprogs-$(CONFIG_SAMPLE_BPF) += tracex4
+hostprogs-$(CONFIG_SAMPLE_BPF) += tracex5
+hostprogs-$(CONFIG_SAMPLE_BPF) += tracex6
+hostprogs-$(CONFIG_SAMPLE_BPF) += trace_output
+hostprogs-$(CONFIG_SAMPLE_BPF) += lathist
+hostprogs-$(CONFIG_SAMPLE_BPF) += offwaketime
+hostprogs-$(CONFIG_SAMPLE_BPF) += spintest
+hostprogs-$(CONFIG_SAMPLE_BPF) += map_perf_test
 
 test_verifier-objs := test_verifier.o libbpf.o
 test_maps-objs := test_maps.o libbpf.o
@@ -39,8 +39,8 @@ offwaketime-objs := bpf_load.o libbpf.o offwaketime_user.o
 spintest-objs := bpf_load.o libbpf.o spintest_user.o
 map_perf_test-objs := bpf_load.o libbpf.o map_perf_test_user.o
 
-# Tell kbuild to always build the programs
-always := $(hostprogs-y)
+ifdef CONFIG_SAMPLE_BPF
+always := $(hostprogs-$(CONFIG_SAMPLE_BPF))
 always += sockex1_kern.o
 always += sockex2_kern.o
 always += sockex3_kern.o
@@ -56,6 +56,7 @@ always += lathist_kern.o
 always += offwaketime_kern.o
 always += spintest_kern.o
 always += map_perf_test_kern.o
+endif
 
 HOSTCFLAGS += -I$(objtree)/usr/include
 
-- 
2.7.4

[PATCH 4/4] samples/bpf: Enable powerpc support

[Naveen N. Rao]

Add the necessary definitions for building bpf samples on ppc.

Since ppc doesn't store function return address on the stack, modify how
PT_REGS_RET() and PT_REGS_FP() work.

Also, introduce PT_REGS_IP() to access the instruction pointer. I have
fixed this to work with x86_64 and arm64, but not s390.

Cc: Alexei Starovoitov <ast@fb.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 samples/bpf/bpf_helpers.h   | 26 ++++++++++++++++++++++++++
 samples/bpf/spintest_kern.c |  2 +-
 samples/bpf/tracex2_kern.c  |  4 ++--
 samples/bpf/tracex4_kern.c  |  2 +-
 4 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
index 9363500..343423c 100644
--- a/samples/bpf/bpf_helpers.h
+++ b/samples/bpf/bpf_helpers.h
@@ -82,6 +82,7 @@ static int (*bpf_l4_csum_replace)(void *ctx, int off, int from, int to, int flag
 #define PT_REGS_FP(x) ((x)->bp)
 #define PT_REGS_RC(x) ((x)->ax)
 #define PT_REGS_SP(x) ((x)->sp)
+#define PT_REGS_IP(x) ((x)->ip)
 
 #elif defined(__s390x__)
 
@@ -94,6 +95,7 @@ static int (*bpf_l4_csum_replace)(void *ctx, int off, int from, int to, int flag
 #define PT_REGS_FP(x) ((x)->gprs[11]) /* Works only with CONFIG_FRAME_POINTER */
 #define PT_REGS_RC(x) ((x)->gprs[2])
 #define PT_REGS_SP(x) ((x)->gprs[15])
+#define PT_REGS_IP(x) ((x)->ip)
 
 #elif defined(__aarch64__)
 
@@ -106,6 +108,30 @@ static int (*bpf_l4_csum_replace)(void *ctx, int off, int from, int to, int flag
 #define PT_REGS_FP(x) ((x)->regs[29]) /* Works only with CONFIG_FRAME_POINTER */
 #define PT_REGS_RC(x) ((x)->regs[0])
 #define PT_REGS_SP(x) ((x)->sp)
+#define PT_REGS_IP(x) ((x)->pc)
+
+#elif defined(__powerpc__)
+
+#define PT_REGS_PARM1(x) ((x)->gpr[3])
+#define PT_REGS_PARM2(x) ((x)->gpr[4])
+#define PT_REGS_PARM3(x) ((x)->gpr[5])
+#define PT_REGS_PARM4(x) ((x)->gpr[6])
+#define PT_REGS_PARM5(x) ((x)->gpr[7])
+#define PT_REGS_RC(x) ((x)->gpr[3])
+#define PT_REGS_SP(x) ((x)->sp)
+#define PT_REGS_IP(x) ((x)->nip)
 
 #endif
+
+#ifdef __powerpc__
+#define BPF_KPROBE_READ_RET_IP(ip, ctx)		{ (ip) = (ctx)->link; }
+#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)	BPF_KPROBE_READ_RET_IP(ip, ctx)
+#else
+#define BPF_KPROBE_READ_RET_IP(ip, ctx)						\
+		bpf_probe_read(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx))
+#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)					\
+		bpf_probe_read(&(ip), sizeof(ip),				\
+				(void *)(PT_REGS_FP(ctx) + sizeof(ip)))
+#endif
+
 #endif
diff --git a/samples/bpf/spintest_kern.c b/samples/bpf/spintest_kern.c
index 4b27619..ce0167d 100644
--- a/samples/bpf/spintest_kern.c
+++ b/samples/bpf/spintest_kern.c
@@ -34,7 +34,7 @@ struct bpf_map_def SEC("maps") stackmap = {
 #define PROG(foo) \
 int foo(struct pt_regs *ctx) \
 { \
-	long v = ctx->ip, *val; \
+	long v = PT_REGS_IP(ctx), *val; \
 \
 	val = bpf_map_lookup_elem(&my_map, &v); \
 	bpf_map_update_elem(&my_map, &v, &v, BPF_ANY); \
diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
index 09c1adc..6d6eefd 100644
--- a/samples/bpf/tracex2_kern.c
+++ b/samples/bpf/tracex2_kern.c
@@ -27,10 +27,10 @@ int bpf_prog2(struct pt_regs *ctx)
 	long init_val = 1;
 	long *value;
 
-	/* x64/s390x specific: read ip of kfree_skb caller.
+	/* read ip of kfree_skb caller.
 	 * non-portable version of __builtin_return_address(0)
 	 */
-	bpf_probe_read(&loc, sizeof(loc), (void *)PT_REGS_RET(ctx));
+	BPF_KPROBE_READ_RET_IP(loc, ctx);
 
 	value = bpf_map_lookup_elem(&my_map, &loc);
 	if (value)
diff --git a/samples/bpf/tracex4_kern.c b/samples/bpf/tracex4_kern.c
index ac46714..6dd8e38 100644
--- a/samples/bpf/tracex4_kern.c
+++ b/samples/bpf/tracex4_kern.c
@@ -40,7 +40,7 @@ int bpf_prog2(struct pt_regs *ctx)
 	long ip = 0;
 
 	/* get ip address of kmem_cache_alloc_node() caller */
-	bpf_probe_read(&ip, sizeof(ip), (void *)(PT_REGS_FP(ctx) + sizeof(ip)));
+	BPF_KRETPROBE_READ_RET_IP(ip, ctx);
 
 	struct pair v = {
 		.val = bpf_ktime_get_ns(),
-- 
2.7.4

Re: [PATCH 1/4] samples/bpf: Fix build breakage with map_perf_test_user.c

[Alexei Starovoitov]

On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> Building BPF samples is failing with the below error:
>
> samples/bpf/map_perf_test_user.c: In function ‘main’:
> samples/bpf/map_perf_test_user.c:134:9: error: variable ‘r’ has
> initializer but incomplete type
>    struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
>           ^
> Fix this by including the necessary header file.
>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>   samples/bpf/map_perf_test_user.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/samples/bpf/map_perf_test_user.c b/samples/bpf/map_perf_test_user.c
> index 95af56e..3147377 100644
> --- a/samples/bpf/map_perf_test_user.c
> +++ b/samples/bpf/map_perf_test_user.c
> @@ -17,6 +17,7 @@
>   #include <linux/bpf.h>
>   #include <string.h>
>   #include <time.h>
> +#include <sys/resource.h>
>   #include "libbpf.h"
>   #include "bpf_load.h"

It's failing this way on powerpc? Odd.
Such hidden header dependency was always puzzling to me. Anyway:
Acked-by: Alexei Starovoitov <ast@kernel.org>

I'm assuming you want this set to go via 'net' tree, so please resubmit
with [PATCH net 1/4] subjects and cc netdev.

Reviewing your other patches...

Re: [PATCH 2/4] samples/bpf: Use llc in PATH, rather than a hardcoded value

[Alexei Starovoitov]

On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> While at it, fix some typos in the comment.
>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>   samples/bpf/Makefile | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 502c9fc..88bc5a0 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -76,16 +76,13 @@ HOSTLOADLIBES_offwaketime += -lelf
>   HOSTLOADLIBES_spintest += -lelf
>   HOSTLOADLIBES_map_perf_test += -lelf -lrt
>
> -# point this to your LLVM backend with bpf support
> -LLC=$(srctree)/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
> -
> -# asm/sysreg.h inline assmbly used by it is incompatible with llvm.
> -# But, ehere is not easy way to fix it, so just exclude it since it is
> +# asm/sysreg.h - inline assembly used by it is incompatible with llvm.
> +# But, there is no easy way to fix it, so just exclude it since it is
>   # useless for BPF samples.
>   $(obj)/%.o: $(src)/%.c
>   	clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
>   		-D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> -		-O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
> +		-O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=obj -o $@
>   	clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
>   		-D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> -		-O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=asm -o $@.s
> +		-O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o $@.s

that was a workaround when clang/llvm didn't have bpf support.
Now clang 3.7 and 3.8 have bpf built-in, so make sense to remove
manual calls to llc completely.
Just use 'clang -target bpf -O2 -D... -c $< -o $@'

Re: [PATCH 3/4] samples/bpf: Simplify building BPF samples

[Alexei Starovoitov]

On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> Make BPF samples build depend on CONFIG_SAMPLE_BPF. We still don't add a
> Kconfig option since that will add a dependency on llvm for allyesconfig
> builds which may not be desirable.
>
> Those who need to build the BPF samples can now just do:
>
> make CONFIG_SAMPLE_BPF=y
>
> or:
>
> export CONFIG_SAMPLE_BPF=y
> make

I don't like this 'simplification'.
make samples/bpf/
is much easier to type than capital letters.

> diff --git a/samples/Makefile b/samples/Makefile
> index 48001d7..3c77fc8 100644
> --- a/samples/Makefile
> +++ b/samples/Makefile
> @@ -2,4 +2,4 @@
>
>   obj-$(CONFIG_SAMPLES)	+= kobject/ kprobes/ trace_events/ livepatch/ \
>   			   hw_breakpoint/ kfifo/ kdb/ hidraw/ rpmsg/ seccomp/ \
> -			   configfs/
> +			   configfs/ bpf/
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 88bc5a0..bc5b675 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -2,23 +2,23 @@
>   obj- := dummy.o
>
>   # List of programs to build
> -hostprogs-y := test_verifier test_maps
> -hostprogs-y += sock_example
> -hostprogs-y += fds_example
> -hostprogs-y += sockex1
> -hostprogs-y += sockex2
> -hostprogs-y += sockex3
> -hostprogs-y += tracex1
> -hostprogs-y += tracex2
> -hostprogs-y += tracex3
> -hostprogs-y += tracex4
> -hostprogs-y += tracex5
> -hostprogs-y += tracex6
> -hostprogs-y += trace_output
> -hostprogs-y += lathist
> -hostprogs-y += offwaketime
> -hostprogs-y += spintest
> -hostprogs-y += map_perf_test
> +hostprogs-$(CONFIG_SAMPLE_BPF) := test_verifier test_maps
> +hostprogs-$(CONFIG_SAMPLE_BPF) += sock_example
> +hostprogs-$(CONFIG_SAMPLE_BPF) += fds_example
> +hostprogs-$(CONFIG_SAMPLE_BPF) += sockex1
> +hostprogs-$(CONFIG_SAMPLE_BPF) += sockex2
> +hostprogs-$(CONFIG_SAMPLE_BPF) += sockex3
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex1
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex2
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex3
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex4
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex5
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex6
> +hostprogs-$(CONFIG_SAMPLE_BPF) += trace_output
> +hostprogs-$(CONFIG_SAMPLE_BPF) += lathist
> +hostprogs-$(CONFIG_SAMPLE_BPF) += offwaketime
> +hostprogs-$(CONFIG_SAMPLE_BPF) += spintest
> +hostprogs-$(CONFIG_SAMPLE_BPF) += map_perf_test
>
>   test_verifier-objs := test_verifier.o libbpf.o
>   test_maps-objs := test_maps.o libbpf.o
> @@ -39,8 +39,8 @@ offwaketime-objs := bpf_load.o libbpf.o offwaketime_user.o
>   spintest-objs := bpf_load.o libbpf.o spintest_user.o
>   map_perf_test-objs := bpf_load.o libbpf.o map_perf_test_user.o
>
> -# Tell kbuild to always build the programs
> -always := $(hostprogs-y)
> +ifdef CONFIG_SAMPLE_BPF
> +always := $(hostprogs-$(CONFIG_SAMPLE_BPF))
>   always += sockex1_kern.o
>   always += sockex2_kern.o
>   always += sockex3_kern.o
> @@ -56,6 +56,7 @@ always += lathist_kern.o
>   always += offwaketime_kern.o
>   always += spintest_kern.o
>   always += map_perf_test_kern.o
> +endif
>
>   HOSTCFLAGS += -I$(objtree)/usr/include
>
>

Re: [PATCH 4/4] samples/bpf: Enable powerpc support

[Alexei Starovoitov]

On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> Add the necessary definitions for building bpf samples on ppc.
>
> Since ppc doesn't store function return address on the stack, modify how
> PT_REGS_RET() and PT_REGS_FP() work.
>
> Also, introduce PT_REGS_IP() to access the instruction pointer. I have
> fixed this to work with x86_64 and arm64, but not s390.
>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
...
> +
> +#ifdef __powerpc__
> +#define BPF_KPROBE_READ_RET_IP(ip, ctx)		{ (ip) = (ctx)->link; }
> +#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)	BPF_KPROBE_READ_RET_IP(ip, ctx)
> +#else
> +#define BPF_KPROBE_READ_RET_IP(ip, ctx)						\
> +		bpf_probe_read(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx))
> +#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)					\
> +		bpf_probe_read(&(ip), sizeof(ip),				\
> +				(void *)(PT_REGS_FP(ctx) + sizeof(ip)))

makes sense, but please use ({ }) gcc extension instead of {} and
open call to make sure that macro body is scoped.

Re: [PATCH 2/4] samples/bpf: Use llc in PATH, rather than a hardcoded value

[Daniel Borkmann]

On 03/31/2016 07:46 PM, Alexei Starovoitov wrote:
> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
>> While at it, fix some typos in the comment.
>>
>> Cc: Alexei Starovoitov <ast@fb.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>>   samples/bpf/Makefile | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 502c9fc..88bc5a0 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -76,16 +76,13 @@ HOSTLOADLIBES_offwaketime += -lelf
>>   HOSTLOADLIBES_spintest += -lelf
>>   HOSTLOADLIBES_map_perf_test += -lelf -lrt
>>
>> -# point this to your LLVM backend with bpf support
>> -LLC=$(srctree)/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
>> -
>> -# asm/sysreg.h inline assmbly used by it is incompatible with llvm.
>> -# But, ehere is not easy way to fix it, so just exclude it since it is
>> +# asm/sysreg.h - inline assembly used by it is incompatible with llvm.
>> +# But, there is no easy way to fix it, so just exclude it since it is
>>   # useless for BPF samples.
>>   $(obj)/%.o: $(src)/%.c
>>       clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
>>           -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
>> -        -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
>> +        -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=obj -o $@
>>       clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
>>           -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
>> -        -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=asm -o $@.s
>> +        -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o $@.s
>
> that was a workaround when clang/llvm didn't have bpf support.
> Now clang 3.7 and 3.8 have bpf built-in, so make sense to remove
> manual calls to llc completely.
> Just use 'clang -target bpf -O2 -D... -c $< -o $@'

+1, the clang part in that Makefile should also more correctly be called
with '-target bpf' as it turns out (despite llc with '-march=bpf' ...).
Better to use clang directly as suggested by Alexei.

Re: [PATCH 1/4] samples/bpf: Fix build breakage with map_perf_test_user.c

[Naveen N. Rao]

On 2016/03/31 10:43AM, Alexei Starovoitov wrote:
> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> >Building BPF samples is failing with the below error:
> >
> >samples/bpf/map_perf_test_user.c: In function ‘main’:
> >samples/bpf/map_perf_test_user.c:134:9: error: variable ‘r’ has
> >initializer but incomplete type
> >   struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
> >          ^
> >Fix this by including the necessary header file.
> >
> >Cc: Alexei Starovoitov <ast@fb.com>
> >Cc: David S. Miller <davem@davemloft.net>
> >Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> >Cc: Michael Ellerman <mpe@ellerman.id.au>
> >Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> >---
> >  samples/bpf/map_perf_test_user.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> >diff --git a/samples/bpf/map_perf_test_user.c b/samples/bpf/map_perf_test_user.c
> >index 95af56e..3147377 100644
> >--- a/samples/bpf/map_perf_test_user.c
> >+++ b/samples/bpf/map_perf_test_user.c
> >@@ -17,6 +17,7 @@
> >  #include <linux/bpf.h>
> >  #include <string.h>
> >  #include <time.h>
> >+#include <sys/resource.h>
> >  #include "libbpf.h"
> >  #include "bpf_load.h"
> 
> It's failing this way on powerpc? Odd.

This fails for me on x86_64 too -- RHEL 7.1.

> Such hidden header dependency was always puzzling to me. Anyway:
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> 
> I'm assuming you want this set to go via 'net' tree, so please resubmit
> with [PATCH net 1/4] subjects and cc netdev.

Sure.

> 
> Reviewing your other patches...

Thanks for your review!

- Naveen

Re: [PATCH 3/4] samples/bpf: Simplify building BPF samples

[Naveen N. Rao]

On 2016/03/31 10:49AM, Alexei Starovoitov wrote:
> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> >Make BPF samples build depend on CONFIG_SAMPLE_BPF. We still don't add a
> >Kconfig option since that will add a dependency on llvm for allyesconfig
> >builds which may not be desirable.
> >
> >Those who need to build the BPF samples can now just do:
> >
> >make CONFIG_SAMPLE_BPF=y
> >
> >or:
> >
> >export CONFIG_SAMPLE_BPF=y
> >make
> 
> I don't like this 'simplification'.
> make samples/bpf/
> is much easier to type than capital letters.

This started out as a patch to have the BPF samples built with a Kconfig 
option. As stated in the commit description, I realised that it won't 
work for allyesconfig builds. However, the reason I retained this patch 
is since it gets us one step closer to building the samples as part of 
the kernel build.

The 'simplification' is since I can now have the export in my .bashrc 
and the kernel build will now build the BPF samples too without 
requiring an additional 'make samples/bpf/' step.

I agree this is subjective, so I am ok if this isn't taken in.


- Naveen

Re: [PATCH 1/4] samples/bpf: Fix build breakage with map_perf_test_user.c

[Alexei Starovoitov]

On 3/31/16 11:46 AM, Naveen N. Rao wrote:
>> It's failing this way on powerpc? Odd.
> This fails for me on x86_64 too -- RHEL 7.1.

indeed. fails on centos 7.1, whereas centos 6.7 is fine.

Re: [PATCH 3/4] samples/bpf: Simplify building BPF samples

[Alexei Starovoitov]

On 3/31/16 11:51 AM, Naveen N. Rao wrote:
> On 2016/03/31 10:49AM, Alexei Starovoitov wrote:
>> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
>>> Make BPF samples build depend on CONFIG_SAMPLE_BPF. We still don't add a
>>> Kconfig option since that will add a dependency on llvm for allyesconfig
>>> builds which may not be desirable.
>>>
>>> Those who need to build the BPF samples can now just do:
>>>
>>> make CONFIG_SAMPLE_BPF=y
>>>
>>> or:
>>>
>>> export CONFIG_SAMPLE_BPF=y
>>> make
>>
>> I don't like this 'simplification'.
>> make samples/bpf/
>> is much easier to type than capital letters.
>
> This started out as a patch to have the BPF samples built with a Kconfig
> option. As stated in the commit description, I realised that it won't
> work for allyesconfig builds. However, the reason I retained this patch
> is since it gets us one step closer to building the samples as part of
> the kernel build.
>
> The 'simplification' is since I can now have the export in my .bashrc
> and the kernel build will now build the BPF samples too without
> requiring an additional 'make samples/bpf/' step.
>
> I agree this is subjective, so I am ok if this isn't taken in.

If you can change it that 'make samples/bpf/' still works then it would
be fine. As it is it breaks our testing setup.

Re: [PATCH 2/4] samples/bpf: Use llc in PATH, rather than a hardcoded value

[Naveen N. Rao]

On 2016/03/31 08:19PM, Daniel Borkmann wrote:
> On 03/31/2016 07:46 PM, Alexei Starovoitov wrote:
> >On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> >>      clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
> >>          -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> >>-        -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
> >>+        -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=obj -o $@
> >>      clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
> >>          -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> >>-        -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=asm -o $@.s
> >>+        -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o $@.s
> >
> >that was a workaround when clang/llvm didn't have bpf support.
> >Now clang 3.7 and 3.8 have bpf built-in, so make sense to remove
> >manual calls to llc completely.
> >Just use 'clang -target bpf -O2 -D... -c $< -o $@'
> 
> +1, the clang part in that Makefile should also more correctly be called
> with '-target bpf' as it turns out (despite llc with '-march=bpf' ...).
> Better to use clang directly as suggested by Alexei.

I'm likely missing something obvious, but I cannot get this to work.  
With this diff:

	 $(obj)/%.o: $(src)/%.c
		clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
			-D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
	-               -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
	-       clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
	-               -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
	-               -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=asm -o $@.s
	+               -O2 -target bpf -c $< -o $@

I see far too many errors thrown starting with:

	clang  -nostdinc -isystem 
	/usr/lib/gcc/x86_64-redhat-linux/4.8.2/include 
	-I./arch/x86/include -Iarch/x86/include/generated/uapi 
	-Iarch/x86/include/generated  -Iinclude 
	-I./arch/x86/include/uapi -Iarch/x86/include/generated/uapi 
	-I./include/uapi -Iinclude/generated/uapi -include 
	./include/linux/kconfig.h  \
		-D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
		-O2 -target bpf -c samples/bpf/map_perf_test_kern.c -o samples/bpf/map_perf_test_kern.o
	In file included from samples/bpf/map_perf_test_kern.c:7:
	In file included from include/linux/skbuff.h:17:
	In file included from include/linux/kernel.h:10:
	In file included from include/linux/bitops.h:36:
	In file included from ./arch/x86/include/asm/bitops.h:500:
	./arch/x86/include/asm/arch_hweight.h:31:10: error: invalid output constraint '=a' in asm
			     : "="REG_OUT (res)
			       ^
	./arch/x86/include/asm/arch_hweight.h:59:10: error: invalid output constraint '=a' in asm
			     : "="REG_OUT (res)


What am I missing?


- Naveen

Re: [PATCH 4/4] samples/bpf: Enable powerpc support

[Naveen N. Rao]

On 2016/03/31 10:52AM, Alexei Starovoitov wrote:
> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> ...
> >+
> >+#ifdef __powerpc__
> >+#define BPF_KPROBE_READ_RET_IP(ip, ctx)		{ (ip) = (ctx)->link; }
> >+#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)	BPF_KPROBE_READ_RET_IP(ip, ctx)
> >+#else
> >+#define BPF_KPROBE_READ_RET_IP(ip, ctx)						\
> >+		bpf_probe_read(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx))
> >+#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)					\
> >+		bpf_probe_read(&(ip), sizeof(ip),				\
> >+				(void *)(PT_REGS_FP(ctx) + sizeof(ip)))
> 
> makes sense, but please use ({ }) gcc extension instead of {} and
> open call to make sure that macro body is scoped.

To be sure I understand this right, do you mean something like this?

+
+#ifdef __powerpc__
+#define BPF_KPROBE_READ_RET_IP(ip, ctx)                ({ (ip) = (ctx)->link; })
+#define BPF_KRETPROBE_READ_RET_IP              BPF_KPROBE_READ_RET_IP
+#else
+#define BPF_KPROBE_READ_RET_IP(ip, ctx)                ({                              \
+               bpf_probe_read(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx)); })
+#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)     ({                              \
+               bpf_probe_read(&(ip), sizeof(ip),                               \
+                               (void *)(PT_REGS_FP(ctx) + sizeof(ip))); })
+#endif
+


Thanks,
Naveen

Re: [PATCH 2/4] samples/bpf: Use llc in PATH, rather than a hardcoded value

[Alexei Starovoitov]

On 4/1/16 7:37 AM, Naveen N. Rao wrote:
> On 2016/03/31 08:19PM, Daniel Borkmann wrote:
>> On 03/31/2016 07:46 PM, Alexei Starovoitov wrote:
>>> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
>>>>       clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
>>>>           -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
>>>> -        -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
>>>> +        -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=obj -o $@
>>>>       clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
>>>>           -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
>>>> -        -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=asm -o $@.s
>>>> +        -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o $@.s
>>>
>>> that was a workaround when clang/llvm didn't have bpf support.
>>> Now clang 3.7 and 3.8 have bpf built-in, so make sense to remove
>>> manual calls to llc completely.
>>> Just use 'clang -target bpf -O2 -D... -c $< -o $@'
>>
>> +1, the clang part in that Makefile should also more correctly be called
>> with '-target bpf' as it turns out (despite llc with '-march=bpf' ...).
>> Better to use clang directly as suggested by Alexei.
>
> I'm likely missing something obvious, but I cannot get this to work.
> With this diff:
>
> 	 $(obj)/%.o: $(src)/%.c
> 		clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
> 			-D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> 	-               -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
> 	-       clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
> 	-               -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> 	-               -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=asm -o $@.s
> 	+               -O2 -target bpf -c $< -o $@
>
> I see far too many errors thrown starting with:
> 	./arch/x86/include/asm/arch_hweight.h:31:10: error: invalid output constraint '=a' in asm
> 			     : "="REG_OUT (res)

ahh. yes. when processing kernel headers clang has to assume x86 style
inline asm, though all of these functions will be ignored.
I don't have a quick fix for this yet.
Let's go back to your original change $(LLC)->llc

Re: [PATCH 4/4] samples/bpf: Enable powerpc support

[Alexei Starovoitov]

On 4/1/16 7:41 AM, Naveen N. Rao wrote:
> On 2016/03/31 10:52AM, Alexei Starovoitov wrote:
>> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
>> ...
>>> +
>>> +#ifdef __powerpc__
>>> +#define BPF_KPROBE_READ_RET_IP(ip, ctx)		{ (ip) = (ctx)->link; }
>>> +#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)	BPF_KPROBE_READ_RET_IP(ip, ctx)
>>> +#else
>>> +#define BPF_KPROBE_READ_RET_IP(ip, ctx)						\
>>> +		bpf_probe_read(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx))
>>> +#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)					\
>>> +		bpf_probe_read(&(ip), sizeof(ip),				\
>>> +				(void *)(PT_REGS_FP(ctx) + sizeof(ip)))
>>
>> makes sense, but please use ({ }) gcc extension instead of {} and
>> open call to make sure that macro body is scoped.
>
> To be sure I understand this right, do you mean something like this?
>
> +
> +#ifdef __powerpc__
> +#define BPF_KPROBE_READ_RET_IP(ip, ctx)                ({ (ip) = (ctx)->link; })
> +#define BPF_KRETPROBE_READ_RET_IP              BPF_KPROBE_READ_RET_IP
> +#else
> +#define BPF_KPROBE_READ_RET_IP(ip, ctx)                ({                              \
> +               bpf_probe_read(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx)); })
> +#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)     ({                              \
> +               bpf_probe_read(&(ip), sizeof(ip),                               \
> +                               (void *)(PT_REGS_FP(ctx) + sizeof(ip))); })
> +#endif

yes. Thanks!