Rust cleanup, etc.

rust-zerotier-core/src/trace.rs

@@ -344,7 +344,34 @@ fn trace_optional_fingerprint(bytes: Option<&Vec<u8>>) -> Option<Fingerprint> {
     })
 }
 
+/// What layer or log level does this trace event belong to?
+pub enum TraceEventLayer {
+    VL1,
+    VL2,
+    VL2Filter,
+    VL2Multicast,
+    Other,
+}
+
 impl TraceEvent {
+    #[inline(always)]
+    pub fn layer(&self) -> TraceEventLayer {
+        match *self {
+            TraceEvent::UnexpectedError => TraceEventLayer::Other,
+
+            TraceEvent::ResetingPathsInScope => TraceEventLayer::VL1,
+            TraceEvent::TryingNewPath => TraceEventLayer::VL1,
+            TraceEvent::LearnedNewPath => TraceEventLayer::VL1,
+            TraceEvent::IncomingPacketDropped => TraceEventLayer::VL1,
+
+            TraceEvent::OutgoingFrameDropped => TraceEventLayer::VL2,
+            TraceEvent::IncomingFrameDropped => TraceEventLayer::VL2,
+            TraceEvent::NetworkConfigRequested => TraceEventLayer::VL2,
+            TraceEvent::NetworkFilter => TraceEventLayer::VL2Filter,
+            TraceEvent::NetworkCredentialRejected => TraceEventLayer::VL2,
+        }
+    }
+
     /// Decode a trace event packaged in a dictionary and return a TraceEvent if it is valid.
     pub fn parse_message(msg: &Dictionary) -> Option<TraceEvent> {
         msg.get_ui(ztcore::ZT_TRACE_FIELD_TYPE).map_or(None, |mt: u64| -> Option<TraceEvent> {

rust-zerotier-service/src/localconfig.rs

@@ -89,6 +89,22 @@ pub struct LocalConfigNetworkSettings {
     pub allow_default_route_override: bool,
 }
 
+#[derive(Serialize, Deserialize, Clone, PartialEq, Eq)]
+#[serde(default)]
+pub struct LocalConfigLogSettings {
+    pub path: Option<String>,
+    #[serde(rename = "maxSize")]
+    pub max_size: usize,
+    pub vl1: bool,
+    pub vl2: bool,
+    #[serde(rename = "vl2TraceRules")]
+    pub vl2_trace_rules: bool,
+    #[serde(rename = "vl2TraceMulticast")]
+    pub vl2_trace_multicast: bool,
+    pub debug: bool,
+    pub stderr: bool,
+}
+
 #[derive(Serialize, Deserialize, Clone, PartialEq, Eq)]
 #[serde(default)]
 pub struct LocalConfigSettings {
@@ -100,20 +116,8 @@ pub struct LocalConfigSettings {
     pub auto_port_search: bool,
     #[serde(rename = "portMapping")]
     pub port_mapping: bool,
-    #[serde(rename = "logPath")]
+    #[serde(rename = "log")]
-    pub log_path: Option<String>,
+    pub log: LocalConfigLogSettings,
-    #[serde(rename = "logSizeMax")]
-    pub log_size_max: usize,
-    #[serde(rename = "logVL1Events")]
-    pub log_vl1_events: bool,
-    #[serde(rename = "logVL2Events")]
-    pub log_vl2_events: bool,
-    #[serde(rename = "logFilterEvents")]
-    pub log_filter_events: bool,
-    #[serde(rename = "logMulticastEvents")]
-    pub log_multicast_events: bool,
-    #[serde(rename = "logToStderr")]
-    pub log_to_stderr: bool,
     #[serde(rename = "interfacePrefixBlacklist")]
     pub interface_prefix_blacklist: Vec<String>,
     #[serde(rename = "explicitAddresses")]
@@ -158,6 +162,22 @@ impl Default for LocalConfigNetworkSettings {
     }
 }
 
+impl Default for LocalConfigLogSettings {
+    fn default() -> Self {
+        // TODO: change before release to saner defaults
+        LocalConfigLogSettings {
+            path: None,
+            max_size: 131072,
+            vl1: true,
+            vl2: true,
+            vl2_trace_rules: true,
+            vl2_trace_multicast: true,
+            debug: true,
+            stderr: true,
+        }
+    }
+}
+
 impl LocalConfigSettings {
     #[cfg(target_os = "macos")]
     const DEFAULT_PREFIX_BLACKLIST: [&'static str; 8] = ["lo", "utun", "gif", "stf", "iptap", "pktap", "feth", "zt"];
@@ -191,13 +211,7 @@ impl Default for LocalConfigSettings {
             secondary_port: Some(zerotier_core::DEFAULT_SECONDARY_PORT),
             auto_port_search: true,
             port_mapping: true,
-            log_path: None,
+            log: LocalConfigLogSettings::default(),
-            log_size_max: 1048576,
-            log_vl1_events: false,
-            log_vl2_events: false,
-            log_filter_events: false,
-            log_multicast_events: false,
-            log_to_stderr: true, // TODO: change for release
             interface_prefix_blacklist: bl,
             explicit_addresses: Vec::new()
         }

rust-zerotier-service/src/log.rs

@@ -26,6 +26,7 @@ struct LogIntl {
     cur_size: u64,
     max_size: usize,
     log_to_stderr: bool,
+    debug: bool,
 }
 
 /// It's big it's heavy it's wood.
@@ -39,7 +40,7 @@ impl Log {
     /// Construct a new logger.
     /// If path is empty logs will not be written to files. If log_to_stderr is also
     /// false then no logs will be output at all.
-    pub fn new(path: &str, max_size: usize, log_to_stderr: bool, prefix: &str) -> Log {
+    pub fn new(path: &str, max_size: usize, log_to_stderr: bool, debug: bool, prefix: &str) -> Log {
         let mut p = String::from(prefix);
         if !p.is_empty() {
             p.push(' ');
@@ -52,6 +53,7 @@ impl Log {
                 cur_size: 0,
                 max_size: if max_size < Log::MIN_MAX_SIZE { Log::MIN_MAX_SIZE } else { max_size },
                 log_to_stderr,
+                debug,
             }),
         }
     }
@@ -64,61 +66,79 @@ impl Log {
         self.inner.lock().unwrap().log_to_stderr = log_to_stderr;
     }
 
+    pub fn set_debug(&self, debug: bool) {
+        self.inner.lock().unwrap().debug = debug;
+    }
+
+    fn log_internal(&self, l: &mut LogIntl, s: &str, pfx: &'static str) {
+        if !s.is_empty() {
+            let log_line = format!("{}[{}] {}{}\n", l.prefix.as_str(), chrono::Local::now().format("%Y-%m-%d %H:%M:%S").to_string(), pfx, s);
+            if !l.path.is_empty() {
+                if l.file.is_none() {
+                    let mut f = OpenOptions::new().read(true).write(true).create(true).open(l.path.as_str());
+                    if f.is_err() {
+                        return;
+                    }
+                    let mut f = f.unwrap();
+                    let eof = f.seek(SeekFrom::End(0));
+                    if eof.is_err() {
+                        return;
+                    }
+                    l.cur_size = eof.unwrap();
+                    l.file = Some(f);
+                }
+
+                if l.max_size > 0 && l.cur_size > l.max_size as u64 {
+                    l.file = None;
+                    l.cur_size = 0;
+
+                    let mut old_path = l.path.clone();
+                    old_path.push_str(".old");
+                    let _ = std::fs::remove_file(old_path.as_str());
+                    let _ = std::fs::rename(l.path.as_str(), old_path.as_str());
+                    let _ = std::fs::remove_file(l.path.as_str()); // should fail
+
+                    let mut f = OpenOptions::new().read(true).write(true).create(true).open(l.path.as_str());
+                    if f.is_err() {
+                        return;
+                    }
+                    l.file = Some(f.unwrap());
+                }
+
+                let f = l.file.as_mut().unwrap();
+                let e = f.write_all(log_line.as_bytes());
+                if e.is_err() {
+                    eprintln!("ERROR: I/O error writing to log: {}", e.err().unwrap().to_string());
+                    l.file = None;
+                } else {
+                    let _ = f.flush();
+                    l.cur_size += log_line.len() as u64;
+                }
+            }
+
+            if l.log_to_stderr {
+                stderr().write_all(log_line.as_bytes());
+            }
+        }
+    }
+
     pub fn log<S: AsRef<str>>(&self, s: S) {
         let mut l = self.inner.lock().unwrap();
+        self.log_internal(&mut (*l), s.as_ref(), "");
+    }
 
-        let ss: &str = s.as_ref();
+    pub fn debug<S: AsRef<str>>(&self, s: S) {
-        if ss.starts_with("FATAL") {
+        let mut l = self.inner.lock().unwrap();
-            eprintln!("{}", ss);
+        if l.debug {
+            self.log_internal(&mut (*l), s.as_ref(), "DEBUG");
         }
-        let log_line = format!("{}[{}] {}\n", l.prefix.as_str(), chrono::Local::now().format("%Y-%m-%d %H:%M:%S").to_string(), ss);
+    }
 
-        if l.path.len() > 0 {
+    pub fn fatal<S: AsRef<str>>(&self, s: S) {
-            if l.file.is_none() {
+        let mut l = self.inner.lock().unwrap();
-                let mut f = OpenOptions::new().read(true).write(true).create(true).open(l.path.as_str());
+        let ss = s.as_ref();
-                if f.is_err() {
+        self.log_internal(&mut (*l), ss, "FATAL");
-                    return;
+        eprintln!("FATAL: {}", ss);
-                }
-                let mut f = f.unwrap();
-                let eof = f.seek(SeekFrom::End(0));
-                if eof.is_err() {
-                    return;
-                }
-                l.cur_size = eof.unwrap();
-                l.file = Some(f);
-            }
-
-            if l.max_size > 0 && l.cur_size > l.max_size as u64 {
-                l.file = None;
-                l.cur_size = 0;
-
-                let mut old_path = l.path.clone();
-                old_path.push_str(".old");
-                let _ = std::fs::remove_file(old_path.as_str());
-                let _ = std::fs::rename(l.path.as_str(), old_path.as_str());
-                let _ = std::fs::remove_file(l.path.as_str()); // should fail
-
-                let mut f = OpenOptions::new().read(true).write(true).create(true).open(l.path.as_str());
-                if f.is_err() {
-                    return;
-                }
-                l.file = Some(f.unwrap());
-            }
-
-            let f = l.file.as_mut().unwrap();
-            let e = f.write_all(log_line.as_bytes());
-            if e.is_err() {
-                eprintln!("ERROR: I/O error writing to log: {}", e.err().unwrap().to_string());
-                l.file = None;
-            } else {
-                let _ = f.flush();
-                l.cur_size += log_line.len() as u64;
-            }
-        }
-
-        if l.log_to_stderr {
-            stderr().write_all(log_line.as_bytes());
-        }
     }
 }
 
@@ -129,6 +149,13 @@ macro_rules! l(
     }
 );
 
+#[macro_export]
+macro_rules! d(
+    ($logger:ident, $($arg:tt)*) => {
+        $logger.debug(format!($($arg)*))
+    }
+);
+
 unsafe impl Sync for Log {}
 
 /*

rust-zerotier-service/src/service.rs

@@ -24,6 +24,7 @@ use warp::http::{HeaderMap, Method, StatusCode};
 use warp::hyper::body::Bytes;
 
 use zerotier_core::*;
+use zerotier_core::trace::{TraceEvent, TraceEventLayer};
 
 use crate::fastudpsocket::*;
 use crate::getifaddrs;
@@ -61,27 +62,43 @@ impl NodeEventHandler<Network> for Service {
     #[inline(always)]
     fn event(&self, event: Event, event_data: &[u8]) {
         match event {
-            Event::Up => {}
+            Event::Up => {
+                let _ = self.node.upgrade().map(|n: Arc<Node<Service, Network>>| {
+                    d!(self.log, "node {} started up in data store '{}'", n.address().to_string(), self.store.base_path.to_str().unwrap());
+                });
+            },
             Event::Down => {
+                d!(self.log, "node shutting down.");
                 self.run.store(false, Ordering::Relaxed);
-            }
+            },
             Event::Online => {
+                d!(self.log, "node is online.");
                 self.online.store(true, Ordering::Relaxed);
-            }
+            },
             Event::Offline => {
+                d!(self.log, "node is offline.");
                 self.online.store(true, Ordering::Relaxed);
-            }
+            },
             Event::Trace => {
                 if !event_data.is_empty() {
                     let _ = Dictionary::new_from_bytes(event_data).map(|tm| {
-                        let tm = zerotier_core::trace::TraceEvent::parse_message(&tm);
+                        let tm = TraceEvent::parse_message(&tm);
-                        let _ = tm.map(|tm| {
+                        let _ = tm.map(|tm: TraceEvent| {
-                            self.log.log(tm.to_string());
+                            let local_config = self.local_config();
+                            if match tm.layer() {
+                                TraceEventLayer::VL1 => local_config.settings.log.vl1,
+                                TraceEventLayer::VL2 => local_config.settings.log.vl2,
+                                TraceEventLayer::VL2Filter => local_config.settings.log.vl2_trace_rules,
+                                TraceEventLayer::VL2Multicast => local_config.settings.log.vl2_trace_multicast,
+                                TraceEventLayer::Other => true,
+                            } {
+                                self.log.log(tm.to_string());
+                            }
                         });
                     });
                 }
-            }
+            },
-            Event::UserMessage => {}
+            Event::UserMessage => {},
         }
     }
 
@@ -101,7 +118,7 @@ impl NodeEventHandler<Network> for Service {
     }
 
     #[inline(always)]
-    fn path_check(&self, address: Address, id: &Identity, local_socket: i64, sock_addr: &InetAddress) -> bool {
+    fn path_check(&self, _: Address, _: &Identity, _: i64, _: &InetAddress) -> bool {
         true
     }
 
@@ -115,6 +132,7 @@ impl NodeEventHandler<Network> for Service {
             } else {
                 let t = c.try_.get((zerotier_core::random() as usize) % c.try_.len());
                 t.map_or(None, |v: &InetAddress| {
+                    d!(self.log, "path lookup for {} returned {}", address.to_string(), v.to_string());
                     Some(v.clone())
                 })
             }
@@ -155,18 +173,20 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
     let local_config = Arc::new(store.read_local_conf(false).unwrap_or_else(|_| { LocalConfig::default() }));
 
     let log = Arc::new(Log::new(
-        if local_config.settings.log_path.as_ref().is_some() {
+        if local_config.settings.log.path.as_ref().is_some() {
-            local_config.settings.log_path.as_ref().unwrap().as_str()
+            local_config.settings.log.path.as_ref().unwrap().as_str()
         } else {
             store.default_log_path.to_str().unwrap()
         },
-        local_config.settings.log_size_max,
+        local_config.settings.log.max_size,
-        local_config.settings.log_to_stderr,
+        local_config.settings.log.stderr,
+        local_config.settings.log.debug,
         "",
     ));
 
     // Generate authtoken.secret from secure random bytes if not already set.
     let auth_token = auth_token.unwrap_or_else(|| -> String {
+        d!(log, "authtoken.secret not found, generating new...");
         let mut rb = [0_u8; 64];
         unsafe { crate::osdep::getSecureRandom(rb.as_mut_ptr().cast(), 64) };
         let mut generated_auth_token = String::new();
@@ -184,7 +204,7 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
         generated_auth_token
     });
     if auth_token.is_empty() {
-        l!(log, "FATAL: unable to write authtoken.secret to '{}'", store.base_path.to_str().unwrap());
+        log.fatal(format!("unable to write authtoken.secret to '{}'", store.base_path.to_str().unwrap()));
         return 1;
     }
     let auth_token = Arc::new(auth_token);
@@ -207,7 +227,7 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
 
         let node = Node::new(service.clone(), ms_since_epoch());
         if node.is_err() {
-            l!(log, "FATAL: error initializing node: {}", node.err().unwrap().to_str());
+            log.fatal(format!("error initializing node: {}", node.err().unwrap().to_str()));
             process_exit_value = 1;
             return;
         }
@@ -220,6 +240,7 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
         loop {
             let mut local_config = service.local_config();
 
+            d!(log, "starting local HTTP API server on 127.0.0.1 port {}", local_config.settings.primary_port);
             let (mut shutdown_tx, mut shutdown_rx) = futures::channel::oneshot::channel();
             let warp_server;
             {
@@ -253,7 +274,7 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
                 ).try_bind_with_graceful_shutdown((IpAddr::from([127_u8, 0_u8, 0_u8, 1_u8]), local_config.settings.primary_port), async { let _ = shutdown_rx.await; });
             }
             if warp_server.is_err() {
-                l!(log, "ERROR: local API http server failed to bind to port {} or failed to start: {}", local_config.settings.primary_port, warp_server.err().unwrap().to_string());
+                l!(log, "ERROR: local API http server failed to bind to port {} or failed to start: {}, restarting inner loop...", local_config.settings.primary_port, warp_server.err().unwrap().to_string());
                 break;
             }
             let warp_server = tokio_rt.spawn(warp_server.unwrap().1);
@@ -265,6 +286,7 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
             // scanning for significant configuration changes. Some major changes may require
             // the inner loop to exit and be restarted.
             let mut last_checked_config: i64 = 0;
+            d!(log, "local HTTP API server running, inner loop starting.");
             loop {
                 let loop_start = ms_since_epoch();
                 let mut now: i64 = 0;
@@ -276,11 +298,12 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
                         now = ms_since_epoch();
                         let actual_delay = now - loop_start;
                         if actual_delay > ((loop_delay as i64) * 4_i64) {
-                            l!(log, "likely sleep/wake detected, reestablishing links...");
+                            l!(log, "likely sleep/wake detected due to excess delay, reestablishing links...");
                             // TODO: handle likely sleep/wake or other system interruption
                         }
                     },
                     _ = interrupt_rx.next() => {
+                        d!(log, "inner loop delay interrupted!");
                         now = ms_since_epoch();
                     },
                     _ = tokio::signal::ctrl_c() => {
@@ -298,6 +321,7 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
                     // Check for changes to local.conf.
                     let new_config = store.read_local_conf(true);
                     if new_config.is_ok() {
+                        d!(log, "local.conf changed on disk, reloading.");
                         service.set_local_config(new_config.unwrap());
                     }
 
@@ -306,11 +330,14 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
                     if local_config.settings.primary_port != next_local_config.settings.primary_port {
                         break;
                     }
-                    if local_config.settings.log_size_max != next_local_config.settings.log_size_max {
+                    if local_config.settings.log.max_size != next_local_config.settings.log.max_size {
-                        log.set_max_size(next_local_config.settings.log_size_max);
+                        log.set_max_size(next_local_config.settings.log.max_size);
                     }
-                    if local_config.settings.log_to_stderr != next_local_config.settings.log_to_stderr {
+                    if local_config.settings.log.stderr != next_local_config.settings.log.stderr {
-                        log.set_log_to_stderr(next_local_config.settings.log_to_stderr);
+                        log.set_log_to_stderr(next_local_config.settings.log.stderr);
+                    }
+                    if local_config.settings.log.debug != next_local_config.settings.log.debug {
+                        log.set_debug(next_local_config.settings.log.debug);
                     }
                     local_config = next_local_config;
 
@@ -342,18 +369,21 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
                         }
                     }
                     for k in udp_sockets_to_close.iter() {
+                        d!(log, "unbinding UDP socket at {} (no longer appears to be present or port has changed)", k.to_string());
                         udp_sockets.remove(k);
                     }
 
                     // Create sockets for unbound addresses.
                     for addr in system_addrs.iter() {
                         if !udp_sockets.contains_key(addr.0) {
-                            let s = FastUDPSocket::new(addr.1.as_str(), addr.0, move |raw_socket: &FastUDPRawOsSocket, from_address: &InetAddress, data: Buffer| {
+                            let _ = FastUDPSocket::new(addr.1.as_str(), addr.0, move |raw_socket: &FastUDPRawOsSocket, from_address: &InetAddress, data: Buffer| {
                                 // TODO: incoming packet handler
-                            });
+                            }).map_or_else(|e| {
-                            if s.is_ok() {
+                                d!(log, "error binding UDP socket to {}: {}", addr.0.to_string(), e.to_string());
+                            }, |s| {
+                                d!(log, "bound UDP socket at {}", addr.0.to_string());
                                 udp_sockets.insert(addr.0.clone(), s.unwrap());
-                            }
+                            });
                         }
                     }
 
@@ -375,21 +405,18 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
                             }
                         }
                     }
-
                     if primary_port_bind_failure {
                         if local_config.settings.auto_port_search {
                             // TODO: port hunting
                         } else {
-                            l!(log, "primary port {} failed to bind, waiting and trying again...", local_config.settings.primary_port);
+                            l!(log, "WARNING: failed to bind to any address at primary port {} (will try again)", local_config.settings.primary_port);
-                            break;
                         }
                     }
-
                     if secondary_port_bind_failure {
                         if local_config.settings.auto_port_search {
                             // TODO: port hunting
                         } else {
-                            l!(log, "secondary port {} failed to bind (non-fatal, will try again)", local_config.settings.secondary_port.unwrap_or(0));
+                            l!(log, "WARNING: failed to bind to any address at secondary port {} (will try again)", local_config.settings.secondary_port.unwrap_or(0));
                         }
                     }
                 }
@@ -402,6 +429,8 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
                 loop_delay = node.process_background_tasks(now);
             }
 
+            d!(log, "inner loop exited, shutting down local API HTTP server...");
+
             // Gracefully shut down the local web server.
             let _ = shutdown_tx.send(());
             let _ = warp_server.await;
@@ -409,10 +438,12 @@ pub(crate) fn run(store: &Arc<Store>, auth_token: Option<String>) -> i32 {
             // Sleep for a brief period of time to prevent thrashing if some invalid
             // state is hit that causes the inner loop to keep breaking.
             if !service.run.load(Ordering::Relaxed) {
+                d!(log, "exiting.");
                 break;
             }
             let _ = tokio::time::sleep(Duration::from_secs(1)).await;
             if !service.run.load(Ordering::Relaxed) {
+                d!(log, "exiting.");
                 break;
             }
         }

rust-zerotier-service/src/utils.rs

@@ -18,9 +18,9 @@ use crate::osdep;
 
 #[inline(always)]
 pub(crate) fn sha512<T: AsRef<[u8]>>(data: T) -> [u8; 64] {
+    let mut r: MaybeUninit<[u8; 64]> = MaybeUninit::uninit();
+    let d = data.as_ref();
     unsafe {
-        let mut r: MaybeUninit<[u8; 64]> = MaybeUninit::uninit();
-        let d = data.as_ref();
         osdep::sha512(d.as_ptr().cast(), d.len() as c_uint, r.as_mut_ptr().cast());
         r.assume_init()
     }
@@ -28,9 +28,9 @@ pub(crate) fn sha512<T: AsRef<[u8]>>(data: T) -> [u8; 64] {
 
 #[inline(always)]
 pub(crate) fn sha384<T: AsRef<[u8]>>(data: T) -> [u8; 48] {
+    let mut r: MaybeUninit<[u8; 48]> = MaybeUninit::uninit();
+    let d = data.as_ref();
     unsafe {
-        let mut r: MaybeUninit<[u8; 48]> = MaybeUninit::uninit();
-        let d = data.as_ref();
         osdep::sha384(d.as_ptr().cast(), d.len() as c_uint, r.as_mut_ptr().cast());
         r.assume_init()
     }

rust-zerotier-service/src/vnic/mac_feth_tap.rs

@@ -58,6 +58,7 @@ use crate::osdep::getifmaddrs;
 
 const BPF_BUFFER_SIZE: usize = 131072;
 const IFCONFIG: &str = "/sbin/ifconfig";
+const SYSCTL: &str = "/usr/sbin/sysctl";
 
 // Holds names of feth devices and destroys them on Drop.
 struct MacFethDevice {
@@ -184,6 +185,12 @@ impl MacFethTap {
         }
         device_ipv6_set_params(&device_name, true, false);
 
+        // Set sysctl for max if_fake MTU. This is allowed to fail since this sysctl doesn't
+        // exist on older versions of MacOS (and isn't required there). 16000 is larger than
+        // anything ZeroTier supports. OS max is 16384 - some overhead.
+        let _ = Command::new(SYSCTL).arg("net.link.fake.max_mtu").arg("16000").spawn().map(|mut c| { let _ = c.wait(); });
+
+        // Create pair of feth interfaces and create MacFethDevice struct.
         let cmd = Command::new(IFCONFIG).arg(&device_name).arg("create").spawn();
         if cmd.is_err() {
             return Err(format!("unable to create device '{}': {}", device_name.as_str(), cmd.err().unwrap().to_string()));
@@ -194,36 +201,43 @@ impl MacFethTap {
             return Err(format!("unable to create device '{}': {}", peer_device_name.as_str(), cmd.err().unwrap().to_string()));
         }
         let _ = cmd.unwrap().wait();
-
+        let device = MacFethDevice {
-        let device = MacFethDevice{
             name: device_name,
             peer_name: peer_device_name,
         };
 
+        // Set link-layer (MAC) address of primary interface.
         let cmd = Command::new(IFCONFIG).arg(&device.name).arg("lladdr").arg(mac.to_string()).spawn();
         if cmd.is_err() {
             return Err(format!("unable to configure device '{}': {}", &device.name, cmd.err().unwrap().to_string()));
         }
         let _ = cmd.unwrap().wait();
 
+        // Bind peer interfaces together.
         let cmd = Command::new(IFCONFIG).arg(&device.peer_name).arg("peer").arg(device.name.as_str()).spawn();
         if cmd.is_err() {
             return Err(format!("unable to configure device '{}': {}", &device.peer_name, cmd.err().unwrap().to_string()));
         }
         let _ = cmd.unwrap().wait();
-        let cmd = Command::new(IFCONFIG).arg(&device.peer_name).arg("mtu").arg("16370").arg("up").spawn();
+
+        // Set MTU of secondary peer interface, bring up.
+        let cmd = Command::new(IFCONFIG).arg(&device.peer_name).arg("mtu").arg(mtu.to_string()).arg("up").spawn();
         if cmd.is_err() {
             return Err(format!("unable to configure device '{}': {}", &device.peer_name, cmd.err().unwrap().to_string()));
         }
         let _ = cmd.unwrap().wait();
 
+        // Set MTU and metric of primary interface, bring up.
         let cmd = Command::new(IFCONFIG).arg(&device.name).arg("mtu").arg(mtu.to_string()).arg("metric").arg(metric.to_string()).arg("up").spawn();
         if cmd.is_err() {
             return Err(format!("unable to configure device '{}': {}", &device.name.as_str(), cmd.err().unwrap().to_string()));
         }
         let _ = cmd.unwrap().wait();
 
-        let mut bpf_no: u32 = 1; // start at 1 since some software hard-codes /dev/bpf0
+        // Look for a /dev/bpf node to open. Start at 1 since some software
+        // hard codes /dev/bpf0 and we don't want to break it. If all BPF nodes
+        // are taken MacOS automatically adds more, so we shouldn't run out.
+        let mut bpf_no: u32 = 1;
         let mut bpf_fd: c_int = -1;
         loop {
             if bpf_devices_used.contains(&bpf_no) {
@@ -245,7 +259,8 @@ impl MacFethTap {
             return Err(String::from("unable to open /dev/bpf## where attempted ## from 1 to 1000"));
         }
 
-        // Set/get buffer length.
+        // Set/get buffer length to use with reads from BPF device, trying to
+        // use up to BPF_BUFFER_SIZE bytes.
         let mut fl: c_int = BPF_BUFFER_SIZE as c_int;
         if unsafe { osdep::ioctl(bpf_fd as c_int, osdep::c_BIOCSBLEN, (&mut fl as *mut c_int).cast::<c_void>()) } != 0 {
             unsafe { osdep::close(bpf_fd); }
@@ -253,21 +268,21 @@ impl MacFethTap {
         }
         let bpf_read_size = fl as osdep::size_t;
 
-        // Set immediate mode.
+        // Set immediate mode for "live" capture.
         fl = 1;
         if unsafe { osdep::ioctl(bpf_fd as c_int, osdep::c_BIOCIMMEDIATE, (&mut fl as *mut c_int).cast::<c_void>()) } != 0 {
             unsafe { osdep::close(bpf_fd); }
             return Err(String::from("unable to configure BPF device"));
         }
 
-        // We don't want to see packets we inject.
+        // Do not send us back packets we inject or send.
         fl = 0;
         if unsafe { osdep::ioctl(bpf_fd as c_int, osdep::c_BIOCSSEESENT, (&mut fl as *mut c_int).cast::<c_void>()) } != 0 {
             unsafe { osdep::close(bpf_fd); }
             return Err(String::from("unable to configure BPF device"));
         }
 
-        // Set device name that we're monitoring.
+        // Bind BPF to secondary feth device.
         let mut bpf_ifr: osdep::ifreq = unsafe { std::mem::zeroed() };
         let peer_dev_name_bytes = device.peer_name.as_bytes();
         unsafe { copy_nonoverlapping(peer_dev_name_bytes.as_ptr(), bpf_ifr.ifr_name.as_mut_ptr().cast::<u8>(), if peer_dev_name_bytes.len() > (bpf_ifr.ifr_name.len() - 1) { bpf_ifr.ifr_name.len() - 1 } else { peer_dev_name_bytes.len() }); }
@@ -276,14 +291,14 @@ impl MacFethTap {
             return Err(String::from("unable to configure BPF device"));
         }
 
-        // Include Ethernet header.
+        // Include Ethernet header in BPF captures.
         fl = 1;
         if unsafe { osdep::ioctl(bpf_fd as c_int, osdep::c_BIOCSHDRCMPLT, (&mut fl as *mut c_int).cast::<c_void>()) } != 0 {
             unsafe { osdep::close(bpf_fd); }
             return Err(String::from("unable to configure BPF device"));
         }
 
-        // Set promiscuous mode so bridging could work, etc.
+        // Set promiscuous mode so bridging can work.
         fl = 1;
         if unsafe { osdep::ioctl(bpf_fd as c_int, osdep::c_BIOCPROMISC, (&mut fl as *mut c_int).cast::<c_void>()) } != 0 {
             unsafe { osdep::close(bpf_fd); }
@@ -347,10 +362,10 @@ impl MacFethTap {
 
         Ok(MacFethTap {
             network_id: nwid.0,
-            device: device,
+            device,
-            ndrv_fd: ndrv_fd,
+            ndrv_fd,
-            bpf_fd: bpf_fd,
+            bpf_fd,
-            bpf_no: bpf_no,
+            bpf_no,
             bpf_read_thread: Cell::new(Some(t.unwrap()))
         })
     }
@@ -413,7 +428,7 @@ impl VNIC for MacFethTap {
     }
 
     #[inline(always)]
-    fn put(&self, source_mac: &zerotier_core::MAC, dest_mac: &zerotier_core::MAC, ethertype: u16, vlan_id: u16, data: *const u8, len: usize) -> bool {
+    fn put(&self, source_mac: &zerotier_core::MAC, dest_mac: &zerotier_core::MAC, ethertype: u16, _vlan_id: u16, data: *const u8, len: usize) -> bool {
         let dm = dest_mac.0;
         let sm = source_mac.0;
         let mut hdr: [u8; 14] = [(dm >> 40) as u8, (dm >> 32) as u8, (dm >> 24) as u8, (dm >> 16) as u8, (dm >> 8) as u8, dm as u8, (sm >> 40) as u8, (sm >> 32) as u8, (sm >> 24) as u8, (sm >> 16) as u8, (sm >> 8) as u8, sm as u8, (ethertype >> 8) as u8, ethertype as u8];
@@ -451,5 +466,6 @@ impl Drop for MacFethTap {
         if t.is_some() {
             let _ = t.unwrap().join();
         }
+        // NOTE: the feth devices are destroyed by MacFethDevice's drop().
     }
 }