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