> On Oct 1, 2019, at 3:42 PM, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Tue, Oct 1, 2019 at 2:46 PM Song Liu <songliubraving@xxxxxx> wrote: >> >> >> >>> On Oct 1, 2019, at 2:25 PM, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: >>> >>> On Tue, Oct 1, 2019 at 2:14 PM Song Liu <songliubraving@xxxxxx> wrote: >>>> >>>> >>>>> On Sep 30, 2019, at 11:58 AM, Andrii Nakryiko <andriin@xxxxxx> wrote: >>>>> >>>>> Add few macros simplifying BCC-like multi-level probe reads, while also >>>>> emitting CO-RE relocations for each read. >>>>> >>>>> Signed-off-by: Andrii Nakryiko <andriin@xxxxxx> >>>>> --- >>>>> tools/lib/bpf/bpf_helpers.h | 151 +++++++++++++++++++++++++++++++++++- >>>>> 1 file changed, 147 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h >>>>> index a1d9b97b8e15..51e7b11d53e8 100644 >>>>> --- a/tools/lib/bpf/bpf_helpers.h >>>>> +++ b/tools/lib/bpf/bpf_helpers.h >>>>> @@ -19,6 +19,10 @@ >>>>> */ >>>>> #define SEC(NAME) __attribute__((section(NAME), used)) >>>>> >>>>> +#ifndef __always_inline >>>>> +#define __always_inline __attribute__((always_inline)) >>>>> +#endif >>>>> + >>>>> /* helper functions called from eBPF programs written in C */ >>>>> static void *(*bpf_map_lookup_elem)(void *map, const void *key) = >>>>> (void *) BPF_FUNC_map_lookup_elem; >>>>> @@ -505,7 +509,7 @@ struct pt_regs; >>>>> #endif >>>>> >>>>> /* >>>>> - * BPF_CORE_READ abstracts away bpf_probe_read() call and captures offset >>>>> + * bpf_core_read() abstracts away bpf_probe_read() call and captures field >>>>> * relocation for source address using __builtin_preserve_access_index() >>>>> * built-in, provided by Clang. >>>>> * >>>>> @@ -520,8 +524,147 @@ struct pt_regs; >>>>> * actual field offset, based on target kernel BTF type that matches original >>>>> * (local) BTF, used to record relocation. >>>>> */ >>>>> -#define BPF_CORE_READ(dst, src) \ >>>>> - bpf_probe_read((dst), sizeof(*(src)), \ >>>>> - __builtin_preserve_access_index(src)) >>>>> +#define bpf_core_read(dst, sz, src) \ >>>>> + bpf_probe_read(dst, sz, \ >>>>> + (const void *)__builtin_preserve_access_index(src)) >>>>> + >>>>> +/* >>>>> + * bpf_core_read_str() is a thin wrapper around bpf_probe_read_str() >>>>> + * additionally emitting BPF CO-RE field relocation for specified source >>>>> + * argument. >>>>> + */ >>>>> +#define bpf_core_read_str(dst, sz, src) \ >>>>> + bpf_probe_read_str(dst, sz, \ >>>>> + (const void *)__builtin_preserve_access_index(src)) >>>>> + >>>>> +#define ___concat(a, b) a ## b >>>>> +#define ___apply(fn, n) ___concat(fn, n) >>>>> +#define ___nth(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, __11, N, ...) N >>>> >>>> We are adding many marcos with simple names: ___apply(), ___nth. So I worry >>>> they may conflict with macro definitions from other libraries. Shall we hide >>>> them in .c files or prefix/postfix them with _libbpf or something? >>> >>> Keep in mind, this is the header that's included from BPF code. >>> >>> They are prefixed with three underscores, I was hoping it's good >>> enough to avoid accidental conflicts. It's unlikely someone will have >>> macros with the same names **in BPF-side code**. >> >> BPF side code would include kernel headers. So there are many headers >> to conflict with. And we won't know until somebody want to trace certain >> kernel structure. > > We have all the kernel sources at our disposal, there's no need to > guess :) There is no instance of ___apply, ___concat, ___nth, > ___arrow, ___last, ___nolast, or ___type, not even speaking about > other more specific names. There are currently two instances of > "____last_____" used in a string. And I'm certainly not afraid that > user code can use triple-underscored identifier with exact those names > and complain about bpf_helpers.h :) I worry more about _future_ conflicts, that someone may add ___apply to some kernel header file and break some BPF programs. Since these BPF programs are not in-tree, it is very difficult to test them properly. We have had name conflicts from other libraries, so I hope we don't create more ourselves. Thanks, Song
