bgp: unify numbered and unnumbered peer config

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

Author: taspelund

bgp/src/config.rs

@@ -2,187 +2,10 @@
 // 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::num::NonZeroU16;
-
-#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
-pub struct PeerConfig {
-    pub name: String,
-    pub group: String,
-    pub id: PeerId,
-    pub port: NonZeroU16,
-    pub hold_time: u64,
-    pub idle_hold_time: u64,
-    pub delay_open: u64,
-    pub connect_retry: u64,
-    pub keepalive: u64,
-    pub resolution: u64,
-}
 
 #[derive(Debug, Clone, Copy)]
 pub struct RouterConfig {
     pub asn: Asn,
     pub id: u32,
 }
-
-// ----- PeerConfig boundary conversions -----
-//
-// `PeerConfig` is bgp-internal (non-published) and tracks only the
-// fields needed to drive the connection state machine — most
-// BgpPeerParameters fields are session-level and don't belong here.
-// Each conversion destructures both the outer Neighbor-shaped type
-// and the embedded BgpPeerParameters so a field addition on either
-// fails to bind here, forcing a deliberate decision about whether
-// the new field belongs on PeerConfig. Bindings prefixed `_:` are
-// intentionally dropped.
-
-impl From<Neighbor> for PeerConfig {
-    fn from(rq: Neighbor) -> Self {
-        let Neighbor {
-            asn: _,
-            name,
-            group,
-            host,
-            parameters,
-        } = rq;
-        let BgpPeerParameters {
-            hold_time,
-            idle_hold_time,
-            delay_open,
-            connect_retry,
-            keepalive,
-            resolution,
-            passive: _,
-            remote_asn: _,
-            min_ttl: _,
-            md5_auth_key: _,
-            multi_exit_discriminator: _,
-            communities: _,
-            local_pref: _,
-            enforce_first_as: _,
-            vlan_id: _,
-            ipv4_unicast: _,
-            ipv6_unicast: _,
-            deterministic_collision_resolution: _,
-            idle_hold_jitter: _,
-            connect_retry_jitter: _,
-            src_addr: _,
-            src_port: _,
-        } = parameters;
-        Self {
-            name,
-            group,
-            id: PeerId::Ip(host.ip()),
-            port: NonZeroU16::new(host.port()).unwrap_or(BGP_PORT),
-            hold_time,
-            idle_hold_time,
-            delay_open,
-            connect_retry,
-            keepalive,
-            resolution,
-        }
-    }
-}
-
-impl From<v1::bgp::config::Neighbor> for PeerConfig {
-    fn from(rq: v1::bgp::config::Neighbor) -> Self {
-        let v1::bgp::config::Neighbor {
-            asn: _,
-            name,
-            group,
-            host,
-            parameters,
-        } = rq;
-        let v1::bgp::config::BgpPeerParameters {
-            hold_time,
-            idle_hold_time,
-            delay_open,
-            connect_retry,
-            keepalive,
-            resolution,
-            passive: _,
-            remote_asn: _,
-            min_ttl: _,
-            md5_auth_key: _,
-            multi_exit_discriminator: _,
-            communities: _,
-            local_pref: _,
-            enforce_first_as: _,
-            allow_import: _,
-            allow_export: _,
-            vlan_id: _,
-        } = parameters;
-        Self {
-            name,
-            group,
-            id: PeerId::Ip(host.ip()),
-            port: NonZeroU16::new(host.port()).unwrap_or(BGP_PORT),
-            hold_time,
-            idle_hold_time,
-            delay_open,
-            connect_retry,
-            keepalive,
-            resolution,
-        }
-    }
-}
-
-impl PeerConfig {
-    /// 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,
-            act_as_a_default_ipv6_router: _,
-            parameters,
-        } = n.clone();
-        let BgpPeerParameters {
-            hold_time,
-            idle_hold_time,
-            delay_open,
-            connect_retry,
-            keepalive,
-            resolution,
-            passive: _,
-            remote_asn: _,
-            min_ttl: _,
-            md5_auth_key: _,
-            multi_exit_discriminator: _,
-            communities: _,
-            local_pref: _,
-            enforce_first_as: _,
-            vlan_id: _,
-            ipv4_unicast: _,
-            ipv6_unicast: _,
-            deterministic_collision_resolution: _,
-            idle_hold_jitter: _,
-            connect_retry_jitter: _,
-            src_addr: _,
-            src_port: _,
-        } = parameters;
-        Self {
-            name,
-            group,
-            id: PeerId::Interface(interface),
-            port: BGP_PORT,
-            hold_time,
-            idle_hold_time,
-            delay_open,
-            connect_retry,
-            keepalive,
-            resolution,
-        }
-    }
-}

