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