Merge pull request #675 from ykulazhenkov/pr-allow-externally-managed…

…-switchdev Allow switchdev+externallyManged configs in webhook
k8snetworkplumbingwg · Apr 24, 2024 · e741850 · e741850
2 parents b7ec5ae + e3ac636
commit e741850
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 97 deletions.
diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go
@@ -225,13 +225,6 @@ 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")
 	}
-
-	// Externally created: we don't support  ExternallyManaged + EswitchMode
-	//TODO: if needed we will need to add this in the future as today EswitchMode is for HWOFFLOAD
-	if cr.Spec.ExternallyManaged && cr.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev {
-		return false, fmt.Errorf("ExternallyManaged doesn't support the device to be configured in switchdev mode")
-	}
-
 	return true, nil
 }
 
@@ -426,16 +419,6 @@ func validateExternallyManage(current, previous *sriovnetworkv1.SriovNetworkNode
 		return fmt.Errorf("externallyManage is inconsistent with existing policy %s", previous.GetName())
 	}
 
-	// reject policy with externallyManage if there is a policy on the same PF with switch dev
-	if current.Spec.ExternallyManaged && previous.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev {
-		return fmt.Errorf("externallyManage overlap with switchdev mode in existing policy %s", previous.GetName())
-	}
-
-	// reject policy with externallyManage if there is a policy on the same PF with switch dev
-	if previous.Spec.ExternallyManaged && current.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev {
-		return fmt.Errorf("switchdev overlap with externallyManage mode in existing policy %s", previous.GetName())
-	}
-
 	return nil
 }
 

diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go
@@ -472,62 +472,6 @@ func TestValidatePolicyForNodePolicyWithExternallyManageConflict(t *testing.T) {
 	g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("externallyManage is inconsistent with existing policy %s", appliedPolicy.ObjectMeta.Name))))
 }
 
