Change BPFMap method Update to unsafe.Pointer
#32
Conversation
This makes a breaking change to the libbpfgo API in which
the BPFMap.Update() method takes two unsafe.Pointer instead
of two interface{}. This is so that libbpfgo does not have
to know or care about the type of the passed value. If bpf
and libbpf can support the datatype, so should libbpfgo.
It's important to note that it would then be on the user to
correctly pass arrays/slices (reference must be passed to the
first element, not the array/slice itself).
Signed-off-by: grantseltzer <[email protected]>
| // valuePtr := unsafe.Pointer(&value[0]) | ||
| // bpfmap.Update(keyPtr, valuePtr) | ||
| // | ||
| func (b *BPFMap) Update(key, value unsafe.Pointer) error { |
There was a problem hiding this comment.
Should the rest of the BPFMap also be updated to reflect this change? E.g. GetValue and DeleteKey? That way you could delete the entire GetUnsafePointer function (note this will also be a breaking API change as that method is current exported).
I assume there aren't any bad implications from a consumer trying to use a pointer to a struct or an array/slice as a key, correct (or anything that specifically wasn't allowed before)?
There was a problem hiding this comment.
Should the rest of the BPFMap also be updated to reflect this change? E.g. GetValue and DeleteKey? That way you could delete the entire GetUnsafePointer function
+1
I assume there aren't any bad implications from a consumer trying to use a pointer to a struct or an array/slice as a key, correct (or anything that specifically wasn't allowed before)?
With this change, the same semantics as of libbpf are applied. This means that the kernel will try to read from the given pointers (key and value) the same type that was defined for the map. It is now the responsibility of the consumer to make sure that the pointer given to this function is valid and points to the correct data type, or else, bad things might happen.
There was a problem hiding this comment.
Updated GetValue and DeleteKey accordingly.
|
Well, technically this lib is still pre 1.0 so breaking changes can happen! Alternative could be a new method such as |
Signed-off-by: grantseltzer <[email protected]>
|
I think since it's pre-1.0 (for both libbpfgo and libbpf) we should just break the API, but we should make an announcement, cut another release, and reach out to people we know are using libbpfgo as best as we can to make sure it's well known. |
|
I've tested this change locally with some work I'm doing with Delve and this change now allows me to pass a struct between the userspace program and the eBPF program using an eBPF map and everything seems to work great. |
This makes a breaking change to the libbpfgo API in which
the BPFMap.Update() method takes two unsafe.Pointer instead
of two interface{}. This is so that libbpfgo does not have
to know or care about the type of the passed value. If bpf
and libbpf can support the datatype, so should libbpfgo.
It's important to note that it would then be on the user to
correctly pass arrays/slices (reference must be passed to the
first element, not the array/slice itself).
This also adds a selftest but I haven't added it to any build
script/Makefile, as I'm waiting for #28 to merge, then I will
rebase and update accordingly.
We should heavily consider the implications of this breaking
API change.
cc @derekparker
Signed-off-by: grantseltzer [email protected]