From ba321eb330e48f3202870d59fe908c14f86aeb13 Mon Sep 17 00:00:00 2001 From: Nikola Dancejic Date: Wed, 26 Aug 2026 12:10:29 -0700 Subject: [PATCH] [active-active] Always drive an explicit mux config to the driver handleMuxConfigNotification() skipped switchMuxState() when the requested mode already matched the in-memory mux state. That state is bootstrapped from the first probe after a linkmgrd restart and is never reconciled against the hardware, so a matching `config mux mode active|standby` was silently dropped: the CLI reported success, the driver was never told, and the port was left reporting an inconsistent mux status. Drop the comparison so an explicit Active or Standby config is always forwarded to the driver. switchMuxState() is already called with forceSwitch on these paths, so the mismatch case is unchanged. Auto, Manual and Detached modes keep their existing behavior. Fixes sonic-net/sonic-buildimage#26391 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Nikola Dancejic --- .../LinkManagerStateMachineActiveActive.cpp | 6 +- ...inkManagerStateMachineActiveActiveTest.cpp | 75 ++++++++++++++++++- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/link_manager/LinkManagerStateMachineActiveActive.cpp b/src/link_manager/LinkManagerStateMachineActiveActive.cpp index 19b7bfc8..1920d64c 100644 --- a/src/link_manager/LinkManagerStateMachineActiveActive.cpp +++ b/src/link_manager/LinkManagerStateMachineActiveActive.cpp @@ -271,9 +271,11 @@ void ActiveActiveStateMachine::handleMuxConfigNotification(const common::MuxPort mMuxPortConfig.setMode(mode); if (mComponentInitState.all()) { CompositeState nextState = mCompositeState; - if (mode == common::MuxPortConfig::Mode::Active && ms(mCompositeState) != mux_state::MuxState::Label::Active) { + // Always drive an explicit config: the in-memory MUX state is bootstrapped on restart + // without being reconciled against the driver, so it is no proof the hardware agrees. + if (mode == common::MuxPortConfig::Mode::Active) { switchMuxState(nextState, mux_state::MuxState::Label::Active, true); - } else if (mode == common::MuxPortConfig::Mode::Standby && ms(mCompositeState) != mux_state::MuxState::Label::Standby) { + } else if (mode == common::MuxPortConfig::Mode::Standby) { switchMuxState(nextState, mux_state::MuxState::Label::Standby, true); } else if (mode == common::MuxPortConfig::Mode::Auto && ms(mCompositeState) == mux_state::MuxState::Label::Unknown) { MUXLOGINFO(boost::format("%s: reset link prober state") % mMuxPortConfig.getPortName()); diff --git a/test/LinkManagerStateMachineActiveActiveTest.cpp b/test/LinkManagerStateMachineActiveActiveTest.cpp index d4bec295..152a9e0c 100644 --- a/test/LinkManagerStateMachineActiveActiveTest.cpp +++ b/test/LinkManagerStateMachineActiveActiveTest.cpp @@ -476,6 +476,78 @@ TEST_F(LinkManagerStateMachineActiveActiveTest, ConfigStandbySocAgentRestart) VALIDATE_STATE(Active, Active, Up); } +TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActiveConfigActiveReDrivesMuxState) +{ + setMuxActive(); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + + handleMuxConfig("active", 1); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 2); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); + VALIDATE_STATE(Active, Active, Up); +} + +TEST_F(LinkManagerStateMachineActiveActiveTest, MuxStandbyConfigStandbyReDrivesMuxState) +{ + setMuxStandby(); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + + handleMuxConfig("standby", 1); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 2); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Standby); + VALIDATE_STATE(Unknown, Standby, Up); +} + +TEST_F(LinkManagerStateMachineActiveActiveTest, MuxConfigActiveAfterRestartMatchingBootstrappedState) +{ + // Restart: the mux state is bootstrapped from state db before the state machine is + // activated, so the config below matches it and calculates no state transition. + postLinkEvent(link_state::LinkState::Up, 0, true); + VALIDATE_STATE(Wait, Wait, Up); + + handleMuxState("active", 0, true); + VALIDATE_STATE(Wait, Active, Up); + + activateStateMachine(); + pollIoService(); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 0); + VALIDATE_STATE(Wait, Active, Up); + + handleMuxConfig("active", 1); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); + VALIDATE_STATE(Wait, Active, Up); +} + +TEST_F(LinkManagerStateMachineActiveActiveTest, MuxConfigActiveBeforeInitMatchingBootstrappedState) +{ + // Config read before initialization is deferred and replayed once all components are up. + handleMuxConfig("active", 0, true); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 0); + + postLinkEvent(link_state::LinkState::Up, 0, true); + VALIDATE_STATE(Wait, Wait, Up); + + handleMuxState("active", 0, true); + VALIDATE_STATE(Wait, Active, Up); + + activateStateMachine(); + pollIoService(); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); + VALIDATE_STATE(Wait, Active, Up); +} + +TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActiveConfigAutoDoesNotReDriveMuxState) +{ + setMuxActive(); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + + handleMuxConfig("auto", 1); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + VALIDATE_STATE(Active, Active, Up); +} + TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActiveLinkProberPeerActive) { setMuxActive(); @@ -634,11 +706,12 @@ TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActivDefaultRouteStateMuxConf handleMuxConfig("active", 2); EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount, 2); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 2); postDefaultRouteEvent("na", 1); EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount, 0); EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount, 3); - EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 2); VALIDATE_STATE(Active, Active, Up); }