From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:46732 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751600AbeCXA7F (ORCPT ); Fri, 23 Mar 2018 20:59:05 -0400 Subject: Re: [PATCH v2 bpf-next 5/8] bpf: introduce BPF_RAW_TRACEPOINT To: Daniel Borkmann , References: <20180321185448.2806324-1-ast@fb.com> <20180321185448.2806324-6-ast@fb.com> <580e6dcd-7deb-9fe2-5e37-6ce9ad0593a4@iogearbox.net> <79700812-58e2-2534-081e-015db6630155@fb.com> CC: , , , , , From: Alexei Starovoitov Message-ID: <50520d4a-1c07-9cca-068c-9bc737c7785f@fb.com> Date: Fri, 23 Mar 2018 17:58:11 -0700 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: On 3/23/18 4:13 PM, Daniel Borkmann wrote: > On 03/22/2018 04:41 PM, Alexei Starovoitov wrote: >> On 3/22/18 2:43 AM, Daniel Borkmann wrote: >>> On 03/21/2018 07:54 PM, Alexei Starovoitov wrote: >>> [...] >>>> @@ -546,6 +556,53 @@ extern void ftrace_profile_free_filter(struct perf_event *event); >>>> void perf_trace_buf_update(void *record, u16 type); >>>> void *perf_trace_buf_alloc(int size, struct pt_regs **regs, int *rctxp); >>>> >>>> +void bpf_trace_run1(struct bpf_prog *prog, u64 arg1); >>>> +void bpf_trace_run2(struct bpf_prog *prog, u64 arg1, u64 arg2); >>>> +void bpf_trace_run3(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3); >>>> +void bpf_trace_run4(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4); >>>> +void bpf_trace_run5(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5); >>>> +void bpf_trace_run6(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6); >>>> +void bpf_trace_run7(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7); >>>> +void bpf_trace_run8(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8); >>>> +void bpf_trace_run9(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9); >>>> +void bpf_trace_run10(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10); >>>> +void bpf_trace_run11(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11); >>>> +void bpf_trace_run12(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12); >>>> +void bpf_trace_run13(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12, >>>> + u64 arg13); >>>> +void bpf_trace_run14(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12, >>>> + u64 arg13, u64 arg14); >>>> +void bpf_trace_run15(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12, >>>> + u64 arg13, u64 arg14, u64 arg15); >>>> +void bpf_trace_run16(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12, >>>> + u64 arg13, u64 arg14, u64 arg15, u64 arg16); >>>> +void bpf_trace_run17(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12, >>>> + u64 arg13, u64 arg14, u64 arg15, u64 arg16, u64 arg17); >>>> void perf_trace_run_bpf_submit(void *raw_data, int size, int rctx, >>>> struct trace_event_call *call, u64 count, >>>> struct pt_regs *regs, struct hlist_head *head, >>> [...] >>>> @@ -896,3 +976,206 @@ int perf_event_query_prog_array(struct perf_event *event, void __user *info) >>>> >>>> return ret; >>>> } >>>> + >>>> +static __always_inline >>>> +void __bpf_trace_run(struct bpf_prog *prog, u64 *args) >>>> +{ >>>> + rcu_read_lock(); >>>> + preempt_disable(); >>>> + (void) BPF_PROG_RUN(prog, args); >>>> + preempt_enable(); >>>> + rcu_read_unlock(); >>>> +} >>>> + >>>> +#define EVAL1(FN, X) FN(X) >>>> +#define EVAL2(FN, X, Y...) FN(X) EVAL1(FN, Y) >>>> +#define EVAL3(FN, X, Y...) FN(X) EVAL2(FN, Y) >>>> +#define EVAL4(FN, X, Y...) FN(X) EVAL3(FN, Y) >>>> +#define EVAL5(FN, X, Y...) FN(X) EVAL4(FN, Y) >>>> +#define EVAL6(FN, X, Y...) FN(X) EVAL5(FN, Y) >>>> + >>>> +#define COPY(X) args[X - 1] = arg##X; >>>> + >>>> +void bpf_trace_run1(struct bpf_prog *prog, u64 arg1) >>>> +{ >>>> + u64 args[1]; >>>> + >>>> + EVAL1(COPY, 1); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run1); >>>> +void bpf_trace_run2(struct bpf_prog *prog, u64 arg1, u64 arg2) >>>> +{ >>>> + u64 args[2]; >>>> + >>>> + EVAL2(COPY, 1, 2); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run2); >>>> +void bpf_trace_run3(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3) >>>> +{ >>>> + u64 args[3]; >>>> + >>>> + EVAL3(COPY, 1, 2, 3); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run3); >>>> +void bpf_trace_run4(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4) >>>> +{ >>>> + u64 args[4]; >>>> + >>>> + EVAL4(COPY, 1, 2, 3, 4); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run4); >>>> +void bpf_trace_run5(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5) >>>> +{ >>>> + u64 args[5]; >>>> + >>>> + EVAL5(COPY, 1, 2, 3, 4, 5); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run5); >>>> +void bpf_trace_run6(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6) >>>> +{ >>>> + u64 args[6]; >>>> + >>>> + EVAL6(COPY, 1, 2, 3, 4, 5, 6); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run6); >>>> +void bpf_trace_run7(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7) >>>> +{ >>>> + u64 args[7]; >>>> + >>>> + EVAL6(COPY, 1, 2, 3, 4, 5, 6); >>>> + EVAL1(COPY, 7); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run7); >>>> +void bpf_trace_run8(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8) >>>> +{ >>>> + u64 args[8]; >>>> + >>>> + EVAL6(COPY, 1, 2, 3, 4, 5, 6); >>>> + EVAL2(COPY, 7, 8); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run8); >>>> +void bpf_trace_run9(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9) >>>> +{ >>>> + u64 args[9]; >>>> + >>>> + EVAL6(COPY, 1, 2, 3, 4, 5, 6); >>>> + EVAL3(COPY, 7, 8, 9); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run9); >>>> +void bpf_trace_run10(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10) >>>> +{ >>>> + u64 args[10]; >>>> + >>>> + EVAL6(COPY, 1, 2, 3, 4, 5, 6); >>>> + EVAL4(COPY, 7, 8, 9, 10); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run10); >>>> +void bpf_trace_run11(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11) >>>> +{ >>>> + u64 args[11]; >>>> + >>>> + EVAL6(COPY, 1, 2, 3, 4, 5, 6); >>>> + EVAL5(COPY, 7, 8, 9, 10, 11); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run11); >>>> +void bpf_trace_run12(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12) >>>> +{ >>>> + u64 args[12]; >>>> + >>>> + EVAL6(COPY, 1, 2, 3, 4, 5, 6); >>>> + EVAL6(COPY, 7, 8, 9, 10, 11, 12); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run12); >>>> +void bpf_trace_run17(struct bpf_prog *prog, u64 arg1, u64 arg2, >>>> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7, >>>> + u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12, >>>> + u64 arg13, u64 arg14, u64 arg15, u64 arg16, u64 arg17) >>>> +{ >>>> + u64 args[17]; >>>> + >>>> + EVAL6(COPY, 1, 2, 3, 4, 5, 6); >>>> + EVAL6(COPY, 7, 8, 9, 10, 11, 12); >>>> + EVAL5(COPY, 13, 14, 15, 16, 17); >>>> + __bpf_trace_run(prog, args); >>>> +} >>>> +EXPORT_SYMBOL_GPL(bpf_trace_run17); >>> >>> Would be nice if we could generate all these above via macro, e.g. when we define >>> a hard upper limit for max number of tracepoint args anyway, so this gets automatically >>> adjusted as well. Maybe some of the logic from BPF_CALL_*() macros could be borrowed >>> for this purpose. >> >> I've thought about it, but couldn't figure out how to do it. >> Suggestions are welcome. >> The preprocessor cannot expand a constant N into N statements. >> There gotta be something like: >> ... >> #define EVAL5(FN, X, Y...) FN(X) EVAL4(FN, Y) >> #define EVAL6(FN, X, Y...) FN(X) EVAL5(FN, Y) >> for whatever maximum we will pick. > > Right. > >> I picked 6 as a good compromise and used it twice in bpf_trace_run1x() >> Similar thing possible for u64 arg1, u64 arg2, ... >> but it will be harder to read. >> Looking forward what you can come up with. > > Just took a quick look, so the below one would work for generating the > signature and function. I did till 9 here: > > #define UNPACK(...) __VA_ARGS__ > #define REPEAT_1(FN, DL, X, ...) FN(X) > #define REPEAT_2(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_1(FN, DL, __VA_ARGS__) > #define REPEAT_3(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_2(FN, DL, __VA_ARGS__) > #define REPEAT_4(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_3(FN, DL, __VA_ARGS__) > #define REPEAT_5(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_4(FN, DL, __VA_ARGS__) > #define REPEAT_6(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_5(FN, DL, __VA_ARGS__) > #define REPEAT_7(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_6(FN, DL, __VA_ARGS__) > #define REPEAT_8(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_7(FN, DL, __VA_ARGS__) > #define REPEAT_9(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_8(FN, DL, __VA_ARGS__) > #define REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__) > > #define SARG(X) u64 arg##X > #define COPY(X) args[X] = arg##X > > #define __DL_COM (,) > #define __DL_SEM (;) > > #define __SEQ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 > > #define BPF_TRACE_DECL_x(x) \ > void bpf_trace_run##x(struct bpf_prog *prog, \ > REPEAT(x, SARG, __DL_COM, __SEQ)) > #define BPF_TRACE_DEFN_x(x) \ > void bpf_trace_run##x(struct bpf_prog *prog, \ > REPEAT(x, SARG, __DL_COM, __SEQ)) \ > { \ > u64 args[x]; \ > REPEAT(x, COPY, __DL_SEM, __SEQ); \ > __bpf_trace_run(prog, args); \ > } \ > EXPORT_SYMBOL_GPL(bpf_trace_run##x) > > So doing a ... > > BPF_TRACE_DECL_x(5); > BPF_TRACE_DEFN_x(5); interestingly that in addition to above defining #define __REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__) to allow recursive expansion and doing __REPEAT(12, BPF_TRACE_DECL_x, __DL_SEM, __SEQ_1_12); almost works... I'm guessing it's hitting preprocessor internal limit on number of expressions to expand. It expands 1-6 nicely and 7-12 are partially expanded :) I guess I have to use BPF_TRACE_DECL_x(1); BPF_TRACE_DECL_x(2); BPF_TRACE_DECL_x(3); BPF_TRACE_DECL_x(4); ... BPF_TRACE_DECL_x(12); which doesn't look better than open coding them. Only for BPF_TRACE_DEFN_x it's probably worth it. > ... will generate in kernel/trace/bpf_trace.i: > > void bpf_foo_trace_run5(struct bpf_prog *prog, u64 arg0 , u64 arg1 , u64 arg2 , u64 arg3 , u64 arg4); > void bpf_foo_trace_run5(struct bpf_prog *prog, u64 arg0 , u64 arg1 , u64 arg2 , u64 arg3 , u64 arg4) > { > u64 args[5]; > args[0] = arg0 ; > args[1] = arg1 ; > args[2] = arg2 ; > args[3] = arg3 ; > args[4] = arg4; > __bpf_trace_run(prog, args); > } [...] > > Meaning, the EVALx() macros could be removed from there, too. Potentially, the > REPEAT() macro could sit in its own include/linux/ header for others to reuse > or such. feels too specific for this use case. I'd wait second user before moving to include/linux/kernel.h