Merge pull request #539 from mlguerrero12/fixconfigvfgroup

Skip vf configuration when not found in VF group
k8snetworkplumbingwg · Nov 23, 2023 · c54ee95 · c54ee95
2 parents 80f2015 + 4dc27ea
commit c54ee95
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 19 deletions.
diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -464,24 +464,26 @@ func configSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetwor
 		}
 
 		for _, addr := range vfAddrs {
-			var group sriovnetworkv1.VfGroup
-			i := 0
-			var dpdkDriver string
-			var isRdma bool
+			var group *sriovnetworkv1.VfGroup
+
 			vfID, err := dputils.GetVFID(addr)
-			for i, group = range iface.VfGroups {
-				if err != nil {
-					log.Log.Error(err, "configSriovDevice(): unable to get VF id", "device", iface.PciAddress)
-				}
-				if sriovnetworkv1.IndexInRange(vfID, group.VfRange) {
-					isRdma = group.IsRdma
-					if sriovnetworkv1.StringInArray(group.DeviceType, DpdkDrivers) {
-						dpdkDriver = group.DeviceType
-					}
+			if err != nil {
+				log.Log.Error(err, "configSriovDevice(): unable to get VF id", "device", iface.PciAddress)
+				return err
+			}
+
+			for i := range iface.VfGroups {
+				if sriovnetworkv1.IndexInRange(vfID, iface.VfGroups[i].VfRange) {
+					group = &iface.VfGroups[i]
 					break
 				}
 			}
 
+			// VF group not found.
+			if group == nil {
+				continue
+			}
+
 			// only set GUID and MAC for VF with default driver
 			// for userspace drivers like vfio we configure the vf mac using the kernel nic mac address
 			// before we switch to the userspace driver
@@ -520,26 +522,26 @@ func configSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetwor
 				}
 			}
 
-			if err = unbindDriverIfNeeded(addr, isRdma); err != nil {
+			if err = unbindDriverIfNeeded(addr, group.IsRdma); err != nil {
 				return err
 			}
 
-			if dpdkDriver == "" {
+			if !sriovnetworkv1.StringInArray(group.DeviceType, DpdkDrivers) {
 				if err := BindDefaultDriver(addr); err != nil {
 					log.Log.Error(err, "configSriovDevice(): fail to bind default driver for device", "device", addr)
 					return err
 				}
 				// only set MTU for VF with default driver
-				if iface.VfGroups[i].Mtu > 0 {
-					if err := setNetdevMTU(addr, iface.VfGroups[i].Mtu); err != nil {
+				if group.Mtu > 0 {
+					if err := setNetdevMTU(addr, group.Mtu); err != nil {
 						log.Log.Error(err, "configSriovDevice(): fail to set mtu for VF", "address", addr)
 						return err
 					}
 				}
 			} else {
-				if err := BindDpdkDriver(addr, dpdkDriver); err != nil {
+				if err := BindDpdkDriver(addr, group.DeviceType); err != nil {
 					log.Log.Error(err, "configSriovDevice(): fail to bind driver for device",
-						"driver", dpdkDriver, "device", addr)
+						"driver", group.DeviceType, "device", addr)
 					return err
 				}
 			}

diff --git a/test/conformance/tests/test_sriov_operator.go b/test/conformance/tests/test_sriov_operator.go
@@ -1118,6 +1118,83 @@ var _ = Describe("[sriov] operator", func() {
 					}))
 				})
 
+				It("Should configure the mtu only for vfs which are part of the partition", func() {
+					defaultMtu := 1500
+					newMtu := 2000
+
+					node := sriovInfos.Nodes[0]
+					intf, err := sriovInfos.FindOneSriovDevice(node)
+					Expect(err).ToNot(HaveOccurred())
+
+					_, err = network.CreateSriovPolicy(clients, "test-policy-", operatorNamespace, intf.Name+"#0-1", node, 5, testResourceName, "netdevice", func(policy *sriovv1.SriovNetworkNodePolicy) {
+						policy.Spec.Mtu = newMtu
+					})
+					Expect(err).ToNot(HaveOccurred())
+
+					Eventually(func() sriovv1.Interfaces {
+						nodeState, err := clients.SriovNetworkNodeStates(operatorNamespace).Get(context.Background(), node, metav1.GetOptions{})
+						Expect(err).ToNot(HaveOccurred())
+						return nodeState.Spec.Interfaces
+					}, 3*time.Minute, 1*time.Second).Should(ContainElement(MatchFields(
+						IgnoreExtras,
+						Fields{
+							"Name":   Equal(intf.Name),
+							"NumVfs": Equal(5),
+							"Mtu":    Equal(newMtu),
+							"VfGroups": ContainElement(
+								MatchFields(
+									IgnoreExtras,
+									Fields{
+										"ResourceName": Equal(testResourceName),
+										"DeviceType":   Equal("netdevice"),
+										"VfRange":      Equal("0-1"),
+									})),
+						})))
+
+					WaitForSRIOVStable()
+
+					Eventually(func() int64 {
+						testedNode, err := clients.CoreV1Interface.Nodes().Get(context.Background(), node, metav1.GetOptions{})
+						Expect(err).ToNot(HaveOccurred())
+						resNum := testedNode.Status.Allocatable["openshift.io/testresource"]
+						capacity, _ := resNum.AsInt64()
+						return capacity
+					}, 3*time.Minute, time.Second).Should(Equal(int64(2)))
+
+					By(fmt.Sprintf("verifying that only VF 0 and 1 have mtu set to %d", newMtu))
+					Eventually(func() sriovv1.InterfaceExts {
+						nodeState, err := clients.SriovNetworkNodeStates(operatorNamespace).Get(context.Background(), node, metav1.GetOptions{})
+						Expect(err).ToNot(HaveOccurred())
+						return nodeState.Status.Interfaces
+					}, 3*time.Minute, 1*time.Second).Should(ContainElement(MatchFields(
+						IgnoreExtras,
+						Fields{
+							"VFs": SatisfyAll(
+								ContainElement(
+									MatchFields(
+										IgnoreExtras,
+										Fields{
+											"VfID": Equal(0),
+											"Mtu":  Equal(newMtu),
+										})),
+								ContainElement(
+									MatchFields(
+										IgnoreExtras,
+										Fields{
+											"VfID": Equal(1),
+											"Mtu":  Equal(newMtu),
+										})),
+								ContainElement(
+									MatchFields(
+										IgnoreExtras,
+										Fields{
+											"VfID": Equal(2),
+											"Mtu":  Equal(defaultMtu),
+										})),
+							),
+						})))
+				})
+
 				// 27630
 				It("Should not be possible to have overlapping pf ranges", func() {
 					node := sriovInfos.Nodes[0]