diff mbox

[ovs-dev] ovsdb: separate json cache for different monitor versions

Message ID 1450198995-20413-1-git-send-email-zhouhan@gmail.com
State Superseded
Headers show

Commit Message

Han Zhou Dec. 15, 2015, 5:03 p.m. UTC 
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 <zhouhan@gmail.com>
---
 ovsdb/monitor.c | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)
diff mbox

Patch

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;