refactor: discovery code cleanups (#6856)

* Use foreach $key => value

* use get_port_by_ifIndex
get rid of the rest of the array_keys()

* improve hex_to_ip() function, add tests, use in more places

* Simplify can_skip_discovery logic

includes/discovery/cisco-sla.inc.php

@@ -58,15 +58,7 @@ if ($config['enable_sla'] && $device['os_group'] == 'cisco') {
                     break;
 
                 case 'echo':
-                    $parts = explode(' ', $sla_config['rttMonEchoAdminTargetAddress']);
-                    if (count($parts) == 4) {
-                        // IPv4
-                        $data['tag'] = implode('.', array_map('hexdec', $parts));
-                    } elseif (count($parts) == 16) {
-                        // IPv6
-                        $data['tag'] = $parts[0].$parts[1].':'.$parts[2].$parts[3].':'.$parts[4].$parts[5].':'.$parts[6].$parts[7].':'.$parts[8].$parts[9].':'.$parts[10].$parts[11].':'.$parts[12].$parts[13].':'.$parts[14].$parts[15];
-                        $data['tag'] = preg_replace('/:0*([0-9])/', ':$1', $data['tag']);
-                    }
+                    $data['tag'] = hex_to_ip($sla_config['rttMonEchoAdminTargetAddress']);
                     break;
 
                 case 'jitter':

includes/discovery/discovery-protocols.inc.php

@@ -10,26 +10,17 @@ if ($device['os'] == 'ironware' && $config['autodiscovery']['xdp'] === true) {
     d_echo($fdp_array);
     if ($fdp_array) {
         unset($fdp_links);
-        foreach (array_keys($fdp_array) as $key) {
-            $interface    = dbFetchRow('SELECT * FROM `ports` WHERE `device_id` = ? AND `ifIndex` = ?', array($device['device_id'], $key));
-            $fdp_if_array = $fdp_array[$key];
+        foreach ($fdp_array as $key => $fdp_if_array) {
+            $interface = get_port_by_ifIndex($device['device_id'], $key);
             d_echo($fdp_if_array);
-            foreach (array_keys($fdp_if_array) as $entry_key) {
-                $fdp              = $fdp_if_array[$entry_key];
+            foreach ($fdp_if_array as $entry_key => $fdp) {
                 $remote_device_id = dbFetchCell('SELECT `device_id` FROM `devices` WHERE `sysName` = ? OR `hostname` = ?', array($fdp['snFdpCacheDeviceId'], $fdp['snFdpCacheDeviceId']));
 
-                if (!$remote_device_id) {
-                    $skip_discovery = false;
-                    if ($skip_discovery === false) {
-                        $skip_discovery = can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysname_regexp'], $fdp['snFdpCacheDeviceId'], $fdp['snFdpCacheDeviceId']);
-                    }
-                    if ($skip_discovery === false) {
-                        $skip_discovery = can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysdesc_regexp'], $fdp['snFdpCacheVersion'], $fdp['snFdpCacheDeviceId']);
-                    }
-
-                    if ($skip_discovery === false) {
-                        $remote_device_id = discover_new_device($fdp['snFdpCacheDeviceId'], $device, 'FDP', $interface);
-                    }
+                if (!$remote_device_id &&
+                    !can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysname_regexp'], $fdp['snFdpCacheDeviceId'], $fdp['snFdpCacheDeviceId']) &&
+                    !can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysdesc_regexp'], $fdp['snFdpCacheVersion'], $fdp['snFdpCacheDeviceId'])
+                ) {
+                    $remote_device_id = discover_new_device($fdp['snFdpCacheDeviceId'], $device, 'FDP', $interface);
                 }
 
                 if ($remote_device_id) {
@@ -52,40 +43,23 @@ if ($config['autodiscovery']['xdp'] === true) {
     d_echo($cdp_array);
     if ($cdp_array) {
         unset($cdp_links);
-        foreach (array_keys($cdp_array) as $key) {
-            $interface        = dbFetchRow('SELECT * FROM `ports` WHERE device_id = ? AND `ifIndex` = ?', array($device['device_id'], $key));
-            $cdp_if_array = $cdp_array[$key];
+        foreach ($cdp_array as $key => $cdp_if_array) {
+            $interface = get_port_by_ifIndex($device['device_id'], $key);
             d_echo($cdp_if_array);
-            foreach (array_keys($cdp_if_array) as $entry_key) {
-                $cdp = $cdp_if_array[$entry_key];
+            foreach ($cdp_if_array as $entry_key => $cdp) {
                 if (is_valid_hostname($cdp['cdpCacheDeviceId']) || ($config['discovery_by_ip'] == true)) {
-                    $ip_arr = explode(" ", $cdp['cdpCacheAddress']);
-                    $a = hexdec($ip_arr[0]);
-                    $b = hexdec($ip_arr[1]);
-                    $c = hexdec($ip_arr[2]);
-                    $d = hexdec($ip_arr[3]);
-
-                    $cdp_ip = "$a.$b.$c.$d";
+                    $cdp_ip = hex_to_ip($cdp['cdpCacheAddress']);
                     $remote_device_id = dbFetchCell('SELECT `device_id` FROM `devices` WHERE `sysName` = ? OR `hostname` = ? OR `hostname` = ?', array($cdp['cdpCacheDeviceId'], $cdp['cdpCacheDeviceId'], $cdp_ip));
 
-                    if (!$remote_device_id) {
-                        $skip_discovery = false;
-                        if ($skip_discovery === false) {
-                            $skip_discovery = can_skip_discovery($config['autodiscovery']['cdp_exclude']['platform_regexp'], $cdp['cdpCachePlatform'], $cdp['cdpCacheDeviceId']);
-                        }
-                        if ($skip_discovery === false) {
-                            $skip_discovery = can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysname_regexp'], $cdp['cdpCacheDeviceId'], $cdp['cdpCacheDeviceId']);
-                        }
-                        if ($skip_discovery === false) {
-                            $skip_discovery = can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysdesc_regexp'], $cdp['cdpCacheVersion'], $cdp['cdpCacheDeviceId']);
-                        }
-
-                        if ($skip_discovery === false) {
-                            if ($config['discovery_by_ip'] !== true) {
-                                $remote_device_id = discover_new_device($cdp['cdpCacheDeviceId'], $device, 'CDP', $interface);
-                            } else {
-                                $remote_device_id = discover_new_device($cdp_ip, $device, 'CDP', $interface);
-                            }
+                    if (!$remote_device_id &&
+                        !can_skip_discovery($config['autodiscovery']['cdp_exclude']['platform_regexp'], $cdp['cdpCachePlatform'], $cdp['cdpCacheDeviceId']) &&
+                        !can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysname_regexp'], $cdp['cdpCacheDeviceId'], $cdp['cdpCacheDeviceId']) &&
+                        !can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysdesc_regexp'], $cdp['cdpCacheVersion'], $cdp['cdpCacheDeviceId'])
+                    ) {
+                        if ($config['discovery_by_ip'] !== true) {
+                            $remote_device_id = discover_new_device($cdp['cdpCacheDeviceId'], $device, 'CDP', $interface);
+                        } else {
+                            $remote_device_id = discover_new_device($cdp_ip, $device, 'CDP', $interface);
                         }
                     }
 
@@ -118,24 +92,17 @@ if ($device['os'] == 'pbn' && $config['autodiscovery']['xdp'] === true) {
     d_echo($lldp_array);
     if ($lldp_array) {
         unset($lldp_links);
-        foreach (array_keys($lldp_array) as $key) {
-            $lldp = $lldp_array[$key];
+        foreach ($lldp_array as $key => $lldp) {
             d_echo($lldp);
-            $interface     = dbFetchRow('SELECT * FROM `ports` WHERE `device_id` = ? AND `ifIndex` = ?', array($device['device_id'], $lldp['lldpRemLocalPortNum']));
+            $interface = get_port_by_ifIndex($device['device_id'], $lldp['lldpRemLocalPortNum']);
             $remote_device_id = dbFetchCell('SELECT `device_id` FROM `devices` WHERE `sysName` = ? OR `hostname` = ?', array($lldp['lldpRemSysName'], $lldp['lldpRemSysName']));
 
-            if (!$remote_device_id && is_valid_hostname($lldp['lldpRemSysName'])) {
-                $skip_discovery = false;
-                if ($skip_discovery === false) {
-                    $skip_discovery = can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysname_regexp'], $lldp['lldpRemSysName'], $lldp['lldpRemSysName']);
-                }
-                if ($skip_discovery === false) {
-                    $skip_discovery = can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysdesc_regexp'], $lldp['lldpRemSysDesc'], $lldp['lldpRemSysName']);
-                }
-
-                if ($skip_discovery === false) {
-                    $remote_device_id = discover_new_device($lldp['lldpRemSysName'], $device, 'LLDP', $interface);
-                }
+            if (!$remote_device_id &&
+                is_valid_hostname($lldp['lldpRemSysName']) &&
+                !can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysname_regexp'], $lldp['lldpRemSysName'], $lldp['lldpRemSysName']) &&
+                !can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysdesc_regexp'], $lldp['lldpRemSysDesc'], $lldp['lldpRemSysName'])
+            ) {
+                $remote_device_id = discover_new_device($lldp['lldpRemSysName'], $device, 'LLDP', $interface);
             }
 
             if ($remote_device_id) {
@@ -159,46 +126,32 @@ if ($device['os'] == 'pbn' && $config['autodiscovery']['xdp'] === true) {
     d_echo($dot1d_array);
     if ($lldp_array) {
         $lldp_links = '';
-        foreach (array_keys($lldp_array) as $key) {
-            $lldp_if_array = $lldp_array[$key];
+        foreach ($lldp_array as $key => $lldp_if_array) {
             d_echo($lldp_if_array);
-            foreach (array_keys($lldp_if_array) as $entry_key) {
+            foreach ($lldp_if_array as $entry_key => $lldp_instance) {
                 if (is_numeric($dot1d_array[$entry_key]['dot1dBasePortIfIndex'])) {
                     $ifIndex = $dot1d_array[$entry_key]['dot1dBasePortIfIndex'];
                 } else {
                     $ifIndex = $entry_key;
                 }
+                $interface = get_port_by_ifIndex($device['device_id'], $ifIndex);
 
-                $interface     = dbFetchRow('SELECT * FROM `ports` WHERE `device_id` = ? AND `ifIndex` = ?', array($device['device_id'], $ifIndex));
-                $lldp_instance = $lldp_if_array[$entry_key];
                 d_echo($lldp_instance);
-                foreach (array_keys($lldp_instance) as $entry_instance) {
-                    $lldp             = $lldp_instance[$entry_instance];
+                foreach ($lldp_instance as $entry_instance => $lldp) {
                     $remote_device_id = dbFetchCell('SELECT `device_id` FROM `devices` WHERE `sysName` = ? OR `hostname` = ?', array($lldp['lldpRemSysName'], $lldp['lldpRemSysName']));
 
                     if (!$remote_device_id && is_valid_hostname($lldp['lldpRemSysName'])) {
-                        $skip_discovery = false;
-                        if ($skip_discovery === false) {
-                            $skip_discovery = can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysname_regexp'], $lldp['lldpRemSysName'], $lldp['lldpRemSysName']);
-                        }
-                        if ($skip_discovery === false) {
-                            $skip_discovery = can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysdesc_regexp'], $lldp['lldpRemSysDesc'], $lldp['lldpRemSysName']);
-                        }
-                        if ($skip_discovery === false) {
+                        if (!can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysname_regexp'], $lldp['lldpRemSysName'], $lldp['lldpRemSysName']) &&
+                            can_skip_discovery($config['autodiscovery']['xdp_exclude']['sysdesc_regexp'], $lldp['lldpRemSysDesc'], $lldp['lldpRemSysName'])
+                        ) {
                             $remote_device_id = discover_new_device($lldp['lldpRemSysName'], $device, 'LLDP', $interface);
                             if (is_numeric($remote_device_id) === false) {
                                 $ptopo_array = snmpwalk_cache_oid($device, 'ptopoConnEntry', array(), 'PTOPO-MIB');
                                 d_echo($ptopo_array);
-                                foreach (array_keys($ptopo_array) as $ptopo_key) {
-                                    if (strcmp(trim($ptopo_array[$ptopo_key]['ptopoConnRemoteChassis']), trim($lldp['lldpRemChassisId'])) == 0) {
-                                        $ip_arr = explode(" ", $ptopo_array[$ptopo_key]['ptopoConnAgentNetAddr']);
-
-                                        $a = hexdec($ip_arr[0]);
-                                        $b = hexdec($ip_arr[1]);
-                                        $c = hexdec($ip_arr[2]);
-                                        $d = hexdec($ip_arr[3]);
-
-                                        $discover_hostname = "$a.$b.$c.$d";
+                                foreach ($ptopo_array as $ptopo) {
+                                    if (strcmp(trim($ptopo['ptopoConnRemoteChassis']), trim($lldp['lldpRemChassisId'])) == 0) {
+                                        $discover_hostname = hex_to_ip($ptopo['ptopoConnAgentNetAddr']);
+                                        break;
                                     }
                                 }
                                 unset(

includes/functions.php

@@ -1357,22 +1357,33 @@ function first_oid_match($device, $list)
     }
 }
 
+
+/**
+ * Convert a hex ip to a human readable string
+ *
+ * @param string $hex
+ * @return string
+ */
 function hex_to_ip($hex)
 {
-    $return = "";
-    if (filter_var($hex, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) === false && filter_var($hex, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) === false) {
-        $hex_exp = explode(' ', $hex);
-        foreach ($hex_exp as $item) {
-            if (!empty($item) && $item != "\"") {
-                $return .= hexdec($item).'.';
-            }
-        }
-        $return = substr($return, 0, -1);
-    } else {
-        $return = $hex;
+    $hex = str_replace(array(' ', '"'), '', $hex);
+
+    if (filter_var($hex, FILTER_VALIDATE_IP)) {
+        return $hex;
     }
-    return $return;
+
+    if (strlen($hex) == 8) {
+        return long2ip(hexdec($hex));
+    }
+
+    if (strlen($hex) == 32) {
+        $ipv6 = implode(':', str_split($hex, 4));
+        return preg_replace('/:0*([0-9a-fA-F])/', ':$1', $ipv6);
+    }
+
+    return ''; // invalid input
 }
+
 function fix_integer_value($value)
 {
     if ($value < 0) {

tests/FunctionsTest.php

@@ -50,4 +50,22 @@ class FunctionsTest extends \PHPUnit_Framework_TestCase
         $this->assertFalse(isHexString('a5 fj 53'));
         $this->assertFalse(isHexString('a5fe53'));
     }
+
+    public function testHexToIp()
+    {
+        $this->assertSame("192.168.1.254", hex_to_ip("c0 a8 01 fe"));
+        $this->assertSame("192.168.1.254", hex_to_ip("c0a801fe"));
+        $this->assertSame("192.168.1.254", hex_to_ip("c0 a8 01 fe "));
+        $this->assertNotSame("192.168.1.1.254", hex_to_ip("c0 a8 01 01 fe"));
+        $this->assertSame("192.168.1.254", hex_to_ip("\"c0 a8 01 fe\""));
+        $this->assertSame("192.168.1.254", hex_to_ip("192.168.1.254"));
+
+        $this->assertSame('2001:db8:0:0:0:0:2:1', hex_to_ip('2001:db8:0:0:0:0:2:1'));
+        $this->assertSame('2001:db8:0:0:0:0:2:1', hex_to_ip('20 01 0d b8 00 00 00 00 00 00 00 00 00 02 00 01'));
+        $this->assertSame('2001:db8:0:0:0:0:2:1', hex_to_ip('20010db8000000000000000000020001'));
+        $this->assertNotSame(
+            '2001:db8:0:0:0:0:2:0:1',
+            hex_to_ip('20 01 0d b8 00 00 00 00 00 00 00 00 00 02 00 00 00 01')
+        );
+    }
 }