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 😠

* nullable vlan

Co-authored-by: PipoCanaja <38363551+PipoCanaja@users.noreply.github.com>

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;
 }

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');

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;
+        // fetch STP config and store it
-        $instances = new Collection;
+        $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) {
+        if (empty($stp)) {
-            // fetch STP config and store it
+            return new Collection;
-            $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,
-            ]));
         }
 
-        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