From 0ecd227dafc8e44e7cdf88e4c2ebc0073e01cf6d Mon Sep 17 00:00:00 2001 From: Vasilis Remmas Date: Wed, 8 May 2024 12:19:54 +0200 Subject: [PATCH] Add more checks on generic plugin Today, we are missing 2 checks on settings that sriov netop is configuring: * PF is up * GUID is set Without checking for these 2, we risk that changes made by the user directly to the system are not reconciled by the netop leaving the system in a bad state. This commit is adding those checks. Signed-off-by: Vasilis Remmas --- api/v1/helper.go | 16 + api/v1/sriovnetworknodestate_types.go | 2 + ...k.openshift.io_sriovnetworknodestates.yaml | 4 + ...k.openshift.io_sriovnetworknodestates.yaml | 4 + pkg/consts/constants.go | 2 + pkg/helper/mock/mock_helper.go | 28 + .../internal/lib/netlink/mock/mock_netlink.go | 15 + pkg/host/internal/lib/netlink/netlink.go | 9 + pkg/host/internal/network/network.go | 52 ++ pkg/host/internal/network/network_test.go | 36 + pkg/host/internal/sriov/sriov.go | 23 +- pkg/host/mock/mock_host.go | 28 + pkg/host/types/interfaces.go | 4 + pkg/plugins/generic/generic_plugin_test.go | 657 +++++++++++++++++- 14 files changed, 845 insertions(+), 35 deletions(-) diff --git a/api/v1/helper.go b/api/v1/helper.go index 8b1cf9c96..0146f96ea 100644 --- a/api/v1/helper.go +++ b/api/v1/helper.go @@ -271,6 +271,12 @@ func NeedToUpdateSriov(ifaceSpec *Interface, ifaceStatus *InterfaceExt) bool { log.V(2).Info("NeedToUpdateSriov(): NumVfs needs update", "desired", ifaceSpec.NumVfs, "current", ifaceStatus.NumVfs) return true } + + if ifaceStatus.LinkAdminState == "down" { + log.V(2).Info("NeedToUpdateSriov(): PF link status needs update", "desired to include", "up", "current", ifaceStatus.LinkAdminState) + return true + } + if ifaceSpec.NumVfs > 0 { for _, vfStatus := range ifaceStatus.VFs { ingroup := false @@ -300,6 +306,16 @@ func NeedToUpdateSriov(ifaceSpec *Interface, ifaceStatus *InterfaceExt) bool { return true } + if (strings.EqualFold(ifaceStatus.LinkType, consts.LinkTypeETH) && groupSpec.IsRdma) || strings.EqualFold(ifaceStatus.LinkType, consts.LinkTypeIB) { + // We do this check only if a Node GUID is set to ensure that we were able to read the + // Node GUID. We intentionally skip empty Node GUID in vfStatus because this may happen + // when the VF is allocated to a workload. + if vfStatus.GUID == consts.UninitializedNodeGUID { + log.V(2).Info("NeedToUpdateSriov(): VF GUID needs update", + "vf", vfStatus.VfID, "current", vfStatus.GUID) + return true + } + } // this is needed to be sure the admin mac address is configured as expected if ifaceSpec.ExternallyManaged { log.V(2).Info("NeedToUpdateSriov(): need to update the device as it's externally manage", diff --git a/api/v1/sriovnetworknodestate_types.go b/api/v1/sriovnetworknodestate_types.go index 74dd1b58b..d1d8a4ce8 100644 --- a/api/v1/sriovnetworknodestate_types.go +++ b/api/v1/sriovnetworknodestate_types.go @@ -63,6 +63,7 @@ type InterfaceExt struct { NumVfs int `json:"numVfs,omitempty"` LinkSpeed string `json:"linkSpeed,omitempty"` LinkType string `json:"linkType,omitempty"` + LinkAdminState string `json:"linkAdminState,omitempty"` EswitchMode string `json:"eSwitchMode,omitempty"` ExternallyManaged bool `json:"externallyManaged,omitempty"` TotalVfs int `json:"totalvfs,omitempty"` @@ -82,6 +83,7 @@ type VirtualFunction struct { Mtu int `json:"mtu,omitempty"` VfID int `json:"vfID"` VdpaType string `json:"vdpaType,omitempty"` + GUID string `json:"guid,omitempty"` } // SriovNetworkNodeStateStatus defines the observed state of SriovNetworkNodeState diff --git a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml index a68c16f4e..2787b48fd 100644 --- a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml +++ b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml @@ -108,6 +108,8 @@ spec: type: string driver: type: string + guid: + type: string mac: type: string mtu: @@ -135,6 +137,8 @@ spec: type: string externallyManaged: type: boolean + linkAdminState: + type: string linkSpeed: type: string linkType: diff --git a/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml b/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml index a68c16f4e..2787b48fd 100644 --- a/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml +++ b/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml @@ -108,6 +108,8 @@ spec: type: string driver: type: string + guid: + type: string mac: type: string mtu: @@ -135,6 +137,8 @@ spec: type: string externallyManaged: type: boolean + linkAdminState: + type: string linkSpeed: type: string linkType: diff --git a/pkg/consts/constants.go b/pkg/consts/constants.go index 8e5e1fdcf..3123107a8 100644 --- a/pkg/consts/constants.go +++ b/pkg/consts/constants.go @@ -43,6 +43,8 @@ const ( LinkTypeIB = "IB" LinkTypeETH = "ETH" + UninitializedNodeGUID = "0000:0000:0000:0000" + DeviceTypeVfioPci = "vfio-pci" DeviceTypeNetDevice = "netdevice" VdpaTypeVirtio = "virtio" diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index 4d8db7bf6..f964641db 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -415,6 +415,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) GetMlxNicFwData(pciAddress inter return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMlxNicFwData", reflect.TypeOf((*MockHostHelpersInterface)(nil).GetMlxNicFwData), pciAddress) } +// GetNetDevLinkAdminState mocks base method. +func (m *MockHostHelpersInterface) GetNetDevLinkAdminState(ifaceName string) string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNetDevLinkAdminState", ifaceName) + ret0, _ := ret[0].(string) + return ret0 +} + +// GetNetDevLinkAdminState indicates an expected call of GetNetDevLinkAdminState. +func (mr *MockHostHelpersInterfaceMockRecorder) GetNetDevLinkAdminState(ifaceName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetDevLinkAdminState", reflect.TypeOf((*MockHostHelpersInterface)(nil).GetNetDevLinkAdminState), ifaceName) +} + // GetNetDevLinkSpeed mocks base method. func (m *MockHostHelpersInterface) GetNetDevLinkSpeed(name string) string { m.ctrl.T.Helper() @@ -443,6 +457,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) GetNetDevMac(name interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetDevMac", reflect.TypeOf((*MockHostHelpersInterface)(nil).GetNetDevMac), name) } +// GetNetDevNodeGUID mocks base method. +func (m *MockHostHelpersInterface) GetNetDevNodeGUID(pciAddr string) string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNetDevNodeGUID", pciAddr) + ret0, _ := ret[0].(string) + return ret0 +} + +// GetNetDevNodeGUID indicates an expected call of GetNetDevNodeGUID. +func (mr *MockHostHelpersInterfaceMockRecorder) GetNetDevNodeGUID(pciAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetDevNodeGUID", reflect.TypeOf((*MockHostHelpersInterface)(nil).GetNetDevNodeGUID), pciAddr) +} + // GetNetdevMTU mocks base method. func (m *MockHostHelpersInterface) GetNetdevMTU(pciAddr string) int { m.ctrl.T.Helper() diff --git a/pkg/host/internal/lib/netlink/mock/mock_netlink.go b/pkg/host/internal/lib/netlink/mock/mock_netlink.go index febef3395..882be4942 100644 --- a/pkg/host/internal/lib/netlink/mock/mock_netlink.go +++ b/pkg/host/internal/lib/netlink/mock/mock_netlink.go @@ -230,6 +230,21 @@ func (mr *MockNetlinkLibMockRecorder) LinkSetVfPortGUID(link, vf, portguid inter return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LinkSetVfPortGUID", reflect.TypeOf((*MockNetlinkLib)(nil).LinkSetVfPortGUID), link, vf, portguid) } +// RdmaLinkByName mocks base method. +func (m *MockNetlinkLib) RdmaLinkByName(name string) (*netlink0.RdmaLink, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RdmaLinkByName", name) + ret0, _ := ret[0].(*netlink0.RdmaLink) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// RdmaLinkByName indicates an expected call of RdmaLinkByName. +func (mr *MockNetlinkLibMockRecorder) RdmaLinkByName(name interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RdmaLinkByName", reflect.TypeOf((*MockNetlinkLib)(nil).RdmaLinkByName), name) +} + // VDPADelDev mocks base method. func (m *MockNetlinkLib) VDPADelDev(name string) error { m.ctrl.T.Helper() diff --git a/pkg/host/internal/lib/netlink/netlink.go b/pkg/host/internal/lib/netlink/netlink.go index 5d764ebf5..19980400d 100644 --- a/pkg/host/internal/lib/netlink/netlink.go +++ b/pkg/host/internal/lib/netlink/netlink.go @@ -58,6 +58,9 @@ type NetlinkLib interface { // cmode argument should contain valid cmode value as uint8, modes are define in nl.DEVLINK_PARAM_CMODE_* constants // value argument should have one of the following types: uint8, uint16, uint32, string, bool DevlinkSetDeviceParam(bus string, device string, param string, cmode uint8, value interface{}) error + // RdmaLinkByName finds a link by name and returns a pointer to the object if + // found and nil error, otherwise returns error code. + RdmaLinkByName(name string) (*netlink.RdmaLink, error) } type libWrapper struct{} @@ -142,3 +145,9 @@ func (w *libWrapper) DevlinkGetDeviceParamByName(bus string, device string, para func (w *libWrapper) DevlinkSetDeviceParam(bus string, device string, param string, cmode uint8, value interface{}) error { return netlink.DevlinkSetDeviceParam(bus, device, param, cmode, value) } + +// RdmaLinkByName finds a link by name and returns a pointer to the object if +// found and nil error, otherwise returns error code. +func (w *libWrapper) RdmaLinkByName(name string) (*netlink.RdmaLink, error) { + return netlink.RdmaLinkByName(name) +} diff --git a/pkg/host/internal/network/network.go b/pkg/host/internal/network/network.go index 3973d0fa4..370d03e6d 100644 --- a/pkg/host/internal/network/network.go +++ b/pkg/host/internal/network/network.go @@ -1,7 +1,10 @@ package network import ( + "errors" "fmt" + "io/fs" + "net" "os" "path/filepath" "strconv" @@ -187,6 +190,35 @@ func (n *network) GetNetDevMac(ifaceName string) string { return link.Attrs().HardwareAddr.String() } +// GetNetDevNodeGUID returns the network interface node GUID if device is RDMA capable otherwise returns empty string +func (n *network) GetNetDevNodeGUID(pciAddr string) string { + if len(pciAddr) == 0 { + return "" + } + + rdmaDevicesPath := filepath.Join(vars.FilesystemRoot, consts.SysBusPciDevices, pciAddr, "infiniband") + rdmaDevices, err := os.ReadDir(rdmaDevicesPath) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + log.Log.Error(err, "GetNetDevNodeGUID(): failed to read RDMA related directory", "pciAddr", pciAddr) + } + return "" + } + + if len(rdmaDevices) != 1 { + log.Log.Error(err, "GetNetDevNodeGUID(): expected just one RDMA device", "pciAddr", pciAddr, "numOfDevices", len(rdmaDevices)) + return "" + } + + rdmaLink, err := n.netlinkLib.RdmaLinkByName(rdmaDevices[0].Name()) + if err != nil { + log.Log.Error(err, "GetNetDevNodeGUID(): failed to get RDMA link", "pciAddr", pciAddr) + return "" + } + + return rdmaLink.Attrs.NodeGuid +} + func (n *network) GetNetDevLinkSpeed(ifaceName string) string { log.Log.V(2).Info("GetNetDevLinkSpeed(): get LinkSpeed", "device", ifaceName) speedFilePath := filepath.Join(vars.FilesystemRoot, consts.SysClassNet, ifaceName, "speed") @@ -329,3 +361,23 @@ func (n *network) EnableHwTcOffload(ifaceName string) error { log.Log.V(0).Info("EnableHwTcOffload(): feature is still disabled, not supported by device", "device", ifaceName) return nil } + +// GetNetDevLinkAdminState returns the admin state of the interface. +func (n *network) GetNetDevLinkAdminState(ifaceName string) string { + log.Log.V(2).Info("GetNetDevLinkAdminState(): get LinkAdminState", "device", ifaceName) + if len(ifaceName) == 0 { + return "" + } + + link, err := n.netlinkLib.LinkByName(ifaceName) + if err != nil { + log.Log.Error(err, "GetNetDevLinkAdminState(): failed to get link", "device", ifaceName) + return "" + } + + if link.Attrs().Flags&net.FlagUp == 0 { + return "down" + } + + return "up" +} diff --git a/pkg/host/internal/network/network_test.go b/pkg/host/internal/network/network_test.go index cd4dbaedc..ef181ddba 100644 --- a/pkg/host/internal/network/network_test.go +++ b/pkg/host/internal/network/network_test.go @@ -15,6 +15,8 @@ import ( ethtoolMockPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/ethtool/mock" netlinkMockPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/netlink/mock" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types" + "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/fakefilesystem" + "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/helpers" ) func getDevlinkParam(t uint8, value interface{}) *netlink.DevlinkParam { @@ -202,4 +204,38 @@ var _ = Describe("Network", func() { Expect(n.EnableHwTcOffload("enp216s0f0np0")).To(MatchError(testErr)) }) }) + Context("GetNetDevNodeGUID", func() { + It("Returns empty when pciAddr is empty", func() { + Expect(n.GetNetDevNodeGUID("")).To(Equal("")) + }) + It("Returns empty when infiniband directory can't be read", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/sys/bus/pci/devices/0000:4b:00.3/"}, + }) + Expect(n.GetNetDevNodeGUID("0000:4b:00.3")).To(Equal("")) + }) + It("Returns empty when more than one RDMA devices are detected for pciAddr", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{ + "/sys/bus/pci/devices/0000:4b:00.3/infiniband/mlx5_2", + "/sys/bus/pci/devices/0000:4b:00.3/infiniband/mlx5_3", + }, + }) + Expect(n.GetNetDevNodeGUID("0000:4b:00.3")).To(Equal("")) + }) + It("Returns empty when it fails to read RDMA link", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/sys/bus/pci/devices/0000:4b:00.3/infiniband/mlx5_2"}, + }) + netlinkLibMock.EXPECT().RdmaLinkByName("mlx5_2").Return(nil, fmt.Errorf("some-error")) + Expect(n.GetNetDevNodeGUID("0000:4b:00.3")).To(Equal("")) + }) + It("Returns populated node GUID on correct setup", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/sys/bus/pci/devices/0000:4b:00.3/infiniband/mlx5_2"}, + }) + netlinkLibMock.EXPECT().RdmaLinkByName("mlx5_2").Return(&netlink.RdmaLink{Attrs: netlink.RdmaLinkAttrs{NodeGuid: "1122:3344:5566:7788"}}, nil) + Expect(n.GetNetDevNodeGUID("0000:4b:00.3")).To(Equal("1122:3344:5566:7788")) + }) + }) }) diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go index 0e2bfa531..0f0e60624 100644 --- a/pkg/host/internal/sriov/sriov.go +++ b/pkg/host/internal/sriov/sriov.go @@ -3,6 +3,7 @@ package sriov import ( "errors" "fmt" + "net" "os" "path/filepath" "strconv" @@ -134,6 +135,7 @@ func (s *sriov) GetVfInfo(pciAddr string, devices []*ghw.PCIDevice) sriovnetwork vf.Mac = link.Attrs().HardwareAddr.String() } } + vf.GUID = s.networkHelper.GetNetDevNodeGUID(pciAddr) for _, device := range devices { if pciAddr == device.Address { @@ -259,15 +261,16 @@ func (s *sriov) DiscoverSriovDevices(storeManager store.ManagerInterface) ([]sri } iface := sriovnetworkv1.InterfaceExt{ - Name: pfNetName, - PciAddress: device.Address, - Driver: driver, - Vendor: device.Vendor.ID, - DeviceID: device.Product.ID, - Mtu: link.Attrs().MTU, - Mac: link.Attrs().HardwareAddr.String(), - LinkType: s.encapTypeToLinkType(link.Attrs().EncapType), - LinkSpeed: s.networkHelper.GetNetDevLinkSpeed(pfNetName), + Name: pfNetName, + PciAddress: device.Address, + Driver: driver, + Vendor: device.Vendor.ID, + DeviceID: device.Product.ID, + Mtu: link.Attrs().MTU, + Mac: link.Attrs().HardwareAddr.String(), + LinkType: s.encapTypeToLinkType(link.Attrs().EncapType), + LinkSpeed: s.networkHelper.GetNetDevLinkSpeed(pfNetName), + LinkAdminState: s.networkHelper.GetNetDevLinkAdminState(pfNetName), } pfStatus, exist, err := storeManager.LoadPfsStatus(iface.PciAddress) @@ -571,7 +574,7 @@ func (s *sriov) configSriovDevice(iface *sriovnetworkv1.Interface, skipVFConfigu if err != nil { return err } - if pfLink.Attrs().OperState != netlink.OperUp { + if pfLink.Attrs().Flags&net.FlagUp == 0 { err = s.netlinkLib.LinkSetUp(pfLink) if err != nil { return err diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go index 4ea2f0e8b..6a7f564e7 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -339,6 +339,20 @@ func (mr *MockHostManagerInterfaceMockRecorder) GetLinkType(name interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLinkType", reflect.TypeOf((*MockHostManagerInterface)(nil).GetLinkType), name) } +// GetNetDevLinkAdminState mocks base method. +func (m *MockHostManagerInterface) GetNetDevLinkAdminState(ifaceName string) string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNetDevLinkAdminState", ifaceName) + ret0, _ := ret[0].(string) + return ret0 +} + +// GetNetDevLinkAdminState indicates an expected call of GetNetDevLinkAdminState. +func (mr *MockHostManagerInterfaceMockRecorder) GetNetDevLinkAdminState(ifaceName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetDevLinkAdminState", reflect.TypeOf((*MockHostManagerInterface)(nil).GetNetDevLinkAdminState), ifaceName) +} + // GetNetDevLinkSpeed mocks base method. func (m *MockHostManagerInterface) GetNetDevLinkSpeed(name string) string { m.ctrl.T.Helper() @@ -367,6 +381,20 @@ func (mr *MockHostManagerInterfaceMockRecorder) GetNetDevMac(name interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetDevMac", reflect.TypeOf((*MockHostManagerInterface)(nil).GetNetDevMac), name) } +// GetNetDevNodeGUID mocks base method. +func (m *MockHostManagerInterface) GetNetDevNodeGUID(pciAddr string) string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNetDevNodeGUID", pciAddr) + ret0, _ := ret[0].(string) + return ret0 +} + +// GetNetDevNodeGUID indicates an expected call of GetNetDevNodeGUID. +func (mr *MockHostManagerInterfaceMockRecorder) GetNetDevNodeGUID(pciAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetDevNodeGUID", reflect.TypeOf((*MockHostManagerInterface)(nil).GetNetDevNodeGUID), pciAddr) +} + // GetNetdevMTU mocks base method. func (m *MockHostManagerInterface) GetNetdevMTU(pciAddr string) int { m.ctrl.T.Helper() diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go index 48d47b424..49aecdee6 100644 --- a/pkg/host/types/interfaces.go +++ b/pkg/host/types/interfaces.go @@ -94,6 +94,8 @@ type NetworkInterface interface { SetNetdevMTU(pciAddr string, mtu int) error // GetNetDevMac returns the network interface mac address GetNetDevMac(name string) string + // GetNetDevNodeGUID returns the network interface node GUID if device is RDMA capable otherwise returns empty string + GetNetDevNodeGUID(pciAddr string) string // GetNetDevLinkSpeed returns the network interface link speed GetNetDevLinkSpeed(name string) string // GetDevlinkDeviceParam returns devlink parameter for the device as a string, if the parameter has multiple values @@ -105,6 +107,8 @@ type NetworkInterface interface { SetDevlinkDeviceParam(pciAddr, paramName, value string) error // EnableHwTcOffload make sure that hw-tc-offload feature is enabled if device supports it EnableHwTcOffload(ifaceName string) error + // GetNetDevLinkAdminState returns the admin state of the interface. + GetNetDevLinkAdminState(ifaceName string) string } type ServiceInterface interface { diff --git a/pkg/plugins/generic/generic_plugin_test.go b/pkg/plugins/generic/generic_plugin_test.go index d2e9c1bbe..cf16efd75 100644 --- a/pkg/plugins/generic/generic_plugin_test.go +++ b/pkg/plugins/generic/generic_plugin_test.go @@ -53,18 +53,118 @@ var _ = Describe("Generic plugin", func() { }, Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ Interfaces: sriovnetworkv1.InterfaceExts{{ - PciAddress: "0000:00:00.0", - NumVfs: 1, - TotalVfs: 1, - DeviceID: "1015", - Vendor: "15b3", - Name: "sriovif1", - Mtu: 1500, - Mac: "0c:42:a1:55:ee:46", - Driver: "mlx5_core", - EswitchMode: "legacy", - LinkSpeed: "25000 Mb/s", - LinkType: "ETH", + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Mac: "8e:d6:2c:62:87:1b", + Driver: "mlx5_core", + }}, + }}, + }, + } + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + }) + + It("should drain because MTU value has changed on PF", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 300, // Bad MTU value, changed by the user application + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Mac: "8e:d6:2c:62:87:1b", + Driver: "mlx5_core", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeTrue()) + }) + + It("should drain because numVFs value has changed on PF", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 2, // Bad numVFs value, changed by the user application + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", VFs: []sriovnetworkv1.VirtualFunction{{ PciAddress: "0000:00:00.1", DeviceID: "1016", @@ -78,13 +178,162 @@ var _ = Describe("Generic plugin", func() { }}, }, } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeTrue()) + }) + + It("should drain because PF link is down", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "IB", + LinkAdminState: "down", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Driver: "mlx5_core", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeTrue()) + }) + + It("should not drain if PF link is not down", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "IB", + LinkAdminState: "", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Driver: "mlx5_core", + }}, + }}, + }, + } + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) Expect(err).ToNot(HaveOccurred()) Expect(needReboot).To(BeFalse()) Expect(needDrain).To(BeFalse()) }) - It("should drain", func() { + It("should drain because driver has changed on VF of type netdevice", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Mac: "8e:d6:2c:62:87:1b", + Driver: "igb_uio", // The user has requested netdevice == mlx5_core + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeTrue()) + }) + + It("should drain because MTU value has changed on VF of type netdevice", func() { networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ Interfaces: sriovnetworkv1.Interfaces{{ @@ -101,18 +350,19 @@ var _ = Describe("Generic plugin", func() { }, Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ Interfaces: sriovnetworkv1.InterfaceExts{{ - PciAddress: "0000:00:00.0", - NumVfs: 2, - TotalVfs: 2, - DeviceID: "1015", - Vendor: "15b3", - Name: "sriovif1", - Mtu: 1500, - Mac: "0c:42:a1:55:ee:46", - Driver: "mlx5_core", - EswitchMode: "legacy", - LinkSpeed: "25000 Mb/s", - LinkType: "ETH", + PciAddress: "0000:00:00.0", + NumVfs: 2, + TotalVfs: 2, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", VFs: []sriovnetworkv1.VirtualFunction{{ PciAddress: "0000:00:00.1", DeviceID: "1016", @@ -127,6 +377,7 @@ var _ = Describe("Generic plugin", func() { DeviceID: "1016", Vendor: "15b3", VfID: 0, + Mtu: 1500, Driver: "mlx5_core", }}, }}, @@ -138,6 +389,362 @@ var _ = Describe("Generic plugin", func() { Expect(needDrain).To(BeTrue()) }) + It("should drain because GUID address value is the default one on VF of type netdevice, rdma enabled and link type ETH", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + IsRdma: true, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Driver: "mlx5_core", + Mac: "0c:42:a1:55:ee:46", + GUID: "0000:0000:0000:0000", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeTrue()) + }) + + It("should not drain if GUID address value is not set on VF of type netdevice, rdma enabled and link type ETH", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + IsRdma: true, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Driver: "mlx5_core", + Mac: "0c:42:a1:55:ee:46", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + }) + + It("should not drain if GUID address value is the default one on VF of type netdevice, rdma disabled and link type ETH", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Driver: "mlx5_core", + Mac: "0c:42:a1:55:ee:46", + GUID: "0000:0000:0000:0000", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + }) + + It("should drain because GUID address value is the default one on VF of type netdevice and link type IB", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "IB", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Driver: "mlx5_core", + GUID: "0000:0000:0000:0000", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeTrue()) + }) + + It("should not drain if GUID address value is not set on VF of type netdevice and link type IB", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "IB", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Driver: "mlx5_core", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + }) + + It("should drain because GUID address value is the default one on VF of type netdevice, rdma enabled and link type ETH", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + IsRdma: true, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Driver: "mlx5_core", + Mac: "0c:42:a1:55:ee:46", + GUID: "0000:0000:0000:0000", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeTrue()) + }) + + It("should not drain if GUID address value is not set on VF of type netdevice, rdma enabled and link type ETH", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + IsRdma: true, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + LinkAdminState: "up", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Driver: "mlx5_core", + Mac: "0c:42:a1:55:ee:46", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + }) + It("should not detect changes on status", func() { networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{