An interesting bug in age-old Perl Net::SNMP code / 2018-03-01

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_PORT1

Using 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};
        }
    }
}