revalidator_purge() iterates and modifies umap->cmap. This should not happen in quiescent state, because cmap implementation based on rcu protected variables. Let's narrow the quiescent period to avoid possible wrong memory accesses.
CC: Joe Stringer <[email protected]> Fixes: 9fce0584a643 ("revalidator: Use 'cmap' for storing ukeys.") Reported-by: Ilya Maximets <[email protected]> Signed-off-by: Ben Pfaff <[email protected]> --- ofproto/ofproto-dpif-upcall.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index e36cfa0eecca..dc3082477106 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -504,34 +504,32 @@ udpif_destroy(struct udpif *udpif) free(udpif); } -/* Stops the handler and revalidator threads, must be enclosed in - * ovsrcu quiescent state unless when destroying udpif. */ +/* Stops the handler and revalidator threads. */ static void udpif_stop_threads(struct udpif *udpif) { if (udpif && (udpif->n_handlers != 0 || udpif->n_revalidators != 0)) { size_t i; + /* Tell the threads to exit. */ latch_set(&udpif->exit_latch); + /* Wait for the threads to exit. Quiesce because this can take a long + * time.. */ + ovsrcu_quiesce_start(); for (i = 0; i < udpif->n_handlers; i++) { - struct handler *handler = &udpif->handlers[i]; - - xpthread_join(handler->thread, NULL); + xpthread_join(udpif->handlers[i].thread, NULL); } - for (i = 0; i < udpif->n_revalidators; i++) { xpthread_join(udpif->revalidators[i].thread, NULL); } - dpif_disable_upcall(udpif->dpif); + ovsrcu_quiesce_end(); + /* Delete ukeys, and delete all flows from the datapath to prevent + * double-counting stats. */ for (i = 0; i < udpif->n_revalidators; i++) { - struct revalidator *revalidator = &udpif->revalidators[i]; - - /* Delete ukeys, and delete all flows from the datapath to prevent - * double-counting stats. */ - revalidator_purge(revalidator); + revalidator_purge(&udpif->revalidators[i]); } latch_poll(&udpif->exit_latch); @@ -549,13 +547,16 @@ udpif_stop_threads(struct udpif *udpif) } } -/* Starts the handler and revalidator threads, must be enclosed in - * ovsrcu quiescent state. */ +/* Starts the handler and revalidator threads. */ static void udpif_start_threads(struct udpif *udpif, size_t n_handlers_, size_t n_revalidators_) { if (udpif && n_handlers_ && n_revalidators_) { + /* Creating a thread can take a significant amount of time on some + * systems, even hundred of milliseconds, so quiesce around it. */ + ovsrcu_quiesce_start(); + udpif->n_handlers = n_handlers_; udpif->n_revalidators = n_revalidators_; @@ -586,6 +587,7 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers_, revalidator->thread = ovs_thread_create( "revalidator", udpif_revalidator, revalidator); } + ovsrcu_quiesce_end(); } } @@ -623,7 +625,6 @@ udpif_set_threads(struct udpif *udpif, size_t n_handlers_, ovs_assert(udpif); ovs_assert(n_handlers_ && n_revalidators_); - ovsrcu_quiesce_start(); if (udpif->n_handlers != n_handlers_ || udpif->n_revalidators != n_revalidators_) { udpif_stop_threads(udpif); @@ -641,7 +642,6 @@ udpif_set_threads(struct udpif *udpif, size_t n_handlers_, udpif_start_threads(udpif, n_handlers_, n_revalidators_); } - ovsrcu_quiesce_end(); } /* Waits for all ongoing upcall translations to complete. This ensures that @@ -657,10 +657,8 @@ udpif_synchronize(struct udpif *udpif) size_t n_handlers_ = udpif->n_handlers; size_t n_revalidators_ = udpif->n_revalidators; - ovsrcu_quiesce_start(); udpif_stop_threads(udpif); udpif_start_threads(udpif, n_handlers_, n_revalidators_); - ovsrcu_quiesce_end(); } /* Notifies 'udpif' that something changed which may render previous @@ -700,13 +698,9 @@ udpif_flush(struct udpif *udpif) size_t n_handlers_ = udpif->n_handlers; size_t n_revalidators_ = udpif->n_revalidators; - ovsrcu_quiesce_start(); - udpif_stop_threads(udpif); dpif_flow_flush(udpif->dpif); udpif_start_threads(udpif, n_handlers_, n_revalidators_); - - ovsrcu_quiesce_end(); } /* Removes all flows from all datapaths. */ -- 2.16.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
