NDP: bind RA sockets to interface, add RA validation (#780)

* falcon-lab: improve failure-time diagnostics collection

When a trio test fails, capture more of the state needed to debug it, and
fix gaps that left peer diagnostics empty:

- ox node: kernel neighbor caches (ndp -a -n, arp -a -n) plus, per BGP
  router, mgd's NDP-manager state and per-interface discovered peers via the
  admin API. These surface the link-locals each unnumbered interface learned.
- EOS node: disable paging (terminal length 0) and capture each show command
  separately — a single bundled Cli call stalled on the pager and produced
  nothing. Also capture the ceos container log and host interface state.
- FRR node: trim and drop blank lines when flattening the vtysh script;
  indented/empty lines became '-c "   show ..."' / '-c ""', which vtysh
  rejects, yielding empty output.

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

* ndp: scope discovery sockets to their interface

Each per-interface NDP socket binds to the unspecified address to catch
both unicast and multicast solicitations/advertisements. That bind does not
scope reception to the interface: illumos only honors the bind sockaddr's
scope_id for a link-scoped address, so ::%index leaves the socket's incoming
ifindex unset. Multicast reception is still constrained by per-interface
group membership, but unicast ICMPv6 is delivered to every NDP socket in the
process regardless of arrival link, so a unicast RA from an off-link source
can poison an interface's discovered peer and break unnumbered peering.

Bind each socket to its interface via socket2's bind_device_by_index_v6
(IPV6_BOUND_IF on illumos, SO_BINDTOIFINDEX on Linux), scoping both unicast
and multicast reception to the intended interface.

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

* ndp: validate received router advertisements per RFC 4861 §6.1.2

Apply the RFC 4861 §6.1.2 RA validity checks that aren't otherwise enforced:

- Source must be link-local: the kernel does not filter raw-socket delivery
  by source scope, so a globally- or otherwise-scoped source would be
  accepted. Reject non-link-local sources in handle_ra. (This does not guard
  against an off-link link-local source on the wrong interface; interface
  scoping handles that — see the discovery-socket commit.)
- ICMP length >= 16 octets: guard in Icmp6RouterAdvertisement::from_wire.
  Besides matching the RFC, this prevents a panic: the ispf deserializer
  indexes the input directly and panics on a buffer shorter than the struct
  rather than returning an error.
- Hop limit 255: already enforced by the kernel via IPV6_MINHOPCOUNT, which
  was previously set with a hardcoded illumos constant in an ungated unsafe
  block. Move it behind a cfg-gated helper that works on Linux (libc constant)
  and illumos (value from <netinet/in.h>), no-op elsewhere.

The checksum check is left to the kernel, which verifies the ICMPv6 checksum
before raw delivery.

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

---------

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

Author: taspelund

falcon-lab/src/eos.rs

@@ -71,39 +71,40 @@ impl EosNode {
         LinuxNode(self.0)
     }
 
-    /// Capture protocol-specific state via the Arista CLI.
+    /// Capture protocol-specific state via the Arista CLI, plus container and
+    /// host state.
     pub async fn collect_diagnostics(
         &self,
         d: &Runner,
         topo: &str,
         protocols: ProtocolDiagnostics,
     ) {
         let name = self.name(d);
-        // `Cli -c` takes a single newline-separated script. Linux diagnostics
-        // below collect host/container interfaces and routes for every test,
-        // so keep this focused on the protocol under test.
-        let script = if protocols.bgp() {
-            "enable
-                show running-config
-                show ip bgp summary
-                show ip bgp
-                show ipv6 bgp
-            "
-        } else {
-            "enable
-                show running-config
-                show bfd peers
-            "
-        };
-        match self.shell(d, script).await {
-            Ok(out) => crate::diagnostics::write_artifact(
-                d,
-                topo,
-                &format!("{name}-cli"),
-                None,
-                &out,
-            ),
-            Err(e) => slog::warn!(d.log, "diagnostics {name}-cli: {e}"),
+        let mut commands = vec![("running-config", "show running-config")];
+        if protocols.bgp() {
+            commands.extend([
+                ("ip-bgp-summary", "show ip bgp summary"),
+                ("ip-bgp", "show ip bgp"),
+                ("ipv6-bgp", "show ipv6 bgp"),
+            ]);
+        }
+        if protocols.bfd() {
+            commands.push(("bfd-peers", "show bfd peers"));
+        }
+        for (suffix, cmd) in commands {
+            let script = format!("enable\n{cmd}");
+            match self.shell(d, &script).await {
+                Ok(out) => crate::diagnostics::write_artifact(
+                    d,
+                    topo,
+                    &format!("{name}-{suffix}"),
+                    Some(cmd),
+                    &out,
+                ),
+                Err(e) => {
+                    slog::warn!(d.log, "diagnostics {name}-{suffix}: {e}")
+                }
+            }
         }
         self.linux().collect_diagnostics(d, topo, &name).await;
         self.linux()

falcon-lab/src/frr.rs

@@ -127,7 +127,7 @@ impl FrrNode {
             .collect::<Vec<_>>()
             .join(" ");
         let output = d
-            .exec(self.0, &format!("vtysh {args}"))
+            .exec(self.0, &format!("/usr/bin/vtysh {args}"))
             .await
             .context("vtysh shell failed")?;
         Ok(output)

falcon-lab/src/illumos.rs

@@ -144,6 +144,8 @@ impl IllumosNode {
             ("ipadm", "ipadm show-addr"),
             ("dladm", "dladm show-link"),
             ("netstat", "netstat -nr"),
+            ("ndp", "ndp -a -n"),
+            ("arp", "arp -a -n"),
         ] {
             crate::diagnostics::capture(
                 d,

falcon-lab/src/mgd.rs

@@ -62,6 +62,49 @@ impl MgdNode {
             Err(e) => slog::warn!(d.log, "diagnostics {label}: {e}"),
         }
     }
+
+    pub async fn collect_ndp_diagnostics(
+        &self,
+        d: &Runner,
+        client: &Client,
+        topo: &str,
+    ) {
+        let name = d.get_node(self.0).name.clone();
+        let routers = match client.read_routers().await {
+            Ok(r) => r.into_inner(),
+            Err(e) => {
+                slog::warn!(d.log, "diagnostics {name}-ndp: read routers: {e}");
+                return;
+            }
+        };
+        for router in routers {
+            let asn = router.asn;
+            for (suffix, result) in [
+                (
+                    "ndp-manager",
+                    client
+                        .get_ndp_manager_state(asn)
+                        .await
+                        .map(|r| format!("{:#?}", r.into_inner())),
+                ),
+                (
+                    "ndp-interfaces",
+                    client
+                        .get_ndp_interfaces(asn)
+                        .await
+                        .map(|r| format!("{:#?}", r.into_inner())),
+                ),
+            ] {
+                let label = format!("{name}-{suffix}-asn{asn}");
+                match result {
+                    Ok(contents) => crate::diagnostics::write_artifact(
+                        d, topo, &label, None, &contents,
+                    ),
+                    Err(e) => slog::warn!(d.log, "diagnostics {label}: {e}"),
+                }
+            }
+        }
+    }
 }
 
 pub async fn wait_for_mgd(

falcon-lab/src/test.rs

@@ -152,6 +152,7 @@ where
     let cr1 = bt.cr1;
     let cr2 = bt.cr2;
     let cr3 = bt.cr3;
+    let mgd = bt.mgd.clone();
     let topo_name = bt.topo_name.clone();
     let protocols = bt.protocols;
     let result = body(bt).await;
@@ -165,6 +166,7 @@ where
         if diag_on_fail {
             collect_diagnostics(&ad, ox, cr1, cr2, cr3, &topo_name, protocols)
                 .await;
+            ox.collect_ndp_diagnostics(&ad, &mgd, &topo_name).await;
         }
     }
     result

ndp/src/manager.rs

@@ -11,7 +11,7 @@ use crate::util::{
 };
 use mg_common::thread::ManagedThread;
 use mg_common::{lock, read_lock, write_lock};
-use slog::{Logger, error};
+use slog::{Logger, debug, error};
 use socket2::Socket;
 use std::mem::MaybeUninit;
 use std::net::Ipv6Addr;
@@ -340,6 +340,16 @@ impl InterfaceNdpManagerInner {
     /// is updated as well as the time of reception and the stored advertisement
     /// containing the reachable time.
     fn handle_ra(&self, ra: Icmp6RouterAdvertisement, src: Ipv6Addr) {
+        // Per RFC 4861 Section 6.1.2: a valid RA's source is always link-local
+        if !src.is_unicast_link_local() {
+            debug!(
+                self.log,
+                "ignoring RA from non-link-local source {src} on {}",
+                self.ifx.name,
+            );
+            return;
+        }
+
         let mut guard = lock!(self.neighbor_router);
         let now = Instant::now();
 

ndp/src/packet.rs

@@ -39,8 +39,13 @@ impl Icmp6RouterAdvertisement {
     const TYPE: u8 = 134;
     const CODE: u8 = 0;
     const DEFAULT_HOPLIMIT: u8 = 255;
+    const MIN_PAYLOAD_LEN: usize = 16;
 
     pub fn from_wire(buf: &[u8]) -> Result<Self, Icmp6RaFromWireError> {
+        // Per RFC 4861 Section 6.1.2: a valid RA has an ICMP payload of >= 16b
+        if buf.len() < Self::MIN_PAYLOAD_LEN {
+            return Err(Icmp6RaFromWireError::TooShort(buf.len()));
+        }
         let s: Self = ispf::from_bytes_be(buf)?;
         if s.typ != Self::TYPE {
             return Err(Icmp6RaFromWireError::WrongType(s.typ));
@@ -86,6 +91,9 @@ pub enum Icmp6RaFromWireError {
     #[error("deserialization error: {0}")]
     Ispf(#[from] ispf::Error),
 
+    #[error("too short: {0} octets, expected at least 16")]
+    TooShort(usize),
+
     #[error("wrong type: expected {expected}, got {0}", expected = Icmp6RouterAdvertisement::TYPE)]
     WrongType(u8),
 

ndp/src/util.rs

@@ -5,13 +5,11 @@
 // Copyright 2026 Oxide Computer Company
 
 use crate::packet::{Icmp6RouterAdvertisement, Icmp6RouterSolicitation};
-use libc::{c_int, socklen_t};
 use slog::{Logger, error};
 use socket2::{Domain, Protocol, Socket, Type};
 use std::{
-    ffi::c_void,
     net::{Ipv6Addr, SocketAddrV6},
-    os::fd::AsRawFd,
+    num::NonZeroU32,
     thread::sleep,
     time::{Duration, Instant},
 };
@@ -70,8 +68,15 @@ pub enum ListeningSocketError {
     #[error("set read timeout error: {0}")]
     SetReadTimeoutError(std::io::Error),
 
+    #[cfg(any(target_os = "linux", target_os = "illumos"))]
     #[error("failed to set ipv6 min hop count: {0}")]
     SetIpv6MinHopCount(std::io::Error),
+
+    #[error("failed to bind socket to interface: {0}")]
+    SetBoundIf(std::io::Error),
+
+    #[error("interface index must be non-zero")]
+    InvalidInterfaceIndex,
 }
 
 pub fn send_ra(
@@ -205,28 +210,52 @@ pub fn create_socket(index: u32) -> Result<Socket, ListeningSocketError> {
     s.join_multicast_v6(&ALL_ROUTERS_MCAST, index)
         .map_err(E::JoinAllRoutersMulticast)?;
 
+    let ifindex = NonZeroU32::new(index).ok_or(E::InvalidInterfaceIndex)?;
+    s.bind_device_by_index_v6(Some(ifindex))
+        .map_err(E::SetBoundIf)?;
+
     s.bind(&sa).map_err(ListeningSocketError::Bind)?;
 
     s.set_read_timeout(Some(READ_TIMEOUT))
         .map_err(E::SetReadTimeoutError)?;
 
-    unsafe {
-        // from <netinet/in.h>
-        const IPV6_MINHOPCOUNT: c_int = 0x2f;
-        let min_hops: c_int = 255;
-        let rc = libc::setsockopt(
+    // Per RFC 4861 Section 6.1.2: a valid RA must have a hop limit of 255
+    set_min_hopcount(&s)?;
+
+    Ok(s)
+}
+
+#[cfg(any(target_os = "linux", target_os = "illumos"))]
+fn set_min_hopcount(s: &Socket) -> Result<(), ListeningSocketError> {
+    use std::os::fd::AsRawFd;
+
+    // illumos does not export IPV6_MINHOPCOUNT via libc; value from
+    // <netinet/in.h>. Linux provides it.
+    #[cfg(target_os = "illumos")]
+    const IPV6_MINHOPCOUNT: libc::c_int = 0x2f;
+    #[cfg(target_os = "linux")]
+    use libc::IPV6_MINHOPCOUNT;
+
+    let min_hops: libc::c_int = 255;
+    // SAFETY: setsockopt with a correctly-sized pointer to an integer option.
+    let rc = unsafe {
+        libc::setsockopt(
             s.as_raw_fd(),
             libc::IPPROTO_IPV6,
             IPV6_MINHOPCOUNT,
-            &min_hops as *const _ as *const c_void,
-            std::mem::size_of::<libc::c_int>() as socklen_t,
-        );
-        if rc < 0 {
-            return Err(ListeningSocketError::SetIpv6MinHopCount(
-                std::io::Error::last_os_error(),
-            ));
-        }
+            &min_hops as *const _ as *const libc::c_void,
+            std::mem::size_of::<libc::c_int>() as libc::socklen_t,
+        )
+    };
+    if rc < 0 {
+        return Err(ListeningSocketError::SetIpv6MinHopCount(
+            std::io::Error::last_os_error(),
+        ));
     }
+    Ok(())
+}
 
-    Ok(s)
+#[cfg(not(any(target_os = "linux", target_os = "illumos")))]
+fn set_min_hopcount(_s: &Socket) -> Result<(), ListeningSocketError> {
+    Ok(())
 }