From f22a24b7b3b8aec61f6778bf9c8e6e7c5514039a Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 1 Sep 2010 12:45:24 -0700 Subject: [PATCH] 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. --- ofproto/netflow.c | 49 +++++++++++++++++++++++++++++++---------------- 1 file 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. */ -- 2.43.0