Merge branch 'main' into trey/unnumbered-cleanup

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

Author: taspelund

.github/buildomat/jobs/falcon-lab.sh

@@ -54,6 +54,14 @@ mkdir -p cargo-bay
 mv mgd cargo-bay/
 mv ddmd cargo-bay/
 
+# Juniper/cRPD images require a runtime license. Fetch it on the CI runner,
+# which has catacomb access, and pass it to the guest by file via cargo-bay.
+# The license contents must never be printed or committed.
+curl -sSfL --retry 10 --retry-all-errors \
+	-o cargo-bay/falcon-juniper-license.key \
+	http://catacomb.eng.oxide.computer:12346/falcon/jl
+chmod 0600 cargo-bay/falcon-juniper-license.key
+
 export EXT_INTERFACE=${EXT_INTERFACE:-igb0}
 
 first=$(bmat address ls -f extra -Ho first)
@@ -62,8 +70,15 @@ gw=$(bmat address ls -f extra -Ho gateway)
 server=$(ipadm show-addr "${EXT_INTERFACE}"/dhcp -po ADDR | sed 's#/.*##g')
 pfexec ./dhcp-server "${first}" "${last}" "${gw}" "${server}" &> /work/dhcp-server.log &
 
-RUST_LOG=debug pfexec ./falcon-lab run \
-	trio-unnumbered
+run_test() {
+	local test=$1
+	local status=0
+
+	RUST_LOG=debug pfexec ./falcon-lab run "${test}" || status=$?
+	pfexec ./falcon-lab cleanup "${test}" || true
+	return "${status}"
+}
 
-RUST_LOG=debug pfexec ./falcon-lab run \
-	trio-bfd-static-routing
+run_test mgd-unnumbered
+run_test quartet-unnumbered
+run_test quartet-bfd-static-routing

bgp/src/config.rs

@@ -2,20 +2,23 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+use crate::BGP_PORT;
 use mg_api_types::bgp::config::{
     BgpPeerParameters, Neighbor, UnnumberedNeighbor,
 };
+use mg_api_types::bgp::peer::PeerId;
 use mg_api_types_versions::v1;
 use rdb::Asn;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use std::net::{SocketAddr, SocketAddrV6};
+use std::num::NonZeroU16;
 
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
 pub struct PeerConfig {
     pub name: String,
     pub group: String,
-    pub host: SocketAddr,
+    pub id: PeerId,
+    pub port: NonZeroU16,
     pub hold_time: u64,
     pub idle_hold_time: u64,
     pub delay_open: u64,
@@ -77,7 +80,8 @@ impl From<Neighbor> for PeerConfig {
         Self {
             name,
             group,
-            host: *host,
+            id: PeerId::Ip(host.ip()),
+            port: NonZeroU16::new(host.port()).unwrap_or(BGP_PORT),
             hold_time,
             idle_hold_time,
             delay_open,
@@ -119,7 +123,8 @@ impl From<v1::bgp::config::Neighbor> for PeerConfig {
         Self {
             name,
             group,
-            host,
+            id: PeerId::Ip(host.ip()),
+            port: NonZeroU16::new(host.port()).unwrap_or(BGP_PORT),
             hold_time,
             idle_hold_time,
             delay_open,
@@ -131,17 +136,15 @@ impl From<v1::bgp::config::Neighbor> for PeerConfig {
 }
 
 impl PeerConfig {
-    /// Construct a `PeerConfig` from an `UnnumberedNeighbor` (uses the supplied
-    /// IPv6 link-local socket address as the connection target).
-    pub fn from_unnumbered_neighbor(
-        n: &UnnumberedNeighbor,
-        addr: SocketAddrV6,
-    ) -> Self {
+    /// Construct a `PeerConfig` from an `UnnumberedNeighbor`. The peer is
+    /// identified by its interface; the connection target is resolved via NDP
+    /// at connect time, and the port is always the well-known BGP port.
+    pub fn from_unnumbered_neighbor(n: &UnnumberedNeighbor) -> Self {
         let UnnumberedNeighbor {
             asn: _,
             name,
             group,
-            interface: _,
+            interface,
             act_as_a_default_ipv6_router: _,
             parameters,
         } = n.clone();
@@ -171,8 +174,9 @@ impl PeerConfig {
         } = parameters;
         Self {
             name,
-            host: addr.into(),
             group,
+            id: PeerId::Interface(interface),
+            port: BGP_PORT,
             hold_time,
             idle_hold_time,
             delay_open,

bgp/src/connection_channel.rs

@@ -237,7 +237,7 @@ impl BgpListener<BgpConnectionChannel> for BgpListenerChannel {
         let runner = lock!(sessions)
             .get(&key)
             .cloned()
-            .ok_or(Error::UnknownPeer(peer.ip()))?;
+            .ok_or_else(|| Error::UnknownPeer(key.clone()))?;
 
         let (connection, min_ttl, md5_key) = {
             let config = lock!(runner.session);

bgp/src/connection_tcp.rs

@@ -29,6 +29,7 @@ use std::{
     io::Read,
     io::Write,
     net::{SocketAddr, TcpListener, TcpStream, ToSocketAddrs},
+    num::NonZeroU32,
     sync::atomic::AtomicBool,
     sync::{Arc, Mutex, atomic::Ordering, mpsc::Sender},
     thread::{JoinHandle, sleep},
@@ -156,7 +157,7 @@ impl BgpListener<BgpConnectionTcp> for BgpListenerTcp {
                     let runner = lock!(sessions)
                         .get(&key)
                         .cloned()
-                        .ok_or(Error::UnknownPeer(ip))?;
+                        .ok_or_else(|| Error::UnknownPeer(key.clone()))?;
 
                     let (connection, min_ttl, md5_key) = {
                         let config = lock!(runner.session);
@@ -315,6 +316,22 @@ impl BgpConnector<BgpConnectionTcp> for BgpConnectorTcp {
                     "timeout" => timeout.as_millis()
                 );
 
+                // Bind the socket to the destination's interface for scoped
+                // (link-local / unnumbered) peers.
+                if let SocketAddr::V6(v6) = peer
+                    && let Some(idx) = NonZeroU32::new(v6.scope_id())
+                    && let Err(e) = s.bind_device_by_index_v6(Some(idx))
+                {
+                    connection_log_lite!(log,
+                        warn,
+                        "failed to bind device index {idx} for {peer}: {e}";
+                        "direction" => ConnectionDirection::Outbound,
+                        "peer" => format!("{peer}"),
+                        "error" => format!("{e}")
+                    );
+                    return;
+                }
+
                 // Bind to source address/port if specified
                 if let Some(src) = config.bind_addr {
                     let ba: socket2::SockAddr = src.into();
@@ -1381,7 +1398,7 @@ fn get_md5_source_addrs(peer_ip: IpAddr) -> Result<Vec<SocketAddr>, Error> {
 
     Ok(sources
         .iter()
-        .map(|x| SocketAddr::new(*x, crate::BGP_PORT))
+        .map(|x| SocketAddr::new(*x, crate::BGP_PORT.get()))
         .collect())
 }
 
@@ -1519,7 +1536,7 @@ fn setup_outbound_md5(
 
     let local: Vec<SocketAddr> = sources
         .iter()
-        .map(|x| SocketAddr::new(*x, crate::BGP_PORT))
+        .map(|x| SocketAddr::new(*x, crate::BGP_PORT.get()))
         .collect();
 
     init_md5_associations(fd, key, local.clone(), peer)?;

bgp/src/error.rs

@@ -7,6 +7,7 @@ use std::{
     net::{IpAddr, Ipv4Addr},
 };
 
+use mg_api_types::bgp::peer::PeerId;
 use num_enum::TryFromPrimitiveError;
 
 #[derive(thiserror::Error, Debug)]
@@ -122,7 +123,7 @@ pub enum Error {
     NotConnected,
 
     #[error("Connection attempt from unknown peer: {0}")]
-    UnknownPeer(IpAddr),
+    UnknownPeer(PeerId),
 
     #[error("Session for peer already exists")]
     PeerExists,

bgp/src/lib.rs

@@ -15,6 +15,8 @@ pub mod policy;
 pub mod router;
 pub mod session;
 
+use std::num::NonZeroU16;
+
 mod rhai_integration;
 
 #[cfg(test)]
@@ -30,7 +32,7 @@ mod test;
 #[cfg(test)]
 pub mod connection_channel;
 
-pub const BGP_PORT: u16 = 179;
+pub const BGP_PORT: NonZeroU16 = NonZeroU16::new(179).unwrap();
 pub const BGP_VERSION: u8 = 4;
 pub const COMPONENT_BGP: &str = "bgp";
 pub const MOD_ROUTER: &str = "router";

bgp/src/router.rs

@@ -305,104 +305,57 @@ impl<Cnx: BgpConnection + 'static> Router<Cnx> {
         }
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn ensure_session(
         self: &Arc<Self>,
         peer: PeerConfig,
         bind_addr: Option<SocketAddr>,
         event_tx: Sender<FsmEvent<Cnx>>,
         event_rx: Receiver<FsmEvent<Cnx>>,
         info: SessionInfo,
+        unnumbered_manager: Option<Arc<dyn BgpUnnumbered>>,
     ) -> Result<EnsureSessionResult<Cnx>, Error> {
         let sessions = lock!(self.sessions);
-        let key = PeerId::Ip(peer.host.ip());
-        if sessions.contains_key(&key) {
+        if sessions.contains_key(&peer.id) {
             drop(sessions);
             Ok(EnsureSessionResult::Updated(
                 self.update_session(peer, info)?,
             ))
-        } else {
-            Ok(EnsureSessionResult::New(self.new_session_locked(
-                sessions, key, peer, bind_addr, event_tx, event_rx, info, None,
-            )?))
-        }
-    }
-
-    #[allow(clippy::too_many_arguments)]
-    pub fn ensure_unnumbered_session(
-        self: &Arc<Self>,
-        interface: String,
-        peer: PeerConfig,
-        bind_addr: Option<SocketAddr>,
-        event_tx: Sender<FsmEvent<Cnx>>,
-        event_rx: Receiver<FsmEvent<Cnx>>,
-        info: SessionInfo,
-        unnumbered_manager: Arc<dyn BgpUnnumbered>,
-    ) -> Result<EnsureSessionResult<Cnx>, Error> {
-        let sessions = lock!(self.sessions);
-        let key = PeerId::Interface(interface.clone());
-        if sessions.contains_key(&key) {
-            drop(sessions);
-            Ok(EnsureSessionResult::Updated(
-                self.update_unnumbered_session(&interface, peer, info)?,
-            ))
         } else {
             Ok(EnsureSessionResult::New(self.new_session_locked(
                 sessions,
-                key,
                 peer,
                 bind_addr,
                 event_tx,
                 event_rx,
                 info,
-                Some(unnumbered_manager),
+                unnumbered_manager,
             )?))
         }
     }
 
-    pub fn new_session(
-        self: &Arc<Self>,
-        peer: PeerConfig,
-        bind_addr: Option<SocketAddr>,
-        event_tx: Sender<FsmEvent<Cnx>>,
-        event_rx: Receiver<FsmEvent<Cnx>>,
-        info: SessionInfo,
-    ) -> Result<Arc<SessionRunner<Cnx>>, Error> {
-        let sessions = lock!(self.sessions);
-        let key = PeerId::Ip(peer.host.ip());
-        if sessions.contains_key(&key) {
-            Err(Error::PeerExists)
-        } else {
-            self.new_session_locked(
-                sessions, key, peer, bind_addr, event_tx, event_rx, info, None,
-            )
-        }
-    }
-
     #[allow(clippy::too_many_arguments)]
-    pub fn new_unnumbered_session(
+    pub fn new_session(
         self: &Arc<Self>,
-        interface: String,
         peer: PeerConfig,
         bind_addr: Option<SocketAddr>,
         event_tx: Sender<FsmEvent<Cnx>>,
         event_rx: Receiver<FsmEvent<Cnx>>,
         info: SessionInfo,
-        unnumbered_manager: Arc<dyn BgpUnnumbered>,
+        unnumbered_manager: Option<Arc<dyn BgpUnnumbered>>,
     ) -> Result<Arc<SessionRunner<Cnx>>, Error> {
         let sessions = lock!(self.sessions);
-        let key = PeerId::Interface(interface);
-        if sessions.contains_key(&key) {
+        if sessions.contains_key(&peer.id) {
             Err(Error::PeerExists)
         } else {
             self.new_session_locked(
                 sessions,
-                key,
                 peer,
                 bind_addr,
                 event_tx,
                 event_rx,
                 info,
-                Some(unnumbered_manager),
+                unnumbered_manager,
             )
         }
     }
@@ -411,7 +364,6 @@ impl<Cnx: BgpConnection + 'static> Router<Cnx> {
     fn new_session_locked(
         self: &Arc<Self>,
         mut sessions: MutexGuard<SessionMap<Cnx>>,
-        peer_id: PeerId,
         peer: PeerConfig,
         bind_addr: Option<SocketAddr>,
         event_tx: Sender<FsmEvent<Cnx>>,
@@ -434,8 +386,8 @@ impl<Cnx: BgpConnection + 'static> Router<Cnx> {
         let neighbor = NeighborInfo {
             name: Arc::new(Mutex::new(peer.name.clone())),
             peer_group: peer.group.clone(),
-            peer: peer_id,
-            port: peer.host.port(),
+            peer: peer.id.clone(),
+            port: peer.port,
         };
 
         let runner = Arc::new(SessionRunner::new(
@@ -460,33 +412,9 @@ impl<Cnx: BgpConnection + 'static> Router<Cnx> {
         peer: PeerConfig,
         info: SessionInfo,
     ) -> Result<Arc<SessionRunner<Cnx>>, Error> {
-        // Use PeerId::Ip for numbered sessions
-        let key = PeerId::Ip(peer.host.ip());
-        let session = match lock!(self.sessions).get(&key) {
-            None => return Err(Error::UnknownPeer(peer.host.ip())),
-            Some(s) => s.clone(),
-        };
-
-        session.update_session_parameters(peer, info)?;
-
-        Ok(session)
-    }
-
-    pub fn update_unnumbered_session(
-        self: &Arc<Self>,
-        interface: &str,
-        peer: PeerConfig,
-        info: SessionInfo,
-    ) -> Result<Arc<SessionRunner<Cnx>>, Error> {
-        // Use PeerId::Interface for unnumbered sessions
-        let key = PeerId::Interface(interface.to_string());
+        let key = peer.id.clone();
         let session = match lock!(self.sessions).get(&key) {
-            None => {
-                return Err(Error::InternalCommunication(format!(
-                    "unnumbered session not found for interface: {}",
-                    interface
-                )));
-            }
+            None => return Err(Error::UnknownPeer(key)),
             Some(s) => s.clone(),
         };