On Wed, Nov 30, 2016 at 05:51:12PM -0800, Jarno Rajahalme wrote: > While a flow modify must keep the original flow's flags, it must reset > counts if (and only if) the reset_counts flag is present in the flow > mod message. > > Behavior prior to this patch is broken in a few ways: > > - OpenFlow 1.0 and 1.1 mod-flows did reset the counts, if the flow had > reset_counts flag set. Only add-flow should reset counts. > - With OpenFlow 1.2 and later, if the old flow had the reset_counts > flag set, the counts would be reset by mod-flows, even if the > flow-mod message does not have the reset_counts flag set. > - With OpenFlow 1.2 and later, mod-flows with a reset_count did not > reset the counts, if the old flow did not have the reset_counts flag > set. > > Even though the prevailing interpretation seems to be that the > reset_counts flag in the flow-mod message should be stored as part of > the flow state (and reported back in flow dumps with OpenFlow >= 1.3), > we should always just look at the reset_counts flag in the current > flow-mod and ignore the reset_counts flag stored in the flow when > processing a flow mod. > > For OpenFlow 1.0 and 1.1 we already implicitly add the reset_counts > flag for add-flow messages (only) to maintain the expected behavior. > > This patch adds a comprehensive test case to prevent future regressions. > > Suggested-by: Tony van der Peet <[email protected]> > Fixes: 748eb2f5b1 ("ofproto-dpif: Always forward 'used' from the old_rule.") > Signed-off-by: Jarno Rajahalme <[email protected]>
This is tagged "net-next", but it's an OVS patch. This is very nice. One rarely sees such a thorough test case. It had never occurred to me to try to match exact values for duration, idle_age, and hard_age in output (at least down to a number of seconds), but if it works that's very nice too. If I may be permitted to nit-pick, the name "modify_forward_counts" took me a bit of thinking to properly understand. Maybe "modify_keep_stats" would be easier for me to understand at first glance. Tony, does this solve the problem for you? Acked-by: Ben Pfaff <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
