Skip to content

hamgrd: gate Active to SwitchingToStandby transition on peer DPU role - #212

Merged
vivekrnv merged 1 commit into
sonic-net:masterfrom
BYGX-wcr:fix/gate-active-to-standby-on-peer-role
Aug 7, 2026
Merged

hamgrd: gate Active to SwitchingToStandby transition on peer DPU role#212
vivekrnv merged 1 commit into
sonic-net:masterfrom
BYGX-wcr:fix/gate-active-to-standby-on-peer-role

Conversation

@BYGX-wcr

@BYGX-wcr BYGX-wcr commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Description of PR

Summary:
Gate the NPU-driven Active to SwitchingToStandby transition until the peer DPU has entered the switching_to_active role. This prevents the active DPU from demoting before the peer is ready and supports either message arrival order.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation update
  • Test improvement

Approach

What is the motivation for this PR?

A SwitchoverRequest could previously move the active HA scope to SwitchingToStandby before the peer DPU acknowledged switching_to_active.

How did you do it?

Require an accepted in-progress switchover, peer SwitchingToActive state, and peer ASIC switching_to_active acknowledgement. Allow either the request or peer state update to complete the gate. Add HA scope unit coverage in ha_scope/mod.rs for both message orderings.

How did you verify/test it?

  • cargo test -p hamgrd (44 passed)
  • cargo fmt --check --all
  • git diff --check

Any platform specific information?

Applies to NPU-driven planned switchover.

Documentation

No documentation changes are required for this behavior fix.

Copilot AI lite review requested due to automatic review settings August 4, 2026 16:29
@mssonicbld

Copy link
Copy Markdown

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@BYGX-wcr BYGX-wcr changed the title hamgrd: gate standby transition on peer DPU role hamgrd: gate Active to SwitchingToStandby transition on peer DPU role Aug 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates hamgrd’s NPU-driven HA scope state machine to prevent the active side from demoting to SwitchingToStandby until the peer DPU is confirmed ready (peer role is SwitchingToActive and the peer ASIC has acknowledged that role), supporting either message arrival order.

Changes:

  • Add a shared gate helper to determine when the peer is ready for the active→SwitchingToStandby transition.
  • Update the HaState::Active transition logic to use the new gate (requires accepted/in-progress switchover + peer role + peer ASIC ack).
  • Add unit tests covering both message orderings (request-first vs peer-state-first).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/hamgrd/src/actors/ha_scope/npu.rs Gates Active→SwitchingToStandby transition on peer DPU role + ASIC ack, allowing either event ordering.
crates/hamgrd/src/actors/ha_scope/mod.rs Adds unit tests for the new gate helper covering both message arrival orderings.

Comment thread crates/hamgrd/src/actors/ha_scope/npu.rs Outdated
self.current_npu_peer_ha_state(state.internal()),
&self.current_npu_peer_acked_asic_ha_state(state.internal()),
) {
Some((HaState::SwitchingToStandby, "peer DPU entered switching_to_active role"))
Signed-off-by: BYGX-wcr <wcr@live.cn>
Copilot AI review requested due to automatic review settings August 4, 2026 18:57
@BYGX-wcr
BYGX-wcr force-pushed the fix/gate-active-to-standby-on-peer-role branch from 256990e to ca2e396 Compare August 4, 2026 18:57
@mssonicbld

Copy link
Copy Markdown

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

crates/hamgrd/src/actors/ha_scope/npu.rs:36

  • ha_role_to_string(HaRole::SwitchingToActive.as_str_name()) allocates a new String on every gate check even though the expected value is a fixed, lowercase role string in the HA-scope state (same format as other acked_asic_ha_state comparisons). This adds avoidable per-event overhead in the state-machine hot path.
    matches!(event, HaEvent::SwitchoverRequested | HaEvent::PeerStateChanged)
        && switchover_state == Some("in_progress")
        && peer_ha_state == HaState::SwitchingToActive
        && peer_acked_asic_ha_state == ha_role_to_string(HaRole::SwitchingToActive.as_str_name())

crates/hamgrd/src/actors/ha_scope/npu.rs:1731

  • In the HaState::Active arm, get_npu_ha_scope_state() is now called (and deserializes the full NPU HA scope state) for every event, even for events like PeerLost/LocalFailure that will immediately transition without needing the NPU snapshot. This is an avoidable performance regression compared to the prior structure where deserialization was skipped for early-exit events.
                let npu_state = self.base.get_npu_ha_scope_state(state.internal());
                let switchover_state = npu_state.as_ref().and_then(|scope| scope.switchover_state.as_deref());
                let peer_ha_state = npu_state
                    .as_ref()
                    .and_then(|scope| scope.peer_ha_state.as_deref())

@vivekrnv
vivekrnv merged commit 11a3522 into sonic-net:master Aug 7, 2026
4 checks passed
@BYGX-wcr
BYGX-wcr deleted the fix/gate-active-to-standby-on-peer-role branch August 7, 2026 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants