Log unresolved processes in more detail + general refactor (#318)

* Refactor & reorganisation of `get_proc_name` * Log both src & dst when we fail to resolve connection to process - Per recommendation in #196 (comment)
imsnif · Oct 31, 2023 · aef30c9 · aef30c9
1 parent 62c0fbf
commit aef30c9
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 79 deletions.
diff --git a/src/display/ui_state.rs b/src/display/ui_state.rs
@@ -1,5 +1,4 @@
 use std::{
-    cell::RefCell,
     cmp,
     collections::{HashMap, HashSet, VecDeque},
     hash::Hash,
@@ -90,71 +89,10 @@ pub struct UIState {
     pub remote_addresses_map: HashMap<IpAddr, NetworkData>,
     pub connections_map: HashMap<Connection, ConnectionData>,
     /// Used for reducing logging noise.
-    known_orphan_sockets: RefCell<VecDeque<LocalSocket>>,
+    known_orphan_sockets: VecDeque<LocalSocket>,
 }
 
 impl UIState {
-    fn get_proc_name<'a>(
-        &self,
-        connections_to_procs: &'a HashMap<LocalSocket, String>,
-        local_socket: &LocalSocket,
-    ) -> Option<&'a String> {
-        let name = connections_to_procs
-            // direct match
-            .get(local_socket)
-            // IPv4-mapped IPv6 addresses
-            .or_else(|| {
-                let swapped: IpAddr = match local_socket.ip {
-                    IpAddr::V4(v4) => v4.to_ipv6_mapped().into(),
-                    IpAddr::V6(v6) => v6.to_ipv4_mapped()?.into(),
-                };
-                connections_to_procs.get(&LocalSocket {
-                    ip: swapped,
-                    ..*local_socket
-                })
-            })
-            // address unspecified
-            .or_else(|| {
-                connections_to_procs.get(&LocalSocket {
-                    ip: Ipv4Addr::UNSPECIFIED.into(),
-                    ..*local_socket
-                })
-            })
-            .or_else(|| {
-                connections_to_procs.get(&LocalSocket {
-                    ip: Ipv6Addr::UNSPECIFIED.into(),
-                    ..*local_socket
-                })
-            });
-
-        if name.is_none() {
-            let mut orphans = self.known_orphan_sockets.borrow_mut();
-            // only log each orphan connection once
-            if !orphans.contains(local_socket) {
-                // newer connections go in the front so that searches are faster
-                // basically recency bias
-                orphans.push_front(*local_socket);
-                orphans.truncate(10_000); // arbitrary maximum backlog
-
-                match connections_to_procs.iter().find(
-                    |(&LocalSocket { port, protocol, .. }, _)| {
-                        port == local_socket.port && protocol == local_socket.protocol
-                    },
-                ) {
-                    Some((lookalike, name)) => {
-                        mt_log!(
-                            warn,
-                            r#""{name}" owns a similar looking connection, but its local ip doesn't match."#
-                        );
-                        mt_log!(warn, "Looking for: {local_socket}; found: {lookalike}");
-                    }
-                    None => mt_log!(warn, "Cannot determine which process owns {local_socket}."),
-                };
-            }
-        }
-
-        name
-    }
     pub fn update(
         &mut self,
         connections_to_procs: HashMap<LocalSocket, String>,
@@ -197,18 +135,46 @@ impl UIState {
                 total_bytes_downloaded += connection_info.total_bytes_downloaded;
                 total_bytes_uploaded += connection_info.total_bytes_uploaded;
 
-                let data_for_process = if let Some(process_name) =
-                    self.get_proc_name(connections_to_procs, &connection.local_socket)
-                {
-                    connection_data.process_name = process_name.clone();
-                    processes
-                        .entry(connection_data.process_name.clone())
-                        .or_default()
-                } else {
-                    connection_data.process_name = String::from("<UNKNOWN>");
-                    processes
-                        .entry(connection_data.process_name.clone())
-                        .or_default()
+                let data_for_process = {
+                    let local_socket = connection.local_socket;
+                    let process_name = get_proc_name(connections_to_procs, &local_socket);
+
+                    // only log each orphan connection once
+                    if process_name.is_none() && !self.known_orphan_sockets.contains(&local_socket)
+                    {
+                        // newer connections go in the front so that searches are faster
+                        // basically recency bias
+                        self.known_orphan_sockets.push_front(local_socket);
+                        self.known_orphan_sockets.truncate(10_000); // arbitrary maximum backlog
+
+                        match connections_to_procs
+                            .iter()
+                            .find(|(&LocalSocket { port, protocol, .. }, _)| {
+                                port == local_socket.port && protocol == local_socket.protocol
+                            })
+                            .and_then(|(local_conn_lookalike, name)| {
+                                network_utilization
+                                    .connections
+                                    .keys()
+                                    .find(|conn| &conn.local_socket == local_conn_lookalike)
+                                    .map(|conn| (conn, name))
+                            }) {
+                            Some((lookalike, name)) => {
+                                mt_log!(
+                                    warn,
+                                    r#""{name}" owns a similar looking connection, but its local ip doesn't match."#
+                                );
+                                mt_log!(warn, "Looking for: {connection:?}; found: {lookalike:?}");
+                            }
+                            None => {
+                                mt_log!(warn, "Cannot determine which process owns {connection:?}");
+                            }
+                        };
+                    }
+
+                    let process_display_name = process_name.unwrap_or("<UNKNOWN>").to_owned();
+                    connection_data.process_name = process_display_name.clone();
+                    processes.entry(process_display_name).or_default()
                 };
 
                 data_for_process.total_bytes_downloaded += connection_info.total_bytes_downloaded;
@@ -252,6 +218,40 @@ impl UIState {
     }
 }
 
+fn get_proc_name<'a>(
+    connections_to_procs: &'a HashMap<LocalSocket, String>,
+    local_socket: &LocalSocket,
+) -> Option<&'a str> {
+    connections_to_procs
+        // direct match
+        .get(local_socket)
+        // IPv4-mapped IPv6 addresses
+        .or_else(|| {
+            let swapped: IpAddr = match local_socket.ip {
+                IpAddr::V4(v4) => v4.to_ipv6_mapped().into(),
+                IpAddr::V6(v6) => v6.to_ipv4_mapped()?.into(),
+            };
+            connections_to_procs.get(&LocalSocket {
+                ip: swapped,
+                ..*local_socket
+            })
+        })
+        // address unspecified
+        .or_else(|| {
+            connections_to_procs.get(&LocalSocket {
+                ip: Ipv4Addr::UNSPECIFIED.into(),
+                ..*local_socket
+            })
+        })
+        .or_else(|| {
+            connections_to_procs.get(&LocalSocket {
+                ip: Ipv6Addr::UNSPECIFIED.into(),
+                ..*local_socket
+            })
+        })
+        .map(String::as_str)
+}
+
 fn merge_bandwidth<K, V>(self_map: &mut HashMap<K, V>, other_map: HashMap<K, V>)
 where
     K: Eq + Hash,

diff --git a/src/network/connection.rs b/src/network/connection.rs
@@ -30,20 +30,30 @@ impl fmt::Display for Protocol {
     }
 }
 
