Assorted cleanups.

Cargo.toml

@@ -39,7 +39,7 @@ anyhow                           = "1.0.71"
 async-trait                      = "0.1.80"
 byteorder                        = "1.4.3"
 bytes                            = "1.*"
-clap                             = { version = "3.2.8", features = ["cargo", "derive"] }
+clap                             = { version = "4.5.11", features = ["cargo", "derive"] }
 eyre                             = "0.6.12"
 futures                          = "0.3"
 ip_network_table-deps-treebitmap = "0.5.0"

bin/src/bgp_server/main.rs

@@ -14,7 +14,7 @@
 
 use bgp_server::bgp_server::Server;
 use bgp_server::config::ServerConfig;
-use clap::{App, Arg};
+use clap::Parser;
 use core::sync::atomic::AtomicBool;
 use libc::SIGUSR1;
 use signal_hook::consts::signal::*;
@@ -28,6 +28,13 @@ use std::process::exit;
 use std::sync::Arc;
 use tracing::info;
 
+#[derive(Parser)]
+#[command(author = "Rayhaan Jaufeerally <rayhaan@rayhaan.ch>", version = "0.1")]
+struct Cli {
+    #[arg(short = 'c', long = "config")]
+    config_path: String,
+}
+
 #[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
     let subscriber = tracing_subscriber::fmt();
@@ -40,16 +47,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         }
     }
 
-    let argv_matches = App::new("bgpd")
+    let args = Cli::parse();
-        .author("Rayhaan Jaufeerally <rayhaan@rayhaan.ch>")
-        .version("0.1")
-        .about("net-control-plane BGP daemon")
-        .arg(Arg::with_name("config").takes_value(true))
-        .get_matches();
 
     info!("Starting BGP Daemon!");
 
-    let config_file = File::open(argv_matches.value_of("config").unwrap_or("config.json")).unwrap();
+    let config_file = File::open(args.config_path).unwrap();
     let reader = BufReader::new(config_file);
     let server_config: ServerConfig = serde_json::from_reader(reader).unwrap();
 

bin/src/client/main.rs

@@ -1,5 +1,6 @@
-use clap::Parser;
+use clap::{Parser, Subcommand};
-use eyre::Result;
+use eyre::{bail, Result};
+use route_client::southbound_interface::{DummyVerifier, SouthboundInterface};
 use tracing::{info, warn};
 
 use route_client::netlink::NetlinkConnector;
@@ -12,11 +13,25 @@ use route_client::{run_connector_v4, run_connector_v6};
     about = "Installs routes from a BGP speaker via streaming RPC to the forwarding plane"
 )]
 struct Cli {
+    /// route_server is the gRPC endpoint to connect to for streaming routes from.
     #[clap(long = "route_server")]
     route_server: String,
-    #[clap(long = "rt_table")]
+    #[clap(subcommand)]
-    rt_table: Option<u32>,
+    command: Option<Commands>,
-    dry_run: bool,
+}
+
+#[derive(Subcommand)]
+enum Commands {
+    /// InstallKernel installs the routes received into the kernel routing table.
+    InstallKernel {
+        #[arg(default_value_t = 201)]
+        rt_table: u32,
+        #[arg(default_value_t = false)]
+        dry_run: bool,
+    },
+    /// Verify performs consistency checks on the inbound stream of routes to ensure
+    /// that there are no spurious removals or duplicate entries.
+    Verify,
 }
 
 #[tokio::main]
