Resolve kfuncs based on essential name - #2060
Conversation
|
While working on this I had some doubts about the best way to go about the changes in the btf package. We can add additional "fuzzy" methods or we can do a breaking API change and allow specifying exact or fuzzy matching via an argument. I implemented both, the second commit implements it with additional methods, and the third commit refactors it to be breaking. That way we can get a feel for what one or the other would look like. I can squash or drop commits depending on which we like better. Please leave your feedback. |
6c76107 to
178089d
Compare
|
Thanks for the proposals! If you ask me: let's pick neither solution. Let's keep essential names an implementation detail, as they don't seem useful outside the scope of conflict resolution for ksyms within the loader. I can't really come up with use cases beyond ELF loading. I don't think we should add another flavor ( I find it rather surprising/problematic that the existing implementation calls This way, it becomes explicit, we don't break API (although it's technically a breaking change we should document), and we don't unnecessarily complicate Spec querying. Alternatively, we export |
178089d to
cd48150
Compare
This test adds two case, first is a weak kfunc with the correct signature but with the ___local suffix. The desired behavior is that this kfunc gets resolved since we should ignore the suffixes when looking up the kfunc in the kernel. The second case is a weak kfunc with a suffix as well but this time it has the wrong signature. We expect this kfunc to be left unresolved and not throw an error. Together these properties allow users to write CO-RE programs that can implement compatibility fallbacks when kfunc signatures change between kernel versions. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
cd48150 to
0131640
Compare
0131640 to
dd88ddd
Compare
The ultimate goal of this change is to apply the ignore suffix rule to kfuncs. This allows a user to define multiple flavors of kfuncs as they may appear in different kernel versions, without name collisions. During kfunc relocation, we strip the ___... suffix before looking up kfunc candidates in the vmlinux BTF. This required changing the behavior of btf.Spec.AnyTypesByName, which would internally call newEssentialName on the provided name to lookup types but then still apply name filtering on the name with the suffix. By moving the suffix stripping out of the btf.Spec method responsibility the caller can decide if they want to find exact matches or if they want to apply the ignore suffix rule. This also surfaced a test with a wrong assumption. We were asserting that if the kernel contained a `ambiguous` and `ambiguous___flavor` type that we should match both during relocation. But this is not how the ignore suffix rule works, suffixes are only stripped off of user provided type names (those in ELF BTF), the logic should not be applied to kernel types. So removed tests with the bad assumptions and added a new test to assert the correct ambiguous relocation behavior. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
dd88ddd to
97f54ce
Compare
|
@ti-mo made the changes, this should be closer to your suggestion. |
As it stands we only ever resolve kfuncs when their name in the kernel matches exactly with the name in a program we are trying to load. However, there is a feature in BTF/CORE called the "ignored suffix rule" which allows users to add a suffix separated by three underscores (___), and have them be ignored during type matching.
The reason for this feature is that it allows a user to define multiple variants of a type or kfunc in this case. For example the
scx_bpf_reenqueue_localwas modified in kernel 6.19, going from returning au32to returningvoid. A valid change since kfuncs do not have the same stability guarantee as old school helper functions.In order for a user to write a single program that works on kernels before and after the change we have to be able to discover which version is available and use the correct one, this looks like:
Original from https://elixir.bootlin.com/linux/v7.1.3/source/tools/sched_ext/include/scx/compat.bpf.h#L396
The expected result is that on pre-6.19 kernels the
___v1symbol is resolved and the___v2___compatversion is unresolved, sobpf_ksym_existsreturns false and the___v1path is always chosen. On newer kernels the v2 kfunc is resolved and thus called.This PR makes the needed changes.
Fixes: #2057