export store_arp and store_node

Netdisco/lib/App/Netdisco/Core/Arpnip.pm

@@ -11,7 +11,7 @@ use Net::MAC;
 
 use base 'Exporter';
 our @EXPORT = ();
-our @EXPORT_OK = qw/ do_arpnip /;
+our @EXPORT_OK = qw/ do_arpnip check_mac store_arp /;
 our %EXPORT_TAGS = (all => \@EXPORT_OK);
 
 =head1 NAME
@@ -44,13 +44,12 @@ sub do_arpnip {
       return;
   }
 
-  my (@v4, @v6);
   my $port_macs = get_port_macs($device);
 
   # get v4 arp table
-  push @v4, _get_arps($device, $port_macs, $snmp->at_paddr, $snmp->at_netaddr);
+  my @v4 = _get_arps($device, $port_macs, $snmp->at_paddr, $snmp->at_netaddr);
   # get v6 neighbor cache
-  push @v6, _get_arps($device, $port_macs, $snmp->ipv6_n2p_mac, $snmp->ipv6_n2p_addr);
+  my @v6 = _get_arps($device, $port_macs, $snmp->ipv6_n2p_mac, $snmp->ipv6_n2p_addr);
 
   # get directly connected networks
   my @subnets = _gather_subnets($device, $snmp);
@@ -62,28 +61,15 @@ sub do_arpnip {
   my $now = 'to_timestamp('. (join '.', gettimeofday) .')';
 
   # update node_ip with ARP and Neighbor Cache entries
-  _store_arp(@$_, $now) for @v4;
+  store_arp(@$_, $now) for @v4;
   debug sprintf ' [%s] arpnip - processed %s ARP Cache entries',
     $device->ip, scalar @v4;
 
-  _store_arp(@$_, $now) for @v6;
+  store_arp(@$_, $now) for @v6;
   debug sprintf ' [%s] arpnip - processed %s IPv6 Neighbor Cache entries',
     $device->ip, scalar @v6;
 
-  # update subnets with new networks
+  _store_subnet($_, $now) for @subnets;
-  foreach my $cidr (@subnets) {
-      schema('netdisco')->txn_do(sub {
-          schema('netdisco')->resultset('Subnet')->update_or_create(
-          {
-            net => $cidr,
-            last_discover => \$now,
-          },
-          {
-            order_by => 'net',
-            for => 'update',
-          });
-      });
-  }
   debug sprintf ' [%s] arpnip - processed %s Subnet entries',
     $device->ip, scalar @subnets;
 }
@@ -96,24 +82,55 @@ sub _get_arps {
   while (my ($arp, $node) = each %$paddr) {
       my $ip = $netaddr->{$arp};
       next unless defined $ip;
-      my $arp = _check_arp($device, $port_macs, $node, $ip);
+      push @arps, [$node, $ip, hostname_from_ip($ip)]
-      push @arps, [@$arp, hostname_from_ip($ip)]
+        if check_mac($device, $node, $port_macs);
-        if ref [] eq ref $arp;
   }
 
   return @arps;
 }
 
-# checks any arpnip entry for sanity and adds to DB
+=head2 check_mac( $device, $node, $port_macs? )
-sub _check_arp {
+
-  my ($device, $port_macs, $node, $ip) = @_;
+Given a Device database object and a MAC address, perform various sanity
+checks which need to be done before writing an ARP/Neighbor entry to the
+database storage.
+
+Returns false, and logs a debug level message, if the checks fail.
+
+Returns a true value if these checks pass:
+
+=over 4
+
+=item *
+
+MAC address is not malformed
+
+=item *
+
+MAC address is not broadcast, CLIP, VRRP or HSRP
+
+=item *
+
+MAC address does not belong to an interface on C<$device>
+
+=back
+
+Optionally pass a cached set of Device port MAC addresses as the fourth
+argument, or else C<check_mac> will retrieve this for itself from the
+database.
+
+=cut
+
+sub check_mac {
+  my ($device, $node, $port_macs) = @_;
+  $port_macs ||= get_port_macs($device);
   my $mac = Net::MAC->new(mac => $node, 'die' => 0, verbose => 0);
 
   # incomplete MAC addresses (BayRS frame relay DLCI, etc)
   if ($mac->get_error) {
       debug sprintf ' [%s] arpnip - mac [%s] malformed - skipping',
         $device->ip, $node;
-      return;
+      return 0;
   }
   else {
       # lower case, hex, colon delimited, 8-bit groups
@@ -121,53 +138,73 @@ sub _check_arp {
   }
 
   # broadcast MAC addresses
-  return if $node eq 'ff:ff:ff:ff:ff:ff';
+  return 0 if $node eq 'ff:ff:ff:ff:ff:ff';
 
   # CLIP
-  return if $node eq '00:00:00:00:00:01';
+  return 0 if $node eq '00:00:00:00:00:01';
 
   # VRRP
   if (index($node, '00:00:5e:00:01:') == 0) {
       debug sprintf ' [%s] arpnip - VRRP mac [%s] - skipping',
         $device->ip, $node;
-      return;
+      return 0;
   }
 
   # HSRP
   if (index($node, '00:00:0c:07:ac:') == 0) {
       debug sprintf ' [%s] arpnip - HSRP mac [%s] - skipping',
         $device->ip, $node;
-      return;
+      return 0;
   }
 
   # device's own MACs
   if (exists $port_macs->{$node}) {
       debug sprintf ' [%s] arpnip - mac [%s] is device port - skipping',
         $device->ip, $node;
-      return;
+      return 0;
   }
 
-  return [$node, $ip];
+  return 1;
 }
 
-# add arp cache entry to the node_ip table
+=head2 store_arp( $mac, $ip, $name, $now? )
-sub _store_arp {
+
+Stores a new entry to the C<node_ip> table with the given MAC, IP (v4 or v6)
+and DNS host name.
+
+Will mark old entries for this IP as no longer C<active>.
+
+Optionally a literal string can be passed in the fourth argument for the
+C<time_last> timestamp, otherwise the current timestamp (C<now()>) is used.
+
+=cut
+
+sub store_arp {
   my ($mac, $ip, $name, $now) = @_;
+  $now ||= 'now()';
 
   schema('netdisco')->txn_do(sub {
     my $current = schema('netdisco')->resultset('NodeIp')
       ->search({ip => $ip, -bool => 'active'})
-      ->search(undef, {order_by => [qw/mac ip/], for => 'update'});
+      ->search(undef, {
-    my $count = scalar $current->all;
+        columns => [qw/mac ip/],
+        order_by => [qw/mac ip/],
+        for => 'update'
+      });
+    $current->first; # lock rows
     $current->update({active => \'false'});
 
     schema('netdisco')->resultset('NodeIp')
       ->search({'me.mac' => $mac, 'me.ip' => $ip})
-      ->search(undef, {order_by => [qw/mac ip/], for => 'update'})
+      ->update_or_create(
-      ->update_or_create({
+      {
         dns => $name,
         active => \'true',
         time_last => \$now,
+      },
+      {
+        order_by => [qw/mac ip/],
+        for => 'update',
       });
   });
 }
@@ -200,4 +237,18 @@ sub _gather_subnets {
   return @subnets;
 }
 
+# update subnets with new networks
+sub _store_subnet {
+  my ($subnet, $now) = @_;
+
+  schema('netdisco')->txn_do(sub {
+    schema('netdisco')->resultset('Subnet')->update_or_create(
+    {
+      net => $subnet,
+      last_discover => \$now,
+    },
+    { for => 'update' });
+  });
+}
+
 1;

Netdisco/lib/App/Netdisco/Core/Macsuck.pm

@@ -4,11 +4,16 @@ use Dancer qw/:syntax :script/;
 use Dancer::Plugin::DBIC 'schema';
 
 use App::Netdisco::Util::PortMAC ':all';
+use App::Netdisco::Util::SNMP 'snmp_comm_reindex';
 use Time::HiRes 'gettimeofday';
 
 use base 'Exporter';
 our @EXPORT = ();
-our @EXPORT_OK = qw/ do_macsuck /;
+our @EXPORT_OK = qw/
+  do_macsuck
+  store_node
+  store_wireless_client_info
+/;
 our %EXPORT_TAGS = (all => \@EXPORT_OK);
 
 =head1 NAME
@@ -55,31 +60,29 @@ sub do_macsuck {
   my $total_nodes = 0;
 
   # do this before we start messing with the snmp community string
-  _wireless_client_info($device, $snmp, $now)
+  store_wireless_client_info($device, $snmp, $now);
-    if setting('store_wireless_client');
 
+  # cache the device ports to save hitting the database for many single rows
+  my $device_ports = {map {($_->port => $_)} $device->ports->all}; 
   my $port_macs = get_port_macs($device);
-  my $fwtable = { 0 => _walk_fwtable($device, $snmp, $port_macs) };
 
+  # get forwarding table data via basic snmp connection
+  my $fwtable = { 0 => _walk_fwtable($device, $snmp, $port_macs, $device_ports) };
+
+  # ...then per-vlan if supported
   my @vlan_list = _get_vlan_list($device, $snmp);
   foreach my $vlan (@vlan_list) {
-      _snmp_comm_reindex($snmp, $vlan);
+      snmp_comm_reindex($snmp, $vlan);
-      $fwtable->{$vlan} = _walk_fwtable($device, $snmp, $port_macs);
+      $fwtable->{$vlan} = _walk_fwtable($device, $snmp, $port_macs, $device_ports);
   }
 
-  # cache the device ports so we can look at them for each mac found
+  # now it's time to call store_node for every node discovered
-  my $uplink_cache = {};
-  my $ports = $device->ports;
-  while (my $p = $ports->next) {
-      $uplink_cache->{ $p->port } = $p->get_column('maybe_uplink');
-  }
-
-  # now it's time to call _store_node for every node discovered
   # on every port on every vlan on every device.
 
-  foreach my $vlan (sort keys %$fwtable) {
+  # reverse sort allows vlan 0 entries to be added as fallback
+  foreach my $vlan (reverse sort keys %$fwtable) {
       foreach my $port (keys %{ $fwtable->{$vlan} }) {
-          if ($uplink_cache->{$port}) {
+          if ($device_ports->{$port}->is_uplink) {
               debug sprintf
                 ' [%s] macsuck - port %s is uplink, topo broken - skipping.',
                 $device->ip, $port;
@@ -89,19 +92,13 @@ sub do_macsuck {
           debug sprintf ' [%s] macsuck - port %s vlan %s : %s nodes',
             $device->ip, $port, $vlan, scalar keys %{ $fwtable->{$vlan}->{$port} };
 
-          MAC: foreach my $mac (keys %{ $fwtable->{$vlan}->{$port} }) {
+          foreach my $mac (keys %{ $fwtable->{$vlan}->{$port} }) {
-              # skip if vlan is 0 and mac exists in another vlan
+              # remove vlan 0 entry for this MAC addr
-              if ($vlan == 0) {
+              delete $fwtable->{0}->{$_}->{$mac}
-                  foreach my $zv (keys %$fwtable) {
+                for keys %{ $fwtable->{0} };
-                      next if $zv == 0;
-                      foreach my $zp (keys %{ $fwtable->{$zv} }) {
-                          next MAC if exists $fwtable->{$zv}->{$zp}->{$mac};
-                      }
-                  }
-              }
 
               ++$total_nodes;
-              _store_node($device->ip, $vlan, $port, $mac, $now);
+              store_node($device->ip, $vlan, $port, $mac, $now);
           }
       }
   }
@@ -111,66 +108,27 @@ sub do_macsuck {
   $device->update({last_macsuck => \$now});
 }
 
-sub _wireless_client_info {
+=head2 store_node( $ip, $vlan, $port, $mac, $now? )
-  my ($device, $snmp, $now) = @_;
 
-  debug sprintf ' [%s] macsuck - wireless client info', $device->ip;
+Writes a fresh entry to the Netdisco C<node> database table. Will mark old
+entries for this data as no longer C<active>.
 
-  my $cd11_txrate = $snmp->cd11_txrate;
+All four fields in the tuple are required. If you don't know the VLAN ID,
-  return unless $cd11_txrate and scalar keys %$cd11_txrate;
+Netdisco supports using ID "0".
 
-  my $cd11_rateset = $snmp->cd11_rateset();
+Optionally, a fifth argument can be the literal string passed to the time_last
-  my $cd11_uptime  = $snmp->cd11_uptime();
+field of the database record. If not provided, it defauls to C<now()>.
-  my $cd11_sigstrength = $snmp->cd11_sigstrength();
-  my $cd11_sigqual = $snmp->cd11_sigqual();
-  my $cd11_mac     = $snmp->cd11_mac();
-  my $cd11_port    = $snmp->cd11_port();
-  my $cd11_rxpkt   = $snmp->cd11_rxpkt();
-  my $cd11_txpkt   = $snmp->cd11_txpkt();
-  my $cd11_rxbyte  = $snmp->cd11_rxbyte();
-  my $cd11_txbyte  = $snmp->cd11_txbyte();
-  my $cd11_ssid    = $snmp->cd11_ssid();
 
-  while (my ($idx, $txrates) = each %$cd11_txrate) {
+=cut
-      my $rates = $cd11_rateset->{$idx};
-      my $mac   = $cd11_mac->{$idx};
-      next unless defined $mac; # avoid null entries
-            # there can be more rows in txrate than other tables
 
-      my $txrate  = defined $txrates->[$#$txrates]
+sub store_node {
-        ? int($txrates->[$#$txrates])
-        : undef;
-
-      my $maxrate = defined $rates->[$#$rates]
-        ? int($rates->[$#$rates])
-        : undef;
-
-      schema('netdisco')->txn_do(sub {
-        schema('netdisco')->resultset('NodeWireless')
-          ->search({ mac => $mac })
-          ->update_or_create({
-            txrate  => $txrate,
-            maxrate => $maxrate,
-            uptime  => $cd11_uptime->{$idx},
-            rxpkt   => $cd11_rxpkt->{$idx},
-            txpkt   => $cd11_txpkt->{$idx},
-            rxbyte  => $cd11_rxbyte->{$idx},
-            txbyte  => $cd11_txbyte->{$idx},
-            sigqual => $cd11_sigqual->{$idx},
-            sigstrength => $cd11_sigstrength->{$idx},
-            ssid    => ($cd11_ssid->{$idx} || 'unknown'),
-            time_last => \$now,
-          }, { for => 'update' });
-      });
-  }
-}
-
-sub _store_node {
   my ($ip, $vlan, $port, $mac, $now) = @_;
+  $now ||= 'now()';
 
   schema('netdisco')->txn_do(sub {
     my $nodes = schema('netdisco')->resultset('Node');
 
+    # TODO: probably needs changing if we're to support VTP domains
     my $old = $nodes->search(
       {
         mac => $mac,
@@ -182,11 +140,11 @@ sub _store_node {
         },
       });
 
-    # selecting the data triggers row lock
+    # lock rows,
     # and get the count so we know whether to set time_recent
     my $old_count = scalar $old->search(undef,
       {
-        # ORDER BY FOR UPDATE avoids need for table lock
+        columns => [qw/switch vlan port mac/],
         order_by => [qw/switch vlan port mac/],
         for => 'update',
       })->all;
@@ -200,13 +158,12 @@ sub _store_node {
         'me.mac' => $mac,
       },
       {
-        # ORDER BY FOR UPDATE avoids need for table lock
         order_by => [qw/switch vlan port mac/],
         for => 'update',
       });
 
-    # trigger row lock
+    # lock rows
-    $new->search({vlan => [$vlan, 0, undef]})->all;
+    $new->search({vlan => [$vlan, 0, undef]})->first;
 
     # upgrade old schema
     $new->search({vlan => [$vlan, 0, undef]})
@@ -222,21 +179,6 @@ sub _store_node {
   });
 }
 
-# make a new snmp connection to $device using community indexing
-sub _snmp_comm_reindex {
-  my ($snmp, $vlan) = @_;
-
-  my $ver  = $snmp->snmp_ver;
-  my $comm = $snmp->snmp_comm;
-
-  if ($ver == 3) {
-      $snmp->update(Context => "vlan-$vlan");
-  }
-  else {
-      $snmp->update(Community => $comm . '@' . $vlan);
-  }
-}
-
 # return a list of vlan numbers which are OK to macsuck on this device
 sub _get_vlan_list {
   my ($device, $snmp) = @_;
@@ -314,7 +256,7 @@ sub _get_vlan_list {
 # walks the forwarding table (BRIDGE-MIB) for the device and returns a
 # table of node entries.
 sub _walk_fwtable {
-  my ($device, $snmp, $port_macs) = @_;
+  my ($device, $snmp, $port_macs, $device_ports) = @_;
   my $cache = {};
 
   my $fw_mac   = $snmp->fw_mac;
@@ -323,13 +265,6 @@ sub _walk_fwtable {
   my $bp_index = $snmp->bp_index;
   my $interfaces = $snmp->interfaces;
 
-  # cache the device ports so we can look at them for each mac found
-  my $ports_cache = {};
-  my $ports = $device->ports;
-  while (my $p = $ports->next) {
-      $ports_cache->{ $p->port } = $p;
-  }
-
   # to map forwarding table port to device port we have
   #   fw_port -> bp_index -> interfaces
 
@@ -370,7 +305,7 @@ sub _walk_fwtable {
       }
 
       # this uses the cached $ports resultset to limit hits on the db
-      my $device_port = $ports_cache->{$port};
+      my $device_port = $device_ports->{$port};
 
       unless (defined $device_port) {
           debug sprintf
@@ -385,9 +320,9 @@ sub _walk_fwtable {
       # we have several ways to detect "uplink" port status:
       #  * a neighbor was discovered using CDP/LLDP
       #  * a mac addr is seen which belongs to any device port/interface
-      #  * (TODO) admin sets is_uplink on the device_port
+      #  * (TODO) admin sets is_uplink_admin on the device_port
 
-      if ($device_port->maybe_uplink) {
+      if ($device_port->is_uplink) {
           if (my $neighbor = $device_port->neighbor) {
               debug sprintf
                 ' [%s] macsuck %s - port %s has neighbor %s - skipping.',
@@ -419,7 +354,7 @@ sub _walk_fwtable {
 
           debug sprintf ' [%s] macsuck %s - port %s is probably an uplink',
             $device->ip, $mac, $port;
-          $device_port->update({maybe_uplink => \'true'});
+          $device_port->update({is_uplink => \'true'});
 
           # when there's no CDP/LLDP, we only want to gather macs at the
           # topology edge, hence skip ports with known device macs.
@@ -441,4 +376,89 @@ sub _walk_fwtable {
   return $cache;
 }
 
+=head2 store_wireless_client_info( $device, $snmp, $now? )
+
+Given a Device database object, and a working SNMP connection, connect to a
+device and discover 802.11 related information for all connected wireless
+clients.
+
+If the device doesn't support the 802.11 MIBs, then this will silently return.
+
+If the device does support the 802.11 MIBs but Netdisco's configuration
+does not permit polling (C<store_wireless_client> must be true) then a debug
+message is logged and the subroutine returns.
+
+Otherwise, client information is gathered and stored to the database.
+
+Optionally, a third argument can be the literal string passed to the time_last
+field of the database record. If not provided, it defauls to C<now()>.
+
+=cut
+
+sub store_wireless_client_info {
+  my ($device, $snmp, $now) = @_;
+  $now ||= 'now()';
+
+  my $cd11_txrate = $snmp->cd11_txrate;
+  return unless $cd11_txrate and scalar keys %$cd11_txrate;
+
+  if (setting('store_wireless_client')) {
+      debug sprintf ' [%s] macsuck - gathering wireless client info',
+        $device->ip;
+  }
+  else {
+      debug sprintf ' [%s] macsuck - dot11 info available but skipped due to config',
+        $device->ip;
+      return;
+  }
+
+  my $cd11_rateset = $snmp->cd11_rateset();
+  my $cd11_uptime  = $snmp->cd11_uptime();
+  my $cd11_sigstrength = $snmp->cd11_sigstrength();
+  my $cd11_sigqual = $snmp->cd11_sigqual();
+  my $cd11_mac     = $snmp->cd11_mac();
+  my $cd11_port    = $snmp->cd11_port();
+  my $cd11_rxpkt   = $snmp->cd11_rxpkt();
+  my $cd11_txpkt   = $snmp->cd11_txpkt();
+  my $cd11_rxbyte  = $snmp->cd11_rxbyte();
+  my $cd11_txbyte  = $snmp->cd11_txbyte();
+  my $cd11_ssid    = $snmp->cd11_ssid();
+
+  while (my ($idx, $txrates) = each %$cd11_txrate) {
+      my $rates = $cd11_rateset->{$idx};
+      my $mac   = $cd11_mac->{$idx};
+      next unless defined $mac; # avoid null entries
+            # there can be more rows in txrate than other tables
+
+      my $txrate  = defined $txrates->[$#$txrates]
+        ? int($txrates->[$#$txrates])
+        : undef;
+
+      my $maxrate = defined $rates->[$#$rates]
+        ? int($rates->[$#$rates])
+        : undef;
+
+      schema('netdisco')->txn_do(sub {
+        schema('netdisco')->resultset('NodeWireless')
+          ->search({ 'me.mac' => $mac })
+          ->update_or_create({
+            txrate  => $txrate,
+            maxrate => $maxrate,
+            uptime  => $cd11_uptime->{$idx},
+            rxpkt   => $cd11_rxpkt->{$idx},
+            txpkt   => $cd11_txpkt->{$idx},
+            rxbyte  => $cd11_rxbyte->{$idx},
+            txbyte  => $cd11_txbyte->{$idx},
+            sigqual => $cd11_sigqual->{$idx},
+            sigstrength => $cd11_sigstrength->{$idx},
+            ssid    => ($cd11_ssid->{$idx} || 'unknown'),
+            time_last => \$now,
+          }, {
+            order_by => [qw/mac ssid/],
+            for => 'update',
+          });
+      });
+  }
+}
+
 1;

Netdisco/lib/App/Netdisco/DB/Result/DevicePort.pm

@@ -53,7 +53,7 @@ __PACKAGE__->add_columns(
   { data_type => "text", is_nullable => 1 },
   "manual_topo",
   { data_type => "bool", is_nullable => 0, default_value => \"false" },
-  "maybe_uplink",
+  "is_uplink",
   { data_type => "bool", is_nullable => 1 },
   "vlan",
   { data_type => "text", is_nullable => 1 },

Netdisco/lib/App/Netdisco/Daemon/Worker/Interactive.pm

@@ -61,7 +61,7 @@ sub close_job {
 
   try {
       schema('netdisco')->resultset('Admin')
-        ->find($job->job)
+        ->find($job->job, {for => 'update'})
         ->update({
           status => $status,
           log => $log,

Netdisco/lib/App/Netdisco/Daemon/Worker/Poller.pm

@@ -63,7 +63,7 @@ sub close_job {
 
   try {
       schema('netdisco')->resultset('Admin')
-        ->find($job->job)
+        ->find($job->job, {for => 'update'})
         ->update({
           status => $status,
           log => $log,

Netdisco/lib/App/Netdisco/Daemon/Worker/Poller/Arpnip.pm

@@ -5,7 +5,7 @@ use Dancer::Plugin::DBIC 'schema';
 
 use App::Netdisco::Util::SNMP 'snmp_connect';
 use App::Netdisco::Util::Device 'get_device';
-use App::Netdisco::Core::Arpnip ':all';
+use App::Netdisco::Core::Arpnip 'do_arpnip';
 use App::Netdisco::Daemon::Util ':all';
 
 use NetAddr::IP::Lite ':lower';

Netdisco/lib/App/Netdisco/Util/SNMP.pm

@@ -10,7 +10,7 @@ use Path::Class 'dir';
 use base 'Exporter';
 our @EXPORT = ();
 our @EXPORT_OK = qw/
-  snmp_connect snmp_connect_rw
+  snmp_connect snmp_connect_rw snmp_comm_reindex
 /;
 our %EXPORT_TAGS = (all => \@EXPORT_OK);
 
@@ -152,4 +152,25 @@ sub _build_mibdirs {
              @{ setting('mibdirs') || [] };
 }
 
+=head2 snmp_comm_reindex( $snmp, $vlan )
+
+Takes an established L<SNMP::Info> instance and makes a fresh connection using
+community indexing, with the given C<$vlan> ID. Works for all SNMP versions.
+
+=cut
+
+sub snmp_comm_reindex {
+  my ($snmp, $vlan) = @_;
+
+  my $ver  = $snmp->snmp_ver;
+  my $comm = $snmp->snmp_comm;
+
+  if ($ver == 3) {
+      $snmp->update(Context => "vlan-$vlan");
+  }
+  else {
+      $snmp->update(Community => $comm . '@' . $vlan);
+  }
+}
+
 1;