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 <tony.vanderp...@alliedtelesis.co.nz>
> Fixes: 748eb2f5b1 ("ofproto-dpif: Always forward 'used' from the old_rule.")
> Signed-off-by: Jarno Rajahalme <ja...@ovn.org>

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 <b...@ovn.org>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to