From 36a57e7a388277d1e45f0cadd8e2601490a76b2d Mon Sep 17 00:00:00 2001

From: Dumitru Ceara <dceara@redhat.com>

Date: Wed, 3 Feb 2021 20:36:30 +0100

Subject: [PATCH 1/4] binding: Correctly set Port_Binding.up for

 container/virtual ports.

For port bindings that are children of other port bindings (container

and virtual ports) set Port_Binding.up directly, when claimed, if their

parent bindings are already up.

For non-VIF port bindings maintain compatibility with older versions

and set Port_Binding.up as soon as they are claimed.

Reported-by: Ben Pfaff <blp@ovn.org>

Fixes: 4d3cb42b076b ("binding: Set Logical_Switch_Port.up when all OVS flows are installed.")

Signed-off-by: Dumitru Ceara <dceara@redhat.com>

Signed-off-by: Numan Siddique <numans@ovn.org>

(cherry picked from upstream commit aae25e67b1ba86e34d670eb808de86f7ba66c3c0)

Conflicts:

	tests/ovn.at

Change-Id: I17c5a4d0f89868190c81c7221caadf29959064b1

---

 controller-vtep/binding.c |  3 +++

 controller/binding.c      | 47 +++++++++++++++++++++++++++++++++++++++++------

 tests/ovn.at              | 32 ++++++++++++++++++++++++++------

 3 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/controller-vtep/binding.c b/controller-vtep/binding.c

index 8337715..d28a598 100644

--- a/controller-vtep/binding.c

+++ b/controller-vtep/binding.c

@@ -109,7 +109,10 @@ update_pb_chassis(const struct sbrec_port_binding *port_binding_rec,

                      port_binding_rec->chassis->name,

                      chassis_rec->name);

         }

+

+        bool up = true;

         sbrec_port_binding_set_chassis(port_binding_rec, chassis_rec);

+        sbrec_port_binding_set_up(port_binding_rec, &up, 1);

     }

 }

diff --git a/controller/binding.c b/controller/binding.c

index d44f635..353debe 100644

--- a/controller/binding.c

+++ b/controller/binding.c

@@ -864,21 +864,52 @@ get_lport_type(const struct sbrec_port_binding *pb)

     return LP_UNKNOWN;

 }

+/* For newly claimed ports, if 'notify_up' is 'false':

+ * - set the 'pb.up' field to true if 'pb' has no 'parent_pb'.

+ * - set the 'pb.up' field to true if 'parent_pb.up' is 'true' (e.g., for

+ *   container and virtual ports).

+ * Otherwise request a notification to be sent when the OVS flows

+ * corresponding to 'pb' have been installed.

+ */

+static void

+claimed_lport_set_up(const struct sbrec_port_binding *pb,

+                     const struct sbrec_port_binding *parent_pb,

+                     const struct sbrec_chassis *chassis_rec,

+                     bool notify_up)

+{

+    if (!notify_up) {

+        bool up = true;

+        if (!parent_pb || (parent_pb->n_up && parent_pb->up[0])) {

+            sbrec_port_binding_set_up(pb, &up, 1);

+        }

+        return;

+    }

+

+    if (pb->chassis != chassis_rec) {

+        binding_iface_bound_add(pb->logical_port);

+    }

+}

+

 /* Returns false if lport is not claimed due to 'sb_readonly'.

  * Returns true otherwise.

  */

 static bool

 claim_lport(const struct sbrec_port_binding *pb,

+            const struct sbrec_port_binding *parent_pb,

             const struct sbrec_chassis *chassis_rec,

             const struct ovsrec_interface *iface_rec,

-            bool sb_readonly, struct hmap *tracked_datapaths)

+            bool sb_readonly, bool notify_up,

