diff --git a/api/v1/helper.go b/api/v1/helper.go index 20d333470..7a6d1551b 100644 --- a/api/v1/helper.go +++ b/api/v1/helper.go @@ -204,6 +204,29 @@ func GetEswitchModeFromStatus(ifaceStatus *InterfaceExt) string { return ifaceStatus.EswitchMode } +func NeedToUpdateDevlinkParams(desired *DevlinkParams, current *DevlinkParams) bool { + for _, dParam := range desired.Params { + found := false + for _, cParam := range current.Params { + if dParam.Name == cParam.Name { + found = true + if dParam.Value != cParam.Value { + log.V(0).Info("NeedToUpdateDevlinkParams(): DevlinkParam needs update", + "name", dParam.Name, "desired", dParam.Value, "current", cParam.Value) + return true + } + break + } + } + if !found { + log.V(0).Info("NeedToUpdateDevlinkParams(): DevlinkParam needs update - not found in status", + "name", dParam.Name, "desired", dParam.Value) + return true + } + } + return false +} + func NeedToUpdateSriov(ifaceSpec *Interface, ifaceStatus *InterfaceExt) bool { if ifaceSpec.Mtu > 0 { mtu := ifaceSpec.Mtu @@ -282,6 +305,11 @@ func NeedToUpdateSriov(ifaceSpec *Interface, ifaceStatus *InterfaceExt) bool { } } } + + if NeedToUpdateDevlinkParams(&ifaceSpec.DevlinkParams, &ifaceStatus.DevlinkParams) { + return true + } + return false } diff --git a/pkg/plugins/mellanox/mellanox_plugin.go b/pkg/plugins/mellanox/mellanox_plugin.go index 97ae3085b..c4f453cec 100644 --- a/pkg/plugins/mellanox/mellanox_plugin.go +++ b/pkg/plugins/mellanox/mellanox_plugin.go @@ -123,7 +123,7 @@ func (p *MellanoxPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeS needReboot = totalVfsNeedReboot || sriovEnNeedReboot changeWithoutReboot = totalVfsChangeWithoutReboot || sriovEnChangeWithoutReboot - needESwitchParamsChange := mlx.HandleESwitchParams(pciPrefix, attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needESwitchParamsChange := mlx.HandleESwitchParams(pciPrefix, attrs, fwCurrent, mellanoxNicsSpec, mellanoxNicsStatus) needReboot = needReboot || needESwitchParamsChange needLinkChange, err := mlx.HandleLinkType(pciPrefix, fwCurrent, attrs, mellanoxNicsSpec, mellanoxNicsStatus) diff --git a/pkg/vendors/mellanox/mellanox.go b/pkg/vendors/mellanox/mellanox.go index 3e5397015..7f33d3620 100644 --- a/pkg/vendors/mellanox/mellanox.go +++ b/pkg/vendors/mellanox/mellanox.go @@ -243,7 +243,7 @@ func (m *mellanoxHelper) MlxConfigFW(attributesToChange map[string]MlxNic) error func (m *mellanoxHelper) GetMlxNicFwData(pciAddress string) (current, next *MlxNic, err error) { log.Log.Info("mellanox-plugin getMlnxNicFwData()", "device", pciAddress) - attrs := []string{TotalVfs, EnableSriov, LinkTypeP1, LinkTypeP2} + attrs := []string{TotalVfs, EnableSriov, LinkTypeP1, LinkTypeP2, LagResourceAllocation} out, stderr, err := m.MstConfigReadData(pciAddress) if err != nil { @@ -398,7 +398,7 @@ func HandleLinkType(pciPrefix string, fwData, attr *MlxNic, } // HandleESwitchParams check if eswitch params should be changes -func HandleESwitchParams(pciPrefix string, attr *MlxNic, +func HandleESwitchParams(pciPrefix string, attr *MlxNic, fwCurrent *MlxNic, mellanoxNicsSpec map[string]sriovnetworkv1.Interface, mellanoxNicsStatus map[string]map[string]sriovnetworkv1.InterfaceExt) bool { needReboot := false @@ -408,17 +408,32 @@ func HandleESwitchParams(pciPrefix string, attr *MlxNic, ifaceStatus := getIfaceStatus(pciAddress, mellanoxNicsStatus) needChange, devlinkParam := isESwitchParamsRequireChange(firstPortSpec, ifaceStatus) if needChange { - log.Log.V(2).Info("Changing eswitch params (multiport), needs reboot", - "device", ifaceStatus.PciAddress) + desiredMultiport := 1 if devlinkParam != nil { if devlinkParam.Value == devlinkMultiportEnableValue { - attr.Multiport = 1 + desiredMultiport = 1 } else { - attr.Multiport = 0 + desiredMultiport = 0 } - } else { - attr.Multiport = 1 } + + if fwCurrent.Multiport == -1 { + log.Log.V(2).Info("LagResourceAllocation not supported in firmware, skipping firmware change", + "device", ifaceStatus.PciAddress) + attr.Multiport = -1 + return false + } + + if fwCurrent.Multiport == desiredMultiport { + log.Log.V(2).Info("Eswitch params (multiport) already set in firmware, skipping reboot", + "device", ifaceStatus.PciAddress) + attr.Multiport = -1 + return false + } + + log.Log.V(2).Info("Changing eswitch params (multiport), needs reboot", + "device", ifaceStatus.PciAddress) + attr.Multiport = desiredMultiport needReboot = true } else { attr.Multiport = -1 @@ -431,7 +446,7 @@ func HandleESwitchParams(pciPrefix string, attr *MlxNic, func mlnxNicFromMap(mstData map[string]string) (*MlxNic, error) { log.Log.Info("mellanox-plugin mlnxNicFromMap()", "data", mstData) - fwData := &MlxNic{} + fwData := &MlxNic{Multiport: -1} if strings.Contains(mstData[EnableSriov], "True") { fwData.EnableSriov = true } @@ -446,6 +461,14 @@ func mlnxNicFromMap(mstData map[string]string) (*MlxNic, error) { fwData.LinkTypeP2 = getLinkType(linkTypeP2) } + if val, ok := mstData[LagResourceAllocation]; ok { + if strings.Contains(val, "(1)") { + fwData.Multiport = 1 + } else if strings.Contains(val, "(0)") { + fwData.Multiport = 0 + } + } + return fwData, nil } diff --git a/pkg/vendors/mellanox/mellanox_test.go b/pkg/vendors/mellanox/mellanox_test.go index 869c1cbe8..02360b122 100644 --- a/pkg/vendors/mellanox/mellanox_test.go +++ b/pkg/vendors/mellanox/mellanox_test.go @@ -635,7 +635,7 @@ var _ = Describe("SRIOV", func() { } attrs := &MlxNic{} - needReboot := HandleESwitchParams("0000:d8:00.", attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 0}, mellanoxNicsSpec, mellanoxNicsStatus) Expect(needReboot).To(BeFalse()) Expect(attrs.Multiport).To(Equal(-1)) }) @@ -670,7 +670,7 @@ var _ = Describe("SRIOV", func() { } attrs := &MlxNic{} - needReboot := HandleESwitchParams("0000:d8:00.", attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 0}, mellanoxNicsSpec, mellanoxNicsStatus) Expect(needReboot).To(BeFalse()) Expect(attrs.Multiport).To(Equal(-1)) }) @@ -705,11 +705,72 @@ var _ = Describe("SRIOV", func() { } attrs := &MlxNic{} - needReboot := HandleESwitchParams("0000:d8:00.", attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 0}, mellanoxNicsSpec, mellanoxNicsStatus) Expect(needReboot).To(BeTrue()) Expect(attrs.Multiport).To(Equal(1)) }) + It("should return false if esw_multiport is missing in status but already enabled in firmware", func() { + ifaceSpec := sriovnetworkv1.Interface{ + PciAddress: "0000:d8:00.0", + Name: "eth0", + NumVfs: 10, + DevlinkParams: sriovnetworkv1.DevlinkParams{ + Params: []sriovnetworkv1.DevlinkParam{ + {Name: "esw_multiport", Value: "true"}, + }, + }, + } + mellanoxNicsSpec := map[string]sriovnetworkv1.Interface{ + "0000:d8:00.0": ifaceSpec, + } + mellanoxNicsExt := map[string]sriovnetworkv1.InterfaceExt{ + "0000:d8:00.0": { + PciAddress: "0000:d8:00.0", + Name: "eth0", + // No esw_multiport in status + }, + } + mellanoxNicsStatus := map[string]map[string]sriovnetworkv1.InterfaceExt{ + "0000:d8:00.": mellanoxNicsExt, + } + attrs := &MlxNic{} + + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 1}, mellanoxNicsSpec, mellanoxNicsStatus) + Expect(needReboot).To(BeFalse()) + Expect(attrs.Multiport).To(Equal(-1)) + }) + + It("should return false and not change Multiport if firmware does not support LagResourceAllocation", func() { + ifaceSpec := sriovnetworkv1.Interface{ + PciAddress: "0000:d8:00.0", + Name: "eth0", + NumVfs: 10, + DevlinkParams: sriovnetworkv1.DevlinkParams{ + Params: []sriovnetworkv1.DevlinkParam{ + {Name: "esw_multiport", Value: "true"}, + }, + }, + } + mellanoxNicsSpec := map[string]sriovnetworkv1.Interface{ + "0000:d8:00.0": ifaceSpec, + } + mellanoxNicsExt := map[string]sriovnetworkv1.InterfaceExt{ + "0000:d8:00.0": { + PciAddress: "0000:d8:00.0", + Name: "eth0", + }, + } + mellanoxNicsStatus := map[string]map[string]sriovnetworkv1.InterfaceExt{ + "0000:d8:00.": mellanoxNicsExt, + } + attrs := &MlxNic{} + + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: -1}, mellanoxNicsSpec, mellanoxNicsStatus) + Expect(needReboot).To(BeFalse()) + Expect(attrs.Multiport).To(Equal(-1)) + }) + It("should return true and set Multiport to 0 if esw_multiport should be disabled", func() { ifaceSpec := sriovnetworkv1.Interface{ PciAddress: "0000:d8:00.0", @@ -740,7 +801,7 @@ var _ = Describe("SRIOV", func() { } attrs := &MlxNic{} - needReboot := HandleESwitchParams("0000:d8:00.", attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 1}, mellanoxNicsSpec, mellanoxNicsStatus) Expect(needReboot).To(BeTrue()) Expect(attrs.Multiport).To(Equal(0)) }) @@ -773,7 +834,7 @@ var _ = Describe("SRIOV", func() { } attrs := &MlxNic{} - needReboot := HandleESwitchParams("0000:d8:00.", attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 0}, mellanoxNicsSpec, mellanoxNicsStatus) Expect(needReboot).To(BeTrue()) Expect(attrs.Multiport).To(Equal(1)) }) @@ -788,7 +849,7 @@ var _ = Describe("SRIOV", func() { } attrs := &MlxNic{} - needReboot := HandleESwitchParams("0000:d8:00.", attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 0}, mellanoxNicsSpec, mellanoxNicsStatus) Expect(needReboot).To(BeFalse()) Expect(attrs.Multiport).To(Equal(-1)) }) @@ -824,7 +885,7 @@ var _ = Describe("SRIOV", func() { } attrs := &MlxNic{} - needReboot := HandleESwitchParams("0000:d8:00.", attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 0}, mellanoxNicsSpec, mellanoxNicsStatus) Expect(needReboot).To(BeFalse()) Expect(attrs.Multiport).To(Equal(-1)) }) @@ -859,7 +920,7 @@ var _ = Describe("SRIOV", func() { } attrs := &MlxNic{} - needReboot := HandleESwitchParams("0000:d8:00.", attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 0}, mellanoxNicsSpec, mellanoxNicsStatus) Expect(needReboot).To(BeFalse()) Expect(attrs.Multiport).To(Equal(-1)) }) @@ -898,7 +959,7 @@ var _ = Describe("SRIOV", func() { } attrs := &MlxNic{} - needReboot := HandleESwitchParams("0000:d8:00.", attrs, mellanoxNicsSpec, mellanoxNicsStatus) + needReboot := HandleESwitchParams("0000:d8:00.", attrs, &MlxNic{Multiport: 0}, mellanoxNicsSpec, mellanoxNicsStatus) Expect(needReboot).To(BeTrue()) Expect(attrs.Multiport).To(Equal(1)) })