From f2f5d908bbcc53d4ec24a76a3a22e4d7790efbdd Mon Sep 17 00:00:00 2001
From: Christian Ramseyer <ramseyer@netnea.com>
Date: Sun, 26 Jan 2020 22:08:49 +0100
Subject: [PATCH] Fix regression from PR #680

Hi @ollyg! Unfortunately I have found some issues with the code we
finally released in #680:

* get_port_macs expects an array ref but values() returns array,
  so the code was never called due to the return unless... check
* When fw_mac_list had exactly two entries, only the second value
  was bound as a scalar to the parameter. This is probably due
  to the shorthand bind formats described in
  https://metacpan.org/pod/DBIx::Class::ResultSet#DBIC-BIND-VALUES,
  but I'm not a 100% on this.
* return unless now checks for an entry in the list, with the old
  check the statement was also executed for empty lists

In cases where only the device(_port)?.mac lookup worked for uplink
detection, users of 02.044005 - 010 might get a lot of uplinks not
labeled as such.
---
 lib/App/Netdisco/Util/PortMAC.pm                | 8 +++++---
 lib/App/Netdisco/Worker/Plugin/Macsuck/Nodes.pm | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/App/Netdisco/Util/PortMAC.pm b/lib/App/Netdisco/Util/PortMAC.pm
index 348ffad2..320d2b82 100644
--- a/lib/App/Netdisco/Util/PortMAC.pm
+++ b/lib/App/Netdisco/Util/PortMAC.pm
@@ -29,13 +29,15 @@ addresses supplied as array reference
 =cut
 
 sub get_port_macs {
-    my ($fw_mac_list) = @_;
+    my ($fw_mac_list) = $_[0];
     my $port_macs = {};
-    return {} unless defined $fw_mac_list and ref [] eq ref $fw_mac_list;
+    return {} unless ref [] eq ref $fw_mac_list and @{$fw_mac_list} >= 1;
+
+    my $bindarray = [ { sqlt_datatype => "array" }, $fw_mac_list ];
 
     my $macs
         = schema('netdisco')->resultset('Virtual::PortMacs')->search({},
-        { bind => [$fw_mac_list], select => [ 'mac', 'ip' ], group_by => [ 'mac', 'ip' ] } );
+        { bind => [$bindarray], select => [ 'mac', 'ip' ], group_by => [ 'mac', 'ip' ] } );
     my $cursor = $macs->cursor;
     while ( my @vals = $cursor->next ) {
         $port_macs->{ $vals[0] } = $vals[1];
diff --git a/lib/App/Netdisco/Worker/Plugin/Macsuck/Nodes.pm b/lib/App/Netdisco/Worker/Plugin/Macsuck/Nodes.pm
index 43568193..d1c1a9f9 100644
--- a/lib/App/Netdisco/Worker/Plugin/Macsuck/Nodes.pm
+++ b/lib/App/Netdisco/Worker/Plugin/Macsuck/Nodes.pm
@@ -278,7 +278,8 @@ sub walk_fwtable {
     ? {} : $snmp->qb_fw_vlan;
   my $bp_index = $snmp->bp_index || {};
 
-  my $port_macs = get_port_macs( values %$fw_mac );
+  my @fw_mac_list = values %$fw_mac;
+  my $port_macs = get_port_macs(\@fw_mac_list);
 
   # to map forwarding table port to device port we have
   #   fw_port -> bp_index -> interfaces