-
Notifications
You must be signed in to change notification settings - Fork 273
feat(bpf): implement runtime vmlinux.h generation,fix CO-RE #1984
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
base: main
Are you sure you want to change the base?
Conversation
rbtr
left a comment
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.
I think this is a good idea already, but there's same ambiguity to me around when/if the header could be regenerated instead of always. Kernel drift underneath us is a concern currently, since we can live-upgrade kernels.
| // Check if vmlinux.h already exists to avoid regenerating it unnecessarily? | ||
| // However, if the pod restarts on a different node (unlikely for same pod instance but possible if volume persisted?), | ||
| // or if we want to be sure. | ||
| // Given the startup time is not critical and it's fast, let's generate it. |
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.
how fast?
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.
i'm thinking we should write it to disk (/tmp/retina like you've already referenced below) and skip regenerating it. but what happens if the kernel is updated? how can we tell when the kernel has drifted from our cached vmlinux.h? if there was an easy way to do that, i would say fully persist it to disk and only regenerate when that happens...but i'm not sure we can do that.
| runtimeHeaderDir := "/tmp/retina/include" | ||
| if err = loader.GenerateVmlinuxH(ctx, runtimeHeaderDir); err != nil { | ||
| dr.l.Warn("Failed to generate vmlinux.h, falling back to static headers", zap.Error(err)) | ||
| } |
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.
I don't think we should invoke it from every plugin. Seems like GenerateVmlinuxH should be invoked before plugins are started, and the path should be a constant plugin can refer to.
| l.Error("Failed to generate vmlinux.h", zap.Error(err)) | ||
| // If bpftool fails (e.g. /sys/kernel/btf/vmlinux doesn't exist), we might want to fallback or error out. | ||
| // If it fails, the compilation will likely fail later if we rely on this header. | ||
| return fmt.Errorf("failed to run bpftool: %w", err) |
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.
I like this error message better - l.Error("Failed to generate vmlinux.h", zap.Error(err)) . Seems more accurate as bpftool can run and fail for myriad reasons.
Also, no need to log it here as caller would probably do that.
| // Generate vmlinux.h | ||
| runtimeHeaderDir := "/tmp/retina/include" | ||
| if err = loader.GenerateVmlinuxH(ctx, runtimeHeaderDir); err != nil { | ||
| dr.l.Warn("Failed to generate vmlinux.h, falling back to static headers", zap.Error(err)) |
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.
I am curious about the consequence of this compilation failure. Seems like subsequent datapath errors can be attributed to this failure, but we won't have any idea if the warning log rotates after some time.
| tar | ||
| RUN mkdir -p /tmp/bin | ||
| RUN arr="clang tcpdump ip ss iptables-legacy iptables-legacy-save iptables-nft iptables-nft-save cp uname" ;\ | ||
| RUN arr="clang bpftool tcpdump ip ss iptables-legacy iptables-legacy-save iptables-nft iptables-nft-save cp uname" ;\ |
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.
I would rather have an init image with bpftool that can compile the vmlinux, and we can mount the directory to the agent image (optional). This way agent image only has the essentials.
Description
This PR implements runtime generation of
vmlinux.husingbpftoolto ensure eBPF programs are compiled against the host kernel's BTF information. This addresses issues where staticvmlinux.hheaders cause CO-RE relocation failures due to kernel structure offset mismatches.Changes
linux-tools-commonandlinux-tools-genericto installbpftool.GenerateVmlinuxHhelper to dump BTF from/sys/kernel/btf/vmlinux.dropreason,packetforward, andpacketparserto generate and use the runtimevmlinux.hduring compilation.Fixes #1777
Related Issue
If this pull request is related to any issue, please mention it here. Additionally, make sure that the issue is assigned to you before submitting this pull request.
Checklist
git commit -S -s ...). See this documentation on signing commits.Screenshots (if applicable) or Testing Completed
Additional Notes
Add any additional notes or context about the pull request here.
Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.