[bpf-next,0/2] Allow attaching to bare tracepoints

Add some missing glue logic to teach bpf about bare tracepoints - tracepoints
without any trace event associated with them.

Bare tracepoints are declare with DECLARE_TRACE(). Full tracepoints are declare
with TRACE_EVENT().

BPF can attach to these tracepoints as RAW_TRACEPOINT() only as there's no
events in tracefs created with them.

Qais Yousef (2):
  trace: bpf: Allow bpf to attach to bare tracepoints
  selftests: bpf: Add a new test for bare tracepoints

 Documentation/bpf/bpf_design_QA.rst                  |  6 ++++++
 include/trace/bpf_probe.h                            | 12 ++++++++++--
 .../selftests/bpf/bpf_testmod/bpf_testmod-events.h   |  6 ++++++
 .../testing/selftests/bpf/bpf_testmod/bpf_testmod.c  |  2 ++
 .../testing/selftests/bpf/prog_tests/module_attach.c |  1 +
 .../testing/selftests/bpf/progs/test_module_attach.c | 10 ++++++++++
 6 files changed, 35 insertions(+), 2 deletions(-)

On 1/11/21 10:20 AM, Qais Yousef wrote:
> Some subsystems only have bare tracepoints (a tracepoint with no

> associated trace event) to avoid the problem of trace events being an

> ABI that can't be changed.

> 

>  From bpf presepective, bare tracepoints are what it calls

> RAW_TRACEPOINT().

> 

> Since bpf assumed there's 1:1 mapping, it relied on hooking to

> DEFINE_EVENT() macro to create bpf mapping of the tracepoints. Since

> bare tracepoints use DECLARE_TRACE() to create the tracepoint, bpf had

> no knowledge about their existence.

> 

> By teaching bpf_probe.h to parse DECLARE_TRACE() in a similar fashion to

> DEFINE_EVENT(), bpf can find and attach to the new raw tracepoints.

> 

> Enabling that comes with the contract that changes to raw tracepoints

> don't constitute a regression if they break existing bpf programs.

> We need the ability to continue to morph and modify these raw

> tracepoints without worrying about any ABI.

> 

> Update Documentation/bpf/bpf_design_QA.rst to document this contract.

> 

> Signed-off-by: Qais Yousef <qais.yousef@arm.com>

> ---

>   Documentation/bpf/bpf_design_QA.rst |  6 ++++++

>   include/trace/bpf_probe.h           | 12 ++++++++++--

>   2 files changed, 16 insertions(+), 2 deletions(-)

> 

> diff --git a/Documentation/bpf/bpf_design_QA.rst b/Documentation/bpf/bpf_design_QA.rst

> index 2df7b067ab93..0e15f9b05c9d 100644

> --- a/Documentation/bpf/bpf_design_QA.rst

> +++ b/Documentation/bpf/bpf_design_QA.rst

> @@ -208,6 +208,12 @@ data structures and compile with kernel internal headers. Both of these

>   kernel internals are subject to change and can break with newer kernels

>   such that the program needs to be adapted accordingly.

>   

> +Q: Are tracepoints part of the stable ABI?

> +------------------------------------------

> +A: NO. Tracepoints are tied to internal implementation details hence they are

> +subject to change and can break with newer kernels. BPF programs need to change

> +accordingly when this happens.

> +

>   Q: How much stack space a BPF program uses?

>   -------------------------------------------

>   A: Currently all program types are limited to 512 bytes of stack

> diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h

> index cd74bffed5c6..cf1496b162b1 100644

> --- a/include/trace/bpf_probe.h

> +++ b/include/trace/bpf_probe.h

> @@ -55,8 +55,7 @@

>   /* tracepoints with more than 12 arguments will hit build error */

>   #define CAST_TO_U64(...) CONCATENATE(__CAST, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)

>   

> -#undef DECLARE_EVENT_CLASS

> -#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\

> +#define __BPF_DECLARE_TRACE(call, proto, args)				\

>   static notrace void							\

>   __bpf_trace_##call(void *__data, proto)					\

>   {									\

> @@ -64,6 +63,10 @@ __bpf_trace_##call(void *__data, proto)					\

>   	CONCATENATE(bpf_trace_run, COUNT_ARGS(args))(prog, CAST_TO_U64(args));	\

>   }

