Your way is a bit clearer so I will go ahead and change it. Ethan
On Tue, Feb 15, 2011 at 12:30 PM, Ben Pfaff <[email protected]> wrote: > On Mon, Feb 14, 2011 at 02:06:00PM -0800, Ethan Jackson wrote: >> Facet statistics are updated once per second during >> ofproto_expire() instead of upon request. This will greatly >> simplify implementation of future patches. This commit also changes >> each facet's packet and byte counters to include the statistics >> stored in the datapath. > > This looks good. Thank you! > > The new code in ofproto_update_used() struck me as a little odd in form, > because it has duplicate assignments of the form "facet->dp_something = > stats->something". I think that I would write the following: > > if (stats->n_packets < facet->dp_packet_count) { > VLOG_WARN_RL(&rl, "unexpected packet count from the datapath"); > facet->dp_packet_count = stats->n_packets; > } > > if (stats->n_bytes < facet->dp_byte_count) { > VLOG_WARN_RL(&rl, "unexpected byte count from datapath"); > facet->dp_byte_count = stats->n_bytes; > } > > facet->packet_count += stats->n_packets - facet->dp_packet_count; > facet->byte_count += stats->n_bytes - facet->dp_byte_count; > > facet->dp_packet_count = stats->n_packets; > facet->dp_byte_count = stats->n_bytes; > > more like this: > > /* Update packet count. */ > if (stats->n_packets >= facet->dp_packet_count) { > facet->packet_count += stats->n_packets - facet->dp_packet_count; > } else { > VLOG_WARN_RL(&rl, "unexpected packet count from the datapath"); > } > facet->dp_packet_count = stats->n_packets; > > /* Update byte count. */ > if (stats->n_bytes >= facet->dp_byte_count) { > facet->byte_count += stats->n_bytes - facet->dp_byte_count; > } else { > VLOG_WARN_RL(&rl, "unexpected byte count from the datapath"); > } > facet->dp_byte_count = stats->n_bytes; > > I'm just quibbling. The code as you wrote it looks correct to em. > > Thanks, > > Ben. > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
