link, internal: generate full linkinfo for union subtypes#1359
link, internal: generate full linkinfo for union subtypes#1359lmb merged 3 commits intocilium:mainfrom
Conversation
rgo3
changed the title
| @@ -355,32 +353,68 @@ func (l *RawLink) Info() (*Info, error) { | |||
| var extra interface{} | |||
There was a problem hiding this comment.
hum, so the idea is to run the link info syscall twice, first to get the link info type and second to read it into specific type?
could we just store the link type, like to have Link interface function that returns that? seems wasteful to do 2 same syscalls just to get it into convenient type
There was a problem hiding this comment.
We can move the info.Type specific code from this function to new Info() methods on the concrete link types, like you did with KprobeMulti.Info() before. Then the only caller of RawLink.Info() is wrapRawLink which only really cares about info.Type anyways. This way Info() only does a single call in the common case (caveat: sometimes we need two calls anyways to retrieve buffer sizes, etc.).
We can make that change in this PR, or as a follow up, or once you've landed your kprobe missed count changes. Up to you and @rgo3.
There was a problem hiding this comment.
How would having separate Info() methods on all concrete link types as well look in practice when loading a pinned link from bpffs and get specific information from it?
- Load pinned link
- Call generic Info method
- Check type of returned object that satisfies the
Linkinterface - Assert concrete link type
- Call type dependent Info method
There was a problem hiding this comment.
That sounds right to me. 1-4 happen behind the scenes in LoadPinnedLink. 5 happens when a user does KprobeMulti.Info().
|
I've talked to @olsajiri yesterday that I would take a look today. I think the fastest solution would be merging this PR, jiri can base his work on top and then I look into refactoring after? That we we enable Jiri to continue and I get a little more time 😁 |
|
SGTM |
This defines the extra LinkInfo types directly in pkg link instead of relying on the generated types in pkg internal/sys. This is a preparatory commit as the next commit changes how we generate extra LinkInfo types. Signed-off-by: Robin Gögge <[email protected]>
Signed-off-by: Robin Gögge <[email protected]>
Signed-off-by: Robin Gögge <[email protected]>
rgo3
force-pushed
the
linkinfo-per-union-member
branch
from
b3c7c48 to
52685fb
Compare
|
Rebased on top of the netkit changes and did the same refactoring needed for that link type, otherwise no changes. |
|
Thanks 🙏 |
4 participants
The current generic abstraction of
bpf_link_infomakes it harder to implement features like #1295. Instead of splitting LinkInfo into a generic part and having extra structs to represent the fields of union members inbpf_link_info, with this PR we generate the full LinkInfo struct for each member the union inbpf_link_info.