From b976379473952bf30b50465aa550ea2998a7217e Mon Sep 17 00:00:00 2001 From: Longxiang Lyu Date: Mon, 27 Jun 2022 12:35:15 +0000 Subject: [PATCH 1/2] Fix inconsistent mux state Signed-off-by: Longxiang Lyu --- .../LinkManagerStateMachineActiveActive.cpp | 28 +++++++++++++++++-- .../LinkManagerStateMachineActiveActive.h | 9 ++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/link_manager/LinkManagerStateMachineActiveActive.cpp b/src/link_manager/LinkManagerStateMachineActiveActive.cpp index 00a6d6ec..5e0d57ba 100644 --- a/src/link_manager/LinkManagerStateMachineActiveActive.cpp +++ b/src/link_manager/LinkManagerStateMachineActiveActive.cpp @@ -507,6 +507,15 @@ void ActiveActiveStateMachine::initializeTransitionFunctionTable() MUXLOGWARNING("Initializing State Transition Table..."); LinkManagerStateMachineBase::initializeTransitionFunctionTable(); + mStateTransitionHandler[link_prober::LinkProberState::Label::Active] + [mux_state::MuxState::Label::Active] + [link_state::LinkState::Label::Up] = + boost::bind( + &ActiveActiveStateMachine::LinkProberActiveMuxActiveLinkUpTransitionFunction, + this, + boost::placeholders::_1 + ); + mStateTransitionHandler[link_prober::LinkProberState::Label::Active] [mux_state::MuxState::Label::Standby] [link_state::LinkState::Label::Up] = @@ -580,6 +589,19 @@ void ActiveActiveStateMachine::initializeTransitionFunctionTable() ); } +// +// ---> LinkProberActiveMuxActiveLinkUpTransitionFunction(CompositeState &nextState); +// +// transition function when entering {LinkProberActive, MuxActive, LinkUp} state +// +void ActiveActiveStateMachine::LinkProberActiveMuxActiveLinkUpTransitionFunction(CompositeState &nextState) +{ + MUXLOGINFO(mMuxPortConfig.getPortName()); + if (ms(mCompositeState) == mux_state::MuxState::Unknown) { + switchMuxState(nextState, mux_state::MuxState::Label::Active); + } +} + // // ---> LinkProberActiveMuxStandbyLinkUpTransitionFunction(CompositeState &nextState); // @@ -1002,9 +1024,9 @@ void ActiveActiveStateMachine::handlePeerMuxWaitTimeout(boost::system::error_cod } } -// +// // ---> handleDefaultRouteStateNotification(const DefaultRoute routeState); -// +// // handle default route state notification from routeorch // void ActiveActiveStateMachine::handleDefaultRouteStateNotification(const DefaultRoute routeState) @@ -1029,7 +1051,7 @@ void ActiveActiveStateMachine::shutdownOrRestartLinkProberOnDefaultRoute() if (mMuxPortConfig.getMode() == common::MuxPortConfig::Mode::Auto && mDefaultRouteState == DefaultRoute::NA) { mShutdownTxFnPtr(); } else { - // If mux mode is in manual/standby/active mode, we should restart link prober. + // If mux mode is in manual/standby/active mode, we should restart link prober. // If default route state is "ok", we should retart link prober. mRestartTxFnPtr(); } diff --git a/src/link_manager/LinkManagerStateMachineActiveActive.h b/src/link_manager/LinkManagerStateMachineActiveActive.h index a85986e4..98ca9458 100644 --- a/src/link_manager/LinkManagerStateMachineActiveActive.h +++ b/src/link_manager/LinkManagerStateMachineActiveActive.h @@ -236,6 +236,15 @@ class ActiveActiveStateMachine : public LinkManagerStateMachineBase, */ void initializeTransitionFunctionTable() override; + /** + * @method LinkProberActiveMuxActiveLinkUpTransitionFunction + * + * @brief transition function when entering {LinkProberActive, MuxActive, LinkUp} state + * + * @param nextState reference to composite state + */ + void LinkProberActiveMuxActiveLinkUpTransitionFunction(CompositeState &nextState); + /** * @method LinkProberActiveMuxStandbyLinkUpTransitionFunction * From 1786f989eae4c2059fe820ce31e1b3f3e20d420c Mon Sep 17 00:00:00 2001 From: Longxiang Lyu Date: Tue, 28 Jun 2022 04:54:50 +0000 Subject: [PATCH 2/2] Add unit test Signed-off-by: Longxiang Lyu --- ...inkManagerStateMachineActiveActiveTest.cpp | 51 +++++++++++++++---- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/test/LinkManagerStateMachineActiveActiveTest.cpp b/test/LinkManagerStateMachineActiveActiveTest.cpp index 52b95b6c..054d6742 100644 --- a/test/LinkManagerStateMachineActiveActiveTest.cpp +++ b/test/LinkManagerStateMachineActiveActiveTest.cpp @@ -364,7 +364,7 @@ TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActiveLinkProberPeerUnknown) TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActiveConfigDetachedLinkProberPeerUnknown) { setMuxActive(); - + postPeerLinkProberEvent(link_prober::LinkProberState::PeerActive); VALIDATE_PEER_STATE(PeerActive, Active); @@ -415,26 +415,57 @@ TEST_F(LinkManagerStateMachineActiveActiveTest, MuxStandbyLinkProberPeerUnknown) EXPECT_EQ(mDbInterfacePtr->mSetPeerMuxStateInvokeCount, 0); } -TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActivDefaultRouteState) +TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActivDefaultRouteState) { setMuxActive(); postDefaultRouteEvent("ok", 1); EXPECT_FALSE(mMuxConfig.getIfEnableDefaultRouteFeature()); - EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount,0); - EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount,1); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount, 0); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount, 1); postDefaultRouteEvent("na", 1); - EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount,0); - EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount,2); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount, 0); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount, 2); mMuxConfig.enableDefaultRouteFeature(true); postDefaultRouteEvent("na", 1); - EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount,1); - EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount,2); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount, 1); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount, 2); postDefaultRouteEvent("ok", 1); - EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount,1); - EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount,3); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount, 1); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount, 3); } + +TEST_F(LinkManagerStateMachineActiveActiveTest, LinkmgrdBootupSequenceHeartBeatFirst) +{ + activateStateMachine(); + VALIDATE_STATE(Wait, Wait, Down); + + postLinkEvent(link_state::LinkState::Up); + VALIDATE_STATE(Wait, Wait, Up); + + // the first toggle fails because the the inital mux state + // is standby when linkmgrd first boots up + postLinkProberEvent(link_prober::LinkProberState::Active, 4); + VALIDATE_STATE(Active, Active, Up); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); + + // now linkmgrd should be stuck in mux wait timeout + + handleProbeMuxState("unknown", 3); + VALIDATE_STATE(Active, Unknown, Up); + + // now linkmgrd should be stuck in mux probe timeout + runIoService(4); + + // xcvrd now answers the mux probe + handleProbeMuxState("active", 4); + VALIDATE_STATE(Active, Active, Up); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 2); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); +} + } /* namespace test */