Cached json objects were reused when sending notifications to clients. This created a problem when there were different versions of monitors coexiting. E.g. clients expecting version2 notification would receive messages with method == "update2" but payload in version1 format, which end up failure of processing the updates.
http://openvswitch.org/pipermail/dev/2015-December/063406.html This patch fixes the issue by using dedicated cache for each version. Signed-off-by: Han Zhou <zhou...@gmail.com> --- ovsdb/monitor.c | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c index f08607a..31f26e7 100644 --- a/ovsdb/monitor.c +++ b/ovsdb/monitor.c @@ -54,7 +54,9 @@ struct ovsdb_monitor { struct ovsdb *db; uint64_t n_transactions; /* Count number of committed transactions. */ struct hmap_node hmap_node; /* Elements within ovsdb_monitors. */ - struct hmap json_cache; /* Contains "ovsdb_monitor_json_cache_node"s.*/ + struct hmap json_cache[OVSDB_MONITOR_VERSION_MAX]; /* Array of caches, one + for each ovsdb monitor version, contains + "ovsdb_monitor_json_cache_node"s. */ }; /* A json object of updates between 'from_txn' and 'dbmon->n_transactions' @@ -136,12 +138,14 @@ static void ovsdb_monitor_table_track_changes(struct ovsdb_monitor_table *mt, static struct ovsdb_monitor_json_cache_node * ovsdb_monitor_json_cache_search(const struct ovsdb_monitor *dbmon, - uint64_t from_txn) + uint64_t from_txn, + enum ovsdb_monitor_version version) { struct ovsdb_monitor_json_cache_node *node; uint32_t hash = hash_uint64(from_txn); - HMAP_FOR_EACH_WITH_HASH(node, hmap_node, hash, &dbmon->json_cache) { + HMAP_FOR_EACH_WITH_HASH(node, hmap_node, hash, + &dbmon->json_cache[version]) { if (node->from_txn == from_txn) { return node; } @@ -152,7 +156,8 @@ ovsdb_monitor_json_cache_search(const struct ovsdb_monitor *dbmon, static void ovsdb_monitor_json_cache_insert(struct ovsdb_monitor *dbmon, - uint64_t from_txn, struct json *json) + uint64_t from_txn, struct json *json, + enum ovsdb_monitor_version version) { struct ovsdb_monitor_json_cache_node *node; uint32_t hash; @@ -163,7 +168,7 @@ ovsdb_monitor_json_cache_insert(struct ovsdb_monitor *dbmon, node->from_txn = from_txn; node->json = json ? json_clone(json) : NULL; - hmap_insert(&dbmon->json_cache, &node->hmap_node, hash); + hmap_insert(&dbmon->json_cache[version], &node->hmap_node, hash); } static void @@ -171,10 +176,13 @@ ovsdb_monitor_json_cache_flush(struct ovsdb_monitor *dbmon) { struct ovsdb_monitor_json_cache_node *node, *next; - HMAP_FOR_EACH_SAFE(node, next, hmap_node, &dbmon->json_cache) { - hmap_remove(&dbmon->json_cache, &node->hmap_node); - json_destroy(node->json); - free(node); + enum ovsdb_monitor_version v; + for (v = OVSDB_MONITOR_V1; v < OVSDB_MONITOR_VERSION_MAX; v++) { + HMAP_FOR_EACH_SAFE(node, next, hmap_node, &dbmon->json_cache[v]) { + hmap_remove(&dbmon->json_cache[v], &node->hmap_node); + json_destroy(node->json); + free(node); + } } } @@ -307,6 +315,7 @@ ovsdb_monitor_create(struct ovsdb *db, struct ovsdb_jsonrpc_monitor *jsonrpc_monitor) { struct ovsdb_monitor *dbmon; + enum ovsdb_monitor_version v; dbmon = xzalloc(sizeof *dbmon); @@ -317,7 +326,9 @@ ovsdb_monitor_create(struct ovsdb *db, dbmon->n_transactions = 0; shash_init(&dbmon->tables); hmap_node_nullify(&dbmon->hmap_node); - hmap_init(&dbmon->json_cache); + for (v = OVSDB_MONITOR_V1; v < OVSDB_MONITOR_VERSION_MAX; v++) { + hmap_init(&dbmon->json_cache[v]); + } ovsdb_monitor_add_jsonrpc_monitor(dbmon, jsonrpc_monitor); return dbmon; @@ -721,7 +732,7 @@ ovsdb_monitor_get_update(struct ovsdb_monitor *dbmon, /* Return a clone of cached json if one exists. Otherwise, * generate a new one and add it to the cache. */ - cache_node = ovsdb_monitor_json_cache_search(dbmon, prev_txn); + cache_node = ovsdb_monitor_json_cache_search(dbmon, prev_txn, version); if (cache_node) { json = cache_node->json ? json_clone(cache_node->json) : NULL; } else { @@ -733,7 +744,7 @@ ovsdb_monitor_get_update(struct ovsdb_monitor *dbmon, json = ovsdb_monitor_compose_update(dbmon, initial, prev_txn, ovsdb_monitor_compose_row_update2); } - ovsdb_monitor_json_cache_insert(dbmon, prev_txn, json); + ovsdb_monitor_json_cache_insert(dbmon, prev_txn, json, version); } /* Maintain transaction id of 'changes'. */ @@ -1072,6 +1083,7 @@ static void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon) { struct shash_node *node; + enum ovsdb_monitor_version v; list_remove(&dbmon->replica.node); @@ -1080,7 +1092,9 @@ ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon) } ovsdb_monitor_json_cache_flush(dbmon); - hmap_destroy(&dbmon->json_cache); + for (v = OVSDB_MONITOR_V1; v < OVSDB_MONITOR_VERSION_MAX; v++) { + hmap_destroy(&dbmon->json_cache[v]); + } SHASH_FOR_EACH (node, &dbmon->tables) { struct ovsdb_monitor_table *mt = node->data; -- 2.1.0 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
