Rather than setting and resetting the 'mark' field in the ukey, this
patch introduces a seq to track whether a flow has been seen during the
most recent dump. This tidies the code and simplifies the logic for
detecting when flows are duplicated from the datapath.

Signed-off-by: Joe Stringer <joestrin...@nicira.com>
---
 ofproto/ofproto-dpif-upcall.c |   33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 7dae150..cee0bed 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -167,8 +167,7 @@ struct udpif_key {
     struct ovs_mutex mutex;                   /* Guards the following. */
     struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/
     long long int created OVS_GUARDED;        /* Estimate of creation time. */
-    bool mark OVS_GUARDED;                    /* For mark and sweep garbage
-                                                 collection. */
+    uint64_t dump_seq OVS_GUARDED;            /* Tracks udpif->dump_seq. */
     bool flow_exists OVS_GUARDED;             /* Ensures flows are only deleted
                                                  once. */
 
@@ -1131,7 +1130,7 @@ ukey_create(const struct nlattr *key, size_t key_len, 
long long int used)
     ukey->key_len = key_len;
 
     ovs_mutex_lock(&ukey->mutex);
-    ukey->mark = false;
+    ukey->dump_seq = 0;
     ukey->flow_exists = true;
     ukey->created = used ? used : time_msec();
     memset(&ukey->stats, 0, sizeof ukey->stats);
@@ -1439,8 +1438,10 @@ revalidate(struct revalidator *revalidator)
 {
     struct udpif *udpif = revalidator->udpif;
     struct dpif_flow_dump_thread *dump_thread;
+    uint64_t dump_seq;
     unsigned int flow_limit;
 
+    dump_seq = seq_read(udpif->dump_seq);
     atomic_read(&udpif->flow_limit, &flow_limit);
     dump_thread = dpif_flow_dump_thread_create(udpif->dump);
     for (;;) {
@@ -1484,13 +1485,13 @@ revalidate(struct revalidator *revalidator)
             uint32_t hash = hash_bytes(f->key, f->key_len, udpif->secret);
             struct udpif_key *ukey = ukey_lookup(udpif, f->key, f->key_len,
                                                  hash);
-            bool mark;
+            bool keep;
 
             if (ukey) {
                 bool already_dumped;
 
                 ovs_mutex_lock(&ukey->mutex);
-                already_dumped = ukey->mark || !ukey->flow_exists;
+                already_dumped = ukey->dump_seq == dump_seq;
                 if (!used) {
                     used = ukey->created;
                 }
@@ -1507,7 +1508,7 @@ revalidate(struct revalidator *revalidator)
             }
 
             if (kill_them_all || (used && used < now - max_idle)) {
-                mark = false;
+                keep = false;
             } else {
                 if (!ukey) {
                     ukey = ukey_create(f->key, f->key_len, used);
@@ -1520,16 +1521,17 @@ revalidate(struct revalidator *revalidator)
                     }
                 }
 
-                mark = revalidate_ukey(udpif, ukey, f);
+                keep = revalidate_ukey(udpif, ukey, f);
             }
 
             if (ukey) {
                 ovs_mutex_lock(&ukey->mutex);
-                ukey->mark = ukey->flow_exists = mark;
+                ukey->dump_seq = dump_seq;
+                ukey->flow_exists = keep;
                 ovs_mutex_unlock(&ukey->mutex);
             }
 
-            if (!mark) {
+            if (!keep) {
                 dump_op_init(&ops[n_ops++], f->key, f->key_len, ukey);
             }
         }
@@ -1548,22 +1550,21 @@ revalidator_sweep__(struct revalidator *revalidator, 
bool purge)
     struct dump_op ops[REVALIDATE_MAX_BATCH];
     struct udpif_key *ukey, *next;
     size_t n_ops;
+    uint64_t dump_seq;
 
     n_ops = 0;
+    dump_seq = seq_read(revalidator->udpif->dump_seq);
 
     /* 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) {
-            ukey->mark = false;
-        } else if (!ukey->flow_exists) {
+        if (!ukey->flow_exists) {
             ukey_delete(revalidator, ukey);
-        } else {
+        } else if (purge || ukey->dump_seq != dump_seq) {
             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. */
+            /* If we have previously seen a flow in the datapath, but it
+             * hasn't been revalidated with the current seq,  delete it. */
             dump_op_init(op, ukey->key, ukey->key_len, ukey);
             if (n_ops == REVALIDATE_MAX_BATCH) {
                 push_dump_ops(revalidator, ops, n_ops);
-- 
1.7.10.4

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to