On Tue, Feb 1, 2022 at 11:32 AM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Mon, Jan 31, 2022 at 10:45:53PM -0800, Andrii Nakryiko wrote: > > > > As Jiri mentioned, for multi-attach kprobes the idea was to keep a > > sorted array of ips and associated cookies to do log(N) search in > > bpf_get_attach_cookie() helper. > > > > For multi-attach fentry, we could use the same approach if we let > > either bpf_link or bpf_prog available to fentry/fexit program at > > runtime. > > Makes sense to me. > It's probably better to land multi-attach kprobe and fentry first, > so we don't need to refactor trampolines once again. > iirc the trampolines were not easy to refactor for Jiri. > I'm afraid that adding prog_id or a pointer to the trampoline > will complicate things even more for multi attach. Yep, sure, makes sense to me. > > It's easy to store hard coded bpf_tramp_image pointer in the generated > trampoline. Storing prog_id or bpf_prog pointer there is a bit > harder, since the [sp-X] store needs to happen right in there invoke_bpf_prog() > (since there can be multiple progs per trampoline). > > From there bpf_get_attach_cookie() can either do binary search > in the ip->cookie array or single load in case of non-multi attach. > > Anyway the cookie support in trampoline seems to be easier to design > when there is a clarity on multi-attach fentry. > True. bpf_tramp_image pointer probably won't work for multi-attach fentry because one bpf_tramp_image will be shared by multiple attachments (bpf_links), right? Ideally we should provide a way to lookup "current bpf_link" at runtime from BPF helpers (given fentry ctx) and do that binary search there. There could be few other options (e.g., bpf_tramp_image + binary search based on prog_id or bpf_prog pointer), but it's a bit more cumbersome. It still feels like something per-program would need to come from the context to be able to distinguish between multiple attachments. But as you said, more clarity on multi-attach fentry first would be best. > I would probably add support for cookie in raw_tp/tp_btf first, > since that part is not going to be affected by multi-* work. Yep, let's try that first. > > > We don't need all BPF program types, but anything that's useful for > > generic tracing is much more powerful with cookies. We have them for > > kprobe, uprobe and perf_event programs already. For multi-attach > > kprobe/kretprobe and fentry/fexit they are essentially a must to let > > users use those BPF program types to their fullest. > > agree. I missed the part that cookie is already supported with kuprobes > when attach is done via bpf_link_create.
