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

prog,btf: explicitly refuse ambiguous AttachTo targets #897

Merged
merged 1 commit into from Jan 13, 2023

Conversation

ti-mo
Copy link
Collaborator

@ti-mo ti-mo commented Jan 13, 2023

See #894 for more context. Explicitly refuse loading programs with multiple AttachTo candidates. BTF needs to carry more information to allow disambiguating between them. The kernel API likely needs to be extended to allow specifying which candidate to pick.

@ti-mo ti-mo requested a review from lmb January 13, 2023 09:59
btf/btf.go Outdated Show resolved Hide resolved
@@ -891,6 +891,12 @@ func findTargetInKernel(name string, progType ProgramType, attachType AttachType
}
return module, id, nil
}
// See cilium/ebpf#894. Until we can disambiguate between equally-named kernel
Copy link
Collaborator

Choose a reason for hiding this comment

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

This reads to me like we'll at some point be able to drop this. Based on your research, we will never be able to disambiguate in all cases, so this won't go away. I'd probably just drop the comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's been talks of including symbol addresses in Func, but I've suggested to also include source file names, otherwise users wouldn't be able to reliably specify (in ELF section names) which symbol they're interested in.

There are more than 500 (unique) ambiguous symbols in 6.1, so this is a pretty heavy limitation that should be lifted at some point.

See cilium#894 for more context. Explicitly refuse loading programs with
multiple AttachTo candidates. BTF needs to carry more information to allow
disambiguating between them. The kernel API likely needs to be extended to
allow specifying which candidate to pick.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo merged commit 80a7e31 into cilium:master Jan 13, 2023
@ti-mo ti-mo deleted the tb/attach-ambiguous-ksyms branch January 13, 2023 14:38
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.

None yet

2 participants