From 618315def52b61dafff255cfd73de799366f9305 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Tue, 2 Apr 2024 15:53:45 +0300 Subject: [PATCH 1/4] Add manageSoftwareBridges feature gate The feature gate controls state of the manage software bridge feature. This feature is disabled by default. Signed-off-by: Yury Kulazhenkov --- bindata/manifests/daemon/daemonset.yaml | 3 +++ cmd/sriov-network-config-daemon/service.go | 1 + cmd/sriov-network-config-daemon/start.go | 13 ++++++++----- controllers/sriovoperatorconfig_controller.go | 1 + pkg/consts/constants.go | 3 +++ pkg/systemd/systemd.go | 8 +++++--- pkg/vars/vars.go | 3 +++ 7 files changed, 24 insertions(+), 8 deletions(-) diff --git a/bindata/manifests/daemon/daemonset.yaml b/bindata/manifests/daemon/daemonset.yaml index 1c3d7d618..e3e5526e0 100644 --- a/bindata/manifests/daemon/daemonset.yaml +++ b/bindata/manifests/daemon/daemonset.yaml @@ -144,6 +144,9 @@ spec: {{- if .ParallelNicConfig }} - --parallel-nic-config {{- end }} + {{- if .ManageSoftwareBridges }} + - --manage-software-bridges + {{ end }} env: - name: NODE_NAME valueFrom: diff --git a/cmd/sriov-network-config-daemon/service.go b/cmd/sriov-network-config-daemon/service.go index 88b60036e..f86151cb7 100644 --- a/cmd/sriov-network-config-daemon/service.go +++ b/cmd/sriov-network-config-daemon/service.go @@ -92,6 +92,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 if err := initSupportedNics(); err != nil { return updateSriovResultErr(setupLog, phaseArg, fmt.Errorf("failed to initialize list of supported NIC ids: %v", err)) diff --git a/cmd/sriov-network-config-daemon/start.go b/cmd/sriov-network-config-daemon/start.go index 4311db11b..efe35bcbf 100644 --- a/cmd/sriov-network-config-daemon/start.go +++ b/cmd/sriov-network-config-daemon/start.go @@ -79,11 +79,12 @@ var ( } startOpts struct { - kubeconfig string - nodeName string - systemd bool - disabledPlugins stringList - parallelNicConfig bool + kubeconfig string + nodeName string + systemd bool + disabledPlugins stringList + parallelNicConfig bool + manageSoftwareBridges bool } ) @@ -94,6 +95,7 @@ func init() { startCmd.PersistentFlags().BoolVar(&startOpts.systemd, "use-systemd-service", false, "use config daemon in systemd mode") startCmd.PersistentFlags().VarP(&startOpts.disabledPlugins, "disable-plugins", "", "comma-separated list of plugins to disable") startCmd.PersistentFlags().BoolVar(&startOpts.parallelNicConfig, "parallel-nic-config", false, "perform NIC configuration in parallel") + startCmd.PersistentFlags().BoolVar(&startOpts.manageSoftwareBridges, "manage-software-bridges", false, "enable management of software bridges") } func runStartCmd(cmd *cobra.Command, args []string) error { @@ -108,6 +110,7 @@ func runStartCmd(cmd *cobra.Command, args []string) error { } vars.ParallelNicConfig = startOpts.parallelNicConfig + vars.ManageSoftwareBridges = startOpts.manageSoftwareBridges if startOpts.nodeName == "" { name, ok := os.LookupEnv("NODE_NAME") diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go index 11cfdfc36..1ff6fb6f2 100644 --- a/controllers/sriovoperatorconfig_controller.go +++ b/controllers/sriovoperatorconfig_controller.go @@ -189,6 +189,7 @@ func (r *SriovOperatorConfigReconciler) syncConfigDaemonSet(ctx context.Context, data.Data["UsedSystemdMode"] = false } data.Data["ParallelNicConfig"] = r.FeatureGate.IsEnabled(consts.ParallelNicConfigFeatureGate) + data.Data["ManageSoftwareBridges"] = r.FeatureGate.IsEnabled(consts.ManageSoftwareBridgesFeatureGate) envCniBinPath := os.Getenv("SRIOV_CNI_BIN_PATH") if envCniBinPath == "" { diff --git a/pkg/consts/constants.go b/pkg/consts/constants.go index 09e20cd89..af217cb01 100644 --- a/pkg/consts/constants.go +++ b/pkg/consts/constants.go @@ -135,6 +135,9 @@ const ( // MetricsExporterFeatureGate: enable SriovNetworkMetricsExporter on the same node as where the config-daemon run MetricsExporterFeatureGate = "metricsExporter" + + // ManageSoftwareBridgesFeatureGate: enables management of software bridges by the operator + ManageSoftwareBridgesFeatureGate = "manageSoftwareBridges" ) const ( diff --git a/pkg/systemd/systemd.go b/pkg/systemd/systemd.go index f04f3e24e..6136a31b0 100644 --- a/pkg/systemd/systemd.go +++ b/pkg/systemd/systemd.go @@ -43,9 +43,10 @@ const ( // TODO: move this to the host interface also type SriovConfig struct { - Spec sriovnetworkv1.SriovNetworkNodeStateSpec `yaml:"spec"` - UnsupportedNics bool `yaml:"unsupportedNics"` - PlatformType consts.PlatformTypes `yaml:"platformType"` + Spec sriovnetworkv1.SriovNetworkNodeStateSpec `yaml:"spec"` + UnsupportedNics bool `yaml:"unsupportedNics"` + PlatformType consts.PlatformTypes `yaml:"platformType"` + ManageSoftwareBridges bool `yaml:"manageSoftwareBridges"` } type SriovResult struct { @@ -70,6 +71,7 @@ func WriteConfFile(newState *sriovnetworkv1.SriovNetworkNodeState) (bool, error) newState.Spec, vars.DevMode, vars.PlatformType, + vars.ManageSoftwareBridges, } _, err := os.Stat(utils.GetHostExtensionPath(SriovSystemdConfigPath)) diff --git a/pkg/vars/vars.go b/pkg/vars/vars.go index c4ff9b9cd..9321261dc 100644 --- a/pkg/vars/vars.go +++ b/pkg/vars/vars.go @@ -51,6 +51,9 @@ var ( // ParallelNicConfig global variable to perform NIC configuration in parallel ParallelNicConfig = false + // ManageSoftwareBridges global variable which reflects state of manageSoftwareBridges feature + ManageSoftwareBridges = false + // FilesystemRoot used by test to mock interactions with filesystem FilesystemRoot = "" From f606e87638ace23c3c4ffcdd33d69239bc9f1039 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Tue, 30 Apr 2024 11:46:01 +0300 Subject: [PATCH 2/4] Add logic to apply OVS bridge policy to state Signed-off-by: Yury Kulazhenkov --- api/v1/helper.go | 85 ++++++++- api/v1/helper_test.go | 412 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 495 insertions(+), 2 deletions(-) diff --git a/api/v1/helper.go b/api/v1/helper.go index 8c51d4530..bfdfbc473 100644 --- a/api/v1/helper.go +++ b/api/v1/helper.go @@ -6,7 +6,9 @@ import ( "fmt" "os" "path/filepath" + "reflect" "regexp" + "slices" "sort" "strconv" "strings" @@ -397,8 +399,7 @@ func UniqueAppend(inSlice []string, strings ...string) []string { // Apply policy to SriovNetworkNodeState CR func (p *SriovNetworkNodePolicy) Apply(state *SriovNetworkNodeState, equalPriority bool) error { s := p.Spec.NicSelector - if s.Vendor == "" && s.DeviceID == "" && len(s.RootDevices) == 0 && len(s.PfNames) == 0 && - len(s.NetFilter) == 0 { + if s.IsEmpty() { // Empty NicSelector match none return nil } @@ -438,6 +439,66 @@ func (p *SriovNetworkNodePolicy) Apply(state *SriovNetworkNodeState, equalPriori return nil } +// ApplyBridgeConfig applies bridge configuration from the policy to the provided state +func (p *SriovNetworkNodePolicy) ApplyBridgeConfig(state *SriovNetworkNodeState) error { + if p.Spec.NicSelector.IsEmpty() { + // Empty NicSelector match none + return nil + } + // sanity check the policy + if !p.Spec.Bridge.IsEmpty() { + if p.Spec.EswitchMode != ESwithModeSwitchDev { + return fmt.Errorf("eSwitchMode must be switchdev to use software bridge management") + } + if p.Spec.LinkType != "" && !strings.EqualFold(p.Spec.LinkType, consts.LinkTypeETH) { + return fmt.Errorf("linkType must be eth or ETH to use software bridge management") + } + if p.Spec.ExternallyManaged { + return fmt.Errorf("software bridge management can't be used when link is externally managed") + } + } + for _, iface := range state.Status.Interfaces { + if p.Spec.NicSelector.Selected(&iface) { + if p.Spec.Bridge.OVS == nil { + // The policy has no OVS bridge config, this means that the node's state should have no managed OVS bridges for the interfaces that match the policy. + // Currently PF to OVS bridge mapping is always 1 to 1 (bonding is not supported at the moment), meaning we can remove the OVS bridge + // config from the node's state if it has the interface (that matches "empty-bridge" policy) in the uplink section. + state.Spec.Bridges.OVS = slices.DeleteFunc(state.Spec.Bridges.OVS, func(br OVSConfigExt) bool { + return slices.ContainsFunc(br.Uplinks, func(uplink OVSUplinkConfigExt) bool { + return uplink.PciAddress == iface.PciAddress + }) + }) + if len(state.Spec.Bridges.OVS) == 0 { + state.Spec.Bridges.OVS = nil + } + continue + } + ovsBridge := OVSConfigExt{ + Name: GenerateBridgeName(&iface), + Bridge: p.Spec.Bridge.OVS.Bridge, + Uplinks: []OVSUplinkConfigExt{{ + PciAddress: iface.PciAddress, + Name: iface.Name, + Interface: p.Spec.Bridge.OVS.Uplink.Interface, + }}, + } + log.Info("Update bridge for interface", "name", iface.Name, "bridge", ovsBridge.Name) + + // We need to keep slices with bridges ordered to avoid unnecessary updates in the K8S API. + // Use binary search to insert (or update) the bridge config to the right place in the slice to keep it sorted. + pos, exist := slices.BinarySearchFunc(state.Spec.Bridges.OVS, ovsBridge, func(x, y OVSConfigExt) int { + return strings.Compare(x.Name, y.Name) + }) + if exist { + state.Spec.Bridges.OVS[pos] = ovsBridge + } else { + state.Spec.Bridges.OVS = slices.Insert(state.Spec.Bridges.OVS, pos, ovsBridge) + } + } + } + return nil +} + // mergeConfigs merges configs from multiple polices where the last one has the // highest priority. This merge is dependent on: 1. SR-IOV partition is // configured with the #-notation in pfName, 2. The VF groups are @@ -568,6 +629,15 @@ func ParseVfRange(device string) (rootDeviceName string, rngSt, rngEnd int, err return } +// IsEmpty returns true if nicSelector is empty +func (selector *SriovNetworkNicSelector) IsEmpty() bool { + return selector.Vendor == "" && + selector.DeviceID == "" && + len(selector.RootDevices) == 0 && + len(selector.PfNames) == 0 && + len(selector.NetFilter) == 0 +} + func (selector *SriovNetworkNicSelector) Selected(iface *InterfaceExt) bool { if selector.Vendor != "" && selector.Vendor != iface.Vendor { return false @@ -923,3 +993,14 @@ func (s *SriovNetworkPoolConfig) MaxUnavailable(numOfNodes int) (int, error) { return maxunavail, nil } + +// GenerateBridgeName generate predictable name for the software bridge +// current format is: br-0000_00_03.0 +func GenerateBridgeName(iface *InterfaceExt) string { + return fmt.Sprintf("br-%s", strings.ReplaceAll(iface.PciAddress, ":", "_")) +} + +// NeedToUpdateBridges returns true if bridge for the host requires update +func NeedToUpdateBridges(bridgeSpec, bridgeStatus *Bridges) bool { + return !reflect.DeepEqual(bridgeSpec, bridgeStatus) +} diff --git a/api/v1/helper_test.go b/api/v1/helper_test.go index 4c899bcc9..06215d067 100644 --- a/api/v1/helper_test.go +++ b/api/v1/helper_test.go @@ -1122,3 +1122,415 @@ func TestNeedToUpdateSriov(t *testing.T) { }) } } + +func TestSriovNetworkNodePolicyApplyBridgeConfig(t *testing.T) { + testtable := []struct { + tname string + currentState *v1.SriovNetworkNodeState + policy *v1.SriovNetworkNodePolicy + expectedBridges v1.Bridges + expectedErr bool + }{ + { + tname: "no selectors", + currentState: newNodeState(), + policy: &v1.SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: v1.SriovNetworkNodePolicySpec{ + DeviceType: consts.DeviceTypeNetDevice, + NicSelector: v1.SriovNetworkNicSelector{}, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 2, + Priority: 99, + EswitchMode: "switchdev", + ResourceName: "p1res", + Bridge: v1.Bridge{OVS: &v1.OVSConfig{}}, + }, + }, + expectedBridges: v1.Bridges{}, + }, + { + tname: "not switchdev config", + currentState: newNodeState(), + policy: &v1.SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: v1.SriovNetworkNodePolicySpec{ + DeviceType: consts.DeviceTypeNetDevice, + NicSelector: v1.SriovNetworkNicSelector{ + RootDevices: []string{"0000:86:00.0"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 2, + Priority: 99, + EswitchMode: "legacy", + ResourceName: "p1res", + Bridge: v1.Bridge{OVS: &v1.OVSConfig{}}, + }, + }, + expectedBridges: v1.Bridges{}, + expectedErr: true, + }, + { + tname: "bad linkType", + currentState: newNodeState(), + policy: &v1.SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: v1.SriovNetworkNodePolicySpec{ + DeviceType: consts.DeviceTypeNetDevice, + NicSelector: v1.SriovNetworkNicSelector{ + RootDevices: []string{"0000:86:00.0"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 2, + Priority: 99, + EswitchMode: "switchdev", + LinkType: "ib", + ResourceName: "p1res", + Bridge: v1.Bridge{OVS: &v1.OVSConfig{}}, + }, + }, + expectedBridges: v1.Bridges{}, + expectedErr: true, + }, + { + tname: "externally managed", + currentState: newNodeState(), + policy: &v1.SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: v1.SriovNetworkNodePolicySpec{ + DeviceType: consts.DeviceTypeNetDevice, + NicSelector: v1.SriovNetworkNicSelector{ + RootDevices: []string{"0000:86:00.0"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 2, + Priority: 99, + EswitchMode: "switchdev", + ExternallyManaged: true, + ResourceName: "p1res", + Bridge: v1.Bridge{OVS: &v1.OVSConfig{}}, + }, + }, + expectedBridges: v1.Bridges{}, + expectedErr: true, + }, + { + tname: "single policy multi match", + currentState: newNodeState(), + policy: &v1.SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: v1.SriovNetworkNodePolicySpec{ + DeviceType: consts.DeviceTypeNetDevice, + NicSelector: v1.SriovNetworkNicSelector{ + RootDevices: []string{"0000:86:00.0", "0000:86:00.2"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 2, + Priority: 99, + EswitchMode: "switchdev", + ResourceName: "p1res", + Bridge: v1.Bridge{OVS: &v1.OVSConfig{ + Bridge: v1.OVSBridgeConfig{DatapathType: "test"}, + Uplink: v1.OVSUplinkConfig{ + Interface: v1.OVSInterfaceConfig{ + Type: "test", + }}, + }}, + }, + }, + expectedBridges: v1.Bridges{OVS: []v1.OVSConfigExt{ + { + Name: "br-0000_86_00.0", + Bridge: v1.OVSBridgeConfig{DatapathType: "test"}, + Uplinks: []v1.OVSUplinkConfigExt{{ + Name: "ens803f0", + PciAddress: "0000:86:00.0", + Interface: v1.OVSInterfaceConfig{Type: "test"}, + }}, + }, + { + Name: "br-0000_86_00.2", + Bridge: v1.OVSBridgeConfig{DatapathType: "test"}, + Uplinks: []v1.OVSUplinkConfigExt{{ + Name: "ens803f2", + PciAddress: "0000:86:00.2", + Interface: v1.OVSInterfaceConfig{Type: "test"}, + }}, + }, + }}, + }, + { + tname: "update bridge set by policy with lover priority", + currentState: &v1.SriovNetworkNodeState{ + Spec: v1.SriovNetworkNodeStateSpec{ + Bridges: v1.Bridges{OVS: []v1.OVSConfigExt{ + { + Name: "br-0000_86_00.0", + Bridge: v1.OVSBridgeConfig{DatapathType: "foo"}, + Uplinks: []v1.OVSUplinkConfigExt{{ + Name: "ens803f0", + PciAddress: "0000:86:00.0", + Interface: v1.OVSInterfaceConfig{Type: "test"}, + }}, + }, + }}, + }, + Status: v1.SriovNetworkNodeStateStatus{ + Interfaces: []v1.InterfaceExt{ + { + VFs: []v1.VirtualFunction{ + {}, + }, + DeviceID: "158b", + Driver: "i40e", + Mtu: 1500, + Name: "ens803f0", + PciAddress: "0000:86:00.0", + Vendor: "8086", + NumVfs: 4, + TotalVfs: 64, + }, + }, + }}, + policy: &v1.SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: v1.SriovNetworkNodePolicySpec{ + DeviceType: consts.DeviceTypeNetDevice, + NicSelector: v1.SriovNetworkNicSelector{ + RootDevices: []string{"0000:86:00.0"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 2, + Priority: 99, + EswitchMode: "switchdev", + ResourceName: "p1res", + Bridge: v1.Bridge{OVS: &v1.OVSConfig{ + Bridge: v1.OVSBridgeConfig{DatapathType: "test"}, + Uplink: v1.OVSUplinkConfig{ + Interface: v1.OVSInterfaceConfig{ + Type: "test", + }}, + }}, + }, + }, + expectedBridges: v1.Bridges{OVS: []v1.OVSConfigExt{ + { + Name: "br-0000_86_00.0", + Bridge: v1.OVSBridgeConfig{DatapathType: "test"}, + Uplinks: []v1.OVSUplinkConfigExt{{ + Name: "ens803f0", + PciAddress: "0000:86:00.0", + Interface: v1.OVSInterfaceConfig{Type: "test"}, + }}, + }, + }}, + }, + { + tname: "remove bridge set by policy with lover priority", + currentState: &v1.SriovNetworkNodeState{ + Spec: v1.SriovNetworkNodeStateSpec{ + Bridges: v1.Bridges{OVS: []v1.OVSConfigExt{ + { + Name: "br-0000_86_00.0", + Bridge: v1.OVSBridgeConfig{DatapathType: "foo"}, + Uplinks: []v1.OVSUplinkConfigExt{{ + Name: "ens803f0", + PciAddress: "0000:86:00.0", + Interface: v1.OVSInterfaceConfig{Type: "test"}, + }}, + }, + }}, + }, + Status: v1.SriovNetworkNodeStateStatus{ + Interfaces: []v1.InterfaceExt{ + { + VFs: []v1.VirtualFunction{ + {}, + }, + DeviceID: "158b", + Driver: "i40e", + Mtu: 1500, + Name: "ens803f0", + PciAddress: "0000:86:00.0", + Vendor: "8086", + NumVfs: 4, + TotalVfs: 64, + }, + }, + }}, + policy: &v1.SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: v1.SriovNetworkNodePolicySpec{ + DeviceType: consts.DeviceTypeNetDevice, + NicSelector: v1.SriovNetworkNicSelector{ + RootDevices: []string{"0000:86:00.0"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 2, + Priority: 99, + EswitchMode: "switchdev", + ResourceName: "p1res", + }, + }, + expectedBridges: v1.Bridges{}, + }, + { + tname: "keep bridge set by other policy", + currentState: &v1.SriovNetworkNodeState{ + Spec: v1.SriovNetworkNodeStateSpec{ + Bridges: v1.Bridges{OVS: []v1.OVSConfigExt{ + { + Name: "br-0000_86_00.2", + Bridge: v1.OVSBridgeConfig{DatapathType: "foo"}, + Uplinks: []v1.OVSUplinkConfigExt{{ + Name: "ens803f2", + PciAddress: "0000:86:00.2", + Interface: v1.OVSInterfaceConfig{Type: "bar"}, + }}, + }, + }, + }}, + Status: v1.SriovNetworkNodeStateStatus{ + Interfaces: []v1.InterfaceExt{ + { + VFs: []v1.VirtualFunction{ + {}, + }, + DeviceID: "158b", + Driver: "i40e", + Mtu: 1500, + Name: "ens803f0", + PciAddress: "0000:86:00.0", + Vendor: "8086", + NumVfs: 4, + TotalVfs: 64, + }, + }, + }}, + policy: &v1.SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: v1.SriovNetworkNodePolicySpec{ + DeviceType: consts.DeviceTypeNetDevice, + NicSelector: v1.SriovNetworkNicSelector{ + RootDevices: []string{"0000:86:00.0"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 2, + Priority: 99, + EswitchMode: "switchdev", + ResourceName: "p1res", + Bridge: v1.Bridge{OVS: &v1.OVSConfig{ + Bridge: v1.OVSBridgeConfig{DatapathType: "test"}, + Uplink: v1.OVSUplinkConfig{ + Interface: v1.OVSInterfaceConfig{ + Type: "test", + }}, + }}, + }, + }, + expectedBridges: v1.Bridges{OVS: []v1.OVSConfigExt{ + { + Name: "br-0000_86_00.0", + Bridge: v1.OVSBridgeConfig{DatapathType: "test"}, + Uplinks: []v1.OVSUplinkConfigExt{{ + Name: "ens803f0", + PciAddress: "0000:86:00.0", + Interface: v1.OVSInterfaceConfig{Type: "test"}, + }}, + }, + { + Name: "br-0000_86_00.2", + Bridge: v1.OVSBridgeConfig{DatapathType: "foo"}, + Uplinks: []v1.OVSUplinkConfigExt{{ + Name: "ens803f2", + PciAddress: "0000:86:00.2", + Interface: v1.OVSInterfaceConfig{Type: "bar"}, + }}, + }, + }}, + }, + } + for _, tc := range testtable { + t.Run(tc.tname, func(t *testing.T) { + err := tc.policy.ApplyBridgeConfig(tc.currentState) + if tc.expectedErr && err == nil { + t.Errorf("ApplyBridgeConfig expecting error.") + } else if !tc.expectedErr && err != nil { + t.Errorf("ApplyBridgeConfig error:\n%s", err) + } + if diff := cmp.Diff(tc.expectedBridges, tc.currentState.Spec.Bridges); diff != "" { + t.Errorf("SriovNetworkNodeState spec diff (-want +got):\n%s", diff) + } + }) + } +} + +func TestGenerateBridgeName(t *testing.T) { + result := v1.GenerateBridgeName(&v1.InterfaceExt{PciAddress: "0000:86:00.2"}) + expected := "br-0000_86_00.2" + if result != expected { + t.Errorf("GenerateBridgeName unexpected result, expected: %s, actual: %s", expected, result) + } +} + +func TestNeedToUpdateBridges(t *testing.T) { + testtable := []struct { + tname string + specBridge *v1.Bridges + statusBridge *v1.Bridges + expectedResult bool + }{ + { + tname: "no update required", + specBridge: &v1.Bridges{OVS: []v1.OVSConfigExt{{Bridge: v1.OVSBridgeConfig{DatapathType: "test"}}}}, + statusBridge: &v1.Bridges{OVS: []v1.OVSConfigExt{{Bridge: v1.OVSBridgeConfig{DatapathType: "test"}}}}, + expectedResult: false, + }, + { + tname: "update required", + specBridge: &v1.Bridges{OVS: []v1.OVSConfigExt{{Bridge: v1.OVSBridgeConfig{DatapathType: "test"}}}}, + statusBridge: &v1.Bridges{OVS: []v1.OVSConfigExt{}}, + expectedResult: true, + }, + } + for _, tc := range testtable { + t.Run(tc.tname, func(t *testing.T) { + result := v1.NeedToUpdateBridges(tc.specBridge, tc.statusBridge) + if result != tc.expectedResult { + t.Errorf("unexpected result want: %t got: %t", tc.expectedResult, result) + } + }) + } +} From 68afdfed4019150db8c4302593bfbe4a0131edba Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Fri, 5 Apr 2024 15:16:16 +0300 Subject: [PATCH 3/4] Add validation for bridge config to webhook Signed-off-by: Yury Kulazhenkov --- pkg/webhook/validate.go | 14 ++++++++ pkg/webhook/validate_test.go | 64 ++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 95c481470..e88136fb3 100644 --- a/pkg/webhook/validate.go +++ b/pkg/webhook/validate.go @@ -213,6 +213,12 @@ func staticValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePol if cr.Spec.DeviceType == consts.DeviceTypeVfioPci && cr.Spec.IsRdma { return false, fmt.Errorf("'deviceType: vfio-pci' conflicts with 'isRdma: true'; Set 'deviceType' to (string)'netdevice' Or Set 'isRdma' to (bool)'false'") } + + // switchdev mode can be used only with ethernet links + if cr.Spec.LinkType != "" && !strings.EqualFold(cr.Spec.LinkType, consts.LinkTypeETH) && cr.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { + return false, fmt.Errorf("'eSwitchMode: switchdev' can be used only with ethernet links") + } + // vdpa: deviceType must be set to 'netdevice' if cr.Spec.DeviceType != consts.DeviceTypeNetDevice && (cr.Spec.VdpaType == consts.VdpaTypeVirtio || cr.Spec.VdpaType == consts.VdpaTypeVhost) { return false, fmt.Errorf("'deviceType: %s' conflicts with '%s'; Set 'deviceType' to (string)'netdevice' Or Remove 'vdpaType'", cr.Spec.DeviceType, cr.Spec.VdpaType) @@ -221,6 +227,14 @@ func staticValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePol if (cr.Spec.VdpaType == consts.VdpaTypeVirtio || cr.Spec.VdpaType == consts.VdpaTypeVhost) && cr.Spec.EswitchMode != sriovnetworkv1.ESwithModeSwitchDev { return false, fmt.Errorf("vdpa requires the device to be configured in switchdev mode") } + // software bridge management: device must be configured in switchdev mode + if !cr.Spec.Bridge.IsEmpty() && cr.Spec.EswitchMode != sriovnetworkv1.ESwithModeSwitchDev { + return false, fmt.Errorf("software bridge management requires the device to be configured in switchdev mode") + } + // software bridge management: device can't be externally managed + if !cr.Spec.Bridge.IsEmpty() && cr.Spec.ExternallyManaged { + return false, fmt.Errorf("software bridge management can't be used when the device externally managed") + } return true, nil } diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go index 259aedbce..1b3cb4ba7 100644 --- a/pkg/webhook/validate_test.go +++ b/pkg/webhook/validate_test.go @@ -1107,6 +1107,70 @@ func TestStaticValidateSriovNetworkNodePolicyWithInvalidNicSelector(t *testing.T g.Expect(ok).To(Equal(false)) } +func TestStaticValidateSriovNetworkNodePolicyWithInvalidLinkTypeForSwitchdev(t *testing.T) { + policy := &SriovNetworkNodePolicy{ + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + LinkType: "ib", + IsRdma: true, + EswitchMode: "switchdev", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + ResourceName: "p0", + }, + } + g := NewGomegaWithT(t) + ok, err := staticValidateSriovNetworkNodePolicy(policy) + g.Expect(err).To(HaveOccurred()) + g.Expect(ok).To(Equal(false)) +} + +func TestStaticValidateSriovNetworkNodePolicyWithBridgeConfigWithoutSwitchdev(t *testing.T) { + policy := &SriovNetworkNodePolicy{ + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + Bridge: Bridge{OVS: &OVSConfig{}}, + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + ResourceName: "p0", + }, + } + g := NewGomegaWithT(t) + ok, err := staticValidateSriovNetworkNodePolicy(policy) + g.Expect(err).To(HaveOccurred()) + g.Expect(ok).To(Equal(false)) +} + +func TestStaticValidateSriovNetworkNodePolicyWithBridgeConfigWithExternallyManaged(t *testing.T) { + policy := &SriovNetworkNodePolicy{ + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + Bridge: Bridge{OVS: &OVSConfig{}}, + EswitchMode: "switchdev", + ExternallyManaged: true, + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1"}, + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + ResourceName: "p0", + }, + } + g := NewGomegaWithT(t) + ok, err := staticValidateSriovNetworkNodePolicy(policy) + g.Expect(err).To(HaveOccurred()) + g.Expect(ok).To(Equal(false)) +} + func TestValidatePolicyForNodeStateWithValidNetFilter(t *testing.T) { interfaceSelected = false state := newNodeState() From f10b9a6a01d07e16d87d6ea9a6008443a2d5ba0f Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Wed, 3 Apr 2024 15:27:22 +0300 Subject: [PATCH 4/4] Add logic to SriovNetworkNodePolicy controller to populate bridge config Signed-off-by: Yury Kulazhenkov --- controllers/sriovnetworknodepolicy_controller.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/controllers/sriovnetworknodepolicy_controller.go b/controllers/sriovnetworknodepolicy_controller.go index e5abc5f08..be46880b7 100644 --- a/controllers/sriovnetworknodepolicy_controller.go +++ b/controllers/sriovnetworknodepolicy_controller.go @@ -364,6 +364,12 @@ func (r *SriovNetworkNodePolicyReconciler) syncSriovNetworkNodeState(ctx context if err != nil { return err } + if r.FeatureGate.IsEnabled(constants.ManageSoftwareBridgesFeatureGate) { + err = p.ApplyBridgeConfig(newVersion) + if err != nil { + return err + } + } // record the evaluated policy priority for next loop ppp = p.Spec.Priority }