revalidator: Fix ukey stats cache updating.

revalidate_ukey() had a bug where it would update the ukey->stats even
if it decided not to push stats (as an optimisation). ukey->stats should
only be updated when those stats are pushed.

This bug would arise in the following situation:
* A flow has been dumped before.
* The flow needs to be revalidated.
* The flow is low-throughput.
* The flow has new statistics to push.

Such cases rely on flow deletion to update the stats. However, that code
pushes the delta between the ukey->stats and the final flow dump. If the
ukey stats cache is updated without the stats being pushed, those stats
would be lost.

This caused intermittent testsuite failures on "learning action -
self-modifying flow with idle_timeout". Introduced by 698ffe3623f1b630ae
"revalidator: Only revalidate high-throughput flows."

Bug #1238927.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 717563a..2c37781 100644 (file)
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1245,7 +1245,6 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
     push.n_bytes = stats->n_bytes > ukey->stats.n_bytes
         ? stats->n_bytes - ukey->stats.n_bytes
         : 0;
-    ukey->stats = *stats;
 
     if (!ukey->flow_exists) {
         /* Don't bother revalidating if the flow was already deleted. */
@@ -1258,6 +1257,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
         goto exit;
     }
 
+    /* We will push the stats, so update the ukey stats cache. */
+    ukey->stats = *stats;
     if (!push.n_packets && !udpif->need_revalidate) {
         ok = true;
         goto exit;