From 3786caf568a34083f6cefcb521caa45d321c57a0 Mon Sep 17 00:00:00 2001 From: Franco Fichtner Date: Tue, 5 Sep 2023 19:35:11 +0200 Subject: [PATCH] system: do not mark defunct gw as disabled as well gatewaysIndexedByName() -> do not omit when defunct getDefaultGW() -> omit when defunct getInterfaceGateway() -> omit when defunct (debatable) Otherwise strip gatewaysIndexedByName(true) where the behaviour was likely to try and get defunct gateways as well and fix the dpinger code accordingly to get rid of raw config access. --- src/etc/inc/plugins.inc.d/dhcpd.inc | 4 +- src/etc/inc/plugins.inc.d/dpinger.inc | 52 ++++++------------- src/etc/inc/system.inc | 4 +- .../app/library/OPNsense/Routing/Gateways.php | 5 +- .../scripts/routes/gateway_status.php | 7 ++- 5 files changed, 24 insertions(+), 48 deletions(-) diff --git a/src/etc/inc/plugins.inc.d/dhcpd.inc b/src/etc/inc/plugins.inc.d/dhcpd.inc index cea989d43..68cbda55f 100644 --- a/src/etc/inc/plugins.inc.d/dhcpd.inc +++ b/src/etc/inc/plugins.inc.d/dhcpd.inc @@ -1654,7 +1654,7 @@ function dhcpd_dhcrelay4_configure($verbose = false) $iflist = get_configured_interface_with_descr(); $ifconfig_details = legacy_interfaces_details(); - $a_gateways = (new \OPNsense\Routing\Gateways())->gatewaysIndexedByName(true); + $a_gateways = (new \OPNsense\Routing\Gateways())->gatewaysIndexedByName(); $dhcifaces = explode(",", $dhcrelaycfg['interface']); foreach ($dhcifaces as $dhcrelayif) { if (isset($iflist[$dhcrelayif]) && get_interface_ip($dhcrelayif, $ifconfig_details)) { @@ -1766,7 +1766,7 @@ function dhcpd_dhcrelay6_configure($verbose = false) $iflist = get_configured_interface_with_descr(); $ifconfig_details = legacy_interfaces_details(); - $a_gateways = (new \OPNsense\Routing\Gateways())->gatewaysIndexedByName(true); + $a_gateways = (new \OPNsense\Routing\Gateways())->gatewaysIndexedByName(); $dhcifaces = explode(",", $dhcrelaycfg['interface']); foreach ($dhcifaces as $dhcrelayif) { if (isset($iflist[$dhcrelayif]) && get_interface_ipv6($dhcrelayif, $ifconfig_details)) { diff --git a/src/etc/inc/plugins.inc.d/dpinger.inc b/src/etc/inc/plugins.inc.d/dpinger.inc index 3c6a0cf31..3e2c1e950 100644 --- a/src/etc/inc/plugins.inc.d/dpinger.inc +++ b/src/etc/inc/plugins.inc.d/dpinger.inc @@ -33,19 +33,9 @@ function dpinger_services() { - global $config; - $services = []; - foreach (config_read_array('gateways', 'gateway_item') as $gateway) { - if (!empty($gateway['monitor_disable']) || isset($gateway['disabled'])) { - continue; - } - - if (empty($config['interfaces'][$gateway['interface']]['enable'])) { - continue; - } - + foreach (dpinger_instances() as $name => $gateway) { $pconfig = []; $pconfig['description'] = sprintf(gettext('Gateway monitor (%s)'), $gateway['name']); $pconfig['php']['restart'] = ['dpinger_configure_do']; @@ -129,17 +119,12 @@ function dpinger_host_routes() return $routes; } -function dpinger_instances() +function dpinger_instances($extended = false) { $instances = []; - $gateways = new \OPNsense\Routing\Gateways(); - - foreach ($gateways->gatewaysIndexedByName(true) as $name => $gateway) { - if (empty($gateway['monitor'])) { - /* skip monitors not currently attached */ - continue; - } elseif (isset($gateway['disabled']) || isset($gateway['monitor_disable'])) { + foreach ((new \OPNsense\Routing\Gateways())->gatewaysIndexedByName() as $name => $gateway) { + if (isset($gateway['monitor_disable']) && !$extended) { /* do not monitor if such was requested */ continue; } @@ -151,12 +136,8 @@ function dpinger_instances() if (is_linklocal($gateway['monitor']) && strpos($gateway['monitor'], '%') === false) { $gateway['monitor'] .= '%' . get_real_interface($gateway['interface'], 'inet6'); } - if (!empty($gateway['gateway'])) { - if (is_linklocal($gateway['gateway']) && strpos($gateway['gateway'], '%') === false) { - $gateway['gateway'] .= '%' . get_real_interface($gateway['interface'], 'inet6'); - } - } else { - log_msg("Gateway currently empty for {$gateway['monitor']} on {$gateway['interface']}"); + if (is_linklocal($gateway['gateway']) && strpos($gateway['gateway'], '%') === false) { + $gateway['gateway'] .= '%' . get_real_interface($gateway['interface'], 'inet6'); } $instances[$name] = $gateway; @@ -190,6 +171,10 @@ function dpinger_configure_do($verbose = false, $gwname = null, $bootup = false) continue; } + if (empty($gateway['gateway'])) { + log_msg("Gateway currently empty for {$gateway['monitor']} on {$gateway['interface']}"); + } + $gwifip = null; /* @@ -346,21 +331,16 @@ function dpinger_run() function dpinger_status() { - $instances = dpinger_instances(); $status = []; - foreach (config_read_array('gateways', 'gateway_item') as $gwitem) { - if (isset($gwitem['disabled'])) { - continue; - } - + foreach (dpinger_instances(true) as $gwitem) { /* we seem to be concerned about disabled monitors just because of force_down */ $gwstatus = isset($gwitem['monitor_disable']) ? 'none' : 'down'; $status[$gwitem['name']] = [ /* grab the runtime monitor from the instance if available */ - 'monitor' => $instances[$gwitem['name']]['monitor'] ?? ($gwitem['monitor'] ?? '~'), 'status' => isset($gwitem['force_down']) ? 'force_down' : $gwstatus, + 'monitor' => $gwitem['monitor'] ?? '~', 'name' => $gwitem['name'], 'stddev' => '~', 'delay' => '~', @@ -398,12 +378,10 @@ function dpinger_status() $settings = dpinger_defaults(); if ($r['status'] != 'force_down') { - $keys = ['latencylow', 'latencyhigh', 'losslow', 'losshigh']; - /* Replace default values by user-defined */ - foreach ($keys as $key) { - if (isset($instances[$gwname][$key]) && is_numeric($instances[$gwname][$key])) { - $settings[$key] = $instances[$gwname][$key]; + foreach (['latencylow', 'latencyhigh', 'losslow', 'losshigh'] as $key) { + if (isset($gwitem[$key]) && is_numeric($gwitem[$key])) { + $settings[$key] = $gwitem[$key]; } } diff --git a/src/etc/inc/system.inc b/src/etc/inc/system.inc index 735682dc8..584f6dcd2 100644 --- a/src/etc/inc/system.inc +++ b/src/etc/inc/system.inc @@ -660,7 +660,7 @@ function system_routing_configure($verbose = false, $interface = null, $monitor $down_gateways = isset($config['system']['gw_switch_default']) ? return_down_gateways() : []; $routes = json_decode(configd_run('interface routes list -n json'), true) ?? []; - foreach ($gateways->gatewaysIndexedByName(true) as $gateway) { + foreach ($gateways->gatewaysIndexedByName() as $gateway) { /* check if we need to add a required interface route to the gateway (IPv4 only) */ if ($gateway['ipprotocol'] !== 'inet' || ($family !== null && $family !== 'inet')) { continue; @@ -766,7 +766,7 @@ function system_routing_configure($verbose = false, $interface = null, $monitor if (!empty($interface)) { $reloads = []; - foreach ($gateways->gatewaysIndexedByName(true) as $name => $gateway) { + foreach ($gateways->gatewaysIndexedByName() as $name => $gateway) { if ($family !== null && $family !== $gateway['ipprotocol']) { continue; } diff --git a/src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php b/src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php index 1118d64d3..035112ad7 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php +++ b/src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php @@ -355,7 +355,6 @@ class Gateways foreach ($dynamic_gw as $intfgws) { foreach ($intfgws as $gw_arr) { if (!empty($gw_arr)) { - $gw_arr['disabled'] = true; $gw_arr['defunct'] = true; unset($gw_arr['gateway']); $this->cached_gateways[] = $gw_arr; @@ -379,7 +378,7 @@ class Gateways if ($gateway['ipprotocol'] == $ipproto) { if (is_array($skip) && in_array($gateway['name'], $skip)) { continue; - } elseif (!empty($gateway['disabled']) || !empty($gateway['is_loopback']) || !empty($gateway['force_down'])) { + } elseif (!empty($gateway['disabled']) || !empty($gateway['defunct']) || !empty($gateway['is_loopback']) || !empty($gateway['force_down'])) { continue; } else { return $gateway; @@ -472,7 +471,7 @@ class Gateways public function getInterfaceGateway($interface, $ipproto = "inet", $only_configured = false, $property = 'gateway') { foreach ($this->getGateways() as $gateway) { - if (!empty($gateway['disabled']) || $gateway['ipprotocol'] != $ipproto) { + if (!empty($gateway['disabled']) || !empty($gateway['defunct']) || $gateway['ipprotocol'] != $ipproto) { continue; } elseif (!empty($gateway['is_loopback']) || empty($gateway['gateway'])) { continue; diff --git a/src/opnsense/scripts/routes/gateway_status.php b/src/opnsense/scripts/routes/gateway_status.php index 58d5d3d32..5f3d1b97a 100755 --- a/src/opnsense/scripts/routes/gateway_status.php +++ b/src/opnsense/scripts/routes/gateway_status.php @@ -34,7 +34,7 @@ require_once 'interfaces.inc'; $result = []; $gateways_status = return_gateways_status(); -foreach ((new \OPNsense\Routing\Gateways())->gatewaysIndexedByName(true) as $gname => $gw) { +foreach ((new \OPNsense\Routing\Gateways())->gatewaysIndexedByName() as $gname => $gw) { $gatewayItem = ['name' => $gname]; $gatewayItem['address'] = !empty($gw['gateway']) ? $gw['gateway'] : '~'; if (!empty($gateways_status[$gname])) { @@ -65,9 +65,6 @@ foreach ((new \OPNsense\Routing\Gateways())->gatewaysIndexedByName(true) as $gna $gatewayItem['status_translated'] = gettext('Pending'); break; } - } elseif (isset($gw['disabled'])) { - /* avoid disappearing an actively monitored instance when down */ - continue; } else { $gatewayItem['status'] = 'none'; $gatewayItem['status_translated'] = gettext('Online'); @@ -75,6 +72,8 @@ foreach ((new \OPNsense\Routing\Gateways())->gatewaysIndexedByName(true) as $gna $gatewayItem['stddev'] = '~'; $gatewayItem['delay'] = '~'; } + $result[] = $gatewayItem; } + echo json_encode($result) . PHP_EOL;