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

Add support for weak kfuncs #1364

Merged
merged 1 commit into from
Mar 15, 2024
Merged

Conversation

dylandreimerink
Copy link
Member

This PR allows users to mark their kfunc defintions as __weak. Weak kfuncs do not cause an error during loading if the kfunc in question can't be found. Instead, a poison value is written which will cause the verifier to bail out if the instruction is evaluated.

In addition, this PR relocates LDIMM64 instructions with kfunc relocation entries. When relocated, the BTF id of the kfunc is written to the instruction's immediate field.

Together these two changes allow users to write eBPF programs which can gracefully handle the absence of a kfunc. To do so the bpf_ksym_exists macro from bpf_helpers.h can be used to check if a kfunc is present.

void invalid_kfunc(void) __ksym __weak;

__section("tp_btf/task_newtask") int weak_kfunc_missing(void *ctx) {
	if (bpf_ksym_exists(invalid_kfunc)) {
		invalid_kfunc();
		return 0;
	}

	return 1;
}

In this example the kfunc is always invalid, yet the program can still load since the branch calling the kfunc is unreachable.

So this effectively provides CO-RE capabilities for kfuncs. Especially important since kfuncs are less stable than helpers, their availability depending on kernel version, kernel compilation flags, or kernel modules being loaded or not.

Fixes: #1355

@dylandreimerink dylandreimerink marked this pull request as ready for review March 5, 2024 14:22
@dylandreimerink dylandreimerink requested a review from a team as a code owner March 5, 2024 14:22
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Nice work! Just some nits.

testdata/kfunc.c Outdated Show resolved Hide resolved
linker.go Show resolved Hide resolved
linker.go Outdated Show resolved Hide resolved
elf_reader.go Outdated Show resolved Hide resolved
@dylandreimerink dylandreimerink force-pushed the feature/weak-kfuncs branch 3 times, most recently from e95384b to fb4c071 Compare March 15, 2024 12:26
This commit allows users to mark their kfunc defintions as `__weak`.
Weak kfuncs do not cause an error during loading if the kfunc in
question can't be found. Instead, a poison value is written which will
cause the verifier to bail out if the instruction is evaluated.

In addition, this commit relocates LDIMM64 instructions with kfunc
relocation entries. When relocated, the BTF id of the kfunc is written
to the instruction's immediate field.

Together these two changes allow users to write eBPF programs which can
gracefully handle the absence of a kfunc. To do so the `bpf_ksym_exists`
macro from `bpf_helpers.h` can be used to check if a kfunc is present.

```
void invalid_kfunc(void) __ksym __weak;

__section("tp_btf/task_newtask") int weak_kfunc_missing(void *ctx) {
	if (bpf_ksym_exists(invalid_kfunc)) {
		invalid_kfunc();
		return 0;
	}

	return 1;
}
```

In this example the kfunc is always invalid, yet the program can still
load since the branch calling the kfunc is unreachable.

So this effectively provides CO-RE capabilities for kfuncs. Especially
important since kfuncs are less stable than helpers, their availability
depending on kernel version, kernel compilation flags, or kernel modules
being loaded or not.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
@lmb lmb merged commit 42cbe8f into cilium:main Mar 15, 2024
15 checks passed
dylandreimerink added a commit to dylandreimerink/ebpf that referenced this pull request Mar 21, 2024
The "test_log_fixup" selftest is broken on purpose, with the goal of
testing error messages. Due to changes in cilium#1364 we now emit a poison
instruction in `bad_relo_subprog` on a instruction which also has a
reference to a BPF-to-BPF function. This causes the symbol/reference
resolution step to fail.

This error is flaky, it only happens when `bad_relo_subprog` is loaded
first. If any other program in the collection is loaded first due to
random hash map ordering we skip the test instead due to the test
checking for verifier errors.

Since this test is broken on purpose, we previously just always hit this
skip condition. This commit disabled this selftest explicitly which
resolves the CI flakiness.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
lmb pushed a commit that referenced this pull request Mar 21, 2024
The "test_log_fixup" selftest is broken on purpose, with the goal of
testing error messages. Due to changes in #1364 we now emit a poison
instruction in `bad_relo_subprog` on a instruction which also has a
reference to a BPF-to-BPF function. This causes the symbol/reference
resolution step to fail.

This error is flaky, it only happens when `bad_relo_subprog` is loaded
first. If any other program in the collection is loaded first due to
random hash map ordering we skip the test instead due to the test
checking for verifier errors.

Since this test is broken on purpose, we previously just always hit this
skip condition. This commit disabled this selftest explicitly which
resolves the CI flakiness.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
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.

loader: handle missing kfunc gracefully
2 participants