diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 727ffe642..cefba03a3 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -100,6 +100,8 @@ type Daemon struct { workqueue workqueue.RateLimitingInterface mcpName string + + storeManager utils.StoreManagerInterface } const ( @@ -218,6 +220,12 @@ func (dn *Daemon) Run(stopCh <-chan struct{}, exitCh <-chan error) error { tryEnableTun() tryEnableVhostNet() + storeManager, err := utils.NewStoreManager() + if err != nil { + return err + } + dn.storeManager = storeManager + if err := dn.tryCreateUdevRuleWrapper(); err != nil { return err } @@ -436,6 +444,12 @@ func (dn *Daemon) nodeStateSyncHandler() error { } if latestState.GetGeneration() == 1 && len(latestState.Spec.Interfaces) == 0 { + err = dn.storeManager.ClearPCIAddressFolder() + if err != nil { + glog.Errorf("failed to clear the PCI address configuration: %v", err) + return err + } + glog.V(0).Infof("nodeStateSyncHandler(): Name: %s, Interface policy spec not yet set by controller", latestState.Name) if latestState.Status.SyncStatus != "Succeeded" { dn.refreshCh <- Message{ @@ -452,6 +466,15 @@ func (dn *Daemon) nodeStateSyncHandler() error { syncStatus: syncStatusInProgress, lastSyncError: "", } + // wait for writer to refresh status then pull again the latest node state + <-dn.syncCh + updatedState, err := dn.client.SriovnetworkV1().SriovNetworkNodeStates(namespace).Get(context.Background(), dn.name, metav1.GetOptions{}) + if err != nil { + glog.Warningf("nodeStateSyncHandler(): Failed to fetch node state %s: %v", dn.name, err) + return err + } + // we only update the status to avoid any race conditions where a new policy can be applied + latestState.Status = updatedState.Status // load plugins if has not loaded if len(dn.enabledPlugins) == 0 { diff --git a/pkg/daemon/writer.go b/pkg/daemon/writer.go index d70ea8843..e2abffe8c 100644 --- a/pkg/daemon/writer.go +++ b/pkg/daemon/writer.go @@ -100,10 +100,7 @@ func (w *NodeStateStatusWriter) Run(stop <-chan struct{}, refresh <-chan Message if err != nil { glog.Errorf("Run() refresh: writing to node status failed: %v", err) } - - if msg.syncStatus == syncStatusSucceeded || msg.syncStatus == syncStatusFailed { - syncCh <- struct{}{} - } + syncCh <- struct{}{} case <-time.After(30 * time.Second): glog.V(2).Info("Run(): period refresh") if err := w.pollNicStatus(platformType); err != nil { diff --git a/pkg/plugins/generic/generic_plugin.go b/pkg/plugins/generic/generic_plugin.go index a2a7e1c00..cc5fda633 100644 --- a/pkg/plugins/generic/generic_plugin.go +++ b/pkg/plugins/generic/generic_plugin.go @@ -2,6 +2,7 @@ package generic import ( "bytes" + "fmt" "os/exec" "reflect" "strconv" @@ -25,6 +26,7 @@ type GenericPlugin struct { LastState *sriovnetworkv1.SriovNetworkNodeState LoadVfioDriver uint LoadVirtioVdpaDriver uint + StoreManager utils.StoreManagerInterface } const scriptsPath = "bindata/scripts/enable-kargs.sh" @@ -37,11 +39,17 @@ const ( // Initialize our plugin and set up initial values func NewGenericPlugin() (plugin.VendorPlugin, error) { + storeManager, err := utils.NewStoreManager() + if err != nil { + return nil, fmt.Errorf("NewGenericPlugin(): error initializing storeManager: %v", err) + } + return &GenericPlugin{ PluginName: PluginName, SpecVersion: "1.0", LoadVfioDriver: unloaded, LoadVirtioVdpaDriver: unloaded, + StoreManager: storeManager, }, nil } @@ -63,8 +71,8 @@ func (p *GenericPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeSt err = nil p.DesireState = new - needDrain = needDrainNode(new.Spec.Interfaces, new.Status.Interfaces) - needReboot = needRebootNode(new, &p.LoadVfioDriver, &p.LoadVirtioVdpaDriver) + needDrain = p.needDrainNode(new.Spec.Interfaces, new.Status.Interfaces) + needReboot = p.needRebootNode(new, &p.LoadVfioDriver, &p.LoadVirtioVdpaDriver) if needReboot { needDrain = true @@ -179,8 +187,9 @@ func isCommandNotFound(err error) bool { return false } -func needDrainNode(desired sriovnetworkv1.Interfaces, current sriovnetworkv1.InterfaceExts) (needDrain bool) { +func (p *GenericPlugin) needDrainNode(desired sriovnetworkv1.Interfaces, current sriovnetworkv1.InterfaceExts) (needDrain bool) { glog.V(2).Infof("generic-plugin needDrainNode(): current state '%+v', desired state '%+v'", current, desired) + needDrain = false for _, ifaceStatus := range current { configured := false @@ -198,6 +207,27 @@ func needDrainNode(desired sriovnetworkv1.Interfaces, current sriovnetworkv1.Int } } if !configured && ifaceStatus.NumVfs > 0 { + // load the PF info + pfStatus, exist, err := p.StoreManager.LoadPfsStatus(ifaceStatus.PciAddress, true) + if err != nil { + glog.Errorf("generic-plugin needDrainNode(): failed to load info about PF status for pci address %s: %v", ifaceStatus.PciAddress, err) + continue + } + + if !exist { + glog.Infof("generic-plugin needDrainNode(): PF name %s with pci address %s has VFs configured but they weren't created by the sriov operator. Skipping drain", + ifaceStatus.Name, + ifaceStatus.PciAddress) + continue + } + + if pfStatus.ExternallyCreated { + glog.Infof("generic-plugin needDrainNode()(): PF name %s with pci address %s was externally created. Skipping drain", + ifaceStatus.Name, + ifaceStatus.PciAddress) + continue + } + glog.V(2).Infof("generic-plugin needDrainNode(): need drain, %v needs to be reset", ifaceStatus) needDrain = true return @@ -206,7 +236,7 @@ func needDrainNode(desired sriovnetworkv1.Interfaces, current sriovnetworkv1.Int return } -func needRebootNode(state *sriovnetworkv1.SriovNetworkNodeState, loadVfioDriver *uint, loadVirtioVdpaDriver *uint) (needReboot bool) { +func (p *GenericPlugin) needRebootNode(state *sriovnetworkv1.SriovNetworkNodeState, loadVfioDriver *uint, loadVirtioVdpaDriver *uint) (needReboot bool) { needReboot = false if *loadVfioDriver != loaded { if needVfioDriver(state) { diff --git a/pkg/utils/host.go b/pkg/utils/host.go new file mode 100644 index 000000000..fbc19d8f3 --- /dev/null +++ b/pkg/utils/host.go @@ -0,0 +1,134 @@ +package utils + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + + "github.com/golang/glog" + + sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" +) + +const ( + SriovConfBasePath = "/etc/sriov-operator" + PfAppliedConfig = SriovConfBasePath + "/pci" + HostSriovConfBasePath = "/host" + SriovConfBasePath + HostPfAppliedConfig = HostSriovConfBasePath + "/pci" +) + +type StoreManagerInterface interface { + ClearPCIAddressFolder() error + SaveLastPfAppliedStatus(pciAddress string, PfInfo *sriovnetworkv1.Interface) error + LoadPfsStatus(pciAddress string, chroot bool) (*sriovnetworkv1.Interface, bool, error) +} + +type StoreManager struct { +} + +// NewStoreManager: create the initial folders needed to store the info about the PF +// and return a storeManager struct that implements the StoreManagerInterface interface +func NewStoreManager() (StoreManagerInterface, error) { + if err := CreateOperatorConfigFolderIfNeeded(); err != nil { + return nil, err + } + + return &StoreManager{}, nil +} + +// CreateOperatorConfigFolderIfNeeded: create the operator base folder on the host +// together with the pci folder to save the PF status objects as json files +func CreateOperatorConfigFolderIfNeeded() error { + _, err := os.Stat(SriovConfBasePath) + if err != nil { + if os.IsNotExist(err) { + err = os.MkdirAll(SriovConfBasePath, os.ModeDir) + if err != nil { + return fmt.Errorf("failed to create the sriov config folder on host in path %s: %v", SriovConfBasePath, err) + } + } else { + return fmt.Errorf("failed to check if the sriov config folder on host in path %s exist: %v", SriovConfBasePath, err) + } + } + + _, err = os.Stat(PfAppliedConfig) + if err != nil { + if os.IsNotExist(err) { + err = os.MkdirAll(PfAppliedConfig, os.ModeDir) + if err != nil { + return fmt.Errorf("failed to create the pci folder on host in path %s: %v", PfAppliedConfig, err) + } + } else { + return fmt.Errorf("failed to check if the pci folder on host in path %s exist: %v", PfAppliedConfig, err) + } + } + + return nil +} + +// ClearPCIAddressFolder: removes all the PFs storage information +func (s *StoreManager) ClearPCIAddressFolder() error { + _, err := os.Stat(HostPfAppliedConfig) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("failed to check the pci address folder path %s", HostPfAppliedConfig) + } + + err = os.RemoveAll(HostPfAppliedConfig) + if err != nil { + return fmt.Errorf("failed to remove the PCI address folder on path %s: %v", HostPfAppliedConfig, err) + } + + err = os.Mkdir(HostPfAppliedConfig, os.ModeDir) + if err != nil { + return fmt.Errorf("failed to create the pci folder on host in path %s: %v", HostPfAppliedConfig, err) + } + + return nil +} + +// SaveLastPfAppliedStatus will save the PF object as a json into the /etc/sriov-operator/pci/ +// this function must be called after running the chroot function +func (s *StoreManager) SaveLastPfAppliedStatus(pciAddress string, PfInfo *sriovnetworkv1.Interface) error { + data, err := json.Marshal(PfInfo) + if err != nil { + glog.Errorf("failed to marshal PF status %+v: %v", *PfInfo, err) + return err + } + + path := filepath.Join(PfAppliedConfig, pciAddress) + err = os.WriteFile(path, data, 0644) + return err +} + +// LoadPfsStatus convert the /etc/sriov-operator/pci/ json to pfstatus +// returns false if the file doesn't exist. +func (s *StoreManager) LoadPfsStatus(pciAddress string, chroot bool) (*sriovnetworkv1.Interface, bool, error) { + if chroot { + exit, err := Chroot("/host") + if err != nil { + return nil, false, err + } + defer exit() + } + + pfStatus := &sriovnetworkv1.Interface{} + path := filepath.Join(PfAppliedConfig, pciAddress) + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil, false, nil + } + glog.Errorf("failed to read PF status from path %s: %v", path, err) + } + + err = json.Unmarshal(data, pfStatus) + if err != nil { + glog.Errorf("failed to unmarshal PF status %s: %v", data, err) + } + + return pfStatus, true, nil +} diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 37888e4b1..b862c6b2f 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -65,6 +65,11 @@ func DiscoverSriovDevices(withUnsupported bool) ([]sriovnetworkv1.InterfaceExt, return nil, fmt.Errorf("DiscoverSriovDevices(): could not retrieve PCI devices") } + storeManager, err := NewStoreManager() + if err != nil { + return nil, fmt.Errorf("DiscoverSriovDevices(): error initializing storeManager: %v", err) + } + for _, device := range devices { devClass, err := strconv.ParseInt(device.Class.ID, 16, 64) if err != nil { @@ -122,6 +127,15 @@ func DiscoverSriovDevices(withUnsupported bool) ([]sriovnetworkv1.InterfaceExt, } iface.LinkType = getLinkType(iface) + pfStatus, exist, err := storeManager.LoadPfsStatus(iface.PciAddress, true) + if err != nil { + glog.Warningf("DiscoverSriovDevices(): failed to load PF status from disk: %v", err) + } + + if exist { + iface.ExternallyCreated = pfStatus.ExternallyCreated + } + if dputils.IsSriovPF(device.Address) { iface.TotalVfs = dputils.GetSriovVFcapacity(device.Address) iface.NumVfs = dputils.GetVFconfigured(device.Address) @@ -152,7 +166,12 @@ func SyncNodeState(newState *sriovnetworkv1.SriovNetworkNodeState, pfsToConfig m glog.Warningf("cannot use mellanox devices when in kernel lockdown mode") return fmt.Errorf("cannot use mellanox devices when in kernel lockdown mode") } - var err error + + storeManager, err := NewStoreManager() + if err != nil { + return fmt.Errorf("SyncNodeState(): error initializing storeManager: %v", err) + } + for _, ifaceStatus := range newState.Status.Interfaces { configured := false for _, iface := range newState.Spec.Interfaces { @@ -165,15 +184,34 @@ func SyncNodeState(newState *sriovnetworkv1.SriovNetworkNodeState, pfsToConfig m if !NeedUpdate(&iface, &ifaceStatus) { glog.V(2).Infof("syncNodeState(): no need update interface %s", iface.PciAddress) + + // Save the PF status to the host + err = storeManager.SaveLastPfAppliedStatus(iface.PciAddress, &iface) + if err != nil { + glog.Errorf("SyncNodeState(): failed to save PF applied config to host: %v", err) + return err + } + break } if err = configSriovDevice(&iface, &ifaceStatus); err != nil { glog.Errorf("SyncNodeState(): fail to configure sriov interface %s: %v. resetting interface.", iface.PciAddress, err) - if resetErr := resetSriovDevice(ifaceStatus); resetErr != nil { - glog.Errorf("SyncNodeState(): fail to reset on error SR-IOV interface: %s", resetErr) + if iface.ExternallyCreated { + glog.Infof("SyncNodeState(): skipping device reset as the nic is marked as externally created") + } else { + if resetErr := resetSriovDevice(ifaceStatus); resetErr != nil { + glog.Errorf("SyncNodeState(): failed to reset on error SR-IOV interface: %s", resetErr) + } } return err } + + // Save the PF status to the host + err = storeManager.SaveLastPfAppliedStatus(iface.PciAddress, &iface) + if err != nil { + glog.Errorf("SyncNodeState(): failed to save PF applied config to host: %v", err) + return err + } break } } @@ -182,6 +220,27 @@ func SyncNodeState(newState *sriovnetworkv1.SriovNetworkNodeState, pfsToConfig m continue } + // load the PF info + pfStatus, exist, err := storeManager.LoadPfsStatus(ifaceStatus.PciAddress, false) + if err != nil { + glog.Errorf("SyncNodeState(): failed to load info about PF status for pci address %s: %v", ifaceStatus.PciAddress, err) + return err + } + + if !exist { + glog.Infof("SyncNodeState(): PF name %s with pci address %s has VFs configured but they weren't created by the sriov operator. Skipping the device reset", + ifaceStatus.Name, + ifaceStatus.PciAddress) + continue + } + + if pfStatus.ExternallyCreated { + glog.Infof("SyncNodeState(): PF name %s with pci address %s was externally created skipping the device reset", + ifaceStatus.Name, + ifaceStatus.PciAddress) + continue + } + if err = resetSriovDevice(ifaceStatus); err != nil { return err } @@ -270,6 +329,12 @@ func NeedUpdate(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetworkv1.Int glog.V(2).Infof("NeedUpdate(): VF %d MTU needs update, desired=%d, current=%d", vf.VfID, group.Mtu, vf.Mtu) return true } + + // this is needed to be sure the admin mac address is configured as expected + if iface.ExternallyCreated { + glog.V(2).Infof("NeedUpdate(): need to update the device as it's externally manage for pci address %s", ifaceStatus.PciAddress) + return true + } } break } @@ -293,6 +358,12 @@ func configSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetwor } // set numVFs if iface.NumVfs != ifaceStatus.NumVfs { + if iface.ExternallyCreated { + errMsg := fmt.Sprintf("configSriovDevice(): number of request virtual functions %d is not equal to configured virtual functions %d but the policy is configured as ExternallyCreated for device %s", iface.NumVfs, ifaceStatus.NumVfs, iface.PciAddress) + glog.Error(errMsg) + return fmt.Errorf(errMsg) + } + err = setSriovNumVfs(iface.PciAddress, iface.NumVfs) if err != nil { glog.Errorf("configSriovDevice(): fail to set NumVfs for device %s", iface.PciAddress)