[ovs-dev] [PATCH] netflow: Send multiple records for byte counts > UINT32_MAX

Jesse Gross jesse at nicira.com
Wed Sep 1 06:24:30 UTC 2010


On Tue, Aug 31, 2010 at 10:00 PM, Justin Pettit <jpettit at nicira.com> wrote:
> +    n_recs = ((expired->byte_count - nf_flow->byte_count_off)
> +               + UINT32_MAX + 1) / UINT32_MAX;

I would probably just do the division and then add 1 at the end.  It
seems clearer to not have the extra UINT32_MAX in there.  Do you even
need the + 1 here as currently written?

> +    avg_pkt_count = (expired->packet_count - nf_flow->packet_count_off)
> +                    / n_recs;
> +    avg_byte_count = (expired->byte_count - nf_flow->byte_count_off)
> +                     / n_recs;

Since this is integer math, avg_XXX_count * n_recs will likely be less
than (expired->XXX_count - nf_flow->XXX_count_off).  The sum of all of
these rounding errors will be made up in the last packet.  However,
what happens if that last packet is already close to the 32 bit limit?

Imagine this case (all integer math):
expired->byte_count - nf_flow->packet_count_off = 21474836472
n_recs = 5
avg_pkt_count = 4294967294
result of subtraction for last packet: 4294967296 = 0 in 32 bit

> +
> +    while (n_recs--) {
> +        uint32_t pkt_count = n_recs ? avg_pkt_count :
> +                    expired->packet_count - nf_flow->packet_count_off;
> +        uint32_t byte_count = n_recs ? avg_byte_count :
> +                    expired->byte_count - nf_flow->byte_count_off;
> +
> +        gen_netflow_rec(nf, nf_flow, expired, pkt_count, byte_count);
> +
> +        nf_flow->packet_count_off += pkt_count;
> +        nf_flow->byte_count_off += byte_count;
> +    }

This loop is trying to be a little too clever for me.  I would just
write it as a for loop, drop the ternary ifs, use a couple extra
temporary variables, etc  You also compute the difference between the
expiration record and the offset 3 different times in this function, I
would put it in a temporary variable to start with and subtract off of
that here.




More information about the dev mailing list