diff --git a/cmd/sriov-network-config-daemon/service.go b/cmd/sriov-network-config-daemon/service.go index afa5a5582..382ad976b 100644 --- a/cmd/sriov-network-config-daemon/service.go +++ b/cmd/sriov-network-config-daemon/service.go @@ -93,6 +93,7 @@ func runServiceCmd(cmd *cobra.Command, args []string) error { setupLog.V(2).Info("sriov-config-service", "config", sriovConf) vars.DevMode = sriovConf.UnsupportedNics vars.ManageSoftwareBridges = sriovConf.ManageSoftwareBridges + vars.OVSDBSocketPath = sriovConf.OVSDBSocketPath if err := initSupportedNics(); err != nil { return updateSriovResultErr(setupLog, phaseArg, fmt.Errorf("failed to initialize list of supported NIC ids: %v", err)) @@ -181,7 +182,7 @@ func callPlugin(setupLog logr.Logger, phase string, conf *systemd.SriovConfig, h return nil } - nodeState, err := getNetworkNodeState(setupLog, conf, hostHelpers) + nodeState, err := getNetworkNodeState(setupLog, conf, phase, hostHelpers) if err != nil { return err } @@ -207,7 +208,9 @@ func getPlugin(setupLog logr.Logger, phase string, case consts.Baremetal: switch phase { case PhasePre: - configPlugin, err = newGenericPluginFunc(hostHelpers, generic.WithSkipVFConfiguration()) + configPlugin, err = newGenericPluginFunc(hostHelpers, + generic.WithSkipVFConfiguration(), + generic.WithSkipBridgeConfiguration()) case PhasePost: configPlugin, err = newGenericPluginFunc(hostHelpers) } @@ -229,10 +232,11 @@ func getPlugin(setupLog logr.Logger, phase string, return configPlugin, nil } -func getNetworkNodeState(setupLog logr.Logger, conf *systemd.SriovConfig, +func getNetworkNodeState(setupLog logr.Logger, conf *systemd.SriovConfig, phase string, hostHelpers helper.HostHelpersInterface) (*sriovv1.SriovNetworkNodeState, error) { var ( ifaceStatuses []sriovv1.InterfaceExt + bridges sriovv1.Bridges err error ) switch conf.PlatformType { @@ -241,6 +245,13 @@ func getNetworkNodeState(setupLog logr.Logger, conf *systemd.SriovConfig, if err != nil { return nil, fmt.Errorf("failed to discover sriov devices on the host: %v", err) } + if phase != PhasePre && vars.ManageSoftwareBridges { + // openvswitch is not available during the pre phase + bridges, err = hostHelpers.DiscoverBridges() + if err != nil { + return nil, fmt.Errorf("failed to discover managed bridges on the host: %v", err) + } + } case consts.VirtualOpenStack: platformHelper, err := newPlatformHelperFunc() if err != nil { @@ -257,7 +268,7 @@ func getNetworkNodeState(setupLog logr.Logger, conf *systemd.SriovConfig, } return &sriovv1.SriovNetworkNodeState{ Spec: conf.Spec, - Status: sriovv1.SriovNetworkNodeStateStatus{Interfaces: ifaceStatuses}, + Status: sriovv1.SriovNetworkNodeStateStatus{Interfaces: ifaceStatuses, Bridges: bridges}, }, nil } diff --git a/cmd/sriov-network-config-daemon/service_test.go b/cmd/sriov-network-config-daemon/service_test.go index d7b9134f8..771cc3b1c 100644 --- a/cmd/sriov-network-config-daemon/service_test.go +++ b/cmd/sriov-network-config-daemon/service_test.go @@ -38,7 +38,6 @@ func restoreOrigFuncs() { func getTestSriovInterfaceConfig(platform int) []byte { return []byte(fmt.Sprintf(`spec: - dpconfigversion: "" interfaces: - pciaddress: 0000:d8:00.0 numvfs: 4 @@ -57,6 +56,7 @@ func getTestSriovInterfaceConfig(platform int) []byte { externallymanaged: false unsupportedNics: false platformType: %d +manageSoftwareBridges: true `, platform)) } @@ -239,6 +239,7 @@ var _ = Describe("Service", func() { hostHelpers.EXPECT().DiscoverSriovDevices(hostHelpers).Return([]sriovnetworkv1.InterfaceExt{{ Name: "enp216s0f0np0", }}, nil) + hostHelpers.EXPECT().DiscoverBridges().Return(sriovnetworkv1.Bridges{}, nil) genericPlugin.EXPECT().OnNodeStateChange(newNodeStateContainsDeviceMatcher("enp216s0f0np0")).Return(true, false, nil) genericPlugin.EXPECT().Apply().Return(nil) Expect(runServiceCmd(&cobra.Command{}, []string{})).NotTo(HaveOccurred()) diff --git a/cmd/sriov-network-config-daemon/start.go b/cmd/sriov-network-config-daemon/start.go index f003e5435..b1ad0f667 100644 --- a/cmd/sriov-network-config-daemon/start.go +++ b/cmd/sriov-network-config-daemon/start.go @@ -87,6 +87,7 @@ var ( disabledPlugins stringList parallelNicConfig bool manageSoftwareBridges bool + ovsSocketPath string } ) @@ -98,6 +99,7 @@ func init() { 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") startCmd.PersistentFlags().BoolVar(&startOpts.manageSoftwareBridges, "manage-software-bridges", false, "enable management of software bridges") + startCmd.PersistentFlags().StringVar(&startOpts.ovsSocketPath, "ovs-socket-path", vars.OVSDBSocketPath, "path for OVSDB socket") } func runStartCmd(cmd *cobra.Command, args []string) error { @@ -113,6 +115,7 @@ func runStartCmd(cmd *cobra.Command, args []string) error { vars.ParallelNicConfig = startOpts.parallelNicConfig vars.ManageSoftwareBridges = startOpts.manageSoftwareBridges + vars.OVSDBSocketPath = startOpts.ovsSocketPath if startOpts.nodeName == "" { name, ok := os.LookupEnv("NODE_NAME") diff --git a/pkg/daemon/writer.go b/pkg/daemon/writer.go index c796546ef..09d06d8f9 100644 --- a/pkg/daemon/writer.go +++ b/pkg/daemon/writer.go @@ -117,17 +117,29 @@ func (w *NodeStateStatusWriter) Run(stop <-chan struct{}, refresh <-chan Message func (w *NodeStateStatusWriter) pollNicStatus() error { log.Log.V(2).Info("pollNicStatus()") var iface []sriovnetworkv1.InterfaceExt + var bridges sriovnetworkv1.Bridges var err error if vars.PlatformType == consts.VirtualOpenStack { iface, err = w.platformHelper.DiscoverSriovDevicesVirtual() + if err != nil { + return err + } } else { iface, err = w.hostHelper.DiscoverSriovDevices(w.hostHelper) + if err != nil { + return err + } + if vars.ManageSoftwareBridges { + bridges, err = w.hostHelper.DiscoverBridges() + if err != nil { + return err + } + } } - if err != nil { - return err - } + w.status.Interfaces = iface + w.status.Bridges = bridges return nil } @@ -169,6 +181,7 @@ func (w *NodeStateStatusWriter) updateNodeStateStatusRetry(f func(*sriovnetworkv func (w *NodeStateStatusWriter) setNodeStateStatus(msg Message) (*sriovnetworkv1.SriovNetworkNodeState, error) { nodeState, err := w.updateNodeStateStatusRetry(func(nodeState *sriovnetworkv1.SriovNetworkNodeState) { nodeState.Status.Interfaces = w.status.Interfaces + nodeState.Status.Bridges = w.status.Bridges if msg.lastSyncError != "" || msg.syncStatus == consts.SyncStatusSucceeded { // clear lastSyncError when sync Succeeded nodeState.Status.LastSyncError = msg.lastSyncError diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index 2a1e5c2b6..cfca2a768 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -209,6 +209,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) ConfigSriovInterfaces(storeManag return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigSriovInterfaces", reflect.TypeOf((*MockHostHelpersInterface)(nil).ConfigSriovInterfaces), storeManager, interfaces, ifaceStatuses, skipVFConfiguration) } +// ConfigureBridges mocks base method. +func (m *MockHostHelpersInterface) ConfigureBridges(bridgesSpec, bridgesStatus v1.Bridges) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConfigureBridges", bridgesSpec, bridgesStatus) + ret0, _ := ret[0].(error) + return ret0 +} + +// ConfigureBridges indicates an expected call of ConfigureBridges. +func (mr *MockHostHelpersInterfaceMockRecorder) ConfigureBridges(bridgesSpec, bridgesStatus interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigureBridges", reflect.TypeOf((*MockHostHelpersInterface)(nil).ConfigureBridges), bridgesSpec, bridgesStatus) +} + // ConfigureVfGUID mocks base method. func (m *MockHostHelpersInterface) ConfigureVfGUID(vfAddr, pfAddr string, vfID int, pfLink netlink.Link) error { m.ctrl.T.Helper() @@ -251,6 +265,35 @@ func (mr *MockHostHelpersInterfaceMockRecorder) DeleteVDPADevice(pciAddr interfa return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVDPADevice", reflect.TypeOf((*MockHostHelpersInterface)(nil).DeleteVDPADevice), pciAddr) } +// DetachInterfaceFromManagedBridge mocks base method. +func (m *MockHostHelpersInterface) DetachInterfaceFromManagedBridge(pciAddr string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DetachInterfaceFromManagedBridge", pciAddr) + ret0, _ := ret[0].(error) + return ret0 +} + +// DetachInterfaceFromManagedBridge indicates an expected call of DetachInterfaceFromManagedBridge. +func (mr *MockHostHelpersInterfaceMockRecorder) DetachInterfaceFromManagedBridge(pciAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DetachInterfaceFromManagedBridge", reflect.TypeOf((*MockHostHelpersInterface)(nil).DetachInterfaceFromManagedBridge), pciAddr) +} + +// DiscoverBridges mocks base method. +func (m *MockHostHelpersInterface) DiscoverBridges() (v1.Bridges, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DiscoverBridges") + ret0, _ := ret[0].(v1.Bridges) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DiscoverBridges indicates an expected call of DiscoverBridges. +func (mr *MockHostHelpersInterfaceMockRecorder) DiscoverBridges() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DiscoverBridges", reflect.TypeOf((*MockHostHelpersInterface)(nil).DiscoverBridges)) +} + // DiscoverSriovDevices mocks base method. func (m *MockHostHelpersInterface) DiscoverSriovDevices(storeManager store.ManagerInterface) ([]v1.InterfaceExt, error) { m.ctrl.T.Helper() diff --git a/pkg/host/internal/bridge/ovs/ovs.go b/pkg/host/internal/bridge/ovs/ovs.go index 6d232b0fb..7ad8a3e8c 100644 --- a/pkg/host/internal/bridge/ovs/ovs.go +++ b/pkg/host/internal/bridge/ovs/ovs.go @@ -5,8 +5,10 @@ import ( "encoding/json" "errors" "fmt" + "os" "reflect" "sort" + "strings" "time" "github.com/google/uuid" @@ -17,6 +19,7 @@ import ( sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" ovsStorePkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/bridge/ovs/store" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" ) @@ -627,6 +630,35 @@ func updateMap(old, new map[string]string) map[string]string { return result } +// returns path for the OVDSB socket +// for unix sockets it is taking into account current FS root and possible symlinks +func getDBSocketPath() (string, error) { + if !strings.HasPrefix(vars.OVSDBSocketPath, "unix://") { + // no need to apply modifications to tcp sockets + return vars.OVSDBSocketPath, nil + } + origPathNoPrefix, _ := strings.CutPrefix(vars.OVSDBSocketPath, "unix://") + resolvedPath := utils.GetHostExtensionPath(origPathNoPrefix) + + // in some OSes /var/run is an absolute symlink to /run, + // this can be a problem when we are trying to access OVSDB socket from the daemon POD. + // first we a trying to use original path, if we can't find the OVSDB socket, + // we try to replace /var/run with /run in the socket path + var err error + _, err = os.Stat(resolvedPath) + if err != nil && !os.IsNotExist(err) { + return "", err + } + if os.IsNotExist(err) { + resolvedPath = strings.Replace(resolvedPath, "/var/run/", "/run/", 1) + _, err = os.Stat(resolvedPath) + if err != nil { + return "", err + } + } + return "unix://" + resolvedPath, nil +} + // initialize and return OVSDB client func getClient(ctx context.Context) (client.Client, error) { openvSwitchEntry := &OpenvSwitchEntry{} @@ -638,8 +670,13 @@ func getClient(ctx context.Context) (client.Client, error) { return nil, fmt.Errorf("can't create client DB model: %v", err) } + socketPath, err := getDBSocketPath() + if err != nil { + return nil, fmt.Errorf("can't find OVSDB socket %s: %v", vars.OVSDBSocketPath, err) + } + dbClient, err := client.NewOVSDBClient(clientDBModel, - client.WithEndpoint(vars.OVSDBSocketPath), + client.WithEndpoint(socketPath), client.WithLogger(&log.Log)) if err != nil { return nil, fmt.Errorf("can't create DB client: %v", err) diff --git a/pkg/host/internal/bridge/ovs/ovs_test.go b/pkg/host/internal/bridge/ovs/ovs_test.go index 039696672..666fe9218 100644 --- a/pkg/host/internal/bridge/ovs/ovs_test.go +++ b/pkg/host/internal/bridge/ovs/ovs_test.go @@ -22,6 +22,8 @@ import ( sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" ovsStoreMockPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/bridge/ovs/store/mock" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" + "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/fakefilesystem" + "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/helpers" ) func getManagedBridges() map[string]*sriovnetworkv1.OVSConfigExt { @@ -162,6 +164,14 @@ var _ = Describe("OVS", func() { ) BeforeEach(func() { ctx = context.Background() + origInChroot := vars.InChroot + origSocketValue := vars.OVSDBSocketPath + origFSRoot := vars.FilesystemRoot + DeferCleanup(func() { + vars.InChroot = origInChroot + vars.OVSDBSocketPath = origSocketValue + vars.FilesystemRoot = origFSRoot + }) }) Context("setDefaultTimeout", func() { It("use default", func() { @@ -179,7 +189,7 @@ var _ = Describe("OVS", func() { timeoutCtx, timeoutFunc := context.WithTimeout(ctx, time.Millisecond*100) defer timeoutFunc() newCtx, _ := setDefaultTimeout(timeoutCtx) - time.Sleep(time.Millisecond * 200) + time.Sleep(time.Second) Expect(newCtx.Err()).To(MatchError(context.DeadlineExceeded)) }) It("use explicit timeout - should return noop cancel function", func() { @@ -212,6 +222,56 @@ var _ = Describe("OVS", func() { }) }) + Context("client", func() { + It("can't find socket", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{}) + c, err := getClient(ctx) + Expect(c).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("can't find OVSDB socket"))) + }) + Context("getDBSocketPath()", func() { + It("tcp socket", func() { + vars.OVSDBSocketPath = "tcp://127.0.0.1:4444" + sock, err := getDBSocketPath() + Expect(err).NotTo(HaveOccurred()) + Expect(sock).To(Equal("tcp://127.0.0.1:4444")) + }) + It("unix socket - in container", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/host/ovs"}, + Files: map[string][]byte{"/host/ovs/ovsdb.sock": {}}, + }) + vars.InChroot = false + vars.OVSDBSocketPath = "unix:///ovs/ovsdb.sock" + sock, err := getDBSocketPath() + Expect(err).NotTo(HaveOccurred()) + Expect(sock).To(Equal("unix://" + vars.FilesystemRoot + "/host/ovs/ovsdb.sock")) + }) + It("unix socket - host", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/ovs"}, + Files: map[string][]byte{"/ovs/ovsdb.sock": {}}, + }) + vars.InChroot = true + vars.OVSDBSocketPath = "unix:///ovs/ovsdb.sock" + sock, err := getDBSocketPath() + Expect(err).NotTo(HaveOccurred()) + Expect(sock).To(Equal("unix://" + vars.FilesystemRoot + "/ovs/ovsdb.sock")) + }) + It("unix socket - alternative /var/run path", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/host/run/ovs"}, + Files: map[string][]byte{"/host/run/ovs/ovsdb.sock": {}}, + }) + vars.InChroot = false + vars.OVSDBSocketPath = "unix:///var/run/ovs/ovsdb.sock" + sock, err := getDBSocketPath() + Expect(err).NotTo(HaveOccurred()) + Expect(sock).To(Equal("unix://" + vars.FilesystemRoot + "/host/run/ovs/ovsdb.sock")) + }) + }) + }) + Context("manage bridges", func() { var ( store *ovsStoreMockPkg.MockStore @@ -224,6 +284,7 @@ var _ = Describe("OVS", func() { ovs Interface ) BeforeEach(func() { + vars.InChroot = true tempDir, err = os.MkdirTemp("", "sriov-operator-ovs-test-dir*") testServerSocket = filepath.Join(tempDir, "ovsdb.sock") Expect(err).NotTo(HaveOccurred()) @@ -231,13 +292,7 @@ var _ = Describe("OVS", func() { store = ovsStoreMockPkg.NewMockStore(testCtrl) _ = store stopServerFunc = startServer("unix", testServerSocket) - - origSocketValue := vars.OVSDBSocketPath vars.OVSDBSocketPath = "unix://" + testServerSocket - DeferCleanup(func() { - vars.OVSDBSocketPath = origSocketValue - }) - ovsClient, err = getClient(ctx) Expect(err).NotTo(HaveOccurred()) ovs = New(store) diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go index 64f1fa537..586b4438f 100644 --- a/pkg/host/internal/sriov/sriov.go +++ b/pkg/host/internal/sriov/sriov.go @@ -43,6 +43,7 @@ type sriov struct { dputilsLib dputilsPkg.DPUtilsLib sriovnetLib sriovnetPkg.SriovnetLib ghwLib ghwPkg.GHWLib + bridgeHelper types.BridgeInterface } func New(utilsHelper utils.CmdInterface, @@ -54,7 +55,8 @@ func New(utilsHelper utils.CmdInterface, netlinkLib netlinkPkg.NetlinkLib, dputilsLib dputilsPkg.DPUtilsLib, sriovnetLib sriovnetPkg.SriovnetLib, - ghwLib ghwPkg.GHWLib) types.SriovInterface { + ghwLib ghwPkg.GHWLib, + bridgeHelper types.BridgeInterface) types.SriovInterface { return &sriov{utilsHelper: utilsHelper, kernelHelper: kernelHelper, networkHelper: networkHelper, @@ -65,6 +67,7 @@ func New(utilsHelper utils.CmdInterface, dputilsLib: dputilsLib, sriovnetLib: sriovnetLib, ghwLib: ghwLib, + bridgeHelper: bridgeHelper, } } @@ -1007,6 +1010,11 @@ func (s *sriov) setEswitchModeAndNumVFs(pciAddr string, desiredEswitchMode strin // always switch NIC to the legacy mode before creating VFs. This is required because some drivers // may not support VF creation in the switchdev mode if s.GetNicSriovMode(pciAddr) != sriovnetworkv1.ESwithModeLegacy { + // detach PF from the managed bridge before switching the mode, + // changing of eSwitch mode may fail if NIC is part of the bridge (has offloaded TC rules) + if err := s.detachPFFromBridge(pciAddr); err != nil { + return err + } if err := s.setEswitchMode(pciAddr, sriovnetworkv1.ESwithModeLegacy); err != nil { return err } @@ -1021,6 +1029,19 @@ func (s *sriov) setEswitchModeAndNumVFs(pciAddr string, desiredEswitchMode strin return nil } +// detach PF from the managed bridge +func (s *sriov) detachPFFromBridge(pciAddr string) error { + log.Log.V(2).Info("detachPFFromBridge(): detach PF", "device", pciAddr) + if !vars.ManageSoftwareBridges { + return nil + } + if err := s.bridgeHelper.DetachInterfaceFromManagedBridge(pciAddr); err != nil { + log.Log.Error(err, "detachPFFromBridge(): failed to detach interface from the managed bridge", "device", pciAddr) + return err + } + return nil +} + // retrieve all VFs for the PF and unbind them from a driver func (s *sriov) unbindAllVFsOnPF(addr string) error { log.Log.V(2).Info("unbindAllVFsOnPF(): unbind all VFs on PF", "device", addr) diff --git a/pkg/host/internal/sriov/sriov_test.go b/pkg/host/internal/sriov/sriov_test.go index f89ecd3c5..6db7d9617 100644 --- a/pkg/host/internal/sriov/sriov_test.go +++ b/pkg/host/internal/sriov/sriov_test.go @@ -49,7 +49,7 @@ var _ = Describe("SRIOV", func() { hostMock = hostMockPkg.NewMockHostManagerInterface(testCtrl) storeManagerMode = hostStoreMockPkg.NewMockManagerInterface(testCtrl) - s = New(nil, hostMock, hostMock, hostMock, hostMock, hostMock, netlinkLibMock, dputilsLibMock, sriovnetLibMock, ghwLibMock) + s = New(nil, hostMock, hostMock, hostMock, hostMock, hostMock, netlinkLibMock, dputilsLibMock, sriovnetLibMock, ghwLibMock, hostMock) }) AfterEach(func() { diff --git a/pkg/host/manager.go b/pkg/host/manager.go index b70f6b387..02a77a659 100644 --- a/pkg/host/manager.go +++ b/pkg/host/manager.go @@ -1,6 +1,7 @@ package host import ( + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/bridge" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/infiniband" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/kernel" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/dputils" @@ -28,6 +29,7 @@ type HostManagerInterface interface { types.SriovInterface types.VdpaInterface types.InfinibandInterface + types.BridgeInterface } type hostManager struct { @@ -39,6 +41,7 @@ type hostManager struct { types.SriovInterface types.VdpaInterface types.InfinibandInterface + types.BridgeInterface } func NewHostManager(utilsInterface utils.CmdInterface) (HostManagerInterface, error) { @@ -56,8 +59,8 @@ func NewHostManager(utilsInterface utils.CmdInterface) (HostManagerInterface, er if err != nil { return nil, err } - sr := sriov.New(utilsInterface, k, n, u, v, ib, netlinkLib, dpUtils, sriovnetLib, ghwLib) - + br := bridge.New() + sr := sriov.New(utilsInterface, k, n, u, v, ib, netlinkLib, dpUtils, sriovnetLib, ghwLib, br) return &hostManager{ utilsInterface, k, @@ -67,5 +70,6 @@ func NewHostManager(utilsInterface utils.CmdInterface) (HostManagerInterface, er sr, v, ib, + br, }, nil } diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go index 36c96b69a..cb4d1480a 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -179,6 +179,20 @@ func (mr *MockHostManagerInterfaceMockRecorder) ConfigSriovInterfaces(storeManag return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigSriovInterfaces", reflect.TypeOf((*MockHostManagerInterface)(nil).ConfigSriovInterfaces), storeManager, interfaces, ifaceStatuses, skipVFConfiguration) } +// ConfigureBridges mocks base method. +func (m *MockHostManagerInterface) ConfigureBridges(bridgesSpec, bridgesStatus v1.Bridges) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConfigureBridges", bridgesSpec, bridgesStatus) + ret0, _ := ret[0].(error) + return ret0 +} + +// ConfigureBridges indicates an expected call of ConfigureBridges. +func (mr *MockHostManagerInterfaceMockRecorder) ConfigureBridges(bridgesSpec, bridgesStatus interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigureBridges", reflect.TypeOf((*MockHostManagerInterface)(nil).ConfigureBridges), bridgesSpec, bridgesStatus) +} + // ConfigureVfGUID mocks base method. func (m *MockHostManagerInterface) ConfigureVfGUID(vfAddr, pfAddr string, vfID int, pfLink netlink.Link) error { m.ctrl.T.Helper() @@ -221,6 +235,35 @@ func (mr *MockHostManagerInterfaceMockRecorder) DeleteVDPADevice(pciAddr interfa return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVDPADevice", reflect.TypeOf((*MockHostManagerInterface)(nil).DeleteVDPADevice), pciAddr) } +// DetachInterfaceFromManagedBridge mocks base method. +func (m *MockHostManagerInterface) DetachInterfaceFromManagedBridge(pciAddr string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DetachInterfaceFromManagedBridge", pciAddr) + ret0, _ := ret[0].(error) + return ret0 +} + +// DetachInterfaceFromManagedBridge indicates an expected call of DetachInterfaceFromManagedBridge. +func (mr *MockHostManagerInterfaceMockRecorder) DetachInterfaceFromManagedBridge(pciAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DetachInterfaceFromManagedBridge", reflect.TypeOf((*MockHostManagerInterface)(nil).DetachInterfaceFromManagedBridge), pciAddr) +} + +// DiscoverBridges mocks base method. +func (m *MockHostManagerInterface) DiscoverBridges() (v1.Bridges, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DiscoverBridges") + ret0, _ := ret[0].(v1.Bridges) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DiscoverBridges indicates an expected call of DiscoverBridges. +func (mr *MockHostManagerInterfaceMockRecorder) DiscoverBridges() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DiscoverBridges", reflect.TypeOf((*MockHostManagerInterface)(nil).DiscoverBridges)) +} + // DiscoverSriovDevices mocks base method. func (m *MockHostManagerInterface) DiscoverSriovDevices(storeManager store.ManagerInterface) ([]v1.InterfaceExt, error) { m.ctrl.T.Helper() diff --git a/pkg/plugins/generic/generic_plugin.go b/pkg/plugins/generic/generic_plugin.go index b01653d90..14b1903e5 100644 --- a/pkg/plugins/generic/generic_plugin.go +++ b/pkg/plugins/generic/generic_plugin.go @@ -48,13 +48,14 @@ type DriverState struct { type DriverStateMapType map[uint]*DriverState type GenericPlugin struct { - PluginName string - SpecVersion string - DesireState *sriovnetworkv1.SriovNetworkNodeState - DriverStateMap DriverStateMapType - DesiredKernelArgs map[string]bool - helpers helper.HostHelpersInterface - skipVFConfiguration bool + PluginName string + SpecVersion string + DesireState *sriovnetworkv1.SriovNetworkNodeState + DriverStateMap DriverStateMapType + DesiredKernelArgs map[string]bool + helpers helper.HostHelpersInterface + skipVFConfiguration bool + skipBridgeConfiguration bool } type Option = func(c *genericPluginOptions) @@ -68,8 +69,16 @@ func WithSkipVFConfiguration() Option { } } +// WithSkipBridgeConfiguration configures generic_plugin to skip configuration of the managed bridges +func WithSkipBridgeConfiguration() Option { + return func(c *genericPluginOptions) { + c.skipBridgeConfiguration = true + } +} + type genericPluginOptions struct { - skipVFConfiguration bool + skipVFConfiguration bool + skipBridgeConfiguration bool } const scriptsPath = "bindata/scripts/enable-kargs.sh" @@ -103,12 +112,13 @@ func NewGenericPlugin(helpers helper.HostHelpersInterface, options ...Option) (p DriverLoaded: false, } return &GenericPlugin{ - PluginName: PluginName, - SpecVersion: "1.0", - DriverStateMap: driverStateMap, - DesiredKernelArgs: make(map[string]bool), - helpers: helpers, - skipVFConfiguration: cfg.skipVFConfiguration, + PluginName: PluginName, + SpecVersion: "1.0", + DriverStateMap: driverStateMap, + DesiredKernelArgs: make(map[string]bool), + helpers: helpers, + skipVFConfiguration: cfg.skipVFConfiguration, + skipBridgeConfiguration: cfg.skipBridgeConfiguration, }, nil } @@ -127,7 +137,7 @@ func (p *GenericPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeSt log.Log.Info("generic plugin OnNodeStateChange()") p.DesireState = new - needDrain = p.needDrainNode(new.Spec.Interfaces, new.Status.Interfaces) + needDrain = p.needDrainNode(new.Spec, new.Status) needReboot, err = p.needRebootNode(new) if err != nil { return needDrain, needReboot, err @@ -161,6 +171,13 @@ func (p *GenericPlugin) CheckStatusChanges(current *sriovnetworkv1.SriovNetworkN } } + if p.shouldConfigureBridges() { + if sriovnetworkv1.NeedToUpdateBridges(¤t.Spec.Bridges, ¤t.Status.Bridges) { + log.Log.Info("CheckStatusChanges(): bridge configuration needs to be updated") + return true, nil + } + } + missingKernelArgs, err := p.getMissingKernelArgs() if err != nil { log.Log.Error(err, "generic-plugin CheckStatusChanges(): failed to verify missing kernel arguments") @@ -215,6 +232,12 @@ func (p *GenericPlugin) Apply() error { return err } + if p.shouldConfigureBridges() { + if err := p.helpers.ConfigureBridges(p.DesireState.Spec.Bridges, p.DesireState.Status.Bridges); err != nil { + return err + } + } + return nil } @@ -323,27 +346,39 @@ func (p *GenericPlugin) syncDesiredKernelArgs(kargs []string) (bool, error) { return needReboot, nil } -func (p *GenericPlugin) needDrainNode(desired sriovnetworkv1.Interfaces, current sriovnetworkv1.InterfaceExts) (needDrain bool) { +func (p *GenericPlugin) needDrainNode(desired sriovnetworkv1.SriovNetworkNodeStateSpec, current sriovnetworkv1.SriovNetworkNodeStateStatus) bool { log.Log.V(2).Info("generic plugin needDrainNode()", "current", current, "desired", desired) - needDrain = false - for _, ifaceStatus := range current { + if p.needToUpdateVFs(desired, current) { + return true + } + + if p.shouldConfigureBridges() { + if sriovnetworkv1.NeedToUpdateBridges(&desired.Bridges, ¤t.Bridges) { + log.Log.V(2).Info("generic plugin needDrainNode(): need drain since bridge configuration needs to be updated") + return true + } + } + return false +} + +func (p *GenericPlugin) needToUpdateVFs(desired sriovnetworkv1.SriovNetworkNodeStateSpec, current sriovnetworkv1.SriovNetworkNodeStateStatus) bool { + for _, ifaceStatus := range current.Interfaces { configured := false - for _, iface := range desired { + for _, iface := range desired.Interfaces { if iface.PciAddress == ifaceStatus.PciAddress { configured = true if ifaceStatus.NumVfs == 0 { - log.Log.V(2).Info("generic plugin needDrainNode(): no need drain, for PCI address, current NumVfs is 0", + log.Log.V(2).Info("generic plugin needToUpdateVFs(): no need drain, for PCI address, current NumVfs is 0", "address", iface.PciAddress) break } if sriovnetworkv1.NeedToUpdateSriov(&iface, &ifaceStatus) { - log.Log.V(2).Info("generic plugin needDrainNode(): need drain, for PCI address request update", + log.Log.V(2).Info("generic plugin needToUpdateVFs(): need drain, for PCI address request update", "address", iface.PciAddress) - needDrain = true - return + return true } - log.Log.V(2).Info("generic plugin needDrainNode(): no need drain,for PCI address", + log.Log.V(2).Info("generic plugin needToUpdateVFs(): no need drain,for PCI address", "address", iface.PciAddress, "expected-vfs", iface.NumVfs, "current-vfs", ifaceStatus.NumVfs) } } @@ -351,32 +386,35 @@ func (p *GenericPlugin) needDrainNode(desired sriovnetworkv1.Interfaces, current // load the PF info pfStatus, exist, err := p.helpers.LoadPfsStatus(ifaceStatus.PciAddress) if err != nil { - log.Log.Error(err, "generic plugin needDrainNode(): failed to load info about PF status for pci device", + log.Log.Error(err, "generic plugin needToUpdateVFs(): failed to load info about PF status for pci device", "address", ifaceStatus.PciAddress) continue } if !exist { - log.Log.Info("generic plugin needDrainNode(): PF name with pci address has VFs configured but they weren't created by the sriov operator. Skipping drain", + log.Log.Info("generic plugin needToUpdateVFs(): PF name with pci address has VFs configured but they weren't created by the sriov operator. Skipping drain", "name", ifaceStatus.Name, "address", ifaceStatus.PciAddress) continue } if pfStatus.ExternallyManaged { - log.Log.Info("generic plugin needDrainNode()(): PF name with pci address was externally created. Skipping drain", + log.Log.Info("generic plugin needToUpdateVFs(): PF name with pci address was externally created. Skipping drain", "name", ifaceStatus.Name, "address", ifaceStatus.PciAddress) continue } - log.Log.V(2).Info("generic plugin needDrainNode(): need drain since interface needs to be reset", + log.Log.V(2).Info("generic plugin needToUpdateVFs(): need drain since interface needs to be reset", "interface", ifaceStatus) - needDrain = true - return + return true } } - return + return false +} + +func (p *GenericPlugin) shouldConfigureBridges() bool { + return vars.ManageSoftwareBridges && !p.skipBridgeConfiguration } func (p *GenericPlugin) addVfioDesiredKernelArg(state *sriovnetworkv1.SriovNetworkNodeState) { diff --git a/pkg/plugins/generic/generic_plugin_test.go b/pkg/plugins/generic/generic_plugin_test.go index cf16efd75..0d6701a64 100644 --- a/pkg/plugins/generic/generic_plugin_test.go +++ b/pkg/plugins/generic/generic_plugin_test.go @@ -11,6 +11,7 @@ import ( "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts" mock_helper "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/helper/mock" plugin "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" ) func TestGenericPlugin(t *testing.T) { @@ -35,6 +36,10 @@ var _ = Describe("Generic plugin", func() { genericPlugin, err = NewGenericPlugin(hostHelper) Expect(err).ToNot(HaveOccurred()) + + manageSoftwareBridgesOrigValue := vars.ManageSoftwareBridges + vars.ManageSoftwareBridges = true + DeferCleanup(func() { vars.ManageSoftwareBridges = manageSoftwareBridgesOrigValue }) }) Context("OnNodeStateChange", func() { @@ -1073,5 +1078,207 @@ var _ = Describe("Generic plugin", func() { Expect(driverState.DriverLoaded).To(BeTrue()) }) }) - + It("should not drain - bridge config", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:d8:00.0", + NumVfs: 1, + Name: "enp216s0f0np0", + EswitchMode: "switchdev", + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-0", + }}}}, + Bridges: sriovnetworkv1.Bridges{ + OVS: []sriovnetworkv1.OVSConfigExt{{ + Name: "br-0000_d8_00.0", + Uplinks: []sriovnetworkv1.OVSUplinkConfigExt{{ + PciAddress: "0000:d8:00.0", + Name: "enp216s0f0np0", + }}, + }}, + }}, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:d8:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "a2d6", + Vendor: "15b3", + Name: "enp216s0f0np0", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "switchdev", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:d8:00.2", + DeviceID: "101e", + Vendor: "15b3", + VfID: 0, + Name: "enp216s0f0v0", + Mtu: 1500, + Mac: "8e:d6:2c:62:87:1b", + Driver: "mlx5_core", + }}, + }}, + Bridges: sriovnetworkv1.Bridges{ + OVS: []sriovnetworkv1.OVSConfigExt{{ + Name: "br-0000_d8_00.0", + Uplinks: []sriovnetworkv1.OVSUplinkConfigExt{{ + PciAddress: "0000:d8:00.0", + Name: "enp216s0f0np0", + }}, + }}, + }, + }} + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + }) + It("should drain - bridge config mismatch", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:d8:00.0", + NumVfs: 1, + Name: "enp216s0f0np0", + EswitchMode: "switchdev", + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-0", + }}}}, + Bridges: sriovnetworkv1.Bridges{ + OVS: []sriovnetworkv1.OVSConfigExt{{ + Name: "br-0000_d8_00.0", + Bridge: sriovnetworkv1.OVSBridgeConfig{ + DatapathType: "netdev", + }, + Uplinks: []sriovnetworkv1.OVSUplinkConfigExt{{ + PciAddress: "0000:d8:00.0", + Name: "enp216s0f0np0", + Interface: sriovnetworkv1.OVSInterfaceConfig{ + Type: "dpdk", + }, + }}, + }}, + }}, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:d8:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "a2d6", + Vendor: "15b3", + Name: "enp216s0f0np0", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "switchdev", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:d8:00.2", + DeviceID: "101e", + Vendor: "15b3", + VfID: 0, + Name: "enp216s0f0v0", + Mtu: 1500, + Mac: "8e:d6:2c:62:87:1b", + Driver: "mlx5_core", + }}, + }}, + Bridges: sriovnetworkv1.Bridges{ + OVS: []sriovnetworkv1.OVSConfigExt{{ + Name: "br-0000_d8_00.0", + Uplinks: []sriovnetworkv1.OVSUplinkConfigExt{{ + PciAddress: "0000:d8:00.0", + Name: "enp216s0f0np0", + }}, + }}, + }, + }} + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeTrue()) + }) + It("check status - bridge config mismatch", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:d8:00.0", + NumVfs: 1, + Name: "enp216s0f0np0", + EswitchMode: "switchdev", + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-0", + }}}}, + Bridges: sriovnetworkv1.Bridges{ + OVS: []sriovnetworkv1.OVSConfigExt{{ + Name: "br-0000_d8_00.0", + Bridge: sriovnetworkv1.OVSBridgeConfig{ + DatapathType: "netdev", + }, + Uplinks: []sriovnetworkv1.OVSUplinkConfigExt{{ + PciAddress: "0000:d8:00.0", + Name: "enp216s0f0np0", + Interface: sriovnetworkv1.OVSInterfaceConfig{ + Type: "dpdk", + }, + }}, + }}, + }}, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:d8:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "a2d6", + Vendor: "15b3", + Name: "enp216s0f0np0", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "switchdev", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:d8:00.2", + DeviceID: "101e", + Vendor: "15b3", + VfID: 0, + Name: "enp216s0f0v0", + Mtu: 1500, + Mac: "8e:d6:2c:62:87:1b", + Driver: "mlx5_core", + }}, + }}, + Bridges: sriovnetworkv1.Bridges{ + OVS: []sriovnetworkv1.OVSConfigExt{{ + Name: "br-0000_d8_00.0", + Uplinks: []sriovnetworkv1.OVSUplinkConfigExt{{ + PciAddress: "0000:d8:00.0", + Name: "enp216s0f0np0", + }}, + }}, + }, + }} + updated, err := genericPlugin.CheckStatusChanges(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(updated).To(BeTrue()) + }) }) diff --git a/pkg/systemd/systemd.go b/pkg/systemd/systemd.go index 6136a31b0..704e72c0f 100644 --- a/pkg/systemd/systemd.go +++ b/pkg/systemd/systemd.go @@ -47,6 +47,7 @@ type SriovConfig struct { UnsupportedNics bool `yaml:"unsupportedNics"` PlatformType consts.PlatformTypes `yaml:"platformType"` ManageSoftwareBridges bool `yaml:"manageSoftwareBridges"` + OVSDBSocketPath string `yaml:"ovsdbSocketPath"` } type SriovResult struct { @@ -72,6 +73,7 @@ func WriteConfFile(newState *sriovnetworkv1.SriovNetworkNodeState) (bool, error) vars.DevMode, vars.PlatformType, vars.ManageSoftwareBridges, + vars.OVSDBSocketPath, } _, err := os.Stat(utils.GetHostExtensionPath(SriovSystemdConfigPath))