Lock PCI allocated file

When running on heavy a load, sriov-cni might be invoked multiple times
by the kubelet or the container runtime. In these situations, it might happen
that a `cmdAdd` or a `cmdDel` run on the same device that is still handled by
a `cmdDel` instance (e.g. when the IPAM plugin takes a long time to finish).

This commit adds a Lock mechanism around the `/var/lib/cni/sriov/<PCI>.lock` file,
to avoid two or more instances handling the same the device.
The lock is based on `flock` [1] using the `O_CLOEXEC` flag, which  garantees that the lock
is released when the process finishes.

[1] https://linux.die.net/man/2/flock

Signed-off-by: Andrea Panattoni <[email protected]>

Author: zeeke

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)
@@ -317,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)
 	}

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

pkg/config/config_test.go

@@ -13,6 +13,11 @@ import (
 )
 
 var _ = Describe("Config", func() {
+	BeforeEach(func() {
+		DeferCleanup(func(x string) { DefaultCNIDir = x }, DefaultCNIDir)
+		DefaultCNIDir = GinkgoT().TempDir()
+	})
+
 	Context("Checking LoadConf function", func() {
 		It("Assuming correct config file - existing DeviceID", func() {
 			conf := []byte(`{
@@ -80,6 +85,7 @@ var _ = Describe("Config", func() {
 		invalidProto := "802"
 		DescribeTable("Vlan ID, QoS and Proto",
 			func(vlanID *int, vlanQoS *int, vlanProto *string, failure bool) {
+
 				s := `{
         "name": "mynet",
         "type": "sriov",

pkg/utils/pci_allocator.go

@@ -3,12 +3,18 @@ package utils
 import (
 	"fmt"
 	"os"
+	"path"
 	"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 +31,41 @@ 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 {
+	lockDir := path.Join(p.dataDir, "vf_lock")
+	if err := os.MkdirAll(lockDir, 0600); err != nil {
+		return fmt.Errorf("failed to create the sriov lock directory(%q): %v", lockDir, err)
+	}
+
+	path := filepath.Join(lockDir, fmt.Sprintf("%s.lock", pciAddress))
+
+	// unix.O_CREAT - Create the file if it doesn't exist
+	// unix.O_RDONLY - Open the file for read
+	// 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_RDONLY|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 {