-
Notifications
You must be signed in to change notification settings - Fork 156
query: Extract tracepoint & k(ret)probe from perf_event links #1213
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
base: master
Are you sure you want to change the base?
Conversation
/// A tracepoint event. | ||
Tracepoint { | ||
/// The tracepoint name. | ||
name: Option<CString>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the other link type that has strings to return is RawTracepointLinkInfo
and it returns String
but I think it's saner to return a CString
/OsString` and let the user decode if needed
1e610b0
to
a451904
Compare
a451904
to
2c37073
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this seems fine, thanks. Would it be possible to add a test for this functionality?
/// TODO: Add support for `BPF_PERF_EVENT_EVENT`, `BPF_PERF_EVENT_UPROBE` | ||
/// `BPF_PERF_EVENT_URETPROBE` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should be using doc comments for TODOs; please add proper documentation instead.
/// TODO: Add support for `BPF_PERF_EVENT_EVENT`, `BPF_PERF_EVENT_UPROBE` | |
/// `BPF_PERF_EVENT_URETPROBE` | |
// TODO: Add support for `BPF_PERF_EVENT_EVENT`, `BPF_PERF_EVENT_UPROBE` | |
// `BPF_PERF_EVENT_URETPROBE` |
|
||
print!(" {probe_type}"); | ||
if *addr != 0 { | ||
print!(" addr={addr:x}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be
print!(" addr={addr:x}"); | |
print!(" addr={addr:#x}"); |
?
// Handle two-phase call for perf event string data if needed (this mimics the | ||
// behavior of bpftool). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Comment (and naming of need_second_call
) seems a bit bogus: there is no first call in scope. It could conceptually have been called from anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(the first call is from the gen_info_impl
macro, but I'll rename to something clearer :) )
.cookie | ||
}; | ||
let name = (tp_name != 0).then(|| unsafe { | ||
CStr::from_ptr(tp_name as *const c_char).to_owned() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the typical contract that the kernel would set the actual name length? Or is this not the case here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually has the length - but I followed bpftool here, which just prints the string. If I have to guess - the returned length is the full one and the provided name is the null-terminated string (truncated), but i'll make sure and comment.
I'll try. There are no tests for link infos from |
heavily inspired by matching bpftool code