On Mon, Feb 14, 2011 at 02:06:01PM -0800, Ethan Jackson wrote: > This patch causes statistics to be updated for rules which are > resubmitted into. Once per second statistics are queried from the > datapath and pushed along the resubmit graph (calculated on demand > from the action list). This approach is simple, easy to understand, > and in most cases accurate. However, when the resubmit graph > changes, it is possible that some statistics will be accounted to > the wrong rule for a short period of time. > > Bug #3730.
I like this approach. Thank you for writing it up. I see a few problems with the implementation, though. The first problem is that the semantics for the in_port in flow_push_stats() do not match those in xlate_table_action(). In xlate_table_action(), the resubmitted in_port is used only for the rule_lookup() and then restored. In flow_push_stats(), the resubmitted in_port is not restored until flow_push_stats() returns. Maybe this isn't a real problem, because OFPP_NORMAL and OFPP_IN_PORT do not have any special semantics in flow_push_stats(), but I do not see a reason for the difference either. (You could write a helper function rule_lookup_with_in_port() to encapsulate this lookup.) The second problem is that flow_push_stats() ignores some actions that will definitely make a difference in behavior, that is, any action that modifies the flow. Some examples are NXAST_SET_TUNNEL and NXAST_REG_MOVE. (It's easy enough to find these actions: just temporarily mark the "flow" member of struct action_xlate_ctx "const" and recompile and the compiler will point them out for you.) I hope that we don't have to duplicate a lot of code to fix the latter problem. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
