[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
Metaswitch · Jan 5, 2021 · e940136 · e940136
1 parent 6385d49
commit e940136
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