[ovs-dev] [PATCHv2 2/2] revalidator: Revalidate missed flows.

[Joe Stringer]

If the datapath doesn't dump a flow for some reason, and the current
dump is expected to revalidate all flows in the datapath, then perform
revalidation for those flows by fetching them during the sweep phase.
If revalidation is not required, then leave the flow in the datapath and
don't revalidate it.

Signed-off-by: Joe Stringer <joestringer at nicira.com>
---
v2: Make the revalidator_sweep__() logic easier to read.
    Rebase.
RFC: First post.
---
 ofproto/ofproto-dpif-upcall.c |   55 +++++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 74d9686..9c2a8a4 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -46,6 +46,7 @@
 VLOG_DEFINE_THIS_MODULE(ofproto_dpif_upcall);
 
 COVERAGE_DEFINE(upcall_duplicate_flow);
+COVERAGE_DEFINE(revalidate_missed_dp_flow);
 
 /* A thread that reads upcalls from dpif, forwards each upcall's packet,
  * and possibly sets up a kernel flow as a cache. */
@@ -1537,6 +1538,31 @@ revalidate(struct revalidator *revalidator)
     dpif_flow_dump_state_uninit(udpif->dpif, state);
 }
 
+/* Called with exclusive access to 'revalidator' and 'ukey'. */
+static bool
+handle_missed_revalidation(struct revalidator *revalidator,
+                           struct udpif_key *ukey)
+    OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+    struct udpif *udpif = revalidator->udpif;
+    struct nlattr *mask, *actions;
+    size_t mask_len, actions_len;
+    struct dpif_flow_stats stats;
+    struct ofpbuf *buf;
+    bool keep = false;
+
+    COVERAGE_INC(revalidate_missed_dp_flow);
+
+    if (!dpif_flow_get(udpif->dpif, ukey->key, ukey->key_len, &buf,
+                       &mask, &mask_len, &actions, &actions_len, &stats)) {
+        keep = revalidate_ukey(udpif, ukey, mask, mask_len, actions,
+                               actions_len, &stats);
+        ofpbuf_delete(buf);
+    }
+
+    return keep;
+}
+
 static void
 revalidator_sweep__(struct revalidator *revalidator, bool purge)
     OVS_NO_THREAD_SAFETY_ANALYSIS
@@ -1550,21 +1576,24 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
     /* During garbage collection, this revalidator completely owns its ukeys
      * map, and therefore doesn't need to do any locking. */
     HMAP_FOR_EACH_SAFE (ukey, next, hmap_node, revalidator->ukeys) {
-        if (!purge && ukey->mark) {
+        if (ukey->flow_exists) {
+            bool missed_flow = !ukey->mark;
+
             ukey->mark = false;
-        } else if (!ukey->flow_exists) {
-            ukey_delete(revalidator, ukey);
-        } else {
-            struct dump_op *op = &ops[n_ops++];
-
-            /* If we have previously seen a flow in the datapath, but didn't
-             * see it during the most recent dump, delete it. This allows us
-             * to clean up the ukey and keep the statistics consistent. */
-            dump_op_init(op, ukey->key, ukey->key_len, ukey);
-            if (n_ops == REVALIDATE_MAX_BATCH) {
-                push_dump_ops(revalidator, ops, n_ops);
-                n_ops = 0;
+            if (purge
+                || (missed_flow
+                    && revalidator->udpif->need_revalidate
+                    && handle_missed_revalidation(revalidator, ukey))) {
+                struct dump_op *op = &ops[n_ops++];
+
+                dump_op_init(op, ukey->key, ukey->key_len, ukey);
+                if (n_ops == REVALIDATE_MAX_BATCH) {
+                    push_dump_ops(revalidator, ops, n_ops);
+                    n_ops = 0;
+                }
             }
+        } else {
+            ukey_delete(revalidator, ukey);
         }
     }
 
-- 
1.7.10.4