fix: fdb tables multiple IPs and IPs from other devices adding extra rows (#6930)

Fix missing IPs because the ARP entries are on different port_id
Update arp discovery to use snmpwalk_group() additional fix in that function
Fix removal of arp entries
Fix a debug output type in fdb discovery

html/includes/table/fdb-search.inc.php

@@ -1,53 +1,59 @@
 <?php
 $param = array();
 
-$sql = ' FROM `ports_fdb` AS `F`';
+$select = "SELECT `F`.`port_id` AS `port_id`, `device_id`, `ifInErrors`, `ifOutErrors`, `ifOperStatus`,";
+$select .= " `ifAdminStatus`, `ifAlias` AS `interface`, `ifDescr`, `mac_address`, `V`.`vlan_vlan` AS `vlan`,";
+$select .= " `hostname`, `hostname` AS `device` , group_concat(`M`.`ipv4_address` SEPARATOR ', ') AS `ipv4_address`";
+
+$sql  = " FROM `ports_fdb` AS `F`";
+$sql .= " LEFT JOIN `devices` AS `D` USING(`device_id`)";
+$sql .= " LEFT JOIN `ports` AS `P` USING(`port_id`, `device_id`)";
+$sql .= " LEFT JOIN `vlans` AS `V` USING(`vlan_id`, `device_id`)";
+
+$where = " WHERE 1";
 
 if (is_admin() === false && is_read() === false) {
-    $sql    .= ' LEFT JOIN `devices_perms` AS `DP` ON `D`.`device_id` = `DP`.`device_id`';
+    $sql    .= ' LEFT JOIN `devices_perms` AS `DP` USING (`device_id`)';
     $where  .= ' AND `DP`.`user_id`=?';
     $param[] = $_SESSION['user_id'];
 }
 
-$sql .= " LEFT JOIN `ports` AS `P` ON `F`.`port_id`=`P`.`port_id`";
-$sql .= " LEFT JOIN `devices` AS `D` ON `F`.`device_id`=`D`.`device_id`";
-$sql .= " LEFT JOIN `vlans` AS `V` ON `F`.`vlan_id`=`V`.`vlan_id`";
-$sql .= " LEFT JOIN `ipv4_mac` ON `F`.`mac_address`=`ipv4_mac`.`mac_address`";
-
-$sql .= " WHERE 1";
 if (is_numeric($_POST['device_id'])) {
-    $sql    .= ' AND `F`.`device_id`=?';
+    $where    .= ' AND `F`.`device_id`=?';
     $param[] = $_POST['device_id'];
 }
 
 if (is_numeric($_POST['port_id'])) {
-    $sql    .= ' AND `F`.`port_id`=?';
+    $where    .= ' AND `F`.`port_id`=?';
     $param[] = $_POST['port_id'];
 }
 
 if (isset($_POST['searchPhrase']) && !empty($_POST['searchPhrase'])) {
-    $vlan_search = mres(trim($_POST['searchPhrase']));
-    $mac_search = '%'.str_replace(array(':', ' ', '-', '.', '0x'), '', mres($_POST['searchPhrase'])).'%';
+    $search = mres(trim($_POST['searchPhrase']));
+    $mac_search = '%'.str_replace(array(':', ' ', '-', '.', '0x'), '', $search).'%';
 
     if (isset($_POST['searchby']) && $_POST['searchby'] == 'vlan') {
-        $sql    .= ' AND `V`.`vlan_vlan` = ?';
-        $param[] = $vlan_search;
-    } elseif (isset($_POST['searchby']) && $_POST['searchby'] == 'mac') {
-        $sql    .= ' AND `F`.`mac_address` LIKE ?';
+        $where  .= ' AND `V`.`vlan_vlan` = ?';
+        $param[] = (int)$search;
+    } elseif ((isset($_POST['searchby']) && $_POST['searchby'] == 'mac') ||
+        (!is_numeric($search) || $search > 4096)
+    ) {
+        $where  .= ' AND `F`.`mac_address` LIKE ?';
         $param[] = $mac_search;
     } else {
-        $sql .= ' AND (`V`.`vlan_vlan` = ? OR `F`.`mac_address` LIKE ?)';
-        $param[] = $vlan_search;
+        $where  .= ' AND (`V`.`vlan_vlan` = ? OR `F`.`mac_address` LIKE ?)';
+        $param[] = (int)$search;
         $param[] = $mac_search;
     }
 }
 
-$count_sql = "SELECT COUNT(`F`.`port_id`) $sql";
+$total = (int)dbFetchCell("SELECT COUNT(*) $sql $where", $param);
 
-$total = dbFetchCell($count_sql, $param);
-if (empty($total)) {
-    $total = 0;
-}
+// Don't use ipv4_mac in count it will inflate the rows unless we aggregate it and it isn't used for search
+$sql .= " LEFT JOIN `ipv4_mac` AS `M` USING (`mac_address`, `device_id`)";
+$sql .= $where;
+$sql .= " GROUP BY `device_id`, `port_id`, `mac_address`, `vlan`, `hostname`, `ifAlias`,";
+$sql .= " `ifAdminStatus`, `ifDescr`, `ifOperStatus`, `ifInErrors`, `ifOutErrors`";
 
 if (!isset($sort) || empty($sort)) {
     $sort = '`F`.`port_id` ASC';
@@ -64,23 +70,26 @@ if ($rowCount != -1) {
     $sql .= " LIMIT $limit_low,$limit_high";
 }
 
-$sql = "SELECT `P`.*, `ifDescr` AS `interface`, `F`.`mac_address`, `ipv4_mac`.`ipv4_address`, `V`.`vlan_vlan` as `vlan`, `D`.`hostname` AS `device` $sql";
-
-foreach (dbFetchRows($sql, $param) as $entry) {
+$response = array();
+foreach (dbFetchRows($select . $sql, $param) as $entry) {
     $entry = cleanPort($entry);
     if (!$ignore) {
         if ($entry['ifInErrors'] > 0 || $entry['ifOutErrors'] > 0) {
-            $error_img = generate_port_link($entry, "<i class='fa fa-flag fa-lg' style='color:red' aria-hidden='true'></i>", 'port_errors');
+            $error_img = generate_port_link(
+                $entry,
+                "<i class='fa fa-flag fa-lg' style='color:red' aria-hidden='true'></i>",
+                'port_errors'
+            );
         } else {
             $error_img = '';
         }
 
         $response[] = array(
-            'device'           => generate_device_link(device_by_id_cache($entry['device_id'])),
-            'mac_address'      => formatMac($entry['mac_address']),
-            'ipv4_address'     => $entry['ipv4_address'],
-            'interface'        => generate_port_link($entry, makeshortif(fixifname(cleanPort($entry['label'])))).' '.$error_img,
-            'vlan'             => $entry['vlan'],
+            'device'       => generate_device_link(device_by_id_cache($entry['device_id'])),
+            'mac_address'  => formatMac($entry['mac_address']),
+            'ipv4_address' => $entry['ipv4_address'],
+            'interface'    => generate_port_link($entry, makeshortif(fixifname($entry['label']))).' '.$error_img,
+            'vlan'         => $entry['vlan'],
         );
     }//end if
 

includes/discovery/arp-table.inc.php

@@ -33,33 +33,34 @@ foreach ($vrfs_lite_cisco as $vrf) {
     $context = $vrf['context_name'];
     $device['context_name']=$context;
 
-    $arp_data = snmpwalk_cache_multi_oid($device, 'ipNetToPhysicalPhysAddress', array(), 'IP-MIB');
-    $arp_data = snmpwalk_cache_multi_oid($device, 'ipNetToMediaPhysAddress', $arp_data, 'IP-MIB');
+    $arp_data = snmpwalk_group($device, 'ipNetToPhysicalPhysAddress', 'IP-MIB');
+    $arp_data = snmpwalk_group($device, 'ipNetToMediaPhysAddress', 'IP-MIB', 1, $arp_data);
 
-    $sql = "SELECT M.* from ipv4_mac AS M, ports AS I WHERE M.port_id=I.port_id AND I.device_id=? AND M.context_name=?";
-    $params = array($device['device_id'], $context);
-    $existing_data = dbFetchRows($sql, $params);
-    $ipv4_addresses = array();
-    foreach ($existing_data as $data) {
-        $ipv4_addresses[] = $data['ipv4_address'];
-    }
+    $existing_data = dbFetchRows(
+        "SELECT * from `ipv4_mac` WHERE `device_id`=? AND `context_name`=?",
+        array($device['device_id'], $context)
+    );
+    $ipv4_addresses = array_map(function ($data) {
+        return $data['ipv4_address'];
+    }, $existing_data);
 
     $arp_table = array();
     $insert_data = array();
-    foreach ($arp_data as $ip => $data) {
-        if (isset($data['ipNetToPhysicalPhysAddress'])) {
-            $raw_mac = $data['ipNetToPhysicalPhysAddress'];
-            list($if, $ipv, $ip) = explode('.', $ip, 3);
-        } elseif (isset($data['ipNetToMediaPhysAddress'])) {
-            $raw_mac = $data['ipNetToMediaPhysAddress'];
-            list($if, $ip)  = explode('.', $ip, 2);
-            $ipv = 'ipv4';
-        }
-
-        $interface = get_port_by_index_cache($device['device_id'], $if);
+    foreach ($arp_data as $ifIndex => $data) {
+        $interface = get_port_by_index_cache($device['device_id'], $ifIndex);
         $port_id = $interface['port_id'];
 
-        if (!empty($ip) && $ipv === 'ipv4' && !empty($raw_mac) && $raw_mac != '0:0:0:0:0:0' && !isset($arp_table[$port_id][$ip])) {
+        $port_arp = array_merge(
+            (array)$data['ipNetToMediaPhysAddress'],
+            (array)$data['ipNetToPhysicalPhysAddress']['ipv4']
+        );
+
+        echo "{$interface['ifName']}: \n";
+        foreach ($port_arp as $ip => $raw_mac) {
+            if (empty($ip) || empty($raw_mac) || $raw_mac == '0:0:0:0:0:0' || isset($arp_table[$port_id][$ip])) {
+                continue;
+            }
+
             $mac = implode(array_map('zeropad', explode(':', $raw_mac)));
             $arp_table[$port_id][$ip] = $mac;
 
@@ -71,24 +72,23 @@ foreach ($vrfs_lite_cisco as $vrf) {
                     log_event("MAC change: $ip : " . mac_clean_to_readable($old_mac) . ' -> ' . mac_clean_to_readable($mac), $device, 'interface', 4, $port_id);
                     dbUpdate(array('mac_address' => $mac), 'ipv4_mac', 'port_id=? AND ipv4_address=? AND context_name=?', array($port_id, $ip, $context));
                 }
-                d_echo(null, '.');
+                d_echo("$raw_mac => $ip\n", '.');
             } elseif (isset($interface['port_id'])) {
-                d_echo(null, '+');
+                d_echo("$raw_mac => $ip\n", '+');
                 $insert_data[] = array(
                     'port_id'      => $port_id,
+                    'device_id'    => $device['device_id'],
                     'mac_address'  => $mac,
                     'ipv4_address' => $ip,
                     'context_name' => $context,
                 );
             }
         }
-
-        unset(
-            $interface
-        );
+        echo PHP_EOL;
     }
 
     unset(
+        $interface,
         $arp_data,
         $ipv4_addresses,
         $data
@@ -111,8 +111,10 @@ foreach ($vrfs_lite_cisco as $vrf) {
     }
 
     // remove entries that no longer have an owner
-    dbQuery('DELETE `ipv4_mac` FROM `ipv4_mac` LEFT JOIN `ports`
- ON `ipv4_mac`.`port_id` = `ports`.`port_id` WHERE `ports`.`port_id` IS NULL');
+    dbQuery('DELETE `ipv4_mac` FROM `ipv4_mac`
+ LEFT JOIN `ports` ON `ipv4_mac`.`port_id` = `ports`.`port_id` 
+ LEFT JOIN `devices` ON `ipv4_mac`.`device_id` = `devices`.`device_id` 
+ WHERE `ports`.`port_id` IS NULL OR `devices`.`device_id` IS NULL');
 
     echo PHP_EOL;
     unset(

includes/discovery/fdb-table.inc.php

@@ -11,7 +11,7 @@ $vlans_by_id = array_flip($vlans_dict);
 $existing_fdbs = array();
 $sql_result = dbFetchRows("SELECT * FROM `ports_fdb` WHERE `device_id` = ?", array($device['device_id']));
 foreach ($sql_result as $entry) {
-    $existing_fdbs[$entry['vlan_id']][$entry['mac_address']] = $entry;
+    $existing_fdbs[(int)$entry['vlan_id']][$entry['mac_address']] = $entry;
 }
 
 $insert = array(); // populate $insert with database entries
@@ -58,16 +58,32 @@ if (!empty($insert)) {
             }
         }
 
-        // Delete old entries from the database
-        foreach ($existing_fdbs[$vlan_id] as $entry) {
-                dbDelete(
-                    'ports_fdb',
-                    '`port_id` = ? AND `mac_address` = ? AND `vlan_id` = ? and `device_id` = ?',
-                    array($entry['port_id'], $entry['mac_address'], $entry['vlan_id'], $entry['device_id'])
-                );
-                echo '-';
-        }
         echo PHP_EOL;
     }
+
+    // Delete old entries from the database
+    foreach ($existing_fdbs as $vlan_id => $entries) {
+        foreach ($entries as $entry) {
+            dbDelete(
+                'ports_fdb',
+                '`port_id` = ? AND `mac_address` = ? AND `vlan_id` = ? and `device_id` = ?',
+                array($entry['port_id'], $entry['mac_address'], $entry['vlan_id'], $entry['device_id'])
+            );
+            d_echo("Deleting: {$entry['mac_address']}\n", '-');
+        }
+    }
 }
-unset($existing_fdbs, $portid_dict, $vlans_dict);
+
+unset(
+    $vlan_entry,
+    $vlans_by_id,
+    $existing_fdbs,
+    $portid_dict,
+    $vlans_dict,
+    $insert,
+    $sql_result,
+    $vlans,
+    $port,
+    $fdbPort_table,
+    $entries
+);

includes/discovery/fdb-table/ios.inc.php

@@ -39,7 +39,7 @@ foreach ($vtpdomains as $vtpdomain_id => $vtpdomain) {
                 $port_id = $portid_dict[$dot1dBasePort];
                 $vlan_id = isset($vlans_dict[$vlan_raw]) ? $vlans_dict[$vlan_raw] : 0;
                 $insert[$vlan_id][$mac_address]['port_id'] = $port_id;
-                d_echo("vlan $vlan mac $mac_address port ($dot1dBasePort) $port_id\n");
+                d_echo("vlan $vlan_id mac $mac_address port ($dot1dBasePort) $port_id\n");
             }
 
             unset($device_vlan);

includes/snmp.inc.php

@@ -561,7 +561,7 @@ function snmpwalk_group($device, $oid, $mib = '', $depth = 1, $array = array())
         // merge the parts into an array, creating keys if they don't exist
         $tmp = &$array;
         foreach ($parts as $part) {
-            $tmp = &$tmp[$part];
+            $tmp = &$tmp[trim($part, '"')];
         }
         $tmp = trim($value, "\" \n\r"); // assign the value as the leaf
     }

misc/db_schema.yaml

@@ -630,6 +630,7 @@ ipv4_addresses:
 ipv4_mac:
   Columns:
     context_name: { Field: context_name, Type: varchar(128), 'Null': true, Default: null, Extra: '' }
+    device_id: { Field: device_id, Type: int(11), 'Null': true, Default: null, Extra: '' }
     ipv4_address: { Field: ipv4_address, Type: varchar(32), 'Null': false, Default: null, Extra: '' }
     mac_address: { Field: mac_address, Type: varchar(32), 'Null': false, Default: null, Extra: '' }
     port_id: { Field: port_id, Type: int(11), 'Null': false, Default: null, Extra: '' }

sql-schema/199.sql

@@ -0,0 +1 @@
+ALTER TABLE `ipv4_mac` ADD `device_id` INT NULL AFTER `port_id`;