diff --git a/cmd/sriov/main.go b/cmd/sriov/main.go index 1281fde3..06893168 100644 --- a/cmd/sriov/main.go +++ b/cmd/sriov/main.go @@ -245,6 +245,17 @@ func cmdDel(args *skel.CmdArgs) error { return nil } + allocator := utils.NewPCIAllocator(config.DefaultCNIDir) + + err = allocator.Lock(netConf.DeviceID) + if err != nil { + return fmt.Errorf("cmdDel() error obtaining lock for device [%s]: %w", netConf.DeviceID, err) + } + + logging.Debug("Acquired device lock", + "func", "cmdDel", + "DeviceID", netConf.DeviceID) + defer func() { if err == nil && cRefPath != "" { _ = utils.CleanCachedNetConf(cRefPath) @@ -270,6 +281,9 @@ func cmdDel(args *skel.CmdArgs) error { sm := sriov.NewSriovManager() + logging.Debug("Reset VF configuration", + "func", "cmdDel", + "netConf.DeviceID", netConf.DeviceID) /* ResetVFConfig resets a VF administratively. We must run ResetVFConfig before ReleaseVF because some drivers will error out if we try to reset netdev VF with trust off. So, reset VF MAC address via PF first. @@ -288,6 +302,10 @@ func cmdDel(args *skel.CmdArgs) error { // IPAM resources _, ok := err.(ns.NSPathNotExistErr) if ok { + logging.Debug("Exiting as the network namespace does not exists anymore", + "func", "cmdDel", + "netConf.DeviceID", netConf.DeviceID, + "args.Netns", args.Netns) return nil } @@ -295,6 +313,11 @@ func cmdDel(args *skel.CmdArgs) error { } defer netns.Close() + logging.Debug("Release the VF", + "func", "cmdDel", + "netConf.DeviceID", netConf.DeviceID, + "args.Netns", args.Netns, + "args.IfName", args.IfName) if err = sm.ReleaseVF(netConf, args.IfName, netns); err != nil { return err } @@ -305,7 +328,6 @@ func cmdDel(args *skel.CmdArgs) error { "func", "cmdDel", "config.DefaultCNIDir", config.DefaultCNIDir, "netConf.DeviceID", netConf.DeviceID) - allocator := utils.NewPCIAllocator(config.DefaultCNIDir) if err = allocator.DeleteAllocatedPCI(netConf.DeviceID); err != nil { return fmt.Errorf("error cleaning the pci allocation for vf pci address %s: %v", netConf.DeviceID, err) } diff --git a/pkg/config/config.go b/pkg/config/config.go index e15fa382..1981788f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -48,6 +48,15 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) { return nil, fmt.Errorf("LoadConf(): VF pci addr is required") } + allocator := utils.NewPCIAllocator(DefaultCNIDir) + err := allocator.Lock(n.DeviceID) + if err != nil { + return nil, err + } + logging.Debug("Acquired device lock", + "func", "LoadConf", + "DeviceID", n.DeviceID) + // Check if the device is already allocated. // This is to prevent issues where kubelet request to delete a pod and in the same time a new pod using the same // vf is started. we can have an issue where the cmdDel of the old pod is called AFTER the cmdAdd of the new one @@ -56,7 +65,6 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) { "func", "LoadConf", "DefaultCNIDir", DefaultCNIDir, "n.DeviceID", n.DeviceID) - allocator := utils.NewPCIAllocator(DefaultCNIDir) isAllocated, err := allocator.IsAllocated(n.DeviceID) if err != nil { return n, err diff --git a/pkg/utils/pci_allocator.go b/pkg/utils/pci_allocator.go index 3bf6ff05..a8dd9907 100644 --- a/pkg/utils/pci_allocator.go +++ b/pkg/utils/pci_allocator.go @@ -4,11 +4,15 @@ import ( "fmt" "os" "path/filepath" + "time" "github.com/containernetworking/plugins/pkg/ns" "github.com/k8snetworkplumbingwg/sriov-cni/pkg/logging" + "golang.org/x/sys/unix" ) +const pciLockAcquireTimeout = 60 * time.Second + type PCIAllocation interface { SaveAllocatedPCI(string, string) error DeleteAllocatedPCI(string) error @@ -25,6 +29,40 @@ func NewPCIAllocator(dataDir string) *PCIAllocator { return &PCIAllocator{dataDir: filepath.Join(dataDir, "pci")} } +// Lock gets an exclusive lock on the given PCI address, ensuring there is no other process configuring / or de-configuring the same device. +func (p *PCIAllocator) Lock(pciAddress string) error { + if err := os.MkdirAll(p.dataDir, 0600); err != nil { + return fmt.Errorf("failed to create the sriov data directory(%q): %v", p.dataDir, err) + } + + path := filepath.Join(p.dataDir, pciAddress) + + // unix.O_CREAT - Create the file if it doesn't exist + // unix.O_RDWR - Open the file for read/write + // unix.O_CLOEXEC - Automatically close the file on exit. This is useful to keep the flock until the process exits + fd, err := unix.Open(path, unix.O_CREAT|unix.O_RDWR|unix.O_CLOEXEC, 0600) + if err != nil { + return fmt.Errorf("failed to open PCI file [%s] for locking: %w", path, err) + } + + errCh := make(chan error) + go func() { + // unix.LOCK_EX - Exclusive lock + errCh <- unix.Flock(fd, unix.LOCK_EX) + }() + + select { + case err = <-errCh: + if err != nil { + return fmt.Errorf("failed to flock PCI file [%s]: %w", path, err) + } + return nil + + case <-time.After(pciLockAcquireTimeout): + return fmt.Errorf("time out while waiting to acquire exclusive lock on [%s]", path) + } +} + // SaveAllocatedPCI creates a file with the pci address as a name and the network namespace as the content // return error if the file was not created func (p *PCIAllocator) SaveAllocatedPCI(pciAddress, ns string) error {