On 12/12/22 20:42, Ilya Maximets wrote:
> On 12/8/22 16:21, Dumitru Ceara wrote:
>> Applications request specific monitor conditions via the
>> ovsdb_cs_set_condition() API. This returns the condition sequence
>> number at which the client can assume that the server acknowledged
>> and applied the new monitor condition.
>>
>> A corner case is when the client's first request is to set the condition
>> to a value that's equivalent to the default one ("<true>"). In such
>> cases, because it's requested explicitly by the client, we now
>> explicitly send the monitor_cond_change request to the server. This
>> avoids cases in which the expected condition sequence number and the
>> acked condition sequence number get out of sync.
>
> Sorry, I think I'm lost again. What exactly we're trying to fix here?
> [True] is a default condition if no other conditions are supplied for
> the table. Why do we need to send it explicitly?
>
> At first I thought that newly created condition from ovsdb_cs_db_get_table()
> has to be sent out, but... if it is the first time we're setting
> conditions for that table, default condition should be [True], hence
> equal to the condition we're trying to set.
>
>
> The test case is a bit synthetic as it doesn't seem to test the issue
> that exists in OVN. The fact that python IDL passes the test without
> changes also doesn't make a lot of sense to me.
>
> Could you provide some more data on what exactly is going on and what
> we're trying to fix? With jsonrpc logs, if possible.
Should we do this instead:
diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
index a6fbd290c..0fca03d72 100644
--- a/lib/ovsdb-cs.c
+++ b/lib/ovsdb-cs.c
@@ -892,7 +892,7 @@ ovsdb_cs_db_get_table(struct ovsdb_cs_db *db, const char
*table)
t = xzalloc(sizeof *t);
t->name = xstrdup(table);
- t->new_cond = json_array_create_1(json_boolean_create(true));
+ t->ack_cond = json_array_create_1(json_boolean_create(true));
hmap_insert(&db->tables, &t->hmap_node, hash);
return t;
}
---
?
This breaks a few tests, but it seems to be a correct change because it
is indeed a default condition that server already has for the table.
What do you think? Will that fix the OVN oproblem?
>
> Thanks.
> Best regards, Ilya Maximets.
>
>>
>> Reported-by: Numan Siddique <[email protected]>
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
>> lib/ovsdb-cs.c | 1 +
>> tests/ovsdb-idl.at | 27 +++++++++++++++++++++++++++
>> tests/test-ovsdb.c | 28 ++++++++++++++++++++++++----
>> tests/test-ovsdb.py | 20 +++++++++++++++++---
>> 4 files changed, 69 insertions(+), 7 deletions(-)
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