+            struct hmap *tracked_datapaths)

 {

+    if (!sb_readonly) {

+        claimed_lport_set_up(pb, parent_pb, chassis_rec, notify_up);

+    }

+

     if (pb->chassis != chassis_rec) {

         if (sb_readonly) {

             return false;

         }

-        binding_iface_bound_add(pb->logical_port);

         if (pb->chassis) {

             VLOG_INFO("Changing chassis for lport %s from %s to %s.",

                     pb->logical_port, pb->chassis->name,

@@ -1041,8 +1072,12 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,

     if (lbinding_set) {

         if (can_bind) {

             /* We can claim the lport. */

-            if (!claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface,

-                             !b_ctx_in->ovnsb_idl_txn,

+            const struct sbrec_port_binding *parent_pb =

+                lbinding->parent ? lbinding->parent->pb : NULL;

+

+            if (!claim_lport(pb, parent_pb, b_ctx_in->chassis_rec,

+                             lbinding->iface, !b_ctx_in->ovnsb_idl_txn,

+                             !lbinding->parent,

                              b_ctx_out->tracked_dp_bindings)){

                 return false;

             }

@@ -1246,8 +1281,8 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb,

                            b_ctx_out->tracked_dp_bindings);

         update_local_lport_ids(pb, b_ctx_out);

-        return claim_lport(pb, b_ctx_in->chassis_rec, NULL,

-                           !b_ctx_in->ovnsb_idl_txn,

+        return claim_lport(pb, NULL, b_ctx_in->chassis_rec, NULL,

+                           !b_ctx_in->ovnsb_idl_txn, false,

                            b_ctx_out->tracked_dp_bindings);

     } else if (pb->chassis == b_ctx_in->chassis_rec) {

         return release_lport(pb, !b_ctx_in->ovnsb_idl_txn,

diff --git a/tests/ovn.at b/tests/ovn.at

index dfb94d2..1956f5c 100644

--- a/tests/ovn.at

+++ b/tests/ovn.at

@@ -8992,6 +8992,7 @@ AT_CHECK([test  -z $bar2_zoneid])

 # Add back bar2

 ovn-nbctl lsp-add bar bar2 vm2 1 \

 -- lsp-set-addresses bar2 "f0:00:00:01:02:08 192.168.2.3"

+wait_for_ports_up

 ovn-nbctl --wait=hv sync

 bar2_zoneid=$(as hv2 ovs-vsctl get bridge br-int external_ids:ct-zone-bar2)

@@ -14671,6 +14672,8 @@ OVS_WAIT_UNTIL(

 logical_port=ls1-lp_ext1`

     test "$chassis" = "$hv1_uuid"])

+wait_for_ports_up ls1-lp_ext1

+

 # There should be DHCPv4/v6 OF flows for the ls1-lp_ext1 port in hv1

 (ovn-sbctl dump-flows lr0; ovn-sbctl dump-flows ls1) > sbflows

 as hv1 ovs-ofctl dump-flows br-int > brintflows

@@ -14951,6 +14954,7 @@ OVS_WAIT_UNTIL(

     [chassis=`ovn-sbctl --bare --columns chassis find port_binding \

 logical_port=ls1-lp_ext1`

     test "$chassis" = "$hv2_uuid"])

+wait_for_ports_up ls1-lp_ext1

 # There should be OF flows for DHCP4/v6 for the ls1-lp_ext1 port in hv2

 AT_CHECK([as hv2 ovs-ofctl dump-flows br-int | \

@@ -15065,6 +15069,7 @@ OVS_WAIT_UNTIL(

     [chassis=`ovn-sbctl --bare --columns chassis find port_binding \

 logical_port=ls1-lp_ext1`

     test "$chassis" = "$hv1_uuid"])

+wait_for_ports_up ls1-lp_ext1

 as hv1

 ovs-vsctl show

@@ -15145,6 +15150,7 @@ OVS_WAIT_UNTIL(

     [chassis=`ovn-sbctl --bare --columns chassis find port_binding \

 logical_port=ls1-lp_ext1`

     test "$chassis" = "$hv3_uuid"])

+wait_for_ports_up ls1-lp_ext1

 as hv1

 ovs-vsctl show

@@ -15229,6 +15235,7 @@ OVS_WAIT_UNTIL(

     [chassis=`ovn-sbctl --bare --columns chassis find port_binding \

 logical_port=ls1-lp_ext1`

     test "$chassis" = "$hv1_uuid"])

+wait_for_ports_up ls1-lp_ext1

 # There should be a flow in hv2 to drop traffic from ls1-lp_ext1 destined

 # to router mac.

@@ -15246,6 +15253,7 @@ OVS_WAIT_UNTIL(

     [chassis=`ovn-sbctl --bare --columns chassis find port_binding \

 logical_port=ls1-lp_ext1`

     test "$chassis" = "$hv2_uuid"])

+wait_for_ports_up ls1-lp_ext1

 as hv1

 OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])

@@ -16604,12 +16612,10 @@ spa=$(ip_to_hex 10 0 0 10)

 tpa=$(ip_to_hex 10 0 0 10)

 send_garp 1 1 $eth_src $eth_dst $spa $tpa

-OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \

-logical_port=sw0-vir) = x$hv1_ch_uuid], [0], [])

-

-AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \

-logical_port=sw0-vir) = xsw0-p1])

-

+wait_row_count Port_Binding 1 logical_port=sw0-vir chassis=$hv1_ch_uuid

+check_row_count Port_Binding 1 logical_port=sw0-vir virtual_parent=sw0-p1

+wait_for_ports_up sw0-vir

+check ovn-nbctl --wait=hv sync

 # There should be an arp resolve flow to resolve the virtual_ip with the

 # sw0-p1's MAC.

@@ -16627,6 +16633,8 @@ ovn-sbctl clear port_binding $pb_uuid virtual_parent

 OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \

 logical_port=sw0-vir) = x])

+wait_row_count nb:Logical_Switch_Port 1 up=false name=sw0-vir

+

 # From sw0-p0 resend GARP for 10.0.0.10. hv1 should reclaim sw0-vir

 # and sw0-p1 should be its virtual_parent.

 send_garp 1 1 $eth_src $eth_dst $spa $tpa

@@ -16637,6 +16645,8 @@ logical_port=sw0-vir) = x$hv1_ch_uuid], [0], [])

 AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \

 logical_port=sw0-vir) = xsw0-p1])

+wait_for_ports_up sw0-vir

+

 # From sw0-p3 send GARP for 10.0.0.10. hv1 should claim sw0-vir

 # and sw0-p3 should be its virtual_parent.

 eth_src=505400000005

@@ -16651,6 +16661,7 @@ logical_port=sw0-vir) = x$hv1_ch_uuid], [0], [])

 OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \

 logical_port=sw0-vir) = xsw0-p3])

+wait_for_ports_up sw0-vir

 # There should be an arp resolve flow to resolve the virtual_ip with the

 # sw0-p2's MAC.

@@ -16676,6 +16687,7 @@ logical_port=sw0-vir) = x$hv2_ch_uuid], [0], [])

 AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \

 logical_port=sw0-vir) = xsw0-p2])

+wait_for_ports_up sw0-vir

 # There should be an arp resolve flow to resolve the virtual_ip with the

 # sw0-p3's MAC.

@@ -16701,6 +16713,8 @@ sleep 1

 AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \

 logical_port=sw0-vir) = xsw0-p1])

+wait_for_ports_up sw0-vir

+

 ovn-sbctl dump-flows lr0 > lr0-flows5

 AT_CAPTURE_FILE([lr0-flows5])

 AT_CHECK([grep lr_in_arp_resolve lr0-flows5 | grep "reg0 == 10.0.0.10" | sed 's/table=../table=??/'], [0], [dnl

@@ -16717,6 +16731,8 @@ sleep 1

 AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \

 logical_port=sw0-vir) = x])

+wait_row_count nb:Logical_Switch_Port 1 up=false name=sw0-vir

+

 # Since the sw0-vir is not claimed by any chassis, eth.dst should be set to

 # zero if the ip4.dst is the virtual ip.

 ovn-sbctl dump-flows lr0 > lr0-flows6

@@ -16740,6 +16756,8 @@ sleep 1

 AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \

 logical_port=sw0-vir) = xsw0-p2])

+wait_for_ports_up sw0-vir

+

 ovn-sbctl dump-flows lr0 > lr0-flows7

 AT_CAPTURE_FILE([lr0-flows7])

 AT_CHECK([grep lr_in_arp_resolve lr0-flows7 | grep "reg0 == 10.0.0.10" | sed 's/table=../table=??/'], [0], [dnl

@@ -16754,6 +16772,8 @@ logical_port=sw0-vir) = x], [0], [])

 AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \

 logical_port=sw0-vir) = x])

+wait_row_count nb:Logical_Switch_Port 1 up=false name=sw0-vir

+

 # Clear virtual_ip column of sw0-vir. There should be no bind_vport flows.

 ovn-nbctl --wait=hv remove logical_switch_port sw0-vir options virtual-ip

-- 

1.8.3.1