diff mbox series

[ovs-dev] ofproto-dpif-upcall: try lock for umap iteration during sweep

Message ID 20240314020510.1923537-1-i@liuyulong.me
State Superseded
Headers show
Series [ovs-dev] ofproto-dpif-upcall: try lock for umap iteration during sweep | expand

Commit Message

LIU Yulong March 14, 2024, 2:05 a.m. UTC
A potential race condition happened with the following 3 threads:
* PMD thread replaced the old_ukey and transitioned the state to
  UKEY_DELETED.
* RCU thread is freeing the old_ukey mutex.
* While the revalidator thread is trying to lock the old_ukey mutex.
Then vswitchd process aborts at the revalidator thread try_lock of
ukey->mutex because of the NULL pointer.

This patch adds the try_lock for the ukeys' basket umap to avoid
the PMD and revalidator access to the same umap for replacing the
ukey and transitioning the ukey state.

More details can be found at:
[1] https://mail.openvswitch.org/pipermail/ovs-discuss/2024-March/052973.html
[2] https://mail.openvswitch.org/pipermail/ovs-discuss/2024-February/052949.html

Signed-off-by: LIU Yulong <i@liuyulong.me>
---
 ofproto/ofproto-dpif-upcall.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 9a5c5c29c..ef13f820a 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2974,6 +2974,10 @@  revalidator_sweep__(struct revalidator *revalidator, bool purge)
         struct umap *umap = &udpif->ukeys[i];
         size_t n_ops = 0;
 
+        if (ovs_mutex_trylock(&umap->mutex)) {
+            continue;
+        }
+
         CMAP_FOR_EACH(ukey, cmap_node, &umap->cmap) {
             enum ukey_state ukey_state;
 
@@ -3013,9 +3017,7 @@  revalidator_sweep__(struct revalidator *revalidator, bool purge)
             if (ukey_state == UKEY_EVICTED) {
                 /* The common flow deletion case involves deletion of the flow
                  * during the dump phase and ukey deletion here. */
-                ovs_mutex_lock(&umap->mutex);
                 ukey_delete(umap, ukey);
-                ovs_mutex_unlock(&umap->mutex);
             }
 
             if (n_ops == REVALIDATE_MAX_BATCH) {
@@ -3025,6 +3027,7 @@  revalidator_sweep__(struct revalidator *revalidator, bool purge)
                 n_ops = 0;
             }
         }
+        ovs_mutex_unlock(&umap->mutex);
 
         if (n_ops) {
             push_ukey_ops(udpif, umap, ops, n_ops);