fix(starry): replace magic numbers in tracepoint and ebpf init#1404
Closed
CN-TangLin wants to merge 6 commits into
Closed
fix(starry): replace magic numbers in tracepoint and ebpf init#1404CN-TangLin wants to merge 6 commits into
CN-TangLin wants to merge 6 commits into
Conversation
Replace raw config values 0/1 in kprobe/uprobe dispatch with named constants PROBE_CONFIG_ENTRY / PROBE_CONFIG_RETURN, matching the Linux PERF_TYPE_PROBE ABI. Also refactor the kprobe config match from a nested if/else chain into flat const-pattern match arms.
为 BpfPerfEventWrapper 实现 try_read_record 方法，使 perf event fd 支持 read(2) 系统调用读取 ringbuf 中的 eBPF 输出记录。 主要变更： - bpf.rs: 添加 mmap_kvirt 字段保存 ringbuf 内核虚拟地址，实现 try_read_record 从 perf_event_mmap_page 读取 data_head/data_tail， 处理环缓冲区 wrapping，读取 perf_event_header 确定记录尺寸， 推进 data_tail；添加 wrapping_add/copy_ring 辅助函数 - mod.rs: 实现 FileLike::read，通过 downcast 获取 BpfPerfEventWrapper 并调用 try_read_record，使用 PAGE_SIZE_4K 作为栈缓冲区尺寸常量
将 PerfEvent::read 从轮询式非阻塞读重构为基于 block_on(poll_io(...)) 的阻塞读模式，匹配 Linux read(perf_fd) 语义。当 ringbuf 无数据时 返回 WouldBlock 让 poll_io 等待，数据到达后由已有的 BpfPerfEventWrapper poll 基础设施（poll_ready/poll_notify/IrqNotify） 唤醒任务继续读取。 重构要点： - 使用 block_on(poll_io(...)) 替代简单轮询，复用已有的 Pollable 实现 - read 闭包在返回 WouldBlock 前释放 SpinNoPreempt 锁 - 通过 PerfEvent::poll → BpfPerfEventWrapper::poll 获取可读性状态 - 通过 PerfEvent::register → BpfPerfEventWrapper::register 注册 waker
消除 perf 模块中的两处硬编码： - BPF_JIT_MEM_PAGES = 4：x86_64 JIT 可执行内存页数 - KRETPROBE_MAX_ACTIVE = 10：kretprobe 最大并发实例数 使用命名常量替代字面量，提高代码可读性和可维护性。
为 PerfEvent 添加 nonblocking 标志支持： - PerfEvent 新增 AtomicBool 字段存储 O_NONBLOCK 状态 - 实现 FileLike::nonblocking() / set_nonblocking() - read(2) 在 nonblocking 模式下，ringbuf 为空时立即返回 EAGAIN 而非阻塞等待（通过 poll_io 的 non_blocking 参数传递）
消除 tracepoint 和 ebpf 模块中的四处硬编码： - TRACE_RAW_PIPE_CAPACITY = 4096：trace pipe 环形缓冲区最大记录数 - TRACE_CMDLINE_CACHE_SIZE = 4096：进程命令行缓存容量 - BPF_FUNC_PROBE_READ = 4：bpf_probe_read helper ID - BPF_FUNC_PROBE_READ_KERNEL = 113：bpf_probe_read_kernel helper ID 使用命名常量替代字面量，提高代码可读性和可维护性。
Contributor
Author
![mai-team-app[bot]](https://avatars.githubusercontent.com/u/34859362?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
PR #1404 Review — 已关闭，合并至 #1412
状态: 此 PR 已关闭（未合并），内容已合并到 #1412。本次审查作为记录性代码审查。
📋 总览
6 个文件变更，+196/-28。标题为「replace magic numbers in tracepoint and ebpf init」，但实际 diff 包含了远超魔数替换的功能：PerfEvent::read() 完整实现、ring buffer 读取逻辑、nonblocking 支持等。建议后续将功能增强和纯重命名/魔数替换分开为独立 PR。
✅ 做得好的部分
-
常量命名：
BPF_FUNC_PROBE_READ、BPF_FUNC_PROBE_READ_KERNEL、PROBE_CONFIG_ENTRY/PROBE_CONFIG_RETURN、KRETPROBE_MAX_ACTIVE、BPF_JIT_MEM_PAGES、TRACE_RAW_PIPE_CAPACITY、TRACE_CMDLINE_CACHE_SIZE命名清晰，注释充分。 -
match 模式重构：
PerfProbeConfig::Raw(PROBE_CONFIG_ENTRY)比原来的if val == 0 / val == 1更清晰、更安全。 -
uprobe.rs复用常量：从kprobe模块导入PROBE_CONFIG_ENTRY/PROBE_CONFIG_RETURN避免了重复定义。
🔍 代码正确性分析
-
wrapping_add+copy_ring环形缓冲区读取 (perf/bpf.rs)：data_region_size是 2 的幂次页面数，base < modulus且offset < modulus，因此wrapping_add的sum % modulus路径正确。copy_ring的 wrap-around 处理正确：to.wrapping_sub(from) % ring_size在 ring_size 为 2 的幂时等价于(to - from + ring_size) % ring_size。- ✅ 无越界风险。
-
try_read_record并发安全 (perf/bpf.rs:125)：- 调用路径受
PerfEvent::read()中SpinNoPreempt锁保护，data_tail更新不会并发竞争。 mmap_kvirt的 header page 和 data region 不重叠（data_region 偏移PAGE_SIZE_4K），无 aliasing 问题。- 建议在 fn doc comment 中补充「必须在 SpinNoPreempt 锁内调用」的并发前提条件。
- 调用路径受
-
PerfEvent::read()中drop(event)位置 (perf/mod.rs:136)：在写入 dst 之前释放 spin lock，避免在持锁期间做 I/O 和poll_io调度。设计合理。
⚠️ 建议改进
-
bpf.rs安全注释：try_read_record中有多处unsafe块。虽然 SAFETY 注释充分，但建议补充并发约束到函数级 doc comment。 -
record_size校验：record_size < hdr_size检查已覆盖 size=0 的情况。✅ -
data_tail累加溢出：u64类型，正常使用下不会溢出。✅
CI 状态
所有 CI check runs 均为 skipped（PR 关闭过快，CI 未实际运行）。本地无需重复构建。
结论
代码质量整体良好，魔数替换和 read() 实现逻辑正确。此 PR 已被作者合并至 #1412，建议在 #1412 上继续审查。
Powered by mimo-v2.5-pro
CN-TangLin
added a commit
to CN-TangLin/tgoskits
that referenced
this pull request
Jul 1, 2026
…pf/tracepoint Replace magic numbers with named constants and improve security in perf, ebpf, and tracepoint subsystems: - perf/kprobe: add PROBE_CONFIG_ENTRY/RETURN constants, refactor config match from if/else chain into const-pattern arms - perf/bpf: add BPF_JIT_MEM_PAGES constant, replace magic 4 with it - perf/bpf: remove vm.register_allowed_memory(0..u64::MAX) to restrict BPF direct memory access, preventing buggy/hostile programs from reading arbitrary kernel memory - perf/kprobe: add KRETPROBE_MAX_ACTIVE constant (10, per Linux max(10, 2*NR_CPUS) for single-CPU), replace magic 10 in builder - tracepoint/mod: add MAX_ACTIVE_TRACEPOINTS (16) and MAX_SLOTS_PER_TRACEPOINT (64) constants - ebpf/mod: add MAX_EXEC_MEM_PAGES (256) constant, replace magic 256 in JIT executable memory allocation Changes combined from PRs rcore-os#1399, rcore-os#1402, rcore-os#1404, rcore-os#1405.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
问题
tracepoint 和 ebpf 模块中存在四处硬编码的数字字面量：
tracepoint/mod.rs:TracePipeRaw::new(4096)和NonZero::new(4096).unwrap()ebpf/mod.rs:set.get(&4)和set.entry(113)—— BPF helper function ID变更
定义为命名常量并替换：
TRACE_RAW_PIPE_CAPACITY: usize = 4096—— trace pipe 环形缓冲区最大记录数TRACE_CMDLINE_CACHE_SIZE: usize = 4096—— 进程命令行缓存容量BPF_FUNC_PROBE_READ: u32 = 4—— bpf_probe_read helper IDBPF_FUNC_PROBE_READ_KERNEL: u32 = 113—— bpf_probe_read_kernel helper ID逻辑
遵循"严禁硬编码"原则，使用命名常量替代字面量，提高代码可读性和可维护性。