On Thu, Jun 10, 2021 at 9:10 AM Lorenz Bauer <lmb@xxxxxxxxxxxxxx> wrote:
>
> bpf2go is the Go equivalent of libbpf skeleton. The convention is that
> the compiled BPF is checked into the repository to facilitate distributing
> BPF as part of Go packages. To make this portable, bpf2go by default
> generates both bpfel and bpfeb variants of the C.
>
> Using bpf_tracing.h is inherently non-portable since the fields of
> struct pt_regs differ between platforms, so CO-RE can't help us here.
> The only way of working around this is to compile for each target
> platform independently. bpf2go can't do this by default since there
> are too many platforms.
>
> Define the various PT_... macros when no target can be determined and
> turn them into compilation failures. This works because bpf2go always
> compiles for bpf targets, so the compiler fallback doesn't kick in.
> Conditionally define __bpf_missing_target so that we can inject a
> more appropriate error message at build time. The user can then
> choose which platform to target explicitly.
>
> Signed-off-by: Lorenz Bauer <lmb@xxxxxxxxxxxxxx>
> ---
> tools/lib/bpf/bpf_tracing.h | 46 +++++++++++++++++++++++++++++++++----
> 1 file changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index c0f3a26aa582..438174adb3f8 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -25,26 +25,35 @@
> #define bpf_target_sparc
> #define bpf_target_defined
> #else
> - #undef bpf_target_defined
> -#endif
>
> /* Fall back to what the compiler says */
> -#ifndef bpf_target_defined
Hm... doesn't this auto-guessing based on host architecture defeats
your goal? You don't want bpf_tracing.h to guess the right set of
PT_REGS macros, no?
I thought you'll do something like
#ifndef bpf_target_guess
#define bpf_target_guess 1
#endif
#if !defined(bpf_target_defined) && bpf_target_guess
/* then try to use host architecture */
But I guess I'm missing something...
> #if defined(__x86_64__)
> #define bpf_target_x86
> + #define bpf_target_defined
> #elif defined(__s390__)
> #define bpf_target_s390
> + #define bpf_target_defined
btw, instead of having this zoo of bpf_target_<arch> and also
bpf_traget_defined, how about simplifying it to a single variable that
would contain the actual architecture:
#define BPF_TARGET_ARCH "s390"
And then do
#if BPF_TARGET_ARCH == "s390"
#elif BPF_TARGET_ARCH == "arm"
...
#else /* unknown bpf_target_arch or not defined */
_Pragma(...)
#endif
WDYT? We can eventually move away from weird-looking __TARGET_ARCH_x86
to just -DBPF_TARGET_ARCH=x86. We'll need to support __TARGET_ARCH_xxx
for backwards compatibility, but at least new use cases can be cleaner
and more meaningful.
> #elif defined(__arm__)
> #define bpf_target_arm
> + #define bpf_target_defined
> #elif defined(__aarch64__)
> #define bpf_target_arm64
> + #define bpf_target_defined
> #elif defined(__mips__)
> #define bpf_target_mips
> + #define bpf_target_defined
> #elif defined(__powerpc__)
> #define bpf_target_powerpc
> + #define bpf_target_defined
> #elif defined(__sparc__)
> #define bpf_target_sparc
> + #define bpf_target_defined
> +#endif /* no compiler target */
> +
> #endif
> +
> +#ifndef __bpf_target_missing
> +#define __bpf_target_missing "GCC error \"Must specify a target arch via __TARGET_ARCH_xxx\""
If you goal is to customize the error message, why not parameterize
the error message part only, not the "GCC error \"\"" part?
> #endif
>
> #if defined(bpf_target_x86)
> @@ -287,7 +296,7 @@ struct pt_regs;
> #elif defined(bpf_target_sparc)
> #define BPF_KPROBE_READ_RET_IP(ip, ctx) ({ (ip) = PT_REGS_RET(ctx); })
> #define BPF_KRETPROBE_READ_RET_IP BPF_KPROBE_READ_RET_IP
> -#else
> +#elif defined(bpf_target_defined)
> #define BPF_KPROBE_READ_RET_IP(ip, ctx) \
> ({ bpf_probe_read_kernel(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx)); })
> #define BPF_KRETPROBE_READ_RET_IP(ip, ctx) \
> @@ -295,6 +304,35 @@ struct pt_regs;
> (void *)(PT_REGS_FP(ctx) + sizeof(ip))); })
> #endif
>
> +#if !defined(bpf_target_defined)
> +
> +#define PT_REGS_PARM1(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_PARM2(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_PARM3(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_PARM4(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_PARM5(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_RET(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_FP(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_RC(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_SP(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_IP(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +
> +#define PT_REGS_PARM1_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_PARM2_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_PARM3_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_PARM4_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_PARM5_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_RET_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_FP_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_RC_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_SP_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define PT_REGS_IP_CORE(x) ({ _Pragma(__bpf_target_missing); 0ull; })
nit: why ull suffix?
> +
> +#define BPF_KPROBE_READ_RET_IP(ip, ctx) ({ _Pragma(__bpf_target_missing); 0ull; })
> +#define BPF_KRETPROBE_READ_RET_IP(ip, ctx) ({ _Pragma(__bpf_target_missing); 0ull; })
> +
> +#endif /* !defined(bpf_target_defined) */
> +
> #ifndef ___bpf_concat
> #define ___bpf_concat(a, b) a ## b
> #endif
> --
> 2.30.2
>
