From e9401364da28259f065b2106fa8590423490f3ea Mon Sep 17 00:00:00 2001 From: Kamil Cudnik Date: Tue, 5 Jan 2021 14:18:42 +0100 Subject: [PATCH] [syncd] Fix bulk multi attrs for same key db update (#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 --- syncd/Syncd.cpp | 14 +++++++++++++- tests/BCM56850.pl | 11 +++++++++++ tests/BCM56850/test_bulk_set_multiple_A.rec | 13 +++++++++++++ tests/BCM56850/test_bulk_set_multiple_B.rec | 13 +++++++++++++ 4 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 tests/BCM56850/test_bulk_set_multiple_A.rec create mode 100644 tests/BCM56850/test_bulk_set_multiple_B.rec diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index a993547e5..fabb9b884 100644 --- 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 index 089229f2b..1ea82142b 100755 --- 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 new file mode 100644 index 000000000..8f6a631a4 --- /dev/null +++ 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 new file mode 100644 index 000000000..71f06d670 --- /dev/null +++ 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