Skip to content

Added checks to only enable HA set actor state update after HA set table entry is programmed#205

Merged
zjswhhh merged 1 commit into
sonic-net:masterfrom
BYGX-wcr:add-check-for-haset-programming-status
Jul 14, 2026
Merged

Added checks to only enable HA set actor state update after HA set table entry is programmed#205
zjswhhh merged 1 commit into
sonic-net:masterfrom
BYGX-wcr:add-check-for-haset-programming-status

Conversation

@BYGX-wcr

@BYGX-wcr BYGX-wcr commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description of PR

Summary:
Ensure the DPU's DASH_HA_SCOPE_TABLE entry is never programmed before the
DASH_HA_SET_TABLE entry it depends on. The HA set actor now only advertises its
HaSetActorState (the signal every HA scope actor gates on) after it has
programmed the DASH_HA_SET_TABLE entry, so a dependent HA scope actor cannot drive
its state machine — and program DASH_HA_SCOPE_TABLE (including the initial dead
role) — until the HA set entry exists.

Fixes # (issue) sonic-net/sonic-buildimage#28073

Background / where to start reviewing:

Dependencies: none.

Type of change

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

Approach

What is the motivation for this PR?

During a DPU reboot test on a SmartSwitch testbed, the initial DASH_HA_SCOPE_TABLE entry (with the dead role) was
observed being programmed before the initial DASH_HA_SET_TABLE entry that it
references. The DPU expects the HA set entry to exist before the HA scope entry, so
this ordering is incorrect.

Root cause: the HA scope actor only needs the in-memory HaSetActorState message to
drive its state machine and program DASH_HA_SCOPE_TABLE — it does not check whether
the HA set actor has actually written DASH_HA_SET_TABLE to the DPU. That
HaSetActorState could be delivered via the registration-reply path
(HaSetActor::handle_haset_state_registration), which replies with the cached state
without ever programming DASH_HA_SET_TABLE. As a result the HA scope actor could
program its (dead-role) scope entry before the HA set entry was written. The two
tables are also written by two independent actors through two independent
ConsumerBridge instances, so there is no inherent ordering guarantee between them.

How did you do it?

  • Added a dash_ha_set_programmed: bool field to HaSetActor.
  • Set it to true in update_dash_ha_set_table immediately after the
    DASH_HA_SET_TABLE write is issued to the DPU (and before the HaSetActorState is
    emitted to registered HA scope actors). Reset it to false in
    delete_dash_ha_set_table so scopes wait again if the entry is removed.
  • In handle_haset_state_registration, defer the HaSetActorState reply until
    dash_ha_set_programmed is true. A registration that arrives before programming
    no longer hands the HA scope actor a state it could act on prematurely; the HA scope
    is instead notified later (proactively) via update_dash_ha_set_table once the entry
    is programmed. Because the registration record persists in the HA set actor's
    incoming state, no re-registration/retry is required from the scope.
  • The NPU-driven HA scope state machine already early-returns in
    drive_npu_state_machine when get_haset returns None. Added a comment there
    documenting that the presence of HaSetActorState now means "the HA set entry is
    programmed," so the machine only functions after the ack.

Note: this change lives in the shared HA set actor, so it applies consistently to both
the NPU-driven and DPU-driven HA scope actors (both gate on receiving
HaSetActorState). No behavioral regression for DPU-driven mode — it gets the same
ordering guarantee.

Known limitation: "programmed" here means the HA set actor has issued the
producer-bridge write, not that it has been confirmed applied in the DPU APPL_DB. The
actor framework does not surface producer-bridge write acks back into actor logic, so a
true end-to-end APPL_DB write confirmation would require additional plumbing in
swbus-actor. This change removes the actual race that was hit (the registration-reply
path, which programmed nothing) and orders the two writes correctly in the normal path.

How did you verify/test it?

  • Ran the full hamgrd unit test suite: cargo test -p hamgrd — 41 tests pass,
    including all ha_set and NPU/DPU-driven ha_scope tests.
  • cargo fmt and cargo clippy -p hamgrd --all-targets are clean (no new warnings).
  • Built the Debian package and deployed to both testbed switches
    across all dash-hadpu0..3 containers, followed
    by config reload, to validate the corrected ordering during the DPU reboot test.

Any platform specific information?

SmartSwitch / DASH HA only. Affects hamgrd behavior on the switch NPU and the DPU
DASH_HA_SET_TABLE / DASH_HA_SCOPE_TABLE programming order.

Documentation

No documentation/Wiki changes required. Behavior is internal to hamgrd actor
sequencing and does not change any external API or schema.

…ble entry is programmed

Signed-off-by: BYGX-wcr <wcr@live.cn>
@mssonicbld

Copy link
Copy Markdown

/azp run

@azure-pipelines

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

@zjswhhh

zjswhhh commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@yue-fred-gao @dypet - please help review.

@zjswhhh
zjswhhh requested review from dypet and yue-fred-gao July 8, 2026 01:25
@BYGX-wcr
BYGX-wcr marked this pull request as ready for review July 8, 2026 17:35
Copilot AI review requested due to automatic review settings July 8, 2026 17:35

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 tightens HA actor sequencing in hamgrd to prevent a dependent HA scope actor from programming DASH_HA_SCOPE_TABLE before the corresponding DASH_HA_SET_TABLE entry exists on the DPU. It does this by ensuring the HA set actor only advertises HaSetActorState (the gating signal consumed by HA scope actors) after it has issued the DASH_HA_SET_TABLE write.

Changes:

  • Added dash_ha_set_programmed to HaSetActor and gate HaSetActorState registration replies until the HA set table write has been issued.
  • Set/reset the flag in the HA set table update/delete paths to re-establish gating when the entry is removed.
  • Documented in the NPU HA scope state machine that HaSetActorState now serves as the “HA set programmed” ack for safely driving the scope state machine.

Reviewed changes

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

File Description
crates/hamgrd/src/actors/ha_set.rs Adds “programmed” gating so HaSetActorState is only emitted/replied after issuing the DASH_HA_SET_TABLE write, preventing premature scope programming.
crates/hamgrd/src/actors/ha_scope/npu.rs Adds clarifying comment tying scope state-machine gating to the HA set actor’s post-programming HaSetActorState ack.

Comment on lines +896 to +904
// Only reply with the HaSetActorState ack once the DASH_HA_SET_TABLE entry has
// actually been programmed. A registration arriving before programming must not
// hand the HA scope actor an ha-set state it could act on prematurely (which would
// let it program DASH_HA_SCOPE_TABLE before the HA set entry exists). The HA scope
// will receive the state later via update_dash_ha_set_table once it is programmed.
if !self.dash_ha_set_programmed {
debug!("DASH_HA_SET_TABLE not programmed yet. Defer HaSetActorState reply to registration");
return Ok(());
}
@zjswhhh
zjswhhh merged commit 1aa2ea8 into sonic-net:master Jul 14, 2026
4 checks passed
@BYGX-wcr
BYGX-wcr deleted the add-check-for-haset-programming-status branch July 14, 2026 17:47
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