Do not choose a route whose next hop cannot be reached (RFC 4271 §9.1.2.1)

Where routes go into the kernel, a route the kernel could not be given
is no longer taken into the table: its next hop is no neighbor on any
interface, or its prefix is a network the machine is itself on. Such a
route used to be able to win selection and then leave its prefix without
a route in the kernel, though another peer had one that worked.

It is held aside the way an RPKI-invalid route is, and by the same
means: judged on arrival against the interfaces as the kernel last
described them, looked for in the table when they change, and taken
from what is held when its next hop can be reached. The fib publishes
the interfaces for this; the next hop is resolved once for an UPDATE
and not for each of its routes.

The lab has a phase, nexthops: FRR's better route with a next hop that
is nowhere is held and BIRD's is installed, and they change places as
an address comes and goes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

bin/bgp/README.md

@@ -262,10 +262,18 @@ kernel {
   address in the network of one of the machine's interfaces that is up. For
   an IPv6 route with a link-local next hop, the interface is the one the
   peer is a neighbor on. A route whose next hop is further away (a route
-  from a peer in our own AS that did not set itself as next hop, say) is not
+  from a peer in our own AS that did not set itself as next hop, say)
-  installed, and neither is whatever it replaced: looking up the way to such
+  cannot be installed: looking up the way to such a next hop through other
-  a next hop through other routes is not done yet. The status page counts
+  routes is not done yet.
-  these, by reason.
+- **Routes that cannot be installed are not chosen.** With `kernel {}`, a
+  route whose next hop cannot be reached is not taken into the table, so
+  that it cannot be the best route of its prefix while another peer has
+  one that works (RFC 4271 §9.1.2.1). It is held aside, counted as refused
+  for "next hop unreachable" and shown as held on the status page, and
+  taken as soon as the machine's interfaces change so that it can be
+  reached; a route in the table whose next hop goes away is held in the
+  same way. Without `kernel {}` the daemon does not know what can be
+  reached, and takes every route.
 - **Nexthop objects.** Where the kernel has them (Linux 5.3 and later) a
   gateway is put there once, as a nexthop object, and routes name it; `ip
   route` shows such routes with `nhid`, and `ip nexthop` the objects. The
