More fixes for bugs found by AI review

- Validate inbound unnumbered BGP peers against the NDP-discovered
  neighbor before resolving them to an interface session.
- Drop unknown inbound BGP peers in the dispatcher accept loop instead
  of treating them as listener errors and rebinding.
- Fix multicast Router Solicitation checksum generation to use the
  all-routers destination (ff02::2) in the ICMPv6 pseudo-header.
- Add regression tests for unnumbered inbound peer resolution:
- discovered link-local peer uses the interface key
- undiscovered/mismatched/inactive/error cases fall back to IP key
- Add NDP checksum regression tests for RS all-routers multicast and RA
  all-nodes multicast behavior.
- Delete unnumbered neighbor DB state before tearing down runtime
  BGP/NDP state, matching numbered neighbor deletion ordering.
- Report unnumbered peer expiry using the same reachable-time semantics
  as runtime neighbor usability, rather than router-lifetime.

Fixes: #802

Signed-off-by: Trey Aspelund <trey@oxidecomputer.com>

Author: taspelund

bgp/src/connection.rs

@@ -9,7 +9,7 @@ use crate::{
     router::SessionMap,
     session::{FsmEvent, PeerId, SessionInfo, SessionRunner},
 };
-use slog::{Logger, error};
+use slog::{Logger, debug, error};
 use std::{
     net::{SocketAddr, ToSocketAddrs},
     sync::{Arc, Mutex, mpsc::Sender},
@@ -52,7 +52,35 @@ pub fn resolve_session_key(
         && v6_addr.ip().is_unicast_link_local()
     {
         match mgr.get_active_interface_by_scope(v6_addr.scope_id()) {
-            Ok(Some(interface)) => return PeerId::Interface(interface),
+            Ok(Some(interface)) => {
+                match mgr.get_discovered_ndp_neighbor(&interface) {
+                    Ok(Some(neighbor)) if neighbor.addr == *v6_addr.ip() => {
+                        return PeerId::Interface(interface);
+                    }
+                    Ok(Some(neighbor)) => {
+                        debug!(log,
+                            "incoming unnumbered peer does not match discovered NDP neighbor";
+                            "interface" => interface.as_str(),
+                            "peer" => peer_addr,
+                            "discovered" => neighbor.addr.to_string(),
+                        );
+                    }
+                    Ok(None) => {
+                        debug!(log,
+                            "incoming unnumbered peer has no discovered NDP neighbor";
+                            "interface" => interface.as_str(),
+                            "peer" => peer_addr,
+                        );
+                    }
+                    Err(e) => {
+                        error!(log,
+                            "discovered unnumbered neighbor query error: {e}";
+                            "interface" => interface.as_str(),
+                            "error" => format!("{e}"),
+                        );
+                    }
+                }
+            }
             // Fallback to IP-based key on failure/missing interface
             Ok(None) => {}
             Err(e) => {

bgp/src/dispatcher.rs

@@ -5,6 +5,7 @@
 use crate::{
     IO_TIMEOUT,
     connection::{BgpConnection, BgpListener},
+    error::Error,
     router::SessionMap,
     session::{FsmEvent, SessionEvent},
 };
@@ -123,7 +124,14 @@ impl<Cnx: BgpConnection + 'static> Dispatcher<Cnx> {
                         );
                         accepted
                     }
-                    Err(crate::error::Error::Timeout) => {
+                    Err(Error::Timeout) => {
+                        continue 'accept;
+                    }
+                    Err(Error::UnknownPeer(peer)) => {
+                        debug!(log,
+                            "no session found for peer, dropping connection";
+                            "peer" => peer.to_string(),
+                        );
                         continue 'accept;
                     }
                     Err(e) => {
@@ -193,6 +201,7 @@ mod tests {
 
     struct TestUnnumbered {
         scope_result: Result<Option<String>, UnnumberedError>,
+        neighbor_result: Result<Option<NdpNeighbor>, UnnumberedError>,
     }
 
     impl BgpUnnumbered for TestUnnumbered {
@@ -207,7 +216,7 @@ mod tests {
             &self,
             _interface: &str,
         ) -> Result<Option<NdpNeighbor>, UnnumberedError> {
-            Ok(None)
+            self.neighbor_result.clone()
         }
     }
 
@@ -218,12 +227,31 @@ mod tests {
     fn unnumbered_manager(
         scope_result: Result<Option<String>, UnnumberedError>,
     ) -> Arc<dyn BgpUnnumbered> {
-        Arc::new(TestUnnumbered { scope_result })
+        Arc::new(TestUnnumbered {
+            scope_result,
+            neighbor_result: Ok(None),
+        })
+    }
+
+    fn unnumbered_manager_with_neighbor(
+        scope_result: Result<Option<String>, UnnumberedError>,
+        neighbor_result: Result<Option<NdpNeighbor>, UnnumberedError>,
+    ) -> Arc<dyn BgpUnnumbered> {
+        Arc::new(TestUnnumbered {
+            scope_result,
+            neighbor_result,
+        })
     }
 
     #[test]
-    fn link_local_scope_resolves_to_interface_session_key() {
-        let manager = unnumbered_manager(Ok(Some("eth0".into())));
+    fn discovered_link_local_uses_interface_key() {
+        let manager = unnumbered_manager_with_neighbor(
+            Ok(Some("eth0".into())),
+            Ok(Some(NdpNeighbor {
+                addr: "fe80::1".parse().unwrap(),
+                scope_id: 7,
+            })),
+        );
         let peer = "[fe80::1%7]:179".parse().unwrap();
 
         let key = resolve_session_key(peer, Some(&manager), &log());
@@ -232,7 +260,33 @@ mod tests {
     }
 
     #[test]
-    fn link_local_without_active_scope_falls_back_to_ip_session_key() {
+    fn undiscovered_link_local_uses_ip_key() {
+        let manager = unnumbered_manager(Ok(Some("eth0".into())));
+        let peer = "[fe80::1%7]:179".parse().unwrap();
+
+        let key = resolve_session_key(peer, Some(&manager), &log());
+
+        assert_eq!(key, PeerId::Ip(peer.ip()));
+    }
+
+    #[test]
+    fn mismatched_link_local_uses_ip_key() {
+        let manager = unnumbered_manager_with_neighbor(
+            Ok(Some("eth0".into())),
+            Ok(Some(NdpNeighbor {
+                addr: "fe80::2".parse().unwrap(),
+                scope_id: 7,
+            })),
+        );
+        let peer = "[fe80::1%7]:179".parse().unwrap();
+
+        let key = resolve_session_key(peer, Some(&manager), &log());
+
+        assert_eq!(key, PeerId::Ip(peer.ip()));
+    }
+
+    #[test]
+    fn inactive_link_local_scope_uses_ip_key() {
         let manager = unnumbered_manager(Ok(None));
         let peer = "[fe80::1%7]:179".parse().unwrap();
 
@@ -242,7 +296,7 @@ mod tests {
     }
 
     #[test]
-    fn link_local_resolution_error_falls_back_to_ip_session_key() {
+    fn link_local_lookup_error_uses_ip_key() {
         let manager =
             unnumbered_manager(Err(UnnumberedError::ResolutionFailed {
                 interface: "eth0".into(),

mgd/src/bgp_admin.rs

@@ -544,9 +544,13 @@ fn convert_discovered_router_to_api(
     let discovered_at = instant_to_iso8601(state.first_seen);
     let last_advertisement = instant_to_iso8601(state.when);
 
-    // Calculate time until expiry
-    let effective_lifetime =
-        Duration::from_secs(u64::from(state.router_lifetime));
+    // Keep this in sync with ndp::ReceivedRouterAdvertisement::expired(): BGP
+    // treats reachability, not router-lifetime, as the peer usability timer.
+    let effective_lifetime = if state.reachable_time == 0 {
+        Duration::from_secs(10)
+    } else {
+        Duration::from_millis(u64::from(state.reachable_time))
+    };
     let time_until_expiry = if state.expired {
         // Calculate time since expiry
         let time_since_expiry = elapsed_since_when
@@ -1928,6 +1932,12 @@ pub(crate) mod helpers {
         let peer = PeerId::Interface(interface.to_string());
         ctx.db.remove_bgp_prefixes_from_peer(&peer);
 
+        // Match numbered neighbor deletion: remove persistent intent before
+        // tearing down runtime state. If the DB removal fails, the live
+        // session/NDP runtime remain consistent with the still-configured peer.
+        ctx.db
+            .remove_unnumbered_bgp_neighbor(asn.into(), interface)?;
+
         // Delete the BGP session for this unnumbered neighbor.
         // Unnumbered sessions are keyed by interface name, not IP address.
         get_router!(&ctx, asn)?.delete_session(peer);
@@ -1937,10 +1947,6 @@ pub(crate) mod helpers {
             .unnumbered_manager
             .unconfigure_interface(interface)?;
 
-        // And now clear out the top level database entry
-        ctx.db
-            .remove_unnumbered_bgp_neighbor(asn.into(), interface)?;
-
         Ok(HttpResponseDeleted())
     }
 

ndp/src/manager.rs

@@ -347,6 +347,33 @@ impl RouterDiscoveryState {
 #[cfg(test)]
 mod test {
     use crate::packet::{Icmp6RouterAdvertisement, Icmp6RouterSolicitation};
+    use crate::util::{ALL_NODES_MCAST, ALL_ROUTERS_MCAST, cksum};
+    use std::net::Ipv6Addr;
+
+    #[test]
+    fn router_solicitation_checksum_uses_all_routers_multicast() {
+        let src = "fe80::1".parse().unwrap();
+        let mut data = ispf::to_bytes_be(&Icmp6RouterSolicitation::default())
+            .expect("serialize router solicitation");
+
+        cksum(src, Some(ALL_ROUTERS_MCAST), 8, &mut data);
+
+        assert_eq!(&data[2..4], &[0x7d, 0x36]);
+    }
+
+    #[test]
+    fn router_advertisement_checksum_defaults_to_all_nodes_multicast() {
+        let src: Ipv6Addr = "fe80::1".parse().unwrap();
+        let mut default_dst =
+            ispf::to_bytes_be(&Icmp6RouterAdvertisement::default())
+                .expect("serialize router advertisement");
+        let mut all_nodes_dst = default_dst.clone();
+
+        cksum(src, None, 16, &mut default_dst);
+        cksum(src, Some(ALL_NODES_MCAST), 16, &mut all_nodes_dst);
+
+        assert_eq!(default_dst[2..4], all_nodes_dst[2..4]);
+    }
 
     #[test]
     fn router_solicitation_with_link_layer_addr() {

ndp/src/util.rs

@@ -96,17 +96,10 @@ pub fn send_ra(
             return;
         }
     };
-    cksum(src, dst, ICMP6_RA_ULP_LEN, &mut out);
+    let dst_ip = dst.unwrap_or(ALL_NODES_MCAST);
+    cksum(src, Some(dst_ip), ICMP6_RA_ULP_LEN, &mut out);
 
-    let dst = SocketAddrV6::new(
-        match dst {
-            Some(d) => d,
-            None => ALL_NODES_MCAST,
-        },
-        0,
-        0,
-        ifindex.get(),
-    );
+    let dst = SocketAddrV6::new(dst_ip, 0, 0, ifindex.get());
     if let Err(e) = s.send_to(&out, &dst.into()) {
         error!(log, "send_ra: send: {e}");
     }
@@ -127,17 +120,10 @@ pub fn send_rs(
             return;
         }
     };
-    cksum(src, dst, ICMP6_RS_ULP_LEN, &mut out);
+    let dst_ip = dst.unwrap_or(ALL_ROUTERS_MCAST);
+    cksum(src, Some(dst_ip), ICMP6_RS_ULP_LEN, &mut out);
 
-    let dst = SocketAddrV6::new(
-        match dst {
-            Some(d) => d,
-            None => ALL_ROUTERS_MCAST,
-        },
-        0,
-        0,
-        ifindex.get(),
-    );
+    let dst = SocketAddrV6::new(dst_ip, 0, 0, ifindex.get());
     if let Err(e) = s.send_to(&out, &dst.into()) {
         error!(log, "send_rs: send: {e}");
     }