[syncd] Fix bulk multi attrs for same key db update (sonic-net#761)

issue was related to bulk set operation, when multiple attributes were set for the same key, in this case bug was causing that only last attribute was actually written to database and previous ones were skipped
pettershao-ragilenetworks · Jan 5, 2021 · 6175008 · 6175008
1 parent 0cfe091
commit 6175008
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 1 deletion.
diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp
@@ -2074,7 +2074,19 @@ void Syncd::syncUpdateRedisBulkQuadEvent(
 
         keys.push_back(key);
 
-        multiHash[key] = strAttributes.at(idx);
+        if (api == SAI_COMMON_API_BULK_SET)
+        {
+            // in case of bulk set operation, it can happen that multiple
+            // attributes will be set for the same key, then when we want to
+            // push them to redis database, we need to combine all attributes
+            // to a single vector of attributes
+
+            multiHash[key].push_back(strAttributes.at(idx).at(0));
+        }
+        else
+        {
+            multiHash[key] = strAttributes.at(idx);
+        }
     }
 
     const bool initView = isInitViewMode();

diff --git a/tests/BCM56850.pl b/tests/BCM56850.pl
@@ -615,7 +615,18 @@ sub test_depreacated_enums
     play "non_depreacated.rec", 0;
 }
 
+sub test_bulk_set_multiple
+{
+    fresh_start;
+
+    play "test_bulk_set_multiple_A.rec";
+    play "test_bulk_set_multiple_B.rec", 0;
+    exit;
+}
+
 # RUN TESTS
+
+test_bulk_set_multiple;
 test_depreacated_enums;
 test_brcm_buffer_pool_zmq_sync_flag;
 test_brcm_buffer_pool_zmq;

diff --git a/tests/BCM56850/test_bulk_set_multiple_A.rec b/tests/BCM56850/test_bulk_set_multiple_A.rec
@@ -0,0 +1,13 @@
+2020-12-31.21:34:31.861834|a|INIT_VIEW
+2020-12-31.21:34:31.862379|A|SAI_STATUS_SUCCESS
+2020-12-31.21:34:31.864591|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true
+2020-12-31.21:34:40.809232|g|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_DEFAULT_VIRTUAL_ROUTER_ID=oid:0x0
+2020-12-31.21:34:40.811276|G|SAI_STATUS_SUCCESS|SAI_SWITCH_ATTR_DEFAULT_VIRTUAL_ROUTER_ID=oid:0x300000000007c
+2020-12-31.21:34:41.115557|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"0.0.0.0/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION=SAI_PACKET_ACTION_FORWARD
+2020-12-31.21:34:41.116761|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"::/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION=SAI_PACKET_ACTION_FORWARD
+2020-12-31.21:35:18.863339|c|SAI_OBJECT_TYPE_NEXT_HOP_GROUP:oid:0x500000000176c|SAI_NEXT_HOP_GROUP_ATTR_TYPE=SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_UNORDERED_ECMP
+2020-12-31.21:35:41.614245|c|SAI_OBJECT_TYPE_NEXT_HOP_GROUP:oid:0x5000000001979|SAI_NEXT_HOP_GROUP_ATTR_TYPE=SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_UNORDERED_ECMP
+2020-12-31.21:35:19.172005|S|SAI_OBJECT_TYPE_ROUTE_ENTRY||{"dest":"0.0.0.0/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x500000000176c
+2020-12-31.21:35:44.181541|S|SAI_OBJECT_TYPE_ROUTE_ENTRY||{"dest":"::/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000001979
+2020-12-31.21:34:41.215885|a|APPLY_VIEW
+2020-12-31.21:34:41.216326|A|SAI_STATUS_SUCCESS
diff --git a/tests/BCM56850/test_bulk_set_multiple_B.rec b/tests/BCM56850/test_bulk_set_multiple_B.rec
@@ -0,0 +1,13 @@
+2020-12-31.21:45:21.481005|a|INIT_VIEW
+2020-12-31.21:45:35.240620|A|SAI_STATUS_SUCCESS
+2020-12-31.21:45:35.244322|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true
+2020-12-31.21:45:35.246412|g|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_DEFAULT_VIRTUAL_ROUTER_ID=oid:0x0
+2020-12-31.21:45:35.249161|G|SAI_STATUS_SUCCESS|SAI_SWITCH_ATTR_DEFAULT_VIRTUAL_ROUTER_ID=oid:0x300000000007c
+2020-12-31.21:45:35.767045|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"0.0.0.0/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION=SAI_PACKET_ACTION_FORWARD
+2020-12-31.21:45:35.767830|c|SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"::/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION=SAI_PACKET_ACTION_FORWARD
+2020-12-31.21:45:40.685907|c|SAI_OBJECT_TYPE_NEXT_HOP_GROUP:oid:0x5000000001cf1|SAI_NEXT_HOP_GROUP_ATTR_TYPE=SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_UNORDERED_ECMP
+2020-12-31.21:45:40.780018|c|SAI_OBJECT_TYPE_NEXT_HOP_GROUP:oid:0x5000000001cf6|SAI_NEXT_HOP_GROUP_ATTR_TYPE=SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_UNORDERED_ECMP
+2020-12-31.21:45:41.208884|S|SAI_OBJECT_TYPE_ROUTE_ENTRY||{"dest":"::/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000001cf6||{"dest":"::/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION=SAI_PACKET_ACTION_FORWARD
+2020-12-31.21:45:41.208884|S|SAI_OBJECT_TYPE_ROUTE_ENTRY||{"dest":"0.0.0.0/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID=oid:0x5000000001cf1||{"dest":"0.0.0.0/0","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000007c"}|SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION=SAI_PACKET_ACTION_FORWARD
+2020-12-31.21:46:01.369832|a|APPLY_VIEW
+2020-12-31.21:46:07.779419|A|SAI_STATUS_SUCCESS