Skip to content

plugins/vrf: make DEL idempotent when the device is already gone - #1282

Open
bgly wants to merge 1 commit into
containernetworking:mainfrom
bgly:fix/vrf-del-idempotent
Open

plugins/vrf: make DEL idempotent when the device is already gone#1282
bgly wants to merge 1 commit into
containernetworking:mainfrom
bgly:fix/vrf-del-idempotent

Conversation

@bgly

@bgly bgly commented Jul 21, 2026

Copy link
Copy Markdown

Summary

Makes the vrf meta plugin's cmdDel idempotent, as the CNI SPEC requires DEL
to be best-effort ("plugins should generally complete a DEL action without error
even if some resources are missing").

Problem

During concurrent netns teardown the VRF can be removed between findVRF and
LinkDel, so netlink.LinkDel(vrf) returns a bare ENODEV and cmdDel
propagated it. resetMaster had the same problem when the enslaved interface
was already gone (and dropped the underlying error by not wrapping it).

When vrf is used in a conflist (e.g. macvlan + vrf, as Multus generates),
a plugin error aborts the remaining reverse-order DELs in the runtime (libcni
DelNetworkList), so the chained macvlan DEL never runs. The orphaned
interface pins its netns via its dev_net reference and the namespace leaks; at
scale this exhausts the kernel namespace limit and new pods fail with
failed to create network namespace: resource temporarily unavailable.

cmdDel already handles the findVRF-not-found case and NSPathNotExistErr,
but not LinkDel/resetMaster. Note LinkDel returns a bare errno
(syscall.ENODEV), not netlink.LinkNotFoundError, so the type-assertion
guard used for findVRF does not catch it.

Changes

  • cmdDel: treat LinkDel(vrf) not-found as success; route the existing
    findVRF guard through a shared helper.
  • resetMaster: treat LinkByName not-found as success; wrap real errors with
    %w.
  • New linkNotFound() helper matching netlink.LinkNotFoundError and
    errors.Is(ENODEV/ENOENT).
  • Regression test: DEL succeeds when the enslaved interface is removed
    out-of-band.

Behaviour change

Only on the DEL error path: an already-removed VRF/interface now yields success
instead of an error. The happy path is unchanged. This mirrors the existing
NSPathNotExistErr handling.

Testing

go build / go vet / test-binary compile clean; added a unit test covering
resetMaster/findVRF idempotency. The LinkDel-ENODEV path is inherently a
race (VRF removed mid-cmdDel); it was validated out-of-tree by driving a
macvlan + vrf conflist through libcni with the race window forced
deterministically: 8/8 leaked without this change, 0/8 with it.

During concurrent pod-netns teardown the VRF can be removed between
findVRF and LinkDel, so LinkDel(vrf) returns ENODEV. cmdDel propagated
that error and (under Multus/libcni) aborted the conflist DEL before the
chained macvlan DEL ran, orphaning the interface and leaking its netns.
resetMaster had the same problem when the enslaved interface was already
gone, and additionally masked the underlying error by not wrapping it.

Per the CNI SPEC, DEL is best-effort and must not fail when a resource
has already been removed. Treat "link not found" as success on both
LinkDel and resetMaster, matching by errno (ENODEV/ENOENT) as well as
the typed LinkNotFoundError -- LinkDel returns a bare errno, so a
type-only check misses it. Also switch the existing findVRF guard to the
shared helper and wrap resetMaster errors with %w.

Signed-off-by: Bryant Ly <bly@coreweave.com>
@bgly

bgly commented Jul 23, 2026

Copy link
Copy Markdown
Author

@squeed Can you possibly review or someone? This is live in production that we reproduced / fixing with a shim.

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 this pull request may close these issues.

1 participant