-
Notifications
You must be signed in to change notification settings - Fork 143
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
Introduce and use vmlinux crate #739
Conversation
This change converts all examples over to using the vmlinux crate for access to the vmlinux.h header file. Signed-off-by: Daniel Müller <deso@posteo.net>
Remove the examples/vmlinux/ folder and all the vendored vmlinux.h header files, now that they are no longer used. Signed-off-by: Daniel Müller <deso@posteo.net>
This change migrates the integration tests over to using the vmlinux crate for their vmlinux.h needs. It then also removes the one remaining vendored vmlinux.h copy from the repository. Signed-off-by: Daniel Müller <deso@posteo.net>
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.
LGTM. Do you plan to convert libbpf-bootstrap rust examples to this as well?
use std::path::Path; | ||
use std::path::PathBuf; | ||
|
||
// TODO: Ideally we'd deduplicate contents at this level (as well; not just on |
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.
those are also huge text files which would compress very nicely, but I assume it's impossible in const context?
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.
Likely not in a const
context, yeah. We could probably have a function based interface if need be and load and cache stuff on demand. I mostly got nerd-sniped here and wanted to make it work as I had originally envisioned...
The other avenue would to exclude unnecessary headers based on features. Either way, it's probably not urgent unless/until we use this functionality in anything but example & test code.
Not immediately, but if we decide to publish this crate eventually then yes, happy to adjust the code there. |
Bump the minimum supported Rust version to 1.71. This version will be required with subsequent changes, as it provides a const version of ptr::read(). Signed-off-by: Daniel Müller <deso@posteo.net>
Our handling of the vmlinux.h header file is super painful. That has many reasons, including, but not limited to: - nothing feels self-contained, as everything is relying on some abstruse C header file somewhere - we kind of sort of wanted to avoid shipping it so far and so every component started vendoring their own copy - that makes maintenance a nightmare - some components do so taking the current target architecture into account, others not; limiting possible compilation targets - because unit tests are part of the containing library, which *is* shipped, we are basically prevented from granting unit tests access to vmlinux.h - just finding the right path to the file is cumbersome and the logic for doing so gets replicated all over the place This change introduces a crate that tries to solve most (all?) of these problems. It vendors vmlinux.h for various architectures and provides file system as well as Rust-level access to contents. It also takes care of honoring the current target architecture in doing so. Long term we probably should consider publishing it, which would enable usage from other repositories such as libbpf-bootstrap. We'd likely need to version it in accordance with the kernel version that the headers correspond to. For now, it's just a workspace member and will only be used as dev-dependency. Signed-off-by: Daniel Müller <deso@posteo.net>
Please see individual commits for descriptions.