diff --git a/crates/ziggurat-driver/src/zigbee_stack/joining.rs b/crates/ziggurat-driver/src/zigbee_stack/joining.rs index 19ef6e0..f35450e 100644 --- a/crates/ziggurat-driver/src/zigbee_stack/joining.rs +++ b/crates/ziggurat-driver/src/zigbee_stack/joining.rs @@ -1122,6 +1122,18 @@ impl ZigbeeStack { return; }; + // Spec 4.7.3.6: an unsecured rejoin re-delivers the network key encrypted + // with the well-known key unless the device holds a unique link key. Reject + // when it has neither, so the network key is never exposed to anyone who + // knows the well-known key. Nobody authenticated this frame, so it is rejected + // before it changes any state. + if !secured && !self.may_rejoin_unsecured(source_ieee) { + tracing::warn!( + "Rejecting unsecured rejoin from {source_ieee:?} without a unique link key" + ); + return; + } + let requested_nwk = nwk_frame.nwk_header.source; let capability = &rejoin_request.capability_information; @@ -1211,16 +1223,6 @@ impl ZigbeeStack { // A trust center rejoin means the device no longer has the network key if !secured { - // Spec 4.7.3.6: an unsecured rejoin re-delivers the network key encrypted - // with the well-known key unless the device holds a unique link key. Reject - // when it has neither, so the network key is never exposed to anyone who - // knows the well-known key. - if !self.may_rejoin_unsecured(source_ieee) { - tracing::warn!( - "Rejecting unsecured rejoin from {source_ieee:?} without a unique link key" - ); - return; - } // The join is announced only once the key transport lands let stack = self .self_weak @@ -1305,7 +1307,7 @@ impl ZigbeeStack { // The address map entry and any negotiated link key are kept around so that the // device can rejoin later self.drop_indirect_transactions(source_ieee, source); - self.core().nib.routing.remove_route(source); + self.forget_nwk_state(source); self.push_notification(ZigbeeNotification::DeviceLeft { nwk: source, diff --git a/crates/ziggurat-driver/src/zigbee_stack/nwk.rs b/crates/ziggurat-driver/src/zigbee_stack/nwk.rs index 5bea6ad..36c5503 100644 --- a/crates/ziggurat-driver/src/zigbee_stack/nwk.rs +++ b/crates/ziggurat-driver/src/zigbee_stack/nwk.rs @@ -32,12 +32,33 @@ use super::{ }; impl ZigbeeStack { - pub fn update_nwk_eui64_mapping(&self, nwk: Nwk, eui64: Eui64) { - let conflict = self.core().nib.address_map.update_mapping(eui64, nwk); + /// A device joined or rejoined as `nwk`: the address starts a new session. The + /// mapping and the state it replaces change under one lock, so no send can be + /// enqueued against the old session in between. + pub(super) fn update_nwk_eui64_mapping(&self, nwk: Nwk, eui64: Eui64) { + let mut core = self.core(); - if conflict { + // `nwk` belongs to another device, whose state must stay + if core.nib.address_map.update_mapping(eui64, nwk) { + drop(core); self.handle_address_conflict(nwk, AddrConflictSource::Local); + return; } + + // The device can be reached again, so sends held for it are kept. The route + // request they wait on went out before it joined. With that discovery gone, the + // timer pass starts a new one. + core.nib.routing.forget(self.state.network_address, nwk); + self.commit_sends(core, SendActions::new()); + } + + /// The device at `nwk` left, so what we know about the address belongs to a session + /// that is over. Held sends fail: nothing will answer a route request for it. + pub(super) fn forget_nwk_state(&self, nwk: Nwk) { + let mut core = self.core(); + core.nib.routing.forget(self.state.network_address, nwk); + let actions = core.sends.fail_unrouted(nwk); + self.commit_sends(core, actions); } /// Filter broadcast frames based on the NWK broadcast transaction table @@ -89,8 +110,8 @@ impl ZigbeeStack { nwk_frame: NwkFrame, attempts: u8, initial_delay: Duration, - ) -> SendActions { - let (_, actions) = self.enqueue_broadcast_in( + ) -> (SendId, SendActions) { + let (handle, actions) = self.enqueue_broadcast_in( core, Frame::nwk(nwk_frame, NwkSecurityMode::NetworkKey), TxPolicy { @@ -104,7 +125,7 @@ impl ZigbeeStack { }, ); - actions + (handle.id(), actions) } /// A random retransmission jitter in `[0, max_broadcast_jitter)` (spec 3.6.6). @@ -153,9 +174,19 @@ impl ZigbeeStack { ); } - // Update the address cache + // Update the address cache. Hearing a device is not a join, so the address keeps + // its session state. if let Some(src_eui64) = nwk_frame.nwk_header.source_ieee { - self.update_nwk_eui64_mapping(nwk_frame.nwk_header.source, src_eui64); + let source = nwk_frame.nwk_header.source; + let conflict = self + .core() + .nib + .address_map + .update_mapping(src_eui64, source); + + if conflict { + self.handle_address_conflict(source, AddrConflictSource::Local); + } } // Spec 3.6.1.10.2: a frame addressed to our network address but a different diff --git a/crates/ziggurat-driver/src/zigbee_stack/route.rs b/crates/ziggurat-driver/src/zigbee_stack/route.rs index bba6670..9904657 100644 --- a/crates/ziggurat-driver/src/zigbee_stack/route.rs +++ b/crates/ziggurat-driver/src/zigbee_stack/route.rs @@ -112,6 +112,13 @@ impl ZigbeeStack { let network_address = self.state.network_address; let many_to_one = route_request_cmd.many_to_one != NwkRouteRequestManyToOne::NotManyToOne; + // Neighbors relay our own route requests back to us. Our discovery entry may + // already be gone, so it cannot be what filters them. + if nwk_frame.nwk_header.source == network_address { + tracing::debug!("Ignoring our own route request relayed by {sender_nwk:?}"); + return; + } + // We need to know who sent the frame let sender_link = self.core().nib.neighbors.link(sender_nwk); @@ -251,7 +258,7 @@ impl ZigbeeStack { let attempts = self.tunables.rreq_retries() + 1; let mut core = self.core(); - let actions = + let (_, actions) = self.broadcast_route_request(&mut core, relayed_route_request_cmd, attempts, jitter); self.commit_sends(core, actions); } @@ -474,11 +481,14 @@ impl ZigbeeStack { // Spec 3.6.4.5.1: the initial broadcast is repeated `nwkcInitialRREQRetries` // times, separated by the retry interval - self.broadcast_route_request( + let (request, mut actions) = self.broadcast_route_request( core, route_request_frame, self.tunables.initial_rreq_retries() + 1, Duration::ZERO, - ) + ); + actions.extend(core.sends.route_request_sent(destination, request)); + + actions } } diff --git a/crates/ziggurat-zigbee/src/nwk/routing.rs b/crates/ziggurat-zigbee/src/nwk/routing.rs index a34a79a..f3b294f 100644 --- a/crates/ziggurat-zigbee/src/nwk/routing.rs +++ b/crates/ziggurat-zigbee/src/nwk/routing.rs @@ -284,6 +284,22 @@ impl Routing { self.route_table.remove(&destination).is_some() } + /// The device at `nwk` left or joined again, clear associated state. + pub fn forget(&mut self, own_address: Nwk, nwk: Nwk) { + // TODO: a discovery toward `nwk` that we relay for another router loses its + // route entry here, so `accept_route_reply` drops the reply as coming from an + // unknown responder. That router gets no route through us for that discovery. + self.route_table.remove(&nwk); + self.route_record_table.remove(&nwk); + + // Only our own discoveries toward it are dropped. Discoveries we relay stay + // until they expire: they are the only duplicate check for late copies of + // their route requests. + self.discovery_table.retain(|_, entry| { + entry.source_address != own_address || entry.destination_address != nwk + }); + } + /// Seed an active routing entry restored by the client on startup. Unlike a route /// learned at runtime, this does not record a change (the client already has it). pub fn restore_route(&mut self, destination: Nwk, next_hop: Nwk, path_cost: u8) { diff --git a/crates/ziggurat-zigbee/src/sends/broadcasts.rs b/crates/ziggurat-zigbee/src/sends/broadcasts.rs index a9bd4e3..3945e59 100644 --- a/crates/ziggurat-zigbee/src/sends/broadcasts.rs +++ b/crates/ziggurat-zigbee/src/sends/broadcasts.rs @@ -306,6 +306,34 @@ impl> Broadcasts { Some(actions) } + /// Send no further copies of a broadcast nobody waits on any more. A copy in the + /// radio right now cannot be recalled: its schedule is emptied instead, so the + /// record settles when it next comes due (a `Once` schedule settles on the copy's + /// own report). + pub(super) fn stop(&mut self, id: SendId) -> Actions { + let mut actions = Actions::new(); + + let Some(record) = self.records.get_mut(&id) else { + return actions; + }; + + if record.copy == CopyState::Transmitting { + match &mut record.schedule { + Schedule::Once => {} + Schedule::PassiveAck { + copies_remaining, .. + } + | Schedule::FixedInterval { + copies_remaining, .. + } => *copies_remaining = 0, + } + } else { + self.settle(id, Err(SendFailure::Cancelled.into()), &mut actions); + } + + actions + } + /// Queue copies that are due, complete or fail schedules that ran out, and fail /// records past their deadline. pub(super) fn tick(&mut self, now: Instant) -> Actions { diff --git a/crates/ziggurat-zigbee/src/sends/mod.rs b/crates/ziggurat-zigbee/src/sends/mod.rs index 77a5c12..9527595 100644 --- a/crates/ziggurat-zigbee/src/sends/mod.rs +++ b/crates/ziggurat-zigbee/src/sends/mod.rs @@ -416,10 +416,28 @@ impl> Sends { self.unrouted_unicasts.retry_discovery(destination) } + /// The driver broadcast `request`, the route request of `destination`'s discovery. + /// Copies of the request it replaces still to come are not sent. + pub fn route_request_sent(&mut self, destination: Nwk, request: SendId) -> Actions { + match self + .unrouted_unicasts + .route_request_sent(destination, request) + { + Some(previous) => self.broadcasts.stop(previous), + None => Actions::new(), + } + } + /// Fail every unrouted send to `destination`: its discovery is over without a - /// route. + /// route. Copies of its route request still to come are not sent. pub fn fail_unrouted(&mut self, destination: Nwk) -> Actions { - self.unrouted_unicasts.fail(destination) + let (request, mut actions) = self.unrouted_unicasts.fail(destination); + + if let Some(request) = request { + actions.extend(self.broadcasts.stop(request)); + } + + actions } /// A route to `destination` is known: move every unrouted send to it to the unicasts, diff --git a/crates/ziggurat-zigbee/src/sends/unrouted.rs b/crates/ziggurat-zigbee/src/sends/unrouted.rs index f324de5..65eb637 100644 --- a/crates/ziggurat-zigbee/src/sends/unrouted.rs +++ b/crates/ziggurat-zigbee/src/sends/unrouted.rs @@ -2,7 +2,8 @@ //! //! Discovery runs once per destination and the whole bucket moves on together, so ten //! frames to one device ride a single discovery. The discovery itself lives in the -//! routing table; this table only counts the restarts it has left. +//! routing table; this table counts the restarts it has left and remembers the +//! broadcast of its route request, to stop it if the discovery is abandoned. use alloc::vec::Vec; use core::marker::PhantomData; @@ -37,12 +38,21 @@ pub(super) struct Record { pub(super) deadline: Instant, } +/// What this table tracks of a destination's route discovery. +#[derive(Debug)] +struct Discovery { + /// Restarts left. The first discovery is the driver's, started at admission; these + /// are the ones after it. + restarts_left: u8, + /// The broadcast of the latest route request. + request: Option, +} + #[derive(Debug)] pub(super) struct UnroutedUnicasts { records: FlatMap>, - /// Discovery restarts left, per destination with unrouted sends. The first - /// discovery is the driver's, started at admission; these are the ones after it. - retries: FlatMap, + /// One per destination with unrouted sends. + discoveries: FlatMap, _error: PhantomData E>, } @@ -50,7 +60,7 @@ impl Default for UnroutedUnicasts { fn default() -> Self { Self { records: FlatMap::new(), - retries: FlatMap::new(), + discoveries: FlatMap::new(), _error: PhantomData, } } @@ -62,7 +72,7 @@ impl> UnroutedUnicasts { } pub(super) fn has_unrouted(&self, destination: Nwk) -> bool { - self.retries.contains_key(&destination) + self.discoveries.contains_key(&destination) } /// Hold a send. It fails if still unrouted within the handoff timeout. The first @@ -87,24 +97,41 @@ impl> UnroutedUnicasts { }, ); - if !self.retries.contains_key(&destination) { - // The tunable counts discoveries; the first one is not a restart - self.retries.insert( + if !self.discoveries.contains_key(&destination) { + self.discoveries.insert( destination, - tunables - .pending_route_discovery_attempts() - .saturating_sub(1), + Discovery { + // The tunable counts discoveries; the first one is not a restart + restarts_left: tunables + .pending_route_discovery_attempts() + .saturating_sub(1), + request: None, + }, ); } } pub(super) fn destinations(&self) -> Vec { - self.retries.keys().copied().collect() + self.discoveries.keys().copied().collect() + } + + /// The driver broadcast a route request for `destination`'s discovery. Returns the + /// broadcast of the request it replaces, for the caller to stop. + pub(super) fn route_request_sent( + &mut self, + destination: Nwk, + request: SendId, + ) -> Option { + self.discoveries + .get_mut(&destination) + .unwrap() + .request + .replace(request) } /// Take every send to `destination`: its route is known. pub(super) fn route(&mut self, destination: Nwk) -> Vec<(SendId, Record)> { - self.retries.remove(&destination); + self.discoveries.remove(&destination); self.take(destination) } @@ -138,28 +165,36 @@ impl> UnroutedUnicasts { .values() .any(|send| send.destination == destination); - let retries = self.retries.get_mut(&destination).unwrap(); + let discovery = self.discoveries.get_mut(&destination).unwrap(); // A discovery nothing waits on any more is not restarted - if *retries == 0 || !waiting { + if discovery.restarts_left == 0 || !waiting { return false; } - *retries -= 1; + discovery.restarts_left -= 1; true } - /// Fail every send to `destination`: its discovery is over without a route. - pub(super) fn fail(&mut self, destination: Nwk) -> Actions { - self.retries.remove(&destination); - self.take(destination) + /// Fail every send to `destination`: its discovery is over without a route. Returns + /// the broadcast of its route request, for the caller to stop. + pub(super) fn fail(&mut self, destination: Nwk) -> (Option, Actions) { + let request = self + .discoveries + .remove(&destination) + .and_then(|discovery| discovery.request); + + let actions = self + .take(destination) .into_iter() .map(|(_, send)| Self::settled(send, SendFailure::RouteDiscoveryTimeout)) - .collect() + .collect(); + + (request, actions) } /// Drop every record and return its waiter: the stack is shutting down. pub(super) fn abandon(&mut self) -> Vec { - self.retries.clear(); + self.discoveries.clear(); let mut waiters = Vec::new(); while let Some((_, send)) = self.records.pop_first() { waiters.push(send.waiter);