[{"id":3676393,"web_url":"http://patchwork.ozlabs.org/comment/3676393/","msgid":"<9e42afb4-558f-499c-a396-55ab1a39eaf0@ovn.org>","list_archive_url":null,"date":"2026-04-12T22:13:15","subject":"Re: [ovs-dev] [PATCH v3] ovsdb: Save user monitor condition request\n in ovsdb_idl_table struct.","submitter":{"id":76798,"url":"http://patchwork.ozlabs.org/api/people/76798/","name":"Ilya Maximets","email":"i.maximets@ovn.org"},"content":"On 4/8/26 10:23 PM, Lorenzo Bianconi wrote:\n> Store the user monitor condition request in pre-json format in\n> ovsdb_idl_table in order to use it in ovsdb_idl_compose_monitor_request\n> routine and create a monitor condition request based on the\n> tables/columns available in db-schema.\n> \n> Reported-at: https://issues.redhat.com/browse/FDP-3114\n> Signed-off-by: Lorenzo Bianconi \n> ---\n> Changes in v3:\n> - Fix seqno reporting when the filtered condition is empty.\n> \n> Changes in v2:\n> - Add missing unit-test.\n> - squash with patch 'ovsdb: Add ovsdb_cs_clear_condition routine to remove\n> stable ovsdb_cs_db_table entries.'.\n> - fix the error condition reported by Ilya.\n> - Remove unnecessary ovsdb_cs_db_sync_condition() in\n> ovsdb_cs_send_monitor_request().\n> - cosmetics.\n> ---\n> lib/ovsdb-cs.c | 51 ++++++++++++++----\n> lib/ovsdb-cs.h | 2 +\n> lib/ovsdb-idl-provider.h | 3 ++\n> lib/ovsdb-idl.c | 110 +++++++++++++++++++++++++++++++++++++--\n> tests/ovsdb-idl.at | 3 ++\n> tests/test-ovsdb.c | 8 +++\n> 6 files changed, 165 insertions(+), 12 deletions(-)\n> \n> diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c\n> index df33a835d..93f59492e 100644\n> --- a/lib/ovsdb-cs.c\n> +++ b/lib/ovsdb-cs.c\n> @@ -663,6 +663,18 @@ ovsdb_cs_wait(struct ovsdb_cs *cs)\n> \f\n> /* Network connection. */\n> \n> +bool\n> +ovsdb_cs_is_remote_changed(struct ovsdb_cs *cs, const char *remote)\n> +{\n> + if (cs &&\n> + ((remote != NULL) != (cs->remote != NULL) ||\n> + (remote && cs->remote && strcmp(remote, cs->remote)))) {\n> + return true;\n> + }\n> +\n> + return false;\n> +}\n> +\n> /* Changes the remote and creates a new session. Keeps existing connection\n> * if current remote is still valid.\n> *\n> @@ -671,9 +683,7 @@ ovsdb_cs_wait(struct ovsdb_cs *cs)\n> void\n> ovsdb_cs_set_remote(struct ovsdb_cs *cs, const char *remote, bool retry)\n> {\n> - if (cs\n> - && ((remote != NULL) != (cs->remote != NULL)\n> - || (remote && cs->remote && strcmp(remote, cs->remote)))) {\n> + if (ovsdb_cs_is_remote_changed(cs, remote)) {\n> struct jsonrpc *rpc = NULL;\n> \n> /* Close the old session, if any. */\n> @@ -912,17 +922,40 @@ ovsdb_cs_db_get_table(struct ovsdb_cs_db *db, const char *table)\n> return t;\n> }\n> \n> +static void\n> +ovsdb_cs_db_destroy_table(struct ovsdb_cs_db_table *table,\n> + struct ovsdb_cs_db *db)\n> +{\n> + json_destroy(table->ack_cond);\n> + json_destroy(table->req_cond);\n> + json_destroy(table->new_cond);\n> + hmap_remove(&db->tables, &table->hmap_node);\n> + free(table->name);\n> + free(table);\n> +}\n> +\n> +/* Destroy a given ovsdb_cs_db_table according to the table name. */\n> +void\n> +ovsdb_cs_clear_condition(struct ovsdb_cs *cs, const char *table)\n> +{\n> + uint32_t hash = hash_string(table, 0);\n> + struct ovsdb_cs_db *db = &cs->data;\n> +\n> + struct ovsdb_cs_db_table *t;\n> + HMAP_FOR_EACH_WITH_HASH (t, hmap_node, hash, &db->tables) {\n> + if (!strcmp(t->name, table)) {\n> + ovsdb_cs_db_destroy_table(t, db);\n\nWe should probbaly clear the last_id here, so the monitor reply clears\nthe data from idl for the table that now doesn't exist. But need to be\ncareful and not actually clear the idl->server_schema_received or we\nrisk to break the monitor_cond_since functionality whenever the client\nconnects to an older database.\n\n> + return;\n> + }\n> + }\n> +}\n> +\n> static void\n> ovsdb_cs_db_destroy_tables(struct ovsdb_cs_db *db)\n> {\n> struct ovsdb_cs_db_table *table;\n> HMAP_FOR_EACH_SAFE (table, hmap_node, &db->tables) {\n> - json_destroy(table->ack_cond);\n> - json_destroy(table->req_cond);\n> - json_destroy(table->new_cond);\n> - hmap_remove(&db->tables, &table->hmap_node);\n> - free(table->name);\n> - free(table);\n> + ovsdb_cs_db_destroy_table(table, db);\n> }\n> hmap_destroy(&db->tables);\n> }\n> diff --git a/lib/ovsdb-cs.h b/lib/ovsdb-cs.h\n> index bcc3dcd71..a46b806c0 100644\n> --- a/lib/ovsdb-cs.h\n> +++ b/lib/ovsdb-cs.h\n> @@ -122,6 +122,7 @@ void ovsdb_cs_run(struct ovsdb_cs *, struct ovs_list *events);\n> void ovsdb_cs_wait(struct ovsdb_cs *);\n> \n> /* Network connection. */\n> +bool ovsdb_cs_is_remote_changed(struct ovsdb_cs *, const char *remote);\n> void ovsdb_cs_set_remote(struct ovsdb_cs *, const char *remote, bool retry);\n> \n> void ovsdb_cs_enable_reconnect(struct ovsdb_cs *);\n> @@ -144,6 +145,7 @@ void ovsdb_cs_set_probe_interval(const struct ovsdb_cs *, int probe_interval);\n> unsigned int ovsdb_cs_set_condition(struct ovsdb_cs *, const char *table,\n> const struct json *condition);\n> unsigned int ovsdb_cs_get_condition_seqno(const struct ovsdb_cs *);\n> +void ovsdb_cs_clear_condition(struct ovsdb_cs *, const char *table);\n> \n> /* Database change awareness. */\n> void ovsdb_cs_set_db_change_aware(struct ovsdb_cs *, bool set_db_change_aware);\n> diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h\n> index 6cf32fb50..9244bbcd5 100644\n> --- a/lib/ovsdb-idl-provider.h\n> +++ b/lib/ovsdb-idl-provider.h\n> @@ -130,6 +130,9 @@ struct ovsdb_idl_table {\n> * or not. */\n> struct ovs_list indexes; /* Contains \"struct ovsdb_idl_index\"s */\n> struct ovs_list track_list; /* Tracked rows (ovsdb_idl_row.track_node). */\n> +\n> + struct ovsdb_idl_condition req_cond; /* User requested monitor\n> + * condition. */\n> };\n> \n> struct ovsdb_idl_class {\n> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c\n> index fe90deda7..6ad0eb36d 100644\n> --- a/lib/ovsdb-idl.c\n> +++ b/lib/ovsdb-idl.c\n> @@ -99,6 +99,7 @@ struct ovsdb_idl {\n> struct ovs_list rows_to_reparse; /* Stores rows that might need to be\n> * re-parsed due to insertion of a\n> * referenced row. */\n> + bool server_schema_received;\n\nNeed to add a comment here.\n\n> };\n> \n> static struct ovsdb_cs_ops ovsdb_idl_cs_ops;\n> @@ -205,6 +206,19 @@ static void ovsdb_idl_add_to_indexes(const struct ovsdb_idl_row *);\n> static void ovsdb_idl_remove_from_indexes(const struct ovsdb_idl_row *);\n> static int ovsdb_idl_try_commit_loop_txn(struct ovsdb_idl_loop *loop,\n> bool *may_need_wakeup);\n> +static void\n> +ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dest,\n> + const struct ovsdb_idl_condition *source);\n> +static void\n> +ovsdb_idl_create_req_condition(struct ovsdb_idl *,\n> + const struct ovsdb_idl_table_class *,\n> + const struct ovsdb_idl_condition *);\n> +static void ovsdb_idl_destroy_req_condition(struct ovsdb_idl_table *);\n> +static bool ovsdb_idl_condition_is_set(struct ovsdb_idl_condition *);\n> +static unsigned int\n> +ovsdb_idl_set_condition__(struct ovsdb_idl *,\n> + const struct ovsdb_idl_table_class *,\n> + const struct ovsdb_idl_condition *);\n\novsdb_idl_condition_clone, ovsdb_idl_create_req_condition and the\novsdb_idl_set_condition__ should not be on new lines, as these are\nprototypes and not implementations.\n\n> \n> static void add_tracked_change_for_references(struct ovsdb_idl_row *);\n> \n> @@ -266,6 +280,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class *class,\n> .txn = NULL,\n> .outstanding_txns = HMAP_INITIALIZER(&idl->outstanding_txns),\n> .verify_write_only = false,\n> + .server_schema_received = false,\n> .deleted_untracked_rows\n> = OVS_LIST_INITIALIZER(&idl->deleted_untracked_rows),\n> .rows_to_reparse\n> @@ -298,6 +313,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class *class,\n> = table->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0;\n> table->idl = idl;\n> table->in_server_schema = false;\n> + ovsdb_idl_condition_init(&table->req_cond);\n> shash_init(&table->schema_columns);\n> }\n> \n> @@ -311,6 +327,9 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class *class,\n> void\n> ovsdb_idl_set_remote(struct ovsdb_idl *idl, const char *remote, bool retry)\n> {\n> + if (ovsdb_cs_is_remote_changed(idl->cs, remote)) {\n> + idl->server_schema_received = false;\n> + }\n\nI don't think this is needed. And if it was needed, then we would probbalyclear\nwhile receiving the RECONNECT event instead. We're not clearing 'in_server_schema'\nfileds, so should not clear this one as well. If the reconnection happens we will\nbe seding a new monitor request that will get the schema, so everything should work\nfine without setting to false on reconnections or remote changes.\n\n> ovsdb_cs_set_remote(idl->cs, remote, retry);\n> }\n> \n> @@ -372,6 +391,7 @@ ovsdb_idl_destroy(struct ovsdb_idl *idl)\n> \n> ovsdb_idl_schema_columns_clear(&table->schema_columns);\n> shash_destroy(&table->schema_columns);\n> + ovsdb_idl_destroy_req_condition(table);\n> \n> hmap_destroy(&table->rows);\n> free(table->modes);\n> @@ -784,6 +804,7 @@ static struct json *\n> ovsdb_idl_compose_monitor_request(const struct json *schema_json, void *idl_)\n> {\n> struct ovsdb_idl *idl = idl_;\n> + idl->server_schema_received = true;\n\nShould add an empty line between these two, or set the value closer to the loop.\n\n> \n> struct shash *schema = ovsdb_cs_parse_schema(schema_json);\n> struct json *monitor_requests = json_object_create();\n> @@ -842,6 +863,7 @@ ovsdb_idl_compose_monitor_request(const struct json *schema_json, void *idl_)\n> idl->class_->database, table->class_->name);\n> json_destroy(columns);\n> table->in_server_schema = false;\n> + ovsdb_cs_clear_condition(idl->cs, table->class_->name);\n> continue;\n> } else if (schema && table_schema) {\n> table->in_server_schema = true;\n> @@ -852,6 +874,14 @@ ovsdb_idl_compose_monitor_request(const struct json *schema_json, void *idl_)\n> json_object_put(monitor_requests, tc->name,\n> json_array_create_1(monitor_request));\n> }\n> +\n> + if (!table->in_server_schema) {\n> + ovsdb_cs_clear_condition(idl->cs, table->class_->name);\n> + } else if (ovsdb_idl_condition_is_set(&table->req_cond)) {\n> + /* Update the monitor condition request according to the\n> + * db schema. */\n> + ovsdb_idl_set_condition__(idl, tc, &table->req_cond);\n\nHmm. The ovsdb_idl_set_condition__() function doesn't filter, it uses the old\nfiltered condition stored in the table->req_cond that was created before we\nreceived the new schema. So, we can still set conditions here for tables that\ndo not exist in on the server.\n\n> + }\n> }\n> ovsdb_cs_free_schema(schema);\n> \n> @@ -1156,6 +1186,45 @@ ovsdb_idl_condition_add_clause__(struct ovsdb_idl_condition *condition,\n> hmap_insert(&condition->clauses, &clause->hmap_node, hash);\n> }\n> \n> +static void\n> +ovsdb_idl_destroy_req_condition(struct ovsdb_idl_table *table)\n> +{\n> + ovsdb_idl_condition_destroy(&table->req_cond);\n> +}\n> +\n> +static bool\n> +ovsdb_idl_condition_is_set(struct ovsdb_idl_condition *condition)\n> +{\n> + return !hmap_is_empty(&condition->clauses) || condition->is_true;\n> +}\n> +\n> +static void\n> +ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dest,\n> + const struct ovsdb_idl_condition *source)\n> +{\n> + ovsdb_idl_condition_init(dest);\n> +\n> + struct ovsdb_idl_clause *clause;\n> + HMAP_FOR_EACH (clause, hmap_node, &source->clauses) {\n> + uint32_t hash = ovsdb_idl_clause_hash(clause);\n\nCan use a hash of the current hmap node.\n\n> + ovsdb_idl_condition_add_clause__(dest, clause, hash);\n> + }\n> + dest->is_true = source->is_true;\n> +}\n> +\n> +static void\n> +ovsdb_idl_create_req_condition(struct ovsdb_idl *idl,\n> + const struct ovsdb_idl_table_class *tc,\n> + const struct ovsdb_idl_condition *condition)\n> +{\n> + struct ovsdb_idl_table *table = shash_find_data(&idl->table_by_name,\n> + tc->name);\n> + if (table) {\n> + ovsdb_idl_destroy_req_condition(table);\n> + ovsdb_idl_condition_clone(&table->req_cond, condition);\n> + }\n> +}\n> +\n> /* Adds a clause to the condition for replicating the table with class 'tc' in\n> * 'idl'.\n> *\n> @@ -1234,6 +1303,17 @@ ovsdb_idl_condition_to_json(const struct ovsdb_idl_condition *cnd)\n> return json_array_create(clauses, n);\n> }\n> \n> +static unsigned int\n> +ovsdb_idl_set_condition__(struct ovsdb_idl *idl,\n> + const struct ovsdb_idl_table_class *tc,\n> + const struct ovsdb_idl_condition *condition)\n> +{\n> + struct json *cond_json = ovsdb_idl_condition_to_json(condition);\n> + unsigned int seqno = ovsdb_cs_set_condition(idl->cs, tc->name, cond_json);\n> + json_destroy(cond_json);\n> + return seqno;\n> +}\n> +\n> /* Sets the replication condition for 'tc' in 'idl' to 'condition' and\n> * arranges to send the new condition to the database server.\n> *\n> @@ -1245,9 +1325,33 @@ ovsdb_idl_set_condition(struct ovsdb_idl *idl,\n> const struct ovsdb_idl_table_class *tc,\n> const struct ovsdb_idl_condition *condition)\n> {\n> - struct json *cond_json = ovsdb_idl_condition_to_json(condition);\n> - unsigned int seqno = ovsdb_cs_set_condition(idl->cs, tc->name, cond_json);\n> - json_destroy(cond_json);\n> + struct ovsdb_idl_condition filter_cond =\n> + OVSDB_IDL_CONDITION_INIT(&filter_cond);\n> +\n> + if (idl->server_schema_received && hmap_count(&condition->clauses)) {\n> + struct ovsdb_idl_clause *clause;\n> + HMAP_FOR_EACH (clause, hmap_node, &condition->clauses) {\n> + struct ovsdb_idl_table *t =\n> + ovsdb_idl_table_from_column(idl, clause->column);\n\nThis shoudld be done outside of the loop. We can get the table directly\nfrom the table class - ovsdb_idl_table_from_class().\n\n> + if (!t || !t->in_server_schema) {\n> + return ovsdb_idl_get_condition_seqno(idl);\n\nWe can't just return here, we still need to save these conditions, otherwise\nwe will not send them on reconnection when the server schema changes.\n\n> + }\n> +\n> + if (ovsdb_idl_server_has_column(idl, clause->column)) {\n> + uint32_t hash = ovsdb_idl_clause_hash(clause);\n> + ovsdb_idl_condition_add_clause__(&filter_cond, clause, hash);\n> + }\n> + }\n> + condition = &filter_cond;\n> + }\n> +\n> + unsigned int seqno = ovsdb_idl_set_condition__(idl, tc, condition);\n> + ovsdb_idl_create_req_condition(idl, tc, &filter_cond);\n\nWe should be saving the original conditions, not the filtered ones, because\nwe need to filter every time we compose a new monitor request.\n\n> +\n> + if (hmap_count(&filter_cond.clauses)) {\n> + ovsdb_idl_condition_destroy(&filter_cond);\n> + }\n> +\n> return seqno;\n> }\n> \n> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at\n> index 728d761d4..194afef13 100644\n> --- a/tests/ovsdb-idl.at\n> +++ b/tests/ovsdb-idl.at\n> @@ -2722,6 +2722,9 @@ unix:socket2 remote has col id in table simple7, type: string\n> --- remote unix:socket2 done ---\n> ], [stderr])\n> \n> +# Check we do not have any errors related to conditional monitoring.\n> +AT_CHECK([grep -q \"received error, error={\\\"details\\\":\\\"no table named link2\\\",\\\"error\\\":\\\"syntax error\\\"}\" stderr], [1])\n> +\n> OVSDB_SERVER_SHUTDOWN\n> AT_CLEANUP\n\nI think we need way more tests for this functionality, including a case where\nwe connect to a server with an older schema that doesn't have some tables and\nsome columns in the tables that exist, then re-connect to a server with the new\nschema that has all, then re-connect back to one with the older schema. The\nclient sets conditions for all the tables before connecting to the first server\nand doesn't change them afterwards. We need to make sure that all the conditions\nare properly filtered every time the cilent re-connects without touching the\nconditions. Ant another test where client sets conditions for tables and\ncolumns that exist and for those that do not exist after the succesful connection\nto make sure the filtering works outside of the monitor request. Also maybe\nsome tests to make sure the monitor-cond-since is respected anf the last-id is\nproperly cleared when it needs to be and not cleared when it doesn't need to\nbe cleared.\n\nTHe commit message also could use a larger description of the problem and what\nthis change is trying to achieve.\n\nBest regards, Ilya Maximets.\n\n> \n> diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c\n> index 95290ae78..037a78743 100644\n> --- a/tests/test-ovsdb.c\n> +++ b/tests/test-ovsdb.c\n> @@ -3596,6 +3596,12 @@ do_idl_table_column_check(struct ovs_cmdl_context *ctx)\n> ovsdb_idl_set_remote(idl, ctx->argv[r], true);\n> ovsdb_idl_force_reconnect(idl);\n> \n> + struct ovsdb_idl_condition cond_link2 =\n> + OVSDB_IDL_CONDITION_INIT(&cond_link2);\n> + idltest_link2_add_clause_i(&cond_link2, OVSDB_F_EQ, 1);\n> +\n> + idltest_link2_set_condition(idl, &cond_link2);\n> +\n> /* Wait for update. */\n> for (;;) {\n> ovsdb_idl_run(idl);\n> @@ -3661,6 +3667,8 @@ do_idl_table_column_check(struct ovs_cmdl_context *ctx)\n> type ? \", type: \" : \"\", type ? type_s : \"\");\n> free(type_s);\n> \n> + ovsdb_idl_condition_destroy(&cond_link2);\n> +\n> printf(\"--- remote %s done ---\\n\", ctx->argv[r]);\n> }\n>","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","ovs-dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@legolas.ozlabs.org","ovs-dev@lists.linuxfoundation.org"],"Authentication-Results":["legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org\n (client-ip=140.211.166.138; helo=smtp1.osuosl.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org)","smtp1.osuosl.org;\n dmarc=none (p=none dis=none) header.from=ovn.org"],"Received":["from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fv4Yj15WJz1yGC\n\tfor ; Mon, 13 Apr 2026 08:13:26 +1000 (AEST)","from localhost (localhost [127.0.0.1])\n\tby smtp1.osuosl.org (Postfix) with ESMTP id E9CFE83E93;\n\tSun, 12 Apr 2026 22:13:23 +0000 (UTC)","from smtp1.osuosl.org ([127.0.0.1])\n by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id 9KCRkyQoEfm7; Sun, 12 Apr 2026 22:13:22 +0000 (UTC)","from lists.linuxfoundation.org (lf-lists.osuosl.org\n [IPv6:2605:bc80:3010:104::8cd3:938])\n\tby smtp1.osuosl.org (Postfix) with ESMTPS id 70F8783EED;\n\tSun, 12 Apr 2026 22:13:22 +0000 (UTC)","from lf-lists.osuosl.org (localhost [127.0.0.1])\n\tby lists.linuxfoundation.org (Postfix) with ESMTP id 49A26C054A;\n\tSun, 12 Apr 2026 22:13:22 +0000 (UTC)","from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138])\n by lists.linuxfoundation.org (Postfix) with ESMTP id 58CEFC0549\n for ; Sun, 12 Apr 2026 22:13:21 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n by smtp1.osuosl.org (Postfix) with ESMTP id 38A9383EED\n for ; Sun, 12 Apr 2026 22:13:21 +0000 (UTC)","from smtp1.osuosl.org ([127.0.0.1])\n by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id Cot0KjJ-qOMt for ;\n Sun, 12 Apr 2026 22:13:20 +0000 (UTC)","from mail-wr1-f65.google.com (mail-wr1-f65.google.com\n [209.85.221.65])\n by smtp1.osuosl.org (Postfix) with ESMTPS id 6508483E93\n for ; Sun, 12 Apr 2026 22:13:19 +0000 (UTC)","by mail-wr1-f65.google.com with SMTP id\n ffacd0b85a97d-43d77f6092eso241780f8f.2\n for ; Sun, 12 Apr 2026 15:13:19 -0700 (PDT)","from [192.168.88.241] (37-48-40-237.nat.epc.tmcz.cz. [37.48.40.237])\n by smtp.gmail.com with ESMTPSA id\n ffacd0b85a97d-43d7b543057sm233545f8f.6.2026.04.12.15.13.15\n (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n Sun, 12 Apr 2026 15:13:16 -0700 (PDT)"],"X-Virus-Scanned":["amavis at osuosl.org","amavis at osuosl.org"],"X-Comment":"SPF check N/A for local connections -\n client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver= ","DKIM-Filter":["OpenDKIM Filter v2.11.0 smtp1.osuosl.org 70F8783EED","OpenDKIM Filter v2.11.0 smtp1.osuosl.org 6508483E93"],"Received-SPF":"Pass (mailfrom) identity=mailfrom; client-ip=209.85.221.65;\n helo=mail-wr1-f65.google.com; envelope-from=i.maximets.ovn@gmail.com;\n receiver=","DMARC-Filter":"OpenDMARC Filter v1.4.2 smtp1.osuosl.org 6508483E93","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20251104; t=1776031997; x=1776636797;\n h=content-transfer-encoding:in-reply-to:autocrypt:from\n :content-language:references:to:subject:cc:user-agent:mime-version\n :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date\n :message-id:reply-to;\n bh=G97JlYCg+tMj7BmvWjWDdNzbs7jDkE8GkaHnPPjzHkA=;\n b=n9lv8u6EeqwNOnQMNQaCG5BvSQYreEdGDBxXp8zafqr+JX579EfpQyBVuT1rETfroJ\n RZ2tvvkA7eLB+2WgcU6ocdxuA1cY3amSC+5RJ85ynfNl+4KJWKJSmRXC6FBnhltBPR50\n 0GlG+XR1ht4B5MCSzEeJfe9GQCFVMLUmunzquvlr1uiqLgujSf9AJNL67pWU2RNm1W0p\n kGvrAKmvGhJAX4P6HVGF+0dl6HePU42s6pGU0vaF8G5c9PFZvOIrSf/ykOTVJfym2Jch\n djFBKHWA4veYlhGz1Ck6kS4ShLByoslZ2Hxnu/ysJi/VhMMWhJhq6vOR/lxkIfnXjPD9\n upnA==","X-Forwarded-Encrypted":"i=1;\n AFNElJ8rsuW4TTTCBvxzg6NFMqvU/CL2eU+5rjep/OkB0YBBwlvK5jhvszoaxvs/NsbW6ygk9LF0bZNl@openvswitch.org","X-Gm-Message-State":"AOJu0YwIWJ6bK08wglk9t/97dFBMqb3iaRjMsQbrckQ3lx6z3RXTjPV+\n mBNtv2Y9RJJNSWZWW9BLH9xz+rQxOcUvxONeRRClcofrCCd+RPZqL5kk","X-Gm-Gg":"AeBDietWWChQvN0ivFgRqaHbpB1CTjayanMUQgYpSttwFusI1NE9dFQt0OcvVncxzI7\n BXmpCRj+9nmkxzWREvOADwuZ6GP+Tsy3Llf+LKjpxgts+gTYdfyQmVQ5ERhBNii8NYI2D2aEvDf\n v0b1w2Hs5OP3bXhKbK1MhicL56cfq6JSefkV+L9gMs9cVIq8Wx9DAYiS+OaM0ACRHO0Mle81BFf\n 9DB5vo6yEk8bbZAusz1kmOQ0ssYaPb1rsXa++zN7vmaGWyaVq/BHc9/lVt9tMuXhnwIx4sFRUql\n y+QsSbKwdLvmTkQfUmD7kEOR1s34SJpa0PVysK4cnEnf5sGsweWGYxwv8BvYfmhF5S8T6HuhHD6\n T1jtcuL2gLwfI8verZY/kH2Zbr3OuYTwJABpqsuZEesqOzZyOwWnHKn0PSm+KVv/Ata3xDCkM4T\n IoPzrY+3CpdaLJEjj1kBLALrYlMu/JGR1yU7MqzwqFKfE5d1RXwIkGR9Q=","X-Received":"by 2002:a05:6000:1789:b0:43d:7748:9282 with SMTP id\n ffacd0b85a97d-43d774893b1mr3451631f8f.0.1776031996748;\n Sun, 12 Apr 2026 15:13:16 -0700 (PDT)","Message-ID":"<9e42afb4-558f-499c-a396-55ab1a39eaf0@ovn.org>","Date":"Mon, 13 Apr 2026 00:13:15 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Cc":"i.maximets@ovn.org, dceara@redhat.com","To":"Lorenzo Bianconi , ovs-dev@openvswitch.org","References":"\n <195d108bcfc9b2337eeb0b7f5fa789804e7c86d0.1775679683.git.lorenzo.bianconi@redhat.com>","Content-Language":"en-US","From":"Ilya Maximets ","Autocrypt":"addr=i.maximets@ovn.org; keydata=\n xsFNBF77bOMBEADVZQ4iajIECGfH3hpQMQjhIQlyKX4hIB3OccKl5XvB/JqVPJWuZQRuqNQG\n /B70MP6km95KnWLZ4H1/5YOJK2l7VN7nO+tyF+I+srcKq8Ai6S3vyiP9zPCrZkYvhqChNOCF\n pNqdWBEmTvLZeVPmfdrjmzCLXVLi5De9HpIZQFg/Ztgj1AZENNQjYjtDdObMHuJQNJ6ubPIW\n cvOOn4WBr8NsP4a2OuHSTdVyAJwcDhu+WrS/Bj3KlQXIdPv3Zm5x9u/56NmCn1tSkLrEgi0i\n /nJNeH5QhPdYGtNzPixKgPmCKz54/LDxU61AmBvyRve+U80ukS+5vWk8zvnCGvL0ms7kx5sA\n tETpbKEV3d7CB3sQEym8B8gl0Ux9KzGp5lbhxxO995KWzZWWokVUcevGBKsAx4a/C0wTVOpP\n FbQsq6xEpTKBZwlCpxyJi3/PbZQJ95T8Uw6tlJkPmNx8CasiqNy2872gD1nN/WOP8m+cIQNu\n o6NOiz6VzNcowhEihE8Nkw9V+zfCxC8SzSBuYCiVX6FpgKzY/Tx+v2uO4f/8FoZj2trzXdLk\n BaIiyqnE0mtmTQE8jRa29qdh+s5DNArYAchJdeKuLQYnxy+9U1SMMzJoNUX5uRy6/3KrMoC/\n 7zhn44x77gSoe7XVM6mr/mK+ViVB7v9JfqlZuiHDkJnS3yxKPwARAQABzSJJbHlhIE1heGlt\n ZXRzIDxpLm1heGltZXRzQG92bi5vcmc+wsGUBBMBCAA+AhsDBQsJCAcCBhUKCQgLAgQWAgMB\n Ah4BAheAFiEEh+ma1RKWrHCY821auffsd8gpv5YFAmfB9JAFCQyI7q0ACgkQuffsd8gpv5YQ\n og/8DXt1UOznvjdXRHVydbU6Ws+1iUrxlwnFH4WckoFgH4jAabt25yTa1Z4YX8Vz0mbRhTPX\n M/j1uORyObLem3of4YCd4ymh7nSu++KdKnNsZVHxMcoiic9ILPIaWYa8kTvyIDT2AEVfn9M+\n vskM0yDbKa6TAHgr/0jCxbS+mvN0ZzDuR/LHTgy3e58097SWJohj0h3Dpu+XfuNiZCLCZ1/G\n AbBCPMw+r7baH/0evkX33RCBZwvh6tKu+rCatVGk72qRYNLCwF0YcGuNBsJiN9Aa/7ipkrA7\n Xp7YvY3Y1OrKnQfdjp3mSXmknqPtwqnWzXvdfkWkZKShu0xSk+AjdFWCV3NOzQaH3CJ67NXm\n aPjJCIykoTOoQ7eEP6+m3WcgpRVkn9bGK9ng03MLSymTPmdINhC5pjOqBP7hLqYi89GN0MIT\n Ly2zD4m/8T8wPV9yo7GRk4kkwD0yN05PV2IzJECdOXSSStsf5JWObTwzhKyXJxQE+Kb67Wwa\n LYJgltFjpByF5GEO4Xe7iYTjwEoSSOfaR0kokUVM9pxIkZlzG1mwiytPadBt+VcmPQWcO5pi\n WxUI7biRYt4aLriuKeRpk94ai9+52KAk7Lz3KUWoyRwdZINqkI/aDZL6meWmcrOJWCUMW73e\n 4cMqK5XFnGqolhK4RQu+8IHkSXtmWui7LUeEvO/OwU0EXvts4wEQANCXyDOic0j2QKeyj/ga\n OD1oKl44JQfOgcyLVDZGYyEnyl6b/tV1mNb57y/YQYr33fwMS1hMj9eqY6tlMTNz+ciGZZWV\n YkPNHA+aFuPTzCLrapLiz829M5LctB2448bsgxFq0TPrr5KYx6AkuWzOVq/X5wYEM6djbWLc\n VWgJ3o0QBOI4/uB89xTf7mgcIcbwEf6yb/86Cs+jaHcUtJcLsVuzW5RVMVf9F+Sf/b98Lzrr\n 2/mIB7clOXZJSgtV79Alxym4H0cEZabwiXnigjjsLsp4ojhGgakgCwftLkhAnQT3oBLH/6ix\n 87ahawG3qlyIB8ZZKHsvTxbWte6c6xE5dmmLIDN44SajAdmjt1i7SbAwFIFjuFJGpsnfdQv1\n OiIVzJ44kdRJG8kQWPPua/k+AtwJt/gjCxv5p8sKVXTNtIP/sd3EMs2xwbF8McebLE9JCDQ1\n RXVHceAmPWVCq3WrFuX9dSlgf3RWTqNiWZC0a8Hn6fNDp26TzLbdo9mnxbU4I/3BbcAJZI9p\n 9ELaE9rw3LU8esKqRIfaZqPtrdm1C+e5gZa2gkmEzG+WEsS0MKtJyOFnuglGl1ZBxR1uFvbU\n VXhewCNoviXxkkPk/DanIgYB1nUtkPC+BHkJJYCyf9Kfl33s/bai34aaxkGXqpKv+CInARg3\n fCikcHzYYWKaXS6HABEBAAHCwXwEGAEIACYCGwwWIQSH6ZrVEpascJjzbVq59+x3yCm/lgUC\n Z8H0qQUJDIjuxgAKCRC59+x3yCm/loAdD/wJCOhPp9711J18B9c4f+eNAk5vrC9Cj3RyOusH\n Hebb9HtSFm155Zz3xiizw70MSyOVikjbTocFAJo5VhkyuN0QJIP678SWzriwym+EG0B5P97h\n FSLBlRsTi4KD8f1Ll3OT03lD3o/5Qt37zFgD4mCD6OxAShPxhI3gkVHBuA0GxF01MadJEjMu\n jWgZoj75rCLG9sC6L4r28GEGqUFlTKjseYehLw0s3iR53LxS7HfJVHcFBX3rUcKFJBhuO6Ha\n /GggRvTbn3PXxR5UIgiBMjUlqxzYH4fe7pYR7z1m4nQcaFWW+JhY/BYHJyMGLfnqTn1FsIwP\n dbhEjYbFnJE9Vzvf+RJcRQVyLDn/TfWbETf0bLGHeF2GUPvNXYEu7oKddvnUvJK5U/BuwQXy\n TRFbae4Ie96QMcPBL9ZLX8M2K4XUydZBeHw+9lP1J6NJrQiX7MzexpkKNy4ukDzPrRE/ruui\n yWOKeCw9bCZX4a/uFw77TZMEq3upjeq21oi6NMTwvvWWMYuEKNi0340yZRrBdcDhbXkl9x/o\n skB2IbnvSB8iikbPng1ihCTXpA2yxioUQ96Akb+WEGopPWzlxTTK+T03G2ljOtspjZXKuywV\n Wu/eHyqHMyTu8UVcMRR44ki8wam0LMs+fH4dRxw5ck69AkV+JsYQVfI7tdOu7+r465LUfg==","In-Reply-To":"\n <195d108bcfc9b2337eeb0b7f5fa789804e7c86d0.1775679683.git.lorenzo.bianconi@redhat.com>","Subject":"Re: [ovs-dev] [PATCH v3] ovsdb: Save user monitor condition request\n in ovsdb_idl_table struct.","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.30","Precedence":"list","List-Id":"","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"ovs-dev-bounces@openvswitch.org","Sender":"\"dev\" "}},{"id":3678240,"web_url":"http://patchwork.ozlabs.org/comment/3678240/","msgid":"","list_archive_url":null,"date":"2026-04-16T15:13:29","subject":"Re: [ovs-dev] [PATCH v3] ovsdb: Save user monitor condition request\n in ovsdb_idl_table struct.","submitter":{"id":73083,"url":"http://patchwork.ozlabs.org/api/people/73083/","name":"Lorenzo Bianconi","email":"lorenzo.bianconi@redhat.com"},"content":"> > Store the user monitor condition request in pre-json format in\n> > ovsdb_idl_table in order to use it in ovsdb_idl_compose_monitor_request\n> > routine and create a monitor condition request based on the\n> > tables/columns available in db-schema.\n> > \n> > Reported-at: https://issues.redhat.com/browse/FDP-3114\n> > Signed-off-by: Lorenzo Bianconi \n> > ---\n> > Changes in v3:\n> > - Fix seqno reporting when the filtered condition is empty.\n\nHi Ilya,\n\nthx for the review.\n\n> > \n> > Changes in v2:\n> > - Add missing unit-test.\n> > - squash with patch 'ovsdb: Add ovsdb_cs_clear_condition routine to remove\n> > stable ovsdb_cs_db_table entries.'.\n> > - fix the error condition reported by Ilya.\n> > - Remove unnecessary ovsdb_cs_db_sync_condition() in\n> > ovsdb_cs_send_monitor_request().\n> > - cosmetics.\n> > ---\n> > lib/ovsdb-cs.c | 51 ++++++++++++++----\n> > lib/ovsdb-cs.h | 2 +\n> > lib/ovsdb-idl-provider.h | 3 ++\n> > lib/ovsdb-idl.c | 110 +++++++++++++++++++++++++++++++++++++--\n> > tests/ovsdb-idl.at | 3 ++\n> > tests/test-ovsdb.c | 8 +++\n> > 6 files changed, 165 insertions(+), 12 deletions(-)\n> > \n> > diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c\n> > index df33a835d..93f59492e 100644\n> > --- a/lib/ovsdb-cs.c\n> > +++ b/lib/ovsdb-cs.c\n> > @@ -663,6 +663,18 @@ ovsdb_cs_wait(struct ovsdb_cs *cs)\n> > \f\n> > /* Network connection. */\n> > \n> > +bool\n> > +ovsdb_cs_is_remote_changed(struct ovsdb_cs *cs, const char *remote)\n> > +{\n> > + if (cs &&\n> > + ((remote != NULL) != (cs->remote != NULL) ||\n> > + (remote && cs->remote && strcmp(remote, cs->remote)))) {\n> > + return true;\n> > + }\n> > +\n> > + return false;\n> > +}\n> > +\n> > /* Changes the remote and creates a new session. Keeps existing connection\n> > * if current remote is still valid.\n> > *\n> > @@ -671,9 +683,7 @@ ovsdb_cs_wait(struct ovsdb_cs *cs)\n> > void\n> > ovsdb_cs_set_remote(struct ovsdb_cs *cs, const char *remote, bool retry)\n> > {\n> > - if (cs\n> > - && ((remote != NULL) != (cs->remote != NULL)\n> > - || (remote && cs->remote && strcmp(remote, cs->remote)))) {\n> > + if (ovsdb_cs_is_remote_changed(cs, remote)) {\n> > struct jsonrpc *rpc = NULL;\n> > \n> > /* Close the old session, if any. */\n> > @@ -912,17 +922,40 @@ ovsdb_cs_db_get_table(struct ovsdb_cs_db *db, const char *table)\n> > return t;\n> > }\n> > \n> > +static void\n> > +ovsdb_cs_db_destroy_table(struct ovsdb_cs_db_table *table,\n> > + struct ovsdb_cs_db *db)\n> > +{\n> > + json_destroy(table->ack_cond);\n> > + json_destroy(table->req_cond);\n> > + json_destroy(table->new_cond);\n> > + hmap_remove(&db->tables, &table->hmap_node);\n> > + free(table->name);\n> > + free(table);\n> > +}\n> > +\n> > +/* Destroy a given ovsdb_cs_db_table according to the table name. */\n> > +void\n> > +ovsdb_cs_clear_condition(struct ovsdb_cs *cs, const char *table)\n> > +{\n> > + uint32_t hash = hash_string(table, 0);\n> > + struct ovsdb_cs_db *db = &cs->data;\n> > +\n> > + struct ovsdb_cs_db_table *t;\n> > + HMAP_FOR_EACH_WITH_HASH (t, hmap_node, hash, &db->tables) {\n> > + if (!strcmp(t->name, table)) {\n> > + ovsdb_cs_db_destroy_table(t, db);\n> \n> We should probbaly clear the last_id here, so the monitor reply clears\n> the data from idl for the table that now doesn't exist. But need to be\n> careful and not actually clear the idl->server_schema_received or we\n> risk to break the monitor_cond_since functionality whenever the client\n> connects to an older database.\n\nack, I will fix it in v4\n\n> \n> > + return;\n> > + }\n> > + }\n> > +}\n> > +\n> > static void\n> > ovsdb_cs_db_destroy_tables(struct ovsdb_cs_db *db)\n> > {\n> > struct ovsdb_cs_db_table *table;\n> > HMAP_FOR_EACH_SAFE (table, hmap_node, &db->tables) {\n> > - json_destroy(table->ack_cond);\n> > - json_destroy(table->req_cond);\n> > - json_destroy(table->new_cond);\n> > - hmap_remove(&db->tables, &table->hmap_node);\n> > - free(table->name);\n> > - free(table);\n> > + ovsdb_cs_db_destroy_table(table, db);\n> > }\n> > hmap_destroy(&db->tables);\n> > }\n> > diff --git a/lib/ovsdb-cs.h b/lib/ovsdb-cs.h\n> > index bcc3dcd71..a46b806c0 100644\n> > --- a/lib/ovsdb-cs.h\n> > +++ b/lib/ovsdb-cs.h\n> > @@ -122,6 +122,7 @@ void ovsdb_cs_run(struct ovsdb_cs *, struct ovs_list *events);\n> > void ovsdb_cs_wait(struct ovsdb_cs *);\n> > \n> > /* Network connection. */\n> > +bool ovsdb_cs_is_remote_changed(struct ovsdb_cs *, const char *remote);\n> > void ovsdb_cs_set_remote(struct ovsdb_cs *, const char *remote, bool retry);\n> > \n> > void ovsdb_cs_enable_reconnect(struct ovsdb_cs *);\n> > @@ -144,6 +145,7 @@ void ovsdb_cs_set_probe_interval(const struct ovsdb_cs *, int probe_interval);\n> > unsigned int ovsdb_cs_set_condition(struct ovsdb_cs *, const char *table,\n> > const struct json *condition);\n> > unsigned int ovsdb_cs_get_condition_seqno(const struct ovsdb_cs *);\n> > +void ovsdb_cs_clear_condition(struct ovsdb_cs *, const char *table);\n> > \n> > /* Database change awareness. */\n> > void ovsdb_cs_set_db_change_aware(struct ovsdb_cs *, bool set_db_change_aware);\n> > diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h\n> > index 6cf32fb50..9244bbcd5 100644\n> > --- a/lib/ovsdb-idl-provider.h\n> > +++ b/lib/ovsdb-idl-provider.h\n> > @@ -130,6 +130,9 @@ struct ovsdb_idl_table {\n> > * or not. */\n> > struct ovs_list indexes; /* Contains \"struct ovsdb_idl_index\"s */\n> > struct ovs_list track_list; /* Tracked rows (ovsdb_idl_row.track_node). */\n> > +\n> > + struct ovsdb_idl_condition req_cond; /* User requested monitor\n> > + * condition. */\n> > };\n> > \n> > struct ovsdb_idl_class {\n> > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c\n> > index fe90deda7..6ad0eb36d 100644\n> > --- a/lib/ovsdb-idl.c\n> > +++ b/lib/ovsdb-idl.c\n> > @@ -99,6 +99,7 @@ struct ovsdb_idl {\n> > struct ovs_list rows_to_reparse; /* Stores rows that might need to be\n> > * re-parsed due to insertion of a\n> > * referenced row. */\n> > + bool server_schema_received;\n> \n> Need to add a comment here.\n\nack, I will fix it in v4\n\n> \n> > };\n> > \n> > static struct ovsdb_cs_ops ovsdb_idl_cs_ops;\n> > @@ -205,6 +206,19 @@ static void ovsdb_idl_add_to_indexes(const struct ovsdb_idl_row *);\n> > static void ovsdb_idl_remove_from_indexes(const struct ovsdb_idl_row *);\n> > static int ovsdb_idl_try_commit_loop_txn(struct ovsdb_idl_loop *loop,\n> > bool *may_need_wakeup);\n> > +static void\n> > +ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dest,\n> > + const struct ovsdb_idl_condition *source);\n> > +static void\n> > +ovsdb_idl_create_req_condition(struct ovsdb_idl *,\n> > + const struct ovsdb_idl_table_class *,\n> > + const struct ovsdb_idl_condition *);\n> > +static void ovsdb_idl_destroy_req_condition(struct ovsdb_idl_table *);\n> > +static bool ovsdb_idl_condition_is_set(struct ovsdb_idl_condition *);\n> > +static unsigned int\n> > +ovsdb_idl_set_condition__(struct ovsdb_idl *,\n> > + const struct ovsdb_idl_table_class *,\n> > + const struct ovsdb_idl_condition *);\n> \n> ovsdb_idl_condition_clone, ovsdb_idl_create_req_condition and the\n> ovsdb_idl_set_condition__ should not be on new lines, as these are\n> prototypes and not implementations.\n\nack, I will fix it in v4\n\n> \n> > \n> > static void add_tracked_change_for_references(struct ovsdb_idl_row *);\n> > \n> > @@ -266,6 +280,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class *class,\n> > .txn = NULL,\n> > .outstanding_txns = HMAP_INITIALIZER(&idl->outstanding_txns),\n> > .verify_write_only = false,\n> > + .server_schema_received = false,\n> > .deleted_untracked_rows\n> > = OVS_LIST_INITIALIZER(&idl->deleted_untracked_rows),\n> > .rows_to_reparse\n> > @@ -298,6 +313,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class *class,\n> > = table->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0;\n> > table->idl = idl;\n> > table->in_server_schema = false;\n> > + ovsdb_idl_condition_init(&table->req_cond);\n> > shash_init(&table->schema_columns);\n> > }\n> > \n> > @@ -311,6 +327,9 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class *class,\n> > void\n> > ovsdb_idl_set_remote(struct ovsdb_idl *idl, const char *remote, bool retry)\n> > {\n> > + if (ovsdb_cs_is_remote_changed(idl->cs, remote)) {\n> > + idl->server_schema_received = false;\n> > + }\n> \n> I don't think this is needed. And if it was needed, then we would probbalyclear\n> while receiving the RECONNECT event instead. We're not clearing 'in_server_schema'\n> fileds, so should not clear this one as well. If the reconnection happens we will\n> be seding a new monitor request that will get the schema, so everything should work\n> fine without setting to false on reconnections or remote changes.\n\nack, I will fix it in v4\n\n> \n> > ovsdb_cs_set_remote(idl->cs, remote, retry);\n> > }\n> > \n> > @@ -372,6 +391,7 @@ ovsdb_idl_destroy(struct ovsdb_idl *idl)\n> > \n> > ovsdb_idl_schema_columns_clear(&table->schema_columns);\n> > shash_destroy(&table->schema_columns);\n> > + ovsdb_idl_destroy_req_condition(table);\n> > \n> > hmap_destroy(&table->rows);\n> > free(table->modes);\n> > @@ -784,6 +804,7 @@ static struct json *\n> > ovsdb_idl_compose_monitor_request(const struct json *schema_json, void *idl_)\n> > {\n> > struct ovsdb_idl *idl = idl_;\n> > + idl->server_schema_received = true;\n> \n> Should add an empty line between these two, or set the value closer to the loop.\n\nack, I will fix it in v4\n\n> \n> > \n> > struct shash *schema = ovsdb_cs_parse_schema(schema_json);\n> > struct json *monitor_requests = json_object_create();\n> > @@ -842,6 +863,7 @@ ovsdb_idl_compose_monitor_request(const struct json *schema_json, void *idl_)\n> > idl->class_->database, table->class_->name);\n> > json_destroy(columns);\n> > table->in_server_schema = false;\n> > + ovsdb_cs_clear_condition(idl->cs, table->class_->name);\n> > continue;\n> > } else if (schema && table_schema) {\n> > table->in_server_schema = true;\n> > @@ -852,6 +874,14 @@ ovsdb_idl_compose_monitor_request(const struct json *schema_json, void *idl_)\n> > json_object_put(monitor_requests, tc->name,\n> > json_array_create_1(monitor_request));\n> > }\n> > +\n> > + if (!table->in_server_schema) {\n> > + ovsdb_cs_clear_condition(idl->cs, table->class_->name);\n> > + } else if (ovsdb_idl_condition_is_set(&table->req_cond)) {\n> > + /* Update the monitor condition request according to the\n> > + * db schema. */\n> > + ovsdb_idl_set_condition__(idl, tc, &table->req_cond);\n> \n> Hmm. The ovsdb_idl_set_condition__() function doesn't filter, it uses the old\n> filtered condition stored in the table->req_cond that was created before we\n> received the new schema. So, we can still set conditions here for tables that\n> do not exist in on the server.\n\nack, in v4 I will move the filtering in ovsdb_idl_set_condition__()\n\n> \n> > + }\n> > }\n> > ovsdb_cs_free_schema(schema);\n> > \n> > @@ -1156,6 +1186,45 @@ ovsdb_idl_condition_add_clause__(struct ovsdb_idl_condition *condition,\n> > hmap_insert(&condition->clauses, &clause->hmap_node, hash);\n> > }\n> > \n> > +static void\n> > +ovsdb_idl_destroy_req_condition(struct ovsdb_idl_table *table)\n> > +{\n> > + ovsdb_idl_condition_destroy(&table->req_cond);\n> > +}\n> > +\n> > +static bool\n> > +ovsdb_idl_condition_is_set(struct ovsdb_idl_condition *condition)\n> > +{\n> > + return !hmap_is_empty(&condition->clauses) || condition->is_true;\n> > +}\n> > +\n> > +static void\n> > +ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dest,\n> > + const struct ovsdb_idl_condition *source)\n> > +{\n> > + ovsdb_idl_condition_init(dest);\n> > +\n> > + struct ovsdb_idl_clause *clause;\n> > + HMAP_FOR_EACH (clause, hmap_node, &source->clauses) {\n> > + uint32_t hash = ovsdb_idl_clause_hash(clause);\n> \n> Can use a hash of the current hmap node.\n\nack, I will fix it in v4\n\n> \n> > + ovsdb_idl_condition_add_clause__(dest, clause, hash);\n> > + }\n> > + dest->is_true = source->is_true;\n> > +}\n> > +\n> > +static void\n> > +ovsdb_idl_create_req_condition(struct ovsdb_idl *idl,\n> > + const struct ovsdb_idl_table_class *tc,\n> > + const struct ovsdb_idl_condition *condition)\n> > +{\n> > + struct ovsdb_idl_table *table = shash_find_data(&idl->table_by_name,\n> > + tc->name);\n> > + if (table) {\n> > + ovsdb_idl_destroy_req_condition(table);\n> > + ovsdb_idl_condition_clone(&table->req_cond, condition);\n> > + }\n> > +}\n> > +\n> > /* Adds a clause to the condition for replicating the table with class 'tc' in\n> > * 'idl'.\n> > *\n> > @@ -1234,6 +1303,17 @@ ovsdb_idl_condition_to_json(const struct ovsdb_idl_condition *cnd)\n> > return json_array_create(clauses, n);\n> > }\n> > \n> > +static unsigned int\n> > +ovsdb_idl_set_condition__(struct ovsdb_idl *idl,\n> > + const struct ovsdb_idl_table_class *tc,\n> > + const struct ovsdb_idl_condition *condition)\n> > +{\n> > + struct json *cond_json = ovsdb_idl_condition_to_json(condition);\n> > + unsigned int seqno = ovsdb_cs_set_condition(idl->cs, tc->name, cond_json);\n> > + json_destroy(cond_json);\n> > + return seqno;\n> > +}\n> > +\n> > /* Sets the replication condition for 'tc' in 'idl' to 'condition' and\n> > * arranges to send the new condition to the database server.\n> > *\n> > @@ -1245,9 +1325,33 @@ ovsdb_idl_set_condition(struct ovsdb_idl *idl,\n> > const struct ovsdb_idl_table_class *tc,\n> > const struct ovsdb_idl_condition *condition)\n> > {\n> > - struct json *cond_json = ovsdb_idl_condition_to_json(condition);\n> > - unsigned int seqno = ovsdb_cs_set_condition(idl->cs, tc->name, cond_json);\n> > - json_destroy(cond_json);\n> > + struct ovsdb_idl_condition filter_cond =\n> > + OVSDB_IDL_CONDITION_INIT(&filter_cond);\n> > +\n> > + if (idl->server_schema_received && hmap_count(&condition->clauses)) {\n> > + struct ovsdb_idl_clause *clause;\n> > + HMAP_FOR_EACH (clause, hmap_node, &condition->clauses) {\n> > + struct ovsdb_idl_table *t =\n> > + ovsdb_idl_table_from_column(idl, clause->column);\n> \n> This shoudld be done outside of the loop. We can get the table directly\n> from the table class - ovsdb_idl_table_from_class().\n\nack, I will fix it in v4\n\n> \n> > + if (!t || !t->in_server_schema) {\n> > + return ovsdb_idl_get_condition_seqno(idl);\n> \n> We can't just return here, we still need to save these conditions, otherwise\n> we will not send them on reconnection when the server schema changes.\n> \n> > + }\n> > +\n> > + if (ovsdb_idl_server_has_column(idl, clause->column)) {\n> > + uint32_t hash = ovsdb_idl_clause_hash(clause);\n> > + ovsdb_idl_condition_add_clause__(&filter_cond, clause, hash);\n> > + }\n> > + }\n> > + condition = &filter_cond;\n> > + }\n> > +\n> > + unsigned int seqno = ovsdb_idl_set_condition__(idl, tc, condition);\n> > + ovsdb_idl_create_req_condition(idl, tc, &filter_cond);\n> \n> We should be saving the original conditions, not the filtered ones, because\n> we need to filter every time we compose a new monitor request.\n\nack, I will fix it in v4\n\n> \n> > +\n> > + if (hmap_count(&filter_cond.clauses)) {\n> > + ovsdb_idl_condition_destroy(&filter_cond);\n> > + }\n> > +\n> > return seqno;\n> > }\n> > \n> > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at\n> > index 728d761d4..194afef13 100644\n> > --- a/tests/ovsdb-idl.at\n> > +++ b/tests/ovsdb-idl.at\n> > @@ -2722,6 +2722,9 @@ unix:socket2 remote has col id in table simple7, type: string\n> > --- remote unix:socket2 done ---\n> > ], [stderr])\n> > \n> > +# Check we do not have any errors related to conditional monitoring.\n> > +AT_CHECK([grep -q \"received error, error={\\\"details\\\":\\\"no table named link2\\\",\\\"error\\\":\\\"syntax error\\\"}\" stderr], [1])\n> > +\n> > OVSDB_SERVER_SHUTDOWN\n> > AT_CLEANUP\n> \n> I think we need way more tests for this functionality, including a case where\n> we connect to a server with an older schema that doesn't have some tables and\n> some columns in the tables that exist, then re-connect to a server with the new\n> schema that has all, then re-connect back to one with the older schema. The\n> client sets conditions for all the tables before connecting to the first server\n> and doesn't change them afterwards. We need to make sure that all the conditions\n> are properly filtered every time the cilent re-connects without touching the\n> conditions. Ant another test where client sets conditions for tables and\n> columns that exist and for those that do not exist after the succesful connection\n> to make sure the filtering works outside of the monitor request. Also maybe\n> some tests to make sure the monitor-cond-since is respected anf the last-id is\n> properly cleared when it needs to be and not cleared when it doesn't need to\n> be cleared.\n\nack, I will add more test in v4.\n\nRegards,\nLorenzo\n\n> \n> THe commit message also could use a larger description of the problem and what\n> this change is trying to achieve.\n> \n> Best regards, Ilya Maximets.\n> \n> > \n> > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c\n> > index 95290ae78..037a78743 100644\n> > --- a/tests/test-ovsdb.c\n> > +++ b/tests/test-ovsdb.c\n> > @@ -3596,6 +3596,12 @@ do_idl_table_column_check(struct ovs_cmdl_context *ctx)\n> > ovsdb_idl_set_remote(idl, ctx->argv[r], true);\n> > ovsdb_idl_force_reconnect(idl);\n> > \n> > + struct ovsdb_idl_condition cond_link2 =\n> > + OVSDB_IDL_CONDITION_INIT(&cond_link2);\n> > + idltest_link2_add_clause_i(&cond_link2, OVSDB_F_EQ, 1);\n> > +\n> > + idltest_link2_set_condition(idl, &cond_link2);\n> > +\n> > /* Wait for update. */\n> > for (;;) {\n> > ovsdb_idl_run(idl);\n> > @@ -3661,6 +3667,8 @@ do_idl_table_column_check(struct ovs_cmdl_context *ctx)\n> > type ? \", type: \" : \"\", type ? type_s : \"\");\n> > free(type_s);\n> > \n> > + ovsdb_idl_condition_destroy(&cond_link2);\n> > +\n> > printf(\"--- remote %s done ---\\n\", ctx->argv[r]);\n> > }\n> > \n>","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","ovs-dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@legolas.ozlabs.org","ovs-dev@lists.linuxfoundation.org"],"Authentication-Results":["legolas.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=LaLdafzF;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=google header.b=N/ZOXiDg;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org\n (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org)","smtp1.osuosl.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key)\n header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=LaLdafzF;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key)\n header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=google\n header.b=N/ZOXiDg","smtp1.osuosl.org; dmarc=pass (p=quarantine dis=none)\n header.from=redhat.com"],"Received":["from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fxM3b1bW0z1yHP\n\tfor ; Fri, 17 Apr 2026 01:13:45 +1000 (AEST)","from localhost (localhost [127.0.0.1])\n\tby smtp1.osuosl.org (Postfix) with ESMTP id F18E980DB9;\n\tThu, 16 Apr 2026 15:13:42 +0000 (UTC)","from smtp1.osuosl.org ([127.0.0.1])\n by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id oE4diibpffRE; Thu, 16 Apr 2026 15:13:41 +0000 (UTC)","from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])\n\tby smtp1.osuosl.org (Postfix) with ESMTPS id 5CBD780D97;\n\tThu, 16 Apr 2026 15:13:41 +0000 (UTC)","from lf-lists.osuosl.org (localhost [127.0.0.1])\n\tby lists.linuxfoundation.org (Postfix) with ESMTP id 35E32C054A;\n\tThu, 16 Apr 2026 15:13:41 +0000 (UTC)","from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138])\n by lists.linuxfoundation.org (Postfix) with ESMTP id C625BC0549\n for ; Thu, 16 Apr 2026 15:13:39 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n by smtp1.osuosl.org (Postfix) with ESMTP id B1E9880DB2\n for ; Thu, 16 Apr 2026 15:13:39 +0000 (UTC)","from smtp1.osuosl.org ([127.0.0.1])\n by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id BQBYSIf3ok3Y for ;\n Thu, 16 Apr 2026 15:13:38 +0000 (UTC)","from us-smtp-delivery-124.mimecast.com\n (us-smtp-delivery-124.mimecast.com [170.10.133.124])\n by smtp1.osuosl.org (Postfix) with ESMTPS id 2A41E80D97\n for ; Thu, 16 Apr 2026 15:13:37 +0000 (UTC)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n us-mta-690-_IlJdd_-Pn6FkUs7eoALWg-1; Thu, 16 Apr 2026 11:13:34 -0400","by mail-wm1-f69.google.com with SMTP id\n 5b1f17b1804b1-488c213b485so61811245e9.2\n for ; Thu, 16 Apr 2026 08:13:34 -0700 (PDT)","from localhost (net-37-119-153-93.cust.vodafonedsl.it.\n [37.119.153.93]) by smtp.gmail.com with ESMTPSA id\n ffacd0b85a97d-43ead2decafsm13029716f8f.0.2026.04.16.08.13.30\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Thu, 16 Apr 2026 08:13:30 -0700 (PDT)"],"X-Virus-Scanned":["amavis at osuosl.org","amavis at osuosl.org"],"X-Comment":"SPF check N/A for local connections - client-ip=140.211.9.56;\n helo=lists.linuxfoundation.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver= ","DKIM-Filter":["OpenDKIM Filter v2.11.0 smtp1.osuosl.org 5CBD780D97","OpenDKIM Filter v2.11.0 smtp1.osuosl.org 2A41E80D97"],"Received-SPF":"Pass (mailfrom) identity=mailfrom; client-ip=170.10.133.124;\n helo=us-smtp-delivery-124.mimecast.com;\n envelope-from=lorenzo.bianconi@redhat.com; receiver=","DMARC-Filter":"OpenDMARC Filter v1.4.2 smtp1.osuosl.org 2A41E80D97","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n s=mimecast20190719; t=1776352416;\n h=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n to:to:cc:cc:mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=whMTJfhA+1Xj/fqukAgL0+KDME6NNRc04LZNmGrjk6c=;\n b=LaLdafzFR1JfuJ0RcX0ujm6VyIc0XTuUPJaR/3eNapT3arSsEWJmSB6F/6L73muQPwA3eV\n oIubLGAevNiT1B5aGZQXpgfBV/ajLMahF/MFtS9YETD7vHb4kVm8djCYr71ssrfNP2sV6C\n hhaYT6IOPqHLetLbdStPpqmuOYap7Lw=","v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=redhat.com; s=google; t=1776352413; x=1776957213; darn=openvswitch.org;\n h=in-reply-to:content-disposition:mime-version:references:message-id\n :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to;\n bh=whMTJfhA+1Xj/fqukAgL0+KDME6NNRc04LZNmGrjk6c=;\n b=N/ZOXiDgyTDUMedVMRye9AoVukLEwlokIyDWKYMz/PlZMxr6cZ/sPatoRNS3qirGZ/\n iedfaqY5r1yItmSsqmMtWcR/wdl471T0O4p/Eyyv4kE5EKx3wn88lHJl4+lWyRniDUi+\n 6LNoPastlNZMVg0TfNjGjLHqFWJnGbVaIjfIqMxIIvj74zDjHGmLfudvrhFtRs7fT5Ky\n S50HDZAZcCKadgYvBTeipaSdlqJf/BzSt8iFesK7ayYmYyNxjx8usQzRxTW4aWjsn+Bh\n d1xlrAzR2D0q7tbfo8iCIqwHj96ymvp2eymyWu4mdz0jQKa5OhfTGPuJ6JFb7n8/MvzC\n CC5Q=="],"X-MC-Unique":"_IlJdd_-Pn6FkUs7eoALWg-1","X-Mimecast-MFC-AGG-ID":"_IlJdd_-Pn6FkUs7eoALWg_1776352413","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20251104; t=1776352413; x=1776957213;\n h=in-reply-to:content-disposition:mime-version:references:message-id\n :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc\n :subject:date:message-id:reply-to;\n bh=whMTJfhA+1Xj/fqukAgL0+KDME6NNRc04LZNmGrjk6c=;\n b=hi2GUy9G3Hp13/nPJ6/DFQUwGu2+xTaEYVPGYS6riJ+O6UEth2qYslBDn79sF7XjFM\n 95+fxK8hkVCPoIp5wSmb+vFEqHGHFfDaLdxXacXL9SZs58EXwVoV5wVO0T3QLEoNxPqw\n yHXiWqJLxcgIeYmR4waoJiMR2KGReFhwzs91v4A88V+PLiTZpTNZ1fOzRwt2Jc6SU2Df\n ebxunw2lRCny7blAKDYM1oSh2nVciHKHkzucStyituKF6p9FxojbhiYhG8Tno1bxDg4K\n EHyqHia9EphF6v4TMj2GqszRwg4quU71NaVsxGd0Kt5NZorPL1b1z/LQF5gqNBNUQ1b0\n oZ7w==","X-Gm-Message-State":"AOJu0YxAgTpJtdtz1Q4QklEHrjvlDgxmIyBXwX06BaO57OhWSNuzx/aq\n ORyE8pPy9LgIhCp0/+GJpW7IHaFqQnYDncd0YwzK9cjy7h/ms62onhzlnrPHCJQ15/qO0kNJF6u\n 7EQyw24fb+RpjnAWB6DheFx7nC7DsLBH31AkJdeKmMFJ2dW6eT5T6+BLCmMI=","X-Gm-Gg":"AeBDiesbFAhytk1so/+OP6WWTZB475A/oFojdTRtPOqwLoo7vbZqTwLlyOcf0Jbyr7c\n 6IqI5WGvsXLtUbBpw93FyepfQDzjtXJU8T7sB0SPlQHWsAySxeclW4LY8Q2toPIJkplt/mCVBcG\n E9UB/LorMMtxkKDVg+34vYeC7prW6T77ceaFOG9h2f4lB9TTAR6+jR54c1qSb7zkwYZ2cnSrTRU\n udcJkt9hYMGFKqeoFZjlKk3By+Or1tijkSlb3TFYvxh9xwDVzqY/2eGbO5ouRs+6VtBPWeZjmhJ\n hn0amydoGNQ3RGLMh43BbtzgGN49cFW89Dd3d2ok8QAeRBAEvi8RcWbtPzYGV2v+wJfRhOEwg37\n kcMvWfefLqgyLRZp76XTNvS3U0MQZ9jFtxabFUhswLXL6VY/jsqk/0ehPi1s2RSg75/ll2t/iH9\n v3K6ao","X-Received":["by 2002:a05:600c:8904:b0:487:1c2:6a56 with SMTP id\n 5b1f17b1804b1-488d67ec89amr278319895e9.3.1776352412796;\n Thu, 16 Apr 2026 08:13:32 -0700 (PDT)","by 2002:a05:600c:8904:b0:487:1c2:6a56 with SMTP id\n 5b1f17b1804b1-488d67ec89amr278319275e9.3.1776352412134;\n Thu, 16 Apr 2026 08:13:32 -0700 (PDT)"],"Date":"Thu, 16 Apr 2026 17:13:29 +0200","To":"Ilya Maximets ","Cc":"ovs-dev@openvswitch.org, dceara@redhat.com","Message-ID":"","References":"\n <195d108bcfc9b2337eeb0b7f5fa789804e7c86d0.1775679683.git.lorenzo.bianconi@redhat.com>\n <9e42afb4-558f-499c-a396-55ab1a39eaf0@ovn.org>","MIME-Version":"1.0","In-Reply-To":"<9e42afb4-558f-499c-a396-55ab1a39eaf0@ovn.org>","X-Content-Filtered-By":"Mailman/MimeDel 2.1.30","Subject":"Re: [ovs-dev] [PATCH v3] ovsdb: Save user monitor condition request\n in ovsdb_idl_table struct.","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.30","Precedence":"list","List-Id":"","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","From":"Lorenzo Bianconi via dev ","Reply-To":"Lorenzo Bianconi ","Content-Type":"multipart/mixed; boundary=\"===============1976437868382333686==\"","Errors-To":"ovs-dev-bounces@openvswitch.org","Sender":"\"dev\" "}}]