On 2/9/22 1:53 PM, Andrii Nakryiko wrote: > On Mon, Feb 7, 2022 at 1:58 PM Andrii Nakryiko > <andrii.nakryiko@xxxxxxxxx> wrote: >> >> On Mon, Feb 7, 2022 at 6:31 AM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote: >>> ... >> > > Now that Ilya's fixes are in again, added a small note about reliance > on BPF CO-RE and pushed to bpf-next, thanks. > > Thanks. > On a relevant note. The whole __x64_sys_close vs sys_close depending > on architecture and kernel version was always super annoying. BCC > makes this transparent to users (AFAIK) and it always bothered me a BCC does this by 'guessing' the syscall prefix ([0]). [0]: https://github.com/iovisor/bcc/blob/v0.24.0/src/python/bcc/__init__.py#L787-L794 > little, but I didn't see a clean solution that fits libbpf. > > I think I finally found it, though. Instead of guessing whether the > kprobe function is a syscall or not based on "sys_" prefix of a kernel > function, we can use libbpf SEC() handling to do this transparently. > What if we define two new SEC() definitions: > > SEC("ksyscall/write") and SEC("kretsyscall/write") (or maybe > SEC("kprobe.syscall/write") and SEC("kretprobe.syscall/write"), not > sure which one is better, voice your opinion, please). And for such ksyscall/kretsyscall is more succinct and in the same vein as kfunc/kretfunc. > special kprobes, libbpf will perform feature detection of this > ARCH_SYSCALL_WRAPPER (we'll need to see the best way to do this in a > simple and fast way, preferably without parsing kallsyms) and By detecting special structs like struct cpuinfo_x86/cpuinfo_arm64 in BTF ? In a word, I like the idea and it would make libbpf-based tools more portable. :) > depending on it substitute either sys_write (or should it be > __se_sys_write, according to Naveen) or __<arch>_sys_write. You get > the idea. > > I like that this is still explicit and in the spirit of libbpf, but > offloads the burden of knowing these intricate differences from users. > > Thoughts? > > >> Unfortunately we had to back out Ilya's patches with >> PT_REGS_SYSCALL_REGS() and PT_REGS_PARMx_CORE_SYSCALL(), so we'll need >> to wait a bit before merging this. >> >> >>> +#define BPF_KPROBE_SYSCALL(name, args...) \ >>> +name(struct pt_regs *ctx); \ >>> +static __attribute__((always_inline)) typeof(name(0)) \ >>> +____##name(struct pt_regs *ctx, ##args); \ >>> +typeof(name(0)) name(struct pt_regs *ctx) \ >>> +{ \ >>> + struct pt_regs *regs = PT_REGS_SYSCALL_REGS(ctx); \ >>> + _Pragma("GCC diagnostic push") \ >>> + _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \ >>> + return ____##name(___bpf_syscall_args(args)); \ >>> + _Pragma("GCC diagnostic pop") \ >>> +} \ >>> +static __attribute__((always_inline)) typeof(name(0)) \ >>> +____##name(struct pt_regs *ctx, ##args) >>> + >>> #endif >>> -- >>> 2.30.2 -- Hengqi
