Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kprobe: fall back to tracefs if kernel doesn't support dots in PMU symbol #605

Merged
merged 3 commits into from
Mar 30, 2022

Conversation

chenhengqi
Copy link
Contributor

Centos 8 (kernel 4.18) has kprobe PMU, but don't allow . in symbol name,
add a new option to workaround this.

@chenhengqi chenhengqi force-pushed the support-legacy-kprobe branch from 74037f8 to 9570ff3 Compare March 25, 2022 02:29
@lmb
Copy link
Collaborator

lmb commented Mar 26, 2022

@chenhengqi how does the centos kernel fail when . is in the name? In the best case we could add an if errors.Is(err, EFOO) case that enables the already existing tracefs fallback.

@chenhengqi
Copy link
Contributor Author

@chenhengqi how does the centos kernel fail when . is in the name? In the best case we could add an if errors.Is(err, EFOO) case that enables the already existing tracefs fallback.

The version of kernel shipped in CentOS 8 Linux is 4.18.

In 4.18, the kernel don't allow dot in symbol name in kprobe PMU case:

https://github.com/torvalds/linux/blob/94710cac0ef4ee177a63b5227664b38c95bbf703/kernel/trace/trace_kprobe.c#L340-L343

Can we always fallback to tracefs interface ? Or check the error against EINVAL ?

@ti-mo
Copy link
Collaborator

ti-mo commented Mar 28, 2022

No need for feature detection IMO, falling back under a specific error like Lorenz suggested sounds like the best option to me. Is this possible?

@chenhengqi chenhengqi force-pushed the support-legacy-kprobe branch from 9570ff3 to 570d306 Compare March 28, 2022 15:18
@chenhengqi
Copy link
Contributor Author

OK, just updated the code as you suggest.

Since we allow `.` in symbol names, we need to sanitize
those symbols to please tracefs.

Signed-off-by: Hengqi Chen <[email protected]>
Centos 8 (with kernel 4.18) has kprobe PMU support, but
don't allow `.` in symbol name. Detect such cases and
fallback to tracefs interface.

Signed-off-by: Hengqi Chen <[email protected]>
@ti-mo ti-mo force-pushed the support-legacy-kprobe branch from 570d306 to fccc6c0 Compare March 30, 2022 16:31
…g dots

These are all local symbols that visible in /proc/kallsyms and traceable
by kprobes. Make sure the library supports attaching to these.

Signed-off-by: Timo Beckers <[email protected]>
@ti-mo ti-mo force-pushed the support-legacy-kprobe branch from fccc6c0 to 5f13ae3 Compare March 30, 2022 19:04
@ti-mo ti-mo changed the title Add a new option to allow kprobe fallback to tracefs interface kprobe: fall back to tracefs if kernel doesn't support dots in PMU symbol Mar 30, 2022
@ti-mo
Copy link
Collaborator

ti-mo commented Mar 30, 2022

I've added a missing call to sanitizedSymbol() in closeTraceFSProbeEvent(). Closing a tracefs event created with a sanitized name would of course fail otherwise.

I've also converted TestK(ret)probe to be table-driven tests and found some kernel symbols with local visibility that were present in kernel versions ranging from 4.4 to 5.15, so this fallback behaviour can actually be exercised.

@ti-mo ti-mo merged commit 51f435a into cilium:master Mar 30, 2022
@chenhengqi
Copy link
Contributor Author

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants