-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Put bpf2bpf caller in a named section #714
base: main
Are you sure you want to change the base?
Conversation
Libbpf logic is that if there are multiple programs then any programs in the .text section are subprograms to be skipped when enumerating programs. So the sample was updated to follow the convention that the non-subprogram is in a section other than ".text", allowing testing the ability to load programs that call subprograms in another section. This PR updates the test accordingly and fixes a bug hit that prevented bpf2bpf calls across sections. Signed-off-by: Dave Thaler <[email protected]>
WalkthroughThe pull request includes updates to the eBPF samples subproject by changing the commit reference from Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range comments (1)
src/asm_files.cpp (1)
Line range hint
422-426
: Consider the implications of removing the symbol section match in function relocationBy simplifying the condition to queue function relocations without checking if the symbol's section matches the current section, function calls to symbols in different sections will now be queued for relocation. Ensure that the relocation logic and subsequent handling correctly process cross-section function calls to prevent potential issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- ebpf-samples (1 hunks)
- src/asm_files.cpp (1 hunks)
- src/test/test_verify.cpp (1 hunks)
🔇 Additional comments (2)
ebpf-samples (1)
1-1
: LGTM! Verify submodule commit hash.The submodule reference has been updated as expected. This change is crucial for implementing the modifications described in the PR objectives, specifically moving non-subprograms to sections other than ".text".
Please ensure that this new commit hash (aca16e72f39a1017a0782442081aaa379b58595b) corresponds to the merged state of the dependent PR (vbpf/ebpf-samples#47).
To verify the commit hash and its relationship to the dependent PR, you can run the following script:
This script will help verify that the submodule is at the correct commit and provide information to manually check its relationship to the dependent PR.
src/test/test_verify.cpp (1)
504-504
: Update to test case for bpf2bpf calls across sectionsThe test case has been modified to use the "test" section instead of ".text" for the "func" program in "bpf2bpf.o". This change aligns with the PR objectives, which mention moving the non-subprogram to a section other than ".text" to facilitate testing the verifier's capability to load programs that invoke subprograms located in different sections.
To ensure this change is consistent with the rest of the codebase, let's verify if there are any other occurrences of "bpf2bpf.o" that might need similar updates:
✅ Verification successful
Verification Complete: All Relevant Test Cases Addressed
The update to use the "test" section for the
func
program inbpf2bpf.o
is consistent with the test suite. The existing test cases appropriately differentiate between successful and failing verification scenarios by using distinct sections.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of "bpf2bpf.o" in the codebase rg "bpf2bpf\.o"Length of output: 273
Libbpf logic is that if there are multiple programs then any programs in the .text section are subprograms to be skipped when enumerating programs.
So update the sample to follow the convention that the non-subprogram is in a section other than ".text". This also enables testing the verifier's ability to load programs that call subprograms in another section.) is that if there are multiple programs then any programs in the .text section are subprograms to be skipped when enumerating programs.
So the sample was updated to follow the convention that the non-subprogram is in a section other than ".text", allowing testing the ability to load programs that call subprograms in another section. This PR updates the test accordingly and fixes a bug hit that prevented bpf2bpf calls across sections.
This PR needs vbpf/ebpf-samples#47 to be merged first.
Summary by CodeRabbit
New Features
Bug Fixes
Tests