Refined network management (#1672)

This PR implements several refinements to the way that NetworkManager
controls the network interface.

- The monitor detects and logs changes to the network address
- The monitor detects and logs changes to the connection and will
reinitialize the connection if needed
- Remove NetworkInterface.java class, which wasn't used anywhere
- Use java.net.NetworkInterface to get IP addresses for any interface
(device)
- Adds a metric for the current IP address (address on the currently
selected interface)

photon-core/src/main/java/org/photonvision/common/hardware/metrics/MetricsManager.java

@@ -20,6 +20,7 @@ package org.photonvision.common.hardware.metrics;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.HashMap;
+import org.photonvision.common.configuration.ConfigManager;
 import org.photonvision.common.configuration.HardwareConfig;
 import org.photonvision.common.dataflow.DataChangeService;
 import org.photonvision.common.dataflow.events.OutgoingUIEvent;
@@ -31,6 +32,7 @@ import org.photonvision.common.hardware.metrics.cmds.PiCmds;
 import org.photonvision.common.hardware.metrics.cmds.RK3588Cmds;
 import org.photonvision.common.logging.LogGroup;
 import org.photonvision.common.logging.Logger;
+import org.photonvision.common.networking.NetworkUtils;
 import org.photonvision.common.util.ShellExec;
 
 public class MetricsManager {
@@ -119,6 +121,14 @@ public class MetricsManager {
         return safeExecute(cmds.ramUsageCommand);
     }
 
+    public String getIpAddress() {
+        String dev = ConfigManager.getInstance().getConfig().getNetworkConfig().networkManagerIface;
+        logger.debug("Requesting IP addresses for \"" + dev + "\"");
+        String addr = NetworkUtils.getIPAddresses(dev);
+        logger.debug("Got value \"" + addr + "\"");
+        return addr;
+    }
+
     public void publishMetrics() {
         logger.debug("Publishing Metrics...");
         final var metrics = new HashMap<String, String>();
@@ -133,6 +143,7 @@ public class MetricsManager {
         metrics.put("gpuMemUtil", this.getMallocedMemory());
         metrics.put("diskUtilPct", this.getUsedDiskPct());
         metrics.put("npuUsage", this.getNpuUsage());
+        metrics.put("ipAddress", this.getIpAddress());
 
         DataChangeService.getInstance().publishEvent(OutgoingUIEvent.wrappedOf("metrics", metrics));
     }

photon-core/src/main/java/org/photonvision/common/networking/NetworkInterface.java

@@ -1,78 +0,0 @@
-/*
- * Copyright (C) Photon Vision.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <https://www.gnu.org/licenses/>.
- */
-
-package org.photonvision.common.networking;
-
-import java.net.InterfaceAddress;
-import org.photonvision.common.logging.LogGroup;
-import org.photonvision.common.logging.Logger;
-
-@SuppressWarnings("WeakerAccess")
-public class NetworkInterface {
-    private static final Logger logger = new Logger(NetworkInterface.class, LogGroup.General);
-
-    public final String name;
-    public final String displayName;
-    public final String ipAddress;
-    public final String netmask;
-    public final String broadcast;
-
-    public NetworkInterface(java.net.NetworkInterface inetface, InterfaceAddress ifaceAddress) {
-        name = inetface.getName();
-        displayName = inetface.getDisplayName();
-
-        var inetAddress = ifaceAddress.getAddress();
-        ipAddress = inetAddress.getHostAddress();
-        netmask = getIPv4LocalNetMask(ifaceAddress);
-
-        // TODO: (low) hack to "get" gateway, this is gross and bad, pls fix
-        var splitIPAddr = ipAddress.split("\\.");
-        splitIPAddr[3] = "1";
-        splitIPAddr[3] = "255";
-        broadcast = String.join(".", splitIPAddr);
-    }
-
-    private static String getIPv4LocalNetMask(InterfaceAddress interfaceAddress) {
-        var netPrefix = interfaceAddress.getNetworkPrefixLength();
-        try {
-            // Since this is for IPv4, it's 32 bits, so set the sign value of
-            // the int to "negative"...
-            int shiftby = (1 << 31);
-            // For the number of bits of the prefix -1 (we already set the sign bit)
-            for (int i = netPrefix - 1; i > 0; i--) {
-                // Shift the sign right... Java makes the sign bit sticky on a shift...
-                // So no need to "set it back up"...
-                shiftby = (shiftby >> 1);
-            }
-            // Transform the resulting value in xxx.xxx.xxx.xxx format, like if
-            /// it was a standard address...
-            // Return the address thus created...
-            return ((shiftby >> 24) & 255)
-                    + "."
-                    + ((shiftby >> 16) & 255)
-                    + "."
-                    + ((shiftby >> 8) & 255)
-                    + "."
-                    + (shiftby & 255);
-            //            return InetAddress.getByName(maskString);
-        } catch (Exception e) {
-            logger.error("Failed to get netmask!", e);
-        }
-        // Something went wrong here...
-        return null;
-    }
-}

photon-core/src/main/java/org/photonvision/common/networking/NetworkManager.java

@@ -17,11 +17,12 @@
 
 package org.photonvision.common.networking;
 
+import java.net.NetworkInterface;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.HashMap;
 import java.util.NoSuchElementException;
-
 import org.photonvision.common.configuration.ConfigManager;
 import org.photonvision.common.configuration.NetworkConfig;
 import org.photonvision.common.dataflow.DataChangeDestination;
@@ -38,6 +39,7 @@ import org.photonvision.common.util.TimedTaskManager;
 
 public class NetworkManager {
     private static final Logger logger = new Logger(NetworkManager.class, LogGroup.General);
+    private HashMap<String, String> activeConnections = new HashMap<String, String>();
 
     private NetworkManager() {}
 
@@ -61,17 +63,21 @@ public class NetworkManager {
 
         if (!Platform.isLinux()) {
             logger.info("Not managing network on non-Linux platforms.");
+            this.networkingIsDisabled = true;
             return;
         }
 
         if (!PlatformUtils.isRoot()) {
             logger.error("Cannot manage network without root!");
+            this.networkingIsDisabled = true;
             return;
         }
 
         // Start tasks to monitor the network interface(s)
         var ethernetDevices = NetworkUtils.getAllWiredInterfaces();
         for (NMDeviceInfo deviceInfo : ethernetDevices) {
+            activeConnections.put(
+                    deviceInfo.devName, NetworkUtils.getActiveConnection(deviceInfo.devName));
             monitorDevice(deviceInfo.devName, 5000);
         }
 
@@ -81,7 +87,9 @@ public class NetworkManager {
             try {
                 // if the configured interface isn't in the list of available ones, select one that is
                 var iFace = physicalDevices.stream().findFirst().orElseThrow();
-                logger.warn("The configured interface doesn't match any available interface. Applying configuration to " + iFace.devName);
+                logger.warn(
+                        "The configured interface doesn't match any available interface. Applying configuration to "
+                                + iFace.devName);
                 // update NetworkConfig with found interface
                 config.networkManagerIface = iFace.devName;
                 ConfigManager.getInstance().requestSave();
@@ -96,7 +104,13 @@ public class NetworkManager {
             }
         }
 
-        logger.info("Setting " + config.connectionType + " with team " + config.ntServerAddress + " on " + config.networkManagerIface);
+        logger.info(
+                "Setting "
+                        + config.connectionType
+                        + " with team "
+                        + config.ntServerAddress
+                        + " on "
+                        + config.networkManagerIface);
 
         // always set hostname (unless it's blank)
         if (!config.hostname.isBlank()) {
@@ -129,15 +143,14 @@ public class NetworkManager {
             var shell = new ShellExec(true, false);
             shell.executeBashCommand("cat /etc/hostname | tr -d \" \\t\\n\\r\"");
             var oldHostname = shell.getOutput().replace("\n", "");
-            logger.debug("Old host name: >" + oldHostname +"<");
-            logger.debug("New host name: >" + hostname +"<");
+            logger.debug("Old host name: \"" + oldHostname + "\"");
+            logger.debug("New host name: \"" + hostname + "\"");
 
             if (!oldHostname.equals(hostname)) {
                 var setHostnameRetCode =
                         shell.executeBashCommand(
                                 "echo $NEW_HOSTNAME > /etc/hostname".replace("$NEW_HOSTNAME", hostname));
-                setHostnameRetCode =
-                        shell.executeBashCommand("hostnamectl set-hostname " + hostname);
+                setHostnameRetCode = shell.executeBashCommand("hostnamectl set-hostname " + hostname);
 
                 // Add to /etc/hosts
                 var addHostRetCode =
@@ -168,31 +181,23 @@ public class NetworkManager {
     private void setConnectionDHCP(NetworkConfig config) {
         String connName = "dhcp-" + config.networkManagerIface;
 
-        String addDHCPcommand = """
-            nmcli connection add
-            con-name "${connection}"
-            ifname "${interface}"
-            type ethernet
-            autoconnect no
-            ipv4.method auto
-            ipv6.method disabled
-            """;
-        addDHCPcommand = addDHCPcommand.replaceAll("[\\n]", " ");
-
         var shell = new ShellExec();
         try {
             if (NetworkUtils.connDoesNotExist(connName)) {
-                // create connection
-                logger.info("Creating the DHCP connection " + connName );
+                logger.info("Creating DHCP connection " + connName);
                 shell.executeBashCommand(
-                    addDHCPcommand
-                        .replace("${connection}", connName)
-                        .replace("${interface}", config.networkManagerIface)
-                    );
+                        NetworkingCommands.addConnectionCommand
+                                .replace("${connection}", connName)
+                                .replace("${interface}", config.networkManagerIface));
             }
+            logger.info("Updating the DHCP connection " + connName);
+            shell.executeBashCommand(
+                    NetworkingCommands.modDHCPCommand.replace("${connection}", connName));
             // activate it
-            logger.info("Activating the DHCP connection " + connName );
-            shell.executeBashCommand("nmcli connection up \"${connection}\"".replace("${connection}", connName), false);
+            logger.info("Activating DHCP connection " + connName);
+            shell.executeBashCommand(
+                    "nmcli connection up \"${connection}\"".replace("${connection}", connName), false);
+            activeConnections.put(config.networkManagerIface, connName);
         } catch (Exception e) {
             logger.error("Exception while setting DHCP!", e);
         }
@@ -200,20 +205,6 @@ public class NetworkManager {
 
     private void setConnectionStatic(NetworkConfig config) {
         String connName = "static-" + config.networkManagerIface;
-        String addStaticCommand = """
-            nmcli connection add
-            con-name "${connection}"
-            ifname "${interface}"
-            type ethernet
-            autoconnect no
-            ipv4.addresses ${ipaddr}/8
-            ipv4.gateway ${gateway}
-            ipv4.method "manual"
-            ipv6.method "disabled"
-            """;
-        addStaticCommand = addStaticCommand.replaceAll("[\\n]", " ");
-
-        String modStaticCommand = "nmcli connection mod \"${connection}\" ipv4.addresses ${ipaddr}/8 ipv4.gateway ${gateway}";
 
         if (config.staticIp.isBlank()) {
             logger.warn("Got empty static IP?");
@@ -222,34 +213,31 @@ public class NetworkManager {
 
         // guess at the gateway from the staticIp
         String[] parts = config.staticIp.split("\\.");
-        parts[parts.length-1] = "1";
+        parts[parts.length - 1] = "1";
         String gateway = String.join(".", parts);
 
         var shell = new ShellExec();
         try {
             if (NetworkUtils.connDoesNotExist(connName)) {
                 // create connection
-                logger.info("Creating the Static connection " + connName );
+                logger.info("Creating Static connection " + connName);
                 shell.executeBashCommand(
-                    addStaticCommand
-                        .replace("${connection}", connName)
-                        .replace("${interface}", config.networkManagerIface)
-                        .replace("${ipaddr}", config.staticIp)
-                        .replace("${gateway}", gateway)
-                    );
-            } else {
-                // modify it in case the static IP address is different
-                logger.info("Modifying the Static connection " + connName );
-                shell.executeBashCommand(
-                    modStaticCommand
-                        .replace("${connection}", connName)
-                        .replace("${ipaddr}", config.staticIp)
-                        .replace("${gateway}", gateway)
-                    );
+                        NetworkingCommands.addConnectionCommand
+                                .replace("${connection}", connName)
+                                .replace("${interface}", config.networkManagerIface));
             }
+            // modify it in case the static IP address is different
+            logger.info("Updating the Static connection " + connName);
+            shell.executeBashCommand(
+                    NetworkingCommands.modStaticCommand
+                            .replace("${connection}", connName)
+                            .replace("${ipaddr}", config.staticIp)
+                            .replace("${gateway}", gateway));
             // activate it
-            logger.info("Activating the Static connection " + connName );
-            shell.executeBashCommand("nmcli connection up \"${connection}\"".replace("${connection}", connName), false);
+            logger.info("Activating the Static connection " + connName);
+            shell.executeBashCommand(
+                    "nmcli connection up \"${connection}\"".replace("${connection}", connName), false);
+            activeConnections.put(config.networkManagerIface, connName);
         } catch (Exception e) {
             logger.error("Error while setting static IP!", e);
         }
@@ -267,31 +255,60 @@ public class NetworkManager {
             logger.error("Can't find " + path + ", so can't monitor " + devName);
             return;
         }
-        logger.debug("Watching network interface at path: " + path);
-        var last = new Object() {boolean carrier = true; boolean exceptionLogged = false;};
-        Runnable task = () -> {
-            try {
-                boolean carrier = Files.readString(path).trim().equals("1");
-                if (carrier != last.carrier) {
-                    if (carrier) {
-                        // carrier came back
-                        logger.info("Interface " + devName + " has re-connected, reinitializing");
-                        reinitialize();
-                    } else {
-                        logger.warn("Interface " + devName + " is disconnected, check Ethernet!");
+        var last =
+                new Object() {
+                    boolean carrier = true;
+                    boolean exceptionLogged = false;
+                    String addresses = "";
+                };
+        Runnable task =
+                () -> {
+                    try {
+                        boolean carrier = Files.readString(path).trim().equals("1");
+                        if (carrier != last.carrier) {
+                            if (carrier) {
+                                // carrier came back
+                                logger.info("Interface " + devName + " has re-connected, reinitializing");
+                                reinitialize();
+                            } else {
+                                logger.warn("Interface " + devName + " is disconnected, check Ethernet!");
+                            }
+                        }
+                        NetworkInterface iFace;
+                        iFace = NetworkInterface.getByName(devName);
+                        if (iFace.isUp()) {
+                            String tmpAddresses = "";
+                            tmpAddresses = iFace.getInterfaceAddresses().toString();
+                            if (!last.addresses.equals(tmpAddresses)) {
+                                // addresses have changed, log the difference
+                                last.addresses = tmpAddresses;
+                                logger.info("Interface " + devName + " has address(es): " + last.addresses);
+                            }
+                            var conn = NetworkUtils.getActiveConnection(devName);
+                            if (!conn.equals(activeConnections.get(devName))) {
+                                logger.warn(
+                                        "Unexpected connection "
+                                                + conn
+                                                + " active on "
+                                                + devName
+                                                + ". Expected "
+                                                + activeConnections.get(devName));
+                                logger.info("Reinitializing");
+                                reinitialize();
+                            }
+                        }
+                        last.carrier = carrier;
+                        last.exceptionLogged = false;
+                    } catch (Exception e) {
+                        if (!last.exceptionLogged) {
+                            // Log the exception only once, but keep trying
+                            logger.error("Could not check network status for " + devName, e);
+                            last.exceptionLogged = true;
+                        }
                     }
-                }
-                last.carrier = carrier;
-                last.exceptionLogged = false;
-                } catch (Exception e) {
-                    if (!last.exceptionLogged) {
-                        // Log the exception only once, but keep trying
-                        logger.error("Could not check network status for " + devName, e);
-                        last.exceptionLogged = true;
-                    }
-                }
-            };
+                };
 
         TimedTaskManager.getInstance().addTask(taskName, task, millisInterval);
+        logger.debug("Watching network interface at path: " + path);
     }
 }

photon-core/src/main/java/org/photonvision/common/networking/NetworkUtils.java

@@ -18,6 +18,7 @@
 package org.photonvision.common.networking;
 
 import java.io.IOException;
+import java.net.NetworkInterface;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.regex.Matcher;
@@ -156,15 +157,50 @@ public class NetworkUtils {
         return null;
     }
 
+    public static String getActiveConnection(String devName) {
+        var shell = new ShellExec(true, true);
+        try {
+            shell.executeBashCommand(
+                    "nmcli -g GENERAL.CONNECTION dev show \"" + devName + "\"", true, false);
+            return shell.getOutput().strip();
+        } catch (Exception e) {
+            logger.error("Exception from nmcli!");
+        }
+        return "";
+    }
+
     public static boolean connDoesNotExist(String connName) {
         var shell = new ShellExec(true, true);
         try {
-            // set nmcli back to DHCP, and re-run dhclient -- this ought to grab a new IP address
-            shell.executeBashCommand("nmcli -f GENERAL.STATE connection show \"" + connName + "\"");
+            shell.executeBashCommand(
+                    "nmcli -g GENERAL.STATE connection show \"" + connName + "\"", true, false);
             return (shell.getExitCode() == 10);
         } catch (Exception e) {
             logger.error("Exception from nmcli!");
         }
         return false;
     }
+
+    public static String getIPAddresses(String iFaceName) {
+        if (iFaceName == null || iFaceName.isBlank()) {
+            return "";
+        }
+        List<String> addresses = new ArrayList<String>();
+        try {
+            var iFace = NetworkInterface.getByName(iFaceName);
+            for (var addr : iFace.getInterfaceAddresses()) {
+                var addrStr = addr.getAddress().toString();
+                if (addrStr.startsWith("/")) {
+                    addrStr = addrStr.substring(1);
+                }
+                addrStr = addrStr + "/" + addr.getNetworkPrefixLength();
+                addresses.add(addrStr);
+            }
+            // addresses = iFace.inetAddresses().map(a ->
+            // a.getAddress().toString()).collect(Collectors.joining(","));
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+        return String.join(", ", addresses);
+    }
 }

photon-core/src/main/java/org/photonvision/common/networking/NetworkingCommands.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) Photon Vision.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package org.photonvision.common.networking;
+
+// using a separate class because Spotless fails on text blocks
+// spotless:off
+public class NetworkingCommands {
+    public static final String addConnectionCommand = """
+        nmcli connection add
+        con-name "${connection}"
+        ifname "${interface}"
+        type ethernet
+        """.replaceAll("[\\n]", " ");
+
+    public static final String modStaticCommand = """
+        nmcli connection modify ${connection}
+        autoconnect yes
+        ipv4.method manual
+        ipv6.method disabled
+        ipv4.addresses ${ipaddr}/8
+        ipv4.gateway ${gateway}
+        """.replaceAll("[\\n]", " ");
+
+    public static final String modDHCPCommand = """
+        nmcli connection modify "${connection}"
+        autoconnect yes
+        ipv4.method auto
+        ipv6.method disabled
+        """.replaceAll("[\\n]", " ");
+}
+//spotless:on

photon-core/src/main/java/org/photonvision/common/util/ShellExec.java

@@ -40,8 +40,15 @@ public class ShellExec {
         this.readError = readError;
     }
 
+    /**
+     * Execute a bash command. We can handle complex bash commands including multiple executions (; |
+     * and ||), quotes, expansions ($), escapes (\), e.g.: "cd /abc/def; mv ghi 'older ghi '$(whoami)"
+     *
+     * @param command Bash command to execute
+     * @return process exit code
+     */
     public int executeBashCommand(String command) throws IOException {
-        return executeBashCommand(command, true);
+        return executeBashCommand(command, true, true);
     }
 
     /**
@@ -49,10 +56,25 @@ public class ShellExec {
      * and ||), quotes, expansions ($), escapes (\), e.g.: "cd /abc/def; mv ghi 'older ghi '$(whoami)"
      *
      * @param command Bash command to execute
-     * @return true if bash got started, but your command may have failed.
+     * @param wait true if the command should wait for the proccess to complete
+     * @return process exit code
      */
     public int executeBashCommand(String command, boolean wait) throws IOException {
-        logger.debug("Executing \"" + command + "\"");
+        return executeBashCommand(command, true, true);
+    }
+
+    /**
+     * Execute a bash command. We can handle complex bash commands including multiple executions (; |
+     * and ||), quotes, expansions ($), escapes (\), e.g.: "cd /abc/def; mv ghi 'older ghi '$(whoami)"
+     * This runs the commands with the default logging.
+     *
+     * @param command Bash command to execute
+     * @param wait true if the command should wait for the proccess to complete
+     * @param debug true if the command and return value should be logged
+     * @return process exit code
+     */
+    public int executeBashCommand(String command, boolean wait, boolean debug) throws IOException {
+        if (debug) logger.debug("Executing \"" + command + "\"");
 
         boolean success = false;
         Runtime r = Runtime.getRuntime();
@@ -68,7 +90,7 @@ public class ShellExec {
         // Consume streams, older jvm's had a memory leak if streams were not read,
         // some other jvm+OS combinations may block unless streams are consumed.
         int retcode = doProcess(wait, process);
-        logger.debug("Got exit code " + retcode);
+        if (debug) logger.debug("Got exit code " + retcode);
         return retcode;
     }