ovn: Remove unreferenced patched datapaths.

Patched datapaths that are no longer referenced should be removed from
the patched_datapaths map; otherwise incorrect state references for a
patched datapath may be used and also datapaths that are absent will be
interpreted as present.

Signed-off-by: Darrell Ball <dlu998@gmail.com>
Acked-by: Ryan Moats <rmoats@us.ibm.com>
Signed-off-by: Gurucharan Shetty <guru@ovn.org>

diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 6a021a0..f3edc43 100644 (file)
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -51,8 +51,10 @@ struct local_datapath *get_local_datapath(const struct hmap *,
  * with at least one logical patch port binding. */
 struct patched_datapath {
     struct hmap_node hmap_node;
-    bool local; /* 'True' if the datapath is for gateway router. */
     const struct sbrec_port_binding *port_binding;
+    bool local; /* 'True' if the datapath is for gateway router. */
+    bool stale; /* 'True' if the datapath is not referenced by any patch
+                 * port. */
 };
 
 struct patched_datapath *get_patched_datapath(const struct hmap *,

diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 589529e..1906cf3 100644 (file)
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -252,18 +252,50 @@ static void
 add_patched_datapath(struct hmap *patched_datapaths,
                      const struct sbrec_port_binding *binding_rec, bool local)
 {
-    if (get_patched_datapath(patched_datapaths,
-                             binding_rec->datapath->tunnel_key)) {
+    struct patched_datapath *pd = get_patched_datapath(patched_datapaths,
+                                       binding_rec->datapath->tunnel_key);
+    if (pd) {
+        /* If the patched datapath is referenced by a logical patch port it is
+         * not stale, by definition, so set 'stale' to false */
+        pd->stale = false;
         return;
     }
 
-    struct patched_datapath *pd = xzalloc(sizeof *pd);
+    pd = xzalloc(sizeof *pd);
     pd->local = local;
     pd->port_binding = binding_rec;
+    /* stale is set to false. */
     hmap_insert(patched_datapaths, &pd->hmap_node,
                 binding_rec->datapath->tunnel_key);
 }
 
+static void
+add_logical_patch_ports_preprocess(struct hmap *patched_datapaths)
+{
+    /* Mark all patched datapaths as stale for later cleanup by
+     * add_logical_patch_ports_postprocess(). */
+    struct patched_datapath *pd;
+    HMAP_FOR_EACH (pd, hmap_node, patched_datapaths) {
+        pd->stale = true;
+    }
+}
+
+/* This function should cleanup stale patched datapaths and any memory
+ * allocated for fields within a stale patched datapath. */
+static void
+add_logical_patch_ports_postprocess(struct hmap *patched_datapaths)
+{
+    /* Clean up stale patched datapaths. */
+    struct patched_datapath *pd_cur_node, *pd_next_node;
+    HMAP_FOR_EACH_SAFE (pd_cur_node, pd_next_node, hmap_node,
+                        patched_datapaths) {
+        if (pd_cur_node->stale == true) {
+            hmap_remove(patched_datapaths, &pd_cur_node->hmap_node);
+            free(pd_cur_node);
+        }
+    }
+}
+
 /* Add one OVS patch port for each OVN logical patch port.
  *
  * This is suboptimal for several reasons.  First, it creates an OVS port for
@@ -299,6 +331,8 @@ add_logical_patch_ports(struct controller_ctx *ctx,
         return;
     }
 
+    add_logical_patch_ports_preprocess(patched_datapaths);
+
     const struct sbrec_port_binding *binding;
     SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
         bool local_port = false;
@@ -332,6 +366,7 @@ add_logical_patch_ports(struct controller_ctx *ctx,
             }
         }
     }
+    add_logical_patch_ports_postprocess(patched_datapaths);
 }
 
 void