From 8ff0d93c1f9501d5da94032a2f33d366b6aeab90 Mon Sep 17 00:00:00 2001 From: Craig Schardt Date: Sun, 20 Oct 2024 20:23:50 -0700 Subject: [PATCH] Improve network management (#1478) This PR changes the way that photonvision interacts with nmcli to control networking on the coprocessor. Instead of modifying an existing connection, Photonvision adds new connections for DHCP and Static IP configurations. It then activiates the proper one at startup and any time that the network configuration is changed. It also now uses the interface name and not the connection name and checks that the interface is available before making any changes. If the saved interface is not found, it updates the stored interface name and applies the network settings to the current interface. This should minimize the failure to control the network if the network interface wasn't available when PhotonVision first booted. One other benefit of not altering the default configuration is that, if PhotonVision fails to run for any reason, the device can be accessed using the original networking configuration. The code has been tested on an OrangePi5 and and a Raspberry Pi 4. Addresses: #1261 --- .../stores/settings/GeneralSettingsStore.ts | 2 +- .../common/configuration/NetworkConfig.java | 16 +- .../configuration/PhotonConfiguration.java | 2 +- .../common/networking/NetworkManager.java | 300 ++++++++++++------ .../common/networking/NetworkUtils.java | 41 ++- .../common/util/TimedTaskManager.java | 4 + shared/common.gradle | 4 +- 7 files changed, 258 insertions(+), 111 deletions(-) diff --git a/photon-client/src/stores/settings/GeneralSettingsStore.ts b/photon-client/src/stores/settings/GeneralSettingsStore.ts index 4ea4f06ac2..f5940789fa 100644 --- a/photon-client/src/stores/settings/GeneralSettingsStore.ts +++ b/photon-client/src/stores/settings/GeneralSettingsStore.ts @@ -78,7 +78,7 @@ export const useSettingsStore = defineStore("settings", { return this.general.gpuAcceleration !== undefined; }, networkInterfaceNames(): string[] { - return this.network.networkInterfaceNames.map((i) => i.connName); + return this.network.networkInterfaceNames.map((i) => i.devName); } }, actions: { diff --git a/photon-core/src/main/java/org/photonvision/common/configuration/NetworkConfig.java b/photon-core/src/main/java/org/photonvision/common/configuration/NetworkConfig.java index 97e84aee13..b38cbdb497 100644 --- a/photon-core/src/main/java/org/photonvision/common/configuration/NetworkConfig.java +++ b/photon-core/src/main/java/org/photonvision/common/configuration/NetworkConfig.java @@ -26,7 +26,6 @@ import java.util.Map; import org.photonvision.common.hardware.Platform; import org.photonvision.common.networking.NetworkMode; -import org.photonvision.common.networking.NetworkUtils; import org.photonvision.common.util.file.JacksonUtils; public class NetworkConfig { @@ -50,23 +49,14 @@ public class NetworkConfig { @JsonIgnore public static final String NM_IFACE_STRING = "${interface}"; @JsonIgnore public static final String NM_IP_STRING = "${ipaddr}"; - public String networkManagerIface; + public String networkManagerIface = ""; + // TODO: remove these strings if no longer needed public String setStaticCommand = "nmcli con mod ${interface} ipv4.addresses ${ipaddr}/8 ipv4.method \"manual\" ipv6.method \"disabled\""; public String setDHCPcommand = "nmcli con mod ${interface} ipv4.method \"auto\" ipv6.method \"disabled\""; public NetworkConfig() { - if (Platform.isLinux()) { - // Default to the name of the first Ethernet connection. Otherwise, "Wired connection 1" is a - // reasonable guess - this.networkManagerIface = - NetworkUtils.getAllWiredInterfaces().stream() - .map(it -> it.connName) - .findFirst() - .orElse("Wired connection 1"); - } - // We can (usually) manage networking on Linux devices, and if we can, we should try to. Command // line inhibitions happen at a level above this class setShouldManage(deviceCanManageNetwork()); @@ -112,7 +102,7 @@ public Map toHashMap() { @JsonIgnore public String getPhysicalInterfaceName() { - return NetworkUtils.getNMinfoForConnName(this.networkManagerIface).devName; + return this.networkManagerIface; } @JsonIgnore diff --git a/photon-core/src/main/java/org/photonvision/common/configuration/PhotonConfiguration.java b/photon-core/src/main/java/org/photonvision/common/configuration/PhotonConfiguration.java index 614c00135c..c776a6cb77 100644 --- a/photon-core/src/main/java/org/photonvision/common/configuration/PhotonConfiguration.java +++ b/photon-core/src/main/java/org/photonvision/common/configuration/PhotonConfiguration.java @@ -120,7 +120,7 @@ public Map toHashMap() { // Hack active interfaces into networkSettings var netConfigMap = networkConfig.toHashMap(); - netConfigMap.put("networkInterfaceNames", NetworkUtils.getAllWiredInterfaces()); + netConfigMap.put("networkInterfaceNames", NetworkUtils.getAllActiveWiredInterfaces()); netConfigMap.put("networkingDisabled", NetworkManager.getInstance().networkingIsDisabled); settingsSubmap.put("networkSettings", netConfigMap); diff --git a/photon-core/src/main/java/org/photonvision/common/networking/NetworkManager.java b/photon-core/src/main/java/org/photonvision/common/networking/NetworkManager.java index 47b6540708..a53451b3a6 100644 --- a/photon-core/src/main/java/org/photonvision/common/networking/NetworkManager.java +++ b/photon-core/src/main/java/org/photonvision/common/networking/NetworkManager.java @@ -17,6 +17,10 @@ package org.photonvision.common.networking; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.NoSuchElementException; + import org.photonvision.common.configuration.ConfigManager; import org.photonvision.common.configuration.NetworkConfig; import org.photonvision.common.dataflow.DataChangeDestination; @@ -27,7 +31,9 @@ import org.photonvision.common.hardware.PlatformUtils; import org.photonvision.common.logging.LogGroup; import org.photonvision.common.logging.Logger; +import org.photonvision.common.networking.NetworkUtils.NMDeviceInfo; import org.photonvision.common.util.ShellExec; +import org.photonvision.common.util.TimedTaskManager; public class NetworkManager { private static final Logger logger = new Logger(NetworkManager.class, LogGroup.General); @@ -52,102 +58,60 @@ public void initialize(boolean shouldManage) { return; } - var config = ConfigManager.getInstance().getConfig().getNetworkConfig(); - logger.info("Setting " + config.connectionType + " with team " + config.ntServerAddress); - if (Platform.isLinux()) { - if (!PlatformUtils.isRoot()) { - logger.error("Cannot manage hostname without root!"); - } + if (!Platform.isLinux()) { + logger.info("Not managing network on non-Linux platforms."); + return; + } - // always set hostname - if (!config.hostname.isEmpty()) { - try { - var shell = new ShellExec(true, false); - shell.executeBashCommand("cat /etc/hostname | tr -d \" \\t\\n\\r\""); - var oldHostname = shell.getOutput().replace("\n", ""); - - var setHostnameRetCode = - shell.executeBashCommand( - "echo $NEW_HOSTNAME > /etc/hostname".replace("$NEW_HOSTNAME", config.hostname)); - setHostnameRetCode = - shell.executeBashCommand("hostnamectl set-hostname " + config.hostname); - - // Add to /etc/hosts - var addHostRetCode = - shell.executeBashCommand( - String.format( - "sed -i \"s/127.0.1.1.*%s/127.0.1.1\\t%s/g\" /etc/hosts", - oldHostname, config.hostname)); - - shell.executeBashCommand("sudo service avahi-daemon restart"); - - var success = setHostnameRetCode == 0 && addHostRetCode == 0; - if (!success) { - logger.error( - "Setting hostname returned non-zero codes (hostname/hosts) " - + setHostnameRetCode - + "|" - + addHostRetCode - + "!"); - } else { - logger.info("Set hostname to " + config.hostname); - } - } catch (Exception e) { - logger.error("Failed to set hostname!", e); - } + if (!PlatformUtils.isRoot()) { + logger.error("Cannot manage network without root!"); + return; + } - } else { - logger.warn("Got empty hostname?"); + // Start tasks to monitor the network interface(s) + var ethernetDevices = NetworkUtils.getAllWiredInterfaces(); + for (NMDeviceInfo deviceInfo : ethernetDevices) { + var task = "deviceStatus-" + deviceInfo.devName; + if (!TimedTaskManager.getInstance().taskActive(task)) { + TimedTaskManager.getInstance().addTask(task, deviceStatus(deviceInfo.devName), 5000); } + } - if (config.connectionType == NetworkMode.DHCP) { - var shell = new ShellExec(); - try { - // set nmcli back to DHCP, and re-run dhclient -- this ought to grab a new IP address - shell.executeBashCommand( - config.setDHCPcommand.replace( - NetworkConfig.NM_IFACE_STRING, config.getEscapedInterfaceName())); - shell.executeBashCommand("dhclient " + config.getPhysicalInterfaceName(), false); - } catch (Exception e) { - logger.error("Exception while setting DHCP!"); - } - } else if (config.connectionType == NetworkMode.STATIC) { - var shell = new ShellExec(); - if (!config.staticIp.isEmpty()) { - try { - shell.executeBashCommand( - config - .setStaticCommand - .replace(NetworkConfig.NM_IFACE_STRING, config.getEscapedInterfaceName()) - .replace(NetworkConfig.NM_IP_STRING, config.staticIp)); - - if (Platform.isRaspberryPi()) { - // Pi's need to manually have their interface adjusted?? and the 5-second sleep is - // integral in my testing (Matt) - shell.executeBashCommand( - "sh -c 'nmcli con down " - + config.getEscapedInterfaceName() - + "; nmcli con up " - + config.getEscapedInterfaceName() - + "'"); - } else { - // for now just bring down /up -- more testing needed on beelink et al. - shell.executeBashCommand( - "sh -c 'nmcli con down " - + config.getEscapedInterfaceName() - + "; nmcli con up " - + config.getEscapedInterfaceName() - + "'"); - } - } catch (Exception e) { - logger.error("Error while setting static IP!", e); - } - } else { - logger.warn("Got empty static IP?"); + var physicalDevices = NetworkUtils.getAllActiveWiredInterfaces(); + var config = ConfigManager.getInstance().getConfig().getNetworkConfig(); + if (physicalDevices.stream().noneMatch(it -> (it.devName.equals(config.networkManagerIface)))) { + 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); + // update NetworkConfig with found interface + config.networkManagerIface = iFace.devName; + ConfigManager.getInstance().requestSave(); + } catch (NoSuchElementException e) { + // if there are no available interfaces, go with the one from settings + logger.warn("No physical interface found. Maybe ethernet isn't connected?"); + if (config.networkManagerIface == null || config.networkManagerIface.isBlank()) { + // if it's also empty, there is nothing to configure + logger.error("No valid network interfaces to manage"); + // TODO: add a thread that monitors the network + return; } } + } + + logger.info("Setting " + config.connectionType + " with team " + config.ntServerAddress + " on " + config.networkManagerIface); + + // always set hostname (unless it's blank) + if (!config.hostname.isBlank()) { + setHostname(config.hostname); } else { - logger.info("Not managing network on non-Linux platforms"); + logger.warn("Got empty hostname?"); + } + + if (config.connectionType == NetworkMode.DHCP) { + setConnectionDHCP(config); + } else if (config.connectionType == NetworkMode.STATIC) { + setConnectionStatic(config); } } @@ -162,4 +126,162 @@ public void reinitialize() { "restartServer", true)); } + + private void setHostname(String hostname) { + try { + 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 +"<"); + + if (!oldHostname.equals(hostname)) { + var setHostnameRetCode = + shell.executeBashCommand( + "echo $NEW_HOSTNAME > /etc/hostname".replace("$NEW_HOSTNAME", hostname)); + setHostnameRetCode = + shell.executeBashCommand("hostnamectl set-hostname " + hostname); + + // Add to /etc/hosts + var addHostRetCode = + shell.executeBashCommand( + String.format( + "sed -i \"s/127.0.1.1.*%s/127.0.1.1\\t%s/g\" /etc/hosts", + oldHostname, hostname)); + + shell.executeBashCommand("systemctl restart avahi-daemon.service"); + + var success = setHostnameRetCode == 0 && addHostRetCode == 0; + if (!success) { + logger.error( + "Setting hostname returned non-zero codes (hostname/hosts) " + + setHostnameRetCode + + "|" + + addHostRetCode + + "!"); + } else { + logger.info("Set hostname to " + hostname); + } + } + } catch (Exception e) { + logger.error("Failed to set hostname!", e); + } + } + + 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 { + // set nmcli back to DHCP, and re-run dhclient -- this ought to grab a new IP address + if (NetworkUtils.connDoesNotExist(connName)) { + // create connection + logger.info("Creating the DHCP connection " + connName ); + shell.executeBashCommand( + addDHCPcommand + .replace("${connection}", connName) + .replace("${interface}", config.networkManagerIface) + ); + } + // activate it + logger.info("Activating the DHCP connection " + connName ); + shell.executeBashCommand("nmcli connection up \"${connection}\"".replace("${connection}", connName), false); + } catch (Exception e) { + logger.error("Exception while setting DHCP!", e); + } + } + + 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?"); + return; + } + + // guess at the gateway from the staticIp + String[] parts = config.staticIp.split("\\."); + parts[parts.length-1] = "1"; + String gateway = String.join(".", parts); + + var shell = new ShellExec(); + try { + // set nmcli back to DHCP, and re-run dhclient -- this ought to grab a new IP address + if (NetworkUtils.connDoesNotExist(connName)) { + // create connection + logger.info("Creating the 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) + ); + } + // activate it + logger.info("Activating the Static connection " + connName ); + shell.executeBashCommand("nmcli connection up \"${connection}\"".replace("${connection}", connName), false); + } catch (Exception e) { + logger.error("Error while setting static IP!", e); + } + } + + // Detects changes in the carrier and reinitializes after re-connect + private Runnable deviceStatus(String devName) { + Path file = Path.of("/sys/class/net/{device}/carrier".replace("{device}", devName)); + logger.debug("Watching network interface at path: " + file.toString()); + var last = new Object() {boolean carrier = true;}; + return () -> + { + try { + boolean carrier = Files.readString(file).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!"); + } + } + last.carrier = carrier; + } catch (Exception e) { + logger.error("Could not check network status", e); + } + }; + } } diff --git a/photon-core/src/main/java/org/photonvision/common/networking/NetworkUtils.java b/photon-core/src/main/java/org/photonvision/common/networking/NetworkUtils.java index cf84309e57..0f9a282ec7 100644 --- a/photon-core/src/main/java/org/photonvision/common/networking/NetworkUtils.java +++ b/photon-core/src/main/java/org/photonvision/common/networking/NetworkUtils.java @@ -102,13 +102,16 @@ public static List getAllInterfaces() { Pattern.compile("GENERAL.CONNECTION:(.*)\nGENERAL.DEVICE:(.*)\nGENERAL.TYPE:(.*)"); Matcher matcher = pattern.matcher(out); while (matcher.find()) { - ret.add(new NMDeviceInfo(matcher.group(1), matcher.group(2), matcher.group(3))); + if (!matcher.group(2).equals("lo")) { + // only include non-loopback devices + ret.add(new NMDeviceInfo(matcher.group(1), matcher.group(2), matcher.group(3))); + } } } catch (IOException e) { - logger.error("Could not get active NM ifaces!", e); + logger.error("Could not get active network interfaces!", e); } - logger.debug("Found network interfaces:\n" + ret); + logger.debug("Found network interfaces: " + ret); allInterfaces = ret; return ret; @@ -123,8 +126,14 @@ public static List getAllActiveInterfaces() { } public static List getAllWiredInterfaces() { - return getAllActiveInterfaces().stream() - .filter(it -> it.nmType == NMType.NMTYPE_ETHERNET) + return getAllInterfaces().stream() + .filter(it -> it.nmType.equals(NMType.NMTYPE_ETHERNET)) + .collect(Collectors.toList()); + } + + public static List getAllActiveWiredInterfaces() { + return getAllWiredInterfaces().stream() + .filter(it -> !it.connName.isBlank()) .collect(Collectors.toList()); } @@ -136,4 +145,26 @@ public static NMDeviceInfo getNMinfoForConnName(String connName) { } return null; } + + public static NMDeviceInfo getNMinfoForDevName(String devName) { + for (NMDeviceInfo info : getAllActiveInterfaces()) { + if (info.devName.equals(devName)) { + return info; + } + } + logger.warn("Could not find a match for network device " + devName); + return null; + } + + 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 + "\""); + return (shell.getExitCode() == 10); + } catch (Exception e) { + logger.error("Exception from nmcli!"); + } + return false; + } } diff --git a/photon-core/src/main/java/org/photonvision/common/util/TimedTaskManager.java b/photon-core/src/main/java/org/photonvision/common/util/TimedTaskManager.java index 7107faae32..6916702391 100644 --- a/photon-core/src/main/java/org/photonvision/common/util/TimedTaskManager.java +++ b/photon-core/src/main/java/org/photonvision/common/util/TimedTaskManager.java @@ -79,4 +79,8 @@ public void cancelTask(String identifier) { activeTasks.remove(identifier); } } + + public boolean taskActive(String identifier) { + return activeTasks.containsKey(identifier); + } } diff --git a/shared/common.gradle b/shared/common.gradle index 33d1a4c8b2..a8fb28b164 100644 --- a/shared/common.gradle +++ b/shared/common.gradle @@ -3,8 +3,8 @@ apply plugin: "java" apply plugin: "jacoco" java { - sourceCompatibility = JavaVersion.VERSION_11 - targetCompatibility = JavaVersion.VERSION_11 + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 } wpilibTools.deps.wpilibVersion = wpilibVersion