From 6dc13af12c660930d5e6bcb225f3538d20d8b7aa Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Mon, 6 Jul 2026 20:32:14 +0000 Subject: [PATCH] [NPU-driven HA] Ack standalone before peer activates dead role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Description of PR Summary: During a planned shutdown of a standby HA scope, the active side previously gated its transition from `Active` to `Standalone` on the standby peer entering the `Dead` role. This ordering is incorrect: the active must ack the standalone role **first** so that it owns all traffic before the standby activates its dead role. Otherwise there is a window where the active is still in `SwitchingToStandalone` while the standby has already dropped out, leaving traffic unowned / inline sync unacked. This PR flips the ordering so the active acks standalone first, and the standby only activates its dead role after observing the active's standalone ack. Fixes # (N/A) ### Type of change - [x] Bug fix - [ ] New feature - [ ] Refactor / cleanup - [ ] Documentation update - [ ] Test improvement ### Approach #### What is the motivation for this PR? Correct the NPU-driven HA planned-shutdown state-transition ordering to avoid a traffic-ownership gap between the active and standby HA scopes. #### How did you do it? - **Active side** (`apply_pending_state_side_effects`, `SwitchingToStandalone`): on `PeerShutdownRequested`, enter `Standalone` immediately instead of waiting for the peer to become `Dead`. - **Standby side** (`next_state`): gate the transition to `Destroying` (dead-role activation) on the active peer having acked the `standalone` role — both in the forced `Shutdown` path and via a new `Standby` branch that fires when the peer's acked ASIC role becomes `standalone` during a planned shutdown (`desired_ha_state == Dead`). #### How did you verify/test it? - Updated the affected NPU-driven unit tests (`ha_scope_npu_launch_to_standby_then_down`, `ha_scope_npu_active_to_standalone_on_peer_shutdown_and_peer_rejoin`, `ha_scope_npu_standby_planned_shutdown_and_relaunch_to_standby`) to reflect the new ordering. - `cargo test -p hamgrd --bin hamgrd actors::ha_scope` — all 13 tests pass. - `cargo fmt -- --check` clean; `cargo clippy -p hamgrd` reports no new warnings. #### Any platform specific information? NPU-driven HA only. DPU-driven mode is unaffected. ### Documentation N/A — behavior fix aligned with the SmartSwitch HA HLD planned-shutdown flow. Signed-off-by: Sonic Build Admin --- crates/hamgrd/src/actors/ha_scope/mod.rs | 15 ++++++--- crates/hamgrd/src/actors/ha_scope/npu.rs | 42 +++++++++++++++++------- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/crates/hamgrd/src/actors/ha_scope/mod.rs b/crates/hamgrd/src/actors/ha_scope/mod.rs index bd205ad0..a7ffa4b4 100644 --- a/crates/hamgrd/src/actors/ha_scope/mod.rs +++ b/crates/hamgrd/src/actors/ha_scope/mod.rs @@ -1050,6 +1050,10 @@ mod test { // Mock the peer's ShutdownReply with "accepted" send! { key: ShutdownReply::msg_key(&peer_scope_id), data: { "response": "accepted" } }, + // The standby node must NOT activate its dead role yet. It waits until the active + // peer acks the standalone role. Mock the active peer acking the standalone role. + send! { key: HaScopeActorState::msg_key(&peer_scope_id), data: { "timestamp": 0, "owner": 0, "new_state": HaState::Standalone.as_str_name(), "term": "1", "vdpu_id": "", "peer_vdpu_id": "", "acked_asic_ha_state": "standalone" } }, + // Expect a DPU DASH_HA_SCOPE_TABLE update (Destroying side effect) recv! { key: &ha_set_id, data: { "key": &ha_set_id, @@ -1241,10 +1245,9 @@ mod test { recv! { key: HaScopeActorState::msg_key(&scope_id), data: { "owner": HaOwner::Switch as i32, "new_state": HaState::SwitchingToStandalone.as_str_name(), "term": "1", "vdpu_id": &vdpu0_id, "peer_vdpu_id": &vdpu1_id }, addr: runtime.sp(HaScopeActor::name(), &peer_scope_id), exclude: "timestamp" }, recv! { key: HaScopeActorState::msg_key(&scope_id), data: { "owner": HaOwner::Switch as i32, "new_state": HaState::SwitchingToStandalone.as_str_name(), "term": "1", "vdpu_id": &vdpu0_id, "peer_vdpu_id": &vdpu1_id }, addr: runtime.sp(HaSetActor::name(), &ha_set_id), exclude: "timestamp" }, - // The active node waits in SwitchingToStandalone until the peer's ASIC acks the dead role. - // Mock the peer completing its shutdown by broadcasting Dead with acked_asic_ha_state="dead". - send! { key: HaScopeActorState::msg_key(&peer_scope_id), data: { "timestamp": 0, "owner": 0, "new_state": HaState::Dead.as_str_name(), "term": "1", "vdpu_id": "", "peer_vdpu_id": "", "acked_asic_ha_state": "dead" } }, - + // The active node acks the standalone role first, without waiting for the peer to + // become Dead. The peer (standby) will only activate its dead role after observing + // this standalone ack. // EnterStandalone self-notification is then processed: // Expect DPU DASH_HA_SCOPE_TABLE update with standalone role and incremented term recv! { key: &ha_set_id, data: { @@ -3245,6 +3248,10 @@ mod test { // Mock the peer's ShutdownReply with "accepted" send! { key: ShutdownReply::msg_key(&peer_scope_id), data: { "response": "accepted" } }, + // The standby node must NOT activate its dead role yet. It waits until the active + // peer acks the standalone role. Mock the active peer acking the standalone role. + send! { key: HaScopeActorState::msg_key(&peer_scope_id), data: { "timestamp": 0, "owner": 0, "new_state": HaState::Standalone.as_str_name(), "term": "1", "vdpu_id": "", "peer_vdpu_id": "", "acked_asic_ha_state": "standalone" } }, + // Expect a DPU DASH_HA_SCOPE_TABLE update (Destroying side effect) recv! { key: &ha_set_id, data: { "key": &ha_set_id, diff --git a/crates/hamgrd/src/actors/ha_scope/npu.rs b/crates/hamgrd/src/actors/ha_scope/npu.rs index 9dd124aa..21a45e7f 100644 --- a/crates/hamgrd/src/actors/ha_scope/npu.rs +++ b/crates/hamgrd/src/actors/ha_scope/npu.rs @@ -1403,17 +1403,13 @@ impl NpuHaScopeActor { // Peer DPU lost self.send_self_notification(state, "EnterStandalone", 0)?; } else if *event == HaEvent::PeerShutdownRequested { - if self.current_npu_peer_acked_asic_ha_state(state.internal()) - == ha_role_to_string(HaRole::Dead.as_str_name()) - && self.current_npu_peer_ha_state(state.internal()) == HaState::Dead - { - // Peer DPU planned shutdown and already dead — enter standalone - self.send_self_notification(state, "EnterStandalone", 0)?; - } - // Else wait for peer to become Dead - } else if *event == HaEvent::PeerStateChanged - && self.current_npu_peer_acked_asic_ha_state(state.internal()) - == ha_role_to_string(HaRole::Dead.as_str_name()) + // Peer requested a planned shutdown. Enter standalone immediately so that + // we ack the standalone role first. The peer (standby) will only activate + // its dead role after observing our standalone ack, avoiding a window where + // inline sync is unacked + self.send_self_notification(state, "EnterStandalone", 0)?; + } else if self.current_npu_peer_acked_asic_ha_state(state.internal()) + == ha_role_to_string(HaRole::Dead.as_str_name()) && self.current_npu_peer_ha_state(state.internal()) == HaState::Dead { // Peer DPU forced shutdown — enter standalone @@ -1616,6 +1612,18 @@ impl NpuHaScopeActor { None } } + HaState::Standby => { + // Planned shutdown was accepted by the active peer. We must not activate the + // dead role until the active peer has acked the standalone role and thus owns + // all traffic. If it already has, proceed; otherwise wait for its state update. + if self.current_npu_peer_acked_asic_ha_state(state.internal()) + == ha_role_to_string(HaRole::Standalone.as_str_name()) + { + Some((HaState::Destroying, "planned shutdown, peer acked standalone")) + } else { + None + } + } _ => Some((HaState::Destroying, "planned shutdown")), }; } else if *event == HaEvent::EnterStandalone { @@ -1742,6 +1750,18 @@ impl NpuHaScopeActor { Some((HaState::SwitchingToStandalone, "local DPU failure")) } else if *event == HaEvent::PeerLost { Some((HaState::SwitchingToStandalone, "peer failure while standby")) + } else if self + .base + .dash_ha_scope_config + .as_ref() + .map(|c| c.desired_ha_state == DesiredHaState::Dead as i32) + .unwrap_or(false) + && self.current_npu_peer_acked_asic_ha_state(state.internal()) + == ha_role_to_string(HaRole::Standalone.as_str_name()) + { + // Planned shutdown: the active peer has acked the standalone role and now + // owns all traffic, so it is safe to activate our dead role. + Some((HaState::Destroying, "planned shutdown, peer acked standalone")) } else if self.current_npu_peer_ha_state(state.internal()) == HaState::Dead { Some((HaState::SwitchingToStandalone, "peer went down")) } else {