new(bpf): errmetrics improvements #4120
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It allows to error-check any bpf helper that return an int, appending a metrics in case of failure. Signed-off-by: Federico Di Pierro <[email protected]>
mtardy
added
the
release-note/misc
FedeDP
force-pushed
the
new/bpf_errmetrics_improvements
branch
from
0f5882b to
8d555c2
Compare
|
cc @kkourt since he made the initial PR :) |
Adapted userspace `tetra` code to inject the bpf func helper in the metric name, like: ``` Location Error Count FnProbeRead@bpf/process/bpf_execve_event.c:61 E2BIG (7) 2 ``` To avoid using a `funcids.h` file like was done for `fileids.h`, i just cast the bpf helper to an u64, since its value is actually the `BPF_FUNC_foo` enum value (`enum bpf_func_id`); see `bpf/include/api.h`. In userspace instead, we leverage `github.com/cilium/ebpf/asm` to cast the `bpf_func_id` back to its string value. Signed-off-by: Federico Di Pierro <[email protected]>
Since it is much harder to prevent people from using relative includes, this is a simple solution to avoid potential issues. Of course we should avoid name clashes for our bpf source files. Also, leverage clang builtin `__auto_type` to use correct return type from real bpf helper call. Signed-off-by: Federico Di Pierro <[email protected]>
… `compile_time_error`. Compiler complained about multiple `extern` definition of same function with different `error` function attributes. Signed-off-by: Federico Di Pierro <[email protected]>
…ore. Signed-off-by: Federico Di Pierro <[email protected]>
For now, only for `map_update_elem` helper. Signed-off-by: Federico Di Pierro <[email protected]>
…ader. Signed-off-by: Federico Di Pierro <[email protected]>
FedeDP
force-pushed
the
new/bpf_errmetrics_improvements
branch
from
8d555c2 to
fffe7e1
Compare
|
Just force-pushed rewording a commit :)
It is complaining about |
mtardy
requested review from
mtardy and
kkourt
and removed request for
ExceptionalHandler
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR improves bpf errmetrics intoduced in #3205.
It does so with multiple things (split by commit):
with_errmetricsmacro that supersedesmap_update_elem__errmetrics__FILE_NAME__macro instead of just__FILE__; this prevents issues with relative includes: prior to this, the files (eg:bpf_process_event.h) included also with relative patsh (eg:../bpf_process_event.h) would require to be added twice to thefileidswith both paths. Now, we will only report the filename. We don't expect any name clash between bpf sources!with_errmetricsmacro worked fine until we had multiple comp time errors in the same source file. At that point, the compiler complained about multipleexternfunction definition with differenterrorattributes. I fixed it by appending a__COUNTER__to the end of the comp time error function namebpf/tetragon/fileids.hshould not be formatted; added it to the ignore listwith_errmetricsmacro formap_update_elemcmd/bpf-fileidsgo command to leveragecgoinstead of manually parsing the C header. Only downside: we are no more able to customizefileids.hpath. I don't think that is needed at all since this command is mostly made to be automatically ran by makefile.Example output: