[1/3] bpf: support writable context for bare tracepoint

Message ID 20210916135511.3787194-2-houtao1@huawei.com
State New
Headers show
Series
  • [1/3] bpf: support writable context for bare tracepoint
Related show

Commit Message

Hou Tao Sept. 16, 2021, 1:55 p.m.
Commit 9df1c28bb752 ("bpf: add writable context for raw tracepoints")
supports writable context for tracepoint, but it misses the support
for bare tracepoint which has no associated trace event.

Bare tracepoint is defined by DECLARE_TRACE(), so adding a corresponding
DECLARE_TRACE_WRITABLE() macro to generate a definition in __bpf_raw_tp_map
section for bare tracepoint in a similar way to DEFINE_TRACE_WRITABLE().

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 include/trace/bpf_probe.h | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

Comments

Hou Tao Sept. 17, 2021, 1:45 p.m. | #1
Hi,

On 9/17/2021 7:16 AM, Yonghong Song wrote:
>

>

> On 9/16/21 6:55 AM, Hou Tao wrote:

>> Commit 9df1c28bb752 ("bpf: add writable context for raw tracepoints")

>> supports writable context for tracepoint, but it misses the support

>> for bare tracepoint which has no associated trace event.

>>

>> Bare tracepoint is defined by DECLARE_TRACE(), so adding a corresponding

>> DECLARE_TRACE_WRITABLE() macro to generate a definition in __bpf_raw_tp_map

>> section for bare tracepoint in a similar way to DEFINE_TRACE_WRITABLE().

>>

>> Signed-off-by: Hou Tao <houtao1@huawei.com>

>> ---

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

>>   1 file changed, 15 insertions(+), 4 deletions(-)

>>

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

>> index a23be89119aa..d08ee1060d82 100644

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

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

>> @@ -93,8 +93,7 @@ __section("__bpf_raw_tp_map") = {                    \

>>     #define FIRST(x, ...) x

>>   -#undef DEFINE_EVENT_WRITABLE

>> -#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size)    \

>> +#define __CHECK_WRITABLE_BUF_SIZE(call, proto, args, size)        \

>>   static inline void bpf_test_buffer_##call(void)                \

>>   {                                    \

>>       /* BUILD_BUG_ON() is ignored if the code is completely eliminated, but \

>> @@ -103,8 +102,12 @@ static inline void

>> bpf_test_buffer_##call(void)                \

>>        */                                \

>>       FIRST(proto);                            \

>>       (void)BUILD_BUG_ON_ZERO(size != sizeof(*FIRST(args)));        \

>> -}                                    \

>> -__DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)

>> +}

>> +

>> +#undef DEFINE_EVENT_WRITABLE

>> +#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \

>> +    __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \

>> +    __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)

>>     #undef DEFINE_EVENT

>>   #define DEFINE_EVENT(template, call, proto, args)            \

>> @@ -119,10 +122,18 @@ __DEFINE_EVENT(template, call, PARAMS(proto),

>> PARAMS(args), size)

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

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

>>   +#undef DECLARE_TRACE_WRITABLE

>> +#define DECLARE_TRACE_WRITABLE(call, proto, args, size) \

>> +    __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \

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

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

>> +

>>   #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)

>>     #undef DEFINE_EVENT_WRITABLE

>> +#undef DECLARE_TRACE_WRITABLE

>>   #undef __DEFINE_EVENT

>> +#undef __CHECK_WRITABLE_BUF_SIZE

>

> Put "#undef __CHECK_WRITABLE_BUF_SIZE" right after "#undef

> DECLARE_TRACE_WRITABLE" since they are related to each other

> and also they are in correct reverse order w.r.t. __DEFINE_EVENT?

If considering __CHECK_WRITABLE_BUF_SIZE is used in both DECLARE_TRACE_WRITABLE and
DEFINE_EVENT_WRITABLE and the order of definitions, is the following order better ?

#undef DECLARE_TRACE_WRITABLE
#undef DEFINE_EVENT_WRITABLE
#undef __CHECK_WRITABLE_BUF_SIZE

>

>>   #undef FIRST

>>     #endif /* CONFIG_BPF_EVENTS */

>>

> .
Yonghong Song Sept. 17, 2021, 2:48 p.m. | #2
On 9/17/21 6:45 AM, Hou Tao wrote:
> Hi,

> 

> On 9/17/2021 7:16 AM, Yonghong Song wrote:

>>

>>

>> On 9/16/21 6:55 AM, Hou Tao wrote:

>>> Commit 9df1c28bb752 ("bpf: add writable context for raw tracepoints")

>>> supports writable context for tracepoint, but it misses the support

>>> for bare tracepoint which has no associated trace event.

>>>

>>> Bare tracepoint is defined by DECLARE_TRACE(), so adding a corresponding

