[ovs-dev] [PATCH] ofproto-dpif: Disable miss handling in rule_get_stats().

[Ethan Jackson]

rule_get_stats() is often called when iterating over every rule in
the flow table.  To ensure up-to-date statistics, rule_get_stats()
calls push_all_stats() which can cause flow misses to be handled.
When using the learn action, this can cause rules to be added (and
potentially removed) from the OpenFlow table.  This could corrupt
the caller's data structures, leading to a segmentation fault.
This patch fixes the issue by disabling flow miss handling from
within rule_get_stats().

Bug #15999.
Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
 ofproto/ofproto-dpif.c |   18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8f00424..0a6438c 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5105,7 +5105,7 @@ facet_push_stats(struct facet *facet)
 }
 
 static void
-push_all_stats(void)
+push_all_stats__(bool run_fast)
 {
     static long long int rl = LLONG_MIN;
     struct ofproto_dpif *ofproto;
@@ -5119,7 +5119,9 @@ push_all_stats(void)
 
         HMAP_FOR_EACH (facet, hmap_node, &ofproto->facets) {
             facet_push_stats(facet);
-            run_fast_rl();
+            if (run_fast) {
+                run_fast_rl();
+            }
         }
     }
 
@@ -5127,6 +5129,12 @@ push_all_stats(void)
 }
 
 static void
+push_all_stats(void)
+{
+    push_all_stats__(true);
+}
+
+static void
 rule_credit_stats(struct rule_dpif *rule, const struct dpif_flow_stats *stats)
 {
     rule->packet_count += stats->n_packets;
@@ -5608,7 +5616,11 @@ rule_get_stats(struct rule *rule_, uint64_t *packets, uint64_t *bytes)
     struct rule_dpif *rule = rule_dpif_cast(rule_);
     struct facet *facet;
 
-    push_all_stats();
+    /* push_all_stats() can handle flow misses which, when using the learn
+     * action, can cause rules to be added and deleted.  This can corrupt our
+     * caller's datastructures which assume that rule_get_stats() doesn't have
+     * an impact on the flow table. To be safe, we disable miss handling. */
+    push_all_stats__(false);
 
     /* Start from historical data for 'rule' itself that are no longer tracked
      * in facets.  This counts, for example, facets that have expired. */
-- 
1.7.9.5