From 1de75fc610732ab2ac045867333cb860f66a8568 Mon Sep 17 00:00:00 2001 From: Rayhaan Jaufeerally Date: Fri, 2 Oct 2026 17:13:44 +0200 Subject: [PATCH] Install a full table in the kernel in about a second A million IPv4 routes took 1.6 s to install (release build); they take 1.1 s now, and 0.5 s to remove instead of 1.15 s. Most of what is left is the kernel's own work on each route. - Nexthop objects (Linux 5.3 and later): a gateway is put in the kernel once and routes name it, so the kernel checks it once and not for every route. The daemon makes its own objects, uses nobody else's, is told when one of its own is removed and puts it back, and removes them when it stops, which takes all the routes that name them along at once. `without_nexthop_objects` turns this off; a kernel without them is noticed and served as before. - Writing to the kernel is a task of its own, fed through a short queue, so the next batch is worked out while the kernel is busy with the last. "In step" is said when the kernel has done the batch that completes the table, not when it was handed over. - What a next hop resolves to is kept, by next hop: it was looked up among the machine's addresses for every route. - A quicker hash for the plan's maps, and no lookups in the ones that are empty. Fixes a hole found on the way: when an interface went down and came back within the second it takes us to react, the kernel had dropped our routes and we did not put them back, since nothing about them had changed for us. After such a change the kernel is now asked what it actually has. scripts/in-namespace.sh runs one command in a scratch namespace; FERRIX_ROUTES sets the size of the timing test. Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 1 + bin/bgp/README.md | 24 +- crates/fib/Cargo.toml | 1 + crates/fib/src/kernel.rs | 706 ++++++++++++++++++----- crates/fib/src/lib.rs | 5 + crates/fib/src/netlink.rs | 219 ++++++- crates/fib/src/plan.rs | 18 +- crates/fib/tests/kernel.rs | 143 ++++- crates/integration-tests/tests/kernel.rs | 11 +- crates/packet/src/message.rs | 2 +- crates/server/proto/config.proto | 7 + crates/server/src/config.rs | 15 +- scripts/in-namespace.sh | 18 + scripts/kernel-tests.sh | 20 +- scripts/two-routers.sh | 2 +- 15 files changed, 1009 insertions(+), 183 deletions(-) create mode 100755 scripts/in-namespace.sh diff --git a/Cargo.lock b/Cargo.lock index a3d0b39..95b011e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -193,6 +193,7 @@ dependencies = [ "bgp-primitives", "bgp-rib", "bytes", + "foldhash", "netlink-sys", "prometheus", "proptest", diff --git a/bin/bgp/README.md b/bin/bgp/README.md index 0266d55..e2b7077 100644 --- a/bin/bgp/README.md +++ b/bin/bgp/README.md @@ -190,6 +190,15 @@ kernel { installed, and neither is whatever it replaced: looking up the way to such a next hop through other routes is not done yet. The status page counts these, by reason. +- **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 + kernel then checks the gateway once and not for every route, which makes + a full table about a third faster to install, and lets all of them be + removed at once when the daemon stops. The objects the daemon makes are + its own: it does not use anyone else's, puts its own back if one is + removed, and removes them when it stops. `without_nexthop_objects: true` + has every route carry its own gateway instead, as on older kernels. - **What is ours.** Routes are marked `proto bgp`. Every route with that mark in the configured table with the configured metric is taken to be this daemon's, and no other route is ever changed or removed. So do not @@ -220,11 +229,24 @@ To forward packets for others the machine also needs `net.ipv4.ip_forward=1` and `net.ipv6.conf.all.forwarding=1`; the daemon does not set them. +### How long it takes + +A million IPv4 routes, about the size of the Internet's table, go into the +kernel in a little over a second and come out in half a second (release +build, in a virtual machine on an Apple M-series laptop; the test +`many_routes_are_installed_and_removed` with `FERRIX_ROUTES=1000000`). +About three quarters of that is the kernel's own work on each route. +What keeps the rest small: requests go to the kernel hundreds to a write, +with one acknowledgement for each write and not for each route; a task of +its own does the writing, so the next batch is worked out while the kernel +is busy with the last; and what a next hop resolves to is worked out once +for each next hop, not once for each route. + ### Testing against a real kernel The tests that change a kernel's routes are not run by `cargo test`. `scripts/kernel-tests.sh` runs them on Linux, each in a network namespace -made for it: the route installer against `ip route`, the daemon with a peer, +made for it (`scripts/in-namespace.sh` runs any one command that way): the route installer against `ip route`, the daemon with a peer, and two daemons that route for each other with pings across. From macOS, `scripts/linux-vm.sh` runs it (or any command, such as `cargo test`) in a Lima virtual machine that it creates on first use. diff --git a/crates/fib/Cargo.toml b/crates/fib/Cargo.toml index 70dac35..8af5352 100644 --- a/crates/fib/Cargo.toml +++ b/crates/fib/Cargo.toml @@ -12,6 +12,7 @@ bgp-packet.workspace = true bgp-primitives.workspace = true bgp-rib.workspace = true bytes.workspace = true +foldhash.workspace = true prometheus.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["full"] } diff --git a/crates/fib/src/kernel.rs b/crates/fib/src/kernel.rs index 36edcf7..7bcb0a3 100644 --- a/crates/fib/src/kernel.rs +++ b/crates/fib/src/kernel.rs @@ -1,7 +1,7 @@ //! Talking to the kernel: the sockets, and the task that keeps the //! kernel's routes in step with the routing table's best routes. -use std::collections::VecDeque; +use std::collections::{HashMap, HashSet}; use std::io; use std::net::IpAddr; use std::sync::{Arc, LazyLock}; @@ -18,7 +18,8 @@ use prometheus::{ IntCounter, IntCounterVec, IntGauge, register_int_counter, register_int_counter_vec, register_int_gauge, }; -use tokio::sync::watch; +use tokio::sync::{mpsc, watch}; +use tokio::task::JoinHandle; use tokio::time::{Instant, sleep_until}; use tracing::{debug, info, warn}; @@ -26,12 +27,27 @@ use crate::interfaces::Interfaces; use crate::netlink::{ self, Change, Dump, KIND_UNICAST, Message, PROTOCOL_BGP, Route, decode, group, }; -use crate::plan::{Op, Plan, Target, resolve}; + +/// "Input/output error": for what should not happen. +const EIO: i32 = 5; +use crate::plan::{Op, Plan, Target, Unresolved, resolve}; use crate::{Fib, FibConfig, FibError, InterfaceStatus}; -/// How many best routes are taken from the routing table, and how many -/// requests are sent to the kernel, at a time. -const BATCH: usize = 256; +/// How many best routes are taken from the routing table at a time. +const BATCH: usize = 1024; + +/// How many requests are sent to the kernel in one write. +const WRITE_BATCH: usize = 512; + +/// How many batches of changes may wait for the kernel. Working out what +/// to ask for is quicker than the kernel is at doing it; this is how far +/// the one may get ahead of the other. +const QUEUE: usize = 8; + +/// How often, at most, the routes are worked out again because the kernel +/// refused one. That mends a nexthop object that went away under us; a +/// route the kernel will never take must not have it done over and over. +const MEND_INTERVAL: Duration = Duration::from_secs(30); /// The most a read from the kernel can return: it sends up to 32 KiB at a /// time in answer to a dump. @@ -118,13 +134,19 @@ impl Socket { Ok(&self.buf) } - /// Asks for all of `what` and gives each message of the answer to - /// `each`. - async fn dump(&mut self, what: Dump, mut each: impl FnMut(Message)) -> Result<(), FibError> { + /// Sends one request and gives each message of the answer to `each`, + /// up to the one that says what became of the request, whose error + /// number (zero for none) is returned. A dump ends without one, and + /// counts as done. + async fn ask( + &mut self, + request: impl FnOnce(&mut BytesMut, u32), + mut each: impl FnMut(Message), + ) -> Result { let sequence = self.next_sequence(); - let mut request = BytesMut::new(); - netlink::dump_request(&mut request, sequence, what); - self.socket.send(&request).await?; + let mut wire = BytesMut::new(); + request(&mut wire, sequence); + self.socket.send(&wire).await?; loop { let mut rest = self.read().await?; while !rest.is_empty() { @@ -134,23 +156,37 @@ impl Socket { continue; } match message { - Message::Done => return Ok(()), - Message::Outcome { errno } if errno != 0 => { - return Err(FibError::Dump { - what: match what { - Dump::Routes(_) => "its routes", - Dump::Addresses => "its addresses", - Dump::Links => "its interfaces", - }, - errno, - }); - } + Message::Done => return Ok(0), + Message::Outcome { errno } => return Ok(errno), message => each(message), } } } } + /// Asks for all of `what` and gives each message of the answer to + /// `each`. + async fn dump(&mut self, what: Dump, each: impl FnMut(Message)) -> Result<(), FibError> { + match self + .ask( + |out, sequence| netlink::dump_request(out, sequence, what), + each, + ) + .await? + { + 0 => Ok(()), + errno => Err(FibError::Dump { + what: match what { + Dump::Routes(_) => "its routes", + Dump::Addresses => "its addresses", + Dump::Links => "its interfaces", + Dump::Nexthops => "its nexthop objects", + }, + errno, + }), + } + } + /// Asks the kernel for `requests`, each a change to a route, and /// returns which of them it refused: the place in `requests` and the /// error's number. @@ -159,8 +195,8 @@ impl Socket { requests: &[(Change, Route)], ) -> Result, FibError> { let mut refused = Vec::new(); - for batch_start in (0..requests.len()).step_by(BATCH) { - let batch = &requests[batch_start..requests.len().min(batch_start + BATCH)]; + for batch_start in (0..requests.len()).step_by(WRITE_BATCH) { + let batch = &requests[batch_start..requests.len().min(batch_start + WRITE_BATCH)]; // One write for all of them. The kernel answers a request only // if it fails, except the last, which it answers either way: // that answer says the batch is done. @@ -198,22 +234,79 @@ impl Socket { /// The sockets the task uses. pub(crate) struct Sockets { - /// For asking, and the answers. - requests: Socket, - /// For what the kernel tells unasked: changes to the interfaces and - /// their addresses. + /// For asking questions, and for the few changes that are made one at + /// a time. + control: Socket, + /// For the changes to routes, which go in batches from a task of + /// their own. + routes: Socket, + /// For what the kernel tells unasked: changes to the interfaces, + /// their addresses, and nexthop objects. news: Socket, } impl Sockets { pub(crate) fn open() -> Result { + let news = Socket::open(group::LINK | group::IPV4_ADDRESS | group::IPV6_ADDRESS)?; + // A kernel without nexthop objects has no such group, and nothing + // to tell of them. + let _ = news + .socket + .socket_ref() + .add_membership(netlink::GROUP_NEXTHOP); Ok(Self { - requests: Socket::open(0)?, - news: Socket::open(group::LINK | group::IPV4_ADDRESS | group::IPV6_ADDRESS)?, + control: Socket::open(0)?, + routes: Socket::open(0)?, + news, }) } } +/// Changes to routes for the kernel, in the order they are to be made. +struct Batch { + ops: Vec, + /// What each of `ops` is to the kernel. + requests: Vec<(Change, Route)>, + /// If set, word is sent back under this number when the kernel has + /// done everything up to here. + mark: Option, +} + +/// What the task that writes to the kernel has to say. +enum Report { + /// The kernel refused `op`, with this error. + Refused { op: Op, errno: i32 }, + /// The batch with this mark, and all before it, are done. + Done(u64), +} + +/// Sends the batches to the kernel as they come, and reports what became +/// of them. A task of its own, so that the kernel can be at work on one +/// batch while the next is being worked out. Ends when there are no more +/// to come. +async fn write( + mut socket: Socket, + mut batches: mpsc::Receiver, + reports: mpsc::UnboundedSender, +) -> Result<(), FibError> { + while let Some(batch) = batches.recv().await { + for (at, errno) in socket.change(&batch.requests).await? { + // Nobody to tell only when everything is being torn down. + let _ = reports.send(Report::Refused { + op: batch.ops[at], + errno, + }); + } + if let Some(mark) = batch.mark { + let _ = reports.send(Report::Done(mark)); + } + } + Ok(()) +} + +/// What a next hop is resolved for: see [`Task::resolved`]. +type Resolving = (Option, IpVersion, Option); + /// A change to the best routes, as taken out of the routing table to be /// dealt with once the table is let go of. enum News { @@ -231,30 +324,39 @@ struct Task { rib: Arc, fib: Fib, config: FibConfig, - requests: Socket, + control: Socket, + /// Where changes to routes go to be written; `None` once they have + /// all been handed over, at the end. + queue: Option>, + writer: JoinHandle>, interfaces: Interfaces, + /// What each next hop seen so far resolves to, by the next hop, the + /// IP version of the routes, and for a link-local one the peer. + /// There are as many next hops as neighbors, and a million routes + /// share them; looking through the interfaces for each route would + /// take as long as there are addresses. Emptied when the interfaces + /// change. + resolved: HashMap, foldhash::fast::RandomState>, plan: Plan, + /// The kernel's nexthop objects that our routes name, by where they + /// lead; `None` if routes carry their own gateway instead, because + /// the kernel has no such objects or the configuration says so. + nexthops: Option>, + /// The numbers of the nexthop objects that are ours: made by this run, + /// or named by routes of ours that an earlier run left. Others of + /// BGP's are another daemon's, and are neither used nor removed: + /// routes that name an object go when the object does. + owned: HashSet, + /// The mark of the batch that ends the whole table, while the kernel + /// has not got that far. + awaiting: Option, + marks: u64, synced: bool, + /// When the routes were last worked out again for a refusal. + mended_at: Option, } impl Task { - fn route(&self, op: &Op) -> (Change, Route) { - let (change, target) = match op { - Op::Install { target, .. } => (Change::Replace, Some(target)), - Op::Remove { .. } => (Change::Remove, None), - }; - let route = Route { - prefix: *op.prefix(), - table: self.config.table, - protocol: PROTOCOL_BGP, - metric: self.config.metric, - gateway: target.map(|target| target.gateway), - interface: target.map(|target| target.interface), - kind: KIND_UNICAST, - }; - (change, route) - } - /// Whether a route the kernel has is one of ours. fn is_ours(&self, route: &Route) -> bool { route.protocol == PROTOCOL_BGP @@ -266,21 +368,71 @@ impl Task { async fn learn_interfaces(&mut self) -> Result<(), FibError> { let mut interfaces = Interfaces::default(); for what in [Dump::Links, Dump::Addresses] { - self.requests + self.control .dump(what, |message| { interfaces.apply(&message); }) .await?; } self.interfaces = interfaces; + self.resolved.clear(); Ok(()) } - /// Finds the routes of ours that the kernel has from before. - async fn adopt_routes(&mut self) -> Result<(), FibError> { + /// Asks which nexthop objects of BGP's the kernel has, and returns + /// those that lead somewhere, by number. + async fn kernel_nexthops(&mut self) -> Result, FibError> { + let mut by_id = HashMap::new(); + if self.nexthops.is_none() { + return Ok(by_id); + } + let asked = self + .control + .dump(Dump::Nexthops, |message| { + if let Message::Nexthop { + present: true, + nexthop, + } = message + && nexthop.protocol == PROTOCOL_BGP + && let (Some(gateway), Some(interface)) = (nexthop.gateway, nexthop.interface) + { + by_id.insert(nexthop.id, Target { gateway, interface }); + } + }) + .await; + match asked { + Ok(()) => {} + // A kernel from before it had them (Linux 5.3). + Err(FibError::Dump { errno, .. }) => { + info!( + errno, + "The kernel has no nexthop objects; routes will carry their own gateway" + ); + self.nexthops = None; + } + Err(e) => return Err(e), + } + Ok(by_id) + } + + /// Takes into use those of the kernel's nexthop objects, `present`, + /// that are ours. + fn use_nexthops(&mut self, present: &HashMap) { + self.owned.retain(|id| present.contains_key(id)); + if let Some(nexthops) = &mut self.nexthops { + *nexthops = present + .iter() + .filter(|(id, _)| self.owned.contains(id)) + .map(|(id, target)| (*target, *id)) + .collect(); + } + } + + /// Every route the kernel has, of either IP version. + async fn kernel_routes(&mut self) -> Result, FibError> { let mut found = Vec::new(); for version in [IpVersion::Ipv4, IpVersion::Ipv6] { - self.requests + self.control .dump(Dump::Routes(version), |message| { if let Message::Route { present: true, @@ -292,91 +444,200 @@ impl Task { }) .await?; } - for route in found { + Ok(found) + } + + /// Finds the routes of ours that the kernel has from before. + async fn adopt_routes(&mut self, nexthops: &HashMap) -> Result<(), FibError> { + for route in self.kernel_routes().await? { if !self.is_ours(&route) { continue; } - let target = match (route.gateway, route.interface) { - (Some(gateway), Some(interface)) if route.kind == KIND_UNICAST => { + let target = match (route.nexthop, route.gateway, route.interface) { + (Some(id), ..) => { + // What a route of ours names is ours. + self.owned.insert(id); + nexthops.get(&id).copied() + } + (None, Some(gateway), Some(interface)) if route.kind == KIND_UNICAST => { Some(Target { gateway, interface }) } _ => None, }; self.plan.adopt(route.prefix, target); } - if self.plan.installed() > 0 { - info!( - routes = self.plan.installed(), - "Found routes of ours in the kernel from an earlier run" - ); - } Ok(()) } - /// Asks the kernel for `ops` and deals with what it refuses. - async fn apply(&mut self, ops: Vec) -> Result<(), FibError> { - let mut pending = VecDeque::from([ops]); - while let Some(ops) = pending.pop_front() { - if ops.is_empty() { - continue; - } - let requests: Vec<(Change, Route)> = ops.iter().map(|op| self.route(op)).collect(); - for (change, _) in &requests { - let label = match change { - Change::Replace => "install", - Change::Remove => "remove", - }; - CHANGES.with_label_values(&[label]).inc(); - } - let mut instead = Vec::new(); - for (at, errno) in self.requests.change(&requests).await? { - let op = &ops[at]; - if errno == errno::EPERM { - return Err(FibError::NotPermitted); - } - // Removing what is not there is as good as done. - if matches!(op, Op::Remove { .. }) && matches!(errno, errno::ESRCH | errno::ENOENT) - { - continue; - } - let error = io::Error::from_raw_os_error(errno); - let what = match op { - Op::Install { prefix, target } => format!( - "install {prefix} via {} on {}", - target.gateway, - self.interface_name(target.interface) - ), - Op::Remove { prefix } => format!("remove {prefix}"), - }; - // Counted always, said once in a while: a table's worth of - // refusals is a table's worth of lines. - ERRORS.inc(); - let said = format!("the kernel refused to {what}: {error}"); - debug!("{said}"); - self.fib.update(|status| { - if status.errors == 0 || status.errors.is_power_of_two() { - warn!(refusals = status.errors + 1, "{said}"); + /// The number of the nexthop object that leads to `target`, made if + /// there is none yet; or the error the kernel refused to make it with. + async fn nexthop(&mut self, target: Target) -> Result, FibError> { + let known = self + .nexthops + .as_ref() + .expect("asked only where nexthop objects are in use"); + if let Some(id) = known.get(&target) { + return Ok(Ok(*id)); + } + // One at a time, and waited for: there is one for each neighbor, + // not for each route, and a route must not name one that is not + // there yet. + let mut made = None; + let errno = self + .control + .ask( + |out, sequence| { + netlink::nexthop_request(out, sequence, &target.gateway, target.interface); + }, + |message| { + if let Message::Nexthop { + present: true, + nexthop, + } = message + { + made = Some(nexthop.id); } - status.errors += 1; - status.last_error = Some(said); - }); - instead.extend(self.plan.refused_by_kernel(op)); + }, + ) + .await?; + Ok(match (errno, made) { + (0, Some(id)) => { + debug!(id, gateway = %target.gateway, "Made a nexthop object"); + self.owned.insert(id); + if let Some(known) = &mut self.nexthops { + known.insert(target, id); + } + Ok(id) } - pending.push_back(instead); + // Made, and the kernel did not say under what number. + (0, None) => Err(EIO), + (errno, _) => Err(errno), + }) + } + + /// What `op` is to the kernel; or the error it cannot be asked for + /// with. + async fn request(&mut self, op: &Op) -> Result, FibError> { + let mut route = Route { + prefix: *op.prefix(), + table: self.config.table, + protocol: PROTOCOL_BGP, + metric: self.config.metric, + gateway: None, + interface: None, + nexthop: None, + kind: KIND_UNICAST, + }; + let change = match op { + Op::Remove { .. } => Change::Remove, + Op::Install { target, .. } => { + if self.nexthops.is_some() { + match self.nexthop(*target).await? { + Ok(id) => route.nexthop = Some(id), + Err(errno) => return Ok(Err(errno)), + } + } else { + route.gateway = Some(target.gateway); + route.interface = Some(target.interface); + } + Change::Replace + } + }; + Ok(Ok((change, route))) + } + + /// Hands `ops` over to be written to the kernel, in order. With + /// `whole_table` they complete a whole table, and the kernel is in + /// step once it has done them. + async fn send(&mut self, ops: Vec, whole_table: bool) -> Result<(), FibError> { + let mut batch = Batch { + ops: Vec::with_capacity(ops.len()), + requests: Vec::with_capacity(ops.len()), + mark: None, + }; + let mut ops = std::collections::VecDeque::from(ops); + while let Some(op) = ops.pop_front() { + match self.request(&op).await? { + Ok(request) => { + CHANGES + .with_label_values(&[match request.0 { + Change::Replace => "install", + Change::Remove => "remove", + }]) + .inc(); + batch.ops.push(op); + batch.requests.push(request); + } + // What takes its place follows at once. + Err(errno) => ops.extend(self.refused(&op, errno)?), + } + } + if whole_table { + self.marks += 1; + batch.mark = Some(self.marks); + self.awaiting = batch.mark; + } + if batch.ops.is_empty() && batch.mark.is_none() { + return Ok(()); + } + let queue = self.queue.as_ref().expect("nothing is sent after the end"); + if queue.send(batch).await.is_err() { + // The writer has ended, which it does only by failing. + return Err(self.writer_failed().await); } Ok(()) } - fn interface_name(&self, index: u32) -> String { - self.interfaces - .link(index) - .map_or_else(|| format!("interface {index}"), |link| link.name.clone()) + /// Why the task that writes to the kernel ended. + async fn writer_failed(&mut self) -> FibError { + match (&mut self.writer).await { + Ok(Err(e)) => e, + Ok(Ok(())) => io::Error::other("the task that writes to the kernel ended").into(), + Err(e) => io::Error::other(e).into(), + } + } + + /// Deals with the kernel having refused `op`. Returns what to ask of + /// it instead. + fn refused(&mut self, op: &Op, errno: i32) -> Result, FibError> { + if errno == errno::EPERM { + return Err(FibError::NotPermitted); + } + // Removing what is not there is as good as done. + if matches!(op, Op::Remove { .. }) && matches!(errno, errno::ESRCH | errno::ENOENT) { + return Ok(None); + } + let error = io::Error::from_raw_os_error(errno); + let what = match op { + Op::Install { prefix, target } => format!( + "install {prefix} via {} on {}", + target.gateway, + self.interfaces.link(target.interface).map_or_else( + || format!("interface {}", target.interface), + |link| link.name.clone() + ) + ), + Op::Remove { prefix } => format!("remove {prefix}"), + }; + // Counted always, said once in a while: a table's worth of + // refusals is a table's worth of lines. + ERRORS.inc(); + let said = format!("the kernel refused to {what}: {error}"); + debug!("{said}"); + self.fib.update(|status| { + if status.errors == 0 || status.errors.is_power_of_two() { + warn!(refusals = status.errors + 1, "{said}"); + } + status.errors += 1; + status.last_error = Some(said); + }); + Ok(self.plan.refused_by_kernel(op)) } /// Takes the next changes of best route from the routing table and - /// brings the kernel in step with them. + /// hands over what brings the kernel in step with them. async fn follow(&mut self, reader: &mut Reader) -> Result<(), FibError> { - let mut news = Vec::new(); + let mut news = Vec::with_capacity(BATCH); // Only what is needed is taken while the table is held. reader.poll(BATCH, |event| { news.push(match event { @@ -397,22 +658,17 @@ impl Task { Event::Routes { .. } => return, }); }); - let mut ops = Vec::new(); + let mut ops = Vec::with_capacity(news.len()); + let mut whole_table = false; for item in news { match item { - News::Reset => self.plan.reset(), + News::Reset => { + self.plan.reset(); + self.synced = false; + } News::Synced => { - let stale = self.plan.synced(); - if !self.synced || !stale.is_empty() { - info!( - installed = self.plan.installed(), - unresolved = self.plan.unresolved(), - removed_stale = stale.len(), - "The kernel's routes are in step with the routing table" - ); - } - self.synced = true; - ops.extend(stale); + ops.extend(self.plan.synced()); + whole_table = true; } News::Best { prefix, @@ -421,20 +677,131 @@ impl Task { } => { // Whose link a link-local next hop is on is asked only // where there is one. - let peer_address = - |rib: &RibTable| -> Option { Some(rib.peer(peer)?.address) }; let needs_peer = matches!( next_hop, Some(NlriNextHop::Ipv6(_) | NlriNextHop::Ipv6WithLl { .. }) ); - let peer = needs_peer.then(|| peer_address(&self.rib)).flatten(); - let target = resolve(&self.interfaces, &prefix, next_hop.as_ref(), peer); + let peer: Option = needs_peer + .then(|| self.rib.peer(peer)) + .flatten() + .map(|peer| peer.address); + let interfaces = &self.interfaces; + let target = *self + .resolved + .entry((next_hop, prefix.address_family(), peer)) + .or_insert_with(|| resolve(interfaces, &prefix, next_hop.as_ref(), peer)); ops.extend(self.plan.best(prefix, target)); } News::Withdrawn(prefix) => ops.extend(self.plan.withdrawn(prefix)), } } - self.apply(ops).await + self.send(ops, whole_table).await + } + + /// Takes in what the writer says became of what it was given. + /// Returns whether the routes should be worked out again. + async fn hear(&mut self, report: Report) -> Result { + match report { + Report::Done(mark) => { + if self.awaiting == Some(mark) { + self.awaiting = None; + self.synced = true; + info!( + installed = self.plan.installed(), + unresolved = self.plan.unresolved(), + "The kernel's routes are in step with the routing table" + ); + } + Ok(false) + } + Report::Refused { op, errno } => { + let instead = self.refused(&op, errno)?; + self.send(instead.into_iter().collect(), false).await?; + // A refused route may have named a nexthop object that + // went away. Working the routes out again mends that. + let due = matches!(op, Op::Install { .. }) + && self.nexthops.is_some() + && self + .mended_at + .is_none_or(|at| at.elapsed() >= MEND_INTERVAL); + if due { + self.mended_at = Some(Instant::now()); + } + Ok(due) + } + } + } + + /// Finds out anew what the kernel has of ours. When interfaces or + /// nexthop objects go, the kernel removes the routes that used them + /// without a word, and what was thought to be there no longer is. + async fn relearn( + &mut self, + heard: &mut mpsc::UnboundedReceiver, + ) -> Result<(), FibError> { + // First everything that is on its way has to arrive, or the kernel + // would be asked while it is still being changed. + self.flush(heard).await?; + self.synced = false; + self.plan = Plan::default(); + let nexthops = self.kernel_nexthops().await?; + self.adopt_routes(&nexthops).await?; + self.use_nexthops(&nexthops); + Ok(()) + } + + /// Waits until the kernel has done everything handed over so far. + async fn flush(&mut self, heard: &mut mpsc::UnboundedReceiver) -> Result<(), FibError> { + self.send(vec![], true).await?; + while let Some(mark) = self.awaiting { + match heard.recv().await { + Some(Report::Done(done)) if done == mark => self.awaiting = None, + Some(report) => { + self.hear(report).await?; + } + None => return Err(self.writer_failed().await), + } + } + Ok(()) + } + + /// Takes everything of ours out of the kernel. + async fn withdraw( + &mut self, + heard: &mut mpsc::UnboundedReceiver, + ) -> Result<(), FibError> { + info!( + routes = self.plan.installed(), + "Taking our routes out of the kernel" + ); + self.synced = false; + if self.nexthops.take().is_some() { + // The quick way: when a nexthop object goes, the kernel + // removes the routes that name it, all at once. The objects + // are ours, and so is every route that names them. + self.flush(heard).await?; + for id in std::mem::take(&mut self.owned) { + let errno = self + .control + .ask( + |out, sequence| netlink::nexthop_removal(out, sequence, id), + |_| {}, + ) + .await?; + debug!(id, errno, "Removed a nexthop object"); + } + // What is left, if anything, is routes from an earlier run + // that carry their own gateway. + self.plan = Plan::default(); + self.adopt_routes(&HashMap::new()).await?; + } + let ops = self.plan.clear(); + self.send(ops, false).await?; + // No more to come: the writer ends when it has written what there + // is. + self.queue = None; + (&mut self.writer).await.map_err(io::Error::other)??; + Ok(()) } /// Says how things stand, for whoever asks. @@ -488,24 +855,48 @@ pub(crate) async fn run( sockets: Sockets, mut stopping: watch::Receiver, ) -> Result<(), FibError> { - let Sockets { requests, mut news } = sockets; + let Sockets { + control, + routes, + mut news, + } = sockets; + let (queue, batches) = mpsc::channel(QUEUE); + let (reports, mut heard) = mpsc::unbounded_channel(); + let config = fib.config(); let mut task = Task { - config: fib.config(), + config, rib, fib, - requests, + control, + queue: Some(queue), + writer: tokio::spawn(write(routes, batches, reports)), interfaces: Interfaces::default(), + resolved: HashMap::default(), plan: Plan::default(), + nexthops: config.nexthop_objects.then(HashMap::default), + owned: HashSet::new(), + awaiting: None, + marks: 0, synced: false, + mended_at: None, }; // The socket for news was bound before this: what changes while the // interfaces are being learned is told there. task.learn_interfaces().await?; - task.adopt_routes().await?; + let nexthops = task.kernel_nexthops().await?; + task.adopt_routes(&nexthops).await?; + task.use_nexthops(&nexthops); + if task.plan.installed() > 0 { + info!( + routes = task.plan.installed(), + "Found routes of ours in the kernel from an earlier run" + ); + } task.publish(true); info!( - table = task.config.table, - metric = task.config.metric, + table = config.table, + metric = config.metric, + nexthop_objects = task.nexthops.is_some(), interfaces = task.interfaces.links().count(), "Installing best routes in the kernel" ); @@ -520,6 +911,12 @@ pub(crate) async fn run( task.follow(&mut reader).await?; task.publish(false); } + Some(report) = heard.recv() => { + if task.hear(report).await? && again_at.is_none() { + again_at = Some(Instant::now() + SETTLE); + } + task.publish(false); + } read = news.read() => { let changed = match read { Ok(mut rest) => { @@ -527,7 +924,19 @@ pub(crate) async fn run( while !rest.is_empty() { let (_, message, used) = decode(rest)?; rest = &rest[used..]; - changed |= task.interfaces.apply(&message); + if task.interfaces.apply(&message) { + task.resolved.clear(); + changed = true; + } + // A nexthop object of ours has been removed + // by someone, and the routes that named it + // with it. + if let Message::Nexthop { present: false, nexthop } = &message + && task.owned.contains(&nexthop.id) + { + warn!(id = nexthop.id, "A nexthop object of ours was removed"); + changed = true; + } } changed } @@ -550,14 +959,19 @@ pub(crate) async fn run( if again_at.is_some() => { again_at = None; - info!("The machine's interfaces changed; working the kernel's routes out again"); + info!("Working the kernel's routes out again"); + task.relearn(&mut heard).await?; reader = task.rib.reader(); } + ended = &mut task.writer => { + return Err(match ended { + Ok(Err(e)) => e, + Ok(Ok(())) => io::Error::other("the task that writes to the kernel ended").into(), + Err(e) => io::Error::other(e).into(), + }); + } _ = stopping.changed() => { - let ops = task.plan.clear(); - info!(routes = ops.len(), "Taking our routes out of the kernel"); - task.apply(ops).await?; - task.synced = false; + task.withdraw(&mut heard).await?; task.publish(false); return Ok(()); } diff --git a/crates/fib/src/lib.rs b/crates/fib/src/lib.rs index 5155d4d..b41c51d 100644 --- a/crates/fib/src/lib.rs +++ b/crates/fib/src/lib.rs @@ -56,6 +56,10 @@ pub struct FibConfig { pub table: u32, /// The metric, which with the table says which routes are ours. pub metric: u32, + /// Have routes name nexthop objects where the kernel has them (Linux + /// 5.3 and later), which makes installing many routes a good deal + /// faster. Without, every route carries its own gateway. + pub nexthop_objects: bool, } impl Default for FibConfig { @@ -63,6 +67,7 @@ impl Default for FibConfig { Self { table: netlink::TABLE_MAIN, metric: DEFAULT_METRIC, + nexthop_objects: true, } } } diff --git a/crates/fib/src/netlink.rs b/crates/fib/src/netlink.rs index 2cea1da..e35497e 100644 --- a/crates/fib/src/netlink.rs +++ b/crates/fib/src/netlink.rs @@ -39,11 +39,16 @@ mod message_type { pub const NEW_ROUTE: u16 = 24; pub const DEL_ROUTE: u16 = 25; pub const GET_ROUTE: u16 = 26; + pub const NEW_NEXTHOP: u16 = 104; + pub const DEL_NEXTHOP: u16 = 105; + pub const GET_NEXTHOP: u16 = 106; } mod flag { pub const REQUEST: u16 = 0x1; pub const ACK: u16 = 0x4; + /// Send the sender what everyone else is told of the change. + pub const ECHO: u16 = 0x8; /// For a request to read: all of them (`NLM_F_ROOT | NLM_F_MATCH`). pub const DUMP: u16 = 0x300; /// For a request to add: take the place of the one there is. @@ -60,6 +65,11 @@ pub mod group { pub const IPV6_ADDRESS: u32 = 0x100; } +/// The number of the group that is told of changes to nexthop objects +/// (`RTNLGRP_NEXTHOP`). Too high to be asked for like those above; it is +/// joined once the socket is bound. +pub const GROUP_NEXTHOP: u32 = 32; + const AF_INET: u8 = 2; const AF_INET6: u8 = 10; @@ -74,6 +84,13 @@ mod route_attribute { pub const GATEWAY: u16 = 5; pub const PRIORITY: u16 = 6; pub const TABLE: u16 = 15; + pub const NEXTHOP_ID: u16 = 30; +} + +mod nexthop_attribute { + pub const ID: u16 = 1; + pub const OUT_INTERFACE: u16 = 5; + pub const GATEWAY: u16 = 6; } mod address_attribute { @@ -101,10 +118,27 @@ pub struct Route { pub gateway: Option, /// The index of the interface packets leave by. pub interface: Option, + /// The [`Nexthop`] that says where packets go, in place of a gateway + /// and an interface of the route's own. + pub nexthop: Option, /// See [`KIND_UNICAST`]. pub kind: u8, } +/// A nexthop object: a gateway and an interface under a number, which +/// any number of routes can name instead of each carrying its own. The +/// kernel then checks that the gateway can be reached once, not once for +/// every route, which makes putting many routes there a good deal faster. +/// Linux 5.3 and later. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct Nexthop { + pub id: u32, + /// Who made it; see [`PROTOCOL_BGP`]. + pub protocol: u8, + pub gateway: Option, + pub interface: Option, +} + /// An address of one of the machine's interfaces. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Address { @@ -150,6 +184,10 @@ pub enum Message { present: bool, link: Link, }, + Nexthop { + present: bool, + nexthop: Nexthop, + }, /// What became of a request: `errno` is zero if it was done, and /// otherwise the error's number. Outcome { @@ -299,6 +337,18 @@ pub fn decode(buf: &[u8]) -> Result<(Header, Message, usize), NetlinkError> { }, } } + message_type::NEW_NEXTHOP | message_type::DEL_NEXTHOP => { + // nhmsg: family, scope, protocol, a reserved octet, and four + // octets of flags. + let (fixed, rest) = body.split_at_checked(8).ok_or(truncated)?; + match nexthop(fixed, rest) { + Some(nexthop) => Message::Nexthop { + present: message_type == message_type::NEW_NEXTHOP, + nexthop, + }, + None => other, + } + } _ => other, }; // The next message begins where this one ends, rounded up; the last @@ -319,6 +369,7 @@ fn route(fixed: &[u8], attributes_of_route: &[u8]) -> Option { metric: 0, gateway: None, interface: None, + nexthop: None, kind: fixed[7], }; for (kind, value) in attributes(attributes_of_route) { @@ -332,12 +383,42 @@ fn route(fixed: &[u8], attributes_of_route: &[u8]) -> Option { route_attribute::OUT_INTERFACE => route.interface = Some(ne_u32(value.get(..4)?)), route_attribute::PRIORITY => route.metric = ne_u32(value.get(..4)?), route_attribute::TABLE => route.table = ne_u32(value.get(..4)?), + route_attribute::NEXTHOP_ID => route.nexthop = Some(ne_u32(value.get(..4)?)), _ => {} } } Some(route) } +/// A nexthop object from the fixed part of its message and its +/// attributes. `None` if it has no number. +fn nexthop(fixed: &[u8], attributes_of_nexthop: &[u8]) -> Option { + // A group of nexthops, or one that drops, has no family; its gateway, + // if it had one, could not be read. + let version = version_of(fixed[0]); + let mut nexthop = Nexthop { + id: 0, + protocol: fixed[2], + gateway: None, + interface: None, + }; + let mut has_id = false; + for (kind, value) in attributes(attributes_of_nexthop) { + match kind { + nexthop_attribute::ID => { + nexthop.id = ne_u32(value.get(..4)?); + has_id = true; + } + nexthop_attribute::GATEWAY => nexthop.gateway = Some(address(version?, value)?), + nexthop_attribute::OUT_INTERFACE => { + nexthop.interface = Some(ne_u32(value.get(..4)?)); + } + _ => {} + } + } + has_id.then_some(nexthop) +} + fn interface_address(fixed: &[u8], attributes_of_address: &[u8]) -> Option
{ let version = version_of(fixed[0])?; let prefix_length = fixed[1]; @@ -381,6 +462,7 @@ pub enum Dump { Routes(IpVersion), Addresses, Links, + Nexthops, } /// Writes a header and returns where the message began, for @@ -459,6 +541,9 @@ pub fn route_request( ); put_attribute(out, route_attribute::TABLE, &route.table.to_ne_bytes()); put_attribute(out, route_attribute::PRIORITY, &route.metric.to_ne_bytes()); + if let Some(nexthop) = route.nexthop { + put_attribute(out, route_attribute::NEXTHOP_ID, &nexthop.to_ne_bytes()); + } if let Some(gateway) = &route.gateway { let (bytes, length) = octets(gateway); put_attribute(out, route_attribute::GATEWAY, &bytes[..length]); @@ -473,6 +558,43 @@ pub fn route_request( finish(out, start); } +/// Writes the request to make a nexthop object for `gateway` on +/// `interface` to the end of `out`. The kernel gives it a number, and says +/// which by sending back the nexthop as made, before it acknowledges. +pub fn nexthop_request(out: &mut BytesMut, sequence: u32, gateway: &IpAddr, interface: u32) { + let start = begin( + out, + message_type::NEW_NEXTHOP, + flag::CREATE | flag::ACK | flag::ECHO, + sequence, + ); + out.put_u8(match gateway { + IpAddr::V4(_) => AF_INET, + IpAddr::V6(_) => AF_INET6, + }); + out.put_u8(SCOPE_UNIVERSE); + out.put_u8(PROTOCOL_BGP); + out.put_u8(0); + out.put_u32_ne(0); + let (bytes, length) = octets(gateway); + put_attribute(out, nexthop_attribute::GATEWAY, &bytes[..length]); + put_attribute( + out, + nexthop_attribute::OUT_INTERFACE, + &interface.to_ne_bytes(), + ); + finish(out, start); +} + +/// Writes the request to remove the nexthop object with the number `id` +/// to the end of `out`. The kernel removes the routes that name it too. +pub fn nexthop_removal(out: &mut BytesMut, sequence: u32, id: u32) { + let start = begin(out, message_type::DEL_NEXTHOP, flag::ACK, sequence); + out.put_bytes(0, 8); + put_attribute(out, nexthop_attribute::ID, &id.to_ne_bytes()); + finish(out, start); +} + /// Writes the request for all of `what` to the end of `out`. The answer is /// a message for each, and then [`Message::Done`]. pub fn dump_request(out: &mut BytesMut, sequence: u32, what: Dump) { @@ -480,6 +602,7 @@ pub fn dump_request(out: &mut BytesMut, sequence: u32, what: Dump) { Dump::Routes(version) => (message_type::GET_ROUTE, family_of(version), 12), Dump::Addresses => (message_type::GET_ADDRESS, 0, 8), Dump::Links => (message_type::GET_LINK, 0, 16), + Dump::Nexthops => (message_type::GET_NEXTHOP, 0, 8), }; let start = begin(out, message_type, flag::DUMP, sequence); // The fixed part of the kind asked for, empty but for the family. @@ -518,6 +641,7 @@ mod tests { metric: 20, gateway: Some("192.0.2.1".parse().unwrap()), interface: Some(3), + nexthop: None, kind: KIND_UNICAST, }; let mut wire = BytesMut::new(); @@ -583,6 +707,7 @@ mod tests { metric: 20, gateway: Some("fe80::1".parse().unwrap()), interface: Some(9), + nexthop: None, kind: KIND_UNICAST, }; let mut wire = BytesMut::new(); @@ -598,6 +723,95 @@ mod tests { ); } + #[test] + fn nexthop_objects_are_asked_for_named_and_read() { + // The request: RTM_NEWNEXTHOP, REQUEST|ACK|ECHO|CREATE; nhmsg with + // AF_INET, scope universe, proto BGP; NHA_GATEWAY and NHA_OIF. + let mut wire = BytesMut::new(); + nexthop_request(&mut wire, 9, &"192.0.2.1".parse().unwrap(), 3); + let mut expected = Vec::new(); + expected.extend(40u32.to_ne_bytes()); + expected.extend(104u16.to_ne_bytes()); + expected.extend((0x1u16 | 0x4 | 0x8 | 0x400).to_ne_bytes()); + expected.extend(9u32.to_ne_bytes()); + expected.extend(0u32.to_ne_bytes()); + expected.extend([2, 0, 186, 0, 0, 0, 0, 0]); + expected.extend(8u16.to_ne_bytes()); + expected.extend(6u16.to_ne_bytes()); + expected.extend([192, 0, 2, 1]); + expected.extend(8u16.to_ne_bytes()); + expected.extend(5u16.to_ne_bytes()); + expected.extend(3u32.to_ne_bytes()); + assert_eq!(wire[..], expected[..]); + + // What the kernel sends back is the same with the number it gave. + let reply = message( + 104, + 9, + &[2, 0, 186, 0, 0, 0, 0, 0], + &[ + (1, &77u32.to_ne_bytes()), + (6, &[192, 0, 2, 1]), + (5, &3u32.to_ne_bytes()), + ], + ); + assert_eq!( + decode(&reply).unwrap().1, + Message::Nexthop { + present: true, + nexthop: Nexthop { + id: 77, + protocol: PROTOCOL_BGP, + gateway: Some("192.0.2.1".parse().unwrap()), + interface: Some(3), + } + } + ); + // A group has no family and no gateway; it is still a nexthop. + let group = message( + 104, + 0, + &[0, 0, 3, 0, 0, 0, 0, 0], + &[(1, &5u32.to_ne_bytes())], + ); + assert!(matches!( + decode(&group).unwrap().1, + Message::Nexthop { + nexthop: Nexthop { + id: 5, + gateway: None, + .. + }, + .. + } + )); + + let mut wire = BytesMut::new(); + nexthop_removal(&mut wire, 10, 77); + assert_eq!(wire.len(), 32); + assert_eq!(wire[4..6], 105u16.to_ne_bytes()); + assert_eq!(wire[28..32], 77u32.to_ne_bytes()); + + // A route that names a nexthop carries no gateway of its own. + let route = Route { + prefix: prefix("203.0.113.0/24"), + table: TABLE_MAIN, + protocol: PROTOCOL_BGP, + metric: 20, + gateway: None, + interface: None, + nexthop: Some(77), + kind: KIND_UNICAST, + }; + assert_eq!( + read_back(&route, Change::Replace).1, + Message::Route { + present: true, + route + } + ); + } + #[test] fn dump_requests_carry_the_fixed_part_of_what_they_ask_for() { for (what, message_type, length, family) in [ @@ -605,6 +819,7 @@ mod tests { (Dump::Routes(IpVersion::Ipv6), 26, 28, 10), (Dump::Addresses, 22, 24, 0), (Dump::Links, 18, 32, 0), + (Dump::Nexthops, 106, 24, 0), ] { let mut wire = BytesMut::new(); dump_request(&mut wire, 3, what); @@ -783,9 +998,10 @@ mod tests { any::(), proptest::option::of(any::<[u8; 16]>()), proptest::option::of(any::()), + proptest::option::of(any::()), ) .prop_map( - |(v6, bytes, length, table, protocol, metric, gateway, interface)| { + |(v6, bytes, length, table, protocol, metric, gateway, interface, nexthop)| { let version = if v6 { IpVersion::Ipv6 } else { IpVersion::Ipv4 }; let gateway = gateway.map(|bytes| match version { IpVersion::Ipv4 => IpAddr::from(<[u8; 4]>::try_from(&bytes[..4]).unwrap()), @@ -798,6 +1014,7 @@ mod tests { metric, gateway, interface, + nexthop, kind: KIND_UNICAST, } }, diff --git a/crates/fib/src/plan.rs b/crates/fib/src/plan.rs index 51d5c80..b713568 100644 --- a/crates/fib/src/plan.rs +++ b/crates/fib/src/plan.rs @@ -3,7 +3,6 @@ //! routes are and what the kernel said, and answers with the changes to //! ask the kernel for. -use std::collections::{HashMap, HashSet}; use std::net::{IpAddr, Ipv6Addr}; use bgp_packet::message::NlriNextHop; @@ -12,6 +11,10 @@ use bgp_primitives::ip_prefix::IpPrefix; use crate::interfaces::Interfaces; +// Asked for every route: a quick hash matters here. +type HashMap = std::collections::HashMap; +type HashSet = std::collections::HashSet; + /// Where the kernel is to send packets for a prefix. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Target { @@ -231,10 +234,15 @@ impl Plan { /// The best route for `prefix` is now one that resolves as `target`. pub fn best(&mut self, prefix: IpPrefix, target: Result) -> Option { - self.refused.remove(&prefix); + // Nearly always there is nothing to forget. + if !self.refused.is_empty() { + self.refused.remove(&prefix); + } match target { Ok(target) => { - self.set_unresolved(prefix, None); + if !self.unresolved.is_empty() { + self.set_unresolved(prefix, None); + } let entry = self.installed.entry(prefix).or_insert(Entry { target: None, confirmed: true, @@ -566,14 +574,14 @@ mod tests { ) { let p = |n: u8| IpPrefix::new(IpVersion::Ipv4, [10, n, 0, 0], 16).unwrap(); let t = |n: u8| target(&format!("192.0.2.{}", n + 2), 2); - let mut kernel: HashMap = HashMap::new(); + let mut kernel: HashMap = HashMap::default(); let mut plan = Plan::default(); for (prefix, gateway) in left_behind { kernel.insert(p(prefix), t(gateway)); plan.adopt(p(prefix), Some(t(gateway))); } // The best routes: the prefix's next hop, if it resolves. - let mut table: HashMap> = HashMap::new(); + let mut table: HashMap> = HashMap::default(); let apply = |kernel: &mut HashMap, op: Option| match op { Some(Op::Install { prefix, target }) => { kernel.insert(prefix, target); } Some(Op::Remove { prefix }) => { kernel.remove(&prefix); } diff --git a/crates/fib/tests/kernel.rs b/crates/fib/tests/kernel.rs index 3475a6f..5ff7609 100644 --- a/crates/fib/tests/kernel.rs +++ b/crates/fib/tests/kernel.rs @@ -50,6 +50,9 @@ fn ip(arguments: &str) -> String { struct Machine { n: u8, rib: Arc, + /// Whether routes name nexthop objects, which is how it is unless a + /// test says otherwise. + nexthop_objects: bool, } impl Machine { @@ -57,6 +60,7 @@ impl Machine { let machine = Self { n, rib: Arc::new(RibTable::new(4)), + nexthop_objects: true, }; ip(&format!("link add {} type dummy", machine.interface())); machine.add_addresses(); @@ -76,6 +80,14 @@ impl Machine { )); } + fn add_addresses_v4(&self) { + ip(&format!( + "addr add 10.{}.0.1/24 dev {}", + self.n, + self.interface() + )); + } + fn table(&self) -> u32 { 1000 + u32::from(self.n) } @@ -91,6 +103,7 @@ impl Machine { FibConfig { table: self.table(), metric: 20, + nexthop_objects: self.nexthop_objects, }, ) .expect("the kernel can be talked to") @@ -144,6 +157,15 @@ impl Machine { .map(|line| line.split_whitespace().collect::>().join(" ")) // IPv6 routes are shown with "pref medium". .map(|line| line.replace(" pref medium", "")) + // A route that names a nexthop object is shown with + // its number, which is the kernel's to choose. + .map(|line| match line.split_once(" nhid ") { + Some((prefix, rest)) => { + let rest = rest.split_once(' ').map_or("", |(_, rest)| rest); + format!("{prefix} {rest}") + } + None => line, + }) .filter(|line| !line.is_empty()), ); } @@ -364,6 +386,18 @@ async fn routes_follow_the_machines_interfaces() { status.installed == 0 && status.unresolved.len() == 1 }); + // The interface goes down and comes straight back, faster than we + // work the routes out again. The kernel dropped the route when it went + // down, though nothing about the route has changed for us. + machine.add_addresses_v4(); + machine.kernel_becomes(&[machine.shown("203.0.113.0/24", machine.neighbor(2))]); + ip(&format!("link set {interface} down")); + ip(&format!("link set {interface} up")); + assert!(machine.kernel_routes().is_empty(), "the kernel dropped it"); + machine.kernel_becomes(&[machine.shown("203.0.113.0/24", machine.neighbor(2))]); + ip(&format!("addr del 10.3.0.1/24 dev {interface}")); + machine.kernel_becomes(&[]); + // And gets it back, with the interface having been down meanwhile. ip(&format!("link set {interface} down")); ip(&format!("addr add 10.3.0.1/24 dev {interface}")); @@ -379,11 +413,15 @@ async fn routes_follow_the_machines_interfaces() { #[tokio::test(flavor = "multi_thread")] #[ignore = "changes the machine's routes; run scripts/kernel-tests.sh"] async fn many_routes_are_installed_and_removed() { - const ROUTES: usize = 200_000; + // FERRIX_ROUTES=1000000 for the size of the Internet's IPv4 table. + let routes: usize = std::env::var("FERRIX_ROUTES") + .ok() + .and_then(|routes| routes.parse().ok()) + .unwrap_or(200_000); let machine = Machine::new(4); let peer = machine.peer(2); let via = machine.via(machine.neighbor(2).into(), 0); - let prefixes: Vec = (0..ROUTES as u32) + let prefixes: Vec = (0..routes as u32) .map(|n| { let [_, a, b, c] = n.to_be_bytes(); IpPrefix::new( @@ -399,10 +437,10 @@ async fn many_routes_are_installed_and_removed() { let started = Instant::now(); let (fib, task) = machine.start(); status_becomes(&fib, "all installed", |status| { - status.synced && status.installed == ROUTES + status.synced && status.installed == routes }); let installed_in = started.elapsed(); - assert_eq!(machine.kernel_routes().len(), ROUTES); + assert_eq!(machine.kernel_routes().len(), routes); assert_eq!(fib.status().errors, 0, "{:?}", fib.status().last_error); let started = Instant::now(); @@ -411,7 +449,100 @@ async fn many_routes_are_installed_and_removed() { let removed_in = started.elapsed(); assert!(machine.kernel_routes().is_empty()); println!( - "{ROUTES} routes: installed in {installed_in:?} ({:.0}/s), removed in {removed_in:?}", - ROUTES as f64 / installed_in.as_secs_f64() + "{routes} routes: installed in {installed_in:?} ({:.0}/s), removed in {removed_in:?}", + routes as f64 / installed_in.as_secs_f64() ); } + +/// Routes name a nexthop object for their gateway where the kernel has +/// such objects. The object is ours: made when first needed, put back if +/// someone removes it, and removed when we stop. Other objects are not +/// touched. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "changes the machine's routes; run scripts/kernel-tests.sh"] +async fn routes_name_a_nexthop_object_that_is_ours_to_keep() { + let machine = Machine::new(5); + let interface = machine.interface(); + // Someone else's object, to the same gateway even. + ip(&format!( + "nexthop add id 50005 via 10.5.0.2 dev {interface} protocol bgp" + )); + let nexthops = || -> Vec { + ip("nexthop show") + .lines() + .filter(|line| line.contains(&format!("dev {interface} "))) + .map(|line| line.trim().to_string()) + .collect() + }; + let peer = machine.peer(2); + let via = machine.via(machine.neighbor(2).into(), 0); + machine.rib.announce( + peer, + &via, + &[prefix("203.0.113.0/24"), prefix("198.18.0.0/15")], + ); + let (fib, task) = machine.start(); + let both = [ + machine.shown("203.0.113.0/24", machine.neighbor(2)), + machine.shown("198.18.0.0/15", machine.neighbor(2)), + ]; + machine.kernel_becomes(&both); + + // One object of ours beside the other, and both routes name it. + let ours: Vec = nexthops() + .into_iter() + .filter(|line| !line.starts_with("id 50005 ")) + .collect(); + assert_eq!(ours.len(), 1, "{:?}", nexthops()); + assert!(ours[0].contains("via 10.5.0.2") && ours[0].contains("proto bgp")); + let id = ours[0].split_whitespace().nth(1).unwrap().to_string(); + let raw = ip(&format!("route show table {}", machine.table())); + assert_eq!(raw.matches(&format!("nhid {id} ")).count(), 2, "{raw}"); + + // Someone removes our object, and the kernel our routes with it. We + // are told, and put both back. + ip(&format!("nexthop del id {id}")); + machine.kernel_becomes(&both); + + // When we stop, our object goes with our routes; the other stays. + fib.stop().await; + task.await.unwrap().unwrap(); + machine.kernel_becomes(&[]); + assert_eq!(nexthops().len(), 1, "{:?}", nexthops()); + assert!(nexthops()[0].starts_with("id 50005 ")); + ip("nexthop del id 50005"); +} + +/// On a kernel without nexthop objects, or where the configuration says +/// so, every route carries its own gateway. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "changes the machine's routes; run scripts/kernel-tests.sh"] +async fn routes_can_carry_their_own_gateway() { + let mut machine = Machine::new(6); + machine.nexthop_objects = false; + let peer = machine.peer(2); + let via = machine.via(machine.neighbor(2).into(), 0); + machine + .rib + .announce(peer, &via, &[prefix("203.0.113.0/24")]); + let (fib, task) = machine.start(); + machine.kernel_becomes(&[machine.shown("203.0.113.0/24", machine.neighbor(2))]); + let raw = ip(&format!("route show table {}", machine.table())); + assert!(!raw.contains("nhid"), "{raw}"); + assert!( + !ip("nexthop show").contains(&format!("dev {} ", machine.interface())), + "no nexthop object was made" + ); + + // The next hop changes, and the route goes. + let other = machine.via(machine.neighbor(3).into(), 0); + machine + .rib + .announce(peer, &other, &[prefix("203.0.113.0/24")]); + machine.kernel_becomes(&[machine.shown("203.0.113.0/24", machine.neighbor(3))]); + machine.rib.withdraw(peer, &[prefix("203.0.113.0/24")]); + machine.kernel_becomes(&[]); + + fib.stop().await; + task.await.unwrap().unwrap(); +} diff --git a/crates/integration-tests/tests/kernel.rs b/crates/integration-tests/tests/kernel.rs index bd9fe19..933b54c 100644 --- a/crates/integration-tests/tests/kernel.rs +++ b/crates/integration-tests/tests/kernel.rs @@ -47,6 +47,15 @@ fn kernel_routes() -> BTreeSet { String::from_utf8_lossy(&output.stdout) .lines() .map(|line| line.split_whitespace().collect::>().join(" ")) + // The number of the nexthop object a route names is the kernel's + // to choose. + .map(|line| match line.split_once(" nhid ") { + Some((prefix, rest)) => { + let rest = rest.split_once(' ').map_or("", |(_, rest)| rest); + format!("{prefix} {rest}") + } + None => line, + }) .filter(|line| !line.is_empty()) .collect() } @@ -106,7 +115,7 @@ async fn test_routes_from_a_session_reach_the_kernel() -> Result<()> { let server = TestServer::start_with(65000, |config| { config.kernel = Some(FibConfig { table: TABLE, - metric: 20, + ..FibConfig::default() }); }) .await?; diff --git a/crates/packet/src/message.rs b/crates/packet/src/message.rs index 937593c..d64f1c9 100644 --- a/crates/packet/src/message.rs +++ b/crates/packet/src/message.rs @@ -1053,7 +1053,7 @@ impl LargeCommunity { } /// The next hop of routes in MP_REACH_NLRI. -#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq, Hash)] pub enum NlriNextHop { Ipv4(Ipv4Addr), /// A global IPv6 address. diff --git a/crates/server/proto/config.proto b/crates/server/proto/config.proto index 98ef70a..791f50e 100644 --- a/crates/server/proto/config.proto +++ b/crates/server/proto/config.proto @@ -72,6 +72,13 @@ message Kernel { // The metric of the routes. Defaults to 20. Of several routes for a // prefix the kernel uses the one with the lowest metric. uint32 metric = 2; + + // By default, where the kernel has nexthop objects (Linux 5.3 and + // later), a gateway is put there once as such an object and routes name + // it, which makes installing a full table about a third faster. `ip + // route` then shows routes with "nhid". True has every route carry its + // own gateway, as on older kernels. + bool without_nexthop_objects = 3; } // Where to get the validated ROA payloads that say which AS may originate diff --git a/crates/server/src/config.rs b/crates/server/src/config.rs index 71534dc..24070fc 100644 --- a/crates/server/src/config.rs +++ b/crates/server/src/config.rs @@ -183,6 +183,7 @@ impl Config { FibConfig { table: or(kernel.table, defaults.table), metric: or(kernel.metric, defaults.metric), + nexthop_objects: !kernel.without_nexthop_objects, } }), rpki, @@ -743,16 +744,20 @@ mod tests { parse(&format!("{BASE}kernel {{}}")).unwrap().kernel, Some(FibConfig { table: 254, - metric: 20 + metric: 20, + nexthop_objects: true, }) ); assert_eq!( - parse(&format!("{BASE}kernel {{ table: 1000 metric: 150 }}")) - .unwrap() - .kernel, + parse(&format!( + "{BASE}kernel {{ table: 1000 metric: 150 without_nexthop_objects: true }}" + )) + .unwrap() + .kernel, Some(FibConfig { table: 1000, - metric: 150 + metric: 150, + nexthop_objects: false, }) ); } diff --git a/scripts/in-namespace.sh b/scripts/in-namespace.sh new file mode 100755 index 0000000..fb01557 --- /dev/null +++ b/scripts/in-namespace.sh @@ -0,0 +1,18 @@ +#!/bin/sh +# Runs a command on Linux in a network namespace made for it, as the +# calling user with CAP_NET_ADMIN and nothing else: what the tests that +# change a kernel's routes need. For running one of them by hand, e.g. +# +# FERRIX_ROUTES=1000000 scripts/in-namespace.sh cargo test --release \ +# -p bgp-fib --test kernel many -- --include-ignored --nocapture +set -eu +namespace="ferrix-ns-$$" +sudo modprobe dummy 2>/dev/null || true +sudo ip netns add "$namespace" +trap 'sudo ip netns del "$namespace"' EXIT +sudo ip netns exec "$namespace" ip link set lo up +sudo ip netns exec "$namespace" \ + setpriv --reuid "$(id -u)" --regid "$(id -g)" --init-groups \ + --inh-caps +net_admin --ambient-caps +net_admin \ + env PATH="$PATH" HOME="$HOME" CARGO_TARGET_DIR="${CARGO_TARGET_DIR:-}" \ + FERRIX_ROUTES="${FERRIX_ROUTES:-}" "$@" diff --git a/scripts/kernel-tests.sh b/scripts/kernel-tests.sh index 6aa170f..db93048 100755 --- a/scripts/kernel-tests.sh +++ b/scripts/kernel-tests.sh @@ -19,20 +19,8 @@ cargo build --locked -p bgp cargo test --locked -p bgp-fib --test kernel --no-run cargo test --locked -p bgp-integration-tests --test kernel --no-run -namespace="ferrix-test-$$" -sudo modprobe dummy 2>/dev/null || true -sudo ip netns add "$namespace" -trap 'sudo ip netns del "$namespace"' EXIT -sudo ip netns exec "$namespace" ip link set lo up - -# In the namespace, as the user, with CAP_NET_ADMIN and nothing else. -in_namespace() { - sudo ip netns exec "$namespace" \ - setpriv --reuid "$(id -u)" --regid "$(id -g)" --init-groups \ - --inh-caps +net_admin --ambient-caps +net_admin \ - env PATH="$PATH" HOME="$HOME" CARGO_TARGET_DIR="${CARGO_TARGET_DIR:-}" "$@" -} - -in_namespace cargo test --locked -p bgp-fib --test kernel -- --include-ignored --nocapture "$@" -in_namespace cargo test --locked -p bgp-integration-tests --test kernel -- --include-ignored --nocapture "$@" +# Each in a namespace of its own, as the user, with CAP_NET_ADMIN and +# nothing else. +scripts/in-namespace.sh cargo test --locked -p bgp-fib --test kernel -- --include-ignored --nocapture "$@" +scripts/in-namespace.sh cargo test --locked -p bgp-integration-tests --test kernel -- --include-ignored --nocapture "$@" scripts/two-routers.sh diff --git a/scripts/two-routers.sh b/scripts/two-routers.sh index 8a4bbfd..078e818 100755 --- a/scripts/two-routers.sh +++ b/scripts/two-routers.sh @@ -104,7 +104,7 @@ start "$b" b expect() { run=$1; family=$2; want=$3; route=$4 for _ in $(seq 100); do - if $run ip "$family" route show proto bgp | sed 's/ */ /g; s/ pref medium//; s/ *$//' | grep -qxF "$route"; then + if $run ip "$family" route show proto bgp | sed 's/ */ /g; s/ nhid [0-9]*//; s/ pref medium//; s/ *$//' | grep -qxF "$route"; then [ "$want" = present ] && return 0 else [ "$want" = absent ] && return 0