fix(syscall-stat): improve builtin flow by yuKing123-king · Pull Request #115 · DKapture/libdkapture

yuKing123-king · 2026-07-27T07:00:03Z

对本次 syscall-stat 工具修改的内容

补齐 BUILTIN 测试入口
- 改为通过返回值处理参数解析结果，增强健壮性。

rwenz2004 · 2026-07-27T07:42:59Z

注意提交信息，应该是fix不是test

yuKing123-king · 2026-07-31T02:18:39Z

注意提交信息，应该是fix不是test

ok，后面统一会改成fix

rwenz2004 · 2026-07-31T03:26:27Z

-static std::atomic<bool> exit_flag(false); // Flag to signal exit
+static std::atomic<bool> exit_flag(false);
+
+#ifdef BUILTIN


这个宏的引入是不必要的，局部函数直接用static就行，没必要考虑这种兼容性

rwenz2004 · 2026-07-31T03:27:13Z


 // Parse command line arguments
-void parse_args(int argc, char **argv)
+static int parse_args(int argc, char **argv)


其他局部函数也应该像这样修改，不要使用BUILTIN_LOCAL宏

所有局部函数已改成static

rwenz2004 · 2026-07-31T03:28:02Z

+	int *log_fd,
+	int *stats_fd
+){
+	test_name = "syscall-stat"; // 告诉 mock/test 框架当前正在运行哪个工具


中文注释只需要保留关键部分

已压缩成简洁的英文

xu-lang · 2026-08-04T07:15:38Z

 												 // stats
-
-		while (0 == bpf_map_get_next_key(stats_fd, &key, &nxt_key))
+		int ret  = bpf_map_get_next_key(stats_fd, NULL, &nxt_key);


这儿为啥改动，没有必要的话，无关改动，不要引入进来

这的改动是有必要的，因为原来的代码是

while (0 == bpf_map_get_next_key(stats_fd, &key, &nxt_key)) { info sys_stat; bpf_map_lookup_elem(stats_fd, &nxt_key, &sys_stat); if (nxt_key >= sizeof(sys_tbl) / sizeof(sys_tbl[0])) { key = nxt_key; continue; } if (sys_stat.cnt == 0) { key = nxt_key; continue; } stats.push_back({nxt_key, sys_stat}); total += sys_stat.cnt; memset(&sys_stat, 0, sizeof(sys_stat)); bpf_map_update_elem(stats_fd, &nxt_key, &sys_stat, BPF_ANY); key = nxt_key; }

while (0 == bpf_map_get_next_key(stats_fd, &key, &nxt_key))这里循环条件一开始就调用next_key，但是key一开始是0，next_key就是1，并且后面用来查询stats_fd这个map一直都用的next_key，也就是说后面的代码永远都不会去获取key=0的stats_fd，所以最后的结果永远都不会有系统调用号为0的read，这个问题是我用syscall-stat去追踪ls的系统调用的时候发现没有read这个系统调用，ls是查询功能，所以理论上来说肯定会调用read这个系统调用的。后来我调整遍历逻辑，确保从代码可以从第一个 key 开始完整遍历。再去使用syscall-stat去追踪ls的系统调用时，结果中就有read这个系统调用了

xu-lang · 2026-08-04T07:16:47Z

 		{
 			pr_error("Error polling ring buffer: %d\n", err);
-			sleep(5); // Sleep before retrying
+			std::this_thread::sleep_for(std::chrono::microseconds(5)); // Sleep before retrying


解释下，为什么改这

因为我觉得这里的sleep(5); 是ring_buffer__poll() 出错后，等待5秒之后再重新试一下，避免错误路径一直占用cpu，但是5s我觉得太久了，会影响使用性能所以改成了5微秒，如果感觉太短也可以改回5s

xu-lang · 2026-08-04T07:38:39Z

+		{"pid2pathhash",{sizeof(pid_t), sizeof(u32), 1024, BPF_MAP_TYPE_LRU_HASH}},
+		{"logs", {0, 0, 1024 * 1024, BPF_MAP_TYPE_RINGBUF}},
+	};
+static std::atomic<int> *condition;


这几个静态变量，都没有使用

condition这个使用了，这个是.cpp代码和test代码用来同步状态的
{"pid2pathhash",{sizeof(pid_t), sizeof(u32), 1024, BPF_MAP_TYPE_LRU_HASH}},
{"logs", {0, 0, 1024 * 1024, BPF_MAP_TYPE_RINGBUF}},这两个确实没使用，我现在删除

xu-lang · 2026-08-04T07:57:22Z

 												 // stats
-
-		while (0 == bpf_map_get_next_key(stats_fd, &key, &nxt_key))
+		int ret  = bpf_map_get_next_key(stats_fd, NULL, &nxt_key);


拆成两个提交，bug修复一个

xu-lang

请先修复评审问题

yuKing123-king · 2026-08-04T08:27:49Z

请先修复评审问题

1.多余静态变量已删除
2.已经将修复bug部分，单独提出一个pr

xu-lang

问题还是很多，以上只是部分，更严重的是修改思路框架设计过于随意，过于依赖全局变量

xu-lang · 2026-08-05T03:13:57Z

 			free(buf);
-			exit(0);
-			break;
+			return 1; // Indicate that help was displayed


这里为什么返回1，破坏了原来的逻辑

原来这里是exit(0)，exit和return的区别是，exit会立即终止当前线程，后续代码全部不执行，而return是终止当前函数，后面代码正常执行。

如何这里还是exit的话，线程终止后面代码不执行，没有相关状态变量传到syscall-stat-test.cpp中，syscall-stat-test.cpp会正常执行，但是无法区分测试失败是因为"用户请求了帮助"还是"程序崩溃"，所以每个地方都需要一个返回值参数返回给syscall-stat-test.cpp区分是什么原因引起的测试失败。

如果这里一定需要exit(0)这样的语义，这里可以通过定义BUILTIN这个宏来继续使用exit(0)。

xu-lang · 2026-08-05T03:19:32Z

 	stats_fd = bpf_get_map_fd(obj->obj, "syscall_stat", goto err_out);
-
+#ifdef BUILTIN
+	*filter_fdp = filter_fd;


为什么把fd传出去，外面保证fd的生命周期了吗?是否导致fd泄漏

xu-lang · 2026-08-05T03:21:45Z

+	*condition = 1;
+	while(!exit_flag)
+	{
+		std::this_thread::sleep_for(std::chrono::microseconds(5));


？？？空转？

xu-lang · 2026-08-05T03:24:50Z

 		}
-
+#ifdef BUILTIN
+      	*conditionPrint = 1;


不要这样传参出去，一堆这种出参，极其ugly

std::atomic<int> *conditionp, std::atomic<int> *conditionPrintP, std::atomic<bool> **exit_flagp, int *filter_fd, int *stats_fd

Signed-off-by: Wang Yu <wangyu6@uniontech.com>

yuKing123-king · 2026-08-06T06:49:11Z

+local_map_info = {
+	{"filter", {sizeof(u32), sizeof(struct Rule), 1, BPF_MAP_TYPE_HASH}},
+	{"syscall_stat",{sizeof(u32), sizeof(struct info), 453, BPF_MAP_TYPE_ARRAY}},
+	{"logs", {0, 0, 1024 * 1024, BPF_MAP_TYPE_RINGBUF}},


这里的filter和syscall_stat和logs，虽然在当前代码中没有使用，这个local_map_info是传给mock.cpp去生成3个伪map供syscall-stat-test.cpp中使用

yuKing123-king · 2026-08-06T06:50:37Z

+			char *end = nullptr;
+			errno = 0;
+			long val = strtol(optarg, &end, 10);
+			if (end == optarg || *end != '\0' ||  errno == ERANGE || val < 0 || (unsigned long)val > UINT32_MAX)


这里的修改是为了检验用户输入负数，字母等非法参数，例如-1，12abc，abc这样的参数，可以提示用户参数输错

yuKing123-king · 2026-08-06T07:02:26Z

 			free(buf);
-			exit(0);
-			break;
+			return 1; // Indicate that help was displayed


原来这里是exit(0)，exit和return的区别是，exit会立即终止当前线程，后续代码全部不执行，而return是终止当前函数，后面代码正常执行。

如何这里还是exit的话，线程终止后面代码不执行，没有相关状态变量传到syscall-stat-test.cpp中，syscall-stat-test.cpp会正常执行，但是无法区分测试失败是因为"用户请求了帮助"还是"程序崩溃"，所以每个地方都需要一个返回值参数返回给syscall-stat-test.cpp区分是什么原因引起的测试失败。

如果这里一定需要exit(0)这样的语义，这里可以通过定义BUILTIN这个宏来继续使用exit(0)。

yuKing123-king · 2026-08-06T07:02:45Z

 			Usage(argv[0]);
 			free(buf);
-			exit(-1);
+			return -1;


这里与上述问题一致

yuKing123-king · 2026-08-06T07:04:17Z

-		int slen = strlen(sys_tbl[i]);
+		if (!sys_tbl[i]) 
+			continue;
+    	int slen = strlen(sys_tbl[i]);


这里的修改是防止sys_tbl[i]是NULL，导致strlen(NULL)

yuKing123-king · 2026-08-06T07:05:53Z

 		if (total)
 		{
-			printf("\ntotal: %d\n", total);
+			printf("\ntotal: %u\n", total);


这里的修改是因为上面定义的total是u32 total = 0;，是u32类型不是int类型

yuKing123-king · 2026-08-06T07:08:13Z

 	if (0 != syscall_stat_bpf::attach(obj))
 	{
-		exit(-1); // Attach BPF program
+		goto err_out; // Attach BPF program


这里不使用exit(-1)是因为如果上面的syscall_stat_bpf::open_and_load();成功了，但是syscall_stat_bpf::attach(obj)失败了，但是使用exit(-1)的话，会终止整个程序，导致后面的obj释放无法执行，造成泄露

yuKing123-king · 2026-08-06T07:15:49Z

 	stats_fd = bpf_get_map_fd(obj->obj, "syscall_stat", goto err_out);
-
+#ifdef BUILTIN
+	runtime_state->fds_promise.set_value(SyscallStatFds{filter_fd, stats_fd});


这里向外传filter_fd和stat_fd的原因是，syscall-stat-test.cpp需要这两个fd去查询mock.cpp生成的rule和stat 这两个map的数据，以便向 map 中注入测试事件（bpf_map_update_elem(stats_fd, ...)）并验证过滤规则（bpf_for_each_map_elem(filter_fd, ...)）。

这里只是传了两个整数值（fd 的拷贝），filter_fd和stat_fd的生命周期依旧还是本程序中控制，在 err_out 标签处通过 syscall_stat_bpf::destroy(obj) 统一释放，测试线程仅持有 fd 的整数副本用于读写操作，不负责关闭，因此不会造成 fd 泄漏或双重释放。

yuKing123-king force-pushed the test/add-syscall-stat-BUILTIN branch from 220b53c to c1c5e24 Compare July 28, 2026 01:37

yuKing123-king force-pushed the test/add-syscall-stat-BUILTIN branch from c1c5e24 to 1cf902e Compare July 31, 2026 03:20

yuKing123-king changed the title ~~test: add support syscall-stat builtin testing and improve runtime fe…~~ fix(syscall-stat): improve builtin flow and fix stats iteration/top parsing Jul 31, 2026

rwenz2004 reviewed Jul 31, 2026

View reviewed changes

yuKing123-king force-pushed the test/add-syscall-stat-BUILTIN branch from 1cf902e to 13a61c2 Compare July 31, 2026 03:37

xu-lang reviewed Aug 4, 2026

View reviewed changes

yuKing123-king force-pushed the test/add-syscall-stat-BUILTIN branch from 13a61c2 to fd937c9 Compare August 4, 2026 08:10

yuKing123-king changed the title ~~fix(syscall-stat): improve builtin flow and fix stats iteration/top parsing~~ fix(syscall-stat): improve builtin flow Aug 4, 2026

yuKing123-king force-pushed the test/add-syscall-stat-BUILTIN branch from fd937c9 to e0ac09a Compare August 4, 2026 08:21

xu-lang reviewed Aug 5, 2026

View reviewed changes

test: improve builtin flow

20d00b5

Signed-off-by: Wang Yu <wangyu6@uniontech.com>

yuKing123-king force-pushed the test/add-syscall-stat-BUILTIN branch from e0ac09a to 20d00b5 Compare August 6, 2026 06:41

yuKing123-king commented Aug 6, 2026

View reviewed changes

Uh oh!

Conversation

yuKing123-king commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

对本次 syscall-stat 工具修改的内容

Uh oh!

rwenz2004 commented Jul 27, 2026

Uh oh!

yuKing123-king commented Jul 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xu-lang left a comment

Choose a reason for hiding this comment

Uh oh!

yuKing123-king commented Aug 4, 2026

Uh oh!

xu-lang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yuKing123-king commented Jul 27, 2026 •

edited

Loading