From 988c7ff425055dac181ce7e4663a428662dab834 Mon Sep 17 00:00:00 2001 From: PINS Working Group Date: Tue, 2 Nov 2021 12:47:48 -0400 Subject: [PATCH] vrforch: introduce sync_mode co-authored-by Runming Wu signed-off-by Don Newton --- orchagent/orch.cpp | 15 +++- orchagent/vrforch.cpp | 168 ++++++++++++++++++++++++++++++++---------- orchagent/vrforch.h | 4 +- tests/test_vrf.py | 91 +++++++++++++++-------- 4 files changed, 204 insertions(+), 74 deletions(-) diff --git a/orchagent/orch.cpp b/orchagent/orch.cpp index 2b6237ddcc8..1ba2990fcef 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -8,6 +8,7 @@ #include "orch.h" #include "subscriberstatetable.h" +#include "converter.h" #include "portsorch.h" #include "tokenize.h" #include "logger.h" @@ -44,7 +45,7 @@ Orch::Orch(DBConnector *db, const vector &tableNames_with } } -Orch::Orch(const vector& tables) +Orch::Orch(const vect { for (auto it : tables) { @@ -878,7 +879,15 @@ void Orch2::doTask(Consumer &consumer) while (it != consumer.m_toSync.end()) { bool erase_from_queue = true; + bool sync_mode = false; ReturnCode rc; + for (const auto &kv : kfvFieldsValues(it->second)) + { + if (to_upper(fvField(kv)) == "SYNC_MODE" && to_upper(fvValue(kv)) == "TRUE") + { + sync_mode = true; + } + } try { request_.parse(it->second); @@ -929,13 +938,13 @@ void Orch2::doTask(Consumer &consumer) SWSS_LOG_ERROR("%s", rc.message().c_str()); } request_.clear(); - if (!rc.ok()) + if (!rc.ok() && sync_mode) { m_publisher.publish(consumer.getTableName(), kfvKey(it->second), kfvFieldsValues(it->second), rc); } - if (erase_from_queue) + if (erase_from_queue || sync_mode) { it = consumer.m_toSync.erase(it); } diff --git a/orchagent/vrforch.cpp b/orchagent/vrforch.cpp index 44d2b5a016d..4097a27c2df 100644 --- a/orchagent/vrforch.cpp +++ b/orchagent/vrforch.cpp @@ -28,6 +28,8 @@ bool VRFOrch::addOperation(const Request& request) SWSS_LOG_ENTER(); uint32_t vni = 0; bool error = true; + bool sync_mode = false; + bool sync_mode_config = false; sai_attribute_t attr; vector attrs; @@ -75,6 +77,12 @@ bool VRFOrch::addOperation(const Request& request) SWSS_LOG_INFO("MGMT VRF field: %s ignored", name.c_str()); continue; } + else if (name == "sync_mode") + { + sync_mode_config = true; + sync_mode = request.getAttrBool("sync_mode"); + continue; + } else { SWSS_LOG_ERROR("Logic error: Unknown attribute: %s", name.c_str()); @@ -95,19 +103,26 @@ bool VRFOrch::addOperation(const Request& request) attrs.data()); if (status != SAI_STATUS_SUCCESS) { - ReturnCode rc = ReturnCode(status) - << "Failed to create virtual router name: " - << vrf_name << ", rv: " << sai_serialize_status(status); - SWSS_LOG_ERROR("%s", rc.message().c_str()); - m_publisher.publish(request.getTableName(), request.getFullKey(), - request.getFullAttrFields(), rc); - handleSaiCreateStatus(SAI_API_VIRTUAL_ROUTER, status); - // Remove from orchagent queue when there is SAI error - return true; + SWSS_LOG_ERROR("Failed to create virtual router name: %s, rv: %d", vrf_name.c_str(), status); + task_process_status handle_status = handleSaiCreateStatus(SAI_API_VIRTUAL_ROUTER, status); + if (sync_mode) + { + ReturnCode rc = ReturnCode(status) + << "Failed to create virtual router name: " + << vrf_name << ", rv: " << sai_serialize_status(status); + m_publisher.publish(request.getTableName(), request.getFullKey(), + request.getFullAttrFields(), rc); + return true; + } + else if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } } vrf_table_[vrf_name].vrf_id = router_id; vrf_table_[vrf_name].ref_count = 0; + vrf_table_[vrf_name].sync_mode = sync_mode; vrf_id_table_[router_id] = vrf_name; if (vni != 0) { @@ -115,7 +130,19 @@ bool VRFOrch::addOperation(const Request& request) error = updateVrfVNIMap(vrf_name, vni); if (error == false) { - return false; + if (sync_mode) + { + ReturnCode rc = ReturnCode(StatusCode::SWSS_RC_INVALID_PARAM) + << "Failed to update VRF vni map."; + SWSS_LOG_ERROR("%s", rc.message().c_str()); + m_publisher.publish(request.getTableName(), request.getFullKey(), + request.getFullAttrFields(), rc); + return true; + } + else + { + return false; + } } } m_stateVrfObjectTable.hset(vrf_name, "state", "ok"); @@ -125,6 +152,11 @@ bool VRFOrch::addOperation(const Request& request) { // Update an existing vrf + if (sync_mode_config) + { + vrf_table_[vrf_name].sync_mode = sync_mode; + } + sai_object_id_t router_id = it->second.vrf_id; for (const auto& attr: attrs) @@ -132,15 +164,22 @@ bool VRFOrch::addOperation(const Request& request) sai_status_t status = sai_virtual_router_api->set_virtual_router_attribute(router_id, &attr); if (status != SAI_STATUS_SUCCESS) { - ReturnCode rc = ReturnCode(status) - << "Failed to update virtual router attribute. vrf name: " - << vrf_name << ", rv: " << sai_serialize_status(status); - SWSS_LOG_ERROR("%s", rc.message().c_str()); - m_publisher.publish(request.getTableName(), request.getFullKey(), - request.getFullAttrFields(), rc); - handleSaiSetStatus(SAI_API_VIRTUAL_ROUTER, status); - // Remove from orchagent queue when there is SAI error - return true; + SWSS_LOG_ERROR("Failed to update virtual router attribute. vrf name: %s, rv: %d", vrf_name.c_str(), status); + task_process_status handle_status = handleSaiSetStatus(SAI_API_VIRTUAL_ROUTER, status); + if (sync_mode) + { + ReturnCode rc = ReturnCode(status) + << "Failed to update virtual router attribute. vrf name: " + << vrf_name << ", rv: " << sai_serialize_status(status); + m_publisher.publish(request.getTableName(), request.getFullKey(), + request.getFullAttrFields(), rc); + return true; + } + else if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } } @@ -148,14 +187,30 @@ bool VRFOrch::addOperation(const Request& request) error = updateVrfVNIMap(vrf_name, vni); if (error == false) { - return false; + if (sync_mode) + { + ReturnCode rc = ReturnCode(StatusCode::SWSS_RC_INVALID_PARAM) + << "Failed to update VRF vni map."; + SWSS_LOG_ERROR("%s", rc.message().c_str()); + m_publisher.publish(request.getTableName(), request.getFullKey(), + request.getFullAttrFields(), rc); + return true; + } + else + { + return false; + } } SWSS_LOG_NOTICE("VRF '%s' was updated", vrf_name.c_str()); } + + if (sync_mode) + { + m_publisher.publish(request.getTableName(), request.getFullKey(), + request.getFullAttrFields(), ReturnCode()); + } - m_publisher.publish(request.getTableName(), request.getFullKey(), - request.getFullAttrFields(), ReturnCode()); return true; } @@ -167,30 +222,48 @@ bool VRFOrch::delOperation(const Request& request) const std::string& vrf_name = request.getKeyString(0); if (vrf_table_.find(vrf_name) == std::end(vrf_table_)) { - ReturnCode rc = ReturnCode(StatusCode::SWSS_RC_NOT_FOUND) - << "VRF '" << vrf_name << "' doesn't exist"; - SWSS_LOG_ERROR("%s", rc.message().c_str()); - m_publisher.publish(request.getTableName(), request.getFullKey(), - request.getFullAttrFields(), rc); + SWSS_LOG_ERROR("VRF '%s' doesn't exist", vrf_name.c_str()); return true; } + bool sync_mode = vrf_table_[vrf_name].sync_mode; if (vrf_table_[vrf_name].ref_count) - return false; + { + if (sync_mode) + { + ReturnCode rc = ReturnCode(StatusCode::SWSS_RC_INVALID_PARAM) + << "Failed to delete VRF " << vrf_name + << ": reference count is not zero."; + SWSS_LOG_ERROR("%s", rc.message().c_str()); + m_publisher.publish(request.getTableName(), request.getFullKey(), + request.getFullAttrFields(), rc); + return true; + } + else + { + return false; + } + } sai_object_id_t router_id = vrf_table_[vrf_name].vrf_id; sai_status_t status = sai_virtual_router_api->remove_virtual_router(router_id); if (status != SAI_STATUS_SUCCESS) { - ReturnCode rc = ReturnCode(status) - << "Failed to remove virtual router name: " - << vrf_name << ", rv:" << sai_serialize_status(status); - SWSS_LOG_ERROR("%s", rc.message().c_str()); - m_publisher.publish(request.getTableName(), request.getFullKey(), - request.getFullAttrFields(), rc); - handleSaiRemoveStatus(SAI_API_VIRTUAL_ROUTER, status); - // Remove from orchagent queue when there is SAI error - return true; + SWSS_LOG_ERROR("Failed to remove virtual router name: %s, rv:%d", vrf_name.c_str(), status); + task_process_status handle_status = handleSaiRemoveStatus(SAI_API_VIRTUAL_ROUTER, status); + if (sync_mode) + { + ReturnCode rc = ReturnCode(status) + << "Failed to remove virtual router name: " + << vrf_name << ", rv:" << sai_serialize_status(status); + m_publisher.publish(request.getTableName(), request.getFullKey(), + request.getFullAttrFields(), rc); + return true; + } + else if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } } vrf_table_.erase(vrf_name); @@ -198,14 +271,29 @@ bool VRFOrch::delOperation(const Request& request) error = delVrfVNIMap(vrf_name, 0); if (error == false) { - return false; + if (sync_mode) + { + ReturnCode rc = ReturnCode(StatusCode::SWSS_RC_INVALID_PARAM) + << "Failed to delete VRF vni map."; + SWSS_LOG_ERROR("%s", rc.message().c_str()); + m_publisher.publish(request.getTableName(), request.getFullKey(), + request.getFullAttrFields(), rc); + return true; + } + else + { + return false; + } } m_stateVrfObjectTable.del(vrf_name); SWSS_LOG_NOTICE("VRF '%s' was removed", vrf_name.c_str()); - m_publisher.publish(request.getTableName(), request.getFullKey(), - request.getFullAttrFields(), ReturnCode()); + if (sync_mode) + { + m_publisher.publish(request.getTableName(), request.getFullKey(), + request.getFullAttrFields(), ReturnCode()); + } return true; } diff --git a/orchagent/vrforch.h b/orchagent/vrforch.h index 195015fa083..40f3f4cab81 100644 --- a/orchagent/vrforch.h +++ b/orchagent/vrforch.h @@ -9,6 +9,7 @@ struct VrfEntry { sai_object_id_t vrf_id; int ref_count; + bool sync_mode; }; struct VNIEntry @@ -34,7 +35,8 @@ const request_description_t request_description = { { "fallback", REQ_T_BOOL }, { "vni", REQ_T_UINT }, { "mgmtVrfEnabled", REQ_T_BOOL }, - { "in_band_mgmt_enabled", REQ_T_BOOL } + { "in_band_mgmt_enabled", REQ_T_BOOL }, + { "sync_mode", REQ_T_BOOL } }, { } // no mandatory attributes }; diff --git a/tests/test_vrf.py b/tests/test_vrf.py index ee8697811af..21613bc8c77 100644 --- a/tests/test_vrf.py +++ b/tests/test_vrf.py @@ -103,15 +103,10 @@ def vrf_create(self, dvs, vrf_name, attributes, expected_attributes): exp_attr[attributes[an][0]] = attributes[an][1] self.is_vrf_attributes_correct(self.pdb, "VRF_TABLE", vrf_name, exp_attr) - # check application state database + # check application state database to be empty in non-sync mode tbl = swsscommon.Table(self.sdb, "VRF_TABLE") intf_entries = tbl.getKeys() - assert len(intf_entries) == 1 - assert intf_entries[0] == vrf_name - self.is_vrf_attributes_correct(self.sdb, "VRF_TABLE", vrf_name, exp_attr) - - # check response channel - self.verify_response(self.response_consumer, vrf_name, exp_attr, "SWSS_RC_SUCCESS") + assert len(intf_entries) == 0 # check that the vrf entry was created assert self.how_many_entries_exist(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER") == 2, "The vrf wasn't created" @@ -144,14 +139,6 @@ def vrf_remove(self, dvs, vrf_name, state): intf_entries = tbl.getKeys() assert vrf_name not in intf_entries - # check application state database - tbl = swsscommon.Table(self.sdb, "VRF_TABLE") - intf_entries = tbl.getKeys() - assert vrf_name not in intf_entries - - # check response channel - self.verify_response(self.response_consumer, vrf_name, {}, "SWSS_RC_SUCCESS") - # check that the vrf entry was removed assert self.how_many_entries_exist(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER") == 1, "The vrf wasn't removed" @@ -172,12 +159,6 @@ def vrf_update(self, vrf_name, attributes, expected_attributes, state): exp_attr[attributes[an][0]] = attributes[an][1] self.is_vrf_attributes_correct(self.pdb, "VRF_TABLE", vrf_name, exp_attr) - # check application state database - self.is_vrf_attributes_correct(self.sdb, "VRF_TABLE", vrf_name, exp_attr) - - # check response channel - self.verify_response(self.response_consumer, vrf_name, exp_attr, "SWSS_RC_SUCCESS") - # check correctness of the created attributes self.is_vrf_attributes_correct( self.adb, @@ -328,10 +309,6 @@ def create_entry_tbl(self, db, table, key, pairs): intf_entries_cnt = self.how_many_entries_exist(self.pdb, "VRF_TABLE") assert intf_entries_cnt == maximum_vrf_cnt - # check app_state_db - intf_entries_cnt = self.how_many_entries_exist(self.sdb, "VRF_TABLE") - assert intf_entries_cnt == maximum_vrf_cnt - # check asic_db current_entries_cnt = self.how_many_entries_exist(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER") assert (current_entries_cnt - initial_entries_cnt) == maximum_vrf_cnt @@ -351,10 +328,6 @@ def create_entry_tbl(self, db, table, key, pairs): intf_entries_cnt = self.how_many_entries_exist(self.pdb, "VRF_TABLE") assert intf_entries_cnt == 0 - # check app_state_db - intf_entries_cnt = self.how_many_entries_exist(self.sdb, "VRF_TABLE") - assert intf_entries_cnt == 0 - # check asic_db current_entries_cnt = self.how_many_entries_exist(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER") assert (current_entries_cnt - initial_entries_cnt) == 0 @@ -363,7 +336,63 @@ def create_entry_tbl(self, db, table, key, pairs): (exitcode, num) = dvs.runcmd(['sh', '-c', "ip link show | grep Vrf | wc -l"]) assert num.strip() == '0' - def test_VRFMgr_Error(self, dvs, testlog): + def test_VRFMgr_SyncMode(self, dvs, testlog): + self.setup_db(dvs) + assert self.how_many_entries_exist(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER") == 1, "The initial state is incorrect" + + # create VRF in APPL DB + appl_tbl = swsscommon.ProducerStateTable(self.pdb, "VRF_TABLE") + vrf_name = "Vrf-sync_mode" + attributes = [ + ('v4', 'true'), + ('v6', 'false'), + ('ttl_action', 'trap'), + ('ip_opt_action', 'trap'), + ('l3_mc_action', 'drop'), + ('sync_mode', 'true'), + ] + exp_attr = {} + for an in range(len(attributes)): + exp_attr[attributes[an][0]] = attributes[an][1] + appl_tbl.set(vrf_name, attributes) + time.sleep(1) + + # check application database + tbl = swsscommon.Table(self.pdb, "VRF_TABLE") + intf_entries = tbl.getKeys() + assert len(intf_entries) == 1 + assert intf_entries[0] == vrf_name + self.is_vrf_attributes_correct(self.pdb, "VRF_TABLE", vrf_name, exp_attr) + + # check application state database + tbl = swsscommon.Table(self.sdb, "VRF_TABLE") + intf_entries = tbl.getKeys() + assert len(intf_entries) == 1 + assert intf_entries[0] == vrf_name + self.is_vrf_attributes_correct(self.sdb, "VRF_TABLE", vrf_name, exp_attr) + + # check response channel + self.verify_response(self.response_consumer, vrf_name, exp_attr, "SWSS_RC_SUCCESS") + + # check ASIC DB + assert self.how_many_entries_exist(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER") == 2, "The vrf wasn't created" + + # Delete VRF in APPL DB + appl_tbl._del(vrf_name) + time.sleep(1) + + # check application state database + tbl = swsscommon.Table(self.sdb, "VRF_TABLE") + intf_entries = tbl.getKeys() + assert len(intf_entries) == 0 + + # check response channel + self.verify_response(self.response_consumer, vrf_name, {}, "SWSS_RC_SUCCESS") + + # check ASIC DB + assert self.how_many_entries_exist(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER") == 1, "The vrf wasn't deleted" + + def test_VRFMgr_SyncModeError(self, dvs, testlog): self.setup_db(dvs) assert self.how_many_entries_exist(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER") == 1, "The initial state is incorrect" @@ -376,12 +405,14 @@ def test_VRFMgr_Error(self, dvs, testlog): ('ttl_action', 'trap'), ('ip_opt_action', 'trap'), ('l3_mc_action', 'drop'), + ('sync_mode' , 'true'), ('invalid', 'true'), ] exp_attr = {} for an in range(len(attributes)): exp_attr[attributes[an][0]] = attributes[an][1] tbl.set(vrf_name, attributes) + time.sleep(1) # check application database tbl = swsscommon.Table(self.pdb, "VRF_TABLE")