>>> DECLARE_TRACE_WRITABLE() macro to generate a definition in __bpf_raw_tp_map

>>> section for bare tracepoint in a similar way to DEFINE_TRACE_WRITABLE().

>>>

>>> Signed-off-by: Hou Tao <houtao1@huawei.com>

>>> ---

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

>>>    1 file changed, 15 insertions(+), 4 deletions(-)

>>>

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

>>> index a23be89119aa..d08ee1060d82 100644

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

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

>>> @@ -93,8 +93,7 @@ __section("__bpf_raw_tp_map") = {                    \

>>>      #define FIRST(x, ...) x

>>>    -#undef DEFINE_EVENT_WRITABLE

>>> -#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size)    \

>>> +#define __CHECK_WRITABLE_BUF_SIZE(call, proto, args, size)        \

>>>    static inline void bpf_test_buffer_##call(void)                \

>>>    {                                    \

>>>        /* BUILD_BUG_ON() is ignored if the code is completely eliminated, but \

>>> @@ -103,8 +102,12 @@ static inline void

>>> bpf_test_buffer_##call(void)                \

>>>         */                                \

>>>        FIRST(proto);                            \

>>>        (void)BUILD_BUG_ON_ZERO(size != sizeof(*FIRST(args)));        \

>>> -}                                    \

>>> -__DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)

>>> +}

>>> +

>>> +#undef DEFINE_EVENT_WRITABLE

>>> +#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \

>>> +    __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \

>>> +    __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)

>>>      #undef DEFINE_EVENT

>>>    #define DEFINE_EVENT(template, call, proto, args)            \

>>> @@ -119,10 +122,18 @@ __DEFINE_EVENT(template, call, PARAMS(proto),

>>> PARAMS(args), size)

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

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

>>>    +#undef DECLARE_TRACE_WRITABLE

>>> +#define DECLARE_TRACE_WRITABLE(call, proto, args, size) \

>>> +    __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \

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

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

>>> +

>>>    #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)

>>>      #undef DEFINE_EVENT_WRITABLE

>>> +#undef DECLARE_TRACE_WRITABLE

>>>    #undef __DEFINE_EVENT

>>> +#undef __CHECK_WRITABLE_BUF_SIZE

>>

>> Put "#undef __CHECK_WRITABLE_BUF_SIZE" right after "#undef

>> DECLARE_TRACE_WRITABLE" since they are related to each other

>> and also they are in correct reverse order w.r.t. __DEFINE_EVENT?

> If considering __CHECK_WRITABLE_BUF_SIZE is used in both DECLARE_TRACE_WRITABLE and

> DEFINE_EVENT_WRITABLE and the order of definitions, is the following order better ?

> 

> #undef DECLARE_TRACE_WRITABLE

> #undef DEFINE_EVENT_WRITABLE

> #undef __CHECK_WRITABLE_BUF_SIZE


This should be okay.

> 

>>

>>>    #undef FIRST

>>>      #endif /* CONFIG_BPF_EVENTS */

>>>

>> .

>

Patch

diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
index a23be89119aa..d08ee1060d82 100644
--- a/include/trace/bpf_probe.h
+++ b/include/trace/bpf_probe.h
@@ -93,8 +93,7 @@  __section("__bpf_raw_tp_map") = {					\
 
 #define FIRST(x, ...) x
 
-#undef DEFINE_EVENT_WRITABLE
-#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size)	\
+#define __CHECK_WRITABLE_BUF_SIZE(call, proto, args, size)		\
 static inline void bpf_test_buffer_##call(void)				\
 {									\
 	/* BUILD_BUG_ON() is ignored if the code is completely eliminated, but \
@@ -103,8 +102,12 @@  static inline void bpf_test_buffer_##call(void)				\
 	 */								\
 	FIRST(proto);							\
 	(void)BUILD_BUG_ON_ZERO(size != sizeof(*FIRST(args)));		\
-}									\
-__DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
+}
+
+#undef DEFINE_EVENT_WRITABLE
+#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
+	__CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
+	__DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
 
 #undef DEFINE_EVENT
 #define DEFINE_EVENT(template, call, proto, args)			\
@@ -119,10 +122,18 @@  __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
 	__BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))		\
 	__DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), 0)
 
+#undef DECLARE_TRACE_WRITABLE
+#define DECLARE_TRACE_WRITABLE(call, proto, args, size) \
+	__CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
+	__BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
+	__DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), size)
+
 #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
 
 #undef DEFINE_EVENT_WRITABLE
+#undef DECLARE_TRACE_WRITABLE
 #undef __DEFINE_EVENT
+#undef __CHECK_WRITABLE_BUF_SIZE
 #undef FIRST
 
 #endif /* CONFIG_BPF_EVENTS */