Skip to content

Commit

Permalink
ovsdb-cs: Consider all tables when computing expected cond seqno.
Browse files Browse the repository at this point in the history
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 condition 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: 46437c5 ("ovsdb-idl: Enhance conditional monitoring API")
Acked-by: Ben Pfaff <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
  • Loading branch information
dceara authored and igsilya committed May 14, 2021
1 parent 7100c22 commit b5bb044
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions lib/ovsdb-cs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b5bb044

Please sign in to comment.