fix: make lacp active nilable

This fixes aset of inconsistencies, one of the examples is #13203.

Make the field in the spec nilable, and treat `nil` as `on` (this is
Linux default if the field is not set explicitly).

When using machine config, the setting can be flipped to `off`
explicitly (if needed).

This brings back compatibility with raw metal network resource specs
pre-1.12 which don't contain any explicit `adLacpActive: on` at all.

Fixes #13203

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
This commit is contained in:
Andrey Smirnov 2026-04-28 20:57:02 +04:00
parent 0d1d95c7da
commit 6a445406e0
No known key found for this signature in database
GPG Key ID: 322C6F63F594CE7C
19 changed files with 119 additions and 44 deletions

View File

@ -65,10 +65,6 @@ func (a bondMaster) FillDefaults() {
bond.MissedMax = 2
}
}
if bond.Mode != nethelpers.BondMode8023AD {
bond.ADLACPActive = nethelpers.ADLACPActiveOn
}
}
// Encode the BondMasterSpec into netlink attributes.
@ -84,7 +80,10 @@ func (a bondMaster) Encode() ([]byte, error) {
if bond.Mode == nethelpers.BondMode8023AD {
encoder.Uint8(unix.IFLA_BOND_AD_LACP_RATE, uint8(bond.LACPRate))
encoder.Uint8(unix.IFLA_BOND_AD_LACP_ACTIVE, uint8(bond.ADLACPActive))
if bond.ADLACPActive != nil {
encoder.Uint8(unix.IFLA_BOND_AD_LACP_ACTIVE, uint8(*bond.ADLACPActive))
}
}
if bond.Mode != nethelpers.BondMode8023AD && bond.Mode != nethelpers.BondModeALB && bond.Mode != nethelpers.BondModeTLB {
@ -270,7 +269,7 @@ func (a bondMaster) Decode(data []byte) error {
case unix.IFLA_BOND_PEER_NOTIF_DELAY:
bond.PeerNotifyDelay = decoder.Uint32()
case unix.IFLA_BOND_AD_LACP_ACTIVE:
bond.ADLACPActive = nethelpers.ADLACPActive(decoder.Uint8())
bond.ADLACPActive = new(nethelpers.ADLACPActive(decoder.Uint8()))
case unix.IFLA_BOND_MISSED_MAX:
bond.MissedMax = decoder.Uint8()
}

View File

@ -16,24 +16,70 @@ import (
)
func TestBondMasterSpec(t *testing.T) {
spec := network.BondMasterSpec{
Mode: nethelpers.BondModeActiveBackup,
MIIMon: 100,
UpDelay: 200,
DownDelay: 300,
t.Parallel()
for _, test := range []struct {
name string
spec network.BondMasterSpec
}{
{
name: "active-backup",
spec: network.BondMasterSpec{
Mode: nethelpers.BondModeActiveBackup,
MIIMon: 100,
UpDelay: 200,
DownDelay: 300,
},
},
{
name: "802.3ad no lacp",
spec: network.BondMasterSpec{
Mode: nethelpers.BondMode8023AD,
MIIMon: 100,
UpDelay: 200,
DownDelay: 300,
},
},
{
name: "802.3ad lacp on",
spec: network.BondMasterSpec{
Mode: nethelpers.BondMode8023AD,
MIIMon: 100,
UpDelay: 200,
DownDelay: 300,
LACPRate: nethelpers.LACPRateFast,
ADLACPActive: new(nethelpers.ADLACPActiveOn),
},
},
{
name: "802.3ad lacp off",
spec: network.BondMasterSpec{
Mode: nethelpers.BondMode8023AD,
MIIMon: 100,
UpDelay: 200,
DownDelay: 300,
ADLACPActive: new(nethelpers.ADLACPActiveOff),
},
},
} {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
b, err := networkadapter.BondMasterSpec(&test.spec).Encode()
require.NoError(t, err)
var decodedSpec network.BondMasterSpec
require.NoError(t, networkadapter.BondMasterSpec(&decodedSpec).Decode(b))
require.Equal(t, test.spec, decodedSpec)
})
}
b, err := networkadapter.BondMasterSpec(&spec).Encode()
require.NoError(t, err)
var decodedSpec network.BondMasterSpec
require.NoError(t, networkadapter.BondMasterSpec(&decodedSpec).Decode(b))
require.Equal(t, spec, decodedSpec)
}
func TestBondMasterSpecDecodeClearsTargets(t *testing.T) {
t.Parallel()
initial := network.BondMasterSpec{
Mode: nethelpers.BondModeActiveBackup,
ARPIPTargets: []netip.Addr{netip.MustParseAddr("198.51.100.254")},

View File

@ -14,6 +14,8 @@ import (
)
func TestBridgeMasterSpec(t *testing.T) {
t.Parallel()
spec := network.BridgeMasterSpec{
STP: network.STPSpec{
Enabled: true,

View File

@ -15,6 +15,8 @@ import (
)
func TestVLANSpec(t *testing.T) {
t.Parallel()
spec := network.VLANSpec{
VID: 25,
Protocol: nethelpers.VLANProtocol8021AD,

View File

@ -14,6 +14,8 @@ import (
)
func TestVRFMasterSpec(t *testing.T) {
t.Parallel()
spec := network.VRFMasterSpec{
Table: 4294967295,
}

View File

@ -20,6 +20,8 @@ import (
)
func TestWireguardSpecDecode(t *testing.T) {
t.Parallel()
fips140.WithoutEnforcement(func() {
priv, err := wgtypes.GeneratePrivateKey()
require.NoError(t, err)
@ -105,6 +107,8 @@ func TestWireguardSpecDecode(t *testing.T) {
}
func TestWireguardSpecDecodeStatus(t *testing.T) {
t.Parallel()
fips140.WithoutEnforcement(func() {
priv, err := wgtypes.GeneratePrivateKey()
require.NoError(t, err)
@ -131,6 +135,8 @@ func TestWireguardSpecDecodeStatus(t *testing.T) {
}
func TestWireguardSpecEncode(t *testing.T) {
t.Parallel()
fips140.WithoutEnforcement(func() {
priv, err := wgtypes.GeneratePrivateKey()
require.NoError(t, err)

View File

@ -58,7 +58,6 @@ func TestCmdlineParse(t *testing.T) {
TLBDynamicLB: 1,
UseCarrier: true,
PrimaryIndex: new(uint32(0)),
ADLACPActive: nethelpers.ADLACPActiveOn,
MissedMax: 2,
},
},
@ -408,7 +407,6 @@ func TestCmdlineParse(t *testing.T) {
TLBDynamicLB: 1,
UseCarrier: true,
PrimaryIndex: new(uint32(0)),
ADLACPActive: nethelpers.ADLACPActiveOn,
},
},
{

View File

@ -457,7 +457,7 @@ func (suite *LinkConfigSuite) TestMachineConfigurationWithAliases() {
asrt.Equal(network.LinkKindBond, r.TypedSpec().Kind)
asrt.Equal(nethelpers.BondModeXOR, r.TypedSpec().BondMaster.Mode)
asrt.True(r.TypedSpec().BondMaster.UseCarrier)
asrt.Equal(nethelpers.ADLACPActiveOn, r.TypedSpec().BondMaster.ADLACPActive)
asrt.Nil(r.TypedSpec().BondMaster.ADLACPActive)
}
},
)
@ -560,7 +560,7 @@ func (suite *LinkConfigSuite) TestMachineConfigurationNewStyle() {
asrt.Equal(network.LinkKindBond, r.TypedSpec().Kind)
asrt.Equal(nethelpers.BondModeActiveBackup, r.TypedSpec().BondMaster.Mode)
asrt.EqualValues(200, r.TypedSpec().BondMaster.UpDelay)
asrt.Equal(nethelpers.ADLACPActiveOn, r.TypedSpec().BondMaster.ADLACPActive)
asrt.Nil(r.TypedSpec().BondMaster.ADLACPActive)
case "br0":
asrt.True(r.TypedSpec().Up)
asrt.True(r.TypedSpec().Logical)

View File

@ -44,7 +44,13 @@ func SendBondMaster(link *network.LinkSpecSpec, bond talosconfig.NetworkBondConf
link.BondMaster.ADSelect = bond.ADSelect().ValueOrZero()
link.BondMaster.ADActorSysPrio = bond.ADActorSysPrio().ValueOrZero()
link.BondMaster.ADUserPortKey = bond.ADUserPortKey().ValueOrZero()
link.BondMaster.ADLACPActive = bond.ADLACPActive().ValueOr(nethelpers.ADLACPActiveOn)
if adLACPActive, ok := bond.ADLACPActive().Get(); ok {
link.BondMaster.ADLACPActive = new(adLACPActive)
} else {
link.BondMaster.ADLACPActive = nil
}
link.BondMaster.PrimaryReselect = bond.PrimaryReselect().ValueOrZero()
link.BondMaster.ResendIGMP = bond.ResendIGMP().ValueOrZero()
link.BondMaster.MinLinks = bond.MinLinks().ValueOrZero()
@ -144,7 +150,6 @@ func SetBondMasterLegacy(link *network.LinkSpecSpec, bond talosconfig.Bond) erro
ADActorSysPrio: bond.ADActorSysPrio(),
ADUserPortKey: bond.ADUserPortKey(),
PeerNotifyDelay: bond.PeerNotifyDelay(),
ADLACPActive: nethelpers.ADLACPActiveOn,
}
networkadapter.BondMasterSpec(&link.BondMaster).FillDefaults()

View File

@ -225,7 +225,7 @@ func (p *EquinixMetal) ParseMetadata(ctx context.Context, equinixMetadata *Metad
}
if bondMode == nethelpers.BondMode8023AD {
bondLink.BondMaster.ADLACPActive = nethelpers.ADLACPActiveOn
bondLink.BondMaster.ADLACPActive = new(nethelpers.ADLACPActiveOn)
}
networkadapter.BondMasterSpec(&bondLink.BondMaster).FillDefaults()

View File

@ -465,7 +465,7 @@ func (n *Nocloud) applyNetworkConfigV1(ctx context.Context, config *NetworkConfi
}
if mode == nethelpers.BondMode8023AD {
bondLink.BondMaster.ADLACPActive = nethelpers.ADLACPActiveOn
bondLink.BondMaster.ADLACPActive = new(nethelpers.ADLACPActiveOn)
}
if ntwrk.MTU != 0 {
@ -872,7 +872,7 @@ func (n *Nocloud) applyNetworkConfigV2(ctx context.Context, config *NetworkConfi
}
if mode == nethelpers.BondMode8023AD {
bondLink.BondMaster.ADLACPActive = nethelpers.ADLACPActiveOn
bondLink.BondMaster.ADLACPActive = new(nethelpers.ADLACPActiveOn)
}
networkadapter.BondMasterSpec(&bondLink.BondMaster).FillDefaults()

View File

@ -146,7 +146,7 @@ func (o *OpenStack) ParseMetadata(
}
if mode == nethelpers.BondMode8023AD {
bondLink.BondMaster.ADLACPActive = nethelpers.ADLACPActiveOn
bondLink.BondMaster.ADLACPActive = new(nethelpers.ADLACPActiveOn)
}
networkadapter.BondMasterSpec(&bondLink.BondMaster).FillDefaults()

View File

@ -1385,9 +1385,9 @@
"off"
],
"title": "adLACPActive",
"description": "Whether to send LACPDU frames periodically.\n",
"markdownDescription": "Whether to send LACPDU frames periodically.",
"x-intellij-html-description": "\u003cp\u003eWhether to send LACPDU frames periodically.\u003c/p\u003e\n"
"description": "Whether to send LACPDU frames periodically, defaults to “on” if mode is 802.3ad.\n",
"markdownDescription": "Whether to send LACPDU frames periodically, defaults to \"on\" if mode is 802.3ad.",
"x-intellij-html-description": "\u003cp\u003eWhether to send LACPDU frames periodically, defaults to \u0026ldquo;on\u0026rdquo; if mode is 802.3ad.\u003c/p\u003e\n"
},
"primaryReselect": {
"enum": [

View File

@ -228,7 +228,7 @@ type BondConfigV1Alpha1 struct {
// 0
BondADUserPortKey *uint16 `yaml:"adUserPortKey,omitempty"`
// description: |
// Whether to send LACPDU frames periodically.
// Whether to send LACPDU frames periodically, defaults to "on" if mode is 802.3ad.
// examples:
// - value: >
// "on"

View File

@ -238,8 +238,8 @@ func (BondConfigV1Alpha1) Doc() *encoder.Doc {
Name: "adLACPActive",
Type: "ADLACPActive",
Note: "",
Description: "Whether to send LACPDU frames periodically.",
Comments: [3]string{"" /* encoder.HeadComment */, "Whether to send LACPDU frames periodically." /* encoder.LineComment */, "" /* encoder.FootComment */},
Description: "Whether to send LACPDU frames periodically, defaults to \"on\" if mode is 802.3ad.",
Comments: [3]string{"" /* encoder.HeadComment */, "Whether to send LACPDU frames periodically, defaults to \"on\" if mode is 802.3ad." /* encoder.LineComment */, "" /* encoder.FootComment */},
Values: []string{
"on",
"off",

View File

@ -39,6 +39,10 @@ func (o BondMasterSpec) DeepCopy() BondMasterSpec {
cp.NSIP6Targets = make([]netip.Addr, len(o.NSIP6Targets))
copy(cp.NSIP6Targets, o.NSIP6Targets)
}
if o.ADLACPActive != nil {
cp.ADLACPActive = new(nethelpers.ADLACPActive)
*cp.ADLACPActive = *o.ADLACPActive
}
return cp
}

View File

@ -86,7 +86,7 @@ type BondMasterSpec struct {
// Maximum of 16 targets are supported.
NSIP6Targets []netip.Addr `yaml:"nsIp6Targets,omitempty" protobuf:"26"`
// ADLACPActive specifies whether to send LACPDU frames periodically.
ADLACPActive nethelpers.ADLACPActive `yaml:"adLacpActive,omitempty" protobuf:"27"`
ADLACPActive *nethelpers.ADLACPActive `yaml:"adLacpActive,omitempty" protobuf:"27"`
// MissedMax is the number of arp_interval monitor checks that must fail in order for an interface to be marked down by the ARP monitor.
MissedMax uint8 `yaml:"missedMax,omitempty" protobuf:"28"`
}
@ -211,7 +211,18 @@ func (spec *BondMasterSpec) Equal(other *BondMasterSpec) bool {
}
}
if spec.ADLACPActive != other.ADLACPActive {
// default value for ADLACPActive is "on" in Linux, so if both are nil or "on", consider them equal
specADLACPActive, otherADLACPActive := nethelpers.ADLACPActiveOn, nethelpers.ADLACPActiveOn
if spec.ADLACPActive != nil {
specADLACPActive = *spec.ADLACPActive
}
if other.ADLACPActive != nil {
otherADLACPActive = *other.ADLACPActive
}
if specADLACPActive != otherADLACPActive {
return false
}
@ -254,7 +265,7 @@ func (spec *BondMasterSpec) IsZero() bool {
spec.PeerNotifyDelay == 0 &&
len(spec.ARPIPTargets) == 0 &&
len(spec.NSIP6Targets) == 0 &&
spec.ADLACPActive == 0 &&
spec.ADLACPActive == nil &&
spec.MissedMax == 0
}

View File

@ -64,7 +64,7 @@ routes:
# # Aggregate selection policy for 802.3ad.
# adSelect: stable
# # Whether to send LACPDU frames periodically.
# # Whether to send LACPDU frames periodically, defaults to "on" if mode is 802.3ad.
# adLACPActive: on
# # Policy under which the primary slave should be reselected.
@ -136,7 +136,7 @@ adActorSysPrio: 65535
|`adUserPortKey` |uint16 |User port key (upper 10 bits) for 802.3ad. <details><summary>Show example(s)</summary>{{< highlight yaml >}}
adUserPortKey: 0
{{< /highlight >}}</details> | |
|`adLACPActive` |ADLACPActive |Whether to send LACPDU frames periodically. <details><summary>Show example(s)</summary>{{< highlight yaml >}}
|`adLACPActive` |ADLACPActive |Whether to send LACPDU frames periodically, defaults to "on" if mode is 802.3ad. <details><summary>Show example(s)</summary>{{< highlight yaml >}}
adLACPActive: on
{{< /highlight >}}</details> |`on`<br />`off`<br /> |
|`primaryReselect` |PrimaryReselect |Policy under which the primary slave should be reselected. <details><summary>Show example(s)</summary>{{< highlight yaml >}}

View File

@ -1385,9 +1385,9 @@
"off"
],
"title": "adLACPActive",
"description": "Whether to send LACPDU frames periodically.\n",
"markdownDescription": "Whether to send LACPDU frames periodically.",
"x-intellij-html-description": "\u003cp\u003eWhether to send LACPDU frames periodically.\u003c/p\u003e\n"
"description": "Whether to send LACPDU frames periodically, defaults to “on” if mode is 802.3ad.\n",
"markdownDescription": "Whether to send LACPDU frames periodically, defaults to \"on\" if mode is 802.3ad.",
"x-intellij-html-description": "\u003cp\u003eWhether to send LACPDU frames periodically, defaults to \u0026ldquo;on\u0026rdquo; if mode is 802.3ad.\u003c/p\u003e\n"
},
"primaryReselect": {
"enum": [