Cross-OS portability: fix BPF load failures, guard/symbol drift, arm64 gaps, and installer breakage#92
Merged
Merged
Conversation
Loading the prober aborted with: LLVM ERROR: Invalid usage of the XADD return value block_rq_issue assigned req_id via __sync_fetch_and_add(gen, 1), whose return value compiles to a BPF XADD instruction that cannot return its old value on all kernels/LLVM versions, causing the loader to abort. Replace the global atomic counter with a per-CPU counter and fold the CPU id into the high 16 bits of req_id. BPF programs run with preemption disabled, so the per-CPU read-increment-store needs no atomic, and the CPU-id prefix keeps ids unique across CPUs. This is stronger than a read-then-lock_xadd workaround, which would race and hand out duplicate ids under concurrent block I/O, defeating req_id's purpose of disambiguating distinct I/Os that reuse the same (dev, sector). req_id is consumed only as an opaque unique identifier in the CSV output (ordering uses the separate ts field), so dropping strict global monotonicity has no downstream impact. Comments updated to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Y9in1s3yxA9ZWLKmfQyUa
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the request ID generation mechanism in the BPF prober to use a per-CPU array instead of a global atomic counter. This change avoids portability issues with the BPF XADD instruction on older kernels and LLVM versions. The unique request ID is now constructed by combining the CPU ID in the high 16 bits with a per-CPU sequence counter in the low 48 bits. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an eBPF verifier/load failure in the block probe caused by using the return value of __sync_fetch_and_add() (which can compile to an unsupported XADD-return sequence in some kernel/LLVM combinations). It changes the request-id generator to a per-CPU counter and encodes the CPU id into req_id to preserve uniqueness without relying on atomics that require returning the old value.
Changes:
- Replace the global
BPF_ARRAYrequest-id counter with aBPF_PERCPU_ARRAYcounter. - Generate
req_idas(cpu_id << 48) | per_cpu_seqand update struct/map comments to reflect the new scheme. - Update
block_rq_issueto use read-increment-store on the per-CPU counter instead of__sync_fetch_and_add().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Make the high-16-bits CPU field explicit at the point of use so the encoding matches its doc comment and no longer relies on silent shift overflow to bound the CPU id. Behaviorally equivalent for realistic CPU counts; the mask just documents the field width in the code itself. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Y9in1s3yxA9ZWLKmfQyUa
…robes Batch 1 of the cross-OS portability audit: every fix below was independently verified against kernel history and the actual code paths. Load blockers (--network aborted the whole tracer): - skb:kfree_skb read args->reason unconditionally; the field only exists on kernels >= 5.17 (backported to 5.15.58 LTS). On anything older BCC cannot generate the member and the single BPF() compile fails. Now gated by HAS_SKB_DROP_REASON, set by sniffing the tracepoint format file - the same mechanism as the existing HAS_CMD_FLAGS - so the gate can never disagree with the generated args struct and backports are handled for free. - tcp:tcp_retransmit_skb read args->state, which was added in 4.20 and never backported to RHEL 8's 4.18; gated by HAS_TCP_RETRANSMIT_STATE the same way. - The format sniffing now checks both debugfs and tracefs mounts; previously a tracefs-only system silently lost HAS_CMD_FLAGS. Guard/symbol mismatches (probe families silently dead or emitting garbage): - Five folio cache handlers were guarded >= 5.17 while their symbols exist from 5.16 (where core MM already calls them): hit/miss/dirty/writeback probes never attached on 5.16 kernels. Guards lowered to 5.16. - trace_do_page_cache_readahead's prototype (mapping, file, offset, nr) matches only pre-5.10 __do_page_cache_readahead, but was guarded >= 5.17: on modern kernels it attached to do_page_cache_ra and misread the readahead_control pointer as the mapping (garbage rows); on < 5.17 it didn't compile at all. Split into three correctly-typed variants (legacy < 5.10, do_page_cache_ra >= 5.10, page_cache_ra_order >= 5.18), each attached to its matching symbol. - trace_shrink_folio_list reads no function arguments, so its >= 5.17 guard only disabled reclaim tracing on older kernels (shrink_page_list exists there); guard removed. - trace_cache_drop_folio declared (mapping, folio) but its attach target __filemap_remove_folio is (folio, shadow) - args were misaligned on every kernel, emitting garbage drop rows. Fixed the prototype, read the mapping from folio->mapping, and lowered the guard 5.18 -> 5.17 (the symbol's actual introduction). arm64 feature loss: - mremap tracing attached only __x64_sys_mremap/sys_mremap; on arm64 the wrapper is __arm64_sys_mremap, so MREMAP events were silently absent. Added the Python branch plus a pt_regs-unwrapping arm64 variant (args in regs[0..3] per SC_ARM64_REGS_TO_ARGS). - io_uring_enter's fallback chain leaned on __io_uring_enter and __sys_io_uring_enter, neither of which ever existed in mainline (the syscall is only reachable via the per-arch SYSCALL_DEFINE wrappers), so arm64 got no io_uring ENTER events at all. Added __arm64_sys_io_uring_enter + arm64 unwrap variant and dropped the two provably-dead branches. CI: bpf_smoke.py now mirrors the loader's format sniffing (both trace mounts, network gates) and attaches every folio/page cache handler conditionally on its symbol, so a guard/symbol mismatch fails CI instead of shipping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Y9in1s3yxA9ZWLKmfQyUa
Batch 2 of the cross-OS portability audit (compile-time class). Each fix was
adversarially verified against kernel/BCC history before being applied.
- Replace the version-guarded BTF special-field placeholders (bpf_timer/
bpf_wq/bpf_task_work) with unconditional tag renames to private empty
placeholder structs. Which side defines the real struct depends on BOTH
the kernel AND the installed BCC (BCC force-includes a vendored uapi
snapshot that defines bpf_timer since 0.23), so no LINUX_VERSION_CODE
guard can be right for every pair: "struct bpf_timer {}" under < 5.17 was
a hard redefinition error on stock Ubuntu 22.04 (5.15 + bcc 0.24). The
rename is correct on every combination because the placeholder tag is
ours alone.
- Map bpf_probe_read_kernel/user(_str) to the legacy bpf_probe_read(_str)
helpers on kernels < 5.5 (ifndef-wrapped, so BCC >= 0.15's own injected
compat defines win when present). The file uses the 5.5+ helper family
~210 times; on bcc 0.12-0.14 (stock Ubuntu 20.04) those names compile to
helper ids a < 5.5 verifier rejects at load ("invalid func unknown#113"),
killing the whole program.
- Guard bpf_get_current_cgroup_id() (kernel 4.18+) behind a compat macro
returning 0 on older kernels; previously the vfs read/write/open probes
silently failed to attach on < 4.18.
- Gate build_dentry_path's d_parent walk on kernel >= 5.3: pre-5.3
verifiers reject any loop the compiler leaves rolled (an LLVM-dependent
outcome — a real deployment logged "loop not unrolled" here), and fully
unrolled it can breach the pre-5.2 4096-insn cap. Older kernels degrade
the readdir probe to basename-only instead of risking the whole program.
- Read vfs_rename's argument via the REAL struct renamedata from
<linux/fs.h> instead of a hand-rolled fixed-offset mirror, and add the
6-arg variant for kernels < 5.12. The mirror silently broke twice: on
< 5.12 (inode internals read as dentries) and on 6.18+ (the kernel merged
the two mnt_idmap fields, moving new_dentry from offset 40 to 32, so the
stale read landed on delegated_inode and every rename event carried
garbage new-side data). BCC recompiles against the running kernel's
headers, so the real type stays correct automatically.
- Add a static assert keeping FILENAME_MAX_LEN <= 1024: constant
memset/memcpy above that cannot be inlined by the BPF backend on any
LLVM (deterministic compile failure if ever raised).
Verified NOT bugs during the audit (no change needed): the existing
256-byte builtins can never lower to libcalls (the cliff is 1024, uniform
across LLVM 7..main); BCC's rewriter handles the direct file->f_path
dereferences; all map types used are >= 4.10 features.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y9in1s3yxA9ZWLKmfQyUa
…tics
Batch 3 of the cross-OS portability audit (environment class). Each fix was
adversarially verified before being applied.
Python compatibility (blocked RHEL 8/9, Debian 11, Ubuntu 20.04, Amazon
Linux stock interpreters entirely):
- Add `from __future__ import annotations` to the six modules using PEP 604
(`X | None`) / PEP 585 (`list[str]`) annotation syntax; without it import
crashes on Python <= 3.9 before argparse even runs. No runtime annotation
introspection exists in the codebase, so lazy annotations are safe.
- Raise install.sh's Python floor from 3.6 to the real minimum 3.7
(time.time_ns, subprocess text=), with an RHEL 8 AppStream pointer.
Installer (install.sh):
- Stop appending the Debian SID repo to stable systems: bpfcc-tools has
shipped in stable main since buster, and an unpinned sid entry risks
partial upgrades to unstable on the next apt upgrade.
- Split kernel-headers install from the bcc install and make it non-fatal:
the fused `apt-get install bpfcc-tools linux-headers-$(uname -r)` under
set -e aborted the ENTIRE install (bcc included) on WSL2 and stale cloud
images where the exact headers package does not exist. The warning now
checks /lib/modules/.../build and CONFIG_IKHEADERS and points WSL2 users
at the right fix.
- Add an Amazon Linux case (AL2023 via dnf incl. kernel-devel; AL2 is EOL
and rejected with a clear message) and install kernel-devel-$(uname -r)
on the dnf paths (covers install_weak_deps=False setups).
- Correct the false wrapper comment claiming iotrc must run from the repo
root.
Startup robustness:
- capture_machine_id(): fall back /etc/machine-id -> /var/lib/dbus/
machine-id -> persisted generated id; treat empty/"uninitialized"
content as missing (container images collide on hash("") otherwise).
Previously a missing /etc/machine-id crashed the tracer at startup.
- iotrc.py: resolve the BPF source relative to __file__ instead of the
CWD. BCC's argv[0] fallback made the old relative path work, but it
checks the CWD first, so a foreign checkout could silently shadow the
installed prober.c.
- Add a real --output DIR option (main parser and dev subcommand). The
systemd unit has always passed --output; argparse exited 2 and the
service crash-looped on every systemd distro.
Failure diagnostics (what users see when BPF load fails):
- Replace the misleading "Your device is incompatible" line and print
actionable remediation hints derived from the collected diagnostics:
missing kernel headers (incl. CONFIG_IKHEADERS), Secure Boot lockdown
(now read from /sys/kernel/security/lockdown, which never appears on the
cmdline), and very old bcc.
- Write a pre-compile breadcrumb diagnostics file that is deleted on
success: a native LLVM abort (e.g. the recently fixed "LLVM ERROR:
Invalid usage of the XADD return value") kills the process inside libbcc
without raising, bypassing the except path entirely - the breadcrumb
still leaves a shareable report.
- SystemSnapper's cmd_flags diagnostic now checks tracefs as well as
debugfs, matching the loader.
Data-inventory correctness:
- FilesystemSnapper's ctypes statfs layout now uses c_long/c_ulong so it
matches the glibc ABI on 32-bit userlands too (f_type was misread on
big-endian 32-bit, failing pseudo-fs detection open).
Docs: README notes PEP 668 (externally-managed pip) and documents
--output; COMPATIBILITY_FIXES.md corrected (mainline block_rq_complete has
never exposed cmd_flags; BCC compiles in-process via libclang, it does not
shell out).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y9in1s3yxA9ZWLKmfQyUa
1a1a11a
changed the title
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
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.
Problem
The tracer kept breaking on different Linux OSes. The trigger for this PR was a hard load abort:
That specific bug is fixed in the first two commits, and a full three-track portability audit (compile-time, attach-time, environment) followed — every finding below was adversarially verified against the actual code and kernel/BCC history before being fixed, and the fix batch itself went through an adversarial review pass before push.
Fixes by failure class
Hard load failures (whole tracer aborts)
req_idgenerator):__sync_fetch_and_add's return value compiles to a BPFXADDthat can't return its old value on all kernels/LLVM. Replaced with a per-CPU counter + CPU id in the high 16 bits (also removes a duplicate-id race).skb:kfree_skbreadargs->reasonunguarded — field exists only on ≥ 5.17 (5.15.58+ LTS backports); on anything older,--networkaborted the entire load. Now gated by sniffing the tracepoint format file (-DHAS_SKB_DROP_REASON), same mechanism asHAS_CMD_FLAGS, so the gate can never disagree with the generated args struct.tcp:tcp_retransmit_skbreadargs->stateunguarded — field added in 4.20, never backported to RHEL 8's 4.18 (-DHAS_TCP_RETRANSMIT_STATE).struct bpf_timer {};under< 5.17is a redefinition error on stock Ubuntu 22.04 (5.15 + bcc 0.24). Replaced version-guarded placeholders with unconditional tag renames to private placeholder structs — correct on every bcc/kernel pair.bpf_probe_read_kernel/user(_str)on kernel < 5.5 + bcc 0.12–0.14 (stock Ubuntu 20.04) — compiles to helper ids the old verifier rejects (invalid func unknown#113). Added#ifndef-wrapped legacy-helper macros under a< 5.5guard (BCC ≥ 0.15's own injected defines win when present).bpf_get_current_cgroup_id()unguarded (4.18+ helper) — silently killed the vfs read/write/open probes on older kernels; now a compat macro returning 0.build_dentry_pathloops on pre-5.3 verifiers — a rolled loop (LLVM-dependent; a real deployment loggedloop not unrolled) is a hard back-edge rejection before 5.3. Readdir now degrades to basename-only there instead of risking the whole program.Guard/symbol drift (probe families silently dead or emitting garbage)
>= 5.17while their symbols exist from 5.16 → hit/miss/dirty/writeback probes never attached on 5.16.__do_page_cache_readahead; on ≥ 5.10 it misread thereadahead_control*as the mapping (garbage rows). Split into three correctly-typed variants (< 5.10,do_page_cache_ra≥ 5.10,page_cache_ra_order≥ 5.18).trace_shrink_folio_listreads no args but was guarded ≥ 5.17 — reclaim tracing was dead on every older kernel including 5.15 LTS. Guard removed.trace_cache_drop_folioargs misaligned on every kernel: declared(mapping, folio)but__filemap_remove_foliois(folio, shadow). Fixed prototype + guard lowered 5.18 → 5.17.vfs_renameread through a hand-rolled struct mirror that broke on < 5.12 (inode internals read as dentries) and on 6.18+ (kernel merged the mnt_idmap fields, movingnew_dentryfrom offset 40 → 32; the stale read landed ondelegated_inode→ garbage rename events). Now reads the realstruct renamedatafrom<linux/fs.h>(BCC recomputes offsets per kernel), with a 6-arg variant for < 5.12.arm64 feature loss
__arm64_sys_mremapbranch + pt_regs-unwrap variant (regs[0..3]): MREMAP events were silently absent on all arm64.__arm64_sys_io_uring_enterbranch + variant: the old fallback chain leaned on__io_uring_enter/__sys_io_uring_enter, which never existed in any mainline kernel — arm64 got no io_uring ENTER events at all. Dead branches removed.Environment / installer (why installs "succeeded" then nothing worked)
X | None,list[str], no future import) crashed at import on the stock interpreters of RHEL 8/9, Debian 11, Ubuntu 20.04, Amazon Linux — whileinstall.shenforced only "3.6+". Addedfrom __future__ import annotationsto the six affected modules; floor raised to the real minimum (3.7).--output, which argparse didn't define → exit 2 → crash-loop on every systemd distro. Added a real--output DIRoption (subparser usesSUPPRESSsoiotrc --output /x devkeeps/x).apt-get install bpfcc-tools linux-headers-$(uname -r)fused into one fatal transaction underset -e. Headers are now a separate non-fatal step with a CONFIG_IKHEADERS-aware warning and a WSL2 pointer.kernel-devel-$(uname -r)on dnf paths; git install covers rocky/alma/amzn./etc/machine-idcrashed the tracer at startup; now falls back to the dbus path, treats empty/"uninitialized" as missing (container hash-collision), and persists a generated id as last resort.__file__(a foreign checkout at the CWD could silently shadow the installedprober.cvia BCC's lookup order)./sys/kernel/security/lockdown, old bcc). A pre-compile breadcrumb file (mkstemp, O_EXCL — not a predictable /tmp name) survives native LLVM aborts that bypass Python exception handling entirely, and skips subprocess probes on the happy path.statfsctypes layout fixed (c_long/c_ulong); README documents PEP 668 pip breakage and--output; docs corrected (mainlineblock_rq_completenever hadcmd_flags; BCC compiles in-process via libclang).CI hardening
bpf_smoke.pynow mirrors the loader exactly (both trace mounts, network-Dgates) and attaches every cache-probe family as ordered fallback chains identical toKernelProbeTracker— a guard/symbol mismatch now fails CI instead of shipping. (The flat version of this check was itself caught by the pre-push adversarial review: old page-API symbols still exist on 6.x as folio-compat wrappers, which would have made CI red on every run.)Verified NOT bugs (no change; documented so nobody re-chases them)
__builtin_memset/memcpysites can never lower to libcalls (the real cliff is 1024 bytes, uniform across LLVM 7→main; a_Static_assertnow guards it).sudo iotrcfrom other directories (BCC'sargv[0]fallback rescues it) — fixed only as shadowing hardening.file->f_pathdereferences are safe (BCC's rewriter instruments them).Known follow-up (documented in-code, deliberately not in this PR)
-mllvm -bpf-stack-size=4096masks LLVM's 512-byte stack diagnostic while the kernel verifier still enforces 512/frame;struct data_t(~408 B) is stack-allocated in ~26 handlers. The durable fix (per-CPU scratch conversion handler-by-handler, then dropping the flag) needs runtime validation on real hardware and is out of scope here.Testing
bpf-compile(compiles + loads + attaches on the runner kernel) validates the modern-kernel path, now with much wider probe coverage.🤖 Generated with Claude Code