diff --git a/src/DbInterface.cpp b/src/DbInterface.cpp index d5f43572..21688b75 100644 --- a/src/DbInterface.cpp +++ b/src/DbInterface.cpp @@ -1076,6 +1076,34 @@ void DbInterface::processMuxLinkmgrConfigNotifiction(std::deque fieldValues = kfvFieldsValues(entry); + + for (auto &fieldValue: fieldValues) { + std::string f = fvField(fieldValue); + std::string v = fvValue(fieldValue); + if (f == "oscillation_enabled") { + if (v == "true") { + mMuxManagerPtr->setOscillationEnabled(true); + } else if (v == "false"){ + mMuxManagerPtr->setOscillationEnabled(false); + } + } else if (f == "interval_sec") { + try { + mMuxManagerPtr->setOscillationInterval_sec(boost::lexical_cast (v)); + } catch (boost::bad_lexical_cast const &badLexicalCast) { + MUXLOGWARNING(boost::format("bad lexical cast: %s") % badLexicalCast.what()); + } + } + MUXLOGINFO(boost::format("key: %s, Operation: %s, f: %s, v: %s") % key % operation % diff --git a/src/MuxManager.h b/src/MuxManager.h index 3ae75b72..d49fe2c7 100644 --- a/src/MuxManager.h +++ b/src/MuxManager.h @@ -123,6 +123,28 @@ class MuxManager */ inline void setTimeoutIpv6_msec(uint32_t timeout_msec) {mMuxConfig.setTimeoutIpv6_msec(timeout_msec);}; + /** + *@method setOscillationEnabled + * + *@brief setter for enable/disable oscillation + * + *@param enable (in) true to enable oscillation + * + *@return none + */ + inline void setOscillationEnabled(bool enable) {mMuxConfig.setOscillationEnabled(enable);}; + + /** + *@method setOscillationInterval_sec + * + *@brief setter for oscillation interval in sec + * + *@param interval_sec (in) interval in sec + * + *@return none + */ + inline void setOscillationInterval_sec(uint32_t interval_sec) {mMuxConfig.setOscillationInterval_sec(interval_sec);}; + /** *@method setLinkProberStatUpdateIntervalCount * diff --git a/src/MuxPort.cpp b/src/MuxPort.cpp index ca1501cf..a9961dff 100644 --- a/src/MuxPort.cpp +++ b/src/MuxPort.cpp @@ -369,12 +369,11 @@ void MuxPort::handleDefaultRouteState(const std::string &routeState) state = link_manager::LinkManagerStateMachineBase::DefaultRoute::NA; } - boost::asio::io_service &ioService = mStrand.context(); - ioService.post(mStrand.wrap(boost::bind( + boost::asio::post(mStrand, boost::bind( &link_manager::LinkManagerStateMachineBase::handleDefaultRouteStateNotification, mLinkManagerStateMachinePtr.get(), state - ))); + )); } // diff --git a/src/common/MuxConfig.h b/src/common/MuxConfig.h index 588492cb..8ad6454e 100644 --- a/src/common/MuxConfig.h +++ b/src/common/MuxConfig.h @@ -97,6 +97,17 @@ class MuxConfig */ inline void setTimeoutIpv6_msec(uint32_t timeout_msec) {mTimeoutIpv6_msec = timeout_msec;}; + /** + *@method setOscillationEnabled + * + *@brief setter for enabling/disabling mux state oscillation + * + *@param enable (in) enable/disable mux state oscillation + * + *@return none + */ + inline void setOscillationEnabled(bool enable) {mEnableTimedOscillationWhenNoHeartbeat = enable;}; + /** *@method setLinkProberStatUpdateIntervalCount * @@ -274,6 +285,15 @@ class MuxConfig */ inline uint32_t getSuspendTimeout_msec() const {return (mNegativeStateChangeRetryCount + 1) * mTimeoutIpv4_msec;}; + /** + *@method getIfOscillationEnabled + * + *@brief getter for oscillation flag + * + *@return oscillation flag + */ + inline bool getIfOscillationEnabled() const {return mEnableTimedOscillationWhenNoHeartbeat;}; + /** *@method getOscillationInterval_sec * @@ -433,7 +453,6 @@ class MuxConfig private: uint8_t mNumberOfThreads = 5; - uint32_t mOscillationTimeout_sec = 300; uint32_t mTimeoutIpv4_msec = 100; uint32_t mTimeoutIpv6_msec = 1000; uint32_t mPositiveStateChangeRetryCount = 1; @@ -443,6 +462,9 @@ class MuxConfig uint32_t mMuxStateChangeRetryCount = 1; uint32_t mLinkStateChangeRetryCount = 1; + bool mEnableTimedOscillationWhenNoHeartbeat = true; + uint32_t mOscillationTimeout_sec = 300; + bool mEnableSwitchoverMeasurement = false; uint32_t mDecreasedTimeoutIpv4_msec = 10; diff --git a/src/common/MuxPortConfig.h b/src/common/MuxPortConfig.h index bc1f82e0..fa8a8b60 100644 --- a/src/common/MuxPortConfig.h +++ b/src/common/MuxPortConfig.h @@ -192,6 +192,15 @@ class MuxPortConfig */ inline uint32_t getLinkWaitTimeout_msec() const {return mMuxConfig.getSuspendTimeout_msec();}; + /** + *@method getIfOscillationEnabled + * + *@brief getter for mux state oscillation enable flag + * + *@return oscillation enable flag + */ + inline bool getIfOscillationEnabled() const {return mMuxConfig.getIfOscillationEnabled();}; + /** *@method getOscillationInterval_sec * diff --git a/src/link_manager/LinkManagerStateMachineActiveStandby.cpp b/src/link_manager/LinkManagerStateMachineActiveStandby.cpp index 52bd2d6f..9045bd4c 100644 --- a/src/link_manager/LinkManagerStateMachineActiveStandby.cpp +++ b/src/link_manager/LinkManagerStateMachineActiveStandby.cpp @@ -61,8 +61,6 @@ ActiveStandbyStateMachine::ActiveStandbyStateMachine( mMuxStateMachine.setWaitStateCause(mux_state::WaitState::WaitStateCause::SwssUpdate); mMuxPortPtr->setMuxLinkmgrState(mLabel); initializeTransitionFunctionTable(); - - mOscillationTimer.expires_from_now(boost::posix_time::seconds(1)); } // @@ -476,12 +474,16 @@ void ActiveStandbyStateMachine::handleStateChange(LinkProberEvent &event, link_p mMuxPortPtr->postLinkProberMetricsEvent(link_manager::ActiveStandbyStateMachine::LinkProberMetrics::LinkProberActiveStart); mStandbyUnknownUpCount = 0; + + tryCancelOscillationTimerIfAlive(); } if (state == link_prober::LinkProberState::Label::Standby) { mMuxPortPtr->postLinkProberMetricsEvent(link_manager::ActiveStandbyStateMachine::LinkProberMetrics::LinkProberStandbyStart); mActiveUnknownUpCount = 0; + + tryCancelOscillationTimerIfAlive(); } CompositeState nextState = mCompositeState; @@ -542,6 +544,10 @@ void ActiveStandbyStateMachine::handleStateChange(MuxStateEvent &event, mux_stat mStandbyUnknownUpCount = 0; } + if (state == mux_state::MuxState::Label::Standby) { + tryCancelOscillationTimerIfAlive(); + } + updateMuxLinkmgrState(); } @@ -567,6 +573,9 @@ void ActiveStandbyStateMachine::handleStateChange(LinkStateEvent &event, link_st // There is a problem with this approach as it will hide link flaps that result in lost heart-beats. initLinkProberState(nextState, true); // enterMuxWaitState(nextState); + + // reset mux probing backoff factor when link goes up + mMuxUnknownBackoffFactor = 1; } else if (ls(mCompositeState) == link_state::LinkState::Up && ls(nextState) == link_state::LinkState::Down && ms(mCompositeState) != mux_state::MuxState::Label::Standby) { @@ -575,6 +584,12 @@ void ActiveStandbyStateMachine::handleStateChange(LinkStateEvent &event, link_st mActiveUnknownUpCount = 0; mStandbyUnknownUpCount = 0; + + mWaitActiveUpCount = 0; + mWaitStandbyUpBackoffFactor = 1; + mUnknownActiveUpBackoffFactor = 1; + + tryCancelOscillationTimerIfAlive(); } else { mStateTransitionHandler[ps(nextState)][ms(nextState)][ls(nextState)](nextState); } @@ -1057,6 +1072,8 @@ void ActiveStandbyStateMachine::handleMuxWaitTimeout(boost::system::error_code e void ActiveStandbyStateMachine::startOscillationTimer() { // Note: This timer is started when Mux state is active and link prober is in wait state. + MUXLOGINFO(boost::format("%s: start the oscillation timer") % mMuxPortConfig.getPortName()); + mOscillationTimerAlive = true; mOscillationTimer.expires_from_now(boost::posix_time::seconds( mMuxPortConfig.getOscillationInterval_sec() )); @@ -1067,6 +1084,20 @@ void ActiveStandbyStateMachine::startOscillationTimer() ))); } +// +// ---> tryCancelOscillationTimerIfAlive(); +// +// cancel the oscillation timer if it is alive +// +void ActiveStandbyStateMachine::tryCancelOscillationTimerIfAlive() +{ + if (mOscillationTimerAlive) { + MUXLOGINFO(boost::format("%s: cancel the oscillation timer") % mMuxPortConfig.getPortName()); + mOscillationTimerAlive = false; + mOscillationTimer.cancel(); + } +} + // // ---> handleOscillationTimeout(boost::system::error_code errorCode); // @@ -1076,7 +1107,9 @@ void ActiveStandbyStateMachine::handleOscillationTimeout(boost::system::error_co { MUXLOGDEBUG(mMuxPortConfig.getPortName()); - if (errorCode == boost::system::errc::success && + mOscillationTimerAlive = false; + if (mMuxPortConfig.getIfOscillationEnabled() && + errorCode == boost::system::errc::success && ps(mCompositeState) == link_prober::LinkProberState::Label::Wait && ms(mCompositeState) == mux_state::MuxState::Label::Active && ls(mCompositeState) == link_state::LinkState::Label::Up) { @@ -1310,7 +1343,7 @@ void ActiveStandbyStateMachine::LinkProberWaitMuxActiveLinkUpTransitionFunction( mSuspendTxFnPtr(mMuxPortConfig.getLinkWaitTimeout_msec()); } - if (mOscillationTimer.expires_at() < boost::posix_time::microsec_clock::local_time()) { + if (!mOscillationTimerAlive) { startOscillationTimer(); } } diff --git a/src/link_manager/LinkManagerStateMachineActiveStandby.h b/src/link_manager/LinkManagerStateMachineActiveStandby.h index 147b952a..c1b1d58e 100644 --- a/src/link_manager/LinkManagerStateMachineActiveStandby.h +++ b/src/link_manager/LinkManagerStateMachineActiveStandby.h @@ -464,6 +464,15 @@ class ActiveStandbyStateMachine: public LinkManagerStateMachineBase, */ void startOscillationTimer(); + /** + *@method tryCancelOscillationTimerIfAlive + * + *@brief cancel the oscillation timer if it is alive + * + *@return none + */ + inline void tryCancelOscillationTimerIfAlive(); + /** *@method handleOscillationTimeout * @@ -859,6 +868,7 @@ class ActiveStandbyStateMachine: public LinkManagerStateMachineBase, boost::asio::deadline_timer mDeadlineTimer; boost::asio::deadline_timer mWaitTimer; boost::asio::deadline_timer mOscillationTimer; + bool mOscillationTimerAlive = false; boost::function mInitializeProberFnPtr; boost::function mStartProbingFnPtr; diff --git a/test/FakeMuxPort.h b/test/FakeMuxPort.h index 8fc80720..07e5a0be 100644 --- a/test/FakeMuxPort.h +++ b/test/FakeMuxPort.h @@ -63,6 +63,7 @@ class FakeMuxPort : public ::mux::MuxPort { link_prober::LinkProberStateMachineBase* getLinkProberStateMachinePtr() { return getLinkManagerStateMachinePtr()->getLinkProberStateMachinePtr().get(); }; mux_state::MuxStateMachine& getMuxStateMachine() { return getLinkManagerStateMachinePtr()->getMuxStateMachine(); }; link_state::LinkStateMachine& getLinkStateMachine() { return getLinkManagerStateMachinePtr()->getLinkStateMachine(); }; + link_manager::LinkManagerStateMachineBase::DefaultRoute getDefaultRouteState() { return getLinkManagerStateMachinePtr()->getDefaultRouteState(); }; bool getPendingMuxModeChange() { return getActiveStandbyStateMachinePtr()->mPendingMuxModeChange; }; common::MuxPortConfig::Mode getTargetMuxMode() { return getActiveStandbyStateMachinePtr()->mTargetMuxMode; }; diff --git a/test/LinkManagerStateMachineTest.cpp b/test/LinkManagerStateMachineTest.cpp index c954c4a8..64001a20 100644 --- a/test/LinkManagerStateMachineTest.cpp +++ b/test/LinkManagerStateMachineTest.cpp @@ -21,8 +21,12 @@ * Author: Tamer Ahmed */ +#include +#include + #include "LinkManagerStateMachineTest.h" #include "link_prober/LinkProberStateMachineBase.h" +#include "common/MuxLogger.h" #define VALIDATE_STATE(p, m, l) \ do { \ @@ -49,7 +53,7 @@ LinkManagerStateMachineTest::LinkManagerStateMachineTest() : mMuxConfig.setPositiveStateChangeRetryCount(mPositiveUpdateCount); mMuxConfig.setMuxStateChangeRetryCount(mPositiveUpdateCount); mMuxConfig.setLinkStateChangeRetryCount(mPositiveUpdateCount); - mMuxConfig.setOscillationInterval_sec(1,true); + mMuxConfig.setOscillationInterval_sec(2, true); } void LinkManagerStateMachineTest::runIoService(uint32_t count) @@ -69,6 +73,21 @@ void LinkManagerStateMachineTest::runIoService(uint32_t count) } } +void LinkManagerStateMachineTest::runIoServiceThreaded(uint32_t count) +{ + mWork = std::make_unique(mIoService); + for (uint8_t i = 0; i < count; i++) { + mThreadGroup.create_thread(boost::bind(&boost::asio::io_service::run, &mIoService)); + } +} + +void LinkManagerStateMachineTest::stopIoServiceThreaded() +{ + mWork.reset(); + mIoService.stop(); + mThreadGroup.join_all(); +} + void LinkManagerStateMachineTest::postLinkProberEvent(link_prober::LinkProberState::Label label, uint32_t count, uint32_t detect_multiplier) { switch (label) { @@ -1452,7 +1471,7 @@ TEST_F(LinkManagerStateMachineTest, TimedOscillation) handleMuxState("active", 3); VALIDATE_STATE(Wait, Active, Up); - runIoService(2); + runIoService(1); VALIDATE_STATE(Wait, Wait, Up); handleProbeMuxState("active", 3); @@ -1465,6 +1484,38 @@ TEST_F(LinkManagerStateMachineTest, TimedOscillation) mMuxConfig.setTimeoutIpv4_msec(10); } +TEST_F(LinkManagerStateMachineTest, TimedOscillationMuxProbeStandbyCancel) +{ + setMuxStandby(); + + // set icmp timeout to be 500ms otherwise it will probe mux state endlessly and get no chance to do timed oscillation + mMuxConfig.setTimeoutIpv4_msec(500); + + postLinkProberEvent(link_prober::LinkProberState::Unknown, 2); + VALIDATE_STATE(Wait, Wait, Up); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, 1); + EXPECT_EQ(mDbInterfacePtr->mLastPostedSwitchCause, link_manager::ActiveStandbyStateMachine::SwitchCause::PeerHeartbeatMissing); + + // swss notification + handleMuxState("active", 3); + VALIDATE_STATE(Wait, Active, Up); + + runIoService(1); + VALIDATE_STATE(Wait, Wait, Up); + + handleProbeMuxState("standby", 3); + VALIDATE_STATE(Wait, Standby, Up); + + boost::this_thread::sleep(boost::posix_time::seconds(2)); + + // 3 mux probe active notifiations + 1 oscillation timeout + 1 mux probe timeout + handleProbeMuxState("active", 5); + VALIDATE_STATE(Wait, Active, Up); + EXPECT_EQ(mDbInterfacePtr->mLastPostedSwitchCause, link_manager::ActiveStandbyStateMachine::SwitchCause::PeerHeartbeatMissing); + + mMuxConfig.setTimeoutIpv4_msec(10); +} + TEST_F(LinkManagerStateMachineTest, OrchagentRollback) { setMuxStandby(); @@ -1529,4 +1580,31 @@ TEST_F(LinkManagerStateMachineTest, ProbeLinkInSuspendTimeout) EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mDetectLinkCallCount, 1); } +TEST_F(LinkManagerStateMachineTest, DefaultRouteStateRaceCondition) +{ + mFakeMuxPort.activateStateMachine(); + runIoServiceThreaded(3); + + mMuxConfig.enableDefaultRouteFeature(true); + for (uint i = 0; i < 10000; ++i) + { + MUXLOGDEBUG(boost::format("Iteration %d") % i); + mFakeMuxPort.handleDefaultRouteState("na"); + mFakeMuxPort.handleDefaultRouteState("ok"); + + int check = 0; + while (((mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount < i + 1) || + (mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount < i + 1)) && (check < 4000)) + { + usleep(2000); + ++check; + } + + EXPECT_EQ(mFakeMuxPort.getDefaultRouteState(), link_manager::LinkManagerStateMachineBase::DefaultRoute::OK); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mShutdownTxProbeCallCount, i + 1); + EXPECT_EQ(mFakeMuxPort.mFakeLinkProber->mRestartTxProbeCallCount, i + 1); + } + stopIoServiceThreaded(); +} + } /* namespace test */ diff --git a/test/LinkManagerStateMachineTest.h b/test/LinkManagerStateMachineTest.h index 41fbe145..38c60fba 100644 --- a/test/LinkManagerStateMachineTest.h +++ b/test/LinkManagerStateMachineTest.h @@ -24,6 +24,7 @@ #ifndef LINKMANAGERSTATEMACHINETEST_H_ #define LINKMANAGERSTATEMACHINETEST_H_ +#include #include "gtest/gtest.h" #include "FakeMuxPort.h" @@ -39,6 +40,8 @@ class LinkManagerStateMachineTest: public ::testing::Test virtual ~LinkManagerStateMachineTest() = default; void runIoService(uint32_t count = 0); + void runIoServiceThreaded(uint32_t count = 3); + void stopIoServiceThreaded(); void postLinkProberEvent(link_prober::LinkProberState::Label label, uint32_t count = 0, uint32_t detect_multiplier = 0); void postMuxEvent(mux_state::MuxState::Label label, uint32_t count = 0); void postLinkEvent(link_state::LinkState::Label label, uint32_t count = 0); @@ -58,6 +61,8 @@ class LinkManagerStateMachineTest: public ::testing::Test public: boost::asio::io_service mIoService; + std::unique_ptr mWork; + boost::thread_group mThreadGroup; common::MuxConfig mMuxConfig; std::shared_ptr mDbInterfacePtr; std::string mPortName = "EtherTest01"; diff --git a/test/MuxManagerTest.cpp b/test/MuxManagerTest.cpp index 461b8ca1..f4d5da9f 100644 --- a/test/MuxManagerTest.cpp +++ b/test/MuxManagerTest.cpp @@ -479,6 +479,20 @@ void MuxManagerTest::resetUpdateEthernetFrameFn(const std::string &portName) } } +uint32_t MuxManagerTest::getOscillationInterval_sec(std::string port) +{ + std::shared_ptr muxPortPtr = mMuxManagerPtr->mPortMap[port]; + + return muxPortPtr->mMuxPortConfig.getOscillationInterval_sec(); +} + +bool MuxManagerTest::getOscillationEnabled(std::string port) +{ + std::shared_ptr muxPortPtr = mMuxManagerPtr->mPortMap[port]; + + return muxPortPtr->mMuxPortConfig.getIfOscillationEnabled(); +} + TEST_F(MuxManagerTest, UpdatePortCableTypeActiveStandby) { std::string port = MuxManagerTest::PortName; @@ -663,6 +677,42 @@ TEST_F(MuxManagerTest, SrcMacAddressUpdate) EXPECT_TRUE(mFakeLinkProber->mUpdateEthernetFrameCallCount == updateEthernetFrameCallCountBefore + 1); } +TEST_P(OscillationIntervalTest, OscillationInterval) +{ + std::string port = "Ethernet0"; + createPort(port); + + std::deque entries = { + {"TIMED_OSCILLATION", "SET", {{"interval_sec", std::get<0>(GetParam())}}} + }; + processMuxLinkmgrConfigNotifiction(entries); + + EXPECT_TRUE(getOscillationInterval_sec(port) == std::get<1>(GetParam())); +} + +INSTANTIATE_TEST_CASE_P( + OscillationInterval, + OscillationIntervalTest, + ::testing::Values( + std::make_tuple("1200", 1200), + std::make_tuple("1", 300), + std::make_tuple("invalid", 300) + ) +); + +TEST_F(MuxManagerTest, OscillationDisabled) +{ + std::string port = "Ethernet0"; + createPort(port); + + std::deque entries = { + {"TIMED_OSCILLATION", "SET", {{"oscillation_enabled", "false"}}} + }; + processMuxLinkmgrConfigNotifiction(entries); + + EXPECT_TRUE(getOscillationEnabled(port) == false); +} + TEST_F(MuxManagerTest, ServerMacAddress) { std::string port = "Ethernet0"; diff --git a/test/MuxManagerTest.h b/test/MuxManagerTest.h index 5874d4fb..8d3fda92 100644 --- a/test/MuxManagerTest.h +++ b/test/MuxManagerTest.h @@ -56,9 +56,11 @@ class MuxManagerTest: public testing::Test uint32_t getTimeoutIpv4_msec(std::string port); uint32_t getTimeoutIpv6_msec(std::string port); uint32_t getLinkWaitTimeout_msec(std::string port); + uint32_t getOscillationInterval_sec(std::string port); bool getIfUseWellKnownMac(std::string port); bool setUseWellKnownMacActiveActive(bool use); bool getIfUseToRMac(std::string port); + bool getOscillationEnabled(std::string port); boost::asio::ip::address getBladeIpv4Address(std::string port); std::array getBladeMacAddress(std::string port); std::array getLastUpdatedMacAddress(std::string port); @@ -123,6 +125,11 @@ class MuxConfigUpdateTest: public MuxManagerTest, { }; +class OscillationIntervalTest: public MuxManagerTest, + public testing::WithParamInterface> +{ +}; + } /* namespace test */ #endif /* MUXMANAGERTEST_H_ */