From b9f3646a2efb1c0f3a39f3daee75a642101a06c3 Mon Sep 17 00:00:00 2001 From: Tony Murray Date: Fri, 11 Feb 2022 08:37:19 -0600 Subject: [PATCH] STP fix, only try contexts on Cisco (#13767) * STP fix, only try contexts on Cisco Only Cisco is known to support this type of polling * merge, not push. * Fix collection construction * Fixed the function signature in Interface def * fixed signature here as well * Lighter code to return a collection of Stp instances * Type enforced in signatures * missing implement for Cisco Class * style * Collection methods are immutable indeed :angry: * nullable vlan Co-authored-by: PipoCanaja <38363551+PipoCanaja@users.noreply.github.com> --- .../Discovery/StpInstanceDiscovery.php | 3 +- LibreNMS/OS/Shared/Cisco.php | 25 ++++- LibreNMS/OS/Traits/BridgeMib.php | 97 +++++++++---------- 3 files changed, 71 insertions(+), 54 deletions(-) diff --git a/LibreNMS/Interfaces/Discovery/StpInstanceDiscovery.php b/LibreNMS/Interfaces/Discovery/StpInstanceDiscovery.php index a8e4dcaa60..3826bb7481 100644 --- a/LibreNMS/Interfaces/Discovery/StpInstanceDiscovery.php +++ b/LibreNMS/Interfaces/Discovery/StpInstanceDiscovery.php @@ -32,7 +32,8 @@ interface StpInstanceDiscovery /** * Discover STP instances on the device. * + * @param string $vlan Vlan ID of the instance to discover (or null for default) * @return Collection<\App\Models\Stp> */ - public function discoverStpInstances(): Collection; + public function discoverStpInstances(?string $vlan = null): Collection; } diff --git a/LibreNMS/OS/Shared/Cisco.php b/LibreNMS/OS/Shared/Cisco.php index 09bb570275..20bf37c7fc 100755 --- a/LibreNMS/OS/Shared/Cisco.php +++ b/LibreNMS/OS/Shared/Cisco.php @@ -31,11 +31,13 @@ use App\Models\Mempool; use App\Models\PortsNac; use App\Models\Sla; use Illuminate\Support\Arr; +use Illuminate\Support\Collection; use LibreNMS\Device\Processor; use LibreNMS\Interfaces\Discovery\MempoolsDiscovery; use LibreNMS\Interfaces\Discovery\OSDiscovery; use LibreNMS\Interfaces\Discovery\ProcessorDiscovery; use LibreNMS\Interfaces\Discovery\SlaDiscovery; +use LibreNMS\Interfaces\Discovery\StpInstanceDiscovery; use LibreNMS\Interfaces\Polling\NacPolling; use LibreNMS\Interfaces\Polling\SlaPolling; use LibreNMS\OS; @@ -43,7 +45,14 @@ use LibreNMS\OS\Traits\YamlOSDiscovery; use LibreNMS\RRD\RrdDefinition; use LibreNMS\Util\IP; -class Cisco extends OS implements OSDiscovery, SlaDiscovery, ProcessorDiscovery, MempoolsDiscovery, NacPolling, SlaPolling +class Cisco extends OS implements + OSDiscovery, + SlaDiscovery, + StpInstanceDiscovery, + ProcessorDiscovery, + MempoolsDiscovery, + NacPolling, + SlaPolling { use YamlOSDiscovery { YamlOSDiscovery::discoverOS as discoverYamlOS; @@ -520,6 +529,20 @@ class Cisco extends OS implements OSDiscovery, SlaDiscovery, ProcessorDiscovery, } } + public function discoverStpInstances(?string $vlan = null): Collection + { + $vlans = $this->getDevice()->vlans; + $instances = new Collection; + + // attempt to discover context based vlan instances + foreach ($vlans->isEmpty() ? [null] : $vlans as $vlan) { + $vlan = (empty($vlan->vlan_vlan) || $vlan->vlan_vlan == '1') ? null : (string) $vlan->vlan_vlan; + $instances = $instances->merge(parent::discoverStpInstances($vlan)); + } + + return $instances; + } + protected function getMainSerial() { $serial_output = snmp_get_multi($this->getDeviceArray(), ['entPhysicalSerialNum.1', 'entPhysicalSerialNum.1001'], '-OQUs', 'ENTITY-MIB:OLD-CISCO-CHASSIS-MIB'); diff --git a/LibreNMS/OS/Traits/BridgeMib.php b/LibreNMS/OS/Traits/BridgeMib.php index 0cf0a515b6..f067f60412 100644 --- a/LibreNMS/OS/Traits/BridgeMib.php +++ b/LibreNMS/OS/Traits/BridgeMib.php @@ -33,7 +33,7 @@ use SnmpQuery; trait BridgeMib { - public function discoverStpInstances(): Collection + public function discoverStpInstances(?string $vlan = null): Collection { $protocol = SnmpQuery::get('BRIDGE-MIB::dot1dStpProtocolSpecification.0')->value(); // 1 = unknown (mstp?), 3 = ieee8021d @@ -44,60 +44,53 @@ trait BridgeMib $timeFactor = $this->stpTimeFactor ?? 0.01; - $vlans = $protocol == 1 ? $this->getDevice()->vlans : new Collection; - $instances = new Collection; + // fetch STP config and store it + $stp = SnmpQuery::context("$vlan", 'vlan-')->enumStrings()->get([ + 'BRIDGE-MIB::dot1dBaseBridgeAddress.0', + 'BRIDGE-MIB::dot1dStpProtocolSpecification.0', + 'BRIDGE-MIB::dot1dStpPriority.0', + 'BRIDGE-MIB::dot1dStpTimeSinceTopologyChange.0', + 'BRIDGE-MIB::dot1dStpTopChanges.0', + 'BRIDGE-MIB::dot1dStpDesignatedRoot.0', + 'BRIDGE-MIB::dot1dStpRootCost.0', + 'BRIDGE-MIB::dot1dStpRootPort.0', + 'BRIDGE-MIB::dot1dStpMaxAge.0', + 'BRIDGE-MIB::dot1dStpHelloTime.0', + 'BRIDGE-MIB::dot1dStpHoldTime.0', + 'BRIDGE-MIB::dot1dStpForwardDelay.0', + 'BRIDGE-MIB::dot1dStpBridgeMaxAge.0', + 'BRIDGE-MIB::dot1dStpBridgeHelloTime.0', + 'BRIDGE-MIB::dot1dStpBridgeForwardDelay.0', + ])->values(); - foreach ($vlans->isEmpty() ? [null] : $vlans as $vlan) { - // fetch STP config and store it - $vlan = (empty($vlan->vlan_vlan) || $vlan->vlan_vlan == '1') ? null : $vlan->vlan_vlan; - $instance = SnmpQuery::context("$vlan", 'vlan-')->enumStrings()->get([ - 'BRIDGE-MIB::dot1dBaseBridgeAddress.0', - 'BRIDGE-MIB::dot1dStpProtocolSpecification.0', - 'BRIDGE-MIB::dot1dStpPriority.0', - 'BRIDGE-MIB::dot1dStpTimeSinceTopologyChange.0', - 'BRIDGE-MIB::dot1dStpTopChanges.0', - 'BRIDGE-MIB::dot1dStpDesignatedRoot.0', - 'BRIDGE-MIB::dot1dStpRootCost.0', - 'BRIDGE-MIB::dot1dStpRootPort.0', - 'BRIDGE-MIB::dot1dStpMaxAge.0', - 'BRIDGE-MIB::dot1dStpHelloTime.0', - 'BRIDGE-MIB::dot1dStpHoldTime.0', - 'BRIDGE-MIB::dot1dStpForwardDelay.0', - 'BRIDGE-MIB::dot1dStpBridgeMaxAge.0', - 'BRIDGE-MIB::dot1dStpBridgeHelloTime.0', - 'BRIDGE-MIB::dot1dStpBridgeForwardDelay.0', - ]); - - $stp = $instance->values(); - if (empty($stp)) { - continue; - } - - \Log::debug('VLAN: ' . ($vlan ?: 1) . " Bridge: {$stp['BRIDGE-MIB::dot1dBaseBridgeAddress.0']} DR: {$stp['BRIDGE-MIB::dot1dStpDesignatedRoot.0']}"); - $bridge = Rewrite::macToHex($stp['BRIDGE-MIB::dot1dBaseBridgeAddress.0'] ?? ''); - $drBridge = Rewrite::macToHex($stp['BRIDGE-MIB::dot1dStpDesignatedRoot.0'] ?? ''); - $instances->push(new \App\Models\Stp([ - 'vlan' => $vlan, - 'rootBridge' => $bridge == $drBridge ? 1 : 0, - 'bridgeAddress' => $bridge, - 'protocolSpecification' => $stp['BRIDGE-MIB::dot1dStpProtocolSpecification.0'] ?? 'unknown', - 'priority' => $stp['BRIDGE-MIB::dot1dStpPriority.0'] ?? 0, - 'timeSinceTopologyChange' => substr($stp['BRIDGE-MIB::dot1dStpTimeSinceTopologyChange.0'] ?? '', 0, -2) ?: 0, - 'topChanges' => $stp['BRIDGE-MIB::dot1dStpTopChanges.0'] ?? 0, - 'designatedRoot' => $drBridge, - 'rootCost' => $stp['BRIDGE-MIB::dot1dStpRootCost.0'] ?? 0, - 'rootPort' => $stp['BRIDGE-MIB::dot1dStpRootPort.0'] ?? 0, - 'maxAge' => ($stp['BRIDGE-MIB::dot1dStpMaxAge.0'] ?? 0) * $timeFactor, - 'helloTime' => ($stp['BRIDGE-MIB::dot1dStpHelloTime.0'] ?? 0) * $timeFactor, - 'holdTime' => ($stp['BRIDGE-MIB::dot1dStpHoldTime.0'] ?? 0) * $timeFactor, - 'forwardDelay' => ($stp['BRIDGE-MIB::dot1dStpForwardDelay.0'] ?? 0) * $timeFactor, - 'bridgeMaxAge' => ($stp['BRIDGE-MIB::dot1dStpBridgeMaxAge.0'] ?? 0) * $timeFactor, - 'bridgeHelloTime' => ($stp['BRIDGE-MIB::dot1dStpBridgeHelloTime.0'] ?? 0) * $timeFactor, - 'bridgeForwardDelay' => ($stp['BRIDGE-MIB::dot1dStpBridgeForwardDelay.0'] ?? 0) * $timeFactor, - ])); + if (empty($stp)) { + return new Collection; } - return $instances; + \Log::debug('VLAN: ' . ($vlan ?: 1) . " Bridge: {$stp['BRIDGE-MIB::dot1dBaseBridgeAddress.0']} DR: {$stp['BRIDGE-MIB::dot1dStpDesignatedRoot.0']}"); + $bridge = Rewrite::macToHex($stp['BRIDGE-MIB::dot1dBaseBridgeAddress.0'] ?? ''); + $drBridge = Rewrite::macToHex($stp['BRIDGE-MIB::dot1dStpDesignatedRoot.0'] ?? ''); + $instance = new \App\Models\Stp([ + 'vlan' => $vlan, + 'rootBridge' => $bridge == $drBridge ? 1 : 0, + 'bridgeAddress' => $bridge, + 'protocolSpecification' => $stp['BRIDGE-MIB::dot1dStpProtocolSpecification.0'] ?? 'unknown', + 'priority' => $stp['BRIDGE-MIB::dot1dStpPriority.0'] ?? 0, + 'timeSinceTopologyChange' => substr($stp['BRIDGE-MIB::dot1dStpTimeSinceTopologyChange.0'] ?? '', 0, -2) ?: 0, + 'topChanges' => $stp['BRIDGE-MIB::dot1dStpTopChanges.0'] ?? 0, + 'designatedRoot' => $drBridge, + 'rootCost' => $stp['BRIDGE-MIB::dot1dStpRootCost.0'] ?? 0, + 'rootPort' => $stp['BRIDGE-MIB::dot1dStpRootPort.0'] ?? 0, + 'maxAge' => ($stp['BRIDGE-MIB::dot1dStpMaxAge.0'] ?? 0) * $timeFactor, + 'helloTime' => ($stp['BRIDGE-MIB::dot1dStpHelloTime.0'] ?? 0) * $timeFactor, + 'holdTime' => ($stp['BRIDGE-MIB::dot1dStpHoldTime.0'] ?? 0) * $timeFactor, + 'forwardDelay' => ($stp['BRIDGE-MIB::dot1dStpForwardDelay.0'] ?? 0) * $timeFactor, + 'bridgeMaxAge' => ($stp['BRIDGE-MIB::dot1dStpBridgeMaxAge.0'] ?? 0) * $timeFactor, + 'bridgeHelloTime' => ($stp['BRIDGE-MIB::dot1dStpBridgeHelloTime.0'] ?? 0) * $timeFactor, + 'bridgeForwardDelay' => ($stp['BRIDGE-MIB::dot1dStpBridgeForwardDelay.0'] ?? 0) * $timeFactor, + ]); + + return (new Collection())->push($instance); } public function discoverStpPorts(Collection $stpInstances): Collection