2018-03-01
An interesting bug in age-old Perl Net::SNMP code
I recently noticed the network traffic statistics weren't updated correctly for the LAN interface of my Draytek Vigor 130 modem. These statistics were extracted using code that I originally started using at the computer science systems group somewhere in the previous decade. It's all Perl Net::SNMP and not very efficient. I don't know if I wrote it myself or copied from somewhere else, I do know a new bug was introduced. To understand the code it is important to realize that interface index numbers in SNMP are dynamic. Across a reboot a certain number can change. Interface names are static, but those are never used directly in SNMP. So to get from a static interface name to a dynamic interface index the interfaces.2.1.2 subtree (ifDescr) has to be fetched from the device and checked for the right names. To get the interface index from an snmp object identifier I used to use this bit of code:# find the current interface indices for the wanted ^ interfaces foreach my $oid (oid_lex_sort(keys(%table))) { if (oid_base_match($ifTable_ifDesc,$oid)){ # printf("%s => %s\n", $oid, $table{$oid}); if (defined $wantstuff{$table{$oid}}){ $wantstuff{$table{$oid}}{ifindex}=substr($oid,1+rindex($oid,'.')); # I am lazy. I fill a hash with the interface indices so I can # use it for lookups $findvlan{substr($oid,1+rindex($oid,'.'))}=$table{$oid}; # printf "Found ifindex %d for %s\n",$wantstuff{$table{$oid}}{ifindex},$table{$oid}; } } }But note how the current ifDesc subtree is from the modem:IF-MIB::ifDescr.1 = STRING: LAN IF-MIB::ifDescr.4 = STRING: VDSL IF-MIB::ifDescr.5 = STRING: Resrved IF-MIB::ifDescr.6 = STRING: IF-MIB::ifDescr.7 = STRING: IF-MIB::ifDescr.8 = STRING: IF-MIB::ifDescr.20.101.1 = STRING: WAN1 IF-MIB::ifDescr.21.101.1 = STRING: WAN2 IF-MIB::ifDescr.22.101.1 = STRING: LAN_PORT1Using that rindex function there are 4 instances of index 1. Which caused the very similar code looking for the ifInOctets, ifOutOctets and other counters to overwrite the result for index 1 with those from WAN1, WAN2 and LAN_PORT1. So that code is now improved, no more rindex but a well-defined use of length:# find the current interface indices for the wanted ^ interfaces foreach my $oid (oid_lex_sort(keys(%table))) { if (oid_base_match($ifTable_ifDesc,$oid)){ #printf("%s => %s\n", $oid, $table{$oid}); if (defined $wantstuff{$table{$oid}}){ my $intindex=substr($oid,length($ifTable_ifDesc)+1); #printf "Submatch found ifindex %d for %s\n",$intindex,$table{$oid}; $wantstuff{$table{$oid}}{ifindex}=$intindex; # I am lazy. I fill a hash with the interface indices so I can # use it for lookups $findvlan{$intindex}=$table{$oid}; #printf "Found ifindex %d for %s\n",$wantstuff{$table{$oid}}{ifindex},$table{$oid}; } } }