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 <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Naveen N. Rao <[email protected]>
---
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
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 <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Naveen N. Rao <[email protected]>
---
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
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 <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Naveen N. Rao <[email protected]>
---
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
While at it, fix some typos in the comment.
Cc: Alexei Starovoitov <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Naveen N. Rao <[email protected]>
---
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 [email protected]
+ -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o [email protected]
--
2.7.4
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 <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Signed-off-by: Naveen N. Rao <[email protected]>
> ---
> 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 <[email protected]>
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...
On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> While at it, fix some typos in the comment.
>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Signed-off-by: Naveen N. Rao <[email protected]>
> ---
> 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 [email protected]
> + -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o [email protected]
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 $@'
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
>
>
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 <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Signed-off-by: Naveen N. Rao <[email protected]>
> ---
...
> +
> +#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.
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 <[email protected]>
>> Cc: David S. Miller <[email protected]>
>> Cc: Ananth N Mavinakayanahalli <[email protected]>
>> Cc: Michael Ellerman <[email protected]>
>> Signed-off-by: Naveen N. Rao <[email protected]>
>> ---
>> 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 [email protected]
>> + -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o [email protected]
>
> 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.
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 <[email protected]>
> >Cc: David S. Miller <[email protected]>
> >Cc: Ananth N Mavinakayanahalli <[email protected]>
> >Cc: Michael Ellerman <[email protected]>
> >Signed-off-by: Naveen N. Rao <[email protected]>
> >---
> > 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 <[email protected]>
>
> 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
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
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.
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.
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 [email protected]
> >>+ -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o [email protected]
> >
> >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 [email protected]
+ -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
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
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 [email protected]
>>>> + -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o [email protected]
>>>
>>> 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 [email protected]
> + -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
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!