pkg/btf : Add some optimisation over the BTF resolution to reduce the amount of steps#4464
Draft
tdaudi wants to merge 7 commits intocilium:mainfrom
Draft
pkg/btf : Add some optimisation over the BTF resolution to reduce the amount of steps#4464tdaudi wants to merge 7 commits intocilium:mainfrom
tdaudi wants to merge 7 commits intocilium:mainfrom
Conversation
For better consitency, the resolveBTFPath wrapper should be placed along with the main algorithm. This commit does : - moves the generic.resolveBTFPath in pkg/btf - Rename the main function algo into private and put the wrapper into public. Note: in `buildResolveBTFConfig`, we should never use the wrapper because we want to have the very same behaviour as `buildManualBTFConfig` which does not have the upper layer of the wrapper. Signed-off-by: Tristan d'Audibert <tdaudibe@isovalent.com>
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
To be able to recall ResolveBTFPath from a specific index, we need to keep track of the value of "i". Like this, when the path is resolved, we can make the optimisation and restart at a specific index. Signed-off-by: Tristan d'Audibert <tdaudibe@isovalent.com>
The `ResolveBTFPath` should be self-sufficient in term of limitation validation. Until now the function could have crashed if the given `pathToFound` was longer than `api.MaxBTFArgDepth`. Furthermore, introducing `ErrMaxBTFDepth`. This error is different than the others because it's not directly related to BTF. So because of this separate error, we can distinguish it from the others. Signed-off-by: Tristan d'Audibert <tdaudibe@isovalent.com>
When an error fires, this will allow more control on the algorithm for optimisation. Signed-off-by: Tristan d'Audibert <tdaudibe@isovalent.com>
This commit does several things :
- When `member.Name == pathToFound[*i]`, add the Offset rather than
overwriting it. This allows to increment the value with the previous
one. This is important if we want to concatenate the values btfArg
because the algorithm will go over already existing elements.
- It will do `btfArg[*i] + 0` when the value has never been defined
- It will do `btfArg[*i] + N` to concatenate the offset
- When anonymous struct/union is encounter, the algorithm do not need
to update btfArg[*i] if `member.Name != pathToFound[*i]`. In the
original code, this was overwritten so even though the code was
useless, it was not possible to see that.
- Simplify and move isNotLastChild to the end of the function. This
ensure the logic is applied for every cases.
This commit does not break the tests :
```bash
$ go test -exec "sudo" ./pkg/sensors/tracing/ -run TestUprobeResolve
ok github.com/cilium/tetragon/pkg/sensors/tracing 0.065s
```
```bash
$ go test -exec "sudo" ./pkg/btf/ -run TestResolveBTFPath
ok github.com/cilium/tetragon/pkg/btf 0.090s
```
Signed-off-by: Tristan d'Audibert <tdaudibe@isovalent.com>
Concatenate all non-ptr entries in the `btfArg` array to reduce the
amount of iterations needed and also allow more pointer resolution.
It always concatenate downwards, because the ptr dereference comes after
the offset calculation on the bpf side
For example, for the following input :
[
{ IsInitialized: 1, IsPointer: 1, Offset: 20 },
{ IsInitialized: 1, IsPointer: 0, Offset: 10 },
{ IsInitialized: 1, IsPointer: 0, Offset: 22 },
{ IsInitialized: 1, IsPointer: 1, Offset: 100 }
]
You will have the following output
[
{ IsInitialized: 1, IsPointer: 1, Offset: 20 },
{ IsInitialized: 1, IsPointer: 1, Offset: 132 }
]
Signed-off-by: Tristan d'Audibert <tdaudibe@isovalent.com>
The test try to resolve a longer path than api.MaxBTFArgDepth. Normally, the test should fail as there is only one entry for every step. But because we concatenated every non-ptr arguments, we can reach a deeper path without changing the BPF logic. In this test path, there are 12 logical steps but 10 only ptrs. So we can concatenated by 2 which gives space for 2 more ptrs. Signed-off-by: Tristan d'Audibert <tdaudibe@isovalent.com>
tdaudi
force-pushed
the
pr/tdaudi/optimize-resolve-algo
branch
from
d066e1c to
b318a64
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR improve the
ResolveBTFPathalgorithm to reduce the amount of steps to reach data.Before explaining what the PR did, let's recap how the original algo works
Lets take an example with the following resolve :
It will produce the following steps
There is one step that is not a pointer. This can be compacted with the next steps
This PR will take the above config as an input, and return as follow
One step is saved, so this mean we can push one more step in the BTFArg config without touching the BPF code.
This PR introduce some changes on top of the
ResolveBTFPathto compact all the resolution steps to push as much elements as possible in the array.If the user defines too many entries, it will produces an error only if there are no more space (aka no non-pointer entries) in the BTFArg array.