[ovs-dev] [PATCH] ofproto-dpif-upcall: Fix race condition while purging

[Roi Dayan]

From: Jianbo Liu <jianbol at nvidia.com>

There is a race condidtion between purger and handler. Handler may
create new ukey and install it while executing 'ovs-appctl
revalidator/purge' command. However, before handler calls
transition_ukey() in handle_upcalls(), purger may get this ukey from
umap, then evict and delete it. This will trigger ovs_abort in
transition_ukey() for handler because it is trying to set state to
EVICTED or OPERATIONAL, but ukey is already in DELETED state.
To fix this issue, purger must not delete ukey in VISIBLE state.

Fixes: 98bb4286970d ("tests: Add command to purge revalidators of flows.")
Signed-off-by: Jianbo Liu <jianbol at nvidia.com>
Reviewed-by: Roi Dayan <roid at nvidia.com>
---
 ofproto/ofproto-dpif-upcall.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index ccf97266c0b9..3add505ff652 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -327,6 +327,12 @@ struct ukey_op {
     struct dpif_op dop;           /* Flow operation. */
 };
 
+enum sweep_type {
+    PURGE_NONE,
+    PURGE_SOFT,
+    PURGE_HARD,
+};
+
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 static struct ovs_list all_udpifs = OVS_LIST_INITIALIZER(&all_udpifs);
 
@@ -345,7 +351,7 @@ static unsigned long udpif_get_n_flows(struct udpif *);
 static void revalidate(struct revalidator *);
 static void revalidator_pause(struct revalidator *);
 static void revalidator_sweep(struct revalidator *);
-static void revalidator_purge(struct revalidator *);
+static void revalidator_purge(struct revalidator *, enum sweep_type);
 static void upcall_unixctl_show(struct unixctl_conn *conn, int argc,
                                 const char *argv[], void *aux);
 static void upcall_unixctl_disable_megaflows(struct unixctl_conn *, int argc,
@@ -541,7 +547,7 @@ udpif_stop_threads(struct udpif *udpif, bool delete_flows)
 
         if (delete_flows) {
             for (i = 0; i < udpif->n_revalidators; i++) {
-                revalidator_purge(&udpif->revalidators[i]);
+                revalidator_purge(&udpif->revalidators[i], PURGE_HARD);
             }
         }
 
@@ -2772,7 +2778,8 @@ revalidator_pause(struct revalidator *revalidator)
 }
 
 static void
-revalidator_sweep__(struct revalidator *revalidator, bool purge)
+revalidator_sweep__(struct revalidator *revalidator,
+                    enum sweep_type sweep_type)
 {
     struct udpif *udpif;
     uint64_t dump_seq, reval_seq;
@@ -2803,13 +2810,13 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
             }
             ukey_state = ukey->state;
             if (ukey_state == UKEY_OPERATIONAL
-                || (ukey_state == UKEY_VISIBLE && purge)) {
+                || (ukey_state == UKEY_VISIBLE && sweep_type == PURGE_HARD)) {
                 struct recirc_refs recircs = RECIRC_REFS_EMPTY_INITIALIZER;
                 bool seq_mismatch = (ukey->dump_seq != dump_seq
                                      && ukey->reval_seq != reval_seq);
                 enum reval_result result;
 
-                if (purge) {
+                if (sweep_type > PURGE_NONE) {
                     result = UKEY_DELETE;
                 } else if (!seq_mismatch) {
                     result = UKEY_KEEP;
@@ -2856,13 +2863,13 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
 static void
 revalidator_sweep(struct revalidator *revalidator)
 {
-    revalidator_sweep__(revalidator, false);
+    revalidator_sweep__(revalidator, PURGE_NONE);
 }
 
 static void
-revalidator_purge(struct revalidator *revalidator)
+revalidator_purge(struct revalidator *revalidator, enum sweep_type sweep_type)
 {
-    revalidator_sweep__(revalidator, true);
+    revalidator_sweep__(revalidator, sweep_type);
 }
 
 /* In reaction to dpif purge, purges all 'ukey's with same 'pmd_id'. */
@@ -3056,7 +3063,7 @@ upcall_unixctl_purge(struct unixctl_conn *conn, int argc OVS_UNUSED,
         int n;
 
         for (n = 0; n < udpif->n_revalidators; n++) {
-            revalidator_purge(&udpif->revalidators[n]);
+            revalidator_purge(&udpif->revalidators[n], PURGE_SOFT);
         }
     }
     unixctl_command_reply(conn, "");
-- 
2.8.0