-func TestValidatePolicyForNodePolicyWithExternallyManageConflictWithSwitchDev(t *testing.T) {
-	appliedPolicy := newNodePolicy()
-	appliedPolicy.Spec.EswitchMode = ESwithModeSwitchDev
-
-	policy := &SriovNetworkNodePolicy{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "p0",
-		},
-		Spec: SriovNetworkNodePolicySpec{
-			DeviceType: "netdevice",
-			NicSelector: SriovNetworkNicSelector{
-				PfNames: []string{"ens803f1#3-4"},
-				Vendor:  "8086",
-			},
-			NodeSelector: map[string]string{
-				"feature.node.kubernetes.io/network-sriov.capable": "true",
-			},
-			NumVfs:            63,
-			Priority:          99,
-			ResourceName:      "p0",
-			ExternallyManaged: true,
-		},
-	}
-	g := NewGomegaWithT(t)
-	err := validatePolicyForNodePolicy(policy, appliedPolicy)
-	g.Expect(err).To(HaveOccurred())
-}
-
-func TestValidatePolicyForNodePolicyWithSwitchDevConflictWithExternallyManage(t *testing.T) {
-	appliedPolicy := newNodePolicy()
-	appliedPolicy.Spec.ExternallyManaged = true
-
-	policy := &SriovNetworkNodePolicy{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "p0",
-		},
-		Spec: SriovNetworkNodePolicySpec{
-			DeviceType: "netdevice",
-			NicSelector: SriovNetworkNicSelector{
-				PfNames: []string{"ens803f1#3-4"},
-				Vendor:  "8086",
-			},
-			NodeSelector: map[string]string{
-				"feature.node.kubernetes.io/network-sriov.capable": "true",
-			},
-			NumVfs:       63,
-			Priority:     99,
-			ResourceName: "p0",
-			EswitchMode:  ESwithModeSwitchDev,
-		},
-	}
-	g := NewGomegaWithT(t)
-	err := validatePolicyForNodePolicy(policy, appliedPolicy)
-	g.Expect(err).To(HaveOccurred())
-}
-
 func TestValidatePolicyForNodeStateWithExternallyManageAndMTU(t *testing.T) {
 	state := newNodeState()
 	policy := &SriovNetworkNodePolicy{
@@ -1024,30 +968,6 @@ func TestStaticValidateSriovNetworkNodePolicyVhostVdpaMustSpecifySwitchDev(t *te
 	g.Expect(ok).To(Equal(false))
 }
 
-func TestStaticValidateSriovNetworkNodePolicyWithExternallyCreatedAndSwitchDev(t *testing.T) {
-	policy := &SriovNetworkNodePolicy{
-		Spec: SriovNetworkNodePolicySpec{
-			DeviceType: "netdevice",
-			NicSelector: SriovNetworkNicSelector{
-				Vendor:   "8086",
-				DeviceID: "158b",
-			},
-			NodeSelector: map[string]string{
-				"feature.node.kubernetes.io/network-sriov.capable": "true",
-			},
-			NumVfs:            63,
-			Priority:          99,
-			ResourceName:      "p0",
-			EswitchMode:       "switchdev",
-			ExternallyManaged: true,
-		},
-	}
-	g := NewGomegaWithT(t)
-	ok, err := staticValidateSriovNetworkNodePolicy(policy)
-	g.Expect(err).To(HaveOccurred())
-	g.Expect(ok).To(BeFalse())
-}
-
 func TestValidatePolicyForNodeStateVirtioVdpaWithNotSupportedVendor(t *testing.T) {
 	state := newNodeState()
 	policy := &SriovNetworkNodePolicy{
@@ -1273,3 +1193,80 @@ func TestValidatePolicyForNodeStateWithValidVFAndNetFilter(t *testing.T) {
 	g.Expect(err).NotTo(HaveOccurred())
 	g.Expect(interfaceSelected).To(Equal(true))
 }
+
+func TestValidatePolicyForNodeStateWithExternallyManageAndSwitchdev(t *testing.T) {
+	state := newNodeState()
+	policy := &SriovNetworkNodePolicy{
+		Spec: SriovNetworkNodePolicySpec{
+			DeviceType: "netdevice",
+			NicSelector: SriovNetworkNicSelector{
+				PfNames:     []string{"ens803f0"},
+				RootDevices: []string{"0000:86:00.0"},
+				Vendor:      "8086",
+			},
+			NodeSelector: map[string]string{
+				"feature.node.kubernetes.io/network-sriov.capable": "true",
+			},
+			NumVfs:            4,
+			ResourceName:      "p0",
+			EswitchMode:       "switchdev",
+			ExternallyManaged: true,
+		},
+	}
+	g := NewGomegaWithT(t)
+	_, err := validatePolicyForNodeState(policy, state, NewNode())
+	g.Expect(err).NotTo(HaveOccurred())
+}
+
+func TestValidatePolicyForNodeStateWithExternallyManageAndSwitchdevAndWrongVFCount(t *testing.T) {
+	state := newNodeState()
+	policy := &SriovNetworkNodePolicy{
+		Spec: SriovNetworkNodePolicySpec{
+			DeviceType: "netdevice",
+			NicSelector: SriovNetworkNicSelector{
+				PfNames:     []string{"ens803f0"},
+				RootDevices: []string{"0000:86:00.0"},
+				Vendor:      "8086",
+			},
+			NodeSelector: map[string]string{
+				"feature.node.kubernetes.io/network-sriov.capable": "true",
+			},
+			NumVfs:            30,
+			ResourceName:      "p0",
+			EswitchMode:       "switchdev",
+			ExternallyManaged: true,
+		},
+	}
+	g := NewGomegaWithT(t)
+	_, err := validatePolicyForNodeState(policy, state, NewNode())
+	g.Expect(err).To(MatchError(ContainSubstring("is higher than the virtual functions allocated for the PF externally value")))
+}
+
+func TestValidatePolicyForNodePolicyAllowSwitchdevWithExternallyManage(t *testing.T) {
+	appliedPolicy := newNodePolicy()
+	appliedPolicy.Spec.ExternallyManaged = true
+
+	policy := &SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "p0",
+		},
+		Spec: SriovNetworkNodePolicySpec{
+			DeviceType: "netdevice",
+			NicSelector: SriovNetworkNicSelector{
+				PfNames: []string{"ens803f1#3-4"},
+				Vendor:  "8086",
+			},
+			NodeSelector: map[string]string{
+				"feature.node.kubernetes.io/network-sriov.capable": "true",
+			},
+			NumVfs:            63,
+			Priority:          99,
+			ResourceName:      "p0",
+			ExternallyManaged: true,
+			EswitchMode:       ESwithModeSwitchDev,
+		},
+	}
+	g := NewGomegaWithT(t)
+	err := validatePolicyForNodePolicy(policy, appliedPolicy)
+	g.Expect(err).NotTo(HaveOccurred())
+}