From 59ad124876aca522c8fa6ecfe401aaf0e1ae3763 Mon Sep 17 00:00:00 2001 From: Harjot Singh Date: Thu, 25 Jun 2026 16:26:34 -0700 Subject: [PATCH] Mux recovery when link up comes before ICMP session Active with ICMP offload Signed-off-by: Harjot Singh --- .../LinkManagerStateMachineActiveActive.cpp | 19 +++++- test/FakeMuxPort.h | 4 ++ ...inkManagerStateMachineActiveActiveTest.cpp | 64 +++++++++++++++++++ 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/link_manager/LinkManagerStateMachineActiveActive.cpp b/src/link_manager/LinkManagerStateMachineActiveActive.cpp index 229382f0..19b7bfc8 100644 --- a/src/link_manager/LinkManagerStateMachineActiveActive.cpp +++ b/src/link_manager/LinkManagerStateMachineActiveActive.cpp @@ -591,8 +591,23 @@ void ActiveActiveStateMachine::handleStateChange( CompositeState nextState = mCompositeState; ls(nextState) = state; if (ls(mCompositeState) == link_state::LinkState::Down && ls(nextState) == link_state::LinkState::Up) { - initLinkProberState(nextState); - initPeerLinkProberState(); + if (ps(mCompositeState) == link_prober::LinkProberState::Label::Active) { + // The link prober already holds a definitive Active state before the + // link-up event is processed. This happens with ICMP hardware offload, + // where the probing session can come up while the physical link is + // still down, driving the prober to Active at (P: Active, M: Unknown, + // L: Down). Re-initializing the prober here (the default behavior for an + // Unknown/Wait/Error mux maps to Wait) would discard that valid Active + // state and strand the port at (P: Wait, M: Unknown, L: Up) with no + // further transition. Preserve the Active prober state and run the + // normal transition function so xcvrd is probed first; orchagent only + // receives switchMuxState(Active) once the forwarding state is confirmed. + initPeerLinkProberState(); + mStateTransitionHandler[ps(nextState)][ms(nextState)][ls(nextState)](nextState); + } else { + initLinkProberState(nextState); + initPeerLinkProberState(); + } } else if (ls(mCompositeState) == link_state::LinkState::Up && ls(nextState) == link_state::LinkState::Down && ms(mCompositeState) != mux_state::MuxState::Label::Standby) { switchMuxState(nextState, mux_state::MuxState::Label::Standby); } else { diff --git a/test/FakeMuxPort.h b/test/FakeMuxPort.h index 5431198f..a845f4cf 100644 --- a/test/FakeMuxPort.h +++ b/test/FakeMuxPort.h @@ -78,6 +78,10 @@ class FakeMuxPort : public ::mux::MuxPort { inline void initLinkProberActiveActive(); inline void initLinkProberActiveStandby(); + inline void setLinkProberType(common::MuxPortConfig::LinkProberType type) + { + mMuxPortConfig.setLinkProberType(type); + } std::shared_ptr mActiveActiveStateMachinePtr; std::shared_ptr mActiveStandbyStateMachinePtr; std::shared_ptr mFakeLinkManagerStateMachinePtr = nullptr; diff --git a/test/LinkManagerStateMachineActiveActiveTest.cpp b/test/LinkManagerStateMachineActiveActiveTest.cpp index 7fdf13fb..d4bec295 100644 --- a/test/LinkManagerStateMachineActiveActiveTest.cpp +++ b/test/LinkManagerStateMachineActiveActiveTest.cpp @@ -363,6 +363,70 @@ TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActiveLinkDownMuxUnknownFirsr VALIDATE_STATE(Unknown, Unknown, Down); } +TEST_F(LinkManagerStateMachineActiveActiveTest, HwProberOffloadUpBeforeLinkUpRecoversToActive) +{ + // Mark this port as using the hardware (ICMP-offload) link prober before + // the state machine is activated so that the gating in + // ActiveActiveStateMachine sees the correct prober type. + mFakeMuxPort.setLinkProberType(common::MuxPortConfig::LinkProberType::Hardware); + + setMuxActive(); + VALIDATE_STATE(Active, Active, Up); + + // 1) Server/switch port goes oper-down. The state machine drives mux to Standby + postLinkEvent(link_state::LinkState::Label::Down, 2); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Standby); + VALIDATE_STATE(Active, Standby, Down); + + // 2) xcvrd / ycabled gRPC becomes unreachable for this port; mux drifts to + // Unknown -> (P: Active, M: Unknown, L: Down). The hardware prober is + // still reporting the session as Active. + handleMuxState("unknown", 3); + VALIDATE_STATE(Active, Unknown, Down); + + uint32_t probeForwardingStateBefore = mDbInterfacePtr->mProbeForwardingStateInvokeCount; + + // 3) Kernel link comes back up. Without the fix, prober would be reset to + // Wait and the FSM would stop. With the fix, prober stays Active and a + // fresh mux probe is fired. + // + // count=3: iteration-2 of the link-state retry loop consumes three slots: + // run1 – link-state machine processes 2nd UpEvent (Down→Up transition, posts B) + // run2 – link-manager handleStateChange(Up) runs, calls startMuxProbeTimer() + // which posts handler C (handleProbeForwardingState) to the DbInterface strand + // run3 – DbInterface strand executes C, incrementing mProbeForwardingStateInvokeCount + postLinkEvent(link_state::LinkState::Label::Up, 3); + VALIDATE_STATE(Active, Unknown, Up); + EXPECT_GT(mDbInterfacePtr->mProbeForwardingStateInvokeCount, probeForwardingStateBefore); + + // 4) xcvrd finally answers the probe with Active -> recovery completes and + // orchagent is told to switch the mux back to Active. + handleProbeMuxState("active", 4); + VALIDATE_STATE(Active, Active, Up); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Active); +} + +TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActiveLinkProberActiveBeforeLinkUp) +{ + // When the prober is already Active and the mux is already Active at the time + // the link recovers, no redundant mux switch should be issued. + setMuxActive(); + VALIDATE_STATE(Active, Active, Up); + + postLinkEvent(link_state::LinkState::Label::Down, 2); + EXPECT_EQ(mDbInterfacePtr->mLastSetMuxState, mux_state::MuxState::Label::Standby); + VALIDATE_STATE(Active, Standby, Down); + + handleMuxState("active", 3); + VALIDATE_STATE(Active, Active, Down); + + uint32_t setMuxStateBefore = mDbInterfacePtr->mSetMuxStateInvokeCount; + + postLinkEvent(link_state::LinkState::Label::Up, 2); + VALIDATE_STATE(Active, Active, Up); + EXPECT_EQ(mDbInterfacePtr->mSetMuxStateInvokeCount, setMuxStateBefore); +} + TEST_F(LinkManagerStateMachineActiveActiveTest, MuxActiveConfigStandby) { setMuxActive();