bgp/src/connection.rs

@@ -13,6 +13,7 @@ use crate::{
 use slog::Logger;
 use std::{
     net::{SocketAddr, ToSocketAddrs},
+    num::NonZeroU16,
     sync::{Arc, Mutex, mpsc::Sender},
     thread::JoinHandle,
     time::Duration,
@@ -31,6 +32,13 @@ pub const MAX_MD5SIG_KEYLEN: usize = 80;
 #[cfg(target_os = "macos")]
 pub const MAX_MD5SIG_KEYLEN: usize = 80;
 
+/// Resolved socket addresses for an outbound connection attempt.
+#[derive(Debug, Clone, Copy)]
+pub struct ConnectTarget {
+    pub destination: SocketAddr,
+    pub source: Option<SocketAddr>,
+}
+
 /// Implementors of this trait listen to and accept inbound BGP connections.
 pub trait BgpListener<Cnx: BgpConnection> {
     /// Bind to an address and listen for connections.
@@ -63,6 +71,7 @@ pub trait BgpListener<Cnx: BgpConnection> {
         conn: &Cnx,
         min_ttl: Option<u8>,
         md5_key: Option<String>,
+        listen_port: NonZeroU16,
     ) -> Result<(), Error>;
 
     /// `SocketAddr` the listener is receiving connections on
@@ -79,7 +88,7 @@ pub trait BgpConnector<Cnx: BgpConnection> {
     /// Returns a handle to the connection attempt, allowing the caller to track
     /// and manage it.
     fn connect(
-        peer: SocketAddr,
+        addrs: ConnectTarget,
         timeout: Duration,
         log: Logger,
         event_tx: Sender<FsmEvent<Cnx>>,

bgp/src/connection_channel.rs

@@ -11,8 +11,8 @@ use crate::{
     IO_TIMEOUT,
     clock::ConnectionClock,
     connection::{
-        BgpConnection, BgpConnector, BgpListener, ConnectionDirection,
-        ConnectionId, ThreadState,
+        BgpConnection, BgpConnector, BgpListener, ConnectTarget,
+        ConnectionDirection, ConnectionId, ThreadState,
     },
     error::Error,
     log::{connection_log, connection_log_lite},
@@ -275,6 +275,7 @@ impl BgpListener<BgpConnectionChannel> for BgpListenerChannel {
         _conn: &BgpConnectionChannel,
         _min_ttl: Option<u8>,
         _md5_key: Option<String>,
+        _listen_port: std::num::NonZeroU16,
     ) -> Result<(), Error> {
         // Policy application is ignored for test connections
         Ok(())
@@ -563,15 +564,16 @@ pub struct BgpConnectorChannel;
 
 impl BgpConnector<BgpConnectionChannel> for BgpConnectorChannel {
     fn connect(
-        peer: SocketAddr,
+        addrs: ConnectTarget,
         timeout: Duration,
         log: Logger,
         event_tx: Sender<FsmEvent<BgpConnectionChannel>>,
         config: SessionInfo,
     ) -> Result<JoinHandle<()>, Error> {
         let direction = ConnectionDirection::Outbound;
-        let addr = config
-            .bind_addr
+        let peer = addrs.destination;
+        let addr = addrs
+            .source
             .expect("source address required for channel-based connection");
 
         connection_log_lite!(log,

bgp/src/connection_tcp.rs

@@ -6,8 +6,8 @@ use crate::{
     IO_TIMEOUT,
     clock::ConnectionClock,
     connection::{
-        BgpConnection, BgpConnector, BgpListener, ConnectionDirection,
-        ConnectionId, ThreadState,
+        BgpConnection, BgpConnector, BgpListener, ConnectTarget,
+        ConnectionDirection, ConnectionId, ThreadState,
     },
     error::Error,
     log::{connection_log, connection_log_lite},
@@ -207,6 +207,7 @@ impl BgpListener<BgpConnectionTcp> for BgpListenerTcp {
         conn: &BgpConnectionTcp,
         min_ttl: Option<u8>,
         md5_key: Option<String>,
+        _listen_port: std::num::NonZeroU16,
     ) -> Result<(), Error> {
         let tcp_stream = lock!(conn.conn);
 
@@ -230,7 +231,8 @@ impl BgpListener<BgpConnectionTcp> for BgpListenerTcp {
 
             #[cfg(target_os = "illumos")]
             {
-                let local = get_md5_source_addrs(conn.peer.ip())?;
+                let local =
+                    get_md5_source_addrs(conn.peer.ip(), _listen_port.get())?;
                 conn.manage_md5_associations(
                     tcp_stream.as_raw_fd(),
                     key,
@@ -258,12 +260,13 @@ pub struct BgpConnectorTcp;
 
 impl BgpConnector<BgpConnectionTcp> for BgpConnectorTcp {
     fn connect(
-        peer: SocketAddr,
+        addrs: ConnectTarget,
         timeout: Duration,
         log: Logger,
         event_tx: Sender<FsmEvent<BgpConnectionTcp>>,
         config: SessionInfo,
     ) -> Result<JoinHandle<()>, Error> {
+        let peer = addrs.destination;
         let s = create_outbound_socket(peer, &log)?;
 
         // Apply MD5 authentication before connecting (Linux)
@@ -290,8 +293,15 @@ impl BgpConnector<BgpConnectionTcp> for BgpConnectorTcp {
         #[cfg(target_os = "illumos")]
         let md5_locals = if let Some(key) = &config.md5_auth_key {
             Some(
-                setup_outbound_md5(s.as_raw_fd(), key, peer.ip(), peer, &log)
-                    .map_err(|e| {
+                setup_outbound_md5(
+                    s.as_raw_fd(),
+                    key,
+                    peer.ip(),
+                    peer,
+                    config.addr.listen_port.get(),
+                    &log,
+                )
+                .map_err(|e| {
                     connection_log_lite!(log,
                         warn,
                         "failed to apply MD5 auth for {peer}: {e}";
@@ -334,7 +344,7 @@ impl BgpConnector<BgpConnectionTcp> for BgpConnectorTcp {
                 }
 
                 // Bind to source address/port if specified
-                if let Some(src) = config.bind_addr {
+                if let Some(src) = addrs.source {
                     let ba: socket2::SockAddr = src.into();
                     if let Err(e) = s.bind(&ba) {
                         connection_log_lite!(log,
@@ -1387,7 +1397,10 @@ fn source_address_select(dst: IpAddr) -> anyhow::Result<Vec<IpAddr>> {
 /// Helper function to select and validate MD5 source addresses.
 /// Eliminates duplication between inbound and outbound paths
 #[cfg(target_os = "illumos")]
-fn get_md5_source_addrs(peer_ip: IpAddr) -> Result<Vec<SocketAddr>, Error> {
+fn get_md5_source_addrs(
+    peer_ip: IpAddr,
+    local_port: u16,
+) -> Result<Vec<SocketAddr>, Error> {
     let sources = source_address_select(peer_ip)
         .map_err(|e| Error::InvalidAddress(e.to_string()))?;
 
@@ -1399,7 +1412,7 @@ fn get_md5_source_addrs(peer_ip: IpAddr) -> Result<Vec<SocketAddr>, Error> {
 
     Ok(sources
         .iter()
-        .map(|x| SocketAddr::new(*x, crate::BGP_PORT.get()))
+        .map(|x| SocketAddr::new(*x, local_port))
         .collect())
 }
 
@@ -1410,8 +1423,17 @@ fn any_port(mut s: SocketAddr) -> SocketAddr {
     s
 }
 
-/// Generate all four bidirectional SA pairs for MD5 protection.
-/// Covers both directions of traffic with both port configurations
+/// Generate PF_KEY TCP-MD5 selectors for both possible BGP TCP 4-tuples.
+///
+/// `src` should use our BGP listening port. During outbound pre-connect setup
+/// this comes from session configuration because the socket is not connected
+/// yet; during inbound setup the dispatcher passes the same shared listen port
+/// used by every session.
+/// `dst` uses the peer's configured/listening port for outbound connections and
+/// the peer's observed client endpoint for inbound connections.
+///
+/// Port 0 is used as the PF_KEY wildcard for the side whose ephemeral client
+/// port is not known.
 #[cfg(target_os = "illumos")]
 fn sa_set(src: SocketAddr, dst: SocketAddr) -> [(SocketAddr, SocketAddr); 4] {
     // There are two directions of traffic we have to cover with two port
@@ -1510,6 +1532,7 @@ fn setup_outbound_md5(
     key: &str,
     peer_ip: IpAddr,
     peer: SocketAddr,
+    local_port: u16,
     logger: &Logger,
 ) -> Result<(String, Vec<SocketAddr>), Error> {
     let sources = source_address_select(peer_ip).map_err(|e| {
@@ -1537,7 +1560,7 @@ fn setup_outbound_md5(
 
     let local: Vec<SocketAddr> = sources
         .iter()
-        .map(|x| SocketAddr::new(*x, crate::BGP_PORT.get()))
+        .map(|x| SocketAddr::new(*x, local_port))
         .collect();
 
     init_md5_associations(fd, key, local.clone(), peer)?;