Network traffic plugin doesn't take into account integer overflows

Bug #615371 reported by Andreas Hasenack
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Landscape Client
Fix Released
High
Thomas Herve
landscape-client (Ubuntu)
Fix Released
Undecided
Unassigned
Jaunty
Fix Released
Undecided
Unassigned
Karmic
Fix Released
Undecided
Unassigned
Lucid
Fix Released
Undecided
Unassigned
Maverick
Fix Released
Undecided
Unassigned

Bug Description

The client sends the network traffic information as deltas, and sometimes these values are negative which indicates the data wrapped around. The client should take this into account.

Related branches

Revision history for this message
Thomas Herve (therve) wrote :

I think it's more problematic than that: a simple restart will make the value wrap around, because we're comparing the /proc/net/dev content before and after a restart.

Changed in landscape-client:
importance: Medium → High
Revision history for this message
Kevin McDermott (bigkevmcd) wrote :
Download full text (5.7 KiB)

There was a bit of discussion about this at the time in the IRC channel...

Aug 06 15:06:33 <andreas>>--bigkevmcd: so /proc/net/dev is rolling over? OR, the interface was reset (brought down and up again, something that resets the number of bytes sent/received)
Aug 06 15:06:41 <bigkevmcd>>andreas: /proc/net/dev is rolling over
Aug 06 15:06:51 <bigkevmcd>>andreas: so, we need to know the rollover point, and delta it accordingly
Aug 06 15:07:04 <andreas>>--bigkevmcd: I suspect that's arch dependent
Aug 06 15:07:16 <andreas>>--we could check how mrtg does it :)
Aug 06 15:07:18 <bigkevmcd>>andreas: what arch are the machines?
Aug 06 15:07:32 <andreas>>--we only support two
Aug 06 15:07:57 <andreas>>--Intel(R) Xeon(R) CPU E5410 @ 2.33GHz
Aug 06 15:07:58 <andreas>>--7.82GB RAM
Aug 06 15:07:58 <andreas>>--that's one
Aug 06 15:08:05 <andreas>>--Ubuntu 10.04.1 LTS (lucid)
Aug 06 15:08:09 <andreas>>--doesn't say the arch in the info page
Aug 06 15:08:12 <bigkevmcd>>pah
Aug 06 15:08:40 <andreas>>--on the "plus" side, the hardware page does list a lot of network interfaces
Aug 06 15:09:14 <bigkevmcd>>andreas: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=199054
Aug 06 15:09:23 <bigkevmcd>>andreas: that bug tells a story :-)
Aug 06 15:11:11 <jkakar>>---bigkevmcd: Bumping to bigint sounds like the right fix.
Aug 06 15:11:45 <bigkevmcd>>jkakar: the fix is up for review
Aug 06 15:11:51 <jkakar>>---bigkevmcd: Cool, checking it out.
Aug 06 15:13:40 <jkakar>>---+ data_point = api.create_traffic("eth0", now, -2814141641, -377168823)
Aug 06 15:13:51 <jkakar>>---bigkevmcd: ^^ Uhm. Those aren't big numbers are they?
Aug 06 15:14:10 <bigkevmcd>>jkakar: they're big enough to trigger the DataError
Aug 06 15:14:23 <jkakar>>---bigkevmcd: Shouldn't they be positive?
Aug 06 15:14:28 <bigkevmcd>>jkakar: they're deltas
Aug 06 15:14:37 <bigkevmcd>>jkakar: i.e. the difference between one data point, and another
Aug 06 15:14:41 <jkakar>>---bigkevmcd: Ah, right.
Aug 06 15:14:48 <jkakar>>---I know what a delta is. ;)
Aug 06 15:14:54 <bigkevmcd>>jkakar: phew ;-)
Aug 06 15:15:11 <bigkevmcd>>jkakar: there's a client side fix needed, but I think it needs a kernel fix
Aug 06 15:15:33 <bigkevmcd>>jkakar: or rather, we can client-side fix, or wait for the kernel fix
Aug 06 15:16:53 <jkakar>>---bigkevmcd: Ah, hmm. So the server-side fix will unwedge clients, right?
Aug 06 15:16:58 <bigkevmcd>>jkakar: precisely
Aug 06 15:17:07 <bigkevmcd>>jkakar: see the debian bug report above
Aug 06 15:17:17 <bigkevmcd>>jkakar: apparently there's a kernel bug, which makes rollover more likely
Aug 06 15:17:35 <bigkevmcd>>jkakar: the data comes from the kernel in 32bits, which will cause us to see larger deltas
Aug 06 15:17:49 <jkakar>>---bigkevmcd: Wow, that bug was opened in 2003. :/
Aug 06 15:17:57 <_mup_>>Branch lp:~bigkevmcd/landscape/bug-614346-network-data-overflow: approved
Aug 06 15:17:57 <bigkevmcd>>jkakar: note the last date...
Aug 06 15:17:59 <jkakar>>---bigkevmcd: I see.
Aug 06 15:18:00 <jkakar>>---bigkevmcd: Yep.
Aug 06 15:18:14 <jkakar>>---bigkevmcd: We should fix the client if we can.
Aug 06 15:18:22 <andreas>>--so the client still has a rollover bug
Aug 06 15:18:25 <bigkevmcd>>yeah
...

Read more...

Revision history for this message
Kevin McDermott (bigkevmcd) wrote :

Thomas,

My reading of the code is that the figure is only compared against an in-memory value, so a restart of the machine would show the 0 -> current value (which is sort of correct, it's just not correct against the previously reported value), but short of resetting the data every time the machine was reset, it's odd?

Is reporting Deltas the correct thing to do?

Kevin

Revision history for this message
Thomas Herve (therve) wrote :

Kevin: the accumulator uses a persist, so it's not just against the memory value.

Thanks for pasting the discussion, the debian bug does indeed raise some interesting discussions.

At any rate, those values are expected to be bytes. The client should never report negative values, and the server should not store it either. I'd rather see us store 0 if it's a negative value.

Thomas Herve (therve)
Changed in landscape-client:
assignee: nobody → Thomas Herve (therve)
Thomas Herve (therve)
Changed in landscape-client:
status: New → In Progress
Thomas Herve (therve)
Changed in landscape-client:
status: In Progress → Fix Committed
tags: added: needs-testing
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Confirmed working with the maverick package.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Confirmed working with the Lucid and Karmic packages.

Revision history for this message
Colin Watson (cjwatson) wrote :

landscape-client (1.5.5-0ubuntu0.10.10.0) maverick; urgency=low

  * New upstream version (LP: #633468)

    - The --help command line option can now be used without being
      root (LP: #613256).

    - The client Unix sockets and symlinks are now cleaned up at shutdown.
      Without this cleaning, the client could refuse to start because of a PID
      collision (LP: #607747).

    - The network traffic plugin didn't use to take into account integer
      overflows. This would cause the plugin to send negative values
      sometimes (LP: #615371).

    - If a payload had many user activities in it, only the last one would be
      carried out (LP: #617624).

    - The Eucalyptus plugin was not enabled by default, which means the Cloud
      Topology feature of Landscape was not available (LP: #614493).

 -- Andreas Hasenack <email address hidden> Wed, 08 Sep 2010 15:34:09 -0400

Changed in landscape-client (Ubuntu):
status: New → Fix Released
Changed in landscape-client:
milestone: 1.5.5 → 1.5.5.1
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Please drop the nomination for Maverick, as it was fixed in 1.5.5.

Changed in landscape-client (Ubuntu):
status: Fix Released → Fix Committed
tags: removed: needs-testing
Revision history for this message
Martin Pitt (pitti) wrote : Please test proposed package

Accepted landscape-client into lucid-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in landscape-client (Ubuntu Maverick):
status: Fix Committed → Fix Released
Changed in landscape-client (Ubuntu Lucid):
status: New → Fix Committed
tags: added: verification-needed
Revision history for this message
Martin Pitt (pitti) wrote :

Accepted landscape-client into karmic-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in landscape-client (Ubuntu Karmic):
status: New → Fix Committed
Changed in landscape-client (Ubuntu Jaunty):
status: New → Fix Committed
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Confirmed working with 1.5.5.1-0ubuntu0.9.04.0, 1.5.5.1-0ubuntu0.10.04.0 and 1.5.5.1-0ubuntu0.9.10.0 from proposed.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package landscape-client - 1.5.5.1-0ubuntu0.10.04.0

---------------
landscape-client (1.5.5.1-0ubuntu0.10.04.0) lucid-proposed; urgency=low

  * The client network plugin would send erroneous data if a network
    interface was removed (and its kernel module removed as well) and
    then readded (LP: #641264).

landscape-client (1.5.5-0ubuntu0.10.04.0) lucid-proposed; urgency=low

  * New upstream version (LP: #633468)

    - The --help command line option can now be used without being
      root (LP: #613256).

    - The client Unix sockets and symlinks are now cleaned up at shutdown.
      Without this cleaning, the client could refuse to start because of a PID
      collision (LP: #607747).

    - The network traffic plugin didn't use to take into account integer
      overflows. This would cause the plugin to send negative values
      sometimes (LP: #615371).

    - If a payload had many user activities in it, only the last one would be
      carried out (LP: #617624).

    - The Eucalyptus plugin was not enabled by default, which means the Cloud
      Topology feature of Landscape was not available (LP: #614493).
 -- Andreas Hasenack <email address hidden> Mon, 20 Sep 2010 13:52:49 -0300

Changed in landscape-client (Ubuntu Lucid):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package landscape-client - 1.5.5.1-0ubuntu0.9.10.0

---------------
landscape-client (1.5.5.1-0ubuntu0.9.10.0) karmic-proposed; urgency=low

  * The client network plugin would send erroneous data if a network
    interface was removed (and its kernel module removed as well) and
    then readded (LP: #641264).

landscape-client (1.5.5-0ubuntu0.9.10.0) karmic-proposed; urgency=low

  * New upstream version (LP: #633468)

    - The --help command line option can now be used without being
      root (LP: #613256).

    - The client Unix sockets and symlinks are now cleaned up at shutdown.
      Without this cleaning, the client could refuse to start because of a PID
      collision (LP: #607747).

    - The network traffic plugin didn't use to take into account integer
      overflows. This would cause the plugin to send negative values
      sometimes (LP: #615371).

    - If a payload had many user activities in it, only the last one would be
      carried out (LP: #617624).

    - The Eucalyptus plugin was not enabled by default, which means the Cloud
      Topology feature of Landscape was not available (LP: #614493).
 -- Andreas Hasenack <email address hidden> Mon, 20 Sep 2010 13:52:49 -0300

Changed in landscape-client (Ubuntu Karmic):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package landscape-client - 1.5.5.1-0ubuntu0.9.04.0

---------------
landscape-client (1.5.5.1-0ubuntu0.9.04.0) jaunty-proposed; urgency=low

  * The client network plugin would send erroneous data if a network
    interface was removed (and its kernel module removed as well) and
    then readded (LP: #641264).

landscape-client (1.5.5-0ubuntu0.9.04.0) jaunty-proposed; urgency=low

  * New upstream version (LP: #633468)

    - The --help command line option can now be used without being
      root (LP: #613256).

    - The client Unix sockets and symlinks are now cleaned up at shutdown.
      Without this cleaning, the client could refuse to start because of a PID
      collision (LP: #607747).

    - The network traffic plugin didn't use to take into account integer
      overflows. This would cause the plugin to send negative values
      sometimes (LP: #615371).

    - If a payload had many user activities in it, only the last one would be
      carried out (LP: #617624).

    - The Eucalyptus plugin was not enabled by default, which means the Cloud
      Topology feature of Landscape was not available (LP: #614493).
 -- Andreas Hasenack <email address hidden> Mon, 20 Sep 2010 13:52:49 -0300

Changed in landscape-client (Ubuntu Jaunty):
status: Fix Committed → Fix Released
Changed in landscape-client:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.