>   

> +#undef DECLARE_EVENT_CLASS

> +#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\

> +	__BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))

> +

>   /*

>    * This part is compiled out, it is only here as a build time check

>    * to make sure that if the tracepoint handling changes, the

> @@ -111,6 +114,11 @@ __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)

>   #define DEFINE_EVENT_PRINT(template, name, proto, args, print)	\

>   	DEFINE_EVENT(template, name, PARAMS(proto), PARAMS(args))

>   

> +#undef DECLARE_TRACE

> +#define DECLARE_TRACE(call, proto, args)				\

> +	(__BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))		\

> +	 __DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), 0))


I applied the patch to my local bpf-next repo, and got the following
compilation error:

In file included from 
/data/users/yhs/work/net-next/include/trace/define_trace.h:104, 

                  from 
/data/users/yhs/work/net-next/include/trace/events/sched.h:740, 

                  from 
/data/users/yhs/work/net-next/kernel/sched/core.c:10: 

/data/users/yhs/work/net-next/include/trace/bpf_probe.h:59:1: error: 
expected identifier or ‘(’ before ‘static’
  static notrace void       \ 

  ^~~~~~ 

/data/users/yhs/work/net-next/include/trace/bpf_probe.h:119:3: note: in 
expansion of macro ‘__BPF_DECLARE_TRACE’
   (__BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))  \ 

    ^~~~~~~~~~~~~~~~~~~ 

/data/users/yhs/work/net-next/include/trace/events/sched.h:693:1: note: 
in expansion of macro ‘DECLARE_TRACE’
  DECLARE_TRACE(pelt_cfs_tp, 

  ^~~~~~~~~~~~~ 

/data/users/yhs/work/net-next/include/trace/bpf_probe.h:59:1: error: 
expected identifier or ‘(’ before ‘static’
  static notrace void       \ 

  ^~~~~~ 

/data/users/yhs/work/net-next/include/trace/bpf_probe.h:119:3: note: in 
expansion of macro ‘__BPF_DECLARE_TRACE’
   (__BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))  \ 

    ^~~~~~~~~~~~~~~~~~~ 

/data/users/yhs/work/net-next/include/trace/events/sched.h:697:1: note: 
in expansion of macro ‘DECLARE_TRACE’
  DECLARE_TRACE(pelt_rt_tp, 

  ^~~~~~~~~~~~~
/data/users/yhs/work/net-next/include/trace/bpf_probe.h:59:1: error: 
expected identifier or ‘(’ before ‘static’
  static notrace void       \

I dumped preprecessor result but after macro expansion, the code
becomes really complex and I have not figured out why it failed.
Do you know what is the possible reason?

> +

>   #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)

>   

>   #undef DEFINE_EVENT_WRITABLE

>

On 01/12/21 12:19, Yonghong Song wrote:
> I applied the patch to my local bpf-next repo, and got the following

> compilation error:


[...]

> 

> I dumped preprecessor result but after macro expansion, the code

> becomes really complex and I have not figured out why it failed.

> Do you know what is the possible reason?


Yeah I did a last minute fix to address a checkpatch.pl error and my
verification of the change wasn't good enough obviously.

If you're keen to try out I can send you a patch with the fix. I should send v2
by the weekend too.

Thanks for having a look.

Cheers

--
Qais Yousef

On 1/13/21 2:16 AM, Qais Yousef wrote:
> On 01/12/21 12:19, Yonghong Song wrote:

>> I applied the patch to my local bpf-next repo, and got the following

>> compilation error:

> 

> [...]

> 

>>

>> I dumped preprecessor result but after macro expansion, the code

>> becomes really complex and I have not figured out why it failed.

>> Do you know what is the possible reason?

> 

> Yeah I did a last minute fix to address a checkpatch.pl error and my

> verification of the change wasn't good enough obviously.

> 

> If you're keen to try out I can send you a patch with the fix. I should send v2

> by the weekend too.


Thanks. I can wait and will check v2 once it is available.

> 

> Thanks for having a look.

> 

> Cheers

> 

> --

> Qais Yousef

>