From 9ea5c449f5f98b11a944c389c99c6840f3d03f24 Mon Sep 17 00:00:00 2001 From: Longxiang Lyu Date: Thu, 30 Jun 2022 08:30:54 +0000 Subject: [PATCH] Enforce switch after config mux to active Signed-off-by: Longxiang Lyu --- .../LinkManagerStateMachineActiveActive.cpp | 38 +++++++++- .../LinkManagerStateMachineActiveActive.h | 27 ++++--- ...inkManagerStateMachineActiveActiveTest.cpp | 72 +++++++++++++++++++ 3 files changed, 126 insertions(+), 11 deletions(-) diff --git a/src/link_manager/LinkManagerStateMachineActiveActive.cpp b/src/link_manager/LinkManagerStateMachineActiveActive.cpp index 5e0d57ba..823f48a8 100644 --- a/src/link_manager/LinkManagerStateMachineActiveActive.cpp +++ b/src/link_manager/LinkManagerStateMachineActiveActive.cpp @@ -163,6 +163,7 @@ void ActiveActiveStateMachine::handleMuxStateNotification(mux_state::MuxState::L MUXLOGWARNING(boost::format("%s: state db mux state: %s") % mMuxPortConfig.getPortName() % mMuxStateName[label]); mWaitTimer.cancel(); + mLastMuxStateNotification = label; if (mComponentInitState.all()) { if (mMuxStateMachine.getWaitStateCause() != mux_state::WaitState::WaitStateCause::SwssUpdate) { @@ -543,6 +544,15 @@ void ActiveActiveStateMachine::initializeTransitionFunctionTable() boost::placeholders::_1 ); + mStateTransitionHandler[link_prober::LinkProberState::Label::Unknown] + [mux_state::MuxState::Label::Standby] + [link_state::LinkState::Label::Up] = + boost::bind( + &ActiveActiveStateMachine::LinkProberUnknownMuxStandbyLinkUpTransitionFunction, + this, + boost::placeholders::_1 + ); + mStateTransitionHandler[link_prober::LinkProberState::Label::Active] [mux_state::MuxState::Label::Unknown] [link_state::LinkState::Label::Up] = @@ -621,9 +631,29 @@ void ActiveActiveStateMachine::LinkProberActiveMuxStandbyLinkUpTransitionFunctio void ActiveActiveStateMachine::LinkProberUnknownMuxActiveLinkUpTransitionFunction(CompositeState &nextState) { MUXLOGINFO(mMuxPortConfig.getPortName()); - switchMuxState(nextState, mux_state::MuxState::Label::Standby); + if (mMuxPortConfig.getMode() == common::MuxPortConfig::Mode::Active && mLastMuxStateNotification != mux_state::MuxState::Label::Active) { + // last switch mux state to active failed, try again + switchMuxState(nextState, mux_state::MuxState::Label::Active, true); + } else { + switchMuxState(nextState, mux_state::MuxState::Label::Standby); + } } +// +// ---> LinkProberUnknownMuxStandbyLinkUpTransitionFunction(CompositeState &nextState); +// +// transition function when entering {LinkProberUnknown, MuxStandby, LinkUp} state +// +void ActiveActiveStateMachine::LinkProberUnknownMuxStandbyLinkUpTransitionFunction(CompositeState &nextState) +{ + MUXLOGINFO(mMuxPortConfig.getPortName()); + if (mMuxPortConfig.getMode() == common::MuxPortConfig::Mode::Active && mLastMuxStateNotification != mux_state::MuxState::Label::Active) { + // last switch mux state to active failed, try again + switchMuxState(nextState, mux_state::MuxState::Label::Active, true); + } +} + + // // ---> LinkProberActiveMuxUnknownLinkUpTransitionFunction(CompositeState &nextState); // @@ -781,10 +811,12 @@ void ActiveActiveStateMachine::enterPeerLinkProberState(link_prober::LinkProberS // void ActiveActiveStateMachine::switchMuxState( CompositeState &nextState, - mux_state::MuxState::Label label + mux_state::MuxState::Label label, + bool forceSwitch ) { - if (mMuxPortConfig.getMode() == common::MuxPortConfig::Mode::Auto || + if (forceSwitch || + mMuxPortConfig.getMode() == common::MuxPortConfig::Mode::Auto || mMuxPortConfig.getMode() == common::MuxPortConfig::Mode::Detached) { MUXLOGWARNING( boost::format("%s: Switching MUX state to '%s'") % diff --git a/src/link_manager/LinkManagerStateMachineActiveActive.h b/src/link_manager/LinkManagerStateMachineActiveActive.h index b5ab5674..1a69445e 100644 --- a/src/link_manager/LinkManagerStateMachineActiveActive.h +++ b/src/link_manager/LinkManagerStateMachineActiveActive.h @@ -150,15 +150,15 @@ class ActiveActiveStateMachine : public LinkManagerStateMachineBase, /** * @method handleDefaultRouteStateNotification(const DefaultRoute routeState) - * + * * @brief handle default route state notification from routeorch - * + * * @param routeState - * + * * @return none - */ + */ void handleDefaultRouteStateNotification(const DefaultRoute routeState) override; - + /** *@method handleGetServerMacNotification * @@ -272,6 +272,15 @@ class ActiveActiveStateMachine : public LinkManagerStateMachineBase, */ void LinkProberUnknownMuxActiveLinkUpTransitionFunction(CompositeState &nextState); + /** + * @method LinkProberUnknownMuxStandbyLinkUpTransitionFunction + * + * @brief transition function when entering {LinkProberUnknown, MuxStandby, LinkUp} state + * + * @param nextState reference to composite state + */ + void LinkProberUnknownMuxStandbyLinkUpTransitionFunction(CompositeState &nextState); + /** * @method LinkProberUnknownMuxUnknownLinkUpTransitionFunction * @@ -374,8 +383,9 @@ class ActiveActiveStateMachine : public LinkManagerStateMachineBase, * * @param nextState reference to composite state * @param label new MuxState label to switch to + * @param forceSwitch force switch mux state, used to match the driver state only */ - inline void switchMuxState(CompositeState &nextState, mux_state::MuxState::Label label); + inline void switchMuxState(CompositeState &nextState, mux_state::MuxState::Label label, bool forceSwitch = false); /** * @method switchPeerMuxState @@ -467,9 +477,9 @@ class ActiveActiveStateMachine : public LinkManagerStateMachineBase, /** * @method shutdownOrRestartLinkProberOnDefaultRoute() - * + * * @brief shutdown or restart link prober based on default route state - * + * * @return none */ void shutdownOrRestartLinkProberOnDefaultRoute() override; @@ -558,6 +568,7 @@ class ActiveActiveStateMachine : public LinkManagerStateMachineBase, private: // peer link prober state and mux state link_prober::LinkProberState::Label mPeerLinkProberState = link_prober::LinkProberState::Label::PeerWait; mux_state::MuxState::Label mPeerMuxState = mux_state::MuxState::Label::Wait; + mux_state::MuxState::Label mLastMuxStateNotification = mux_state::MuxState::Label::Unknown; private: uint32_t mMuxProbeBackoffFactor = 1; diff --git a/test/LinkManagerStateMachineActiveActiveTest.cpp b/test/LinkManagerStateMachineActiveActiveTest.cpp index 8aee8622..4222f8d2 100644 --- a/test/LinkManagerStateMachineActiveActiveTest.cpp +++ b/test/LinkManagerStateMachineActiveActiveTest.cpp @@ -496,4 +496,76 @@ TEST_F(LinkManagerStateMachineActiveActiveTest, LinkmgrdBootupSequenceHeartBeatF EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); } +TEST_F(LinkManagerStateMachineActiveActiveTest, LinkmgrdBootupSequenceMuxConfigActiveProbeActive) +{ + activateStateMachine(); + VALIDATE_STATE(Wait, Wait, Down); + + postLinkEvent(link_state::LinkState::Up); + VALIDATE_STATE(Wait, Wait, Up); + + postLinkProberEvent(link_prober::LinkProberState::Unknown, 3); + VALIDATE_STATE(Unknown, Standby, Up); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Standby); + + handleMuxState("unknown", 3); + VALIDATE_STATE(Unknown, Unknown, Up); + + handleMuxConfig("active", 1); + VALIDATE_STATE(Unknown, Active, Up); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 2); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); + + handleMuxState("unknown", 5); + VALIDATE_STATE(Unknown, Unknown, Up); + + handleProbeMuxState("unknown", 3); + VALIDATE_STATE(Unknown, Unknown, Up); + + // xcvrd now answers the mux probe + handleProbeMuxState("active", 3); + VALIDATE_STATE(Unknown, Active, Up); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 3); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); + + handleMuxState("active", 3); + VALIDATE_STATE(Unknown, Active, Up); +} + +TEST_F(LinkManagerStateMachineActiveActiveTest, LinkmgrdBootupSequenceMuxConfigActiveProbeStandby) +{ + activateStateMachine(); + VALIDATE_STATE(Wait, Wait, Down); + + postLinkEvent(link_state::LinkState::Up); + VALIDATE_STATE(Wait, Wait, Up); + + postLinkProberEvent(link_prober::LinkProberState::Unknown, 3); + VALIDATE_STATE(Unknown, Standby, Up); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Standby); + + handleMuxState("unknown", 3); + VALIDATE_STATE(Unknown, Unknown, Up); + + handleMuxConfig("active", 1); + VALIDATE_STATE(Unknown, Active, Up); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 2); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); + + handleMuxState("unknown", 5); + VALIDATE_STATE(Unknown, Unknown, Up); + + handleProbeMuxState("unknown", 3); + VALIDATE_STATE(Unknown, Unknown, Up); + + // xcvrd now answers the mux probe + handleProbeMuxState("standby", 3); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 3); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); + handleMuxState("active", 3); + VALIDATE_STATE(Unknown, Active, Up); +} + } /* namespace test */