Merge lp:~therve/landscape-client/network-plugin-reboot into lp:~landscape/landscape-client/trunk

[Thomas Herve]

The branch handles the rollovers of /proc/net/dev, hopefully.

Restarting was indeed not a problem, because we keep a memory safeguard. I'm still a little bit worried about the persist, but I guess it's fine.

[lp:~therve/landscape-client/network-plugin-reboot]

287. By Thomas Herve on 2010-08-16

Put back safeguard, handle packet rollover

[Jamu Kakar (jkakar)]

[1]

+ self._rolloverunit = pow(2, 64) if is_64() else pow(2, 32)

Please s/_rolloverunit/_rollover_unit/.

The change seems slightly odd to me, but I guess it's a step up from
what we have, +1!

review: Approve

[Kevin McDermott (bigkevmcd)]

I'd prefer _rollover_unit to be named something like _rollover_maxint or something, as it's essentially the MAXINT value for the platform...

Other than that, it's cool, subject to the kernel bugs we've seen...

+1

review: Approve

[lp:~therve/landscape-client/network-plugin-reboot]

288. By Thomas Herve on 2010-08-18

Rename rollover variable.

=== modified file 'landscape/lib/network.py'
--- landscape/lib/network.py	2010-07-23 08:08:41 +0000
+++ landscape/lib/network.py	2010-08-18 08:45:58 +0000
@@ -6,7 +6,7 @@
 import socket
 import struct
 
-__all__ = ["get_active_device_info", "get_network_traffic"]
+__all__ = ["get_active_device_info", "get_network_traffic", "is_64"]
 
 # from header /usr/include/bits/ioctls.h
 SIOCGIFCONF = 0x8912

=== modified file 'landscape/monitor/networkactivity.py'
--- landscape/monitor/networkactivity.py	2010-06-11 09:12:51 +0000
+++ landscape/monitor/networkactivity.py	2010-08-18 08:45:58 +0000
@@ -5,7 +5,7 @@
 
 import time
 
-from landscape.lib.network import get_network_traffic
+from landscape.lib.network import get_network_traffic, is_64
 from landscape.accumulate import Accumulator
 
 from landscape.monitor.plugin import MonitorPlugin
@@ -28,6 +28,7 @@
         # our last traffic sample for calculating a traffic delta
         self._last_activity = {}
         self._create_time = create_time
+        self._rollover_maxint = pow(2, 64) if is_64() else pow(2, 32)
 
     def register(self, registry):
         super(NetworkActivity, self).register(registry)
@@ -61,14 +62,34 @@
         for interface in new_traffic:
             traffic = new_traffic[interface]
             if interface in self._last_activity:
-                previous_out, previous_in = self._last_activity[interface]
+                (previous_out, previous_in, previous_packet_out,
+                 previous_packet_in) = self._last_activity[interface]
                 delta_out = traffic["send_bytes"] - previous_out
                 delta_in = traffic["recv_bytes"] - previous_in
                 if not delta_out and not delta_in:
                     continue
+                if delta_out < 0:
+                    delta_out += self._rollover_maxint
+                if delta_in < 0:
+                    delta_in += self._rollover_maxint
+                packets_delta_out = (
+                    traffic["send_packets"] - previous_packet_out)
+                if packets_delta_out < 0:
+                    packets_delta_out += self._rollover_maxint
+                # 28 bytes is the minimum packet size, roughly
+                if packets_delta_out * 28 > delta_out:
+                    delta_out += self._rollover_maxint
+                packets_delta_in = (
+                    traffic["recv_packets"] - previous_packet_in)
+                if packets_delta_in < 0:
+                    packets_delta_in += self._rollover_maxint
+                if packets_delta_in * 28 > delta_in:
+                    delta_in += self._rollover_maxint
+
                 yield interface, delta_out, delta_in
             self._last_activity[interface] = (
-                traffic["send_bytes"], traffic["recv_bytes"])
+                traffic["send_bytes"], traffic["recv_bytes"],
+                traffic["send_packets"], traffic["recv_packets"])
 
     def run(self):
         """
@@ -79,10 +100,10 @@
         new_traffic = get_network_traffic(self._source_file)
         for interface, delta_out, delta_in in self._traffic_delta(new_traffic):
             out_step_data = self._accumulate(
-                new_timestamp, delta_out, "delta-out-%s"%interface)
+                new_timestamp, delta_out, "delta-out-%s" % interface)
 
             in_step_data = self._accumulate(
-                new_timestamp, delta_in, "delta-in-%s"%interface)
+                new_timestamp, delta_in, "delta-in-%s" % interface)
 
             # there's only data when we cross a step boundary
             if not (in_step_data and out_step_data):

=== modified file 'landscape/monitor/tests/test_networkactivity.py'
--- landscape/monitor/tests/test_networkactivity.py	2010-06-11 09:12:51 +0000
+++ landscape/monitor/tests/test_networkactivity.py	2010-08-18 08:45:58 +0000
@@ -10,7 +10,7 @@
     stats_template = """\
 Inter-|   Receive                           |  Transmit
  face |bytes    packets compressed multicast|bytes    packets errs drop fifo
-    lo:%(lo_in)d   0       0         0       %(lo_out)d 3321049    0    0    0
+    lo:%(lo_in)d   %(lo_in_p)d       0         0       %(lo_out)d %(lo_out_p)d    0    0    0
     eth0: %(eth0_in)d   12539      0     62  %(eth0_out)d   12579    0    0   0
     %(extra)s
 """
@@ -29,10 +29,12 @@
         super(NetworkActivityTest, self).tearDown()
 
     def write_activity(self, lo_in=0, lo_out=0, eth0_in=0, eth0_out=0,
-                        extra="", **kw):
+                        extra="", lo_in_p=0, lo_out_p=0, **kw):
         kw.update(dict(
             lo_in = lo_in,
             lo_out = lo_out,
+            lo_in_p = lo_in_p,
+            lo_out_p = lo_out_p,
             eth0_in = eth0_in,
             eth0_out = eth0_out,
             extra=extra))
@@ -80,6 +82,66 @@
                           [(300, 10, 99)])
         self.assertNotIn("eth0", message["activities"])
 
+    def test_proc_rollover(self):
+        """
+        If /proc/net/dev rollovers, the network plugin handles the value and
+        gives a positive value instead.
+        """
+        self.plugin._rollover_maxint = 10000
+        self.write_activity(lo_in=2000, lo_out=1900)
+        self.plugin.run()
+        self.reactor.advance(self.monitor.step_size)
+        self.write_activity(lo_in=1010, lo_out=999)
+        self.plugin.run()
+        message = self.plugin.create_message()
+        self.assertTrue(message)
+        self.assertTrue("type" in message)
+        self.assertEquals(message["type"], "network-activity")
+        self.assertEquals(message["activities"]["lo"],
+                          [(300, 9010, 9099)])
+        self.assertNotIn("eth0", message["activities"])
+
+    def test_proc_huge_rollover(self):
+        """
+        If /proc/net/dev rollovers *and* that we pass the previous measured
+        value (so, more than 4GB in 30 seconds on 32 bits), we use the number
+        of packets to check if the number makes sense. It doesn't solve
+        everything, but it helps in some cases.
+        """
+        self.plugin._rollover_maxint = 10000
+        self.write_activity(lo_in=2000, lo_out=1900)
+        self.plugin.run()
+        self.reactor.advance(self.monitor.step_size)
+        self.write_activity(lo_in=3000, lo_out=1999, lo_in_p=100, lo_out_p=50)
+        self.plugin.run()
+        message = self.plugin.create_message()
+        self.assertTrue(message)
+        self.assertTrue("type" in message)
+        self.assertEquals(message["type"], "network-activity")
+        self.assertEquals(message["activities"]["lo"],
+                          [(300, 11000, 10099)])
+        self.assertNotIn("eth0", message["activities"])
+
+    def test_proc_huge_packets_rollover(self):
+        """
+        The value of packets can rollover, too, even if it's unlikely at the
+        same time as bytes, let's handle the case.
+        """
+        self.plugin._rollover_maxint = 10000
+        self.write_activity(lo_in=2000, lo_out=1900, lo_in_p=600, lo_out_p=500)
+        self.plugin.run()
+        self.reactor.advance(self.monitor.step_size)
+        self.write_activity(lo_in=3000, lo_out=1999, lo_in_p=500, lo_out_p=200)
+        self.plugin.run()
+        message = self.plugin.create_message()
+        self.assertTrue(message)
+        self.assertTrue("type" in message)
+        self.assertEquals(message["type"], "network-activity")
+        self.assertEquals(message["activities"]["lo"],
+                          [(300, 11000, 10099)])
+        self.assertNotIn("eth0", message["activities"])
+
+
     def test_no_message_without_traffic_delta(self):
         """
         If no traffic delta is detected between runs, no message will be