-
Notifications
You must be signed in to change notification settings - Fork 185
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
btfgen: Embed btf generated files into the binary #2044
Conversation
2c3c81d
to
33dd5c8
Compare
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 the idea as we only get the BTF files at compile time and we will use the good one at runtime only if ENABLE_BTFGEN was set a compile time and if current kernel does not have BTF.
I would advice to split the commit by having one which adds the btfgen
package and the other using it.
I will test it later though.
33dd5c8
to
f35098d
Compare
I wanted to do that but given it involves changes to the CI, makefile and gadges, it's difficult to split in logic pieces that work alone, so I prefer to have a big commit with all changes in. |
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.
Sort of tested it on custom built kernel but as btfhub
does not know my kernel this of course does not work:
root@vm-amd64:~# zgrep DEBUG_INFO /proc/config.gz
CONFIG_DEBUG_INFO_NONE=y
# CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT is not set
# CONFIG_DEBUG_INFO_DWARF4 is not set
# CONFIG_DEBUG_INFO_DWARF5 is not set
root@vm-amd64:~# ./share/kinvolk/inspektor-gadget/ig-linux-amd64 trace open --host
Error: initializing operators: initializing operator "LocalManager": initializing manager: starting container fanotify: readPrivateDataF0
root@vm-amd64:~# more /tmp/foo
Error: initializing operators: initializing operator "LocalManager": initializin
g manager: starting container fanotify: readPrivateDataFromFd: private_data is 0
root@vm-amd64:~# uname -a
Linux vm-amd64 6.5.0-rc7-00021-ga6fda6d4d5fc #96 SMP PREEMPT_DYNAMIC Tue Sep 19 10:12:26 EEST 2023 x86_64 GNU/Linux
I will create the corresponding BTF file for this kernel and test again but the logic seems to be good.
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.
It works like a charm! Awesome job:
# Just removed the one about map.
$ bpftool gen min_core_btf ../../linux/linux/.tmp_vmlinux.btf foo.btf $(find . -name '*.o')
$ mv foo.btf pkg/btfgen/btfs/x86/debian/11/x86_64/6.5.0-rc7-00021-ga6fda6d4d5fc.btf
# Inside VM
root@vm-amd64:~# uname -a
Linux vm-amd64 6.5.0-rc7-00021-ga6fda6d4d5fc #98 SMP PREEMPT_DYNAMIC Tue Sep 19 14:13:16 EEST 2023 x86_64 GNU/Linux
root@vm-amd64:~# zgrep DEBUG_INFO /proc/con
config.gz consoles
root@vm-amd64:~# zgrep DEBUG_INFO /proc/config.gz
CONFIG_DEBUG_INFO_NONE=y
# CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT is not set
# CONFIG_DEBUG_INFO_DWARF4 is not set
# CONFIG_DEBUG_INFO_DWARF5 is not set
root@vm-amd64:~# ./share/kinvolk/inspektor-gadget/ig-linux-amd64 trace open --host | head
RUNTIME.CONTAINERNAME PID COMM FD ERR PATH
283 ig-linux-amd64 43 0 /proc
283 ig-linux-amd64 43 0 /proc
283 ig-linux-amd64 43 0 /proc
283 ig-linux-amd64 43 0 /proc
283 ig-linux-amd64 43 0 /sys/kernel/tracing/events/sysc…
283 ig-linux-amd64 43 0 /proc
283 ig-linux-amd64 43 0 /proc
283 ig-linux-amd64 43 0 /proc
283 ig-linux-amd64 43 0 /proc
root@vm-amd64:~#
I am wondering if some documentation which links to a tutorial regarding bpftool gen min_core_btf
would help?
f35098d
to
d75a044
Compare
I wrote this blog post some time ago -> https://www.inspektor-gadget.io//blog/2022/03/btfgen-one-step-closer-to-truly-portable-ebpf-programs/, however it's not clear for me where to link this in IG documentation. |
I was refeering to this blog post. |
d75a044
to
0fe1ba2
Compare
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.
Initial comments. I'll continue soon.
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.
One blocking comment about the network dispatcher.
The rest looks good to me.
I didn't review the CI part.
# Filter out BPF objects that only contain BPF maps without BPF programs | ||
BPF_PROGS_O_FILES = $(filter-out pkg/gadgets/trace/network/tracer/graphmap_bpfel%,$(BPF_ALL_O_FILES)) | ||
BPF_PROGS_O_FILES = $(filter-out pkg/gadgettracermanager/containers-map/containersmap_bpfel%,$(BPF_ALL_O_FILES)) |
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.
Similarly to the discussion in #2003 (comment), the map from the pkg/gadgettracermanager/containers-map package could be created programmatically.
But that can be in a different PR.
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.
Right, the difference is that in this case providing BTF information manually for the map is not that easy, and in this case it could be appreciated to pretty print the contents of the map while debugging.
@@ -73,6 +74,12 @@ func (se *SocketEnricher) start() error { | |||
|
|||
kallsyms.SpecUpdateAddresses(specIter, []string{"socket_file_ops"}) | |||
|
|||
opts := ebpf.CollectionOptions{ | |||
Programs: ebpf.ProgramOptions{ | |||
KernelTypes: btfgen.GetBTFSpec(), |
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.
Happy to see that the socket enricher can use btfgen!
This commit changes the approach regarding btfgen, now the btf generated files are embeded in the golang binary, which means that now it's also available for ig. This also updates the CI to execute btfgen before building ig or ig-k8s. Signed-off-by: Mauricio Vásquez <mauriciov@microsoft.com>
0fe1ba2
to
d8f37ff
Compare
This commit changes the approach regarding btfgen, now the btf generated files are embeded in the golang binary, which means that now it's also available for ig.
This also updates the CI to execute btfgen before building ig or ig-k8s.
How to test
Getting a host without BTF
Ironically it's difficult to get a machine without BTF support these days. I used a ubuntu focal64 through vagrant (
config.vm.box = "ubuntu/focal64"
). It ships a kernel version that already has btf support, hence I installed another kernel that doesn't include BTF and rebooted the machine:After reboot
Install docker (it should not be required, I'll open an issue soon about this)
Before this PR
After this PR
Option A: Locally compiling ig with btf enabled
Option B: Getting ig from a CI run
ig is compiled in the CI with btfgen enabled, hence downloading the ig binary from a run of this PR should work too.
Fixes #752