-
Notifications
You must be signed in to change notification settings - Fork 4k
tools: introduce path helpers and filelife,filegone support file path #5345
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
a0a14fb to
3803a20
Compare
97a3232 to
8af1b95
Compare
b2bca4c to
32642f1
Compare
|
Rebase to master. |
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 have a few questions about the overall structure first.
-
Is the use of bpf_map_ringbuf mandatory for path_helper?
-
If not mandatory, can we split the application commits for each tool (e.g., filegone, filelife) into two separate changes?
(1) One commit for replacing the perf buffer map with the ring buffer map
(2) Another commit for actually applying path_helper
I believe separating the structural (infrastructure) change (ring buffer migration) from the functional change (full-path helper adoption) would make the history easier to manage and help with future maintenance. Also, having a dedicated commit for path_helper integration will serve as a clear reference for other developers who may want to apply it to additional tools later. -
Would it be possible to separate the application of path_helper to opensnoop into its own commit? The first commit is quite large, so splitting out the path_helper integration—similar to what was done for the other tools—would make the code easier to review and understand.
-
Is there a specific reason for using -F (full-path) instead of -P/--path? Personally, when I see the -F option, it’s not immediately clear that it refers to “path output,” and the meaning of “F” isn’t very intuitive. I’m curious about the reasoning behind this choice.
Thank you.
32642f1 to
1695645
Compare
|
@ekyooo There are 6 commits ( minimize code modifications per commit )
Please review the code again, thank you |
1695645 to
a6160df
Compare
Add the path_helpers code and header file. These functions are separate from opensnoop.py, and because the code changes are large, this commit does not modify the opensnoop.py code. Add source code: - full_path.h: defined FULL_PATH_FIELD(name); - path_helpers.bpf.c: add bpf_dentry_full_path() and bpf_getcwd() helpers; - path_helpers.py: add get_full_path() to parse full-path in full_path.h; Signed-off-by: Rong Tao <[email protected]>
Apply path_helpers to opensnoop. Signed-off-by: Rong Tao <[email protected]>
In order for filelife to support file paths, it is necessary to replace perf-buffer with ring-buffer, because the single event size of path information transmission is large, and it is impossible to statically allocate events in the stack, so it is necessary to use ring-buffer reservation mechanism. At the same time, 'create_arg' and 'unlink_event' structures are separated from 'data_t' for file creation and deletion events and data record transfer. Signed-off-by: Rong Tao <[email protected]>
Support for file paths using path_helpers.
For example:
$ sudo ./filelife.py -P
TIME PID COMM AGE(s) FILE
20:51:32 55738 rm 0.21 /home/sdb/Git/bcc/build/a.out
20:51:44 47715 Chrome_ChildIOT 0.00 /home/sdb/.org.chromium.Chromium.3hn6CS
20:51:44 3490 ThreadPoolForeg 10.00 /home/rongtao/.cache/google-chrome/Default/Cache/Cache_Data/todelete_8829186fc5f5441a_0_1
20:51:49 3490 ThreadPoolForeg 10.00 /home/rongtao/.cache/google-chrome/Default/Cache/Cache_Data/todelete_25ef4b49ebd6a803_0_1
20:51:49 55767 rm 6.42 /home/sdb/Git/bcc/build/a.out
Signed-off-by: Rong Tao <[email protected]>
In order for filegone to support file paths, it is necessary to replace perf-buffer with ring-buffer, because the single event size of path information transmission is large, and it is impossible to statically allocate events in the stack, so it is necessary to use ring-buffer reservation mechanism. Add 'struct entry_t' to pass the information from kprobe to kretprobe. Signed-off-by: Rong Tao <[email protected]>
Support for file paths using path_helpers.
For example:
$ realpath .
/home/sdb/Git/bcc/build
$ touch a.out && sleep 0.2 && mv a.out b.out && sleep 0.2 && rm b.out
$ sudo ./filegone.py -P
TIME PID COMM ACTION FILE
21:22:37 58683 mv RENAME /home/sdb/Git/bcc/build/a.out > /home/sdb/Git/bcc/build/b.out
21:22:37 58685 rm DELETE /home/sdb/Git/bcc/build/b.out
Signed-off-by: Rong Tao <[email protected]>
a6160df to
82f17b0
Compare
|
rebase to master |
Could you answer my question with clear reasoning? To clarify the buffer type in #5340:
The use of the ring buffer is a modern change, but it excludes users of kernel versions below 5.8. |
Thanks for your reply, the #5340 is for
Indeed, thanks, you're right, maybe we should introduce compat.h like method for |
|
I have a question: was perf_buffer also verified to work correctly in PR #5340? If so, I think it would be beneficial to apply compat.h to the Python tools as well. Even without using path_helper, this change would likely benefit multiple tools. Thanks! |
|
Thank you for your reply, I will confirm this ;) |
Sorry about the late reply, i tried this change, it's not works. diff --git a/libbpf-tools/compat.c b/libbpf-tools/compat.c
index 7d932ef30591..31065e178984 100644
--- a/libbpf-tools/compat.c
+++ b/libbpf-tools/compat.c
@@ -35,7 +35,7 @@ struct bpf_buffer *bpf_buffer__new(struct bpf_map *events, struct bpf_map *heap)
bool use_ringbuf;
int type;
- use_ringbuf = probe_ringbuf();
+ use_ringbuf = 0;
if (use_ringbuf) {
bpf_map__set_autocreate(heap, false);
type = BPF_MAP_TYPE_RINGBUF; |
Do the same thing #5340 to tools.
add
filelife,filegone