Bad calculations?

crep has asked for the wisdom of the Perl Monks concerning the following question:

Hello all!

My below code is basically a scaled down version on MRTG (multi router traffic grapher). It grabs values by snmp, then updates a database and puts thoses values in a file I can include with php. However, these values are drastically wrong. See, I grab "ifInOctets", then snmp value for bytes that came in. But then, when I divide the delta over the time since last check, the value I get is way too high (up to 40 Megs.) That is impossible, the max for 100MBit network is 12.5 MBytes. So I thought that perhaps it actually was bits, not bytes. So I devided it by 8, right? Well, the value I get back is too small. MRTG reports about 400 bytes, I get about 70.

#!/usr/bin/perl -w

my $etime=0;
my $eth0in="";
my $eth0out="";
my $lo="";
my $petime=0;
my $peth0in=0;
my $peth0out=0;
my $plo=0;
my $tmp=0;
my $i=0;
my $n=1;

#NOTE!!! snmp counters reset at 4,294,967,295!

for (;;) {
   $i=$i+1;
   $etime=time;

   system("snmpwalk localhost public interfaces.ifTable.ifEntry.ifInOc
+tets.3 >/tmp/eth0in");
   system("snmpwalk localhost public interfaces.ifTable.ifEntry.ifOutO
+ctets.3 >/tmp/eth0out");
   system("snmpwalk localhost public interfaces.ifTable.ifEntry.ifOutO
+ctets.1 >/tmp/loout");

   open(fileIN,"/tmp/eth0in") or die("Cannot open /tmp/eth0in: $!");
   my @fileData = <fileIN>;
   close(fileIN);

   foreach $line (@fileData)
   {
       chomp($line);
       $eth0in=substr($line,54,10);
   }

   open(fileIN,"/tmp/eth0out") or die("Cannot open /tmp/eth0out: $!");
   @fileData = <fileIN>;
   close(fileIN);

   foreach $line (@fileData)
   {
       chomp($line);
       $eth0out=substr($line,54,10);
   }

   open(fileIN,"/tmp/loout") or die("Cannot open /tmp/loout: $!");
   @fileData = <fileIN>;
   close(fileIN);

   foreach $line (@fileData)
   {
       chomp($line);
       $lo=substr($line,54,10);
   }

   $eth0out = 0 + $eth0out;
   $eth0in  = 0 + $eth0in;
   $lo      = 0 + $lo;

   #time
   $tmp=$etime;
   $etime=$etime-$petime;
   $petime=$tmp;

   #eth0in
   if ($eth0in<$peth0in) { #the counter was reset
      $tmp=$eth0in;
      $eth0in=$eth0in + 4294967295;
      $eth0in=$eth0in-$peth0in;
      $eth0in=$eth0in/$etime;
      $eth0in=$eth0in/8; #bytes per second
      $peth0in=$tmp;
   } else {
      $tmp=$eth0in;
      $eth0in=$eth0in-$peth0in;
      $peth0in=$tmp;
      $eth0in=$eth0in/$etime;
      $eth0in=$eth0in/8; #bytes per second
   }

   if ($eth0out<$peth0out) { #the counter was reset
      $tmp=$eth0out;
      $eth0out=$eth0out + 4294967295;
      $eth0out=$eth0out-$peth0out;
      $eth0out=$eth0out/$etime;
      $eth0out=$eth0out/8; #bytes per second
      $peth0out=$tmp;
   } else {
      $tmp=$eth0out;
      $eth0out=$eth0out-$peth0out;
      $peth0out=$tmp;
      $eth0out=$eth0out/$etime;
      $eth0out=$eth0out/8; #bytes per second
   }

   #lo
   if ($lo<$plo) { #the counter was reset
      $tmp=$lo;
      $lo=$lo + 4294967295;
      $lo=$lo-$plo;
      $lo=$lo/$etime;
      $lo=$lo/8; #bytes per second
      $plo=$tmp;
   } else {
      $tmp=$lo;
      $lo=$lo-$plo;
      $plo=$tmp;
      $lo=$lo/$etime;
      $lo=$lo/8; #bytes per second
   }

   open(fileOUT, ">int.php") or dienice("Can't open log.txt for writin
+g: $!");
      print fileOUT "<?php\n\$eth0in=\"$eth0in\";\n\$eth0out=\"$eth0ou
+t\";\n\$lo=\"$lo\";\n?>\n";
   close(fileOUT);

   $etime=time;
   if ($n!=1) {system("rrdupdate sysgraphint.rrd $etime:$eth0in:$eth0o
+ut:$lo");}
   $n=$n+1;
   sleep(30);
}
[download]

So, great geniuses of the world, where have a gone wrong???

Thank you very much,
Jack C
jack@crepinc.com

Comment on Bad calculations? Download Code

Replies are listed 'Best First'.
Re: Bad calculations? by Eimi Metamorphoumai (Deacon) on Jul 26, 2004 at 17:09 UTC
First of all, your code is very repetitive--you're doing the same thing three times. You might find it better to put most of the code into a subroutine that could get called three times with different values for your different interfaces. There's also a lot of potential cleanup that could be done. But I think your real problem is that when the counter resets, you're setting the previous value (say, $peth0in) too late, after the division. So you're setting it to the number of bytes per second, not the total number so far.	[reply]
Re^2: Bad calculations? by crep (Novice) on Jul 26, 2004 at 17:37 UTC
Yes, I'm sorry about the bad coding. But I'm not sure what you're saying about setting the previous values too early... Before I do any calculation with $eth0in, say, I set $tmp to that value, and set $peth0in to $tmp at the end. So in essance, we still get the original (bytes, not b/s) value. Well, at least that's how I designed it to work, maybe it's not? Thanks,Jack C jack@crepinc.com	[reply]
Re: Bad calculations? by waswas-fng (Curate) on Jul 26, 2004 at 17:29 UTC
In addition to simplifying your code with subs, you could consider using SNMP to grab the stats. This would simplify the retrieval process for you (without scraping the output of an external command that may change in the future). -Waswas	[reply]
Re^2: Bad calculations? by crep (Novice) on Jul 26, 2004 at 17:38 UTC
The system commands that I run at the beggining do get the values through snmp. I understand your point though about using a module to do it. Thanks, -Jack C jack@crepinc.com	[reply]
Re: Bad calculations? by graff (Chancellor) on Jul 27, 2004 at 03:35 UTC
I was going to try boiling your code down a bit, as per comments made previously, but I ran into some mysterious things, and I felt I'd have to ask... Each of the three "snmpwalk" system calls at the top of the "for (;;;)" loop creates an output file in /tmp, and you read each whole file into an array; then you use only the last value (the last line) from each file. Is it true that you only need to use the last line of output from each run of "snmpwalk"? If that's true (and if there weren't a better way to do this with a module -- which there probably is, I'm guessing), then you'd save yourself some trouble using a more suitable command line with backticks; something like: my $cmdoutput = `snmpwalk blah blah blah \| tail -1`; my $snmpval = substr( $cmdoutput, 54, 10 ); [download] (and are you really sure that substr, with those parameter values, is always doing the right thing? Using the right module would avoid the risk of messing up on this.) Then there's that funny arithmetic with $etime and $petime. Both start out as 0, but $etime gets set to current system time at the top of each iteration, then gets reset to that current system time minus $petime, then $petime gets set to the system time that used to be the value of $etime. Huh?? Have you actually gone through that step by step, and is that really what you want? Think about it. `loop iter. step etime= petime= ---------------------------------------- 1 at "#time" time1 0 at "#eth0in" time1 time1 2 at "#time" time2 time1 at "#eth0in" time2-time1 time2 3 at "#time" time3 time2 at "#eth0in" time3-time2 time3` [download] and so on. I'm not sure about what you're really after, but I'd say that having $etime equal to system time (seconds since the epoch) will make the results of the first iteration very different from later ones, where $etime represents seconds elapsed since the previous iteration. It might be best for you to step through this with the perl debugger, and watch what happens to the values to see whether it is what you expect and want. As for reducing the code, think about a hash of hashes that stores the things you need to have for each ethernet interface: `%iface = ( eth0in => { param => 'InOctets.3', now => 0, prv => 0 }, eth0out=> { param => 'OutOctets.3', now => 0, prv => 0 }, loout => { param => 'OutOctets.1', now => 0, prv => 0 }, ); # use $iface{eth0in}{param} to run snmpwalk for that item, # use $iface{ethoin}{now} to store snmpwalk output, etc...` [download] Then loop over the hash elements. This way, if you add another interface to track later on, you only need to add an item to the hash, and the rest of the code shouldn't need to change at all.	[reply] [d/l] [select]