Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zjswhhh merged 1 commit into from
Jul 14, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/hamgrd/src/actors/ha_scope/npu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1500,6 +1500,11 @@ impl NpuHaScopeActor {
}

let ha_set_id = self.base.get_haset_id().unwrap_or_default();
// Do not drive the state machine (which programs DASH_HA_SCOPE_TABLE, including the
// initial dead role) until the HA set actor has acked that DASH_HA_SET_TABLE is
// programmed. That ack is the HaSetActorState message: the HA set actor only emits it
// after it has issued the DASH_HA_SET_TABLE write, so its presence here guarantees the
// HA set entry is programmed before we program any dependent HA scope entry.
let Some(_haset) = self.base.get_haset(state.incoming()) else {
info!("HA-SET {} has not been initialized yet", &ha_set_id);
return Ok(());
Expand Down
37 changes: 35 additions & 2 deletions crates/hamgrd/src/actors/ha_set.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ pub struct HaSetActor {
bridges: Vec<ConsumerBridge>,
bfd_session_npu_ips: HashSet<String>,
ha_scope_states: HashMap<String, CachedHaScopeState>,
/// Whether this actor has programmed the DASH_HA_SET_TABLE entry into the DPU.
/// The HaSetActorState message (which HA scope actors depend on to drive their
/// state machine) is only emitted once this is true, so that a dependent HA
/// scope never programs DASH_HA_SCOPE_TABLE before the HA set entry exists.
dash_ha_set_programmed: bool,
}

impl DbBasedActor for HaSetActor {
Expand All @@ -46,6 +51,7 @@ impl DbBasedActor for HaSetActor {
bridges: Vec::new(),
bfd_session_npu_ips: HashSet::new(),
ha_scope_states: HashMap::new(),
dash_ha_set_programmed: false,
};
Ok(actor)
}
Expand Down Expand Up @@ -167,7 +173,7 @@ impl HaSetActor {
}

fn update_dash_ha_set_table(
&self,
&mut self,
vdpus: &[VDpuStateExt],
incoming: &Incoming,
outgoing: &mut Outgoing,
Expand All @@ -185,6 +191,11 @@ impl HaSetActor {
let msg = ActorMessage::new(self.id.clone(), &kfv)?;
outgoing.send(outgoing.common_bridge_sp::<DashHaSetTable>(), msg);

// The DASH_HA_SET_TABLE write has now been issued to the DPU. From this point on the
// HaSetActorState message we emit below serves as the ack that dependent HA scope
// actors wait on before driving their state machine / programming DASH_HA_SCOPE_TABLE.
self.dash_ha_set_programmed = true;

let vdpu_ids = self
.dash_ha_set_config
.as_ref()
Expand All @@ -210,7 +221,7 @@ impl HaSetActor {
Ok(())
}

fn delete_dash_ha_set_table(&self, vdpus: &[VDpuStateExt], outgoing: &mut Outgoing) -> Result<()> {
fn delete_dash_ha_set_table(&mut self, vdpus: &[VDpuStateExt], outgoing: &mut Outgoing) -> Result<()> {
if !vdpus.iter().any(|vdpu_ext| vdpu_ext.vdpu.dpu.is_managed) {
debug!("None of DPUs is managed by local HAMGRD. Skip dash_ha_set deletion");
return Ok(());
Expand All @@ -225,6 +236,10 @@ impl HaSetActor {
let msg = ActorMessage::new(self.id.clone(), &kfv)?;
outgoing.send(outgoing.common_bridge_sp::<DashHaSetTable>(), msg);

// The DASH_HA_SET_TABLE entry is being removed; dependent HA scope actors must again
// wait for it to be reprogrammed before acting.
self.dash_ha_set_programmed = false;

Ok(())
}

Expand Down Expand Up @@ -878,6 +893,15 @@ impl HaSetActor {
.ok_or_else(|| anyhow!("Entry not found for key: {}", key))?;
let ActorRegistration { active, .. } = entry.msg.deserialize_data()?;
if active {
// 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(());
}
Comment on lines +896 to +904
let Some(vdpus) = self.get_vdpus_if_ready(incoming) else {
return Ok(());
};
Expand Down Expand Up @@ -1096,6 +1120,7 @@ mod test {
bridges: Vec::new(),
bfd_session_npu_ips: HashSet::new(),
ha_scope_states,
dash_ha_set_programmed: false,
};

assert_eq!(
Expand Down Expand Up @@ -1188,6 +1213,7 @@ mod test {
bridges: Vec::new(),
bfd_session_npu_ips: HashSet::new(),
ha_scope_states: HashMap::new(),
dash_ha_set_programmed: false,
};

let handle = runtime.spawn(ha_set_actor, HaSetActor::name(), &ha_set_id);
Expand Down Expand Up @@ -1396,6 +1422,7 @@ mod test {
bridges: Vec::new(),
bfd_session_npu_ips: HashSet::new(),
ha_scope_states: HashMap::new(),
dash_ha_set_programmed: false,
};

let handle = runtime.spawn(ha_set_actor, HaSetActor::name(), &ha_set_id);
Expand Down Expand Up @@ -1590,6 +1617,7 @@ mod test {
bridges: Vec::new(),
bfd_session_npu_ips: HashSet::new(),
ha_scope_states: HashMap::new(),
dash_ha_set_programmed: false,
};

let handle = runtime.spawn(ha_set_actor, HaSetActor::name(), &ha_set_id);
Expand Down Expand Up @@ -1749,6 +1777,7 @@ mod test {
bridges: Vec::new(),
bfd_session_npu_ips: HashSet::new(),
ha_scope_states: HashMap::new(),
dash_ha_set_programmed: false,
};

let handle = runtime.spawn(ha_set_actor, HaSetActor::name(), &ha_set_id);
Expand Down Expand Up @@ -1884,6 +1913,7 @@ mod test {
bridges: Vec::new(),
bfd_session_npu_ips: HashSet::new(),
ha_scope_states: HashMap::new(),
dash_ha_set_programmed: false,
};

let handle = runtime.spawn(ha_set_actor, HaSetActor::name(), &ha_set_id);
Expand Down Expand Up @@ -1983,6 +2013,7 @@ mod test {
bfd_session_npu_ips: HashSet::new(),
ha_scope_states: HashMap::new(),
ha_owner: HaOwner::Dpu,
dash_ha_set_programmed: false,
};

let handle = runtime.spawn(ha_set_actor, HaSetActor::name(), &ha_set_id);
Expand Down Expand Up @@ -2092,6 +2123,7 @@ mod test {
bridges: Vec::new(),
bfd_session_npu_ips: HashSet::new(),
ha_scope_states: HashMap::new(),
dash_ha_set_programmed: false,
};

let handle = runtime.spawn(ha_set_actor, HaSetActor::name(), &ha_set_id);
Expand Down Expand Up @@ -2218,6 +2250,7 @@ mod test {
bridges: Vec::new(),
bfd_session_npu_ips: HashSet::new(),
ha_scope_states: HashMap::new(),
dash_ha_set_programmed: false,
};

let handle = runtime.spawn(ha_set_actor, HaSetActor::name(), &ha_set_id);
Expand Down
Loading