allow to pin kprobe and kprobe_multi links

[olsajiri]

we need to pin kprobe and kprobe_multi links, ATM I'm using this, but I guess there's better way at least for standard kprobes

I'm not sure if the issue described in Pin function comment is also valid for kprobes created via perf_link,
but the pinning seems to work, and that's what we'd like to use (we don't need to re-open the link)

note the wrapRawLink for perf event kprobe needs to be done properly somehow

cc: @lmb @ti-mo

[lmb]

Please correct me if I'm wrong @mmat11:

• ioctl attach doesn't have pinning, so we don't allow it.
• bpf_link attach does have pinning. so far we don't allow it because we can't recover perfEventLink.pe.
• perfEventLink.pe is used by Close() and returned by PerfEvent().
• contrary to ioctl attach, closing the perf event when doing bpf_link attach is not an issue.

If all of these are true, why do we have perfEvent.pe in the first place? We could remove the field. To replace PerfEvent() we expose an API that explicitly attaches via bpf_link (we've done similar for cgroup IIRC) and returns (Link, perfEvent *os.File, error) or similar.

Thoughts?

[lmb]

I think you can just remove this method + Unpin and then you'll get the implementation from RawLink.

[mmat11]

Please correct me if I'm wrong @mmat11:

* ioctl attach doesn't have pinning, so we don't allow it.

* bpf_link attach does have pinning. so far we don't allow it because we can't recover `perfEventLink.pe`.

* `perfEventLink.pe` is used by `Close()` and returned by `PerfEvent()`.

* contrary to ioctl attach, closing the perf event when doing bpf_link attach is not an issue.

If all of these are true, why do we have perfEvent.pe in the first place? We could remove the field. To replace PerfEvent() we expose an API that explicitly attaches via bpf_link (we've done similar for cgroup IIRC) and returns (Link, perfEvent *os.File, error) or similar.

Thoughts?

should all be correct, I think we can remove perfEventLink.pe if the kernel takes care of the perf event fd via bpf_link (does this FD get closed once the refcount gets to 0?)

[olsajiri]

I'm trying the pushed code, but I'm not sure where to put the comments with fd/link states.. @lmb

[lmb]

@mmat11 do you think they are still useful? Seems like the table doesn't match the reality of bpf_link attached with a perf_event (why?) and also not for the ioctl case (since that doesn't have a link fd in the first place).

[mmat11]

@mmat11 do you think they are still useful? Seems like the table doesn't match the reality of bpf_link attached with a perf_event (why?) and also not for the ioctl case (since that doesn't have a link fd in the first place).

I think we can remove the comment, it refers only to the bpf_link implementation which now has pinning

Seems like the table doesn't match the reality of bpf_link attached with a perf_event (why?)

These are some quick tests I made a while ago ("works" refers to the link, not the pinning) so I don't exactly remember how I got to these results, and that's what I could observe at the time. The only line that doesn't match should be // | Closed | Open | No |, one explanation could be that I forgot to Pin() before closing the perf event fd (?)

and also not for the ioctl case (since that doesn't have a link fd in the first place).

This table refers to bpf_link only

[lmb]

Makes me think we should see whether we can return ErrNotSupported from RawLink.Pin directly instead of hardcoding it in a bunch of places.

[lmb]

What about uprobe_multi, can that be pinned as well?

[olsajiri]

What about uprobe_multi, can that be pinned as well?

yep, added ;-) thanks

[olsajiri]

hm, I guess we could actually just remove Pin/Unpin methods from kprobe/uprobe multi links as well.. it'll be taken care of via raw link

[lmb]

Also removed the shadowed Pin/Unpin methods from kprobe multi, they aren't necessary.