Conversation
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
📝 WalkthroughWalkthroughTwo eBPF program files receive conditional compilation directives to exclude specific syscall tracepoint handlers from arm64 builds. The Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
pkg/ebpf/gadgets/hardlink/program.bpf.c (2)
37-84: Correct fix for arm64 compatibility.Same as the symlink tracer—arm64 only implements
linkat, notlink. The guard is appropriate.Consider adding a matching inline comment for consistency:
+// arm64 only implements linkat, not link `#ifndef` __TARGET_ARCH_arm64 SEC("tracepoint/syscalls/sys_enter_link"),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/ebpf/gadgets/hardlink/program.bpf.c` around lines 37 - 84, The existing guard excluding arm64 is correct because arm64 only implements linkat; keep the `#ifndef` __TARGET_ARCH_arm64 around the SEC("tracepoint/syscalls/sys_enter_link") enter_link function, and add a short inline comment above that guard (matching the symlink tracer style) stating that arm64 implements only linkat so link tracepoint is not available on arm64; ensure the symbol names referenced are enter_link and SEC("tracepoint/syscalls/sys_enter_link") so reviewers can find the function to update the comment.
41-43: Pre-existing: commented-out filtering code.The
gadget_should_discard_data_current()check is commented out here but is active in the symlink tracer. This appears to be pre-existing and unrelated to this PR, but may warrant a follow-up to align behavior between the two tracers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/ebpf/gadgets/hardlink/program.bpf.c` around lines 41 - 43, The hardlink tracer currently has the gadget_should_discard_data_current() check commented out while the symlink tracer keeps it active; restore consistent filtering by re-enabling the same call in the hardlink tracer (uncomment or re-add gadget_should_discard_data_current() with the same early-return pattern used in the symlink tracer) inside the relevant probe function in program.bpf.c so both tracers uniformly discard data when that predicate is true.pkg/ebpf/gadgets/symlink/program.bpf.c (1)
32-79: Correct fix for arm64 compatibility.The arm64 kernel ABI doesn't include the legacy
symlinksyscall—onlysymlinkatis available. The#ifndef __TARGET_ARCH_arm64guard is the appropriate solution.Consider adding a brief inline comment explaining why this exclusion is needed for future maintainers:
+// arm64 only implements symlinkat, not symlink `#ifndef` __TARGET_ARCH_arm64 SEC("tracepoint/syscalls/sys_enter_symlink"),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/ebpf/gadgets/symlink/program.bpf.c` around lines 32 - 79, The current guard using `#ifndef` __TARGET_ARCH_arm64 around the tracepoint handler enter_symlink (SEC("tracepoint/syscalls/sys_enter_symlink")) is correct for arm64 because the legacy symlink syscall is not present there; add a short inline comment above that `#ifndef` explaining that arm64 uses symlinkat only and therefore this tracepoint should be excluded on __TARGET_ARCH_arm64 to help future maintainers locate the rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/ebpf/gadgets/hardlink/program.bpf.c`:
- Around line 37-84: The existing guard excluding arm64 is correct because arm64
only implements linkat; keep the `#ifndef` __TARGET_ARCH_arm64 around the
SEC("tracepoint/syscalls/sys_enter_link") enter_link function, and add a short
inline comment above that guard (matching the symlink tracer style) stating that
arm64 implements only linkat so link tracepoint is not available on arm64;
ensure the symbol names referenced are enter_link and
SEC("tracepoint/syscalls/sys_enter_link") so reviewers can find the function to
update the comment.
- Around line 41-43: The hardlink tracer currently has the
gadget_should_discard_data_current() check commented out while the symlink
tracer keeps it active; restore consistent filtering by re-enabling the same
call in the hardlink tracer (uncomment or re-add
gadget_should_discard_data_current() with the same early-return pattern used in
the symlink tracer) inside the relevant probe function in program.bpf.c so both
tracers uniformly discard data when that predicate is true.
In `@pkg/ebpf/gadgets/symlink/program.bpf.c`:
- Around line 32-79: The current guard using `#ifndef` __TARGET_ARCH_arm64 around
the tracepoint handler enter_symlink
(SEC("tracepoint/syscalls/sys_enter_symlink")) is correct for arm64 because the
legacy symlink syscall is not present there; add a short inline comment above
that `#ifndef` explaining that arm64 uses symlinkat only and therefore this
tracepoint should be excluded on __TARGET_ARCH_arm64 to help future maintainers
locate the rationale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 897013e2-1205-4e37-ba4c-32d7312bf5dc
📒 Files selected for processing (2)
pkg/ebpf/gadgets/hardlink/program.bpf.cpkg/ebpf/gadgets/symlink/program.bpf.c
2 participants
Summary by CodeRabbit