ebpf
ebpf copied to clipboard
ErrMapIncompatible should contain additional information
In https://github.com/cilium/cilium/pull/22693, Cilium will start relying exclusively on the library to tell it when a map is incompatible with an existing pinned map, but right now ErrMapIncompatible is a little vague. Callers need to open the old map explicitly, inspect its info, and work out the differences with the MapSpec.
Updated to echo https://github.com/cilium/ebpf/pull/1093#issuecomment-1640078398.
I think there are 2 issues with the existing code:
- No clear communication (towards the user) of what 'expected' and 'got' means in (
expected type %v, got %v). We should settle on language that clarifies which side is the existing Map and which is the incoming Spec. - Only one error is surfaced at a time, being the first-found one.
As a bonus, it would be nice if this would return a new ebpf.IncompatibleMapError{OldType, NewType ebpf.MapType, ...} that .Unwrap()s to ErrMapIncompatible so it can be used like:
var merr *ebpf.IncompatibleMapError
if errors.As(err, &merr) {
if merr.OldType == ebpf.ArrayOfMaps && merr.NewType == ebpf.HashOfMaps {
// Convert the old map from ArrayOfMaps to HashOfMaps here..
}
}
This would be really useful to trigger up/downgrade logic.
Also, .Error() should only return a diff of the map's properties, not a dump of all old/new properties. That's just something I hacked up in Cilium to move ahead with the changes there.
@ti-mo hi I am new to this org and want to contribute to this issue can you please guide me through it
@swastik959 There's a short document at https://github.com/cilium/ebpf/blob/master/CONTRIBUTING.md. Do you have any specific questions?
Hello @ti-mo, I would like to work on this if no one is actively working on this.
@sharathsivakumar Sure thing! Assigned the issue to you.
Thanks!
Closing this for now, since it's actually quite tricky to implement. This should happen if / when we use the info from the cilium side.