On 6/23/26 1:51 PM, Lorenzo Bianconi wrote: > Filter outgoing monitor requests in the IDL layer based on the > db-schema received from the server, skipping tables and columns not > available in the schema. > To support this, store the user-provided monitor condition in pre-JSON > format in ovsdb_idl_table when ovsdb_idl_set_condition() is called, > allowing ovsdb_idl_compose_monitor_request() to build monitor > conditional requests that only include tables and columns present in > the db-schema. > > Reported-at: https://issues.redhat.com/browse/FDP-3114 > Signed-off-by: Lorenzo Bianconi <[email protected]> > ---
Hi Lorenzo, Ilya, I have a few minor comments down below. But I'd also be OK if this exact version gets merged. I'll leave it up to you to decide if you want to integrate my feedback or not. In any case: Acked-by: Dumitru Ceara <[email protected]> Thanks for all the work on this, it will make it a bit easier for clients (OVN) to use the IDL IMO. Regards, Dumitru > Changes in v6: > - Rebase on top of ovs main branch. > - cosmetics. > Changes in v5: > - Add self-test for last_id management in raft mode. > - Use a pointer for req_cond instead of a struct. > - Fix typos. > - Do not reset server_schema_received in ovsdb_idl_run() > - Move hmap_is_empty() after in_server_schema check in > ovsdb_idl_set_condition__(). > - Add for coverage tests. > Changes in v4: > - Filter monitor requests in ovsdb_idl_set_condition__() > - Cosmetics > - Move ovsdb_cs_db_sync_condition() in ovsdb_cs_send_monitor_request() to > properly update ack_cond json struct > - Store original user monitor request in req_cond in ovsdb_idl_set_condition() > - Add more uni-tests > - Add more comments > Changes in v3: > - Fix seqno reporting when the filtered condition is empty. > Changes in v2: > - Add missing unit-test. > - squash with patch 'ovsdb: Add ovsdb_cs_clear_condition routine to remove > stable ovsdb_cs_db_table entries.'. > - fix the error condition reported by Ilya. > - Remove unnecessary ovsdb_cs_db_sync_condition() in > ovsdb_cs_send_monitor_request(). > - cosmetics. > --- > lib/ovsdb-cs.c | 50 +++++++--- > lib/ovsdb-cs.h | 2 + > lib/ovsdb-idl-provider.h | 3 + > lib/ovsdb-idl.c | 111 ++++++++++++++++++++++- > tests/ovsdb-idl.at | 142 ++++++++++++++++++++++++++++- > tests/test-ovsdb.c | 191 ++++++++++++++++++++++++++++++++++++--- > 6 files changed, 468 insertions(+), 31 deletions(-) > > diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c > index d96a97599..aaa1611c3 100644 > --- a/lib/ovsdb-cs.c > +++ b/lib/ovsdb-cs.c > @@ -412,12 +412,6 @@ ovsdb_cs_retry_at(struct ovsdb_cs *cs, const char *where) > static void > ovsdb_cs_restart_fsm(struct ovsdb_cs *cs) > { > - /* Resync data DB table conditions to avoid missing updates due to > - * conditions that were in flight or changed locally while the connection > - * was down. > - */ > - ovsdb_cs_db_sync_condition(&cs->data); > - > ovsdb_cs_send_schema_request(cs, &cs->server); > ovsdb_cs_transition(cs, CS_S_SERVER_SCHEMA_REQUESTED); > cs->data.monitor_version = 0; > @@ -912,17 +906,47 @@ ovsdb_cs_db_get_table(struct ovsdb_cs_db *db, const > char *table) > return t; > } > > +static void > +ovsdb_cs_db_destroy_table(struct ovsdb_cs_db_table *table, > + struct ovsdb_cs_db *db) > +{ > + json_destroy(table->ack_cond); > + json_destroy(table->req_cond); > + json_destroy(table->new_cond); > + hmap_remove(&db->tables, &table->hmap_node); > + free(table->name); > + free(table); > +} > + > +/* Destroy a given ovsdb_cs_db_table according to the table name. */ > +void > +ovsdb_cs_clear_condition(struct ovsdb_cs *cs, const char *table) Nit: The function name reads a bit weird. The comment speaks about destroying the table, which is what the code does but the function name is about clearing the condition. Should we call this "ovsdb_cs_destroy_table()"? > +{ > + uint32_t hash = hash_string(table, 0); > + struct ovsdb_cs_db *db = &cs->data; > + > + struct ovsdb_cs_db_table *t; > + HMAP_FOR_EACH_WITH_HASH (t, hmap_node, hash, &db->tables) { > + if (!strcmp(t->name, table)) { > + ovsdb_cs_db_destroy_table(t, db); > + db->last_id = UUID_ZERO; > + return; > + } > + } > +} > + > +void > +ovsdb_cs_reset_last_id(struct ovsdb_cs *cs) > +{ > + cs->data.last_id = UUID_ZERO; > +} > + > static void > ovsdb_cs_db_destroy_tables(struct ovsdb_cs_db *db) > { > struct ovsdb_cs_db_table *table; > HMAP_FOR_EACH_SAFE (table, hmap_node, &db->tables) { > - json_destroy(table->ack_cond); > - json_destroy(table->req_cond); > - json_destroy(table->new_cond); > - hmap_remove(&db->tables, &table->hmap_node); > - free(table->name); > - free(table); > + ovsdb_cs_db_destroy_table(table, db); > } > hmap_destroy(&db->tables); > } > @@ -1501,6 +1525,8 @@ ovsdb_cs_send_monitor_request(struct ovsdb_cs *cs, > struct ovsdb_cs_db *db, > /* XXX handle failure */ > ovs_assert(mrs->type == JSON_OBJECT); > > + ovsdb_cs_db_sync_condition(db); > + > if (version > 1) { > struct ovsdb_cs_db_table *table; > HMAP_FOR_EACH (table, hmap_node, &db->tables) { > diff --git a/lib/ovsdb-cs.h b/lib/ovsdb-cs.h > index bcc3dcd71..98c921cd8 100644 > --- a/lib/ovsdb-cs.h > +++ b/lib/ovsdb-cs.h > @@ -144,6 +144,8 @@ void ovsdb_cs_set_probe_interval(const struct ovsdb_cs *, > int probe_interval); > unsigned int ovsdb_cs_set_condition(struct ovsdb_cs *, const char *table, > const struct json *condition); > unsigned int ovsdb_cs_get_condition_seqno(const struct ovsdb_cs *); > +void ovsdb_cs_clear_condition(struct ovsdb_cs *, const char *table); > +void ovsdb_cs_reset_last_id(struct ovsdb_cs *); > > /* Database change awareness. */ > void ovsdb_cs_set_db_change_aware(struct ovsdb_cs *, bool > set_db_change_aware); > diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h > index 6cf32fb50..2e0ab3003 100644 > --- a/lib/ovsdb-idl-provider.h > +++ b/lib/ovsdb-idl-provider.h > @@ -130,6 +130,9 @@ struct ovsdb_idl_table { > * or not. */ > struct ovs_list indexes; /* Contains "struct ovsdb_idl_index"s */ > struct ovs_list track_list; /* Tracked rows (ovsdb_idl_row.track_node). > */ > + > + struct ovsdb_idl_condition *req_cond; /* User requested monitor > + * condition. */ > }; > > struct ovsdb_idl_class { > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index fe90deda7..8d85a23f1 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -99,6 +99,8 @@ struct ovsdb_idl { > struct ovs_list rows_to_reparse; /* Stores rows that might need to be > * re-parsed due to insertion of a > * referenced row. */ > + bool server_schema_received; /* Set to true when the IDL has received > + * the DB schema from the server. */ > }; > > static struct ovsdb_cs_ops ovsdb_idl_cs_ops; > @@ -205,6 +207,17 @@ static void ovsdb_idl_add_to_indexes(const struct > ovsdb_idl_row *); > static void ovsdb_idl_remove_from_indexes(const struct ovsdb_idl_row *); > static int ovsdb_idl_try_commit_loop_txn(struct ovsdb_idl_loop *loop, > bool *may_need_wakeup); > +static void ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dest, > + const struct ovsdb_idl_condition *); > +static void ovsdb_idl_create_req_condition( > + struct ovsdb_idl *, > + const struct ovsdb_idl_table_class *, > + const struct ovsdb_idl_condition *); > +static void ovsdb_idl_destroy_req_condition(struct ovsdb_idl_table *); > +static unsigned int ovsdb_idl_set_condition__( > + struct ovsdb_idl *, > + const struct ovsdb_idl_table_class *, > + const struct ovsdb_idl_condition *); > > static void add_tracked_change_for_references(struct ovsdb_idl_row *); > > @@ -266,6 +279,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class > *class, > .txn = NULL, > .outstanding_txns = HMAP_INITIALIZER(&idl->outstanding_txns), > .verify_write_only = false, > + .server_schema_received = false, > .deleted_untracked_rows > = OVS_LIST_INITIALIZER(&idl->deleted_untracked_rows), > .rows_to_reparse > @@ -298,6 +312,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class > *class, > = table->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0; > table->idl = idl; > table->in_server_schema = false; > + table->req_cond = NULL; > shash_init(&table->schema_columns); > } > > @@ -372,6 +387,7 @@ ovsdb_idl_destroy(struct ovsdb_idl *idl) > > ovsdb_idl_schema_columns_clear(&table->schema_columns); > shash_destroy(&table->schema_columns); > + ovsdb_idl_destroy_req_condition(table); > > hmap_destroy(&table->rows); > free(table->modes); > @@ -788,6 +804,7 @@ ovsdb_idl_compose_monitor_request(const struct json > *schema_json, void *idl_) > struct shash *schema = ovsdb_cs_parse_schema(schema_json); > struct json *monitor_requests = json_object_create(); > > + idl->server_schema_received = true; > for (size_t i = 0; i < idl->class_->n_tables; i++) { > struct ovsdb_idl_table *table = &idl->tables[i]; > const struct ovsdb_idl_table_class *tc = table->class_; > @@ -842,8 +859,12 @@ ovsdb_idl_compose_monitor_request(const struct json > *schema_json, void *idl_) > idl->class_->database, table->class_->name); > json_destroy(columns); > table->in_server_schema = false; > + ovsdb_cs_clear_condition(idl->cs, table->class_->name); > continue; > } else if (schema && table_schema) { > + if (!table->in_server_schema) { > + ovsdb_cs_reset_last_id(idl->cs); > + } I initially got confused here and thought that this would cause issues and always reset the last_id every time the IDL client reconnects to the server. But that's actually not true. This is correct because: - when the client starts up (last_id is already 0), no tables are in the server schema - then the schema is received so the tables that are actually on the server side get 'in_server_schema': false->true and last_id is "reset" (it was already 0 so that's fine) - only if we reconnect and the server has upgraded the schema (more tables are added) we reset last_id. So it's fine but it makes me wonder if we should add a tiny comment here for it. > table->in_server_schema = true; > } > > @@ -852,6 +873,14 @@ ovsdb_idl_compose_monitor_request(const struct json > *schema_json, void *idl_) > json_object_put(monitor_requests, tc->name, > json_array_create_1(monitor_request)); > } > + > + if (!table->in_server_schema) { > + ovsdb_cs_clear_condition(idl->cs, table->class_->name); > + } else if (table->req_cond) { > + /* Update the monitor condition request according to the > + * db schema. */ > + ovsdb_idl_set_condition__(idl, tc, table->req_cond); > + } > } > ovsdb_cs_free_schema(schema); > > @@ -1156,6 +1185,41 @@ ovsdb_idl_condition_add_clause__(struct > ovsdb_idl_condition *condition, > hmap_insert(&condition->clauses, &clause->hmap_node, hash); > } > > +static void > +ovsdb_idl_destroy_req_condition(struct ovsdb_idl_table *table) > +{ > + ovsdb_idl_condition_destroy(table->req_cond); > + free(table->req_cond); > + table->req_cond = NULL; > +} > + > +static void > +ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dest, > + const struct ovsdb_idl_condition *source) > +{ > + ovsdb_idl_condition_init(dest); > + > + struct ovsdb_idl_clause *clause; > + HMAP_FOR_EACH (clause, hmap_node, &source->clauses) { > + ovsdb_idl_condition_add_clause__(dest, clause, > clause->hmap_node.hash); > + } > + dest->is_true = source->is_true; > +} > + > +static void > +ovsdb_idl_create_req_condition(struct ovsdb_idl *idl, > + const struct ovsdb_idl_table_class *tc, > + const struct ovsdb_idl_condition *condition) > +{ > + struct ovsdb_idl_table *table = shash_find_data(&idl->table_by_name, > + tc->name); > + if (table) { > + ovsdb_idl_destroy_req_condition(table); > + table->req_cond = xzalloc(sizeof *table->req_cond); > + ovsdb_idl_condition_clone(table->req_cond, condition); > + } > +} > + > /* Adds a clause to the condition for replicating the table with class 'tc' > in > * 'idl'. > * > @@ -1234,6 +1298,47 @@ ovsdb_idl_condition_to_json(const struct > ovsdb_idl_condition *cnd) > return json_array_create(clauses, n); > } > > +static unsigned int > +ovsdb_idl_set_condition__(struct ovsdb_idl *idl, > + const struct ovsdb_idl_table_class *tc, > + const struct ovsdb_idl_condition *condition) > +{ > + struct ovsdb_idl_condition filter_cond = > + OVSDB_IDL_CONDITION_INIT(&filter_cond); > + struct json *cond_json; > + unsigned int seqno; > + > + if (!idl->server_schema_received) { > + goto out; > + } > + > + struct ovsdb_idl_table *t = ovsdb_idl_table_from_class(idl, tc); > + if (!t || !t->in_server_schema) { nit: We _don't_ leak anything from 'fiter_cond' here but it looks weird that we don't destroy it. I'm not sure if we should change this or not, I'm on the fence with this one. I'm not against leaving it as is though. > + return ovsdb_idl_get_condition_seqno(idl); > + } > + > + if (hmap_is_empty(&condition->clauses)) { > + goto out; > + } > + > + struct ovsdb_idl_clause *clause; > + HMAP_FOR_EACH (clause, hmap_node, &condition->clauses) { > + if (ovsdb_idl_server_has_column(idl, clause->column)) { > + ovsdb_idl_condition_add_clause__(&filter_cond, clause, > + clause->hmap_node.hash); > + } > + } > + condition = &filter_cond; > +out: > + cond_json = ovsdb_idl_condition_to_json(condition); > + seqno = ovsdb_cs_set_condition(idl->cs, tc->name, cond_json); > + json_destroy(cond_json); > + > + ovsdb_idl_condition_destroy(&filter_cond); > + > + return seqno; > +} > + > /* Sets the replication condition for 'tc' in 'idl' to 'condition' and > * arranges to send the new condition to the database server. > * > @@ -1245,10 +1350,8 @@ ovsdb_idl_set_condition(struct ovsdb_idl *idl, > const struct ovsdb_idl_table_class *tc, > const struct ovsdb_idl_condition *condition) > { > - struct json *cond_json = ovsdb_idl_condition_to_json(condition); > - unsigned int seqno = ovsdb_cs_set_condition(idl->cs, tc->name, > cond_json); > - json_destroy(cond_json); > - return seqno; > + ovsdb_idl_create_req_condition(idl, tc, condition); > + return ovsdb_idl_set_condition__(idl, tc, condition); > } > > /* Turns off OVSDB_IDL_ALERT and OVSDB_IDL_TRACK for 'column' in 'idl'. > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at > index 15d0e4764..cd534e09d 100644 > --- a/tests/ovsdb-idl.at > +++ b/tests/ovsdb-idl.at > @@ -2696,7 +2696,7 @@ dnl idltest2.ovsschema and outputs the presence of > tables and columns. > dnl And then it connectes to the server with the schema idltest.ovsschema > dnl and does the same. > AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 dnl > - idl-table-column-check unix:socket unix:socket2], [0], [dnl > + idl-table-column-check unix:socket unix:socket2 > unix:socket], [0], [dnl > unix:socket remote has table simple > unix:socket remote has table link1 > unix:socket remote doesn't have table link2 > @@ -2715,11 +2715,151 @@ unix:socket2 remote has col l2 in table link1, type: > set of up to 1 uuids > unix:socket2 remote has col i in table link1, type: integer > unix:socket2 remote has col id in table simple7, type: string > --- remote unix:socket2 done --- > +unix:socket remote has table simple > +unix:socket remote has table link1 > +unix:socket remote doesn't have table link2 > +unix:socket remote doesn't have table simple5 > +unix:socket remote doesn't have col irefmap in table simple5 > +unix:socket remote doesn't have col l2 in table link1 > +unix:socket remote has col i in table link1, type: integer > +unix:socket remote doesn't have col id in table simple7 > +--- remote unix:socket done --- > +], [stderr]) > + > +# Check we do not have any errors related to conditional monitoring. > +AT_CHECK([grep 'received error' stderr], [1]) > + > +# Check the monitor_cond_since request has properly formatted > +# conditions connecting to the idltest2 server. > +AT_CHECK([grep monitor_cond_since stderr | \ > + grep -qF > '"l2","==",@<:@"uuid","12345678-dd3f-4616-ab6a-83a490bb0991"@:>@']) > +AT_CHECK([grep monitor_cond_since stderr | grep -qF '"i","==",1']) > + > +OVSDB_SERVER_SHUTDOWN > +AT_CLEANUP > + > +AT_SETUP([idl set_condition after connection]) > +AT_KEYWORDS([ovsdb server idl set_condition post connect]) > +OVSDB_START_IDLTEST([], ["$abs_srcdir/idltest2.ovsschema"]) > + > +dnl This test connects to a server with the limited idltest2 schema, > +dnl waits for the initial data, and only then calls set_condition(). > +dnl This exercises the code path where conditions are set after a > +dnl successful connection (server_schema_received is already true). > +dnl > +dnl Two conditions are set: > +dnl - link1.l2 == <uuid>: link1 is in idltest2 but l2 column is not, > +dnl so the clause must be filtered out. > +dnl - link2.i == 1: link2 table is not in idltest2 at all, so the > +dnl condition must be silently dropped. > +AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' \ > + -vjsonrpc -t10 \ > + idl-set-condition-post-connect \ > + unix:socket], [0], [dnl > +link1 set_condition: seqno=1 > +link2 set_condition: seqno=0 > +current condition seqno: 0 > +condition acked: seqno=1 > ], [stderr]) > > +# No errors about missing columns/tables in monitor_cond_change. > +AT_CHECK([grep 'received error' stderr], [1]) > + > +# The monitor_cond_change must NOT contain the l2 clause > +# (column absent from idltest2 schema). > +AT_CHECK([grep monitor_cond_change stderr | \ > + grep -qF > '"l2","==",@<:@"uuid","12345678-dd3f-4616-ab6a-83a490bb0991"@:>@'], \ > + [1]) > + > +# The monitor_cond_change must contain the i clause > +# (column present in idltest2 schema). > +AT_CHECK([grep monitor_cond_change stderr | grep -qF '"i","==",1']) > + > +# The monitor_cond_change must NOT contain link2 table > +# (table absent from idltest2 schema). > +AT_CHECK([grep monitor_cond_change stderr | grep -qF '"link2"'], [1]) > + > OVSDB_SERVER_SHUTDOWN > AT_CLEANUP > > +AT_SETUP([idl reconnect does not reset last_id for missing tables]) > +AT_KEYWORDS([ovsdb server idl reconnect last_id cluster]) > + > +dnl Use single-node raft clusters so that the servers maintain > +dnl transaction history and monitor_cond_since with a non-zero > +dnl last_id returns found=true with incremental updates. > +dnl > +dnl Server 1 (socket): limited schema (idltest2) - lacks link2 table > +dnl and l2 column in link1. > +dnl Server 2 (socket2): full schema (idltest) - has all tables/columns. > +AT_CHECK([ovsdb-tool create-cluster s1.db \ > + $abs_srcdir/idltest2.ovsschema unix:s1.raft]) > +schema_name=$(ovsdb-tool schema-name $abs_srcdir/idltest2.ovsschema) > +on_exit 'kill $(cat s*.pid)' > +AT_CHECK([ovsdb-server -vraft -vconsole:warn -vfile:dbg --detach \ > + --no-chdir --log-file=s1.log --pidfile=s1.pid \ > + --unixctl=s1 --remote=punix:socket s1.db]) > +OVS_WAIT_UNTIL([ovs-appctl -t $(pwd)/s1 cluster/status \ > + ${schema_name} | grep -q 'Status: cluster member']) > + > +AT_CHECK([ovsdb-tool create-cluster s2.db \ > + $abs_srcdir/idltest.ovsschema unix:s2.raft]) > +AT_CHECK([ovsdb-server -vraft -vconsole:warn -vfile:dbg --detach \ > + --no-chdir --log-file=s2.log --pidfile=s2.pid \ > + --unixctl=s2 --remote=punix:socket2 s2.db]) > +OVS_WAIT_UNTIL([ovs-appctl -t $(pwd)/s2 cluster/status \ > + ${schema_name} | grep -q 'Status: cluster member']) > + > +dnl This test sets a condition on link2 (not in idltest2 schema) before > +dnl connecting, then: > +dnl 1. Connects to the limited-schema server (socket) and gets data. > +dnl 2. Reconnects to the same server - last_id must be preserved > +dnl (non-zero) because the CS entry for link2 was already destroyed > +dnl on the first connection, so ovsdb_cs_clear_condition() is a > +dnl no-op. > +dnl 3. Connects to the full-schema server (socket2), simulating a > +dnl schema upgrade. The client must reset last_id to zero because > +dnl previously missing tables/columns are now available and the > +dnl old transaction history does not cover them. > +AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' \ > + -vjsonrpc -t10 \ > + idl-reconnect-last-id \ > + unix:socket unix:socket2], [0], [dnl > +done > +], [stderr]) > + > +# There should be three data DB monitor_cond_since requests. > +AT_CHECK([grep monitor_cond_since stderr | grep 'send request' | \ > + grep -c '"idltest"'], [0], [3 > +]) > + > +# First data DB request (initial connect to limited schema): > +# must use UUID_ZERO as last_id (no prior state). > +AT_CHECK([grep monitor_cond_since stderr | grep 'send request' | \ > + grep '"idltest"' | sed -n '1p' | \ > + grep -qF '"00000000-0000-0000-0000-000000000000"']) > + > +# Second data DB request (reconnect to same limited schema): > +# must NOT use UUID_ZERO, proving last_id was preserved. > +AT_CHECK([grep monitor_cond_since stderr | grep 'send request' | \ > + grep '"idltest"' | sed -n '2p' | \ > + grep -qF '"00000000-0000-0000-0000-000000000000"'], [1]) > + > +# Third data DB request (connect to full schema / upgrade): > +# must use UUID_ZERO because previously missing tables are now > +# present and the old transaction history does not cover them. > +AT_CHECK([grep monitor_cond_since stderr | grep 'send request' | \ > + grep '"idltest"' | sed -n '3p' | \ > + grep -qF '"00000000-0000-0000-0000-000000000000"']) > + > +# No errors about missing columns/tables. > +AT_CHECK([grep 'received error' stderr], [1]) > + > +OVS_APP_EXIT_AND_WAIT_BY_TARGET([$(pwd)/s1], [s1.pid]) > +OVS_APP_EXIT_AND_WAIT_BY_TARGET([$(pwd)/s2], [s2.pid]) > + > +AT_CLEANUP > + > dnl This test checks that inserting and deleting the source of a reference > dnl doesn't remove the reference in the (deleted) source tracked record. > OVSDB_CHECK_IDL_TRACK([track, insert and delete, refs to link1], > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c > index bef01486f..014519ccc 100644 > --- a/tests/test-ovsdb.c > +++ b/tests/test-ovsdb.c > @@ -3565,6 +3565,24 @@ do_idl_compound_index(struct ovs_cmdl_context *ctx) > printf("%03d: done\n", step); > } > > +static void > +idl_wait_for_seqno_change(struct ovsdb_idl *idl, struct jsonrpc *rpc, > + unsigned int seqno) > +{ > + for (;;) { > + ovsdb_idl_run(idl); > + ovsdb_idl_check_consistency(idl); > + if (ovsdb_idl_get_seqno(idl) != seqno) { > + break; > + } > + jsonrpc_run(rpc); > + > + ovsdb_idl_wait(idl); > + jsonrpc_wait(rpc); > + poll_block(); > + } > +} > + > static void > do_idl_table_column_check(struct ovs_cmdl_context *ctx) > { > @@ -3577,6 +3595,21 @@ do_idl_table_column_check(struct ovs_cmdl_context *ctx) > ovsdb_idl_omit(idl, &idltest_link1_col_i); > ovsdb_idl_omit(idl, &idltest_simple7_col_id); > ovsdb_idl_set_leader_only(idl, false); > + > + struct ovsdb_idl_condition cond_link1 = > + OVSDB_IDL_CONDITION_INIT(&cond_link1); > + struct uuid uuid; > + > + uuid_from_string(&uuid, "12345678-dd3f-4616-ab6a-83a490bb0991"); > + idltest_link1_add_clause_l2(&cond_link1, OVSDB_F_EQ, &uuid); > + idltest_link1_set_condition(idl, &cond_link1); > + > + struct ovsdb_idl_condition cond_link2 = > + OVSDB_IDL_CONDITION_INIT(&cond_link2); > + > + idltest_link2_add_clause_i(&cond_link2, OVSDB_F_EQ, 1); > + idltest_link2_set_condition(idl, &cond_link2); > + > struct stream *stream; > > error = stream_open_block(jsonrpc_stream_open(ctx->argv[1], &stream, > @@ -3586,23 +3619,12 @@ do_idl_table_column_check(struct ovs_cmdl_context > *ctx) > } > rpc = jsonrpc_open(stream); > > - for (int r = 1; r <= 2; r++) { > + for (int r = 1; r <= 3; r++) { > ovsdb_idl_set_remote(idl, ctx->argv[r], true); > ovsdb_idl_force_reconnect(idl); > > /* Wait for update. */ > - for (;;) { > - ovsdb_idl_run(idl); > - ovsdb_idl_check_consistency(idl); > - if (ovsdb_idl_get_seqno(idl) != seqno) { > - break; > - } > - jsonrpc_run(rpc); > - > - ovsdb_idl_wait(idl); > - jsonrpc_wait(rpc); > - poll_block(); > - } > + idl_wait_for_seqno_change(idl, rpc, seqno); > > seqno = ovsdb_idl_get_seqno(idl); > > @@ -3658,6 +3680,143 @@ do_idl_table_column_check(struct ovs_cmdl_context > *ctx) > printf("--- remote %s done ---\n", ctx->argv[r]); > } > > + ovsdb_idl_condition_destroy(&cond_link1); > + ovsdb_idl_condition_destroy(&cond_link2); > + > + jsonrpc_close(rpc); > + ovsdb_idl_destroy(idl); > +} > + > +static void > +do_idl_set_condition_post_connect(struct ovs_cmdl_context *ctx) > +{ > + struct jsonrpc *rpc; > + struct ovsdb_idl *idl; > + unsigned int seqno = 0; > + int error; > + > + idl = ovsdb_idl_create(ctx->argv[1], &idltest_idl_class, true, true); > + ovsdb_idl_omit(idl, &idltest_link1_col_i); > + ovsdb_idl_omit(idl, &idltest_simple7_col_id); > + ovsdb_idl_set_leader_only(idl, false); > + > + struct stream *stream; > + > + error = stream_open_block(jsonrpc_stream_open(ctx->argv[1], &stream, > + DSCP_DEFAULT), -1, &stream); > + if (error) { > + ovs_fatal(error, "failed to connect to \"%s\"", ctx->argv[1]); > + } > + rpc = jsonrpc_open(stream); > + > + ovsdb_idl_set_remote(idl, ctx->argv[1], true); > + > + /* Wait for initial data to arrive. */ > + idl_wait_for_seqno_change(idl, rpc, seqno); > + > + /* Set conditions after the schema has been received. > + * For link1, set two clauses: l2 (not in idltest2 schema, will be > + * filtered) and i (in idltest2 schema, will pass through). */ > + struct ovsdb_idl_condition cond_link1 = > + OVSDB_IDL_CONDITION_INIT(&cond_link1); > + struct uuid uuid; > + unsigned int link1_seqno; > + unsigned int link2_seqno; > + > + uuid_from_string(&uuid, "12345678-dd3f-4616-ab6a-83a490bb0991"); > + idltest_link1_add_clause_l2(&cond_link1, OVSDB_F_EQ, &uuid); > + idltest_link1_add_clause_i(&cond_link1, OVSDB_F_EQ, 1); > + link1_seqno = idltest_link1_set_condition(idl, &cond_link1); > + > + struct ovsdb_idl_condition cond_link2 = > + OVSDB_IDL_CONDITION_INIT(&cond_link2); > + > + idltest_link2_add_clause_i(&cond_link2, OVSDB_F_EQ, 1); > + link2_seqno = idltest_link2_set_condition(idl, &cond_link2); > + > + printf("link1 set_condition: seqno=%u\n", link1_seqno); > + printf("link2 set_condition: seqno=%u\n", link2_seqno); > + printf("current condition seqno: %u\n", > + ovsdb_idl_get_condition_seqno(idl)); > + > + /* Wait for condition acknowledgement. */ > + for (;;) { > + ovsdb_idl_run(idl); > + ovsdb_idl_check_consistency(idl); > + if (ovsdb_idl_get_condition_seqno(idl) == link1_seqno) { > + break; > + } > + jsonrpc_run(rpc); > + > + ovsdb_idl_wait(idl); > + jsonrpc_wait(rpc); > + poll_block(); > + } > + > + printf("condition acked: seqno=%u\n", > + ovsdb_idl_get_condition_seqno(idl)); > + > + ovsdb_idl_condition_destroy(&cond_link1); > + ovsdb_idl_condition_destroy(&cond_link2); > + > + jsonrpc_close(rpc); > + ovsdb_idl_destroy(idl); > +} > + > +static void > +do_idl_reconnect_last_id(struct ovs_cmdl_context *ctx) > +{ > + struct jsonrpc *rpc; > + struct ovsdb_idl *idl; > + unsigned int seqno = 0; > + int error; > + > + idl = ovsdb_idl_create(ctx->argv[1], &idltest_idl_class, true, true); > + ovsdb_idl_set_leader_only(idl, false); > + > + /* Set a condition on link2 before connecting. This creates a CS > + * table entry that will be destroyed on the first connection (link2 > + * is not in idltest2). On reconnect, the entry is already gone so > + * ovsdb_cs_clear_condition() must be a no-op and must NOT reset > + * last_id. > + * > + * When later connecting to a server with the full schema (all > + * tables/columns present), the client must reset last_id to zero > + * to force a full re-download that covers the newly available > + * tables. */ > + struct ovsdb_idl_condition cond_link2 = > + OVSDB_IDL_CONDITION_INIT(&cond_link2); > + idltest_link2_add_clause_i(&cond_link2, OVSDB_F_EQ, 1); > + idltest_link2_set_condition(idl, &cond_link2); > + > + struct stream *stream; > + > + error = stream_open_block(jsonrpc_stream_open(ctx->argv[1], &stream, > + DSCP_DEFAULT), -1, &stream); > + if (error) { > + ovs_fatal(error, "failed to connect to \"%s\"", ctx->argv[1]); > + } > + rpc = jsonrpc_open(stream); > + > + /* Step 1: connect to the limited-schema server. */ > + ovsdb_idl_set_remote(idl, ctx->argv[1], true); > + idl_wait_for_seqno_change(idl, rpc, seqno); > + seqno = ovsdb_idl_get_seqno(idl); > + > + /* Step 2: reconnect to the same limited-schema server. */ > + ovsdb_idl_force_reconnect(idl); > + idl_wait_for_seqno_change(idl, rpc, seqno); > + seqno = ovsdb_idl_get_seqno(idl); > + > + /* Step 3: connect to the full-schema server (upgrade scenario). */ > + ovsdb_idl_set_remote(idl, ctx->argv[2], true); > + ovsdb_idl_force_reconnect(idl); > + idl_wait_for_seqno_change(idl, rpc, seqno); > + > + printf("done\n"); > + > + ovsdb_idl_condition_destroy(&cond_link2); > + > jsonrpc_close(rpc); > ovsdb_idl_destroy(idl); > } > @@ -3700,8 +3859,12 @@ static struct ovs_cmdl_command all_commands[] = { > do_idl_partial_update_map_column, OVS_RO }, > { "idl-partial-update-set-column", NULL, 1, INT_MAX, > do_idl_partial_update_set_column, OVS_RO }, > - { "idl-table-column-check", NULL, 2, 2, > + { "idl-table-column-check", NULL, 3, 3, > do_idl_table_column_check, OVS_RO }, > + { "idl-set-condition-post-connect", NULL, 1, 1, > + do_idl_set_condition_post_connect, OVS_RO }, > + { "idl-reconnect-last-id", NULL, 2, 2, > + do_idl_reconnect_last_id, OVS_RO }, > { "help", NULL, 0, INT_MAX, do_help, OVS_RO }, > { NULL, NULL, 0, 0, NULL, OVS_RO }, > }; _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
