Skip to contentSkip to conversation

bpf: make tracing helpers gpl only

PATCH Applied Alexei Starovoitov 3 participants 4 messages Oct 23, 2015 – Oct 27, 2015

Subsystems (6)
Networking Network drivers BPF Tracing +2 more subsystems
lore
Share & tools
lore

Alexei Starovoitov

author
#0 lore raw

PATCH 1/1 bpf: make tracing helpers gpl only

exported perf symbols are GPL only, mark eBPF helper functions used in tracing as GPL only as well.

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

---
this is on top of this patch that touches the same file:
http://patchwork.ozlabs.org/patch/534664/

 kernel/trace/bpf_trace.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/trace/bpf_trace.c +2 -2
index 003df3887287..4228fd3682c3 100644
@@ -214,7 +214,7 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5)
 
 static const struct bpf_func_proto bpf_perf_event_read_proto = {
 	.func		= bpf_perf_event_read,
-	.gpl_only	= false,
+	.gpl_only	= true,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_CONST_MAP_PTR,
 	.arg2_type	= ARG_ANYTHING,

@@ -255,7 +255,7 @@ static u64 bpf_perf_event_output(u64 r1, u64 r2, u64 index, u64 r4, u64 size)
 
 static const struct bpf_func_proto bpf_perf_event_output_proto = {
 	.func		= bpf_perf_event_output,
-	.gpl_only	= false,
+	.gpl_only	= true,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_CONST_MAP_PTR,
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 26 lines · Alexei Starovoitov

exported perf symbols are GPL only, mark eBPF helper functions used in tracing as GPL only as well.

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

---
this is on top of this patch that touches the same file:
http://patchwork.ozlabs.org/patch/534664/

 kernel/trace/bpf_trace.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/trace/bpf_trace.c +1 -1
index 003df3887287..4228fd3682c3 100644
@@ -214,7 +214,7 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5)
 
 static const struct bpf_func_proto bpf_perf_event_read_proto = {
 	.func		= bpf_perf_event_read,
-	.gpl_only	= false,
+	.gpl_only	= true,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_CONST_MAP_PTR,
 	.arg2_type	= ARG_ANYTHING,

kernel/trace/bpf_trace.c static u64 bpf_perf_event_output(u64 r1, u64 r2, u64 index, u64 r4, u64 size) · line 261

Quoted 8 lines
kernel/trace/bpf_trace.c +1 -1
@@ -255,7 +255,7 @@ static u64 bpf_perf_event_output(u64 r1, u64 r2, u64 index, u64 r4, u64 size)
 
 static const struct bpf_func_proto bpf_perf_event_output_proto = {
 	.func		= bpf_perf_event_output,
-	.gpl_only	= false,
+	.gpl_only	= true,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_CONST_MAP_PTR,

So there's a lot more 'gpl_only = false' cases that I can see:

triton:~/tip> git grep gpl_only | grep false
kernel/bpf/core.c: .gpl_only = false,
kernel/bpf/helpers.c: .gpl_only = false,
kernel/bpf/helpers.c: .gpl_only = false,
kernel/bpf/helpers.c: .gpl_only = false,
kernel/bpf/helpers.c: .gpl_only = false,
kernel/bpf/helpers.c: .gpl_only = false,
kernel/bpf/helpers.c: .gpl_only = false,
kernel/bpf/helpers.c: .gpl_only = false,
kernel/bpf/helpers.c: .gpl_only = false,
kernel/trace/bpf_trace.c: .gpl_only = false,
net/core/filter.c: .gpl_only = false,
net/core/filter.c: .gpl_only = false,
net/core/filter.c: .gpl_only = false,
net/core/filter.c: .gpl_only = false,
net/core/filter.c: .gpl_only = false,
net/core/filter.c: .gpl_only = false,
net/core/filter.c: .gpl_only = false,
net/core/filter.c: .gpl_only = false,
net/core/filter.c: .gpl_only = false,

and only a handful of 'true' cases:

triton:~/tip> git grep gpl_only | grep true

kernel/bpf/helpers.c: .gpl_only = true,
kernel/trace/bpf_trace.c:       .gpl_only   = true,
kernel/trace/bpf_trace.c:       .gpl_only   = true,

so I don't know the netfilter ones, but in particular the tracing ones I'm sure should be gpl-only. Do we really, really want proprietary modules mucking with such internals??

Thanks,

Ingo

author
#2 lore raw
Quoted 3 lines · Ingo Molnar

so I don't know the netfilter ones, but in particular the tracing ones I'm sure should be gpl-only. Do we really, really want proprietary modules mucking with such internals??

of course, not. Therefore bpf_probe_read() was gpl_only from very beginning. Same goes to bpf_trace_printk() and bpf_ktime_get_ns(). and after this patch the only remaining non-gpl tracing helpers will be bpf_get_current_pid_tgid()/uid_gid()/comm()
and imo it's fine.

On the networking side the helpers are non-gpl, since all of them are generic. They don't mess with kernel, but examine/modify the packet. Just like cBPF can read the packet data, get_processor_id, get_random, eBPF can do the same plus write into the packet, compute checksum, redirect, push/pop vlan.

When we introduce eBPF for seccomp, I would argue that helpers on that side should be non-gpl as well, otherwise people will try to add features to cBPF instead of moving to eBPF.