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

Proposed by Thomas Herve on 2010-08-16
Status: Merged
Approved by: Kevin McDermott on 2010-08-18
Approved revision: 287
Merged at revision: 284
Proposed branch: lp:~therve/landscape-client/network-plugin-reboot
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 177 lines (+91/-8)
3 files modified
landscape/lib/network.py (+1/-1)
landscape/monitor/networkactivity.py (+26/-5)
landscape/monitor/tests/test_networkactivity.py (+64/-2)
To merge this branch: bzr merge lp:~therve/landscape-client/network-plugin-reboot
Reviewer Review Type Date Requested Status
Kevin McDermott (community) 2010-08-16 Approve on 2010-08-18
Jamu Kakar (community) 2010-08-16 Approve on 2010-08-17
Review via email: mp+32755@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
287. By Thomas Herve on 2010-08-16

Put back safeguard, handle packet rollover

Jamu Kakar (jkakar) wrote :

[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) wrote :

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
288. By Thomas Herve on 2010-08-18

Rename rollover variable.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'landscape/lib/network.py'
2--- landscape/lib/network.py 2010-07-23 08:08:41 +0000
3+++ landscape/lib/network.py 2010-08-18 08:45:58 +0000
4@@ -6,7 +6,7 @@
5 import socket
6 import struct
7
8-__all__ = ["get_active_device_info", "get_network_traffic"]
9+__all__ = ["get_active_device_info", "get_network_traffic", "is_64"]
10
11 # from header /usr/include/bits/ioctls.h
12 SIOCGIFCONF = 0x8912
13
14=== modified file 'landscape/monitor/networkactivity.py'
15--- landscape/monitor/networkactivity.py 2010-06-11 09:12:51 +0000
16+++ landscape/monitor/networkactivity.py 2010-08-18 08:45:58 +0000
17@@ -5,7 +5,7 @@
18
19 import time
20
21-from landscape.lib.network import get_network_traffic
22+from landscape.lib.network import get_network_traffic, is_64
23 from landscape.accumulate import Accumulator
24
25 from landscape.monitor.plugin import MonitorPlugin
26@@ -28,6 +28,7 @@
27 # our last traffic sample for calculating a traffic delta
28 self._last_activity = {}
29 self._create_time = create_time
30+ self._rollover_maxint = pow(2, 64) if is_64() else pow(2, 32)
31
32 def register(self, registry):
33 super(NetworkActivity, self).register(registry)
34@@ -61,14 +62,34 @@
35 for interface in new_traffic:
36 traffic = new_traffic[interface]
37 if interface in self._last_activity:
38- previous_out, previous_in = self._last_activity[interface]
39+ (previous_out, previous_in, previous_packet_out,
40+ previous_packet_in) = self._last_activity[interface]
41 delta_out = traffic["send_bytes"] - previous_out
42 delta_in = traffic["recv_bytes"] - previous_in
43 if not delta_out and not delta_in:
44 continue
45+ if delta_out < 0:
46+ delta_out += self._rollover_maxint
47+ if delta_in < 0:
48+ delta_in += self._rollover_maxint
49+ packets_delta_out = (
50+ traffic["send_packets"] - previous_packet_out)
51+ if packets_delta_out < 0:
52+ packets_delta_out += self._rollover_maxint
53+ # 28 bytes is the minimum packet size, roughly
54+ if packets_delta_out * 28 > delta_out:
55+ delta_out += self._rollover_maxint
56+ packets_delta_in = (
57+ traffic["recv_packets"] - previous_packet_in)
58+ if packets_delta_in < 0:
59+ packets_delta_in += self._rollover_maxint
60+ if packets_delta_in * 28 > delta_in:
61+ delta_in += self._rollover_maxint
62+
63 yield interface, delta_out, delta_in
64 self._last_activity[interface] = (
65- traffic["send_bytes"], traffic["recv_bytes"])
66+ traffic["send_bytes"], traffic["recv_bytes"],
67+ traffic["send_packets"], traffic["recv_packets"])
68
69 def run(self):
70 """
71@@ -79,10 +100,10 @@
72 new_traffic = get_network_traffic(self._source_file)
73 for interface, delta_out, delta_in in self._traffic_delta(new_traffic):
74 out_step_data = self._accumulate(
75- new_timestamp, delta_out, "delta-out-%s"%interface)
76+ new_timestamp, delta_out, "delta-out-%s" % interface)
77
78 in_step_data = self._accumulate(
79- new_timestamp, delta_in, "delta-in-%s"%interface)
80+ new_timestamp, delta_in, "delta-in-%s" % interface)
81
82 # there's only data when we cross a step boundary
83 if not (in_step_data and out_step_data):
84
85=== modified file 'landscape/monitor/tests/test_networkactivity.py'
86--- landscape/monitor/tests/test_networkactivity.py 2010-06-11 09:12:51 +0000
87+++ landscape/monitor/tests/test_networkactivity.py 2010-08-18 08:45:58 +0000
88@@ -10,7 +10,7 @@
89 stats_template = """\
90 Inter-| Receive | Transmit
91 face |bytes packets compressed multicast|bytes packets errs drop fifo
92- lo:%(lo_in)d 0 0 0 %(lo_out)d 3321049 0 0 0
93+ lo:%(lo_in)d %(lo_in_p)d 0 0 %(lo_out)d %(lo_out_p)d 0 0 0
94 eth0: %(eth0_in)d 12539 0 62 %(eth0_out)d 12579 0 0 0
95 %(extra)s
96 """
97@@ -29,10 +29,12 @@
98 super(NetworkActivityTest, self).tearDown()
99
100 def write_activity(self, lo_in=0, lo_out=0, eth0_in=0, eth0_out=0,
101- extra="", **kw):
102+ extra="", lo_in_p=0, lo_out_p=0, **kw):
103 kw.update(dict(
104 lo_in = lo_in,
105 lo_out = lo_out,
106+ lo_in_p = lo_in_p,
107+ lo_out_p = lo_out_p,
108 eth0_in = eth0_in,
109 eth0_out = eth0_out,
110 extra=extra))
111@@ -80,6 +82,66 @@
112 [(300, 10, 99)])
113 self.assertNotIn("eth0", message["activities"])
114
115+ def test_proc_rollover(self):
116+ """
117+ If /proc/net/dev rollovers, the network plugin handles the value and
118+ gives a positive value instead.
119+ """
120+ self.plugin._rollover_maxint = 10000
121+ self.write_activity(lo_in=2000, lo_out=1900)
122+ self.plugin.run()
123+ self.reactor.advance(self.monitor.step_size)
124+ self.write_activity(lo_in=1010, lo_out=999)
125+ self.plugin.run()
126+ message = self.plugin.create_message()
127+ self.assertTrue(message)
128+ self.assertTrue("type" in message)
129+ self.assertEquals(message["type"], "network-activity")
130+ self.assertEquals(message["activities"]["lo"],
131+ [(300, 9010, 9099)])
132+ self.assertNotIn("eth0", message["activities"])
133+
134+ def test_proc_huge_rollover(self):
135+ """
136+ If /proc/net/dev rollovers *and* that we pass the previous measured
137+ value (so, more than 4GB in 30 seconds on 32 bits), we use the number
138+ of packets to check if the number makes sense. It doesn't solve
139+ everything, but it helps in some cases.
140+ """
141+ self.plugin._rollover_maxint = 10000
142+ self.write_activity(lo_in=2000, lo_out=1900)
143+ self.plugin.run()
144+ self.reactor.advance(self.monitor.step_size)
145+ self.write_activity(lo_in=3000, lo_out=1999, lo_in_p=100, lo_out_p=50)
146+ self.plugin.run()
147+ message = self.plugin.create_message()
148+ self.assertTrue(message)
149+ self.assertTrue("type" in message)
150+ self.assertEquals(message["type"], "network-activity")
151+ self.assertEquals(message["activities"]["lo"],
152+ [(300, 11000, 10099)])
153+ self.assertNotIn("eth0", message["activities"])
154+
155+ def test_proc_huge_packets_rollover(self):
156+ """
157+ The value of packets can rollover, too, even if it's unlikely at the
158+ same time as bytes, let's handle the case.
159+ """
160+ self.plugin._rollover_maxint = 10000
161+ self.write_activity(lo_in=2000, lo_out=1900, lo_in_p=600, lo_out_p=500)
162+ self.plugin.run()
163+ self.reactor.advance(self.monitor.step_size)
164+ self.write_activity(lo_in=3000, lo_out=1999, lo_in_p=500, lo_out_p=200)
165+ self.plugin.run()
166+ message = self.plugin.create_message()
167+ self.assertTrue(message)
168+ self.assertTrue("type" in message)
169+ self.assertEquals(message["type"], "network-activity")
170+ self.assertEquals(message["activities"]["lo"],
171+ [(300, 11000, 10099)])
172+ self.assertNotIn("eth0", message["activities"])
173+
174+
175 def test_no_message_without_traffic_delta(self):
176 """
177 If no traffic delta is detected between runs, no message will be

Subscribers

People subscribed via source and target branches

to all changes: