Skip to content
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

btf: fix some split BTF shortcomings #861

Merged
merged 4 commits into from
Nov 23, 2022
Merged

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Nov 20, 2022

Fix some outright bugs and some annoyances with split BTF support. It's still not entirely great, but this is better than nothing.

Requirement for #859.

@lmb
Copy link
Collaborator Author

lmb commented Nov 21, 2022

API changes:

--- old.txt	2022-11-20 16:21:35.946436516 +0000
+++ new.txt	2022-11-20 16:21:29.014452875 +0000
@@ -21,6 +21,9 @@

 FUNCTIONS

+func ClearCachedKernelSpec()
+    ClearCachedKernelSpec flushes any cached kernel type information.
+
 func LoadSpecAndExtInfosFromReader(rd io.ReaderAt) (*Spec, *ExtInfos, error)
     LoadSpecAndExtInfosFromReader reads from an ELF.

@@ -270,11 +273,9 @@
 func (h *Handle) Info() (*HandleInfo, error)
     Info returns metadata about the handle.

-func (h *Handle) Spec(base *Spec) (*Spec, error)
+func (h *Handle) Spec() (*Spec, error)
     Spec parses the kernel BTF into Go types.

-    base is used to decode split BTF and may be nil.
-
 type HandleInfo struct {
 	// ID of this handle in the kernel. The ID is only valid as long as the
 	// associated handle is kept alive.

Copy link
Collaborator

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack overall, but this needs to take the changes of #855 into account, I think there's some overlap with btf_spec.go:vmlinux{,Spec}.

btf/btf.go Outdated Show resolved Hide resolved
btf/btf.go Outdated Show resolved Hide resolved
btf/btf.go Outdated Show resolved Hide resolved
btf/btf.go Show resolved Hide resolved
No other source code changes intended.
The kernel's BTF is currently cached in package ebpf, since there are
some users that don't want to keep the cached types around. This is
a problem if we want to this data from package btf.

Move caching into the btf package, but provide a helper to remove the
cached copy.

Internal users of the package can also tell whether the kernel BTF
comes from /sys or from parsing a vmlinux ELF. This is useful when
parsing kmod split BTF.
We currently punt on the user to provide us with a base spec to
parse kmod BTF via a Handle. This isn't very ergonomic, and also unnecessary.

If kmod / split BTF is present, we can also rely on /sys/... being available.
We can therefore automatically use the (cached) kernel types when retrieving
a Spec from a Handle.
Copy link
Collaborator

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Would like some clarification around the changes to TypeByID.

btf/btf.go Show resolved Hide resolved
btf/btf.go Show resolved Hide resolved
btf/btf.go Outdated Show resolved Hide resolved
TypeByID currently assumes that the first ID in s.types is 0, which
is not correct when dealing with split BTF.
@lmb lmb merged commit 365d07f into cilium:master Nov 23, 2022
@lmb lmb deleted the btf-split-fixes branch November 23, 2022 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants