[ovs-dev] [PATCH] ovn-controller: Drop remove_local_datapath_by_binding().

[Russell Bryant]

ovn-controller has an hmap called 'local_datapaths' which tracks
all OVN datapaths that have at least one port binding on the local
chassis.  This patch corrects the case where a port binding row is
deleted from the southbound DB while it's still bound to the chassis,
meaning it was deleted before the ovs interface was deleted.

The previous code tried to handle this case by calling
remove_local_datapath_by_binding().  The function appears to try
to look up local_datapath by the binding UUID.  If it finds it,
it will delete the local datapath entry.  On the surface, this
looks like a bug where it deletes a local datapath entry even
when there could be other ports still bound to the chassis.
The reality is that this function was always a no-op.  It was
doing a lookup using a different hash value than how local_datapath
entries are actually hashed. In practice, this wasn't a big problem
because local_datapaths are correctly cleaned in in the
process_full_binding case after an ovs interface is added or removed.

The new change ensures that we run the process_full_binding code
in this case right away, even if the interface is not deleted.

Fixes: 263064aeaa31 ("Convert binding_run to incremental processing.")
Signed-off-by: Russell Bryant <russell at ovn.org>
---
 ovn/controller/binding.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index adf4127..bc6df32 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -18,6 +18,7 @@
 
 #include "lib/bitmap.h"
 #include "lib/hmap.h"
+#include "lib/poll-loop.h"
 #include "lib/sset.h"
 #include "lib/util.h"
 #include "lib/vswitch-idl.h"
@@ -131,25 +132,6 @@ remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld)
 }
 
 static void
-remove_local_datapath_by_binding(struct hmap *local_datapaths,
-                                 const struct sbrec_port_binding *binding_rec)
-{
-    const struct uuid *uuid = &binding_rec->header_.uuid;
-    struct local_datapath *ld = local_datapath_lookup_by_uuid(local_datapaths,
-                                                              uuid);
-    if (ld) {
-        remove_local_datapath(local_datapaths, ld);
-    } else {
-        struct local_datapath *ld;
-        HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
-            if (ld->localnet_port == binding_rec) {
-                ld->localnet_port = NULL;
-            }
-        }
-    }
-}
-
-static void
 add_local_datapath(struct hmap *local_datapaths,
         const struct sbrec_port_binding *binding_rec)
 {
@@ -289,7 +271,14 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
     } else {
         SBREC_PORT_BINDING_FOR_EACH_TRACKED(binding_rec, ctx->ovnsb_idl) {
             if (sbrec_port_binding_is_deleted(binding_rec)) {
-                remove_local_datapath_by_binding(local_datapaths, binding_rec);
+                /* If a port binding was bound to this chassis and removed before
+                 * the ovs interface was removed, we'll catch that here and trigger
+                 * a full bindings refresh.  This is to see if we need to clear
+                 * an entry out of local_datapaths. */
+                if (binding_rec->chassis == chassis_rec) {
+                    process_full_binding = true;
+                    poll_immediate_wake();
+                }
             } else {
                 consider_local_datapath(ctx, chassis_rec, binding_rec,
                                         local_datapaths, &lport_to_iface);
-- 
2.7.4