| Message ID | 1605018868-30691-1-git-send-email-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovsdb-idl: Return correct seqno from ovsdb_idl_db_set_condition(). | expand |
On 11/10/20 3:34 PM, Dumitru Ceara wrote: > If an IDL client sets the same monitor condition twice, the expected > seqno when the IDL contents are updated should be the same for both > calls. > > In the following scenario: > 1. Client calls ovsdb_idl_db_set_condition(db, table, cond1) > 2. ovsdb_idl sends monitor_cond_change(cond1) but the server doesn't yet > reply. > 3. Client calls ovsdb_idl_db_set_condition(db, table, cond1) > > At step 3 the returned expected seqno should be the same as at step 1. > Similarly, if step 2 is skipped, i.e., the client calls sets > the condition twice in the same iteration, then both > ovsdb_idl_db_set_condition() calls should return the same value. > > Fixes: 46437c5232bd ("ovsdb-idl: Enhance conditional monitoring API") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- Thanks! Applied to master and backported down to 2.12. It's hard to backport further since we have no fix for in-flight conditions below 2.12, but we likely do not need it there. Best regards, Ilya Maximets.
On 11/16/20 8:37 PM, Ilya Maximets wrote: > On 11/10/20 3:34 PM, Dumitru Ceara wrote: >> If an IDL client sets the same monitor condition twice, the expected >> seqno when the IDL contents are updated should be the same for both >> calls. >> >> In the following scenario: >> 1. Client calls ovsdb_idl_db_set_condition(db, table, cond1) >> 2. ovsdb_idl sends monitor_cond_change(cond1) but the server doesn't yet >> reply. >> 3. Client calls ovsdb_idl_db_set_condition(db, table, cond1) >> >> At step 3 the returned expected seqno should be the same as at step 1. >> Similarly, if step 2 is skipped, i.e., the client calls sets >> the condition twice in the same iteration, then both >> ovsdb_idl_db_set_condition() calls should return the same value. >> >> Fixes: 46437c5232bd ("ovsdb-idl: Enhance conditional monitoring API") >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> --- > > Thanks! > > Applied to master and backported down to 2.12. Thanks! > > > It's hard to backport further since we have no fix for in-flight conditions > below 2.12, but we likely do not need it there. > I think it's fine (for OVN at least). Thanks, Dumitru
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index fdb9d85..d1fa6ab 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -1564,7 +1564,6 @@ ovsdb_idl_db_set_condition(struct ovsdb_idl_db *db, { struct ovsdb_idl_condition *table_cond; struct ovsdb_idl_table *table = ovsdb_idl_db_table_from_class(db, tc); - unsigned int seqno = db->cond_seqno; /* Compare the new condition to the last known condition which can be * either "new" (not sent yet), "requested" or "acked", in this order. @@ -1582,10 +1581,14 @@ ovsdb_idl_db_set_condition(struct ovsdb_idl_db *db, ovsdb_idl_condition_clone(&table->new_cond, condition); db->cond_changed = true; poll_immediate_wake(); - return seqno + 1; + return db->cond_seqno + 1; + } else if (table_cond != table->ack_cond) { + /* 'condition' was already set but has not been "acked" yet. The IDL + * will be up to date when db->cond_seqno gets incremented. */ + return db->cond_seqno + 1; } - return seqno; + return db->cond_seqno; } /* Sets the replication condition for 'tc' in 'idl' to 'condition' and diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index b1a4be3..bb9deba 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -2391,6 +2391,10 @@ update_conditions(struct ovsdb_idl *idl, char *commands) if (seqno == next_seqno ) { ovs_fatal(0, "condition unchanged"); } + unsigned int new_next_seqno = ovsdb_idl_set_condition(idl, tc, &cond); + if (next_seqno != new_next_seqno) { + ovs_fatal(0, "condition expected seqno changed"); + } ovsdb_idl_condition_destroy(&cond); json_destroy(json); }
If an IDL client sets the same monitor condition twice, the expected seqno when the IDL contents are updated should be the same for both calls. In the following scenario: 1. Client calls ovsdb_idl_db_set_condition(db, table, cond1) 2. ovsdb_idl sends monitor_cond_change(cond1) but the server doesn't yet reply. 3. Client calls ovsdb_idl_db_set_condition(db, table, cond1) At step 3 the returned expected seqno should be the same as at step 1. Similarly, if step 2 is skipped, i.e., the client calls sets the condition twice in the same iteration, then both ovsdb_idl_db_set_condition() calls should return the same value. Fixes: 46437c5232bd ("ovsdb-idl: Enhance conditional monitoring API") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- lib/ovsdb-idl.c | 9 ++++++--- tests/test-ovsdb.c | 4 ++++ 2 files changed, 10 insertions(+), 3 deletions(-)