| Message ID | 20210517092227.9813-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,branch-2.14] ovsdb-idl: Consider all tables when computing expected cond seqno. | expand |
On 5/17/21 11:22 AM, Dumitru Ceara wrote: > In ovsdb_idl_db_set_condition(), take into account all pending > condition changes for all tables when computing the db->cond_seqno at > which the monitor is expected to be updated. > > In the following scenario, with two tables, A and B, the old code > performed the following steps: > 1. Initial db->cond_seqno = X. > 2. Client changes condition for table A: > - A->new_cond gets set > - expected cond seqno returned to the client: X + 1 > 3. ovsdb-idl sends the monitor_cond_change for table A > - A->req_cond <- A->new_cond > 4. Client changes condition for table B: > - B->new_cond gets set > - expected cond seqno returned to the client: X + 1 > - however, because the condition change at step 3 is still not replied > to, table B's monitor_cond_change request is not sent yet. > 5. ovsdb-idl receives the reply for the condition change at step 3: > - db->cond_seqno <- X + 1 > 6. ovsdb-idl sends the monitor_cond_change for table B > 7. ovsdb-idl receives the reply for the condition change at step 6: > - db->cond_seqno <- X + 2 > > The client was incorrectly informed that it will have all relevant > updates for table B at seqno X + 1 while actually that happens later, at > seqno X + 2. > > Fixes: 46437c5232bd ("ovsdb-idl: Enhance conditional monitoring API") > Acked-by: Ben Pfaff <blp@ovn.org> > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > (cherry picked from commit b5bb044fbe4c1395dcde5cc7d5081ef0099bb8b3) > --- > Note: This backport applies cleanly to both 2.14 and 2.13. > --- Thanks! Applied to branches 2.14 and 2.13. Best regards, Ilya Maximets.
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index b5edaca860..da2c42c361 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -1592,7 +1592,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 curr_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. @@ -1613,8 +1612,27 @@ ovsdb_idl_db_set_condition(struct ovsdb_idl_db *db, } /* Conditions will be up to date when we receive replies for already - * requested and new conditions, if any. */ - return curr_seqno + (table->new_cond ? 1 : 0) + (table->req_cond ? 1 : 0); + * requested and new conditions, if any. This includes condition change + * requests for other tables too. + */ + if (table->new_cond) { + /* New condition will be sent out after all already requested ones + * are acked. + */ + bool any_req_cond = false; + for (size_t i = 0; i < db->class_->n_tables; i++) { + if (db->tables[i].req_cond) { + any_req_cond = true; + break; + } + } + return db->cond_seqno + any_req_cond + 1; + } else { + /* Already requested conditions should be up to date at + * db->cond_seqno + 1 while acked conditions are already up to date. + */ + return db->cond_seqno + !!table->req_cond; + } } /* Sets the replication condition for 'tc' in 'idl' to 'condition' and
In ovsdb_idl_db_set_condition(), take into account all pending condition changes for all tables when computing the db->cond_seqno at which the monitor is expected to be updated. In the following scenario, with two tables, A and B, the old code performed the following steps: 1. Initial db->cond_seqno = X. 2. Client changes condition for table A: - A->new_cond gets set - expected cond seqno returned to the client: X + 1 3. ovsdb-idl sends the monitor_cond_change for table A - A->req_cond <- A->new_cond 4. Client changes condition for table B: - B->new_cond gets set - expected cond seqno returned to the client: X + 1 - however, because the condition change at step 3 is still not replied to, table B's monitor_cond_change request is not sent yet. 5. ovsdb-idl receives the reply for the condition change at step 3: - db->cond_seqno <- X + 1 6. ovsdb-idl sends the monitor_cond_change for table B 7. ovsdb-idl receives the reply for the condition change at step 6: - db->cond_seqno <- X + 2 The client was incorrectly informed that it will have all relevant updates for table B at seqno X + 1 while actually that happens later, at seqno X + 2. Fixes: 46437c5232bd ("ovsdb-idl: Enhance conditional monitoring API") Acked-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Dumitru Ceara <dceara@redhat.com> (cherry picked from commit b5bb044fbe4c1395dcde5cc7d5081ef0099bb8b3) --- Note: This backport applies cleanly to both 2.14 and 2.13. --- lib/ovsdb-idl.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)