Parallel NICs configuration

k8snetworkplumbingwg · Feb 6, 2024 · f6bfd53 · f6bfd53
1 parent d162021
commit f6bfd53
Show file tree

Hide file tree

Showing 6 changed files with 223 additions and 59 deletions.
diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go
diff --git a/bindata/manifests/daemon/daemonset.yaml b/bindata/manifests/daemon/daemonset.yaml
@@ -102,6 +102,9 @@ spec:
         {{- with index . "DisablePlugins" }}
           - --disable-plugins={{.}}
         {{- end }}
+        {{- if .ParallelNicConfig }}
+        - --parallel-nic-config
+        {{- end }}
         env:
           - name: NODE_NAME
             valueFrom:

diff --git a/cmd/sriov-network-config-daemon/start.go b/cmd/sriov-network-config-daemon/start.go
@@ -78,10 +78,11 @@ var (
 	}
 
 	startOpts struct {
-		kubeconfig      string
-		nodeName        string
-		systemd         bool
-		disabledPlugins stringList
+		kubeconfig        string
+		nodeName          string
+		systemd           bool
+		disabledPlugins   stringList
+		parallelNicConfig bool
 	}
 )
 
@@ -91,6 +92,7 @@ func init() {
 	startCmd.PersistentFlags().StringVar(&startOpts.nodeName, "node-name", "", "kubernetes node name daemon is managing")
 	startCmd.PersistentFlags().BoolVar(&startOpts.systemd, "use-systemd-service", false, "use config daemon in systemd mode")
 	startCmd.PersistentFlags().VarP(&startOpts.disabledPlugins, "disable-plugins", "", "comma-separated list of plugins to disable")
+	startCmd.PersistentFlags().BoolVar(&startOpts.parallelNicConfig, "parallel-nic-config", false, "perform NIC configuration in parallel")
 }
 
 func runStartCmd(cmd *cobra.Command, args []string) error {
@@ -104,6 +106,8 @@ func runStartCmd(cmd *cobra.Command, args []string) error {
 		vars.UsingSystemdMode = true
 	}
 
+	vars.ParallelNicConfig = startOpts.parallelNicConfig
+
 	if startOpts.nodeName == "" {
 		name, ok := os.LookupEnv("NODE_NAME")
 		if !ok || name == "" {

diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go
@@ -187,6 +187,10 @@ func (r *SriovOperatorConfigReconciler) syncConfigDaemonSet(ctx context.Context,
 	} else {
 		data.Data["UsedSystemdMode"] = false
 	}
+	data.Data["ParallelNicConfig"] = false
+	if parallelConfig, ok := dc.Spec.FeatureGates["parallelNicConfig"]; ok {
+		data.Data["ParallelNicConfig"] = parallelConfig
+	}
 
 	envCniBinPath := os.Getenv("SRIOV_CNI_BIN_PATH")
 	if envCniBinPath == "" {

diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go
@@ -26,6 +26,11 @@ import (
 	mlx "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vendors/mellanox"
 )
 
+type interfaceToConfigure struct {
+	iface       sriovnetworkv1.Interface
+	ifaceStatus sriovnetworkv1.InterfaceExt
+}
+
 type sriov struct {
 	utilsHelper   utils.CmdInterface
 	kernelHelper  types.KernelInterface
@@ -450,13 +455,43 @@ func (s *sriov) ConfigSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *
 	return nil
 }
 
-func (s *sriov) ConfigSriovInterfaces(storeManager store.ManagerInterface,
-	interfaces []sriovnetworkv1.Interface, ifaceStatuses []sriovnetworkv1.InterfaceExt, pfsToConfig map[string]bool) error {
+func (s *sriov) ConfigSriovInterfaces(storeManager store.ManagerInterface, interfaces []sriovnetworkv1.Interface, ifaceStatuses []sriovnetworkv1.InterfaceExt, pfsToConfig map[string]bool) error {
 	if s.kernelHelper.IsKernelLockdownMode() && mlx.HasMellanoxInterfacesInSpec(ifaceStatuses, interfaces) {
 		log.Log.Error(nil, "cannot use mellanox devices when in kernel lockdown mode")
 		return fmt.Errorf("cannot use mellanox devices when in kernel lockdown mode")
 	}
 
+	toBeConfigured, toBeResetted, err := s.getConfigureAndReset(storeManager, interfaces, ifaceStatuses, pfsToConfig)
+	if err != nil {
+		log.Log.Error(err, "cannot get a list of interfaces to configure")
+		return fmt.Errorf("cannot get a list of interfaces to configure")
+	}
+
+	if vars.ParallelNicConfig {
+		err = s.configSriovInterfacesInParallel(storeManager, toBeConfigured)
+	} else {
+		err = s.configSriovInterfaces(storeManager, toBeConfigured)
+	}
+	if err != nil {
+		log.Log.Error(err, "cannot configure sriov interfaces")
+		return fmt.Errorf("cannot configure sriov interfaces")
+	}
+
+	if vars.ParallelNicConfig {
+		err = s.resetSriovInterfacesInParallel(storeManager, toBeResetted)
+	} else {
+		err = s.resetSriovInterfaces(storeManager, toBeResetted)
+	}
+	if err != nil {
+		log.Log.Error(err, "cannot reset reset interfaces")
+		return fmt.Errorf("cannot reset sriov interfaces")
+	}
+	return nil
+}
+
+func (s *sriov) getConfigureAndReset(storeManager store.ManagerInterface, interfaces []sriovnetworkv1.Interface, ifaceStatuses []sriovnetworkv1.InterfaceExt, pfsToConfig map[string]bool) ([]interfaceToConfigure, []sriovnetworkv1.InterfaceExt, error) {
+	toBeConfigured := []interfaceToConfigure{}
+	toBeResetted := []sriovnetworkv1.InterfaceExt{}
 	for _, ifaceStatus := range ifaceStatuses {
 		configured := false
 		for _, iface := range interfaces {
@@ -467,76 +502,184 @@ func (s *sriov) ConfigSriovInterfaces(storeManager store.ManagerInterface,
 					break
 				}
 
-				if !sriovnetworkv1.NeedToUpdateSriov(&iface, &ifaceStatus) {
-					log.Log.V(2).Info("syncNodeState(): no need update interface", "address", iface.PciAddress)
-
-					// Save the PF status to the host
-					err := storeManager.SaveLastPfAppliedStatus(&iface)
-					if err != nil {
-						log.Log.Error(err, "SyncNodeState(): failed to save PF applied config to host")
-						return err
-					}
-
-					break
-				}
-				if err := s.ConfigSriovDevice(&iface, &ifaceStatus); err != nil {
-					log.Log.Error(err, "SyncNodeState(): fail to configure sriov interface. resetting interface.", "address", iface.PciAddress)
-					if iface.ExternallyManaged {
-						log.Log.Info("SyncNodeState(): skipping device reset as the nic is marked as externally created")
-					} else {
-						if resetErr := s.ResetSriovDevice(ifaceStatus); resetErr != nil {
-							log.Log.Error(resetErr, "SyncNodeState(): failed to reset on error SR-IOV interface")
-						}
-					}
-					return err
-				}
-
-				// Save the PF status to the host
-				err := storeManager.SaveLastPfAppliedStatus(&iface)
+				skip, err := checkNeedUpdate(&iface, &ifaceStatus, storeManager)
 				if err != nil {
-					log.Log.Error(err, "SyncNodeState(): failed to save PF applied config to host")
-					return err
+					log.Log.Error(err, "getConfigureAndReset(): failed to check interface")
+					return nil, nil, err
 				}
-				break
+				if skip {
+					break
+				}
+				toBeConfigured = append(toBeConfigured, interfaceToConfigure{iface: iface, ifaceStatus: ifaceStatus})
 			}
 		}
+
 		if !configured && ifaceStatus.NumVfs > 0 {
-			if skip := pfsToConfig[ifaceStatus.PciAddress]; skip {
-				continue
+			if skip := pfsToConfig[ifaceStatus.PciAddress]; !skip {
+				toBeResetted = append(toBeResetted, ifaceStatus)
 			}
+		}
+	}
+	return toBeConfigured, toBeResetted, nil
+}
 
-			// load the PF info
-			pfStatus, exist, err := storeManager.LoadPfsStatus(ifaceStatus.PciAddress)
-			if err != nil {
-				log.Log.Error(err, "SyncNodeState(): failed to load info about PF status for device",
-					"address", ifaceStatus.PciAddress)
-				return err
+func (s *sriov) configSriovInterfacesInParallel(storeManager store.ManagerInterface, interfaces []interfaceToConfigure) error {
+	log.Log.V(2).Info("ConfigSriovInterfacesInParallel(): start sriov configuration")
+
+	// TODO(e0ne): store all errors in SriovNetworkNodeState
+	var result error
+	errChannel := make(chan error)
+	interfacesToConfigure := 0
+	for ifaceIndex, iface := range interfaces {
+		interfacesToConfigure += 1
+		go func(iface *interfaceToConfigure) {
+			if err := s.ConfigSriovDevice(&iface.iface, &iface.ifaceStatus); err != nil {
+				log.Log.Error(err, "ConfigSriovInterfacesInParallel(): fail to configure sriov interface. resetting interface.", "address", iface.iface.PciAddress)
+				if iface.iface.ExternallyManaged {
+					log.Log.Info("ConfigSriovInterfacesInParallel(): skipping device reset as the nic is marked as externally created")
+				} else {
+					if resetErr := s.ResetSriovDevice(iface.ifaceStatus); resetErr != nil {
+						log.Log.Error(resetErr, "ConfigSriovInterfacesInParallel(): failed to reset on error SR-IOV interface")
+					}
+				}
+				errChannel <- err
+			} else {
+				errChannel <- nil
 			}
+		}(&interfaces[ifaceIndex])
+		// Save the PF status to the host
+		err := storeManager.SaveLastPfAppliedStatus(&iface.iface)
+		if err != nil {
+			log.Log.Error(err, "ConfigSriovInterfacesInParallel(): failed to save PF applied config to host")
+			return err
+		}
+		break
+	}
 
-			if !exist {
-				log.Log.Info("SyncNodeState(): PF name with pci address has VFs configured but they weren't created by the sriov operator. Skipping the device reset",
-					"pf-name", ifaceStatus.Name,
-					"address", ifaceStatus.PciAddress)
-				continue
+	for i := 0; i < interfacesToConfigure; i++ {
+		errMsg := <-errChannel
+		result = errors.Join(result, errMsg)
+	}
+	if result != nil {
+		log.Log.Error(result, "ConfigSriovInterfacesInParallel(): fail to configure sriov interface")
+		return result
+	}
+	log.Log.V(2).Info("ConfigSriovInterfacesInParallel(): sriov configuration finished")
+	return nil
+}
+
+func (s *sriov) resetSriovInterfacesInParallel(storeManager store.ManagerInterface, interfaces []sriovnetworkv1.InterfaceExt) error {
+	var result error
+	errChannel := make(chan error)
+	interfacesToReset := 0
+	for ifaceIndex := range interfaces {
+		interfacesToReset += 1
+		go func(iface *sriovnetworkv1.InterfaceExt) {
+			if err := s.checkForConfigAndReset(*iface, storeManager); err != nil {
+				log.Log.Error(err, "resetSriovInterfacesInParallel(): fail to configure sriov interface. resetting interface.", "address", iface.PciAddress)
+				errChannel <- err
+			} else {
+				errChannel <- nil
 			}
+		}(&interfaces[ifaceIndex])
+	}
 
-			if pfStatus.ExternallyManaged {
-				log.Log.Info("SyncNodeState(): PF name with pci address was externally created skipping the device reset",
-					"pf-name", ifaceStatus.Name,
-					"address", ifaceStatus.PciAddress)
-				continue
+	for i := 0; i < interfacesToReset; i++ {
+		errMsg := <-errChannel
+		result = errors.Join(result, errMsg)
+	}
+	if result != nil {
+		log.Log.Error(result, "resetSriovInterfacesInParallel(): fail to reset sriov interface")
+		return result
+	}
+	log.Log.V(2).Info("resetSriovInterfacesInParallel(): sriov reset finished")
+
+	return nil
+}
+
+func (s *sriov) configSriovInterfaces(storeManager store.ManagerInterface, interfaces []interfaceToConfigure) error {
+	for _, iface := range interfaces {
+		if err := s.ConfigSriovDevice(&iface.iface, &iface.ifaceStatus); err != nil {
+			log.Log.Error(err, "ConfigSriovInterfaces(): fail to configure sriov interface. resetting interface.", "address", iface.iface.PciAddress)
+			if iface.iface.ExternallyManaged {
+				log.Log.Info("ConfigSriovInterfaces(): skipping device reset as the nic is marked as externally created")
 			} else {
-				err = s.udevHelper.RemoveUdevRule(ifaceStatus.PciAddress)
-				if err != nil {
-					return err
+				if resetErr := s.ResetSriovDevice(iface.ifaceStatus); resetErr != nil {
+					log.Log.Error(resetErr, "ConfigSriovInterfaces(): failed to reset on error SR-IOV interface")
 				}
 			}
+			return err
+		}
 
-			if err = s.ResetSriovDevice(ifaceStatus); err != nil {
-				return err
-			}
+		// Save the PF status to the host
+		err := storeManager.SaveLastPfAppliedStatus(&iface.iface)
+		if err != nil {
+			log.Log.Error(err, "ConfigSriovInterfaces(): failed to save PF applied config to host")
+			return err
+		}
+	}
+	return nil
+}
+
+func (s *sriov) resetSriovInterfaces(storeManager store.ManagerInterface, interfaces []sriovnetworkv1.InterfaceExt) error {
+	for _, iface := range interfaces {
+		if err := s.checkForConfigAndReset(iface, storeManager); err != nil {
+			log.Log.Error(err, "resetSriovInterfaces(): fail to configure sriov interface. resetting interface.", "address", iface.PciAddress)
+			return err
+		}
+	}
+	log.Log.V(2).Info("resetSriovInterfaces(): sriov reset finished")
+	return nil
+}
+
+func checkNeedUpdate(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetworkv1.InterfaceExt, storeManager store.ManagerInterface) (bool, error) {
+	if !sriovnetworkv1.NeedToUpdateSriov(iface, ifaceStatus) {
+		log.Log.V(2).Info("ConfigSriovInterfaces(): no need update interface", "address", iface.PciAddress)
+
+		// Save the PF status to the host
+		err := storeManager.SaveLastPfAppliedStatus(iface)
+		if err != nil {
+			log.Log.Error(err, "ConfigSriovInterfaces(): failed to save PF applied config to host")
+			return false, err
 		}
+
+		return true, nil
 	}
+	return false, nil
+}
+
+func (s *sriov) checkForConfigAndReset(ifaceStatus sriovnetworkv1.InterfaceExt, storeManager store.ManagerInterface) error {
+	// load the PF info
+	pfStatus, exist, err := storeManager.LoadPfsStatus(ifaceStatus.PciAddress)
+	if err != nil {
+		log.Log.Error(err, "checkForConfigAndReset(): failed to load info about PF status for device",
+			"address", ifaceStatus.PciAddress)
+		return err
+	}
+
+	if !exist {
+		log.Log.Info("checkForConfigAndReset(): PF name with pci address has VFs configured but they weren't created by the sriov operator. Skipping the device reset",
+			"pf-name", ifaceStatus.Name,
+			"address", ifaceStatus.PciAddress)
+		return nil
+	}
+
+	if pfStatus.ExternallyManaged {
+		log.Log.Info("checkForConfigAndReset(): PF name with pci address was externally created skipping the device reset",
+			"pf-name", ifaceStatus.Name,
+			"address", ifaceStatus.PciAddress)
+		return nil
+	} else {
+		err = s.udevHelper.RemoveUdevRule(ifaceStatus.PciAddress)
+		if err != nil {
+			return err
+		}
+	}
+
+	if err = s.ResetSriovDevice(ifaceStatus); err != nil {
+		return err
+	}
+
 	return nil
 }
 

diff --git a/pkg/vars/vars.go b/pkg/vars/vars.go
@@ -48,6 +48,9 @@ var (
 	// UsingSystemdMode global variable to mark the config-daemon is running on systemd mode
 	UsingSystemdMode = false
 
+	// ParallelNicConfig global variable to perform NIC configuration in parallel
+	ParallelNicConfig = false
+
 	// FilesystemRoot used by test to mock interactions with filesystem
 	FilesystemRoot = ""