@@ -643,23 +643,6 @@ udpif_set_threads(struct udpif *udpif, size_t n_handlers_,
}
}
-/* Waits for all ongoing upcall translations to complete. This ensures that
- * there are no transient references to any removed ofprotos (or other
- * objects). In particular, this should be called after an ofproto is removed
- * (e.g. via xlate_remove_ofproto()) but before it is destroyed. */
-void
-udpif_synchronize(struct udpif *udpif)
-{
- /* This is stronger than necessary. It would be sufficient to ensure
- * (somehow) that each handler and revalidator thread had passed through
- * its main loop once. */
- size_t n_handlers_ = udpif->n_handlers;
- size_t n_revalidators_ = udpif->n_revalidators;
-
- udpif_stop_threads(udpif);
- udpif_start_threads(udpif, n_handlers_, n_revalidators_);
-}
-
/* Notifies 'udpif' that something changed which may render previous
* xlate_actions() results invalid. */
void
@@ -33,7 +33,6 @@ struct udpif *udpif_create(struct dpif_backer *, struct dpif *);
void udpif_run(struct udpif *udpif);
void udpif_set_threads(struct udpif *, size_t n_handlers,
size_t n_revalidators);
-void udpif_synchronize(struct udpif *);
void udpif_destroy(struct udpif *);
void udpif_revalidate(struct udpif *);
void udpif_get_memory_usage(struct udpif *, struct simap *usage);
@@ -1165,11 +1165,15 @@ xlate_xport_copy(struct xbridge *xbridge, struct xbundle *xbundle,
*
* A sample workflow:
*
- * xlate_txn_start();
- * ...
- * edit_xlate_configuration();
- * ...
- * xlate_txn_commit(); */
+ * xlate_txn_start();
+ * ...
+ * edit_xlate_configuration();
+ * ...
+ * xlate_txn_commit();
+ *
+ * The ovsrcu_synchronize() call here also ensures that the upcall threads
+ * retain no references to anything in the previous configuration.
+ */
void
xlate_txn_commit(void)
{
@@ -1706,10 +1706,6 @@ destruct(struct ofproto *ofproto_, bool del)
xlate_remove_ofproto(ofproto);
xlate_txn_commit();
- /* Ensure that the upcall processing threads have no remaining references
- * to the ofproto or anything in it. */
- udpif_synchronize(ofproto->backer->udpif);
-
hmap_remove(&all_ofproto_dpifs_by_name,
&ofproto->all_ofproto_dpifs_by_name_node);
hmap_remove(&all_ofproto_dpifs_by_uuid,
RCU provides the semantics we want from udpif_synchronize() and it should be much more lightweight than killing and restarting all the upcall threads. It looks like udpif_synchronize() was written before the OVS tree had RCU support, which is probably why we didn't use it here from the beginning. So we can just change udpif_synchronize() to a single ovsrcu_synchronize() call. However, udpif_synchronize() only has a single caller, which calls ovsrcu_synchronize() anyway just beforehand, via xlate_txn_commit(). So we can get rid of udpif_synchronize() entirely, which this patch does. As a side effect, this eliminates one reason why terminating OVS cleanly clears the datapath flow table. An upcoming patch will eliminate other reasons. Signed-off-by: Ben Pfaff <blp@ovn.org> --- ofproto/ofproto-dpif-upcall.c | 17 ----------------- ofproto/ofproto-dpif-upcall.h | 1 - ofproto/ofproto-dpif-xlate.c | 14 +++++++++----- ofproto/ofproto-dpif.c | 4 ---- 4 files changed, 9 insertions(+), 27 deletions(-)