On 3/10/26 12:59 PM, Ilya Maximets wrote: > On 3/3/26 6:08 PM, Lorenzo Bianconi wrote: >> Introduce idl_connect_notifier callback in idl codebase in order to notify >> IDL >> consumer (e.g. ovn-controller) when the connection to remote db has been >> completed and db schema has been downloaded. >> ovn-controller requires this info to properly manage condition monitoring for >> tables that are not present in old db schemas and keep backward >> compatibility. >> >> Reported-at: https://issues.redhat.com/browse/FDP-3114 >> Signed-off-by: Lorenzo Bianconi <[email protected]> >> --- >> lib/ovsdb-cs.c | 10 ++++------ >> lib/ovsdb-idl.c | 16 ++++++++++++++++ >> lib/ovsdb-idl.h | 3 +++ >> 3 files changed, 23 insertions(+), 6 deletions(-) >> >> diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c >> index df33a835d..2ddda6837 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; >> @@ -1511,6 +1505,10 @@ ovsdb_cs_send_monitor_request(struct ovsdb_cs *cs, >> struct ovsdb_cs_db *db, >> /* XXX handle failure */ >> ovs_assert(mrs->type == JSON_OBJECT); >> >> + /* We need to resync the condition since the IDL cosumer can update >> monitor > > * consumer > >> + * conditions in compose_monitor_requests callback. */ >> + ovsdb_cs_db_sync_condition(&cs->data); >> + >> if (version > 1) { >> struct ovsdb_cs_db_table *table; >> HMAP_FOR_EACH (table, hmap_node, &db->tables) { >> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c >> index d8094458d..e8e9141d2 100644 >> --- a/lib/ovsdb-idl.c >> +++ b/lib/ovsdb-idl.c >> @@ -99,6 +99,9 @@ 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. */ >> + void (*idl_connect_notifier)(void *idl); /* callback to notify consumer > > Comments should start with a capital letter. > >> + * the idl connects to the >> remote >> + * db successfully. */ >> }; >> >> static struct ovsdb_cs_ops ovsdb_idl_cs_ops; >> @@ -304,6 +307,13 @@ ovsdb_idl_create_unconnected(const struct >> ovsdb_idl_class *class, >> return idl; >> } >> >> +void >> +ovsdb_idl_set_notifier(struct ovsdb_idl *idl, >> + void (*connect_notifier)(void *idl_)) >> +{ >> + idl->idl_connect_notifier = connect_notifier; >> +} >> + >> /* Changes the remote and creates a new session. >> * >> * If 'retry' is true, the connection to the remote will automatically retry >> @@ -853,8 +863,14 @@ ovsdb_idl_compose_monitor_request(const struct json >> *schema_json, void *idl_) >> json_array_create_1(monitor_request)); >> } >> } >> + >> ovsdb_cs_free_schema(schema); >> >> + if (idl->idl_connect_notifier) { >> + /* Notify consumers the idl connects to remote db successfully. */ >> + idl->idl_connect_notifier(idl); >> + } > > Hi, Lorenzo. thanks for working on this! >
Hi Lorenzo, Ilya, > I was thinking more about how we could make it simpler and more generic, so > the > application doesn't have to think much about which tables and columns are > present > and which are not. The IDL layer knows and checks the existence of them while > constructing the monitor request and the cs layer only deals with json and > doesn't > really know much about the database structure. At the same time only the cs > layer > can easily handle all the condition states across reconnections, which is > problematic if we want to filter them. > > Moving the condition handling to ovsdb-idl would be hard and practically mean > a lot > more callbacks from cs to idl, which is not good. But could we maybe store > the > latest requested conditions from the user in their original pre-JSON form in > the IDL > layer and then construct a proper JSON without the missing tables/columns and > call > ovsdb_cs_set_condition() here instead of making the callback to an > application to do > the same thing? It's logically should be exactly the same workflow, but we > handle > the filtering internally instead of asking the user to do that. > > What do you think? > > CC: Dumitru > > Note: ovsdb_idl_set_condition() will still need to call > ovsdb_cs_set_condition() > right away, since conditions can be changed not only during the monitor > request. > But if we integrate filtering into ovsdb_idl_condition_to_json(), then it > should > work just fine. And it's still OK to set them unfiltered before we know the > schema, since monitor request callback will filter them later. And > application > should not need to care about missing tables/columns at all while setting > conditions in this case. > That sounds like a nice improvement for the IDL users! And I think it would cover all the cases we currently want to cover. > Best regards, Ilya Maximets. > Regards, Dumitru > >> + >> return monitor_requests; >> } >> >> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h >> index f6060e324..dbf99e57d 100644 >> --- a/lib/ovsdb-idl.h >> +++ b/lib/ovsdb-idl.h >> @@ -502,6 +502,9 @@ void ovsdb_idl_cursor_next_eq(struct ovsdb_idl_cursor *); >> >> struct ovsdb_idl_row *ovsdb_idl_cursor_data(struct ovsdb_idl_cursor *); >> >> +void ovsdb_idl_set_notifier(struct ovsdb_idl *idl, >> + void (*connect_notifier)(void *idl_)); >> + >> #ifdef __cplusplus >> } >> #endif > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
