On Tue, May 03, 2011 at 04:24:25PM -0700, Ethan Jackson wrote: > In some cases, when a facet's actions change because it resubmits > into a different rule, it will account all packets it as accrued > in the datapath to the new rule. Due to the algorithm we are > using, it is acceptable for a facet to miscount at most 1 second > worth of packets in this manner. This patch implements the proper > behavior. > > Generally speaking, when a facet is facet_put__() into the > datapath, the kernel returns the old flow's statistics so they may > be accounted for in user space. These statistics are generally > pushed down to the relevant facet's resubmit children. Before this > patch, facet_put__() did not compensate for the fact that many of > the statistics in the datapath may have been already pushed. > Thus the entire packet count stored in the datapath would be pushed > to its children instead of simply the packets which have accrued > since the last accounting. This patch fixes the behavior by > subtracting already accounted for packets from the statistics > returned by the datapath.
The only potential issue I see here is with logging on error paths. If dpif_flow_put() or dpif_flow_del() returns an error then they will zero their stats arguments, which will then almost certainly cause an error to be logged in facet_reset_dp_stats(). That's going to be a bit confusing, although it won't cause any actual problems. If you see a nice way to avoid it, though, I'd think about it. Looks good. Thank you. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
