From cb4ca0ccfb682ffb1a469ada01237a2f81316540 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 3 Jul 2023 15:10:04 +0300 Subject: [PATCH] Create the needed functions to save info on the host this commit adds the functionality needed to save the PF information into the host. we need this in case the user removes a policy that was externallyCreated. we must NOT delete reset the VFs in that case and the only way to know that the device was externallyCreated after the user removes the policy is to check this file on the host. Signed-off-by: Sebastian Sch --- pkg/daemon/daemon.go | 23 +++++ pkg/daemon/writer.go | 5 +- pkg/plugins/generic/generic_plugin.go | 38 +++++++- pkg/utils/host.go | 134 ++++++++++++++++++++++++++ pkg/utils/utils.go | 77 ++++++++++++++- 5 files changed, 266 insertions(+), 11 deletions(-) create mode 100644 pkg/utils/host.go 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)