From 28c5588e8e1a8d091c5d2275232c35f2968a97fa Mon Sep 17 00:00:00 2001 From: Joe Stringer Date: Fri, 25 Apr 2014 15:23:43 -0700 Subject: [PATCH] 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 Acked-by: Alex Wang --- ofproto/ofproto-dpif-upcall.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 717563a3f..2c37781f7 100644 --- 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; -- 2.43.0