-#[derive(Clone, Ord, PartialOrd, PartialEq, Eq, Hash, Debug, Copy)]
+#[derive(Clone, Ord, PartialOrd, PartialEq, Eq, Hash, Copy)]
 pub struct Socket {
     pub ip: IpAddr,
     pub port: u16,
 }
 
-#[derive(PartialEq, Hash, Eq, Clone, PartialOrd, Ord, Debug, Copy)]
+impl fmt::Debug for Socket {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let Socket { ip, port } = self;
+        match ip {
+            IpAddr::V4(v4) => write!(f, "{v4}:{port}"),
+            IpAddr::V6(v6) => write!(f, "[{v6}]:{port}"),
+        }
+    }
+}
+
+#[derive(PartialEq, Hash, Eq, Clone, PartialOrd, Ord, Copy)]
 pub struct LocalSocket {
     pub ip: IpAddr,
     pub port: u16,
     pub protocol: Protocol,
 }
 
-impl fmt::Display for LocalSocket {
+impl fmt::Debug for LocalSocket {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let LocalSocket { ip, port, protocol } = self;
         match ip {
@@ -53,12 +63,22 @@ impl fmt::Display for LocalSocket {
     }
 }
 
-#[derive(PartialEq, Hash, Eq, Clone, PartialOrd, Ord, Debug, Copy)]
+#[derive(PartialEq, Hash, Eq, Clone, PartialOrd, Ord, Copy)]
 pub struct Connection {
     pub remote_socket: Socket,
     pub local_socket: LocalSocket,
 }
 
+impl fmt::Debug for Connection {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let Connection {
+            remote_socket,
+            local_socket,
+        } = self;
+        write!(f, "{local_socket:?} => {remote_socket:?}")
+    }
+}
+
 pub fn display_ip_or_host(ip: IpAddr, ip_to_host: &HashMap<IpAddr, String>) -> String {
     match ip_to_host.get(&ip) {
         Some(host) => host.clone(),