"dev" <dev-boun...@openvswitch.org> wrote on 05/20/2016 01:17:57 PM:
> From: Amitabha Biswas/San Jose/IBM@IBMUS > To: dev@openvswitch.org > Date: 05/20/2016 01:18 PM > Subject: [ovs-dev] [v1 1/1] Fix "raceful" E2E ovn tests > Sent by: "dev" <dev-boun...@openvswitch.org> > > This patch fixes the "raceful" situation that occurs when ovn-nbctl > configures the OVN Northbound database till the point when the > ovn-controller picks up the corresponding logical flows and applies > them as OpenFlow rules on the hypervisor. > > The algorithm is outlined in the email - > http://openvswitch.org/pipermail/dev/2016-April/070042.html which > began as a thread here - > http://openvswitch.org/pipermail/dev/2016-April/069602.html > > A new table, Config, is added to OVN_Northbound and OVN_Southbound > DBs. This table is restricted to a single row in both the DBs. > The OVN_Northbound Config contains a single row, with next_cfg and > cur_cfg. The next_cfg is updated by ovn-nbctl whenever it commits > an update to the OVN_Northbound DB. The ovn-northd picks up the > next_cfg from the OVN_Northbound DB and sets the same value in the > OVN_Southbound's Config table next_cfg column. > > A new column (cur_cfg) is added to the Chassis table in the > OVN_Southbound DB. Every ovn-controller updates the cur_cfg in its > Chassis row (to the value in the OVN_Southbound DB cur_cfg column). > This is done in the next iteration after the ovn-controller picks up > the latest logical flows and pends them to be applied as OpenFlow > rules. The ovn-northd computes the mimumum of all the cur_cfg values > in the Chassis table and sets the OVN_Northbound's DB cur_cfg value. > > The ovn-nbctl utility increments the OVN_Northbound's next_cfg value > whenever it commits a transaction that modifies the OVN_Northbound > DB. A "--wait" option is added to the ovn-nbctl utility. When the > "--wait" option is used, the ovn-nbctl waits for the > OVN_Northbound's cur_cfg to equal next_cfg before it exits. A new > operation "nop" is added to use when the ovn-nbctl does not have any > read or write operation but wants to wait for all previous > configurations to have been picked up by the ovn-controllers by > using - "ovn-nbctl --wait nop" command. > > The ovn E2E test cases have been modified to use the > "ovn-nbctl --wait nop" command instead of using "sleep 1" to assume > synchronization between the OVN_Northbound settings and the > ovn-controllers. > > Caveats: > > Not all ovn-nbctl operations that modify the OVN_Northbound DB and > therefore increment the next_cfg value actually result in OpenFlow > rules being modified in the hypervisors. So a --wait option must not > be used under such circumstances. > > If a logical datapath doesn't exist on a particular hypervisor, the > ovn-controller will not apply any OpenFlow rules associated with that > datapath and not update the cur_cfg. Use caution with the "--wait" > option. > > Signed-off-by: Amitabha Biswas <abis...@us.ibm.com> > > Reported-by: Ryan Moats <rmo...@us.ibm.com> > > Reported-at: http://openvswitch.org/pipermail/dev/2016-April/069602.html > --- > ovn/controller/ofctrl.c | 15 ++++++++--- > ovn/controller/ofctrl.h | 2 +- > ovn/controller/ovn-controller.c | 43 ++++++++++++++++++++++++++++++- > ovn/northd/ovn-northd.c | 56 ++++++++++++++++++++++++++++++ > +++++++++++ > ovn/ovn-nb.ovsschema | 10 ++++++-- > ovn/ovn-nb.xml | 21 +++++++++++++++- > ovn/ovn-sb.ovsschema | 12 +++++++-- > ovn/ovn-sb.xml | 19 ++++++++++++++ > ovn/utilities/ovn-nbctl.8.xml | 29 +++++++++++++++++++++ > ovn/utilities/ovn-nbctl.c | 56 ++++++++++++++++++++++++++++++ > ++++++----- > tests/ovn.at | 37 ++++++++++++++++----------- > 11 files changed, 269 insertions(+), 31 deletions(-) > > diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c > index f537bc0..9d211bb 100644 > --- a/ovn/controller/ofctrl.c > +++ b/ovn/controller/ofctrl.c > @@ -597,10 +597,14 @@ queue_flow_mod(struct ofputil_flow_mod *fm) > * flows from 'flow_table' and frees them. (The hmap itself isn't > * destroyed.) > * > - * This called be called be ofctrl_run() within the main loop. */ > -void > + * This called be called be ofctrl_run() within the main loop. > + * Returns true if the installed flows were updated (queued) false otherwise. > + */ > +bool > ofctrl_put(struct hmap *flow_table) > { > + bool flows_updated = false; > + > /* The flow table can be updated if the connection to the > switch is up and > * in the correct state and not backlogged with existing flow_mods. (Our > * criteria for being backlogged appear very conservative, but the socket > @@ -610,7 +614,7 @@ ofctrl_put(struct hmap *flow_table) > if (state != S_UPDATE_FLOWS > || rconn_packet_counter_n_packets(tx_counter)) { > ovn_flow_table_clear(flow_table); > - return; > + return flows_updated; > } > > /* Iterate through all of the installed flows. If any of them are no > @@ -629,6 +633,7 @@ ofctrl_put(struct hmap *flow_table) > .command = OFPFC_DELETE_STRICT, > }; > queue_flow_mod(&fm); > + flows_updated = true; > ovn_flow_log(i, "removing"); > > hmap_remove(&installed_flows, &i->hmap_node); > @@ -647,6 +652,7 @@ ofctrl_put(struct hmap *flow_table) > }; > queue_flow_mod(&fm); > ovn_flow_log(i, "updating"); > + flows_updated = true; > > /* Replace 'i''s actions by 'd''s. */ > free(i->ofpacts); > @@ -676,10 +682,13 @@ ofctrl_put(struct hmap *flow_table) > .command = OFPFC_ADD, > }; > queue_flow_mod(&fm); > + flows_updated = true; > ovn_flow_log(d, "adding"); > > /* Move 'd' from 'flow_table' to installed_flows. */ > hmap_remove(flow_table, &d->hmap_node); > hmap_insert(&installed_flows, &d->hmap_node, d->hmap_node.hash); > } > + > + return flows_updated; > } > diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h > index bc9cfba..dd26f6e 100644 > --- a/ovn/controller/ofctrl.h > +++ b/ovn/controller/ofctrl.h > @@ -30,7 +30,7 @@ struct ovsrec_bridge; > /* Interface for OVN main loop. */ > void ofctrl_init(void); > enum mf_field_id ofctrl_run(const struct ovsrec_bridge *br_int); > -void ofctrl_put(struct hmap *flows); > +bool ofctrl_put(struct hmap *flows); > void ofctrl_wait(void); > void ofctrl_destroy(void); > > diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c > index 511b184..2a06f7c 100644 > --- a/ovn/controller/ovn-controller.c > +++ b/ovn/controller/ovn-controller.c > @@ -252,6 +252,34 @@ get_ovnsb_remote_probe_interval(struct > ovsdb_idl *ovs_idl, int *value) > return false; > } > > +/* Set the current configuration of the local chassis to the OVN Southbound > + * next_cfg value. This indicates that the ovn-controller has picked up and > + * applied (queued) the latest flows. > + */ > +static bool > +update_chassis_config(struct controller_ctx *ctx, const char *chassis_id) > +{ > + const struct sbrec_chassis *chassis_rec; > + > + if (!ctx->ovnsb_idl || !ctx->ovnsb_idl_txn) { > + return false; > + } > + > + chassis_rec = get_chassis(ctx->ovnsb_idl, chassis_id); > + if (!chassis_rec) { > + return false; > + } > + > + const struct sbrec_config *sb_config > + = sbrec_config_first(ctx->ovnsb_idl); > + if (!sb_config) { > + return false; > + } > + > + sbrec_chassis_set_cur_cfg(chassis_rec, sb_config->next_cfg); > + return true; > +} > + > int > main(int argc, char *argv[]) > { > @@ -384,7 +412,20 @@ main(int argc, char *argv[]) > br_int, chassis_id, &ct_zones, &flow_table, > &local_datapaths, &patched_datapaths); > } > - ofctrl_put(&flow_table); > + /* Update the cur_cfg of the local chassis if the flow table > + * was (pended to be) modified in a previous iteration but no > + * modified in the current iteration. This indicates that the > + * flow modification has reached a steady state (for now). > + */ > + static bool new_flows_pended = false; > + if (ofctrl_put(&flow_table)) { > + new_flows_pended = true; > + } > + else if (new_flows_pended && chassis_id) { One nit: this is usually pulled together into one line: "} else if ..." > + if (update_chassis_config(&ctx, chassis_id)) { > + new_flows_pended = false; > + } > + } > hmap_destroy(&flow_table); > mcgroup_index_destroy(&mcgroups); > lport_index_destroy(&lports); > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index 44e9430..954978a 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -2382,6 +2382,54 @@ add_column_noalert(struct ovsdb_idl *idl, > ovsdb_idl_omit_alert(idl, column); > } > > +static void > +cp_nextcfg_to_sb(struct northd_context *ctx) > +{ > + const struct nbrec_config *nb; > + const struct sbrec_config *sb; > + > + nb = nbrec_config_first(ctx->ovnnb_idl); > + if (!nb && ctx->ovnnb_txn) { > + /* XXX add verification that table is empty */ > + nb = nbrec_config_insert(ctx->ovnnb_txn); > + } > + > + sb = sbrec_config_first(ctx->ovnsb_idl); > + if (!sb && ctx->ovnsb_txn) { > + /* XXX add verification that table is empty */ > + sb = sbrec_config_insert(ctx->ovnsb_txn); > + } > + > + if (sb && nb && ctx->ovnsb_txn && sb->next_cfg < nb->next_cfg) { > + sbrec_config_set_next_cfg(sb, nb->next_cfg); > + } > +} > + > +/* Compute the cur_cfg of the OVN_Northbound as the minimum of all cur_cfg > + * values in the Chassis Table. > + */ > +static void > +compute_curcfg(struct northd_context *ctx) > +{ > + const struct nbrec_config *nb; > + const struct sbrec_chassis *chassis_rec; > + int64_t cur_cfg_min = (((int64_t)1) << 62) - 1; > + bool cur_cfg_modified = false; > + > + SBREC_CHASSIS_FOR_EACH(chassis_rec, ctx->ovnsb_idl) { > + if (cur_cfg_min > chassis_rec->cur_cfg) { > + cur_cfg_min = chassis_rec->cur_cfg; > + cur_cfg_modified = true; > + } > + } > + > + nb = nbrec_config_first(ctx->ovnnb_idl); > + if (nb && ctx->ovnnb_txn && cur_cfg_modified && > + nb->cur_cfg < cur_cfg_min) { > + nbrec_config_set_cur_cfg(nb, cur_cfg_min); > + } > +} > + > int > main(int argc, char *argv[]) > { > @@ -2452,6 +2500,12 @@ main(int argc, char *argv[]) > add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac); > ovsdb_idl_add_column(ovnsb_idl_loop.idl, > &sbrec_port_binding_col_chassis); > > + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_chassis); > + ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_cur_cfg); > + > + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_config); > + ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_config_col_next_cfg); > + > /* Main loop. */ > exiting = false; > while (!exiting) { > @@ -2462,6 +2516,8 @@ main(int argc, char *argv[]) > .ovnsb_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop), > }; > > + cp_nextcfg_to_sb(&ctx); > + compute_curcfg(&ctx); > ovnnb_db_run(&ctx); > ovnsb_db_run(&ctx); > > diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema > index 8163f6a..fd6e16f 100644 > --- a/ovn/ovn-nb.ovsschema > +++ b/ovn/ovn-nb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Northbound", > "version": "2.1.1", > - "cksum": "2615511875 5108", > + "cksum": "3226126619 5306", > "tables": { > "Logical_Switch": { > "columns": { > @@ -99,6 +99,12 @@ > "ip_prefix": {"type": "string"}, > "nexthop": {"type": "string"}, > "output_port": {"type": {"key": "string", "min": 0, > "max": 1}}}, > - "isRoot": false} > + "isRoot": false}, > + "Config": { > + "columns": { > + "cur_cfg": {"type": "integer"}, > + "next_cfg": {"type": "integer"}}, > + "isRoot": true, > + "maxRows": 1} > } > } > diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml > index c01455d..96dabd4 100644 > --- a/ovn/ovn-nb.xml > +++ b/ovn/ovn-nb.xml > @@ -637,7 +637,7 @@ > column is set to <code>false</code>, the router is disabled. A disabled > router has all ingress and egress traffic dropped. > </column> > - > + > <group title="Common Columns"> > <column name="external_ids"> > See <em>External IDs</em> at the beginning of this document. > @@ -757,4 +757,23 @@ > </column> > </table> > > + <table name="Config" title="OVN Northbound configuration."> > + Configuration for an OVN Northbound DB. There must be exactly > + one record in the <ref table="Config"/> table. > + > + <group title="Status"> > + <column name="next_cfg"> > + Sequence number for client to increment. When a client > + modifies any part of the database configuration and wishes to > + wait for all the ovn-controllers to finish applying the > + changes, it may increment this sequence number. > + </column> > + > + <column name="cur_cfg"> > + Sequence number that ovn-northd sets to the current value of > + <ref column="next_cfg"/> after all the ovn-controllers have > + applied a (configuration) sequence number. > + </column> > + </group> > + </table> > </database> > diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema > index 06e8a07..5d266e2 100644 > --- a/ovn/ovn-sb.ovsschema > +++ b/ovn/ovn-sb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Southbound", > "version": "1.3.0", > - "cksum": "654726257 5528", > + "cksum": "329629017 5732", > "tables": { > "Chassis": { > "columns": { > @@ -13,6 +13,7 @@ > "vtep_logical_switches" : {"type": {"key": "string", > "min": 0, > "max": "unlimited"}}, > + "cur_cfg": {"type": "integer"}, > "external_ids": { > "type": {"key": "string", "value": "string", > "min": 0, "max": "unlimited"}}}, > @@ -110,4 +111,11 @@ > "ip": {"type": "string"}, > "mac": {"type": "string"}}, > "indexes": [["logical_port", "ip"]], > - "isRoot": true}}} > + "isRoot": true}, > + "Config": { > + "columns": { > + "next_cfg": {"type": "integer"}}, > + "isRoot": true, > + "maxRows": 1} > + } > +} > diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml > index efd2f9a..28938ee 100644 > --- a/ovn/ovn-sb.xml > +++ b/ovn/ovn-sb.xml > @@ -179,6 +179,13 @@ > information. > </column> > > + <column name='cur_cfg'> > + <code>ovn-controller</code> populates this value with the value > + of <ref column="next_cfg"/> in <ref table='Config'/> after it > + successfully applies all the logical flows in the OVN > + Southbound database as open flow rules. > + </column> > + > <group title="Common Columns"> > The overall purpose of these columns is described under <code>Common > Columns</code> at the beginning of this document. > @@ -1511,4 +1518,16 @@ tcp.flags = RST; > The Ethernet address to which the IP is bound. > </column> > </table> > + > + <table name="Config" title="OVN Southbound configuration."> > + Configuration for an OVN Northbound DB. There must be exactly > + one record in the <ref table="Config"/> table. > + > + <group title="Status"> > + <column name="next_cfg"> > + Sequence number copied by <code>ovn-northd</code> from the > + OVN Northbound database when that value changes. > + </column> > + </group> > + </table> > </database> > diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml > index 8375ab7..52b092a 100644 > --- a/ovn/utilities/ovn-nbctl.8.xml > +++ b/ovn/utilities/ovn-nbctl.8.xml > @@ -12,6 +12,35 @@ > <h1>General Commands</h1> > > <dl> > + <dt><code>nop</code></dt> > + <dd> > + <p> > + Implements an empty action. It should be used with the > + <var>--wait</var> to wait for the ovn-controllers to sync up with > + the OVN Northbound DB. > + </p> > + <p> > + The <code>ovn-nbctl</code> utility increments the > <var>next_cfg</var> > + in the <code>Config</code> Table of the OVN Northbound. When > + a <code>ovn-controllers</code> picks up and pends the corresponding > + OpenFlow rules on the hypervisors, it sets its own > + <code>Chassis's</code> <var>cur_cfg</var > to the > <var>next_cfg</var> > + of the OVN Southbound DB, which is essentially a copy of > + <var>next_cfg</var> of the OVN Northbound DB. The > + <code>ovn-northd</code> sets the <var>cur_cfg</var> of in the > + OVN Northbound to the minimum value of <var>cur_cfg</var> > + reported by the <code>Chassis'</code>. > + </p> > + <p> > + The <var>--wait</var> option informs <code>ovn-nbctl</code> to > + wait for <var>cur_cfg</var> to equal <var>next_cfg</var>.It should > + be noted that there may be <code>ovn-nbctl</code> actions that > + modify the OVN Northbound <var>next_cfg</var> but do not result in > + OpenFlow rules. In such cases the <var>cur_cfg</var> > will not equal > + the <var>next_cfg</var> till a future operation updates > the OpenFlow > + rules in the hypervisor. > + </p> > + </dd> > <dt><code>show [<var>lswitch</var>]</code></dt> > <dd> > Prints a brief overview of the database contents. If > diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c > index d267829..fd1620c 100644 > --- a/ovn/utilities/ovn-nbctl.c > +++ b/ovn/utilities/ovn-nbctl.c > @@ -48,6 +48,9 @@ static bool oneline; > /* --dry-run: Do not commit any changes. */ > static bool dry_run; > > +/* --wait: Wait for all ovn-controllers to apply open flow rules. */ > +static bool wait_for_controllers = false; > + > /* --timeout: Time to wait for a connection to 'db'. */ > static int timeout; > > @@ -68,7 +71,7 @@ static const char *nbctl_default_db(void); > static void run_prerequisites(struct ctl_command[], size_t n_commands, > struct ovsdb_idl *); > static bool do_nbctl(const char *args, struct ctl_command *, size_t n, > - struct ovsdb_idl *); > + struct ovsdb_idl *, bool might_write_to_db); > > int > main(int argc, char *argv[]) > @@ -79,6 +82,7 @@ main(int argc, char *argv[]) > unsigned int seqno; > size_t n_commands; > char *args; > + bool might_write_to_db; > > set_program_name(argv[0]); > fatal_ignore_sigpipe(); > @@ -90,8 +94,8 @@ main(int argc, char *argv[]) > > /* Log our arguments. This is often valuable for debugging systems. */ > args = process_escape_args(argv); > - VLOG(ctl_might_write_to_db(argv) ? VLL_INFO : VLL_DBG, > - "Called as %s", args); > + might_write_to_db = ctl_might_write_to_db(argv); > + VLOG(might_write_to_db ? VLL_INFO : VLL_DBG, "Called as %s", args); > > /* Parse command line. */ > shash_init(&local_options); > @@ -125,7 +129,8 @@ main(int argc, char *argv[]) > > if (seqno != ovsdb_idl_get_seqno(idl)) { > seqno = ovsdb_idl_get_seqno(idl); > - if (do_nbctl(args, commands, n_commands, idl)) { > + if (do_nbctl(args, commands, n_commands, idl, > + might_write_to_db)) { > free(args); > exit(EXIT_SUCCESS); > } > @@ -157,6 +162,7 @@ parse_options(int argc, char *argv[], struct > shash *local_options) > enum { > OPT_DB = UCHAR_MAX + 1, > OPT_NO_SYSLOG, > + OPT_WAIT, > OPT_DRY_RUN, > OPT_ONELINE, > OPT_LOCAL, > @@ -168,6 +174,7 @@ parse_options(int argc, char *argv[], struct > shash *local_options) > static const struct option global_long_options[] = { > {"db", required_argument, NULL, OPT_DB}, > {"no-syslog", no_argument, NULL, OPT_NO_SYSLOG}, > + {"wait", optional_argument, NULL, OPT_WAIT}, > {"dry-run", no_argument, NULL, OPT_DRY_RUN}, > {"oneline", no_argument, NULL, OPT_ONELINE}, > {"timeout", required_argument, NULL, 't'}, > @@ -224,6 +231,10 @@ parse_options(int argc, char *argv[], struct > shash *local_options) > vlog_set_levels(&this_module, VLF_SYSLOG, VLL_WARN); > break; > > + case OPT_WAIT: > + wait_for_controllers = true; > + break; > + > case OPT_DRY_RUN: > dry_run = true; > break; > @@ -339,6 +350,7 @@ Options:\n\ > --db=DATABASE connect to DATABASE\n\ > (default: %s)\n\ > -t, --timeout=SECS wait at most SECS seconds\n\ > + --wait wait for ovn-controllers to sync with NB\n \ > --dry-run do not commit changes to database\n\ > --oneline print exactly one line of output per > command\n", > program_name, program_name, ctl_get_db_cmd_usage(), > nbctl_default_db()); > @@ -1162,22 +1174,36 @@ run_prerequisites(struct ctl_command > *commands, size_t n_commands, > > static bool > do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands, > - struct ovsdb_idl *idl) > + struct ovsdb_idl *idl, bool might_write_to_db) > { > struct ovsdb_idl_txn *txn; > + const struct nbrec_config *nb; > enum ovsdb_idl_txn_status status; > struct ovsdb_symbol_table *symtab; > struct ctl_context ctx; > struct ctl_command *c; > struct shash_node *node; > char *error = NULL; > + int64_t next_cfg = 0; > > txn = the_idl_txn = ovsdb_idl_txn_create(idl); > if (dry_run) { > ovsdb_idl_txn_set_dry_run(txn); > } > > - ovsdb_idl_txn_add_comment(txn, "ovs-nbctl: %s", args); > + ovsdb_idl_txn_add_comment(txn, "ovn-nbctl: %s", args); > + > + nb = nbrec_config_first(idl); > + if (!nb) { > + /* XXX add verification that table is empty */ > + nb = nbrec_config_insert(txn); > + } > + > + next_cfg = nb->next_cfg; > + if (might_write_to_db) { > + ovsdb_idl_txn_increment(txn, &nb->header_, > + &nbrec_config_col_next_cfg); > + } > > symtab = ovsdb_symbol_table_create(); > for (c = commands; c < &commands[n_commands]; c++) { > @@ -1220,6 +1246,9 @@ do_nbctl(const char *args, struct ctl_command > *commands, size_t n_commands, > } > > status = ovsdb_idl_txn_commit_block(txn); > + if (status == TXN_SUCCESS && might_write_to_db) { > + next_cfg = ovsdb_idl_txn_get_increment_new_value(txn); > + } > if (status == TXN_UNCHANGED || status == TXN_SUCCESS) { > for (c = commands; c < &commands[n_commands]; c++) { > if (c->syntax->postprocess) { > @@ -1296,6 +1325,20 @@ do_nbctl(const char *args, struct ctl_command > *commands, size_t n_commands, > shash_destroy_free_data(&c->options); > } > free(commands); > + > + if (wait_for_controllers) { > + for (;;) { > + ovsdb_idl_run(idl); > + NBREC_CONFIG_FOR_EACH(nb, idl) { > + if (nb->cur_cfg >= next_cfg) { > + goto done; > + } > + } > + ovsdb_idl_wait(idl); > + poll_block(); > + } > + done: ; > + } > ovsdb_idl_txn_destroy(txn); > ovsdb_idl_destroy(idl); > > @@ -1337,6 +1380,7 @@ nbctl_exit(int status) > } > > static const struct ctl_command_syntax nbctl_commands[] = { > + { "nop", 0, 1, "[]", NULL, NULL, NULL, "", RO }, > { "show", 0, 1, "[LSWITCH]", NULL, nbctl_show, NULL, "", RO }, > > /* lswitch commands. */ > diff --git a/tests/ovn.at b/tests/ovn.at > index e6ac1d7..1458134 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -589,8 +589,7 @@ ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type > == 0x1236 && outport == "lp33"' d > ovn_populate_arp > > # Allow some time for ovn-northd and ovn-controller to catch up. > -# XXX This should be more systematic. > -sleep 1 > +ovn-nbctl --wait nop > > # Given the name of a logical port, prints the name of the hypervisor > # on which it is located. > @@ -762,6 +761,10 @@ done > # set address for lp13 with invalid characters. > # lp13 should be configured with only 192.168.0.13. > ovn-nbctl lport-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. > +ovn-nbctl --wait nop > + > sip=`ip_to_hex 192 168 0 11` > tip=`ip_to_hex 192 168 0 13` > test_arp 11 f00000000011 $sip $tip f00000000013 > @@ -898,6 +901,9 @@ for i in 1 2; do > done > done > > +# Allow some time for ovn-northd and ovn-controller to catch up. > +ovn-nbctl --wait nop > + > ovn_populate_arp > > # XXX This is now the 3rd copy of these functions in this file ... > @@ -1108,8 +1114,7 @@ ovs-vsctl add-port br-phys vif3 -- set > Interface vif3 options:tx_pcap=hv3/vif3-t > ovn_populate_arp > > # Allow some time for ovn-northd and ovn-controller to catch up. > -# XXX This should be more systematic. > -sleep 1 > +ovn-nbctl --wait nop > > # test_packet INPORT DST SRC ETHTYPE OUTPORT... > # > @@ -1339,8 +1344,7 @@ done > ovn_populate_arp > > # Allow some time for ovn-northd and ovn-controller to catch up. > -# XXX This should be more systematic. > -sleep 1 > +ovn-nbctl --wait nop > > # test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT... > # > @@ -1698,8 +1702,7 @@ done > ovn_populate_arp > > # Allow some time for ovn-northd and ovn-controller to catch up. > -# XXX This should be more systematic. > -sleep 1 > +ovn-nbctl --wait nop > > # Given the name of a logical port, prints the name of the hypervisor > # on which it is located. > @@ -1937,7 +1940,8 @@ done > # configure lport13 to send and received IPv4 packets with an address range > ovn-nbctl lport-set-port-security lp13 "f0:00:00:00:00:13 192.168. > 0.13 20.0.0.4/24 10.0.0.0/24" > > -sleep 2 > +# Allow some time for ovn-northd and ovn-controller to catch up. > +ovn-nbctl --wait nop > > sip=`ip_to_hex 10 0 0 13` > tip=`ip_to_hex 192 168 0 22` > @@ -2130,8 +2134,7 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ > ovn_populate_arp > > # Allow some time for ovn-northd and ovn-controller to catch up. > -# XXX This should be more systematic. > -sleep 1 > +ovn-nbctl --wait nop > > # Send ip packets between the two ports. > ip_to_hex() { > @@ -2262,8 +2265,7 @@ ovs-vsctl -- add-port br-int vif2 -- \ > > > # Allow some time for ovn-northd and ovn-controller to catch up. > -# XXX This should be more systematic. > -sleep 1 > +ovn-nbctl --wait nop > > # Send ip packets between the two ports. > ip_to_hex() { > @@ -2303,6 +2305,9 @@ 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. > +ovn-nbctl --wait nop > + > echo "---------SB dump-----" > ovn-sbctl list datapath_binding > echo "---------------------" > @@ -2459,8 +2464,7 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ > ovn_populate_arp > > # Allow some time for ovn-northd and ovn-controller to catch up. > -# XXX This should be more systematic. > -sleep 1 > +ovn-nbctl --wait nop > > ip_to_hex() { > printf "%02x%02x%02x%02x" "$@" > @@ -2577,6 +2581,9 @@ AT_CHECK([ovn-nbctl lport-set-options ln_port > network_name=physnet1]) > > AT_CHECK([ovs-vsctl add-port br-int localvif1 -- set Interface > localvif1 external_ids:iface-id=localvif1]) > > +# Allow some time for ovn-northd and ovn-controller to catch up. > +ovn-nbctl --wait nop > + > # Wait for packet to be received. > OVS_WAIT_UNTIL([test `wc -c < "hv/snoopvif-tx.pcap"` -ge 50]) > trim_zeros() { > -- > 2.7.4 (Apple Git-66) > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev Other than the above nit, this looks like what I had in mind... Still running through it in some more detail to make sure it handles some of the nastier problems I've seen... Ryan _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
