From 44955fb2395677c9d9bb1afa3985b24317c84431 Mon Sep 17 00:00:00 2001
From: Dumitru Ceara <dceara@redhat.com>
Date: Mon, 18 Jan 2021 17:50:23 +0100
Subject: [PATCH 1/2] binding: Fix container port removal from local bindings.

When the Port_Binding associated to a container port is removed make
sure we also remove it from the parent's 'children' shash.  Container
ports don't have any VIFs associated so it's safe to destroy the
container port local binding when the SB.Port_Binding is deleted.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from master commit 68cf9fdceba80ce0c03e3ddb3e0a5531f248fa04)

Change-Id: I65f4bd461f3f94ca90dbcc0646c0037c301c71a1
---
 controller/binding.c | 18 +++++++++++++++++-
 controller/binding.h |  1 +
 tests/ovn.at         | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/controller/binding.c b/controller/binding.c
index e632203..3512a1d 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -688,6 +688,7 @@ local_binding_add_child(struct local_binding *lbinding,
                         struct local_binding *child)
 {
     local_binding_add(&lbinding->children, child);
+    child->parent = lbinding;
 }
 
 static struct local_binding *
@@ -697,6 +698,13 @@ local_binding_find_child(struct local_binding *lbinding,
     return local_binding_find(&lbinding->children, child_name);
 }
 
+static void
+local_binding_delete_child(struct local_binding *lbinding,
+                           struct local_binding *child)
+{
+    shash_find_and_delete(&lbinding->children, child->name);
+}
+
 static bool
 is_lport_vif(const struct sbrec_port_binding *pb)
 {
@@ -2062,6 +2070,14 @@ handle_deleted_vif_lport(const struct sbrec_port_binding *pb,
      * when the interface change happens. */
     if (is_lport_container(pb)) {
         remove_local_lports(pb->logical_port, b_ctx_out);
+
+        /* If the container port is removed we should also remove it from
+         * its parent's children set.
+         */
+        if (lbinding->parent) {
+            local_binding_delete_child(lbinding->parent, lbinding);
+        }
+        local_binding_destroy(lbinding);
     }
 
     handle_deleted_lport(pb, b_ctx_in, b_ctx_out);
@@ -2147,7 +2163,7 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in,
         enum en_lport_type lport_type = get_lport_type(pb);
         if (lport_type == LP_VIF || lport_type == LP_VIRTUAL) {
             handled = handle_deleted_vif_lport(pb, lport_type, b_ctx_in,
-                                                b_ctx_out);
+                                               b_ctx_out);
         } else {
             handle_deleted_lport(pb, b_ctx_in, b_ctx_out);
         }
diff --git a/controller/binding.h b/controller/binding.h
index c974056..2885971 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -100,6 +100,7 @@ struct local_binding {
 
     /* shash of 'struct local_binding' representing children. */
     struct shash children;
+    struct local_binding *parent;
 };
 
 static inline struct local_binding *
diff --git a/tests/ovn.at b/tests/ovn.at
index 27cb2e4..2cdc036 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -22144,6 +22144,59 @@ AT_CHECK_UNQUOTED([grep -c "output:4" offlows_table65_2.txt], [0], [dnl
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 
+AT_SETUP([ovn -- Container port Incremental Processing])
+ovn_start
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.10
+
+as hv1
+ovs-vsctl \
+    -- add-port br-int vif1 \
+    -- set Interface vif1 external_ids:iface-id=lsp1 \
+    ofport-request=1
+
+check ovn-nbctl ls-add ls1 \
+    -- ls-add ls2 \
+    -- lsp-add ls1 lsp1 \
+    -- lsp-add ls2 lsp-cont1 lsp1 1
+check ovn-nbctl --wait=hv sync
+
+# Wait for ports to be bound.
+wait_row_count Chassis 1 name=hv1
+ch=$(fetch_column Chassis _uuid name=hv1)
+wait_row_count Port_Binding 1 logical_port=lsp1 chassis=$ch
+wait_row_count Port_Binding 1 logical_port=lsp-cont1 chassis=$ch
+
+AS_BOX([delete OVS VIF and OVN container port])
+as hv1 ovn-appctl -t ovn-controller debug/pause
+as hv1 ovs-vsctl del-port vif1
+
+check ovn-nbctl --wait=sb lsp-del lsp-cont1
+as hv1 ovn-appctl -t ovn-controller debug/resume
+
+check ovn-nbctl --wait=hv sync
+check_row_count Port_Binding 1 logical_port=lsp1 chassis="[[]]"
+
+AS_BOX([readd OVS VIF])
+as hv1
+ovs-vsctl \
+    -- add-port br-int vif1 \
+    -- set Interface vif1 external_ids:iface-id=lsp1 \
+    ofport-request=1
+wait_row_count Port_Binding 1 logical_port=lsp1 chassis=$ch
+
+AS_BOX([readd OVN container port])
+check ovn-nbctl lsp-add ls2 lsp-cont1 lsp1 1
+check ovn-nbctl --wait=hv sync
+check_row_count Port_Binding 1 logical_port=lsp-cont1 chassis=$ch
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+
 # Test dropping traffic destined to router owned IPs.
 AT_SETUP([ovn -- gateway router drop traffic for own IPs])
 ovn_start
-- 
1.8.3.1