@@ -359,7 +367,8 @@ Routinator 0.15.2. What it covers:
 | `restart` | The daemon stopped in good order (kernel emptied, peers told) and killed (routes kept, stale one cleared by the next start) |
 | `scale` | A table from BIRD (`SCALE_ROUTES`, a million for the Internet's size) into the table and the kernel; the daemon killed and restarted under it; the table withdrawn |
 | `limits` | A peer whose role does not fit gets no session; a peer over its prefix limit gets Cease / Maximum Number of Prefixes and is held down |
-| `hijacks` | FRR announces our own prefix, a route whose next hop is BIRD, and the exchange LAN's own prefix: the first two are refused, the third stays out of the kernel |
+| `hijacks` | FRR announces our own prefix, a route whose next hop is BIRD, and the exchange LAN's own prefix: none is taken |
+| `nexthops` | A better route from FRR whose next hop cannot be reached is held and BIRD's is chosen and installed; they swap when the next hop becomes a neighbor, and swap back when it stops being one |
 | `v6only` | The exchange LAN without IPv4 addresses: one IPv6 session with each router for both families, IPv4 routes with IPv6 next hops (RFC 8950) in BGP and in the kernel, IPv4 pings across |
 | `linklocal` | The exchange LAN with link-local addresses only, and the same ones on a second interface: sessions with FRR (by interface, with radvd beside the daemon) and BIRD at `fe80::…%ix0`, both families in BGP and in the kernel, pings across, the interface taken down and up |
 

crates/fib/src/kernel.rs

@@ -977,6 +977,7 @@ impl Task {
         UNRESOLVED.set(self.plan.unresolved() as i64);
         REFUSED.set(self.plan.refused() as i64);
         let interfaces = interfaces_changed.then(|| {
+            self.fib.publish_interfaces(&self.interfaces);
             let mut interfaces: Vec<InterfaceStatus> = self
                 .interfaces
                 .links()

crates/fib/src/lib.rs

@@ -135,9 +135,14 @@ pub struct Status {
     pub interfaces: Vec<InterfaceStatus>,
 }
 
+/// The machine's interfaces as the kernel last described them; `None`
+/// until it has. Whoever holds one is told when they change.
+pub type InterfacesWatch = watch::Receiver<Option<Arc<interfaces::Interfaces>>>;
+
 struct Shared {
     config: FibConfig,
     status: Mutex<Status>,
+    interfaces: watch::Sender<Option<Arc<interfaces::Interfaces>>>,
     stop: watch::Sender<bool>,
     /// Whether the task has ended.
     ended: watch::Sender<bool>,
@@ -165,6 +170,7 @@ impl Fib {
             shared: Arc::new(Shared {
                 config,
                 status: Mutex::default(),
+                interfaces: watch::channel(None).0,
                 stop,
                 ended: watch::channel(false).0,
             }),
@@ -177,6 +183,19 @@ impl Fib {
         self.shared.config
     }
 
+    /// The machine's interfaces, for those who want to know whether a
+    /// next hop can be reached before the kernel is asked to use it.
+    pub fn interfaces(&self) -> InterfacesWatch {
+        self.shared.interfaces.subscribe()
+    }
+
+    #[cfg_attr(not(target_os = "linux"), allow(dead_code))]
+    fn publish_interfaces(&self, interfaces: &interfaces::Interfaces) {
+        self.shared
+            .interfaces
+            .send_replace(Some(Arc::new(interfaces.clone())));
+    }
+
     pub fn status(&self) -> Status {
         self.shared
             .status

crates/fib/src/plan.rs

@@ -97,6 +97,19 @@ pub fn resolve(
     if interfaces.has_network_around(prefix) {
         return Err(Unresolved::OwnNetwork);
     }
+    resolve_next_hop(interfaces, prefix.address_family(), next_hop, peer)
+}
+
+/// The part of [`resolve`] that is the same for every route of IP version
+/// `routes` with one next hop from one peer: where the next hop is. For
+/// those who ask it of many such routes, and ask of each only whether it
+/// is for one of the machine's own networks.
+pub fn resolve_next_hop(
+    interfaces: &Interfaces,
+    routes: IpVersion,
+    next_hop: Option<&NlriNextHop>,
+    peer: Option<Neighbor>,
+) -> Result<Target, Unresolved> {
     let next_hop = next_hop.ok_or(Unresolved::Missing)?;
     let version = match next_hop {
         NlriNextHop::Ipv4(_) => IpVersion::Ipv4,
@@ -105,7 +118,7 @@ pub fn resolve(
     // An IPv4 route may have an IPv6 next hop: that is how routes go over
     // a link with only IPv6 addresses (RFC 8950), and the kernel takes
     // such a route. The other way about means nothing.
-    if version != prefix.address_family() && version == IpVersion::Ipv4 {
+    if version != routes && version == IpVersion::Ipv4 {
         return Err(Unresolved::OtherFamily);
     }
     let direct = |gateway: IpAddr| {

crates/integration-tests/tests/kernel.rs

@@ -142,9 +142,20 @@ async fn test_routes_from_a_session_reach_the_kernel() -> Result<()> {
     assert_eq!(kernel["enabled"], true);
     assert_eq!(kernel["table"], TABLE);
     assert_eq!(kernel["installed"], 2);
+    // The route by way of a router that is no neighbor is not in the
+    // table for the kernel to be asked for: it is held, in case the
+    // router becomes one.
+    assert_eq!(kernel["unresolved"], serde_json::json!({}));
+    let session = server.registry.list().pop().expect("the session");
+    assert_eq!(session.counters.held(), 1);
+    assert_eq!(server.rib.len(), 2);
+    let refused: Value = reqwest::get(format!("{base}/api/routes/rejected"))
+        .await?
+        .json()
+        .await?;
     assert_eq!(
-        kernel["unresolved"],
+        refused[0]["by_reason"],
-        serde_json::json!({"next hop not directly connected": 1})
+        serde_json::json!({"next hop unreachable": 1})
     );
     let shown = kernel["interfaces"]
         .as_array()

crates/server/src/connection.rs

@@ -13,6 +13,7 @@ use std::pin::Pin;
 use std::sync::Arc;
 use std::time::Duration;
 
+use bgp_fib::InterfacesWatch;
 use bgp_packet::errors::{BgpError, DecodeError, MsgHeaderSubcode};
 use bgp_packet::message::RouteRefreshMessage;
 use bgp_packet::message::{
@@ -45,6 +46,7 @@ use crate::metrics::{
 };
 use crate::panic::contained;
 use crate::policy::{self, HeldDown, HoldDowns, ImportPolicy, PeerRole};
+use crate::reach::{Reach, neighbor_at};
 use crate::refusals::{PeerRefusals, RefusalLog};
 use crate::routes::{self, Held};
 use crate::rpki::{Checks, Subject, Validator};
@@ -82,6 +84,8 @@ pub struct ServerContext {
     pub refusals: RefusalLog,
     /// The data for route origin validation, where that is configured.
     pub rpki: Option<Rpki>,
+    /// The machine's interfaces, where routes are installed in the kernel.
+    pub interfaces: Option<InterfacesWatch>,
 }
 
 /// What is configured for the peer at the other end of a connection.
@@ -787,14 +791,20 @@ impl Session {
             negotiated.peer_asn,
             negotiated.ibgp,
         );
-        let validation = self
+        // And where routes go into the kernel, a route has to have a next
+        // hop the kernel can use.
+        let reach = self
             .server
-            .rpki
+            .interfaces
             .clone()
-            .zip(subject)
+            .map(|interfaces| Reach::new(interfaces, neighbor_at(self.peer_addr)));
-            .map(|(rpki, subject)| Validation {
+        let subject = subject.filter(|_| self.server.rpki.is_some());
-                validator: Validator::new(rpki, subject),
+        let validation =
-                changes: handle.validate(subject),
+            Validator::new(self.server.rpki.clone().zip(subject), reach).map(|validator| {
+                Validation {
+                    validator,
+                    changes: handle.validate(subject),
+                }
             });
         self.established = Some(Established {
             since: Instant::now(),
@@ -1162,6 +1172,7 @@ mod tests {
             registry: SessionRegistry::new(),
             collisions: CollisionTable::new(false),
             hold_downs: HoldDowns::default(),
+            interfaces: None,
             refusals: RefusalLog::default(),
             rpki: None,
         });
@@ -1210,6 +1221,7 @@ mod tests {
             registry: SessionRegistry::new(),
             collisions: CollisionTable::new(false),
             hold_downs: HoldDowns::default(),
+            interfaces: None,
             refusals: RefusalLog::default(),
             rpki: None,
         });

crates/server/src/lib.rs

@@ -10,6 +10,7 @@ mod fsm;
 pub mod metrics;
 mod panic;
 pub mod policy;
+mod reach;
 pub mod refusals;
 mod routes;
 pub mod rpki;

crates/server/src/policy.rs

@@ -185,10 +185,23 @@ pub enum Rejection {
     NextHopNotPeer,
     /// The prefix is one of ours, or inside one.
     OwnPrefix,
+    /// The next hop cannot be reached from this machine as its interfaces
+    /// are now, so the kernel could not be given the route.
+    NextHopUnreachable,
 }
 
 impl Rejection {
-    pub const ALL: [Self; 16] = [
+    /// Whether the reason may pass by itself, with the route unchanged:
+    /// the RPKI's data changes, and so do the machine's interfaces. A
+    /// route refused for such a reason is held, to be taken when it does.
+    pub(crate) fn may_pass(self) -> bool {
+        matches!(
+            self,
+            Self::RpkiInvalid | Self::AspaInvalid | Self::NextHopUnreachable
+        )
+    }
+
+    pub const ALL: [Self; 17] = [
         Self::BogonPrefix,
         Self::TooShort,
         Self::TooSpecific,
@@ -205,6 +218,7 @@ impl Rejection {
         Self::NextHopUnusable,
         Self::NextHopNotPeer,
         Self::OwnPrefix,
+        Self::NextHopUnreachable,
     ];
 
     /// Where the reason is in [`Rejection::ALL`].
@@ -230,6 +244,7 @@ impl Rejection {
             Self::NextHopUnusable => "next hop unusable",
             Self::NextHopNotPeer => "next hop is not the peer",
             Self::OwnPrefix => "prefix is our own",
+            Self::NextHopUnreachable => "next hop unreachable",
         }
     }
 }

crates/server/src/reach.rs

@@ -0,0 +1,438 @@
+//! Whether a route's next hop can be reached at all.
+//!
+//! A route with a next hop that packets cannot be sent to is not to be
+//! chosen over one that works (RFC 4271 §9.1.2.1). What can be reached is
+//! known where the daemon installs routes in the kernel, which is where
+//! it matters: it is what the kernel says of the machine's interfaces.
+//! There, a route the kernel could not be given is not taken into the
+//! table, so that it cannot be the best route of its prefix; it is held
+//! aside like a route the RPKI finds invalid, and looked at again when
+//! the interfaces change (see `crate::routes::Held`).
+//!
+//! Only next hops that are neighbors on one of the machine's interfaces
+//! count as reachable: the way to any other is not looked up. Routes from
+//! a peer in our own AS that did not set itself as next hop are therefore
+//! held, where they used to be chosen and then not installed.
+
+use std::cell::RefCell;
+use std::collections::HashMap;
+use std::net::SocketAddr;
+use std::sync::Arc;
+use std::time::Duration;
+
+use bgp_fib::InterfacesWatch;
+use bgp_fib::interfaces::Interfaces;
+use bgp_fib::plan::{Neighbor, resolve, resolve_next_hop};
+use bgp_packet::message::NlriNextHop;
+use bgp_primitives::constants::IpVersion;
+use bgp_primitives::ip_prefix::{IpPrefix, Nlri};
+use bgp_rib::table::{PeerIndex, RibTable};
+use tokio::task::JoinHandle;
+use tracing::{info, warn};
+
+use crate::address::PeerAddress;
+use crate::session::SessionRegistry;
+
+/// How long after the interfaces change the table is looked through:
+/// such changes come in bursts, and a session in the middle of an UPDATE
+/// judged it by the interfaces as they were.
+const SETTLE: Duration = Duration::from_millis(500);
+
+/// The interfaces to judge one peer's next hops by.
+#[derive(Clone, Copy)]
+pub(crate) struct NextHops<'a> {
+    pub(crate) interfaces: &'a Interfaces,
+    /// The peer: where a link-local next hop is.
+    pub(crate) neighbor: Neighbor,
+}
+
+impl NextHops<'_> {
+    /// Whether the kernel can be given a route for `prefix` by way of
+    /// `next_hop`.
+    pub(crate) fn reaches(&self, prefix: &IpPrefix, next_hop: Option<&NlriNextHop>) -> bool {
+        resolve(self.interfaces, prefix, next_hop, Some(self.neighbor)).is_ok()
+    }
+
+    /// The two halves of [`NextHops::reaches`], for an UPDATE's worth of
+    /// routes with one next hop: whether the next hop is somewhere, which
+    /// is asked once, and whether a prefix is one the machine reaches by
+    /// itself, which is asked of each.
+    pub(crate) fn next_hop_is_near(&self, routes: IpVersion, next_hop: &NlriNextHop) -> bool {
+        resolve_next_hop(self.interfaces, routes, Some(next_hop), Some(self.neighbor)).is_ok()
+    }
+
+    pub(crate) fn is_own_network(&self, prefix: &IpPrefix) -> bool {
+        self.interfaces.has_network_around(prefix)
+    }
+}
+
+/// A session's view of the interfaces: those in use, replaced when the
+/// kernel says they have changed. Asking whether they have costs the load
+/// of one number, which is done for every UPDATE.
+pub(crate) struct Reach {
+    watch: InterfacesWatch,
+    neighbor: Neighbor,
+    interfaces: Option<Arc<Interfaces>>,
+}
+
+impl Reach {
+    pub(crate) fn new(watch: InterfacesWatch, neighbor: Neighbor) -> Self {
+        let interfaces = watch.borrow().clone();
+        Self {
+            watch,
+            neighbor,
+            interfaces,
+        }
+    }
+
+    /// What to judge next hops by now; nothing until the kernel has said
+    /// what interfaces there are.
+    pub(crate) fn next_hops(&mut self) -> Option<NextHops<'_>> {
+        if self.watch.has_changed().unwrap_or(false) {
+            self.interfaces = self.watch.borrow_and_update().clone();
+        }
+        self.interfaces.as_deref().map(|interfaces| NextHops {
+            interfaces,
+            neighbor: self.neighbor,
+        })
+    }
+}
+
+/// Starts the task that, whenever the machine's interfaces change, finds
+/// the routes in the table whose next hop can no longer be reached and has
+/// their sessions take them out, and has every session look again at the
+/// routes it holds.
+pub(crate) fn spawn_revalidator(
+    mut watch: InterfacesWatch,
+    rib: Arc<RibTable>,
+    registry: SessionRegistry,
+) -> JoinHandle<()> {
+    tokio::spawn(async move {
+        // Ends only if the task that publishes the interfaces has gone,
+        // which the server notices for itself.
+        while watch.changed().await.is_ok() {
+            tokio::time::sleep(SETTLE).await;
+            let Some(interfaces) = watch.borrow_and_update().clone() else {
+                continue;
+            };
+            let sessions = registry.list();
+            // By peer, for looking up as often as there are routes.
+            let mut neighbors: Vec<Option<Neighbor>> = Vec::new();
+            for session in &sessions {
+                let at = session.rib_peer.as_usize();
+                if neighbors.len() <= at {
+                    neighbors.resize(at + 1, None);
+                }
+                neighbors[at] = Some(neighbor_at(session.peer.addr));
+            }
+            let table = Arc::clone(&rib);
+            // Looks at every route: not for a thread that sessions run on.
+            let found = tokio::task::spawn_blocking(move || {
+                now_unreachable(&table, &interfaces, &neighbors)
+            })
+            .await;
+            let mut suspects = match found {
+                Ok(suspects) => suspects,
+                Err(e) => {
+                    warn!(err = %e, "Looking for routes whose next hop is gone failed");
+                    HashMap::new()
+                }
+            };
+            info!(
+                now_unreachable = suspects.values().map(Vec::len).sum::<usize>(),
+                sessions = sessions.len(),
+                "The machine's interfaces changed; looking at next hops again"
+            );
+            // Every session, also those with nothing found: each has its
+            // held routes to look at again.
+            for session in sessions {
+                session.revalidate(suspects.remove(&session.rib_peer).unwrap_or_default());
+            }
+        }
+    })
+}
+
+/// The peer at `address`, as far as its next hops are concerned.
+pub(crate) fn neighbor_at(address: SocketAddr) -> Neighbor {
+    let address = PeerAddress::from(address);
+    Neighbor {
+        address: address.ip,
+        interface: address.interface,
+    }
+}
+
+/// The routes in the table that the kernel could not be given, as the
+/// interfaces are now, by peer. `neighbors` says, for each peer by its
+/// number in the table, where it is.
+fn now_unreachable(
+    rib: &RibTable,
+    interfaces: &Interfaces,
+    neighbors: &[Option<Neighbor>],
+) -> HashMap<PeerIndex, Vec<Nlri>> {
+    // There are as many next hops as neighbors, and a table's worth of
+    // routes share them: each is worked out once.
+    type Key = (Option<NlriNextHop>, IpVersion, PeerIndex);
+    let resolved: RefCell<HashMap<Key, bool>> = RefCell::default();
+    let routes = rib.routes_where(usize::MAX, |route| {
+        let Some(Some(neighbor)) = neighbors.get(route.peer.as_usize()) else {
+            return false;
+        };
+        if interfaces.has_network_around(&route.prefix) {
+            return true;
+        }
+        let next_hop = route.attrs.next_hop();
+        let key = (next_hop, route.prefix.address_family(), route.peer);
+        !*resolved.borrow_mut().entry(key).or_insert_with(|| {
+            resolve(
+                interfaces,
+                &route.prefix,
+                next_hop.as_ref(),
+                Some(*neighbor),
+            )
+            .is_ok()
+        })
+    });
+    let mut by_peer: HashMap<PeerIndex, Vec<Nlri>> = HashMap::new();
+    for route in routes {
+        by_peer.entry(route.peer).or_default().push(Nlri {
+            prefix: route.prefix,
+            path_id: route.path_id,
+        });
+    }
+    by_peer
+}
+
+#[cfg(test)]
+mod tests {
+    use std::net::{IpAddr, Ipv4Addr};
+
+    use bgp_fib::netlink::{Address, Link, Message};
+    use bgp_packet::message::{
+        AsPathAttribute, NextHopPathAttribute, OriginPathAttribute, PathAttribute, UpdateMessage,
+    };
+    use bgp_rib::table::PeerInfo;
+
+    use super::*;
+    use crate::policy::{ImportPolicy, Rejection, RouteSource};
+    use crate::refusals::RefusalLog;
+    use crate::routes::{self, Held, Import};
+    use crate::rpki::Checks;
+
+    fn prefix(s: &str) -> IpPrefix {
+        IpPrefix::try_from(s).unwrap()
+    }
+
+    /// A machine with eth2 (192.0.2.1/24), and eth3 (198.51.100.1/24) if
+    /// `both`.
+    fn interfaces(both: bool) -> Interfaces {
+        let mut interfaces = Interfaces::default();
+        let all = [
+            (2, "192.0.2.1", "192.0.2.0/24"),
+            (3, "198.51.100.1", "198.51.100.0/24"),
+        ];
+        for (index, local, network) in all.into_iter().take(if both { 2 } else { 1 }) {
+            interfaces.apply(&Message::Link {
+                present: true,
+                link: Link {
+                    index,
+                    name: format!("eth{index}"),
+                    up: true,
+                },
+            });
+            interfaces.apply(&Message::Address {
+                present: true,
+                address: Address {
+                    interface: index,
+                    local: local.parse().unwrap(),
+                    network: prefix(network),
+                },
+            });
+        }
+        interfaces
+    }
+
+    struct Peer {
+        index: PeerIndex,
+        address: Ipv4Addr,
+        asn: u32,
+        held: Held,
+    }
+
+    struct Bed {
+        table: RibTable,
+        policy: ImportPolicy,
+        refusals: RefusalLog,
+    }
+
+    impl Bed {
+        fn new() -> Self {
+            Self {
+                table: RibTable::new(4),
+                policy: ImportPolicy::permissive(),
+                refusals: RefusalLog::default(),
+            }
+        }
+
+        fn peer(&self, address: &str, asn: u32) -> Peer {
+            let address: Ipv4Addr = address.parse().unwrap();
+            Peer {
+                index: self
+                    .table
+                    .add_peer(PeerInfo {
+                        asn,
+                        router_id: address,
+                        address: address.into(),
+                        interface: 0,
+                        ibgp: false,
+                    })
+                    .unwrap(),
+                address,
+                asn,
+                held: Held::default(),
+            }
+        }
+
+        /// What `work` is given: the peer's routes as a session judges
+        /// them, with `interfaces` to go by.
+        fn with_import<T>(
+            &self,
+            peer: &mut Peer,
+            interfaces: &Interfaces,
+            work: impl FnOnce(&RibTable, &Import<'_>, &mut Held) -> T,
+        ) -> T {
+            let address = IpAddr::V4(peer.address);
+            let import = Import {
+                peer: peer.index,
+                from: RouteSource {
+                    peer_asn: peer.asn,
+                    local_asn: 65000,
+                    ibgp: false,
+                    four_octet_asn: true,
+                    role: None,
+                    peer_address: address,
+                    local_address: Ipv4Addr::new(192, 0, 2, 1).into(),
+                    extended_next_hop: false,
+                },
+                policy: &self.policy,
+                checks: Checks {
+                    next_hops: Some(NextHops {
+                        interfaces,
+                        neighbor: address.into(),
+                    }),
+                    ..Checks::NONE
+                },
+                refusals: &self.refusals.of_peer(address.into(), "test"),
+            };
+            work(&self.table, &import, &mut peer.held)
+        }
+
+        /// The peer announces `route` with a path of `hops` AS numbers and
+        /// itself as next hop.
+        fn announce(&self, peer: &mut Peer, interfaces: &Interfaces, route: &str, hops: usize) {
+            let update = UpdateMessage {
+                withdrawn_routes: vec![],
+                path_attributes: vec![
+                    PathAttribute::Origin(OriginPathAttribute::IGP),
+                    AsPathAttribute::from_asns(vec![peer.asn; hops]),
+                    PathAttribute::NextHop(NextHopPathAttribute(peer.address)),
+                ],
+                announced_routes: vec![prefix(route).into()],
+            };
+            self.with_import(peer, interfaces, |table, import, held| {
+                routes::apply(table, import, held, &update, None).unwrap()
+            });
+        }
+
+        fn revalidate(&self, peer: &mut Peer, interfaces: &Interfaces, suspects: &[Nlri]) {
+            self.with_import(peer, interfaces, |table, import, held| {
+                routes::revalidate(table, import, held, suspects)
+            });
+        }
+
+        fn best(&self, route: &str) -> Option<PeerIndex> {
+            self.table.best(&prefix(route)).map(|(peer, _)| peer)
+        }
+    }
+
+    /// RFC 4271 §9.1.2.1: a route whose next hop cannot be reached is not
+    /// chosen, however good it looks; and it is when it can be.
+    #[test]
+    fn a_route_is_in_the_running_only_while_its_next_hop_can_be_reached() {
+        let bed = Bed::new();
+        let one_link = interfaces(false);
+        let two_links = interfaces(true);
+        // The better route is from the peer on the link that is not there.
+        let mut near = bed.peer("192.0.2.2", 65001);
+        let mut far = bed.peer("198.51.100.2", 65002);
+        let route = "203.0.113.0/24";
+        bed.announce(&mut near, &one_link, route, 3);
+        bed.announce(&mut far, &one_link, route, 1);
+        assert_eq!(bed.best(route), Some(near.index));
+        assert_eq!(bed.table.len(), 1, "the other is not in the table at all");
+        assert_eq!((near.held.len(), far.held.len()), (0, 1));
+        let refused = bed.refusals.peers();
+        assert_eq!(refused.len(), 1);
+        assert_eq!(
+            refused[0].1.by_reason(),
+            [(Rejection::NextHopUnreachable, 1)]
+        );
+
+        // The link comes: nothing in the table is wrong, and what was
+        // held is taken, and is the better route.
+        let neighbors = |peers: &[&Peer]| {
+            let mut by_index = vec![None; 8];
+            for peer in peers {
+                by_index[peer.index.as_usize()] = Some(Neighbor::from(IpAddr::V4(peer.address)));
+            }
+            by_index
+        };
+        let both = neighbors(&[&near, &far]);
+        assert!(now_unreachable(&bed.table, &two_links, &both).is_empty());
+        bed.revalidate(&mut near, &two_links, &[]);
+        bed.revalidate(&mut far, &two_links, &[]);
+        assert_eq!(bed.best(route), Some(far.index));
+        assert_eq!((bed.table.len(), far.held.len()), (2, 0));
+
+        // The link goes: the route by way of it is found in the table,
+        // taken out and held, and the other is the best again.
+        let mut suspects = now_unreachable(&bed.table, &one_link, &both);
+        assert_eq!(
+            suspects.keys().copied().collect::<Vec<_>>(),
+            [far.index],
+            "only the routes of the peer that is cut off"
+        );
+        let suspects = suspects.remove(&far.index).unwrap();
+        assert_eq!(suspects, [Nlri::from(prefix(route))]);
+        bed.revalidate(&mut far, &one_link, &suspects);
+        bed.revalidate(&mut near, &one_link, &[]);
+        assert_eq!(bed.best(route), Some(near.index));
+        assert_eq!((bed.table.len(), far.held.len()), (1, 1));
+
+        // A peer that withdraws a held route does not get it back.
+        let withdrawal = UpdateMessage {
+            withdrawn_routes: vec![prefix(route).into()],
+            path_attributes: vec![],
+            announced_routes: vec![],
+        };
+        bed.with_import(&mut far, &one_link, |table, import, held| {
+            routes::apply(table, import, held, &withdrawal, None).unwrap()
+        });
+        bed.revalidate(&mut far, &two_links, &[]);
+        assert_eq!(bed.best(route), Some(near.index));
+        assert_eq!(far.held.len(), 0);
+    }
+
+    /// A route for a network the machine is on is not one the kernel is
+    /// given either, and so not one to choose.
+    #[test]
+    fn a_route_for_one_of_our_own_networks_is_held() {
+        let bed = Bed::new();
+        let interfaces = interfaces(true);
+        let mut peer = bed.peer("192.0.2.2", 65001);
+        bed.announce(&mut peer, &interfaces, "198.51.100.128/25", 1);
+        bed.announce(&mut peer, &interfaces, "203.0.113.0/24", 1);
+        assert_eq!(bed.table.len(), 1);
+        assert_eq!(peer.held.len(), 1);
+        assert_eq!(bed.best("198.51.100.128/25"), None);
+    }
+}

crates/server/src/routes.rs

@@ -216,18 +216,25 @@ impl Applying<'_> {
         // Whatever becomes of them, these replace what was held of the
         // same routes.
         self.held.forget(nlri);
-        let origin = match self.judge_together(first.prefix.address_family(), next_hop) {
+        let family = first.prefix.address_family();
-            Ok(origin) => origin,
+        let (origin, used_next_hop) = match self.judge_together(family, next_hop) {
+            Ok(judged) => judged,
             Err(rejection) => {
                 self.refuse(nlri, rejection);
                 return Ok(());
             }
         };
+        // Asked once for all of them: they have the one next hop.
+        let near = self
+            .import
+            .checks
+            .next_hops
+            .is_none_or(|next_hops| next_hops.next_hop_is_near(family, &used_next_hop));
         // Nearly always every route is taken, and then nothing is copied.
         let mut accepted = Cow::Borrowed(nlri);
         let mut invalid = Vec::new();
         for (at, route) in nlri.iter().enumerate() {
-            match self.judge(route, origin) {
+            match self.judge(route, origin, near) {
                 Ok(()) => {
                     if let Cow::Owned(accepted) = &mut accepted {
                         accepted.push(*route);
@@ -237,7 +244,7 @@ impl Applying<'_> {
                     if let Cow::Borrowed(_) = accepted {
                         accepted = Cow::Owned(nlri[..at].to_vec());
                     }
-                    if matches!(rejection, Rejection::RpkiInvalid | Rejection::AspaInvalid) {
+                    if rejection.may_pass() {
                         invalid.push(*route);
                     }
                     self.refuse(std::slice::from_ref(route), rejection);
@@ -259,12 +266,12 @@ impl Applying<'_> {
 
     /// Judges what routes announced together have in common: the path,
     /// and the next hop. `routes` is their IP version. Returns the AS they
-    /// originate from, if the path says.
+    /// originate from, if the path says, and their next hop.
     fn judge_together(
         &self,
         routes: IpVersion,
         next_hop: Option<&NlriNextHop>,
-    ) -> Result<Option<u32>, Rejection> {
+    ) -> Result<(Option<u32>, NlriNextHop), Rejection> {
         let origin = self.path_verdict?;
         let in_attributes = || {
             self.update
@@ -283,11 +290,13 @@ impl Applying<'_> {
         self.import
             .policy
             .check_next_hop(&next_hop, routes, &self.import.from)?;
-        Ok(origin)
+        Ok((origin, next_hop))
     }
 
-    /// Judges one route, whose origin is `origin`.
+    /// Judges one route, whose origin is `origin`; `near` says whether
-    fn judge(&self, route: &Nlri, origin: Option<u32>) -> Result<(), Rejection> {
+    /// its next hop can be reached. What is wrong with the route for good
+    /// comes before what may pass.
+    fn judge(&self, route: &Nlri, origin: Option<u32>, near: bool) -> Result<(), Rejection> {
         let import = self.import;
         import.policy.check_prefix(&route.prefix)?;
         let origin_invalid = import
@@ -303,6 +312,14 @@ impl Applying<'_> {
         {
             return Err(Rejection::AspaInvalid);
         }
+        let reachable = near
+            && !import
+                .checks
+                .next_hops
+                .is_some_and(|next_hops| next_hops.is_own_network(&route.prefix));
+        if !reachable {
+            return Err(Rejection::NextHopUnreachable);
+        }
         Ok(())
     }
 
@@ -382,7 +399,7 @@ pub(crate) fn revalidate(
 ) -> Applied {
     let mut applied = Applied::default();
     let checks = &import.checks;
-    if checks.vrps.is_none() && checks.aspas.is_none() {
+    if checks.is_none() {
         return applied;
     }
     let mut path = Vec::new();

crates/server/src/rpki.rs

@@ -29,6 +29,7 @@ use tracing::{info, warn};
 
 use crate::metrics::{RPKI_ASPAS, RPKI_UPDATES, RPKI_VRPS};
 use crate::policy::{ImportPolicy, PeerRole, Rejection};
+use crate::reach::{NextHops, Reach};
 use crate::session::{SessionHandle, SessionRegistry};
 
 /// How long after new data is published the table is looked through. A
@@ -117,6 +118,9 @@ impl Subject {
 pub(crate) struct Checks<'a> {
     pub(crate) vrps: Option<&'a VrpTable>,
     pub(crate) aspas: Option<(&'a AspaTable, PathCheck)>,
+    /// The machine's interfaces, where routes are installed in the kernel
+    /// and a next hop therefore has to be one that can be reached.
+    pub(crate) next_hops: Option<NextHops<'a>>,
 }
 
 impl<'a> Checks<'a> {
@@ -124,13 +128,12 @@ impl<'a> Checks<'a> {
     pub(crate) const NONE: Self = Self {
         vrps: None,
         aspas: None,
+        next_hops: None,
     };
 
-    fn new(subject: Subject, data: &'a Update) -> Self {
+    /// Whether there is nothing to check.
-        Self {
+    pub(crate) fn is_none(&self) -> bool {
-            vrps: subject.origin.then_some(&*data.vrps),
+        self.vrps.is_none() && self.aspas.is_none() && self.next_hops.is_none()
-            aspas: subject.path.map(|check| (&*data.aspas, check)),
-        }
     }
 
     /// Why a route for `prefix` with the attributes `attrs` is invalid,
@@ -155,35 +158,49 @@ impl<'a> Checks<'a> {
                 return Some(Rejection::AspaInvalid);
             }
         }
+        if let Some(next_hops) = self.next_hops
+            && !next_hops.reaches(prefix, attrs.next_hop().as_ref())
+        {
+            return Some(Rejection::NextHopUnreachable);
+        }
         None
     }
 }
 
-/// A session's view of the validation data: the data in use, replaced when
+/// A session's view of what its peer's routes are checked against and
-/// newer is published. Asking whether there is newer costs the load of one
+/// that changes while the session lasts: the RPKI's data and the
-/// number, which is done for every UPDATE.
+/// machine's interfaces, each as in use, replaced when there is newer.
+/// Asking whether there is costs the load of a number, which is done for
+/// every UPDATE.
 pub(crate) struct Validator {
-    rpki: Rpki,
+    rpki: Option<(Rpki, Subject, Update)>,
-    subject: Subject,
+    reach: Option<Reach>,
-    data: Update,
 }
 
 impl Validator {
-    pub(crate) fn new(rpki: Rpki, subject: Subject) -> Self {
+    /// `None` if the peer's routes are subject to none of the checks.
-        let data = rpki.current();
+    pub(crate) fn new(rpki: Option<(Rpki, Subject)>, reach: Option<Reach>) -> Option<Self> {
-        Self {
+        let rpki = rpki.map(|(rpki, subject)| {
-            rpki,
+            let data = rpki.current();
-            subject,
+            (rpki, subject, data)
-            data,
+        });
-        }
+        (rpki.is_some() || reach.is_some()).then_some(Self { rpki, reach })
     }
 
     /// What to validate against now.
     pub(crate) fn checks(&mut self) -> Checks<'_> {
-        if self.rpki.version() != self.data.version {
+        let mut checks = Checks::NONE;
-            self.data = self.rpki.current();
+        if let Some((rpki, subject, data)) = &mut self.rpki {
+            if rpki.version() != data.version {
+                *data = rpki.current();
+            }
+            checks.vrps = subject.origin.then_some(&*data.vrps);
+            checks.aspas = subject.path.map(|check| (&*data.aspas, check));
         }
-        Checks::new(self.subject, &self.data)
+        if let Some(reach) = &mut self.reach {
+            checks.next_hops = reach.next_hops();
+        }
+        checks
     }
 }
 

crates/server/src/server.rs

@@ -28,6 +28,7 @@ use crate::error::ServerError;
 use crate::metrics::{CONNECTIONS_REFUSED, PEER_MAX_PREFIXES};
 use crate::panic;
 use crate::policy::HoldDowns;
+use crate::reach;
 use crate::refusals::RefusalLog;
 use crate::rpki::spawn_revalidator;
 use crate::session::{ConfiguredPeer, SessionRegistry};
@@ -192,6 +193,7 @@ impl BgpServer {
             hold_downs: HoldDowns::default(),
             refusals: self.refusals.clone(),
             rpki: self.rpki.clone(),
+            interfaces: self.kernel.as_ref().map(Fib::interfaces),
         });
 
         // The tasks the server cannot do without. None of them ends while
@@ -214,6 +216,12 @@ impl BgpServer {
                 spawn_revalidator(rpki, Arc::clone(&self.rib), self.registry.clone()),
             ));
         }
+        if let Some(interfaces) = context.interfaces.clone() {
+            tasks.push(named(
+                "next hop revalidator",
+                reach::spawn_revalidator(interfaces, Arc::clone(&self.rib), self.registry.clone()),
+            ));
+        }
         for dialer in spawn_dialers(&self.config.peers, &context, self.config.connect_retry) {
             tasks.push(named("dialer", dialer));
         }

crates/server/src/session.rs

@@ -148,7 +148,7 @@ pub struct SessionHandle {
     outbound: mpsc::Sender<Message>,
     /// Which of the RPKI's checks the peer's routes are subject to, if
     /// any, and where the session is told to look at its routes again.
-    validation: Option<(Subject, mpsc::UnboundedSender<Vec<Nlri>>)>,
+    validation: Option<(Option<Subject>, mpsc::UnboundedSender<Vec<Nlri>>)>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
@@ -189,7 +189,10 @@ impl SessionHandle {
     /// Marks the session's routes as subject to validation with the
     /// RPKI's data. What is sent with [`SessionHandle::revalidate`]
     /// arrives at the receiver returned.
-    pub(crate) fn validate(&mut self, subject: Subject) -> mpsc::UnboundedReceiver<Vec<Nlri>> {
+    pub(crate) fn validate(
+        &mut self,
+        subject: Option<Subject>,
+    ) -> mpsc::UnboundedReceiver<Vec<Nlri>> {
         let (tx, rx) = mpsc::unbounded_channel();
         self.validation = Some((subject, tx));
         rx
@@ -198,7 +201,7 @@ impl SessionHandle {
     /// Which of the RPKI's checks the session's routes are subject to, if
     /// any.
     pub fn validated(&self) -> Option<Subject> {
-        self.validation.as_ref().map(|(subject, _)| *subject)
+        self.validation.as_ref().and_then(|(subject, _)| *subject)
     }
 
     /// Tells the session that the validation data has changed. `suspects`

docs/RFCs.md

@@ -33,7 +33,8 @@ The base protocol specification.
 | §6.2 | Bad BGP Identifier check (peer using our identifier, or 0.0.0.0) | ✅ |
 | §9 | Decision process / best-path selection | ✅ |
 | §9.1.2 | Best routes installed for forwarding (Linux kernel, over netlink), where the next hop is directly connected | ✅ |
-| §9.1.2.1 | Routes with an unresolvable next hop excluded from selection; next hops resolved through other routes | ❌ (such a route can be chosen, and is then not installed) |
+| §9.1.2.1 | Routes with an unresolvable next hop excluded from selection | ✅ where routes are installed in the kernel: such a route is held out of the table until its next hop can be reached |
+| §9.1.2.1 | Next hops resolved through other routes | ❌ (only next hops that are neighbors count as reachable) |
 | §9 | Adj-RIB-In (per-peer table of received routes) | ✅ |
 | §9 | Locally originated routes, advertised to all established peers | ✅ |
 | §9 | Loc-RIB / Adj-RIB-Out for routes learned from peers (re-advertisement) | ❌ |

scripts/interop/lab.sh

@@ -29,7 +29,7 @@ cd "$(dirname "$0")/../.."
 bgp="${CARGO_TARGET_DIR:-target}/release/bgp"
 work=/tmp/ferrix-lab
 pathspace=ferrixlab
-all_phases="basics roles rpki refresh flaps restart scale limits hijacks v6only linklocal"
+all_phases="basics roles rpki refresh flaps restart scale limits hijacks nexthops v6only linklocal"
 phases="${PHASES:-$all_phases}"
 scale_routes="${SCALE_ROUTES:-200000}"
 
@@ -129,9 +129,9 @@ END
 
 # --- the daemon under test ------------------------------------------------------
 
-# dut_config [frr-role] [frr-max-prefixes]
+# dut_config [frr-role] [frr-max-prefixes] [more of frr4's import policy]
 dut_config() {
-    local frr_role=${1:-PEER} frr_limit=${2:-2000000}
+    local frr_role=${1:-PEER} frr_limit=${2:-2000000} frr_import=${3:-}
     cat > "$work/dut.textpb" <<END
 router_id: "10.9.0.1"
 local_asn: 64601
@@ -142,7 +142,7 @@ peers {
   name: "frr4" endpoint { address: "10.9.0.2" } remote_asn: 64602
   address_families: IPV4_UNICAST
   role: $frr_role
-  import { accept_bogons: true max_prefixes: $frr_limit hold_down_seconds: 5 }
+  import { accept_bogons: true max_prefixes: $frr_limit hold_down_seconds: 5 $frr_import }
 }
 peers {
   name: "frr6" endpoint { address: "2001:db8:9::2" } remote_asn: 64602
@@ -751,22 +751,74 @@ phase_hijacks() {
     dut_lacks 10.2.7.0/24 || fail "a route by way of a third party was taken"
     ok "a route whose next hop is someone else is not taken"
 
-    # The LAN's own prefix. The lab takes bogons, so the route is taken
+    # The LAN's own prefix. The lab takes bogons, so the policy lets it
-    # into the table; it must not get into the kernel, where it would
+    # by; it is still not a route the kernel could be given, and so not
-    # compete with the connected route.
+    # one to take: it would compete with the connected route.
-    wait_for "the LAN's prefix at the daemon" dut_has 10.9.0.0/24 frr4
+    wait_for "the LAN's prefix to be held" refused_for "next hop unreachable"
-    sleep 1
+    dut_lacks 10.9.0.0/24 || fail "the LAN's own prefix was taken from FRR"
     in_ns dut ip -4 route show proto bgp | grep -q "^10.9.0.0/24 " && fail "the LAN's own prefix was installed by way of a peer"
-    api /api/kernel | jq -e '.unresolved["inside a network of this machine"] >= 1' >/dev/null \
-        || fail "the kernel's status does not say why: $(api /api/kernel | jq -c .unresolved)"
     in_ns dut ping -q -c 1 -W 2 10.9.0.3 >/dev/null || fail "the LAN is not reachable any more"
-    ok "a route for the exchange LAN itself stays out of the kernel"
+    ok "a route for the exchange LAN itself is not taken, and stays out of the kernel"
 
     frr_configure \
         "router bgp 64602" "address-family ipv4 unicast" \
         "no network 10.1.0.0/24" "no network 10.9.0.0/24" "no network 10.2.7.0/24" \
         "no neighbor 10.9.0.1 route-map TO-DUT out"
-    wait_for "the LAN's prefix to be withdrawn" dut_lacks 10.9.0.0/24
+    converged
+    ok "back to normal"
+}
+
+# FRR and BIRD both have a route for one prefix, and FRR's is the better.
+# But its next hop is nowhere the machine can reach: then BIRD's is the
+# one chosen and installed (RFC 4271 §9.1.2.1), until the next hop is
+# somewhere, and again when it no longer is.
+best_is() {  # best_is prefix peer-name
+    api "/api/routes/received?prefix=$1&best=true" | jq -e --arg peer "$2" '.routes | length == 1 and .[0].peer_name == $peer'
+}
+phase_nexthops() {
+    dut_down TERM
+    dut_config PEER 2000000 "accept_third_party_next_hop: true"
+    dut_up
+    converged
+    # A prefix nobody has said anything about in the RPKI, from both.
+    echo "route 10.77.0.0/24 blackhole;" > "$work/bird-both.conf"
+    bird_config "$work/bird-both.conf"
+    bird_ctl configure >/dev/null
+    wait_for "BIRD's route for it" dut_has 10.77.0.0/24 bird4
+    frr_configure \
+        "ip prefix-list FAR seq 5 permit 10.77.0.0/24" \
+        "route-map TO-DUT permit 10" "match ip address prefix-list FAR" "set ip next-hop 10.250.0.1" \
+        "route-map TO-DUT permit 20" \
+        "router bgp 64602" "address-family ipv4 unicast" \
+        "neighbor 10.9.0.1 route-map TO-DUT out" "network 10.77.0.0/24"
+
+    wait_for "FRR's route to be held" refused_for "next hop unreachable"
+    peer_is frr4 '.held_routes >= 1' >/dev/null || fail "the route is not held: $(peer_is frr4 .)"
+    best_is 10.77.0.0/24 bird4 >/dev/null || fail "BIRD's route is not the one chosen"
+    dut_kernel -4 "10.77.0.0/24 via 10.9.0.3 dev ix0 metric 20" || fail "BIRD's route is not in the kernel"
+    ok "a better route whose next hop is nowhere is held; the one that works is chosen and installed"
+
+    # The next hop becomes a neighbor.
+    in_ns dut ip addr add 10.250.0.9/24 dev lan
+    wait_for "FRR's route to be taken" best_is 10.77.0.0/24 frr4
+    wait_for "it in the kernel" dut_kernel -4 "10.77.0.0/24 via 10.250.0.1 dev lan metric 20"
+    ok "when its next hop can be reached the better route is taken, chosen and installed"
+
+    # And stops being one.
+    in_ns dut ip addr del 10.250.0.9/24 dev lan
+    wait_for "BIRD's route to be chosen again" best_is 10.77.0.0/24 bird4
+    wait_for "it in the kernel" dut_kernel -4 "10.77.0.0/24 via 10.9.0.3 dev ix0 metric 20"
+    peer_is frr4 '.held_routes >= 1' >/dev/null || fail "FRR's route is not held again"
+    ok "and when it no longer can, the other route is back"
+
+    frr_configure \
+        "router bgp 64602" "address-family ipv4 unicast" \
+        "no network 10.77.0.0/24" "no neighbor 10.9.0.1 route-map TO-DUT out"
+    bird_config
+    bird_ctl configure >/dev/null
+    dut_down TERM
+    dut_config
+    dut_up
     converged
     ok "back to normal"
 }