Thanks for the update! Acked-by: Mark Michelson <[email protected]>
I pushed the change to main. On Sat, Oct 17, 2026 at 12:58 AM Rosemarie O'Riorden <[email protected]> wrote: > > ovn-controller would continue with the next iteration before waiting > for patch ports to finish updating. > > Thus when using --wait=hv for a localnet port operation, ovn-nbctl would > not actually wait for patch ports. This sometimes led to failures in > the "localnet port change and chassisredirect bridged redirect" test, > making it flaky. > > To remedy this issue, functions performing patch port operations now > report a status, and ovn-controller will not increment nb_cfg if they > show to be incomplete. This allows --wait=hv to actually wait as > intended. > > Also remove OVN_WAIT_PATCH_PORT_FLOWS, which is no longer needed > now that --wait=hv properly waits for patch ports, and replace all > callers with "check ovn-nbctl --wait=hv sync". > > Fixes: 84748d013155 ("ovn: Make it possible for CMS to detect when the OVN > system is up-to-date.") > Reported-at: https://issues.redhat.com/browse/FDP-4156 > Signed-off-by: Rosemarie O'Riorden <[email protected]> > --- > As discussed on the mailing list, this is intended as a temporary fix for a > larger issue. In the future I intend to post another patch fixing this same > issue more generally with ofctrl-seqno, as this affects more than just patch > ports. > > v4: > Made create_patch_port() return an ENUM instead of a bool because it really > does have three return cases. > > v3: > - Move loop checking ofport numbers into patch_run(). > - Remove test macro OVN_WAIT_PATCH_PORT_FLOWS and replace it with > "check ovn-nbctl --wait=hv sync". > - Add localnet port deletion to test. > - Re-add fix for broken test that was handled in v1. > - Fix Fixes tag. > > v2: > - Also check that patch port interfaces have a positive ofport (not just that > patch_run() didn't create/delete ports), so nb_cfg is held until OVS > actually installs the ports. > - Extract find_patch_ports() from patch_run() for use in ovn-controller.c. > - Replace modified existing test with a new dedicated test. > --- > controller/ovn-controller.c | 39 ++++++---- > controller/patch.c | 135 ++++++++++++++++++++++++++------- > controller/patch.h | 2 +- > tests/ovn-macros.at | 22 ------ > tests/ovn.at | 147 ++++++++++++++++++++++++++---------- > tests/system-ovn.at | 2 +- > 6 files changed, 242 insertions(+), 105 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index f5ee8b75a..75a1dfbea 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -8591,16 +8591,19 @@ main(int argc, char *argv[]) > } > } > > + bool patch_ports_synced = true; > + > runtime_data = engine_get_data(&en_runtime_data); > if (runtime_data) { > stopwatch_start(PATCH_RUN_STOPWATCH_NAME, > time_msec()); > - patch_run(ovs_idl_txn, > - sbrec_port_binding_by_type, > + patch_ports_synced = patch_run( > + ovs_idl_txn, sbrec_port_binding_by_type, > ovsrec_bridge_table_get(ovs_idl_loop.idl), > ovsrec_open_vswitch_table_get(ovs_idl_loop.idl), > - ovsrec_port_by_name, > - br_int, chassis, &runtime_data->local_datapaths); > + ovsrec_port_by_name, br_int, chassis, > + &runtime_data->local_datapaths); > stopwatch_stop(PATCH_RUN_STOPWATCH_NAME, > time_msec()); > + > if (vif_plug_provider_has_providers() && > ovs_idl_txn) { > struct vif_plug_ctx_in vif_plug_ctx_in = { > .ovs_idl_txn = ovs_idl_txn, > @@ -8727,18 +8730,22 @@ main(int argc, char *argv[]) > chassis, mac_cache_data); > } > > - /* Snapshot (nb_cfg, sb_ts) atomically from SB_Global > - * and pair them through the barrier ack so the > - * eventual completion can be attributed to the > - * timestamp that corresponded to this exact nb_cfg > - * generation -- not whatever SB_Global value has > - * moved on to by the time the barrier acks. */ > - struct nb_cfg_snap snap = get_nb_cfg( > - sbrec_sb_global_table_get(ovnsb_idl_loop.idl), > - ovnsb_cond_seqno, ovnsb_expected_cond_seqno); > - > ofctrl_stamped_seqno_update_create(ofctrl_seq_type_nb_cfg, > - snap.nb_cfg, > - snap.ts); > + /* Wait for patch ports to be installed and synced before > + * incrementing nb_cfg so that --wait=hv properly waits > + * for patch ports. */ > + if (patch_ports_synced) { > + /* Snapshot (nb_cfg, sb_ts) atomically from SB_Global > + * and pair them through the barrier ack so the > + * eventual completion can be attributed to the > + * timestamp that corresponded to this exact nb_cfg > + * generation -- not whatever SB_Global value has > + * moved on to by the time the barrier acks. */ > + struct nb_cfg_snap snap = get_nb_cfg( > + sbrec_sb_global_table_get(ovnsb_idl_loop.idl), > + ovnsb_cond_seqno, ovnsb_expected_cond_seqno); > + ofctrl_stamped_seqno_update_create( > + ofctrl_seq_type_nb_cfg, snap.nb_cfg, snap.ts); > + } > > struct local_binding_data *binding_data = > runtime_data ? &runtime_data->lbinding_data : NULL; > diff --git a/controller/patch.c b/controller/patch.c > index 4fed6e375..230dc4763 100644 > --- a/controller/patch.c > +++ b/controller/patch.c > @@ -34,6 +34,12 @@ VLOG_DEFINE_THIS_MODULE(patch); > /* Contains list of physical bridges that were missing. */ > static struct sset missed_bridges; > > +enum create_patch_port_result { > + PATCH_PORT_EXISTS, > + PATCH_PORT_INSERTED, > + PATCH_PORT_INSERTION_FAILED, > +}; > + > void > patch_init(void) > { > @@ -73,8 +79,14 @@ match_patch_port(const struct ovsrec_port *port, const > char *peer) > * 'dst_name' in bridge 'dst'. Initializes the patch port's > external-ids:'key' > * to 'key'. > * > - * If such a patch port already exists, removes it from 'existing_ports'. */ > -static void > + * If such a patch port already exists in 'src', removes it from > + * 'existing_ports' and returns PATCH_PORT_EXISTS. > + * > + * The port is only created if 'ovs_idl_txn' is non-NULL. If NULL, returns > + * PATCH_PORT_INSERTION_FAILED. > + * > + * Otherwise, creates port and returns PATCH_PORT_INSERTED. */ > +static enum create_patch_port_result > create_patch_port(struct ovsdb_idl_txn *ovs_idl_txn, > const char *key, const char *value, > const struct ovsrec_bridge *src, const char *src_name, > @@ -85,10 +97,14 @@ create_patch_port(struct ovsdb_idl_txn *ovs_idl_txn, > if (match_patch_port(src->ports[i], dst_name)) { > /* Patch port already exists on 'src'. */ > shash_find_and_delete(existing_ports, src->ports[i]->name); > - return; > + return PATCH_PORT_EXISTS; > } > } > > + if (!ovs_idl_txn) { > + return PATCH_PORT_INSERTION_FAILED; > + } > + > ovsdb_idl_txn_add_comment(ovs_idl_txn, > "ovn-controller: creating patch port '%s' from '%s' to '%s'", > src_name, src->name, dst->name); > @@ -97,6 +113,7 @@ create_patch_port(struct ovsdb_idl_txn *ovs_idl_txn, > const struct smap port_ids = SMAP_CONST1(&port_ids, key, value); > ovsport_create(ovs_idl_txn, src, src_name, "patch", &port_ids, NULL, > &if_options, 0); > + return PATCH_PORT_INSERTED; > } > > static void > @@ -165,7 +182,12 @@ add_ovs_bridge_mappings(const struct > ovsrec_open_vswitch_table *ovs_table, > } > } > > -static void > +/* Adds the patch ports needed by the port bindings of type 'pb_type' that > are > + * local to this chassis. > + * > + * Returns true if all of those patch ports are already present in the > + * database. */ > +static bool > add_bridge_mappings_by_type(struct ovsdb_idl_txn *ovs_idl_txn, > struct ovsdb_idl_index > *sbrec_port_binding_by_type, > const struct ovsrec_bridge *br_int, > @@ -178,6 +200,8 @@ add_bridge_mappings_by_type(struct ovsdb_idl_txn > *ovs_idl_txn, > { > struct sbrec_port_binding *target = > sbrec_port_binding_index_init_row(sbrec_port_binding_by_type); > + bool synced = true; > + > sbrec_port_binding_index_set_type(target, pb_type); > > const struct sbrec_port_binding *binding; > @@ -225,20 +249,33 @@ add_bridge_mappings_by_type(struct ovsdb_idl_txn > *ovs_idl_txn, > > char *name1 = patch_port_name(br_int->name, binding->logical_port); > char *name2 = patch_port_name(binding->logical_port, br_int->name); > - create_patch_port(ovs_idl_txn, patch_port_id, binding->logical_port, > - br_int, name1, br_ln, name2, existing_ports); > - create_patch_port(ovs_idl_txn, patch_port_id, binding->logical_port, > - br_ln, name2, br_int, name1, existing_ports); > + enum create_patch_port_result br_int_result; > + enum create_patch_port_result br_ln_result; > + br_int_result = create_patch_port(ovs_idl_txn, patch_port_id, > + binding->logical_port, > + br_int, name1, br_ln, name2, > + existing_ports); > + br_ln_result = create_patch_port(ovs_idl_txn, patch_port_id, > + binding->logical_port, > + br_ln, name2, br_int, name1, > + existing_ports); > + > + synced = synced && (br_int_result == PATCH_PORT_EXISTS) && > + (br_ln_result == PATCH_PORT_EXISTS); > free(name1); > free(name2); > } > sbrec_port_binding_index_destroy_row(target); > + return synced; > } > > /* Obtains external-ids:ovn-bridge-mappings from OVSDB and adds patch ports > for > * the local bridge mappings. Removes any patch ports for bridge mappings > that > - * already existed from 'existing_ports'. */ > -static void > + * already existed from 'existing_ports'. > + * > + * Returns true if all of the required patch ports are already present in the > + * database. */ > +static bool > add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn, > struct ovsdb_idl_index *sbrec_port_binding_by_type, > const struct ovsrec_bridge_table *bridge_table, > @@ -253,10 +290,12 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn, > > add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings); > > - add_bridge_mappings_by_type(ovs_idl_txn, sbrec_port_binding_by_type, > - br_int, existing_ports, chassis, > - &bridge_mappings, "l2gateway", > - "ovn-l2gateway-port", local_datapaths, true); > + bool l2gateway_synced = > + add_bridge_mappings_by_type(ovs_idl_txn, sbrec_port_binding_by_type, > + br_int, existing_ports, chassis, > + &bridge_mappings, "l2gateway", > + "ovn-l2gateway-port", local_datapaths, > + true); > > /* Since having localnet ports that are not mapped on some chassis is a > * supported configuration used to implement multisegment switches with > @@ -264,11 +303,15 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn, > * run but don't unnecessarily pollute the log file; pass > * 'log_missing_bridge = false'. > */ > - add_bridge_mappings_by_type(ovs_idl_txn, sbrec_port_binding_by_type, > - br_int, existing_ports, NULL, > - &bridge_mappings, "localnet", > - "ovn-localnet-port", local_datapaths, false); > + bool localnet_synced = > + add_bridge_mappings_by_type(ovs_idl_txn, sbrec_port_binding_by_type, > + br_int, existing_ports, NULL, > + &bridge_mappings, "localnet", > + "ovn-localnet-port", local_datapaths, > + false); > + > shash_destroy(&bridge_mappings); > + return l2gateway_synced && localnet_synced; > } > > static const struct ovsrec_port * > @@ -285,7 +328,31 @@ get_port(struct ovsdb_idl_index *ovsrec_port_by_name, > const char *name) > return port; > } > > -void > +/* Returns true if ovs-vswitchd has assigned an ofport number to every > + * interface in 'port'. */ > +static bool > +patch_port_is_installed(const struct ovsrec_port *port) > +{ > + for (size_t i = 0; i < port->n_interfaces; i++) { > + const struct ovsrec_interface *iface = port->interfaces[i]; > + > + /* An ofport of 0 means that it has not been assigned yet and > + * -1 means that ovs-vswitchd failed to add the interface. */ > + if (!iface->n_ofport || *iface->ofport < 1) { > + return false; > + } > + } > + return true; > +} > + > +/* Adds to the local OVS database the patch ports required by the localnet > + * and l2gateway ports that are local to this chassis and removes the ones > + * that are no longer needed. The database is only updated if 'ovs_idl_txn' > + * is non-NULL. > + * > + * Returns true if the database already reflects the required set of patch > + * ports and ovs-vswitchd has assigned an ofport number to each of them. */ > +bool > patch_run(struct ovsdb_idl_txn *ovs_idl_txn, > struct ovsdb_idl_index *sbrec_port_binding_by_type, > const struct ovsrec_bridge_table *bridge_table, > @@ -295,10 +362,6 @@ patch_run(struct ovsdb_idl_txn *ovs_idl_txn, > const struct sbrec_chassis *chassis, > const struct hmap *local_datapaths) > { > - if (!ovs_idl_txn) { > - return; > - } > - > /* Figure out what patch ports already exist. > * > * ovn-controller does not create or use ports of type > "ovn-l3gateway-port" > @@ -332,12 +395,29 @@ patch_run(struct ovsdb_idl_txn *ovs_idl_txn, > } > } > > + /* The physical flows that use a patch port can't be installed until > + * ovs-vswitchd assigns it an ofport number, so the configuration is not > + * applied yet even though the port is already in the database. */ > + bool ports_installed = true; > + struct shash_node *node; > + SHASH_FOR_EACH (node, &existing_ports) { > + if (!patch_port_is_installed(node->data)) { > + ports_installed = false; > + break; > + } > + } > + > /* Create in the database any patch ports that should exist. Remove from > * 'existing_ports' any patch ports that do exist in the database and > * should be there. */ > - add_bridge_mappings(ovs_idl_txn, sbrec_port_binding_by_type, > bridge_table, > - ovs_table, br_int, &existing_ports, chassis, > - local_datapaths); > + bool synced = add_bridge_mappings(ovs_idl_txn, > sbrec_port_binding_by_type, > + bridge_table, ovs_table, br_int, > + &existing_ports, chassis, > + local_datapaths); > + > + if (!ports_installed || !shash_is_empty(&existing_ports)) { > + synced = false; > + } > > /* Now 'existing_ports' only still contains patch ports that exist in the > * database but shouldn't. Delete them from the database. */ > @@ -350,9 +430,10 @@ patch_run(struct ovsdb_idl_txn *ovs_idl_txn, > * data is not completely downloaded yet after last restart of > * ovn-controller. Otherwise it may cause unncessary dataplane > * interruption during restart/upgrade. */ > - if (!daemon_started_recently()) { > + if (!daemon_started_recently() && ovs_idl_txn) { > remove_port(bridge_table, port); > } > } > shash_destroy(&existing_ports); > + return synced; > } > diff --git a/controller/patch.h b/controller/patch.h > index db4a888e6..26ebf401c 100644 > --- a/controller/patch.h > +++ b/controller/patch.h > @@ -36,7 +36,7 @@ struct shash; > void add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table > *ovs_table, > const struct ovsrec_bridge_table *bridge_table, > struct shash *bridge_mappings); > -void patch_run(struct ovsdb_idl_txn *ovs_idl_txn, > +bool patch_run(struct ovsdb_idl_txn *ovs_idl_txn, > struct ovsdb_idl_index *sbrec_port_binding_by_type, > const struct ovsrec_bridge_table *, > const struct ovsrec_open_vswitch_table *, > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index 7a19c0130..e0f70bcde 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -84,25 +84,6 @@ m4_divert_text([PREPARE_TESTS], > [dump_diff__ "$rcv_pcap" "$exp_text"]) > } > > - ovn_wait_patch_port_flows () { > - for localnet in $1; do > - patch_port="patch-br-int-to-$localnet" > - for hv in $2; do > - echo "$3: waiting for flows for $patch_port on $hv" > - # Patch port might be created after ports are reported up > - OVS_WAIT_UNTIL([ > - test 1 = $(as $hv ovs-vsctl show | grep "Port \b$patch_port\b" > | wc -l) > - ]) > - # Wait for a flow outputing to patch port > - OVS_WAIT_UNTIL([ > - hv_patch_ofport=$(as $hv ovs-vsctl --bare --columns ofport find > Interface name=$patch_port) > - echo "$patch_port=$hv_patch_ofport" > - test -n "$hv_patch_ofport" && test 1 -le $(as $hv ovs-ofctl > dump-flows br-int | grep -c "output:\b$hv_patch_ofport\b") > - ]) > - done > - done > - } > - > ovn_wait_remote_output_flows () { > hv1=$1 > hv2=$2 > @@ -180,9 +161,6 @@ m4_define([OVN_CHECK_PACKETS_CONTAIN], > m4_define([OVN_CHECK_PACKETS_UNIQ], > [ovn_wait_packets_uniq__ "$1" "$2" "__file__:__line__" $3]) > > -m4_define([OVN_WAIT_PATCH_PORT_FLOWS], > - [ovn_wait_patch_port_flows "$1" "$2" "__file__:__line__"]) > - > m4_define([OVN_WAIT_REMOTE_OUTPUT_FLOWS], > [ovn_wait_remote_output_flows "$1" "$2" "__file__:__line__"]) > > diff --git a/tests/ovn.at b/tests/ovn.at > index 1a86d9a3d..49674266f 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -3375,7 +3375,7 @@ for i in 1 2; do > done > done > wait_for_ports_up > -OVN_WAIT_PATCH_PORT_FLOWS(["ln1" "ln2" "ln3"], ["hv1"] ["hv2"]) > +check ovn-nbctl --wait=hv sync > > check ovn-nbctl --wait=sb sync > ovn-sbctl dump-flows > sbflows > @@ -3542,8 +3542,7 @@ done > > > wait_for_ports_up > -check ovn-nbctl --wait=sb sync > -OVN_WAIT_PATCH_PORT_FLOWS(["ln-11" "ln-21"], ["hv-1" "hv-2"]) > +check ovn-nbctl --wait=hv sync > > ovn-sbctl dump-flows > sbflows > AT_CAPTURE_FILE([sbflows]) > @@ -3831,7 +3830,7 @@ for i in 1 2; do > check ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:00:0$i > > OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup]) > - OVN_WAIT_PATCH_PORT_FLOWS(["ln-$i-20"], ["hv-$i"]) > + check ovn-nbctl --wait=hv sync > done > > wait_for_ports_up > @@ -4083,7 +4082,7 @@ for i in 1 2; do > > # Patch port might be created after ports are reported up > # Wait for a flow outputing to patch port > - OVN_WAIT_PATCH_PORT_FLOWS(["ln-100"], ["hv-$i"]) > + check ovn-nbctl --wait=hv sync > done > > test_packet() { > @@ -4151,7 +4150,7 @@ for i in 1 2; do > > # Patch port might be created after ports are reported up > # Wait for a flow outputing to patch port > - OVN_WAIT_PATCH_PORT_FLOWS(["ln-100"], ["hv-$i"]) > + check ovn-nbctl --wait=hv sync > done > # create taps on fabric to check vlan encapsulation there > for i in 1 2; do > @@ -4233,7 +4232,7 @@ for i in 1 2; do > > # Patch port might be created after ports are reported up > # Wait for a flow outputing to patch port > - OVN_WAIT_PATCH_PORT_FLOWS(["ln"], ["hv-$i"]) > + check ovn-nbctl --wait=hv sync > done > > for i in 1 2; do > @@ -4455,7 +4454,7 @@ for tag in 10 20; do > check ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:0$i:$tag > > OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup]) > - OVN_WAIT_PATCH_PORT_FLOWS(["ln-$tag"], ["hv-$tag-$i"]) > + check ovn-nbctl --wait=hv sync > done > done > wait_for_ports_up > @@ -10146,9 +10145,9 @@ AT_CAPTURE_FILE([sbflows]) > > # Wait until the patch ports are created in hv1 and hv2 to connect br-int to > br-eth0 > AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1]) > -OVN_WAIT_PATCH_PORT_FLOWS(["ln_port"], ["hv1"]) > +check ovn-nbctl --wait=hv sync > AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv2]) > -OVN_WAIT_PATCH_PORT_FLOWS(["ln_port"], ["hv2"]) > +check ovn-nbctl --wait=hv sync > > # Temporarily remove lr0 chassis > # Wait for hv confirmation to make sure chassis is removed before we reset > pcap > @@ -12871,7 +12870,6 @@ for i in 1 2; do > > bfd$i.expected > done > > -OVN_WAIT_PATCH_PORT_FLOWS([ln-ext], [gw1 gw2]) > check ovn-nbctl --wait=hv sync > > check_bfd_packets() { > @@ -13310,8 +13308,7 @@ check as ext1 ovs-vsctl set open . > external-ids:ovn-bridge-mappings=phys:br-phys > > wait_for_ports_up > check ovn-nbctl --wait=sb sync > -OVN_WAIT_PATCH_PORT_FLOWS(["ln-alice"], ["gw1"] ["gw2"],["gw3"]) > -OVN_WAIT_PATCH_PORT_FLOWS(["ln-outside"], ["ext1"]) > +check ovn-nbctl --wait=hv sync > > ovn-sbctl dump-flows > sbflows > AT_CAPTURE_FILE([sbflows]) > @@ -13555,7 +13552,6 @@ check as hv3 ovs-vsctl set open . > external-ids:ovn-bridge-mappings=phys:br-phys > dnl Allow some time for ovn-northd and ovn-controller to catch up. > wait_for_ports_up > check ovn-nbctl --wait=hv sync > -OVN_WAIT_PATCH_PORT_FLOWS(["ln-outside"], ["hv3"]) > > (echo "---------NB dump-----" > ovn-nbctl show > @@ -13670,7 +13666,7 @@ OVN_CHECK_PACKETS_UNIQ([hv3/vif1-tx.pcap], > [hv3-vif1.expected]) > as hv2 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys > > # Wait until the patch ports are created in hv2 to connect br-int to br-phys > -OVN_WAIT_PATCH_PORT_FLOWS(["ln-alice"], ["hv2"]) > +check ovn-nbctl --wait=hv sync > > dnl Allow some time for ovn-controller to catch up. > wait_for_ports_up > @@ -13742,7 +13738,6 @@ check ovn-nbctl lsp-add foo foo2 \ > # Allow some time for ovn-northd and ovn-controller to catch up. > wait_for_ports_up > check ovn-nbctl --wait=hv sync > -OVN_WAIT_PATCH_PORT_FLOWS(["ln-alice"], ["hv1"]) > > : > hv1-vif2.expected > > @@ -13842,7 +13837,7 @@ OVN_CHECK_PACKETS([hv1/snoopvif-tx.pcap], [packets]) > AT_CHECK([as hv2 ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=physnet1:br-phys]) > > # Wait until the patch ports are created in hv2 to connect br-int to br-phys > -OVN_WAIT_PATCH_PORT_FLOWS(["ln_port"], ["hv2"]) > +check ovn-nbctl --wait=hv sync > > # Wait for packets to be received. > OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100]) > @@ -13870,7 +13865,7 @@ ovs-vsctl -- add-port br-int hv3-vif2 -- \ > AT_CHECK([as hv3 ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=physnet1:br-phys]) > > # Wait until the patch ports are created in hv3 to connect br-int to br-phys > -OVN_WAIT_PATCH_PORT_FLOWS(["ln_port"], ["hv3"]) > +check ovn-nbctl --wait=hv sync > > # Re-add nat-addresses option > check ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 > nat-addresses="router" > @@ -15550,7 +15545,6 @@ AT_CHECK([ovn-nbctl lsp-add-localnet-port ls0 ln_port > physnet1]) > # wait for earlier changes to take effect > wait_for_ports_up > check ovn-nbctl --wait=hv sync > -OVN_WAIT_PATCH_PORT_FLOWS(["ln_port"], ["hv2"] ["hv3"]) > > # reset hv1/snoopvif after hv2/br-phys_n1 and hv3/br-phys_n1 as we wait > # for packets to be received on hv1/snoopvif-tx. > @@ -16880,8 +16874,7 @@ reset_env > # Start port migration hv1 -> hv2: both hypervisors are now bound > check ovn-nbctl --wait=hv lsp-set-options migrator requested-chassis=hv1,hv2 > wait_for_ports_up > -OVN_WAIT_PATCH_PORT_FLOWS(["public"], ["hv1"]) > -OVN_WAIT_PATCH_PORT_FLOWS(["public"], ["hv2"]) > +check ovn-nbctl --wait=hv sync > OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv1"],["hv2"]) > OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv2"],["hv1"]) > OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv3"],["hv1"]) > @@ -23920,7 +23913,7 @@ check ovn-nbctl --wait=sb sync > #ovn-sbctl dump-flows > for i in 1 2; do > for j in 1 2; do > - OVN_WAIT_PATCH_PORT_FLOWS(["ln$j"], ["hv$i"]) > + check ovn-nbctl --wait=hv sync > done > done > > @@ -26364,8 +26357,7 @@ m4_define([DVR_N_S_ARP_HANDLING], > check ovn-nbctl lrp-set-gateway-chassis router-to-underlay hv3 > wait_for_ports_up > check ovn-nbctl --wait=sb sync > - OVN_WAIT_PATCH_PORT_FLOWS(["ln1"] ["ln2"], ["hv1"] ["hv2"] ["hv3"]) > - OVN_WAIT_PATCH_PORT_FLOWS(["ln4"], ["hv4"]) > + check ovn-nbctl --wait=hv sync > > wait_row_count Port_Binding 1 logical_port=cr-router-to-underlay > > @@ -26589,8 +26581,7 @@ m4_define([DVR_N_S_PING], > > wait_for_ports_up > check ovn-nbctl --wait=sb sync > - OVN_WAIT_PATCH_PORT_FLOWS(["ln1"] ["ln2"], ["hv1"] ["hv2"] ["hv3"]) > - OVN_WAIT_PATCH_PORT_FLOWS(["ln4"], ["hv4"]) > + check ovn-nbctl --wait=hv sync > > OVN_POPULATE_ARP > > @@ -27328,8 +27319,7 @@ check ovn-nbctl lrp-set-redirect-type > router-to-underlay bridged > wait_for_ports_up > check ovn-nbctl --wait=sb sync > > -OVN_WAIT_PATCH_PORT_FLOWS(["ln1"] ["ln2"] ["ln3"], ["hv1"] ["hv2"] ["hv3"]) > -OVN_WAIT_PATCH_PORT_FLOWS(["ln4"], ["hv4"]) > +check ovn-nbctl --wait=hv sync > > OVN_POPULATE_ARP > > @@ -40418,7 +40408,7 @@ for i in 1 2; do > > # Patch port might be created after ports are reported up > # Wait for a flow outputing to patch port > - OVN_WAIT_PATCH_PORT_FLOWS(["public0"], ["hv-$i"]) > + check ovn-nbctl --wait=hv sync > done > > check ovn-nbctl set Logical_Switch_Port lsp1 options:qos_min_rate=200000 > @@ -40936,14 +40926,19 @@ check ovn-nbctl --wait=hv sync > > sleep_ovsdb hv1 > > -# Do following operations through one nbctl command > -# Otherwise, they would probably result in multiple sb updates to > ovn-controller > -# As ovsdb is sleeping those multiple updates would resulti in requests to > ovsdb being > -# postponed as ovsdb becomes ro. > -# Hence, patch port creation would be delayed after ovsdb becomes rw. > -check ovn-nbctl --wait=hv lsp-add-localnet-port ls1 ln1 phys > +# Create a localnet port and wait for ovn-controller to process the change. > +# --wait=hv cannot be used here because hv1's ovsdb is asleep. > +# Make sure ovn-controller has not updated nb_cfg. > +runtime_data_runs=$(as hv1 ovn-appctl -t ovn-controller > inc-engine/show-stats runtime_data compute) > +nb_cfg="$(fetch_column nb:NB_Global nb_cfg)" > + > +check ovn-nbctl --wait=sb lsp-add-localnet-port ls1 ln1 phys > + > +OVS_WAIT_UNTIL([test $(as hv1 ovn-appctl -t ovn-controller > inc-engine/show-stats runtime_data compute) -gt $runtime_data_runs]) > +check_row_count Chassis_Private 1 name=hv1 nb_cfg="${nb_cfg}" > > -# Controller should now have created the patch port, but is not yet notified > of the ofport of those interfaces > +# Controller should now have sent transaction to create the patch port, but > it > +# has not yet been received by ovsdb-server. > sleep_controller hv1 > wake_up_ovsdb hv1 > check as hv1 ovs-vsctl remove Interface vif1 external_ids iface-id > @@ -41746,7 +41741,7 @@ as hv1 check ovs-vsctl set Open_vSwitch . > external-ids:garp-max-timeout-sec=2 > > # Wait until the patch ports are created in hv1 to connect br-int to br-eth0 > check ovn-nbctl set logical_router lr0 options:chassis=hv1 > -OVN_WAIT_PATCH_PORT_FLOWS(["ln_port"], ["hv1"]) > +check ovn-nbctl --wait=hv sync > > # sleep for 12s to get a garp every ~ 2s > sleep 12 > @@ -44707,7 +44702,7 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int > table=OFTABLE_PHY_TO_LOG | grep -v > # Add localnet port to sw0 > check ovn-nbctl --wait=hv lsp-add-localnet-port sw0 ln-sw0 physnet1 -- set > logical_switch_port ln-sw0 tag_request=100 > > -OVN_WAIT_PATCH_PORT_FLOWS(["ln-sw0"], ["hv1"]) > +check ovn-nbctl --wait=hv sync > AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | grep > -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v > "in_port=$of2" | wc -l], [0], [dnl > 3 > ]) > @@ -47968,3 +47963,79 @@ AT_CHECK([grep "skipping output to input port" \ > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([--wait=hv waits for patch port flows]) > + > +ovn_start > + > +check ovn-nbctl ls-add ls1 > +net_add n > + > +sim_add hv1 > +as hv1 > +check ovs-vsctl add-br br-phys > +check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys > +ovn_attach n br-phys 192.168.0.1 > + > +check ovn-nbctl lsp-add ls1 lsp1 > +check as hv1 ovs-vsctl --no-wait -- add-port br-int vif1 \ > + -- set Interface vif1 external_ids:iface-id=lsp1 \ > + -- set Interface vif1 type=internal > +wait_for_ports_up > +check ovn-nbctl --wait=hv sync > +n_flows=$(ovs-ofctl dump-flows br-int | wc -l) > +nb_cfg=$(fetch_column nb:NB_Global nb_cfg) > + > +sleep_ovsdb hv1 > + > +# Add a localnet port. ovn-controller will try to create patch ports, but > +# ovsdb-server is asleep so the transaction cannot commit. > +check ovn-nbctl --wait=sb lsp-add-localnet-port ls1 ln1 phys > + > +# Wait for ovn-controller to process the localnet port change before > +# pausing ovs-vswitchd. > +OVS_WAIT_UNTIL([test "$(ovs-ofctl dump-flows br-int | wc -l)" -gt $n_flows]) > + > +# Pause ovs-vswitchd so it cannot assign ofport when ovsdb wakes up. > +sleep_ovs hv1 > +wake_up_ovsdb hv1 > + > +# ovn-controller is now notified about patch port creation, but > +# ovs-vswitchd is asleep so ofport is not assigned. --wait=hv must > +# not succeed yet. > +if ! ovn-nbctl --timeout=1 --wait=hv sync; then > + wake_up_ovs hv1 > +fi > + > +check ovn-nbctl --wait=hv sync > + > +# Verify that nb_cfg has advanced past the value from before the localnet > port > +# was added. > +check_row_count Chassis_Private 1 name=hv1 nb_cfg\>${nb_cfg} > + > +# Pause ovn-controller so we can inspect the flow table. > +sleep_controller hv1 > +wake_up_ovs hv1 > + > +# Verify that the patch port has an ofport and a corresponding flow. > +check ovs-vsctl wait-until Interface patch-br-int-to-ln1 "ofport>0" > +ofport=$(ovs-vsctl --bare --columns ofport \ > + find Interface name=patch-br-int-to-ln1) > +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG \ > + | grep -q in_port=$ofport], [0], []) > + > +wake_up_controller hv1 > + > +# Delete the localnet port and verify --wait=hv waits for cleanup. > +check ovn-nbctl --wait=hv lsp-del ln1 > + > +# Verify that the patch port and its flow are gone. > +AT_CHECK([ovs-vsctl show | grep -c patch-br-int-to-ln1], [1], [0 > +]) > +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG \ > + | grep in_port=$ofport], [1], []) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index f79aa7b3f..3f43132cc 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -14211,7 +14211,7 @@ check ovn-nbctl lsp-add public public-rp -- set > Logical_Switch_Port public-rp \ > -- lsp-set-addresses public-rp router > > # Wait for a flow outputing to patch port, so we know ovn-controller handled > the patch port creation in OVS. > -OVN_WAIT_PATCH_PORT_FLOWS(["ln1"], ["./"]) > +check ovn-nbctl --wait=hv sync > > check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24 > > -- > 2.56.0 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
