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

tools/profile: add support for PID-namespacing #4709

Merged
merged 3 commits into from
Aug 31, 2023

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Aug 16, 2023

This adds translation logic to perform PID remapping across PID-namespaces.
It is now possible to profile a target process from within a nested PID-namespace (e.g. in a container).

This adds translation logic to perform PID translation across
PID-namespaces.
It is now possible to profile a target process from within a
nested PID-namespace (e.g. in a container).
@lucab
Copy link
Contributor Author

lucab commented Aug 16, 2023

/cc @yonghong-song for review.

tools/profile.py Outdated Show resolved Hide resolved
@yonghong-song
Copy link
Collaborator

Can we add an option for this new functionality, something like '--with-pid-namespace'? Note that with '--with-pid-namespace', the command line arguments like pid/tid should be the one in the pid namespace to make the tool work.
With introducing new option, please add a few examples in the example file and update the man page as well.

@lucab
Copy link
Contributor Author

lucab commented Aug 23, 2023

@yonghong-song thanks for the feedback! I've happily added a note to the examples file, but if possible I'd push back against adding a CLI option.

This is really a core bugfix more than a new feature: the profile tool is already always running in some kind of pid-namespace (either the top-level host one, or a nested children down the chain), but currently it produces wrong results for anything other than the host namespace, without giving any clues to the user.

Specifically, when running in a children namespace it will happily target a host-PID which by chance shares the same number as a current-children-ns-PID.
The resulting stacktrace will have some proper content with meaningless addresses (because they belong to an unexpected process) and symbol resolution will be incorrect (same reason).

I think a --with-pid-namespace flag could be added in the future for some new advanced feature (e.g. targeting arbitrary non-self namespaces), but this PR instead is only about fixing a bug in the current eBPF code.

@yonghong-song
Copy link
Collaborator

Okay, @lucab Your argument above indeed makes sense. Without this symbolization will be incorrect in child pid namespace.

@yonghong-song yonghong-song merged commit 442f658 into iovisor:master Aug 31, 2023
5 of 11 checks passed
@lucab
Copy link
Contributor Author

lucab commented Aug 31, 2023

@yonghong-song good to hear, and thanks for taking the time to review this!

captain5050 pushed a commit to captain5050/bcc that referenced this pull request Oct 12, 2023
This adds translation logic to perform PID translation across
PID-namespaces.
It is now possible to profile a target process from within a
nested PID-namespace (e.g. in a container).
Also add a note in profile_example.txt file.
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.

3 participants