@@ -27,36 +42,42 @@ async fn main() -> Result<()> {
 
     info!("Starting route client");
 
-    let rt_table = match args.rt_table {
+    match args.command {
-        Some(table) => table,
+        Some(Commands::InstallKernel { rt_table, dry_run }) => {
-        None => 201,
+            let southbound = NetlinkConnector::new(Some(rt_table)).await?;
+            run_connector::<NetlinkConnector>(args.route_server, dry_run, southbound).await
+        }
+        Some(Commands::Verify) => {
+            let southbound = DummyVerifier::default();
+            run_connector::<DummyVerifier>(args.route_server, false, southbound).await
+        }
+        None => bail!("A subcommand must be specified."),
     };
 
+    Ok(())
+}
+
+async fn run_connector<S: SouthboundInterface + Clone + Send + Sync + 'static>(
+    server_addr: String,
+    dry_run: bool,
+    southbound: S,
+) {
     let v4_joinhandle = {
-        let server_addr = args.route_server.clone();
+        let server_addr = server_addr.clone();
+        let southbound = southbound.clone();
         tokio::task::spawn(async move {
-            run_connector_v4::<NetlinkConnector>(
+            run_connector_v4::<S>(server_addr.clone(), dry_run, southbound)
-                server_addr.clone(),
+                .await
-                rt_table,
+                .unwrap();
-                args.dry_run,
-                NetlinkConnector::new(Some(rt_table)).await.unwrap(),
-            )
-            .await
-            .unwrap();
         })
     };
 
     let v6_joinhandle = {
-        let server_addr = args.route_server.clone();
+        let server_addr = server_addr.clone();
         tokio::task::spawn(async move {
-            run_connector_v6::<NetlinkConnector>(
+            run_connector_v6::<S>(server_addr, dry_run, southbound)
-                server_addr,
+                .await
-                rt_table,
+                .unwrap();
-                args.dry_run,
-                NetlinkConnector::new(Some(rt_table)).await.unwrap(),
-            )
-            .await
-            .unwrap();
         })
     };
 
@@ -68,6 +89,4 @@ async fn main() -> Result<()> {
             warn!("Unexpected exit of IPv6 connector");
         }
     }
-
-    Ok(())
 }

crates/route_client/src/fib_state.rs

@@ -41,12 +41,11 @@ pub struct FibState<A: Address, S: SouthboundInterface> {
     pub fib: IpLookupTable<A, Arc<Mutex<FibEntry>>>,
     pub southbound: S,
     pub af: AddressFamilyIdentifier,
-    pub table: u32,
 }
 
 impl<A: Address, S: SouthboundInterface> std::fmt::Debug for FibState<A, S> {
     fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> {
-        write!(f, "FibState af: {:?}, table: {}", self.af, self.table)
+        write!(f, "FibState af: {:?}", self.af)
     }
 }
 

crates/route_client/src/lib.rs

@@ -65,7 +65,6 @@ fn select_best_route(ps: &proto::PathSet) -> Option<proto::Path> {
 
 pub async fn run_connector_v4<S: SouthboundInterface>(
     route_server: String,
-    rt_table: u32,
     dry_run: bool,
     southbound: S,
 ) -> Result<()> {
@@ -74,7 +73,6 @@ pub async fn run_connector_v4<S: SouthboundInterface>(
         fib: IpLookupTable::new(),
         southbound,
         af: AddressFamilyIdentifier::Ipv4,
-        table: rt_table,
     };
 
     let uri = Uri::from_str(route_server.as_str()).unwrap();
@@ -135,7 +133,6 @@ pub async fn run_connector_v4<S: SouthboundInterface>(
 
 pub async fn run_connector_v6<S: SouthboundInterface>(
     route_server: String,
-    rt_table: u32,
     dry_run: bool,
     southbound: S,
 ) -> Result<()> {
@@ -143,7 +140,6 @@ pub async fn run_connector_v6<S: SouthboundInterface>(
         fib: IpLookupTable::new(),
         southbound,
         af: AddressFamilyIdentifier::Ipv6,
-        table: rt_table,
     };
 
     let uri = Uri::from_str(route_server.as_str()).unwrap();

crates/route_client/src/netlink.rs

@@ -17,6 +17,7 @@ use super::southbound_interface::SouthboundInterface;
 
 /// NetlinkConnector implements methods to read/update Linux networking stuff including
 /// routes and link level info.
+#[derive(Clone)]
 pub struct NetlinkConnector {
     handle: rtnetlink::Handle,
     table: Option<u32>,

crates/route_client/src/southbound_interface.rs

@@ -38,6 +38,7 @@ pub trait SouthboundInterface {
 
 /// DummyVerifier is a SouthboundInterface that checks that routes are not added more than
 /// once and not removed when there are none.
+#[derive(Clone)]
 pub struct DummyVerifier {
     route_state: HashMap<NLRI, IpAddr>,
 }

crates/server/src/peer.rs

@@ -568,7 +568,6 @@ where
                     self.hold_timer_expired().await?;
                 }
                 PeerTimerEvent::KeepaliveTimerExpire() => {
-                    trace!("Keepalive timer expired");
                     self.send_keepalive().await?;
                 }
             },
@@ -867,7 +866,6 @@ where
     /// keepalive message.
     /// Takes a lock on the peer object.
     async fn send_keepalive(&mut self) -> Result<(), std::io::Error> {
-        info!("Sending keepalive");
         match self.tcp_stream.as_mut() {
             Some(conn) => {
                 let keepalive = BGPMessage {