[ovs-dev] [PATCH 4/4] ofproto-dpif-upcall: Attribute statistics to OpenFlow flows more precisely.

[Joe Stringer]

On 23 August 2014 10:47, Ben Pfaff <blp at nicira.com> wrote:

> OpenFlow packet and byte counters have always been something of an
> approximation in Open vSwitch.  First, they are updated only periodically.
> Second, they can be misattributed because statistics collection does a
> retranslation and gives the statistics to whichever OpenFlow flow or flows
> they happen to hit, which could be different from the original set of
> flows because the OpenFlow flow table may have changed.
>
> This commit is intended to somewhat improve on the second point, by always
> attributing statistics to the set of flows that were cached at the datapath
> flow's initial translation by the revalidator.  This reduces the race in
> which the flow table could have changed to the interval between the initial
> installation and the first translation by the revalidator.  We plan to
> start having the thread that does the initial installation also create the
> ukey, which would eliminate that interval entirely.  At that point, packets
> could get lost from OpenFlow statistics because of the periodic collection
> of datapath statistics, but they should no longer be misattributed.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
>

We previously merged a similar patch:
http://openvswitch.org/pipermail/dev/2014-June/041120.html

..which caused issues with xlate side-effects for mac-learning, so we
reverted it:
http://openvswitch.org/pipermail/dev/2014-June/041868.html

Around that time, I also proposed an alternative, which failed to gain
traction:
http://openvswitch.org/pipermail/dev/2014-June/041718.html


In summary, I agree with the reasoning around statistics. The trouble is
that xlate_push_stats() also performs various side-effects that
xlate_push_stats(). If this xlate_cache is out of date, and refers to rules
that cause learning, then OVS may learn rules when it is not supposed to.
These may cause unexpected behaviour, and the incorrect rules can hang
around for an extended period.

I think that the decision from the third patch above was that the current
behaviour was "good enough", and we didn't have a good solution short of
tracking flowtable transitions to implement retroactive side-effects
(Excerpt from thread below)

> I think that the correct solution is to perform "retroactive
side-effects",
> that is, perform side effects as though they had happened at the time that
> the flow was hit. Anything that may hide these effects (like mac table
> flush) would need to be taken into account. However, such information is
> unavailable, and would be non-trivial to collect and maintain.

Happy to revisit this discussion if you'd like.