-
Notifications
You must be signed in to change notification settings - Fork 95
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
New CreateMap API implementing bpf_map_create, Type/name accessors #138
Conversation
Signed-off-by: grantseltzer <[email protected]>
2f2b397
to
bd41892
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome! Thanks for handling it, and examples are very self-explanatory. I think I'm gonna find some spare time next week to integrate this to Parca. I'll let you know how it goes.
I have just added a comment about tests in BPF program.
#include <bpf/bpf_helpers.h> | ||
|
||
SEC("kprobe/sys_execve") | ||
int kprobe__sys_execve(struct pt_regs *ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// CreateMap creates a BPF map from userspace. This can be used for populating
// BPF array of maps or hash of maps. However, this function uses a low-level
// libbpf API; maps created in this way do not conform to libbpf map formats,
// and therefore do not have access to libbpf high level bpf_map__* APIS
// which causes different behavior from maps created in the kernel side code
Considering the comment above, is it possible to add links or example APIs here to demonstrate accessing the maps that are created in the user space?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just pushed a comment into the selftest with further explanation. If you want to try using this in Parca and are up for working on it together please tag me in the issue/PR and i'd gladly help figure it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯 Thanks!
Signed-off-by: grantseltzer <[email protected]>
@yanivagman can you take a look at this when you have spare time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition @grantseltzer
LGTM, just a small comment/question
@@ -341,6 +470,30 @@ func (m *Module) GetMap(mapName string) (*BPFMap, error) { | |||
}, nil | |||
} | |||
|
|||
func (b *BPFMap) Name() string { | |||
cs := C.bpf_map__name(b.bpfMap) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quoting from CreateMap above:
// and therefore do not have access to libbpf high level bpf_map__* APIS
So, this won't work for maps created with CreateMap, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct, the struct bpf_map
is NULL which bpf_map__name()
uses as it's parameter. CreateMap
only provides a file descriptor for the map.
// with a file descriptor already. If the map is already associated | ||
// with a file descriptor the libbpf API will return error code EBUSY | ||
func (b *BPFMap) SetType(mapType MapType) error { | ||
errC := C.bpf_map__set_type(b.bpfMap, C.enum_bpf_map_type(int(mapType))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
No description provided.