From 517a32c67b0354ac22cac6d26dbc987a21a3062c Mon Sep 17 00:00:00 2001
From: Slashdoom <5092581+slashdoom@users.noreply.github.com>
Date: Fri, 10 May 2019 14:37:30 +1200
Subject: [PATCH] Fix: netstats-udp, tcp and ip rrd's not updating (#10197)

* netstats fixes

* code climate

* Update netstats-ip.inc.php

* fixes

* code climate

* GH feedback changes

* codeclimate
---
 includes/polling/netstats/netstats-ip.inc.php | 40 ++++++---------
 .../polling/netstats/netstats-tcp.inc.php     | 50 ++++++++-----------
 .../polling/netstats/netstats-udp.inc.php     | 40 ++++++---------
 3 files changed, 52 insertions(+), 78 deletions(-)

diff --git a/includes/polling/netstats/netstats-ip.inc.php b/includes/polling/netstats/netstats-ip.inc.php
index af9c3b8e71..c7966bb09e 100644
--- a/includes/polling/netstats/netstats-ip.inc.php
+++ b/includes/polling/netstats/netstats-ip.inc.php
@@ -2,11 +2,10 @@
 
 use LibreNMS\RRD\RrdDefinition;
 
-if (!starts_with($device['os'], array('Snom', 'asa'))) {
+if (!starts_with($device['os'], ['Snom', 'asa'])) {
     echo ' IP';
 
-    // These are at the start of large trees that we don't want to walk the entirety of, so we snmp_get_multi them
-    $oids = array(
+    $oids = [
         'ipForwDatagrams',
         'ipInDelivers',
         'ipInReceives',
@@ -23,34 +22,25 @@ if (!starts_with($device['os'], array('Snom', 'asa'))) {
         'ipInUnknownProtos',
         'ipInHdrErrors',
         'ipInAddrErrors',
-    );
+    ];
+    $data = snmp_getnext_multi($device, $oids, '-OQUs', 'IP-MIB');
 
-    $rrd_def = new RrdDefinition();
-    $snmpstring = '';
-    foreach ($oids as $oid) {
-        $rrd_def->addDataset($oid, 'COUNTER', null, 100000000000);
-        $snmpstring .= ' IP-MIB::'.$oid.'.0';
-    }
-
-    $data = snmp_get_multi($device, $snmpstring, '-OQUs', 'IP-MIB');
-
-    $fields = array();
-    foreach ($oids as $oid) {
-        if (is_numeric($data[0][$oid])) {
-            $value = $data[0][$oid];
-        } else {
-            $value = 'U';
+    if (is_numeric($data['ipOutRequests']) && is_numeric($data['ipInReceives'])) {
+        $rrd_def = new RrdDefinition();
+        $fields = [];
+        foreach ($oids as $oid) {
+            $rrd_def->addDataset($oid, 'COUNTER', null, 100000000000);
+            $fields[$oid] = is_numeric($data[$oid]) ? $data[$oid] : 'U';
         }
-        $fields[$oid] = $value;
-    }
 
-    if (isset($data[0]['ipOutRequests']) && isset($data[0]['ipInReceives'])) {
         $tags = compact('rrd_def');
         data_update($device, 'netstats-ip', $tags, $fields);
 
         $graphs['netstat_ip']      = true;
         $graphs['netstat_ip_frag'] = true;
-    }
-}//end if
 
-unset($oids, $data, $snmpstring, $rrd_def, $fields, $tags);
+        unset($rrd_def, $fields, $tags, $oid);
+    }
+
+    unset($oids, $data);
+}//end if
diff --git a/includes/polling/netstats/netstats-tcp.inc.php b/includes/polling/netstats/netstats-tcp.inc.php
index 1f5c21379d..42c4af3923 100644
--- a/includes/polling/netstats/netstats-tcp.inc.php
+++ b/includes/polling/netstats/netstats-tcp.inc.php
@@ -2,10 +2,9 @@
 
 use LibreNMS\RRD\RrdDefinition;
 
-if (!starts_with($device['os'], array('Snom', 'asa'))) {
+if (!starts_with($device['os'], ['Snom', 'asa'])) {
     echo ' TCP';
-
-    $oids = array(
+    $oids = [
         'tcpActiveOpens',
         'tcpPassiveOpens',
         'tcpAttemptFails',
@@ -16,40 +15,35 @@ if (!starts_with($device['os'], array('Snom', 'asa'))) {
         'tcpRetransSegs',
         'tcpInErrs',
         'tcpOutRsts',
-    );
+    ];
+    $data = snmp_getnext_multi($device, $oids, '-OQUs', 'TCP-MIB');
 
-    $rrd_def = new RrdDefinition();
-    $snmpstring = '';
-    foreach ($oids as $oid) {
-        $rrd_def->addDataset($oid, 'COUNTER', null, 10000000);
-        $snmpstring .= ' TCP-MIB::'.$oid.'.0';
-    }
+    echo ' TCPHC';
+    $hc_oids = [
+        'tcpHCInSegs.0',
+        'tcpHCOutSegs.0',
+    ];
+    $hc_data = snmp_getnext_multi($device, $hc_oids, '-OQUs', 'TCP-MIB');
 
-    $snmpstring .= ' tcpHCInSegs.0';
-    $snmpstring .= ' tcpHCOutSegs.0';
-
-    $data = snmp_get_multi($device, $snmpstring, '-OQUs', 'TCP-MIB');
-    $data = $data[0];
-
-    if (isset($data['tcpInSegs']) && isset($data['tcpOutSegs'])) {
-        $fields = array();
+    if ((is_numeric($data['tcpInSegs']) && is_numeric($data['tcpOutSegs'])) || (is_numeric($hc_data['tcpHCInSegs']) && is_numeric($hc_data['tcpHCOutSegs']))) {
+        $rrd_def = new RrdDefinition();
+        $fields = [];
         foreach ($oids as $oid) {
-            $fields[$oid] = isset($data[$oid]) ? $data[$oid] : 'U';
-        }
-
-        // use HC Segs if we have them.
-        if (isset($data['tcpHCInSegs'])) {
-            if (!empty($data['tcpHCInSegs'])) {
-                $fields['tcpInSegs'] = $data['tcpHCInSegs'];
-                $fields['tcpOutSegs'] = $data['tcpHCOutSegs'];
-            }
+            $rrd_def->addDataset($oid, 'COUNTER', null, 10000000);
+            $fields[$oid] = is_numeric($data[$oid]) ? $data[$oid] : 'U';
         }
 
+        // Replace Segs with HC Segs if we have them.
+        $fields['tcpInSegs'] = !empty($hc_data['tcpHCInSegs']) ? $hc_data['tcpHCInSegs'] : $fields['tcpInSegs'];
+        $fields['tcpOutSegs'] = !empty($hc_data['tcpHCOutSegs']) ? $hc_data['tcpHCOutSegs'] : $fields['tcpOutSegs'];
+    
         $tags = compact('rrd_def');
         data_update($device, 'netstats-tcp', $tags, $fields);
 
         $graphs['netstat_tcp'] = true;
+
+        unset($rrd_def, $fields, $tags, $oid);
     }
 
-    unset($oids, $data, $fields, $oid, $snmpstring);
+    unset($oids, $hc_oids, $data, $hc_data);
 }//end if
diff --git a/includes/polling/netstats/netstats-udp.inc.php b/includes/polling/netstats/netstats-udp.inc.php
index 360fcc29e8..c7236b2b26 100644
--- a/includes/polling/netstats/netstats-udp.inc.php
+++ b/includes/polling/netstats/netstats-udp.inc.php
@@ -2,42 +2,32 @@
 
 use LibreNMS\RRD\RrdDefinition;
 
-if (!starts_with($device['os'], array('Snom', 'asa'))) {
+if (!starts_with($device['os'], ['Snom', 'asa'])) {
     echo ' UDP';
 
-    // These are at the start of large trees that we don't want to walk the entirety of, so we snmpget_multi them
-    $oids = array(
+    $oids = [
         'udpInDatagrams',
         'udpOutDatagrams',
         'udpInErrors',
         'udpNoPorts',
-    );
+    ];
+    $data = snmp_getnext_multi($device, $oids, '-OQUs', 'UDP-MIB');
 
-    $rrd_def = new RrdDefinition();
-    $snmpstring = '';
-    foreach ($oids as $oid) {
-        $rrd_def->addDataset($oid, 'COUNTER', null, 1000000); // Limit to 1MPPS?
-        $snmpstring .= ' UDP-MIB::'.$oid.'.0';
-    }
-
-    $data = snmp_get_multi($device, $snmpstring, '-OQUs', 'UDP-MIB');
-
-    $fields = array();
-    foreach ($oids as $oid) {
-        if (is_numeric($data[0][$oid])) {
-            $value = $data[0][$oid];
-        } else {
-            $value = 'U';
+    if (is_numeric($data['udpInDatagrams']) && is_numeric($data['udpOutDatagrams'])) {
+        $rrd_def = new RrdDefinition();
+        $fields = [];
+        foreach ($oids as $oid) {
+            $rrd_def->addDataset($oid, 'COUNTER', null, 1000000); // Limit to 1MPPS?
+            $fields[$oid] = is_numeric($data[$oid]) ? $data[$oid] : 'U';
         }
-        $fields[$oid] = $value;
-    }
 
-    if (isset($data[0]['udpInDatagrams']) && isset($data[0]['udpOutDatagrams'])) {
         $tags = compact('rrd_def');
         data_update($device, 'netstats-udp', $tags, $fields);
 
         $graphs['netstat_udp'] = true;
-    }
-}//end if
 
-unset($oids, $data, $rrd_def, $fields, $tags, $snmpstring);
+        unset($rrd_def, $fields, $tags, $oid);
+    }
+
+    unset($oids, $data);
+}//end if