In ovsdb_cs_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-cs 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 conditon change at step 3 is still not replied
to, table B's monitor_cond_change request is not sent yet.
5. ovsdb-cs receives the reply for the condition change at step 3:
- db->cond_seqno <- X + 1
6. ovsdb-cs sends the monitor_cond_change for table B
7. ovsdb-cs 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 <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
---
V2:
- Addressed Ben's comments:
- move hmap walk inside the 'if' branch.
- don't use !! for bool.
- Added Ben's ack.
---
lib/ovsdb-cs.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
index 6f9f912ac4..911b71dd4f 100644
--- a/lib/ovsdb-cs.c
+++ b/lib/ovsdb-cs.c
@@ -903,8 +903,27 @@ ovsdb_cs_db_set_condition(struct ovsdb_cs_db *db, const
char *table,
}
/* Conditions will be up to date when we receive replies for already
- * requested and new conditions, if any. */
- return db->cond_seqno + (t->new_cond ? 1 : 0) + (t->req_cond ? 1 : 0);
+ * requested and new conditions, if any. This includes condition change
+ * requests for other tables too.
+ */
+ if (t->new_cond) {
+ /* New condition will be sent out after all already requested ones
+ * are acked.
+ */
+ bool any_req_cond = false;
+ HMAP_FOR_EACH (t, hmap_node, &db->tables) {
+ if (t->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 + !!t->req_cond;
+ }
}
/* Sets the replication condition for 'tc' in 'cs' to 'condition' and arranges
--
2.27.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
