libbpf-tools: improve fentry_exists check #3917
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cobrien7
requested review from
drzaeus77,
goldshtn,
yonghong-song,
4ast,
brendangregg and
davemarchevsky
as code owners
|
Are these failed checks a regression caused by this change? It looks like the failing tests involve the python tools, so I'm not sure I see how this would affect them. |
Agreed, test failures look unrelated. Will look into them separately. |
chenhengqi
reviewed
Mar 23, 2022
libbpf-tools/trace_helpers.c
Outdated
| @@ -1042,7 +1065,7 @@ bool fentry_exists(const char *name, const char *mod) | |||
| err_out: | |||
| btf__free(btf); | |||
| btf__free(base); | |||
| return id > 0; | |||
| return id > 0 ? fentry_can_attach(id) : false; | |||
There was a problem hiding this comment.
I think this commit will also close #3781 :)
There was a problem hiding this comment.
It should help, but I don't think all of the tools listed there use fentry_exists to fall back to kprobes
|
Looks like these are the tools which are affected by this change: Just listing for posterity |
davemarchevsky
requested changes
Mar 23, 2022
On architectures that lack support for fentry programs, tools should fall back to using kprobes even if the kernel version is new enough to include BPF_TRACE_FENTRY, but fentry_exists() cannot currently detect this case. Instead of searching for BPF_TRACE_FENTRY, verify that attaching a BPF_TRACE_FENTRY program can actually succeed. Rename fentry_exists to fentry_can_attach to reflect this change. Signed-off-by: Connor O'Brien <connoro@google.com> Change-Id: I5ad0341cb060c7a4a2ee17245337170963dfefad
cobrien7
force-pushed
the
fix-fentry-check
branch
from
3ba7c30
to
3787a0d
Compare
davemarchevsky
approved these changes
Mar 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On architectures that lack support for fentry programs, tools should
fall back to using kprobes even if the kernel version is new enough to
include BPF_TRACE_FENTRY, but fentry_exists() cannot currently detect
this case.
Add an additional check verifying that attaching a BPF_TRACE_FENTRY
program can actually succeed.
Signed-off-by: Connor O'Brien connoro@google.com