[active-standby] Fix show mux status inconsistency introduced by or…

…chagent rollback (sonic-net#225) (sonic-net#226) Approach What is the motivation for this PR? This is to fix the show mux status inconsistency introduced by orchagent roll back. In mux port state machine design, linkmgrd honors hardware state for active-standby ports, and never intends to trigger a secondary toggle when everything is healthy. But after we introduce orchagent rollback, show mux status can return unmatched APP_DB and STATE_DB entries for this, which blocks upgrade. Hence, submitting this PR as a workaround. sign-off: Jing Zhang [email protected] Work item tracking Microsoft ADO (number only): 26136887 How did you do it? How did you verify/test it?
lolyu · Dec 15, 2023 · 6fa4adb · 6fa4adb
1 parent d7ab364
commit 6fa4adb
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 3 deletions.
diff --git a/src/link_manager/LinkManagerStateMachineActiveStandby.cpp b/src/link_manager/LinkManagerStateMachineActiveStandby.cpp
@@ -317,6 +317,8 @@ void ActiveStandbyStateMachine::switchMuxState(
             mWaitActiveUpCount = 0;
         }
 
+        mLastSetMuxState = label;
+
         enterMuxState(nextState, mux_state::MuxState::Label::Wait);
         mMuxStateMachine.setWaitStateCause(mux_state::WaitState::WaitStateCause::SwssUpdate);
         mMuxPortPtr->postMetricsEvent(Metrics::SwitchingStart, label);
@@ -614,10 +616,11 @@ void ActiveStandbyStateMachine::handleGetMuxStateNotification(mux_state::MuxStat
 {
     MUXLOGINFO(boost::format("%s: state db mux state: %s") % mMuxPortConfig.getPortName() % mMuxStateName[label]);
 
-    if (mComponentInitState.all() && ms(mCompositeState) != label &&
+    if (mComponentInitState.all() &&
         ms(mCompositeState) != mux_state::MuxState::Wait &&
         ms(mCompositeState) != mux_state::MuxState::Error &&
-        ms(mCompositeState) != mux_state::MuxState::Unknown) {
+        ms(mCompositeState) != mux_state::MuxState::Unknown &&
+        (ms(mCompositeState) != label || mLastSetMuxState != label)) {
         // notify swss of mux state change
         MUXLOGWARNING(boost::format("%s: Switching MUX state from '%s' to '%s' to match linkmgrd/xcvrd state") %
             mMuxPortConfig.getPortName() %

diff --git a/src/link_manager/LinkManagerStateMachineActiveStandby.h b/src/link_manager/LinkManagerStateMachineActiveStandby.h
@@ -849,6 +849,8 @@ class ActiveStandbyStateMachine: public LinkManagerStateMachineBase,
     bool mPendingMuxModeChange = false;
     common::MuxPortConfig::Mode mTargetMuxMode = common::MuxPortConfig::Mode::Auto;
 
+    mux_state::MuxState::Label mLastSetMuxState = mux_state::MuxState::Label::Wait;
+
     bool mContinuousLinkProberUnknownEvent = false; // When posting unknown_end event, we want to make sure the previous state is unknown.
 
     link_manager::ActiveStandbyStateMachine::SwitchCause mSendSwitchActiveCommandCause;

diff --git a/test/LinkManagerStateMachineTest.cpp b/test/LinkManagerStateMachineTest.cpp
@@ -1435,5 +1435,32 @@ TEST_F(LinkManagerStateMachineTest, MuxStandbyLinkProberStandbyLinkDownLinkUpRes
     VALIDATE_STATE(Standby, Standby, Up);
 }
 
+TEST_F(LinkManagerStateMachineTest, OrchagentRollback)
+{
+    setMuxStandby();
+
+    EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 0);
+
+    handleMuxConfig("active", 4);
+    VALIDATE_STATE(Wait, Wait, Up);
+    EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1);
 
+    // orchagent rollback, toggle is not successful.
+    postLinkProberEvent(link_prober::LinkProberState::Standby, 3);
+    VALIDATE_STATE(Standby, Wait, Up);
+    handleMuxState("standby", 3);
+    VALIDATE_STATE(Standby, Standby, Up);
+
+    // extra toggle to make app_db entry in sync with state_db
+    handleGetMuxState("standby", 2);
+    EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 2);
+    VALIDATE_STATE(Standby, Wait, Up);
+
+    handleMuxState("standby", 3);
+    handleGetMuxState("standby", 2);
+    VALIDATE_STATE(Standby, Standby, Up);
+
+    // no 3rd toggle
+    EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 2);
+}
 } /* namespace test */
diff --git a/test/MuxManagerTest.cpp b/test/MuxManagerTest.cpp
@@ -924,7 +924,7 @@ INSTANTIATE_TEST_CASE_P(
     MuxState,
     GetMuxStateTest,
     ::testing::Values(
-        std::make_tuple("active", mux_state::MuxState::Label::Active),
+        std::make_tuple("active", mux_state::MuxState::Label::Wait),
         std::make_tuple("standby", mux_state::MuxState::Label::Wait),
         std::make_tuple("unknown", mux_state::MuxState::Label::Wait),
         std::make_tuple("error", mux_state::MuxState::Label::Wait)