Skip to content
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

Delete and recreate pinned maps in case of mismatch with the map's spec #289

Closed
jibi opened this issue Apr 20, 2021 · 1 comment · Fixed by #291
Closed

Delete and recreate pinned maps in case of mismatch with the map's spec #289

jibi opened this issue Apr 20, 2021 · 1 comment · Fixed by #291
Assignees

Comments

@jibi
Copy link
Member

jibi commented Apr 20, 2021

To address cilium/cilium#15629 I need to delete and recreate a pinned map in case there's a mismatch with the map's spec and the actual pinned map (type, k/v size, flags etc).

I see 2 approaches :

  • have this logic in cilium/ebpf by introducing a new option in MapOptions.LoadPinOptions. If there's a mismatch and the option is set, NewMapWithOptions will delete the existing map and try to recreate it.
    Code changes are small but I'm not sure if this logic would be too specialized/high level for the package.
  • have this logic in cilium/cilium. The caller of ebpf.NewMapWithOptions would be responsible for detecting the mismatch, deleting the map and recreating it.
    My concern here would be on detecting the mismatch, as the best I could come up with is:
newMap, err := ebpf.NewMapWithOptions(spec, opts)
if err != nil {
    if strings.HasPrefix(err.Error(), "use pinned map ") {
        // delete the map and call again NewMapWithOptions

which to me looks fragile 😕

@jibi
Copy link
Member Author

jibi commented Apr 20, 2021

We agreed on Slack to introduce for now a new ErrMapIncompatible error and return that from New{Map,Collection} in order to enable the caller to properly detect when there's a mismatch

jibi added a commit to jibi/ebpf that referenced this issue Apr 21, 2021
This error will be returned by NewMapWithOptions whether the spec for a
pinned map is incompatible with the actual instance of the map, in order
to allow the caller to properly detect this mismatch.

Fixes cilium#289

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
jibi added a commit to jibi/ebpf that referenced this issue Apr 21, 2021
This error will be returned by NewMapWithOptions whether the spec for a
pinned map is incompatible with the actual instance of the map, in order
to allow the caller to properly detect this mismatch.

Fixes cilium#289

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
jibi added a commit to jibi/ebpf that referenced this issue Apr 21, 2021
This error will be returned by NewMapWithOptions whether the spec for a
pinned map is incompatible with the actual instance of the map, in order
to allow the caller to properly detect this mismatch.

Fixes cilium#289

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
jibi added a commit to jibi/ebpf that referenced this issue Apr 21, 2021
This error will be returned by NewMapWithOptions whether the spec for a
pinned map is incompatible with the actual instance of the map, in order
to allow the caller to properly detect this mismatch.

Fixes cilium#289

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
jibi added a commit to jibi/ebpf that referenced this issue Apr 21, 2021
This error will be returned by NewMapWithOptions whether the spec for a
pinned map is incompatible with the actual instance of the map, in order
to allow the caller to properly detect this mismatch.

Fixes cilium#289

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
@lmb lmb closed this as completed in #291 Apr 21, 2021
lmb pushed a commit that referenced this issue Apr 21, 2021
This error will be returned by NewMapWithOptions whether the spec for a
pinned map is incompatible with the actual instance of the map, in order
to allow the caller to properly detect this mismatch.

Fixes #289

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant