summaryrefslogtreecommitdiff
path: root/ofproto/netflow.c
diff options
context:
space:
mode:
authorBen Pfaff <blp@nicira.com>2010-09-01 12:45:24 -0700
committerBen Pfaff <blp@nicira.com>2010-10-01 14:31:48 -0700
commit9ebc44ae8c5940513a8dcc2aab8dcca8aff9d2a2 (patch)
treed78b29f3847f4da9af25f852886f986eab111aec /ofproto/netflow.c
parent48f846e66ef06c39228fb1d3f8d8bfd1c695c93d (diff)
downloadopenvswitch-9ebc44ae8c5940513a8dcc2aab8dcca8aff9d2a2.tar.gz
netflow: Avoid (theoretically) looping 2**32 times.
If the netflow byte counter is UINT64_MAX, or at any rate much larger than UINT32_MAX, netflow_expire() could loop for a very long time. This commit avoids that case. This is only a theoretical bug fix. I don't know of any actual bug that would cause a counter to be that high.
Diffstat (limited to 'ofproto/netflow.c')
-rw-r--r--ofproto/netflow.c49
1 files changed, 32 insertions, 17 deletions
diff --git a/ofproto/netflow.c b/ofproto/netflow.c
index a70b2fce8..4881c5fdb 100644
--- a/ofproto/netflow.c
+++ b/ofproto/netflow.c
@@ -184,23 +184,38 @@ netflow_expire(struct netflow *nf, struct netflow_flow *nf_flow,
return;
}
- /* NetFlow v5 records are limited to 32-bit counters. If we've wrapped
- * a counter, send as multiple records so we don't lose track of any
- * traffic. We try to evenly distribute the packet and byte counters,
- * so that the bytes-per-packet lengths don't look wonky across the
- * records. */
- while (byte_delta > UINT32_MAX) {
- uint32_t n_recs = byte_delta >> 32;
- uint32_t pkt_count = pkt_delta / n_recs;
- uint32_t byte_count = byte_delta / n_recs;
-
- gen_netflow_rec(nf, nf_flow, expired, pkt_count, byte_count);
-
- pkt_delta -= pkt_count;
- byte_delta -= byte_count;
- }
- if (byte_delta > 0) {
- gen_netflow_rec(nf, nf_flow, expired, pkt_delta, byte_delta);
+ if ((byte_delta >> 32) <= 175) {
+ /* NetFlow v5 records are limited to 32-bit counters. If we've wrapped
+ * a counter, send as multiple records so we don't lose track of any
+ * traffic. We try to evenly distribute the packet and byte counters,
+ * so that the bytes-per-packet lengths don't look wonky across the
+ * records. */
+ while (byte_delta > UINT32_MAX) {
+ uint32_t n_recs = byte_delta >> 32;
+ uint32_t pkt_count = pkt_delta / n_recs;
+ uint32_t byte_count = byte_delta / n_recs;
+
+ gen_netflow_rec(nf, nf_flow, expired, pkt_count, byte_count);
+
+ pkt_delta -= pkt_count;
+ byte_delta -= byte_count;
+ }
+ if (byte_delta > 0) {
+ gen_netflow_rec(nf, nf_flow, expired, pkt_delta, byte_delta);
+ }
+ } else {
+ /* In 600 seconds, a 10GbE link can theoretically transmit 75 * 10**10
+ * == 175 * 2**32 bytes. The byte counter is bigger than that, so it's
+ * probably a bug--for example, the netdev code uses UINT64_MAX to
+ * report "unknown value", and perhaps that has leaked through to here.
+ *
+ * We wouldn't want to hit the loop above in this case, because it
+ * would try to send up to UINT32_MAX netflow records, which would take
+ * a long time.
+ */
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+
+ VLOG_WARN_RL(&rl, "impossible byte counter %"PRIu64, byte_delta);
}
/* Update flow tracking data. */