Discovery on Windows: ask table-only neighbours again before listing them

CHANGELOG.md

@@ -26,6 +26,16 @@ its heading and collects entries. The date and the link go on with the tag.
   Felix Stubner. Upgrading from an earlier version leaves one entry, under the
   new name.
 
+### Fixed
+
+- **Discovery on Windows listed devices that had left the network.** The
+  Windows device table keeps an entry for a while after its device goes,
+  and discovery reported those entries as devices. On one network that was
+  4 of 26, none of which answered anything afterwards. Devices found only in
+  the table are now asked again directly, which can add about 2 seconds
+  and only when the table holds such entries, and are listed only if they
+  answer. Devices that ignore ping still answer, so they are still found.
+
 ## [0.3.4] - 2026-10-04
 
 ### Added

crates/netscli-core/src/arp.rs

@@ -1,7 +1,9 @@
 mod platform;
 mod types;
 
-use std::net::IpAddr;
+use std::collections::HashSet;
+use std::net::{IpAddr, Ipv4Addr};
+use std::time::Duration;
 
 use mac_address::MacAddress;
 
@@ -37,3 +39,43 @@ impl NetworkManager {
         platform::clear_table()
     }
 }
+
+/// Which of `candidates` answer ARP within `wait`, for discovery's
+/// table-only neighbours. On Windows the neighbour table keeps devices that
+/// have gone (see platform/windows_resolve.rs), so each is asked again. On
+/// Linux and macOS this is not measured yet and every candidate is kept, as
+/// before.
+pub(crate) async fn still_present(candidates: &[Ipv4Addr], wait: Duration) -> HashSet<Ipv4Addr> {
+    #[cfg(target_os = "windows")]
+    {
+        let checks = candidates.iter().map(|&ip| async move {
+            // A lookup that outlives `wait` keeps its blocking thread until
+            // Windows gives up on it, about 4 s; its answer is just ignored.
+            let asked = tokio::task::spawn_blocking(move || platform::answers_arp(ip));
+            matches!(tokio::time::timeout(wait, asked).await, Ok(Ok(true))).then_some(ip)
+        });
+        futures::future::join_all(checks)
+            .await
+            .into_iter()
+            .flatten()
+            .collect()
+    }
+    #[cfg(not(target_os = "windows"))]
+    {
+        let _ = wait;
+        candidates.iter().copied().collect()
+    }
+}
+
+#[cfg(all(test, target_os = "windows"))]
+mod tests {
+    use super::*;
+
+    #[tokio::test]
+    async fn a_neighbour_that_does_not_answer_arp_is_dropped() {
+        // TEST-NET-1 (RFC 5737): never assigned, so nothing can answer.
+        let gone = Ipv4Addr::new(192, 0, 2, 77);
+        let present = still_present(&[gone], Duration::from_millis(300)).await;
+        assert!(present.is_empty(), "kept {present:?}");
+    }
+}

crates/netscli-core/src/arp/platform.rs

@@ -3,6 +3,8 @@ mod interfaces;
 mod mutate;
 mod table;
 #[cfg(target_os = "windows")]
+mod windows_resolve;
+#[cfg(target_os = "windows")]
 mod windows_table;
 
 use std::net::IpAddr;
@@ -31,3 +33,9 @@ pub(super) fn delete_entry(ip: IpAddr) -> Result<()> {
 pub(super) fn clear_table() -> Result<()> {
     mutate::clear_table()
 }
+
+/// Whether `ip` answers ARP right now. Blocking; see windows_resolve.rs.
+#[cfg(target_os = "windows")]
+pub(super) fn answers_arp(ip: std::net::Ipv4Addr) -> bool {
+    windows_resolve::resolve(ip).is_some()
+}

crates/netscli-core/src/arp/platform/windows_resolve.rs

@@ -0,0 +1,32 @@
+//! Ask a neighbour to answer ARP now, rather than trusting the table.
+//!
+//! The neighbour table remembers devices that have since gone. Read straight
+//! after a ping sweep it still held entries in the Stale, Delay and Probe
+//! states for devices that no longer answered anything, and discovery
+//! reported them as present: 4 of 26 hosts on one LAN (measured 2026-10-06),
+//! every one of them failing three ARP lookups in a row afterwards.
+//!
+//! `SendARP` sends a fresh ARP request and blocks until a reply or Windows'
+//! own retries run out, about 4 s for an address with nothing behind it. A
+//! device that is there but ignores ping still answers ARP, so this keeps
+//! the ICMP-silent devices the table is there to find and drops the ghosts.
+
+use std::net::Ipv4Addr;
+
+use windows_sys::Win32::NetworkManagement::IpHelper::SendARP;
+
+/// The MAC `ip` answers ARP with, or `None` if nothing answered. Blocking.
+pub(super) fn resolve(ip: Ipv4Addr) -> Option<[u8; 6]> {
+    let mut mac = [0u8; 8];
+    let mut len = mac.len() as u32;
+    // SendARP takes the address in network byte order as a u32.
+    let dest = u32::from_ne_bytes(ip.octets());
+    // SAFETY: an 8-byte buffer and its length; SendARP writes at most `len`.
+    let status = unsafe { SendARP(dest, 0, mac.as_mut_ptr().cast(), &mut len) };
+    if status != 0 || len != 6 {
+        return None;
+    }
+    let mut out = [0u8; 6];
+    out.copy_from_slice(&mac[..6]);
+    Some(out)
+}

crates/netscli-core/src/discover.rs

@@ -11,12 +11,18 @@ use std::sync::{
     Arc,
 };
 
+mod host_address;
 #[cfg(feature = "mdns")]
 mod mdns_fusion;
 mod types;
 
+use host_address::is_host_address;
 pub use types::{DiscoverPhase, DiscoverProgress, FoundBy, Host, NameSource};
 
+/// How long a table-only neighbour gets to answer ARP before discovery drops
+/// it. The slowest real reply measured was 2.2 s (2026-10-06).
+const NEIGHBOR_CONFIRM_WAIT: std::time::Duration = std::time::Duration::from_millis(2500);
+
 pub struct DiscoverEngine {
     ping_scanner: PingScanner,
     concurrency: usize,
@@ -138,6 +144,44 @@ impl DiscoverEngine {
                 .map(|e| (e.ip, e))
                 .collect();
 
+        // Anything the OS has an ARP entry for is on this link, whether or
+        // not it answered us. Plenty of devices do not: consumer IoT
+        // routinely ignores ICMP, and Windows drops echo requests by
+        // default. Discarding them meant discovery reported a fraction of
+        // the network -- measured at 13 of 25 known devices on one ordinary
+        // LAN -- while the table needed to find them was already loaded, and
+        // used only to decorate the hosts that had replied.
+        let answered: std::collections::HashSet<IpAddr> = alive.iter().map(|r| r.ip).collect();
+        let mut neighbors: Vec<IpAddr> = arp_map
+            .keys()
+            .copied()
+            .filter(|ip| !answered.contains(ip))
+            .filter(|ip| match ip {
+                // Only within the range that was asked for. The neighbour
+                // table spans every interface, so it holds addresses from
+                // other subnets entirely.
+                IpAddr::V4(v4) => is_host_address(&subnet, *v4),
+                IpAddr::V6(_) => false,
+            })
+            .filter(|ip| arp_map.get(ip).is_none_or(|e| e.mac.bytes()[0] & 1 == 0))
+            .collect();
+        neighbors.sort_unstable();
+
+        // Ask the table-only candidates again, in the background, while names
+        // are looked up below. See crate::arp::still_present for why; the
+        // wait covers the slowest real answer measured, 2.2 s, from a device
+        // that replied to Windows' third ARP retry.
+        let neighbor_v4: Vec<std::net::Ipv4Addr> = neighbors
+            .iter()
+            .filter_map(|ip| match ip {
+                IpAddr::V4(v4) => Some(*v4),
+                IpAddr::V6(_) => None,
+            })
+            .collect();
+        let confirm = tokio::spawn(async move {
+            crate::arp::still_present(&neighbor_v4, NEIGHBOR_CONFIRM_WAIT).await
+        });
+
         // 3) Optionally reverse-DNS alive hosts using a single resolver.
         let hostname_map: HashMap<IpAddr, Option<String>> = if resolve {
             let ips = alive.iter().map(|r| r.ip).collect::<Vec<_>>();
@@ -226,67 +270,14 @@ impl DiscoverEngine {
             .map(|r| build(r.ip, r.rtt_ms, FoundBy::Probe))
             .collect();
 
-        // Anything the OS has an ARP entry for is on this link, whether or
-        // not it answered us. Plenty of devices do not: consumer IoT
-        // routinely ignores ICMP, and Windows drops echo requests by
-        // default. Discarding them meant discovery reported a fraction of
-        // the network -- measured at 13 of 25 known devices on one ordinary
-        // LAN -- while the table needed to find them was already loaded, and
-        // used only to decorate the hosts that had replied.
-        let answered: std::collections::HashSet<IpAddr> = alive.iter().map(|r| r.ip).collect();
-        let mut neighbors: Vec<IpAddr> = arp_map
-            .keys()
-            .copied()
-            .filter(|ip| !answered.contains(ip))
-            .filter(|ip| match ip {
-                // Only within the range that was asked for. The neighbour
-                // table spans every interface, so it holds addresses from
-                // other subnets entirely.
-                IpAddr::V4(v4) => is_host_address(&subnet, *v4),
-                IpAddr::V6(_) => false,
-            })
-            .filter(|ip| arp_map.get(ip).is_none_or(|e| e.mac.bytes()[0] & 1 == 0))
-            .collect();
-        neighbors.sort_unstable();
+        let present = confirm.await.unwrap_or_default();
         hosts.extend(
             neighbors
                 .into_iter()
+                .filter(|ip| matches!(ip, IpAddr::V4(v4) if present.contains(v4)))
                 .map(|ip| build(ip, None, FoundBy::Neighbor)),
         );
 
         Ok(hosts)
     }
 }
-
-/// An address in `subnet` that a device can hold: not the network address
-/// and not the broadcast address, except on /31 and /32 where every address
-/// is usable. The Windows neighbour table lists x.x.x.255
-/// (ff:ff:ff:ff:ff:ff), and discovery reported it as a host.
-fn is_host_address(subnet: &Ipv4Net, ip: std::net::Ipv4Addr) -> bool {
-    subnet.contains(&ip)
-        && (subnet.prefix_len() >= 31 || (ip != subnet.network() && ip != subnet.broadcast()))
-}
-
-#[cfg(test)]
-mod host_address_tests {
-    use super::is_host_address;
-
-    #[test]
-    fn network_and_broadcast_are_not_hosts() {
-        let net = "192.168.1.0/24".parse().unwrap();
-        assert!(!is_host_address(&net, "192.168.1.0".parse().unwrap()));
-        assert!(!is_host_address(&net, "192.168.1.255".parse().unwrap()));
-        assert!(is_host_address(&net, "192.168.1.1".parse().unwrap()));
-        assert!(is_host_address(&net, "192.168.1.254".parse().unwrap()));
-        assert!(!is_host_address(&net, "192.168.2.1".parse().unwrap()));
-    }
-
-    #[test]
-    fn every_address_counts_on_a_31_or_32() {
-        let p2p = "10.0.0.0/31".parse().unwrap();
-        assert!(is_host_address(&p2p, "10.0.0.0".parse().unwrap()));
-        assert!(is_host_address(&p2p, "10.0.0.1".parse().unwrap()));
-        let single = "10.0.0.5/32".parse().unwrap();
-        assert!(is_host_address(&single, "10.0.0.5".parse().unwrap()));
-    }
-}

crates/netscli-core/src/discover/host_address.rs

@@ -0,0 +1,34 @@
+use ipnet::Ipv4Net;
+
+/// An address in `subnet` that a device can hold: not the network address
+/// and not the broadcast address, except on /31 and /32 where every address
+/// is usable. The Windows neighbour table lists x.x.x.255
+/// (ff:ff:ff:ff:ff:ff), and discovery reported it as a host.
+pub(super) fn is_host_address(subnet: &Ipv4Net, ip: std::net::Ipv4Addr) -> bool {
+    subnet.contains(&ip)
+        && (subnet.prefix_len() >= 31 || (ip != subnet.network() && ip != subnet.broadcast()))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::is_host_address;
+
+    #[test]
+    fn network_and_broadcast_are_not_hosts() {
+        let net = "192.168.1.0/24".parse().unwrap();
+        assert!(!is_host_address(&net, "192.168.1.0".parse().unwrap()));
+        assert!(!is_host_address(&net, "192.168.1.255".parse().unwrap()));
+        assert!(is_host_address(&net, "192.168.1.1".parse().unwrap()));
+        assert!(is_host_address(&net, "192.168.1.254".parse().unwrap()));
+        assert!(!is_host_address(&net, "192.168.2.1".parse().unwrap()));
+    }
+
+    #[test]
+    fn every_address_counts_on_a_31_or_32() {
+        let p2p = "10.0.0.0/31".parse().unwrap();
+        assert!(is_host_address(&p2p, "10.0.0.0".parse().unwrap()));
+        assert!(is_host_address(&p2p, "10.0.0.1".parse().unwrap()));
+        let single = "10.0.0.5/32".parse().unwrap();
+        assert!(is_host_address(&single, "10.0.0.5".parse().unwrap()));
+    }
+}