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.
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 | 127 ++++++++++++++++++++++++-------
controller/patch.h | 2 +-
tests/ovn-macros.at | 22 ------
tests/ovn.at | 147 ++++++++++++++++++++++++++----------
tests/system-ovn.at | 2 +-
6 files changed, 233 insertions(+), 106 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d57ff316d..2ebe26631 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -8484,16 +8484,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,
@@ -8611,18 +8614,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..78acfc8b9 100644
--- a/controller/patch.c
+++ b/controller/patch.c
@@ -71,10 +71,13 @@ match_patch_port(const struct ovsrec_port *port, const char
*peer)
/* Creates a patch port in bridge 'src' named 'src_name', whose peer is
* 'dst_name' in bridge 'dst'. Initializes the patch port's external-ids:'key'
- * to 'key'.
+ * to 'key'. The port is only created if 'ovs_idl_txn' is non-NULL.
*
- * 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 false.
+ *
+ * Otherwise, creates port (if 'ovs_idl_txn'), and returns true. */
+static bool
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 +88,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 false;
}
}
+ if (!ovs_idl_txn) {
+ return true;
+ }
+
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 +104,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 true;
}
static void
@@ -165,7 +173,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 +191,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 +240,32 @@ 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);
+ bool br_int_port_exists =
+ !create_patch_port(ovs_idl_txn, patch_port_id,
+ binding->logical_port,
+ br_int, name1, br_ln, name2,
+ existing_ports);
+ bool br_ln_port_exists =
+ !create_patch_port(ovs_idl_txn, patch_port_id,
+ binding->logical_port,
+ br_ln, name2, br_int, name1,
+ existing_ports);
+
+ synced = synced && br_int_port_exists && br_ln_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 +280,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 +293,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 +318,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 +352,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 +385,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 +420,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 13e62bf9b..3d8a4fd09 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
