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