Skip to content
This repository was archived by the owner on Jan 9, 2026. It is now read-only.

Implement perf_event_open ABI to attach kprobes and uprobes#40

Open
Gui774ume wants to merge 4 commits intomasterfrom
will/perf-event-open-abi
Open

Implement perf_event_open ABI to attach kprobes and uprobes#40
Gui774ume wants to merge 4 commits intomasterfrom
will/perf-event-open-abi

Conversation

@Gui774ume
Copy link
Collaborator

What does this PR do ?

This PR introduces the perf_event_open ABI to attach kprobes and uprobes (e12f03d "perf/core: Implement the 'perf_kprobe' PMU"). With this PR, the manager will no longer use kprobe_events and uprobe_events to attach kprobes and uprobes. This has multiple benefits that are explained in the Kernel patch notes.

The manager will start by using this ABI, and if it fails, will fall back to the usual kprobe_events / uprobe_events implementation.

kprobeID, err = EnableKprobeEvent(probeType, funcName, p.UID, "", p.attachPID)
}
// Try to use the perf_event_open API first (e12f03d "perf/core: Implement the 'perf_kprobe' PMU")
perfEventFD, err := perfEventOpenWithProbe(funcName, 0, -1, sectionPrefix, 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know how this reacts when the kernel doesn't support it? Rather than attempt this for every probe attach, it would be great to disable it when not available, such as kernels < 4.17

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could maybe try using an internal.FeatureTest for this.

Copy link
Collaborator Author

@Gui774ume Gui774ume Apr 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it simply fails and returns an error ... I'm simply following BCC design again, but I'll try to figure out a cleaner way of dealing with this and get back to you 😄
https://github.com/iovisor/bcc/blob/887e05abd49c6587ede708de6b7040830d90e075/src/cc/libbpf.c#L1055-L1064

manager/probe.go Outdated
return errors.Wrapf(err, "couldn't enable kprobe %s", p.Section)
// Try to create the event using debugfs
// Write kprobe_events line to register kprobe
kprobeID, err := EnableKprobeEvent(probeType, funcName, p.UID, maxactiveStr, p.attachPID)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to have EnableKprobeEvent try to use perf_event_open and fall back to procfs instead of doing it in attachKprobe? EnableKprobeEvent is public, so it should probably try to use the new method of attaching just like probe.Attach()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The perf_event_open only method doesn't generate a kprobeID like debugfs does. I'm also not sure why this function is exported.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, EnableKprobeEvent is by design the "old" ABI, so its only job is to write a line in kprobe_events. The perf_event_open happens after this stage with either the kprobeID or the returned fd of the new ABI. It's role is to activate the kprobe so that it triggers the eBPF program (just like when you write 1 in the enable file when you're using perf). So if that's ok with you both I can:

  • rename EnableKprobeEvent to RegisterKprobeEvent (since it doesn't enable it per say ...)
  • unexport the function

(I initially exported it because we used to write tests manually to check that the right probes were activated)

Copy link
Collaborator Author

@Gui774ume Gui774ume Apr 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(same goes for the Disable function & Uprobe ones)

@brycekahle
Copy link
Member

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants