It's slow to add --wait to every ovn-nbctl command; only the last command
needs it. But it's sometimes inconvenient to add it to the last command
if it's in a loop, etc. This makes it possible to separately wait for
the OVN southbound or hypervisors to catch up to the northbound.
Also, modify the tests to replace "sleep" invocations that were waiting
for hypervisors to catch up by --wait=hv or by invocations of "ovn-nbctl
--wait=hv sync".
Signed-off-by: Ben Pfaff <b...@ovn.org>
---
lib/ovsdb-idl.c | 18 +++++--
lib/ovsdb-idl.h | 3 +-
ovn/utilities/ovn-nbctl.8.xml | 25 ++++++++++
ovn/utilities/ovn-nbctl.c | 27 ++++++++++-
tests/ovn.at | 108 ++++++++++++++++++------------------------
tests/test-ovsdb.c | 5 +-
utilities/ovs-vsctl.c | 2 +-
7 files changed, 116 insertions(+), 72 deletions(-)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index d70fb10..816cc70 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -125,6 +125,7 @@ struct ovsdb_idl_txn {
const char *inc_table;
const char *inc_column;
struct uuid inc_row;
+ bool inc_force;
unsigned int inc_index;
int64_t inc_new_value;
@@ -2234,6 +2235,12 @@ ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
* successfully, the client may retrieve the final (incremented) value of
* 'column' with ovsdb_idl_txn_get_increment_new_value().
*
+ * If at time of commit the transaction is otherwise empty, that is, it doesn't
+ * change the database, then 'force' is important. If 'force' is false in this
+ * case, the IDL suppresses the increment and skips a round trip to the
+ * database server. If 'force' is true, the IDL will still increment the
+ * column.
+ *
* The client could accomplish something similar with ovsdb_idl_read(),
* ovsdb_idl_txn_verify() and ovsdb_idl_txn_write(), or with ovsdb-idlc
* generated wrappers for these functions. However, ovsdb_idl_txn_increment()
@@ -2244,7 +2251,8 @@ ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
void
ovsdb_idl_txn_increment(struct ovsdb_idl_txn *txn,
const struct ovsdb_idl_row *row,
- const struct ovsdb_idl_column *column)
+ const struct ovsdb_idl_column *column,
+ bool force)
{
ovs_assert(!txn->inc_table);
ovs_assert(column->type.key.type == OVSDB_TYPE_INTEGER);
@@ -2253,6 +2261,7 @@ ovsdb_idl_txn_increment(struct ovsdb_idl_txn *txn,
txn->inc_table = row->table->class->name;
txn->inc_column = column->name;
txn->inc_row = row->uuid;
+ txn->inc_force = force;
}
/* Destroys 'txn' and frees all associated memory. If ovsdb_idl_txn_commit()
@@ -2740,12 +2749,11 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
}
/* Add increment. */
- if (txn->inc_table && any_updates) {
- struct json *op;
-
+ if (txn->inc_table && (any_updates || txn->inc_force)) {
+ any_updates = true;
txn->inc_index = operations->u.array.n - 1;
- op = json_object_create();
+ struct json *op = json_object_create();
json_object_put_string(op, "op", "mutate");
json_object_put_string(op, "table", txn->inc_table);
json_object_put(op, "where",
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index e25bfef..45befb0 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -251,7 +251,8 @@ void ovsdb_idl_txn_add_comment(struct ovsdb_idl_txn *,
const char *, ...)
void ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *);
void ovsdb_idl_txn_increment(struct ovsdb_idl_txn *,
const struct ovsdb_idl_row *,
- const struct ovsdb_idl_column *);
+ const struct ovsdb_idl_column *,
+ bool force);
void ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *);
void ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *);
enum ovsdb_idl_txn_status ovsdb_idl_txn_commit(struct ovsdb_idl_txn *);
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index 122a114..11a5a46 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -475,6 +475,22 @@
<xi:include href="lib/db-ctl-base.xml"
xmlns:xi="http://www.w3.org/2003/XInclude"/>
+ <h1>Synchronization Commands</h1>
+
+ <dl>
+ <dt>sync</dt>
+ <dd>
+ Ordinarily, <code>--wait=sb</code> or <code>--wait=hv</code> only waits
+ for changes by the current <code>ovn-nbctl</code> invocation to take
+ effect. This means that, if none of the commands supplied to
+ <code>ovn-nbctl</code> change the database, then the command does not
+ wait at all. With the <code>sync</code> command, however,
+ <code>ovn-nbctl</code> waits even for earlier changes to the database
+ to propagate down to the southbound database or all of the OVN chassis,
+ according to the argument to <code>--wait</code>.
+ </dd>
+ </dl>
+
<h1>Options</h1>
<dl>
@@ -508,6 +524,15 @@
become up-to-date with the northbound database updates. (This can
become an indefinite wait if any chassis is malfunctioning.)
</p>
+
+ <p>
+ Ordinarily, <code>--wait=sb</code> or <code>--wait=hv</code> only
+ waits for changes by the current <code>ovn-nbctl</code> invocation to
+ take effect. This means that, if none of the commands supplied to
+ <code>ovn-nbctl</code> change the database, then the command does not
+ wait at all. Use the <code>sync</code> command to override this
+ behavior.
+ </p>
</dd>
<dt><code>--db</code> <var>database</var></dt>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index e594a32..d3eae3b 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -57,6 +57,10 @@ enum nbctl_wait_type {
};
static enum nbctl_wait_type wait_type = NBCTL_WAIT_NONE;
+/* Should we wait (if specified by 'wait_type') even if the commands don't
+ * change the database at all? */
+static bool force_wait = false;
+
/* --timeout: Time to wait for a connection to 'db'. */
static int timeout;
@@ -408,6 +412,9 @@ DHCP Options commands:\n\
\n\
%s\
\n\
+Synchronization command (use with --wait=sb|hv):\n\
+ sync wait even for earlier changes to take effect\n\
+\n\
Options:\n\
--db=DATABASE connect to DATABASE\n\
(default: %s)\n\
@@ -559,6 +566,21 @@ nbctl_init(struct ctl_context *ctx OVS_UNUSED)
}
static void
+nbctl_pre_sync(struct ctl_context *ctx OVS_UNUSED)
+{
+ if (wait_type != NBCTL_WAIT_NONE) {
+ force_wait = true;
+ } else {
+ VLOG_INFO("\"sync\" command has no effect without --wait");
+ }
+}
+
+static void
+nbctl_sync(struct ctl_context *ctx OVS_UNUSED)
+{
+}
+
+static void
nbctl_show(struct ctl_context *ctx)
{
const struct nbrec_logical_switch *ls;
@@ -2209,8 +2231,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
}
if (wait_type != NBCTL_WAIT_NONE) {
- ovsdb_idl_txn_increment(txn, &nb->header_,
- &nbrec_nb_global_col_nb_cfg);
+ ovsdb_idl_txn_increment(txn, &nb->header_, &nbrec_nb_global_col_nb_cfg,
+ force_wait);
}
symtab = ovsdb_symbol_table_create();
@@ -2394,6 +2416,7 @@ nbctl_exit(int status)
static const struct ctl_command_syntax nbctl_commands[] = {
{ "init", 0, 0, "", NULL, nbctl_init, NULL, "", RW },
+ { "sync", 0, 0, "", nbctl_pre_sync, nbctl_sync, NULL, "", RO },
{ "show", 0, 1, "[SWITCH]", NULL, nbctl_show, NULL, "", RO },
/* logical switch commands. */
diff --git a/tests/ovn.at b/tests/ovn.at
index 34fdd5d..8e4847a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -731,9 +731,8 @@ ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type == 0x1237 &&
eth.src == $set1 &&
# for ARP resolution).
ovn_populate_arp
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
# Given the name of a logical port, prints the name of the hypervisor
# on which it is located.
@@ -913,9 +912,14 @@ done
# lp13 should be configured with only 192.168.0.13.
ovn-nbctl lsp-set-addresses lp13 "f0:00:00:00:00:13 192.168.0.13 invalid
192.169.0.13"
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
+# Wait for ovs-vswitchd to catch up.
+# XXX We should make this possible via OpenFlow, and after that we should
+# make ovn-controller do it for us.
+for i in 1 2 3; do
+ as hv$i ovs-appctl revalidator/purge
+done
sip=`ip_to_hex 192 168 0 11`
tip=`ip_to_hex 192 168 0 13`
@@ -1380,9 +1384,8 @@ ovs-vsctl add-port br-phys vif3 -- set Interface vif3
options:tx_pcap=hv3/vif3-t
# for ARP resolution).
ovn_populate_arp
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
# test_packet INPORT DST SRC ETHTYPE OUTPORT...
#
@@ -1586,9 +1589,8 @@ done
# for ARP resolution).
ovn_populate_arp
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
# test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT...
#
@@ -1942,9 +1944,8 @@ done
# for ARP resolution).
ovn_populate_arp
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
# Given the name of a logical port, prints the name of the hypervisor
# on which it is located.
@@ -2361,9 +2362,8 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \
# for ARP resolution).
ovn_populate_arp
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
# Send ip packets between the two ports.
ip_to_hex() {
@@ -2459,10 +2459,8 @@ ovs-vsctl -- add-port br-int vif2 -- \
options:rxq_pcap=hv1/vif2-rx.pcap \
ofport-request=1
-
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
# Send ip packets between the two ports.
ip_to_hex() {
@@ -2499,9 +2497,12 @@ as hv1 ovs-ofctl dump-flows br-int
#Disable router R1
ovn-nbctl set Logical_Router R1 enabled=false
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
+# Wait for ovs-vswitchd to catch up.
+# XXX We should make this possible via OpenFlow, and after that we should
+# make ovn-controller do it for us.
+as hv1 ovs-appctl revalidator/purge
echo "---------SB dump-----"
ovn-sbctl list datapath_binding
@@ -2523,7 +2524,6 @@ $PYTHON "$top_srcdir/utilities/ovs-pcap.in"
hv1/vif2-tx.pcap | trim_zeros > rece
echo $expected | trim_zeros > expout
AT_CHECK([cat received.packets], [0], [expout])
-
as hv1
OVS_APP_EXIT_AND_WAIT([ovn-controller])
OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
@@ -2596,10 +2596,8 @@ ovs-vsctl -- add-port br-int vif2 -- \
options:rxq_pcap=hv1/vif2-rx.pcap \
ofport-request=1
-
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
# Send ip packets between the two ports.
ip_to_hex() {
@@ -2635,6 +2633,13 @@ as hv1 ovs-ofctl dump-flows br-int
#Disable router R1
ovn-nbctl set Logical_Router R1 enabled=false
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
+# Wait for ovs-vswitchd to catch up.
+# XXX We should make this possible via OpenFlow, and after that we should
+# make ovn-controller do it for us.
+as hv1 ovs-appctl revalidator/purge
+
echo "---------SB dump-----"
ovn-sbctl list datapath_binding
echo "---------------------"
@@ -2644,10 +2649,6 @@ echo "---------------------"
echo "------ hv1 dump ----------"
as hv1 ovs-ofctl dump-flows br-int
-# Allow some time for the disabling of logical router R1 to propagate.
-# XXX This should be more systematic.
-sleep 1
-
as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
# Packet to Expect
@@ -2753,9 +2754,8 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \
# for ARP resolution).
ovn_populate_arp
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
ip_to_hex() {
printf "%02x%02x%02x%02x" "$@"
@@ -2975,9 +2975,8 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \
# for ARP resolution).
ovn_populate_arp
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
ip_to_hex() {
printf "%02x%02x%02x%02x" "$@"
@@ -3411,14 +3410,10 @@ ovn-nbctl lsp-add foo foo1 \
-- lsp-set-addresses foo1 "f0:00:00:01:02:03 192.168.1.2"
# Create logical port alice1 in alice
-ovn-nbctl lsp-add alice alice1 \
+# and wait for ovn-northd and ovn-controller to catch up.
+ovn-nbctl --timeout=5 --wait=hv lsp-add alice alice1 \
-- lsp-set-addresses alice1 "f0:00:00:01:02:04 172.16.1.2"
-
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 2
-
ip_to_hex() {
printf "%02x%02x%02x%02x" "$@"
}
@@ -3480,13 +3475,10 @@ ovn-nbctl lrp-add R2 alice 00:00:02:01:02:03
172.16.1.1/24
# Connect R2 to join
ovn-nbctl lrp-add R2 R2_join 00:00:04:01:02:04 20.0.0.2/24
-ovn-nbctl -- --id=@lrt create Logical_Router_Static_Route \
+ovn-nbctl --timeout=5 --wait=hv -- --id=@lrt create
Logical_Router_Static_Route \
ip_prefix=192.168.1.0/24 nexthop=20.0.0.1 -- add Logical_Router \
R2 static_routes @lrt
-# Wait for ovn-controller to catch up.
-sleep 1
-
# Send the packet again.
as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
@@ -3557,11 +3549,8 @@ ovs-vsctl -- add-port br-int vif2 -- \
options:rxq_pcap=hv1/vif2-rx.pcap \
ofport-request=1
-
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
-
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
ip_to_hex() {
printf "%02x%02x%02x%02x" "$@"
@@ -3720,9 +3709,8 @@ ovn-nbctl acl-add lsw0 from-lport 1002 'ip6 && icmp6'
allow-related
ovn-nbctl acl-add lsw0 to-lport 1002 'outport == "lp1" && ip6 && icmp6'
allow-related
ovn-nbctl acl-add lsw0 to-lport 1002 'outport == "lp2" && ip6 && icmp6'
allow-related
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+# Wait for ovn-northd and ovn-controller to catch up.
+AT_CHECK([ovn-nbctl --timeout=5 --wait=hv sync])
# Given the name of a logical port, prints the name of the hypervisor
# on which it is located.
@@ -3774,9 +3762,7 @@ ovn_attach n1 br-phys 192.168.0.1
row=`ovn-nbctl create Address_Set name=set1 addresses=\"1.1.1.1\"`
ovn-nbctl set Address_Set $row name=set1 addresses=\"1.1.1.1,1.1.1.2\"
-ovn-nbctl destroy Address_Set $row
-
-sleep 1
+ovn-nbctl --timeout=5 --wait=hv destroy Address_Set $row
# A bug previously existed in the address set support code
# that caused ovn-controller to crash after an address set
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index c21001e..eeba3dd 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -2120,7 +2120,8 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step)
"i=%d", atoi(arg1));
}
- ovsdb_idl_txn_increment(txn, &s->header_, &idltest_simple_col_i);
+ ovsdb_idl_txn_increment(txn, &s->header_, &idltest_simple_col_i,
+ false);
increment = true;
} else if (!strcmp(name, "abort")) {
ovsdb_idl_txn_abort(txn);
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 722dcd9..300c7fb 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -2544,7 +2544,7 @@ do_vsctl(const char *args, struct ctl_command *commands,
size_t n_commands,
if (wait_for_reload) {
ovsdb_idl_txn_increment(txn, &ovs->header_,
- &ovsrec_open_vswitch_col_next_cfg);
+ &ovsrec_open_vswitch_col_next_cfg, false);
}
post_db_reload_check_init();
--
2.1.3
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
