When 'ovs-appctl revalidator/purge' is called, the main thread
sweeps and destroys all ukeys and the associated datapath flows.
If, at the same time, revalidators are dumping those flows from
datapath, the ukey lookup of dumped flows could fail due to
deletion by main thread.  This race will also cause the mistaken
issue of few warnings.

To fix the above issue, this commit adds a mutex which must be
acquired during the revalidation cycle by the leader revalidator,
and during the purge by the main thread.  Thusly, the two operations
will never overlap.

Signed-off-by: Alex Wang <al...@nicira.com>
---
 ofproto/ofproto-dpif-upcall.c |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 6feaa75..e218f8e 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -104,6 +104,8 @@ struct udpif {
 
     struct revalidator *revalidators;  /* Flow revalidators. */
     size_t n_revalidators;
+    struct ovs_mutex revalidate_mutex; /* Must be taken during revalidation */
+                                       /* cycle or during purge. */
 
     struct latch exit_latch;           /* Tells child threads to exit. */
 
@@ -326,6 +328,7 @@ udpif_create(struct dpif_backer *backer, struct dpif *dpif)
     atomic_init(&udpif->flow_limit, MIN(ofproto_flow_limit, 10000));
     udpif->reval_seq = seq_create();
     udpif->dump_seq = seq_create();
+    ovs_mutex_init(&udpif->revalidate_mutex);
     latch_init(&udpif->exit_latch);
     list_push_back(&all_udpifs, &udpif->list_node);
     atomic_init(&udpif->enable_ufid, false);
@@ -372,6 +375,7 @@ udpif_destroy(struct udpif *udpif)
 
     list_remove(&udpif->list_node);
     latch_destroy(&udpif->exit_latch);
+    ovs_mutex_destroy(&udpif->revalidate_mutex);
     seq_destroy(udpif->reval_seq);
     seq_destroy(udpif->dump_seq);
     ovs_mutex_destroy(&udpif->n_flows_mutex);
@@ -709,7 +713,7 @@ free_dupcall:
 }
 
 static void *
-udpif_revalidator(void *arg)
+udpif_revalidator(void *arg) OVS_NO_THREAD_SAFETY_ANALYSIS
 {
     /* Used by all revalidators. */
     struct revalidator *revalidator = arg;
@@ -726,6 +730,8 @@ udpif_revalidator(void *arg)
         if (leader) {
             uint64_t reval_seq;
 
+            ovs_mutex_lock(&udpif->revalidate_mutex);
+
             reval_seq = seq_read(udpif->reval_seq);
             last_reval_seq = reval_seq;
 
@@ -750,6 +756,9 @@ udpif_revalidator(void *arg)
         /* Wait for the leader to start the flow dump. */
         ovs_barrier_block(&udpif->reval_barrier);
         if (udpif->reval_exit) {
+            if (leader) {
+                ovs_mutex_unlock(&udpif->revalidate_mutex);
+            }
             break;
         }
         revalidate(revalidator);
@@ -788,6 +797,8 @@ udpif_revalidator(void *arg)
                           duration);
             }
 
+            ovs_mutex_unlock(&udpif->revalidate_mutex);
+
             poll_timer_wait_until(start_time + MIN(ofproto_max_idle, 500));
             seq_wait(udpif->reval_seq, last_reval_seq);
             latch_wait(&udpif->exit_latch);
@@ -2150,9 +2161,13 @@ upcall_unixctl_purge(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
     LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
         int n;
 
+        /* Prevents the revalidators from lookup/modify the ukeys
+         * at the same time. */
+        ovs_mutex_lock(&udpif->revalidate_mutex);
         for (n = 0; n < udpif->n_revalidators; n++) {
             revalidator_purge(&udpif->revalidators[n]);
         }
+        ovs_mutex_unlock(&udpif->revalidate_mutex);
     }
     unixctl_command_reply(conn, "");
 }
-- 
1.7.9.5

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

Reply via email to