Skip to content

fix: reboot loop and improve devlink parameter reconciliation #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
e0ne merged 1 commit into from
Jan 19, 2026
Merged

fix: reboot loop and improve devlink parameter reconciliation #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions api/v1/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -282,6 +305,11 @@ func NeedToUpdateSriov(ifaceSpec *Interface, ifaceStatus *InterfaceExt) bool {
}
}
}

if NeedToUpdateDevlinkParams(&ifaceSpec.DevlinkParams, &ifaceStatus.DevlinkParams) {
return true
}

return false
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/plugins/mellanox/mellanox_plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
41 changes: 32 additions & 9 deletions pkg/vendors/mellanox/mellanox.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down
79 changes: 70 additions & 9 deletions pkg/vendors/mellanox/mellanox_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
Expand Down Expand Up @@ -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))
})
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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))
})
Expand Down Expand Up @@ -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))
})
Expand All @@ -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))
})
Expand Down Expand Up @@ -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))
})
Expand Down Expand Up @@ -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))
})
Expand Down Expand Up @@ -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))
})
Expand Down
Loading