From 9ea820d216c89524a9a30b8a3894d9e27bfcd446 Mon Sep 17 00:00:00 2001 From: Marcelo Guerrero Date: Tue, 7 Nov 2023 10:18:34 +0100 Subject: [PATCH 1/3] Skip vf configuration when not found in VF group Signed-off-by: Marcelo Guerrero --- pkg/utils/utils.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 051a4aebb..cfc8e02ff 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -462,12 +462,14 @@ func configSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetwor i := 0 var dpdkDriver string var isRdma bool + found := false 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) { + found = true isRdma = group.IsRdma if sriovnetworkv1.StringInArray(group.DeviceType, DpdkDrivers) { dpdkDriver = group.DeviceType @@ -476,6 +478,11 @@ func configSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetwor } } + // Do not configure VF which is not part of any VF Group. + if !found { + 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 From 7390d137bd07c9a00565b70aba9798f2a6ee53f0 Mon Sep 17 00:00:00 2001 From: Marcelo Guerrero Date: Wed, 8 Nov 2023 09:54:24 +0100 Subject: [PATCH 2/3] Refactor vfGroup lookup in configSriovDevice Signed-off-by: Marcelo Guerrero --- pkg/utils/utils.go | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index cfc8e02ff..aacae82e9 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -458,28 +458,23 @@ func configSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetwor } for _, addr := range vfAddrs { - var group sriovnetworkv1.VfGroup - i := 0 - var dpdkDriver string - var isRdma bool - found := false + 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) { - found = true - 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 } } - // Do not configure VF which is not part of any VF Group. - if !found { + // VF group not found. + if group == nil { continue } @@ -521,26 +516,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 } } From 4dc27ea25366d12a30c1f8287ec18ebe721f88bc Mon Sep 17 00:00:00 2001 From: Marcelo Guerrero Date: Wed, 8 Nov 2023 11:49:58 +0100 Subject: [PATCH 3/3] Verify policy is only applied to partions' VFs --- test/conformance/tests/test_sriov_operator.go | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/test/conformance/tests/test_sriov_operator.go b/test/conformance/tests/test_sriov_operator.go index a206bf4d9..b5aa69ca8 100644 --- a/test/conformance/tests/test_sriov_operator.go +++ b/test/conformance/tests/test_sriov_operator.go @@ -1071,6 +1071,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]