bpftool: add support for split BTF to gen min_core_btf #129
Conversation
brycekahle
force-pushed
the
bryce.kahle/kmod-min
branch
from
8dc2c36
to
8e59ae3
Compare
There was a problem hiding this comment.
Hi, and thanks for the patch!
Can you please motivate the change in your commit description? I'm not super familiar with split BTF and minified BTF generation, but unless I'm wrong we need to pass the base BTF on the command line for this to work, don't we? If so, I'd like to have:
- An example invocation in the commit description, to help with the review
- Ideally, an example in the relevant docs (
Documentation/bpftool-gen.rst) - An update of the docs if option
-Bis required for this to work (so far we only document-Bin thebpftool btfman page), along with the update of the help string in gen.c'sdo_help()
Then I won't merge the commit from this repo, sorry. Instead, you need to submit it to the BPF mailing list (see the relevant docs - I can help if necessary). Please be sure to CC the BPF maintainers and myself, and to prefix your commit title with bpftool: .
brycekahle
force-pushed
the
bryce.kahle/kmod-min
branch
from
8e59ae3
to
b719879
Compare
|
Updated, let me know if it looks good for me to submit to the mailing list. |
brycekahle
changed the title
There was a problem hiding this comment.
Looks better, thanks! A few more nits:
- Please wrap your commit description (except for the example invocations) on ~72 char-wide lines
- The second paragraph for the description of the
-Boption is probably not required forbpftool gen, see below.
After that it looks good from my side, although I've not tested the changes.
I forgot to say it because it seemed obvious to me, but just to be sure, to submit to the BPF mailing list you'll have to port your changes to the Linux git repo (the bpf-next tree). Bpftool sources are under tools/bpf/bpftool/.
brycekahle
force-pushed
the
bryce.kahle/kmod-min
branch
from
b719879
to
bdcd0a4
Compare
|
Updated |
There was a problem hiding this comment.
Reading again the description, I'd recommend adding this note:
A minimized module BTF will still not contain vmlinux BTF types, so you should always minimize the vmlinux file first, and then minimize the kernel module file.
to either the option description, or to your example, in the man page. I suspect that, otherwise, users will try to create a the minimised object in one step by passing the path of both the vmlinux and module BTF paths.
By the way, with your patch, you currently end up with two generated minimised BTF objects, mod-min.btf and vm-min.btf, right? Wouldn't it make sense to look into passing all BTF objects (vmlinux and modules) in one step, and generate a single minimised BTF object from that? Or does it defeat what you're trying to achieve? (Sorry for failing to raise this on the previous review.)
Also, one other nit below.
There may be multiple kernel modules in play for a single eBPF object file, since it can contain multiple programs and they each may probe different modules. Since a kernel module BTF has conflicting type IDs (they all start at max(vmlinux) + 1) with other kernel module BTF, I'm not sure there is a way with libbpf to load them all at the same time. |
brycekahle
force-pushed
the
bryce.kahle/kmod-min
branch
from
bdcd0a4
to
ccb9112
Compare
|
Updated |
There was a problem hiding this comment.
There may be multiple kernel modules in play...
Might be worth mentioning in your commit description. I don't have a suggestion to address this, but others on the BPF ML may have ideas.
Otherwise, it looks nearly all good from my side (with a last nit below).
Looking forwards to seeing your patch on the ML :)
Enables a user to generate minimized kernel module BTF. If an eBPF program probes a function within a kernel module or uses types that come from a kernel module, split BTF is required. The split module BTF contains only the BTF types that are unique to the module. It will reference the base/vmlinux BTF types and always starts its type IDs at X+1 where X is the largest type ID in the base BTF. Minimization allows a user to ship only the types necessary to do relocations for the program(s) in the provided eBPF object file(s). A minimized module BTF will still not contain vmlinux BTF types, so you should always minimize the vmlinux file first, and then minimize the kernel module file. Example: bpftool gen min_core_btf vmlinux.btf vm-min.btf prog.bpf.o bpftool -B vm-min.btf gen min_core_btf mod.btf mod-min.btf prog.bpf.o Signed-off-by: Bryce Kahle <[email protected]>
brycekahle
force-pushed
the
bryce.kahle/kmod-min
branch
from
ccb9112
to
7ba210f
Compare
|
Welp, first ML attempt was a failure. Gmail borked the formatting. I'm looking into my options, but my work email is pretty locked down so I cannot even get an app password. |
|
Yeah GMail interface is known to break patch formatting. I'm using Have you looked into using a local client that supports 0Auth2 to connect to GMail (I think Thunderbird at least can)? I don't think you'd need an app password to do that? If you don't find a solution for your work email, you can always use a personal email to send your patch. If none of these work or is convenient, I'm happy to post the patch on your behalf. |
|
Closing because it is on ML now |
No description provided.