link: always include requested symbol in kprobe creation errors #959
Conversation
There was a problem hiding this comment.
Thanks, that would be lovely! 😸
So now it will look like this:
creating perf_kprobe PMU (tried '_x64_sys_listen'): token __arm64___x64_sys_listen: not found: no such file or directory
It still requires knowing that the lib is retrying on fail with an arch-specific prefix but at least the original symbol appears in the error which is excellent!
link/kprobe.go
Outdated
| @@ -199,7 +199,7 @@ func kprobe(symbol string, prog *ebpf.Program, opts *KprobeOptions, ret bool) (* | |||
| return tp, nil | |||
| } | |||
| if err != nil && !errors.Is(err, ErrNotSupported) { | |||
| return nil, fmt.Errorf("creating perf_kprobe PMU: %w", err) | |||
| return nil, fmt.Errorf("creating perf_kprobe PMU (tried '%s'): %w", symbol, err) | |||
There was a problem hiding this comment.
Maybe it's better to just return the original error?
origErr := pmuKprobe(...)
fallbackErr := pmuKprobe(...)
if fallbackErr != nil { return origErr }There was a problem hiding this comment.
The best would be both, being explicit about the fact that symbol1 and adaptedSymbol1 failed. Losing the fallbackErr could make the behavior not very clear I think.
There was a problem hiding this comment.
Okay if we stick with my proposal? It's a simple change and conveys that both symbols were attempted in a familiar format. Omitting the fallback error will obscure the prefixing behaviour, so the caller will probably conclude they need to add the prefix explicitly and retry, which I'd like to avoid.
Go 1.20 introduces errors.Join(), but that uses newlines, which I'd also like to avoid. 🙂
ti-mo
force-pushed
the
tb/kprobe-prefix-error
branch
from
a2aa214
to
1679907
Compare
Before, when a caller would try to set a kprobe on e.g. __x64_sys_listen, the error message would look something like 'creating perf_kprobe PMU: token __arm64___x64_sys_listen: not found: no such file or directory'. This doesn't mention that the requested symbol has been attempted in addition to the prefixed symbol mentioned in the error message. If a symbol doesn't exist, which is going to be the vast majority of cases, the caller will only see the prefixed symbol echoed in the error message, which can make them wonder if the original symbol has been attempted at all. This patch makes PMU and tracefs errors more uniform and always mentions the requested symbol in the error string, which only used to happen for tracefs. Also documents the behaviour on K(ret)probe(). Fixes cilium#956 Signed-off-by: Timo Beckers <timo@isovalent.com>
ti-mo
force-pushed
the
tb/kprobe-prefix-error
branch
from
1679907
to
2c68741
Compare
Before, when a caller would try to set a kprobe on e.g. __x64_sys_listen, the error message would look something like 'creating perf_kprobe PMU: token __arm64___x64_sys_listen: not found: no such file or directory'. This doesn't mention that the requested symbol has been attempted in addition to the prefixed symbol mentioned in the error message.
If a symbol doesn't exist, which is going to be the vast majority of cases, the caller will only see the prefixed symbol echoed in the error message, which can make them wonder if the original symbol has been attempted at all.
This patch makes PMU and tracefs errors more uniform and always mentions the requested symbol in the error string, which only used to happen for tracefs.
Fixes #956