From 9e5c80252830017b6c2f9ee0f8c0ca45da8dcb99 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Thu, 11 Apr 2024 15:21:47 +0300 Subject: [PATCH] block overlap policies using the same rootDevice Signed-off-by: Sebastian Sch --- api/v1/helper.go | 30 ++++++++---- pkg/webhook/validate.go | 58 +++++++++++++++++------- pkg/webhook/validate_test.go | 25 ++++++++++ test/e2e/sriovoperatornodepolicy_test.go | 2 +- 4 files changed, 89 insertions(+), 26 deletions(-) diff --git a/api/v1/helper.go b/api/v1/helper.go index 462afa356..24ed05eb8 100644 --- a/api/v1/helper.go +++ b/api/v1/helper.go @@ -475,13 +475,13 @@ func (gr VfGroup) isVFRangeOverlapping(group VfGroup) bool { return IndexInRange(rngSt, group.VfRange) || IndexInRange(rngEnd, group.VfRange) } -func (p *SriovNetworkNodePolicy) generateVfGroup(iface *InterfaceExt) (*VfGroup, error) { +func (p *SriovNetworkNodePolicy) generatePfNameVfGroup(iface *InterfaceExt) (*VfGroup, error) { var err error pfName := "" var rngStart, rngEnd int found := false for _, selector := range p.Spec.NicSelector.PfNames { - pfName, rngStart, rngEnd, err = ParsePFName(selector) + pfName, rngStart, rngEnd, err = ParseVfRange(selector) if err != nil { log.Error(err, "Unable to parse PF Name.") return nil, err @@ -534,15 +534,27 @@ func parseRange(r string) (rngSt, rngEnd int, err error) { return } -// Parse PF name with VF range -func ParsePFName(name string) (ifName string, rngSt, rngEnd int, err error) { +// SplitDeviceFromRange return the device name and the range. +// the split is base on # +func SplitDeviceFromRange(device string) (string, string) { + if strings.Contains(device, "#") { + fields := strings.Split(device, "#") + return fields[0], fields[1] + } + + return device, "" +} + +// ParseVfRange: parse a device with VF range +// this can be rootDevices or PFName +// if no range detect we just return the device name +func ParseVfRange(device string) (rootDeviceName string, rngSt, rngEnd int, err error) { rngSt, rngEnd = invalidVfIndex, invalidVfIndex - if strings.Contains(name, "#") { - fields := strings.Split(name, "#") - ifName = fields[0] - rngSt, rngEnd, err = parseRange(fields[1]) + rootDeviceName, splitRange := SplitDeviceFromRange(device) + if splitRange != "" { + rngSt, rngEnd, err = parseRange(splitRange) } else { - ifName = name + rootDeviceName = device } return } diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 07ea8ea82..a4cca97c0 100644 --- a/pkg/webhook/validate.go +++ b/pkg/webhook/validate.go @@ -367,6 +367,11 @@ func validatePolicyForNodePolicy(current *sriovnetworkv1.SriovNetworkNodePolicy, return err } + err = validateRootDevices(current, previous) + if err != nil { + return err + } + err = validateExludeTopologyField(current, previous) if err != nil { return err @@ -377,28 +382,18 @@ func validatePolicyForNodePolicy(current *sriovnetworkv1.SriovNetworkNodePolicy, func validatePfNames(current *sriovnetworkv1.SriovNetworkNodePolicy, previous *sriovnetworkv1.SriovNetworkNodePolicy) error { for _, curPf := range current.Spec.NicSelector.PfNames { - curName, curRngSt, curRngEnd, err := sriovnetworkv1.ParsePFName(curPf) + curName, curRngSt, curRngEnd, err := sriovnetworkv1.ParseVfRange(curPf) if err != nil { return fmt.Errorf("invalid PF name: %s", curPf) } for _, prePf := range previous.Spec.NicSelector.PfNames { - // Not validate return err of ParsePFName for previous PF + // Not validate return err for previous PF // since it should already be evaluated in previous run. - preName, preRngSt, preRngEnd, _ := sriovnetworkv1.ParsePFName(prePf) + preName, preRngSt, preRngEnd, _ := sriovnetworkv1.ParseVfRange(prePf) if curName == preName { - // reject policy with externallyManage if there is a policy on the same PF without it - if current.Spec.ExternallyManaged != previous.Spec.ExternallyManaged { - 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()) + err = validateExternallyManage(current, previous) + if err != nil { + return err } // Check for overlapping ranges @@ -413,6 +408,37 @@ func validatePfNames(current *sriovnetworkv1.SriovNetworkNodePolicy, previous *s return nil } +func validateRootDevices(current *sriovnetworkv1.SriovNetworkNodePolicy, previous *sriovnetworkv1.SriovNetworkNodePolicy) error { + for _, curRootDevice := range current.Spec.NicSelector.RootDevices { + for _, preRootDevice := range previous.Spec.NicSelector.RootDevices { + // TODO: (SchSeba) implement range for root devices + if curRootDevice == preRootDevice { + return fmt.Errorf("root device %s is overlapped with existing policy %s", curRootDevice, previous.GetName()) + } + } + } + return nil +} + +func validateExternallyManage(current, previous *sriovnetworkv1.SriovNetworkNodePolicy) error { + // reject policy with externallyManage if there is a policy on the same PF without it + if current.Spec.ExternallyManaged != previous.Spec.ExternallyManaged { + 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 +} + func validateExludeTopologyField(current *sriovnetworkv1.SriovNetworkNodePolicy, previous *sriovnetworkv1.SriovNetworkNodePolicy) error { if current.Spec.ResourceName != previous.Spec.ResourceName { return nil diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go index 2f1f2ffab..ccb69a5b7 100644 --- a/pkg/webhook/validate_test.go +++ b/pkg/webhook/validate_test.go @@ -774,6 +774,31 @@ func TestValidatePoliciesWithSameExcludeTopologyForTheSameResource(t *testing.T) g.Expect(err).NotTo(HaveOccurred()) } +func TestValidatePoliciesWithDifferentNumVfForTheSameResourceAndTheSameRootDevice(t *testing.T) { + current := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "currentPolicy"}, + Spec: SriovNetworkNodePolicySpec{ + ResourceName: "resourceX", + NumVfs: 10, + NicSelector: SriovNetworkNicSelector{RootDevices: []string{"0000:86:00.1"}}, + }, + } + + previous := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "previousPolicy"}, + Spec: SriovNetworkNodePolicySpec{ + ResourceName: "resourceX", + NumVfs: 5, + NicSelector: SriovNetworkNicSelector{RootDevices: []string{"0000:86:00.1"}}, + }, + } + + err := validatePolicyForNodePolicy(current, previous) + + g := NewGomegaWithT(t) + g.Expect(err).To(MatchError("root device 0000:86:00.1 is overlapped with existing policy previousPolicy")) +} + func TestStaticValidateSriovNetworkNodePolicyWithValidVendorDevice(t *testing.T) { policy := &SriovNetworkNodePolicy{ Spec: SriovNetworkNodePolicySpec{ diff --git a/test/e2e/sriovoperatornodepolicy_test.go b/test/e2e/sriovoperatornodepolicy_test.go index ba2ec5e64..10eb86589 100644 --- a/test/e2e/sriovoperatornodepolicy_test.go +++ b/test/e2e/sriovoperatornodepolicy_test.go @@ -268,7 +268,7 @@ var _ = Describe("Operator", func() { Expect(iface.VfGroups[0].DeviceType).To(Equal(policy.Spec.DeviceType)) Expect(iface.VfGroups[0].ResourceName).To(Equal(policy.Spec.ResourceName)) - pfName, rngStart, rngEnd, err := sriovnetworkv1.ParsePFName(policy.Spec.NicSelector.PfNames[0]) + pfName, rngStart, rngEnd, err := sriovnetworkv1.ParseVfRange(policy.Spec.NicSelector.PfNames[0]) Expect(err).NotTo(HaveOccurred()) rng := strconv.Itoa(rngStart) + "-" + strconv.Itoa(rngEnd) Expect(iface.Name).To(Equal(pfName))