Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions plugins/meta/vrf/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,9 @@ func cmdDel(args *skel.CmdArgs) error {
}
err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
vrf, err := findVRF(conf.VRFName)
if _, ok := err.(netlink.LinkNotFoundError); ok {
if linkNotFound(err) {
return nil
}

if err != nil {
return err
}
Expand All @@ -119,8 +118,11 @@ func cmdDel(args *skel.CmdArgs) error {

// Meaning, we are deleting the last interface assigned to the VRF
if len(interfaces) == 0 {
err = netlink.LinkDel(vrf)
if err != nil {
// CNI DEL must be idempotent: a concurrent netns teardown can remove
// the VRF between findVRF and here, so LinkDel returns ENODEV. Treat
// "already gone" as success (LinkDel returns a bare errno, not the
// typed netlink.LinkNotFoundError).
if err = netlink.LinkDel(vrf); err != nil && !linkNotFound(err) {
return err
}
}
Expand Down
26 changes: 22 additions & 4 deletions plugins/meta/vrf/vrf.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
package main

import (
"errors"
"fmt"
"math"
"net"
"syscall"
"time"

"github.com/vishvananda/netlink"
Expand Down Expand Up @@ -214,16 +216,32 @@ func findFreeRoutingTableID(links []netlink.Link) (uint32, error) {

func resetMaster(interfaceName string) error {
intf, err := netlinksafe.LinkByName(interfaceName)
if err != nil {
return fmt.Errorf("resetMaster: could not get link by name %s", interfaceName)
if linkNotFound(err) {
// Interface already gone (e.g. concurrent netns teardown). DEL is
// best-effort, so there is nothing left to reset.
return nil
}
err = netlink.LinkSetNoMaster(intf)
if err != nil {
return fmt.Errorf("resetMaster: could reset master to %s", interfaceName)
return fmt.Errorf("resetMaster: could not get link by name %s: %w", interfaceName, err)
}
if err := netlink.LinkSetNoMaster(intf); err != nil {
return fmt.Errorf("resetMaster: could not reset master of %s: %w", interfaceName, err)
}
return nil
}

// linkNotFound reports whether err indicates the link is already gone. netlink
// returns a typed LinkNotFoundError from LinkByName, but a bare errno (ENODEV)
// from LinkDel, so both must be matched. Per the CNI spec, DEL is best-effort
// and must not fail when a resource has already been removed.
func linkNotFound(err error) bool {
if err == nil {
return false
}
var lnf netlink.LinkNotFoundError
return errors.As(err, &lnf) || errors.Is(err, syscall.ENODEV) || errors.Is(err, syscall.ENOENT)
}

// getGlobalAddresses returns the global addresses of the given interface
func getGlobalAddresses(link netlink.Link, family int) ([]netlink.Addr, error) {
addresses, err := netlinksafe.AddrList(link, family)
Expand Down
48 changes: 48 additions & 0 deletions plugins/meta/vrf/vrf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,54 @@ var _ = Describe("vrf plugin", func() {
})
})

It("returns success on DEL when the enslaved interface is already gone (idempotent)", func() {
conf0 := configFor("test", IF0Name, VRF0Name, "10.0.0.2/24")

By("Adding the interface to the VRF", func() {
err := originalNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
args := &skel.CmdArgs{
ContainerID: "dummy",
Netns: targetNS.Path(),
IfName: IF0Name,
StdinData: conf0,
}
_, _, err := testutils.CmdAddWithArgs(args, func() error {
return cmdAdd(args)
})
Expect(err).NotTo(HaveOccurred())
return nil
})
Expect(err).NotTo(HaveOccurred())
})

By("Removing the enslaved interface out-of-band (simulating a teardown race)", func() {
err := targetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
link, err := netlinksafe.LinkByName(IF0Name)
Expect(err).NotTo(HaveOccurred())
return netlink.LinkDel(link)
})
Expect(err).NotTo(HaveOccurred())
})

By("DEL succeeding even though the interface is gone", func() {
err := originalNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
args := &skel.CmdArgs{
ContainerID: "dummy",
Netns: targetNS.Path(),
IfName: IF0Name,
StdinData: conf0,
}
return testutils.CmdDelWithArgs(args, func() error {
return cmdDel(args)
})
})
Expect(err).NotTo(HaveOccurred())
})
})

It("configures and deconfigures VRF with CNI 0.4.0 ADD/DEL", func() {
conf := []byte(fmt.Sprintf(`{
"name": "test",
Expand Down
Loading