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 <b...@ovn.org>
Fixes: 4d3cb42b076b ("binding: Set Logical_Switch_Port.up when all OVS flows
are installed.")
Signed-off-by: Dumitru Ceara <dce...@redhat.com>
---
controller-vtep/binding.c | 3 +++
controller/binding.c | 47 +++++++++++++++++++++++++++++++++++++++------
tests/ovn.at | 24 ++++++++++++++++++++++-
3 files changed, 67 insertions(+), 7 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 e814c18..db86183 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -8923,9 +8923,9 @@ bar2_zoneid=$(as hv2 ovs-vsctl get bridge br-int
external_ids:ct-zone-bar2)
AT_CHECK([test -z $bar2_zoneid])
# Add back bar2
-wait_for_ports_up
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)
@@ -14726,6 +14726,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
@@ -15006,6 +15008,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 | \
@@ -15120,6 +15123,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
@@ -15200,6 +15204,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
@@ -15284,6 +15289,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.
@@ -15301,6 +15307,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])
@@ -16667,6 +16674,7 @@ send_garp 1 1 $eth_src $eth_dst $spa $tpa
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
@@ -16685,6 +16693,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
@@ -16695,6 +16705,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
@@ -16709,6 +16721,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.
@@ -16734,6 +16747,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.
@@ -16759,6 +16773,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
@@ -16775,6 +16791,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
@@ -16798,6 +16816,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
@@ -16812,6 +16832,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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
