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 717563a3fe68599116793a8343e82e324eb058e5..2c37781f7d15b5aa04dbe6af392f06b06b278ad7 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;