ebpf: move parca-only bpfdefs.h helpers to bottom of file#288
Merged
Conversation
The bpf_ringbuf_*, bpf_probe_read_user_str, and bpf_get_attach_cookie declarations are parca-only (used by cuda.ebpf.c and usdt_test.ebpf.c) but sit in the same block where upstream adds new helpers, so they collide with most upstream merges. Move them to a trailing block to keep that region byte-aligned with upstream. Also drops the obsolete \n comment above #define printt (the macro no longer adds \n).
0a64b0e to
64a3a4a
Compare
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.
Moves parca-only helper declarations in
support/ebpf/bpfdefs.h(bpf_ringbuf_reserve/submit/discard,bpf_probe_read_user_str,bpf_get_attach_cookie) from their current interleaved positions into a single trailing block at the bottom of the non-TESTING_COREDUMParm. No behavior change.Why
These three helpers are only used by parca-specific eBPF programs (
support/ebpf/cuda.ebpf.candsupport/ebpf/usdt_test.ebpf.c), but they sit inside thestatic <type> (*<name>)(...) = (void *)BPF_FUNC_<name>;block that upstream actively maintains. Upstream's typical insertion pattern is "add the new helper alphabetically next to the existing ones", which is exactly where parca's additions live now.Concrete consequence:
upstream-merge.shcurrently can't auto-resolve upstream PR0320a2a8(open-telemetry#1278 Recover python frames when BPF fails to read PyCodeObject), which only adds a single small helper. The 3-way merge sees parca's additions overlap with upstream's hunk and stops the script before open-telemetry#1278.What's moved
static void *(*bpf_ringbuf_reserve)/_submit/_discard(lines 122-125, betweenbpf_get_prandom_u32andbpf_trace_printk)#endif // !TESTING_COREDUMPbpf_probe_read_user_strdeclaration + kernel-doc (lines 137-184)bpf_get_attach_cookiedeclaration + kernel-doc (lines 186-203)Also dropped: the now-obsolete
// The sizeof in bpf_trace_printk() must include \0, else no output is generated. The \n is not needed on 5.8+ kernels, but definitely on 5.4 kernels.comment above#define printt. The macro no longer adds\n(upstream PR open-telemetry#1422 made____fmtstatic and removed it), so the comment is historical context, and it sits in the same conflict-prone region.Net effect on bpfdefs.h
96 lines removed, 21 added — the kernel-doc comments for
bpf_probe_read_user_strandbpf_get_attach_cookie(66 lines combined) are stripped because they're verbatim copies of the Linux kernel man pages and aren't necessary for compilation. If we want them back as reference, we can keep a one-line comment with a kernel.org link.The upstream-touched region of bpfdefs.h is now byte-identical to upstream's view of the same region (modulo unrelated parca commits in
tsd_get_base, etc.).Validation
make -C support/ebpf clean && make TARGET_ARCH=amd64 && make TARGET_ARCH=arm64: clean build, instructions amd64 51384 / arm64 64397.TestCUDAEndToEnd{SingleShot,MultiProbe}.Follow-up
After this lands, the next attempted
upstream-merge.shrun should be able to auto-resolve open-telemetry#1278 (and likely several commits past it). That progress is unblocked by this PR.