From 1b9843d959256b346c743ee71a96680860eac44f Mon Sep 17 00:00:00 2001 From: Tom cappleman Date: Fri, 15 Oct 2021 17:19:08 +0100 Subject: [PATCH] [cbf] Added initial CBF support (#2) * Added CBF NHG support to NhgOrch * Added NhgMapOrch to handle DSCP_TO_FC and EXP_TO_FC tables * Added UT for the new NhgOrch function and NhgMapOrch Signed-off-by: Alexandru Banu --- doc/Configuration.md | 2 + doc/swss-schema.md | 57 ++ orchagent/Makefile.am | 5 +- orchagent/cbfnhghandler.cpp | 788 +++++++++++++++ orchagent/cbfnhghandler.h | 96 ++ orchagent/mplsrouteorch.cpp | 8 +- orchagent/neighorch.cpp | 4 +- orchagent/nhgbase.cpp | 45 + orchagent/nhgbase.h | 465 +++++++++ orchagent/{nhgorch.cpp => nhghandler.cpp} | 368 ++----- orchagent/nhghandler.h | 116 +++ orchagent/nhgmaporch.cpp | 366 +++++++ orchagent/nhgmaporch.h | 61 ++ orchagent/nhgorch.h | 309 ++---- orchagent/orchdaemon.cpp | 13 +- orchagent/orchdaemon.h | 1 + orchagent/qosorch.cpp | 192 +++- orchagent/qosorch.h | 18 + orchagent/routeorch.cpp | 12 +- tests/mock_tests/Makefile.am | 5 +- tests/test_nhg.py | 1086 +++++++++++++++++---- tests/test_qos_map.py | 155 ++- 22 files changed, 3497 insertions(+), 675 deletions(-) create mode 100644 orchagent/cbfnhghandler.cpp create mode 100644 orchagent/cbfnhghandler.h create mode 100644 orchagent/nhgbase.cpp create mode 100644 orchagent/nhgbase.h rename orchagent/{nhgorch.cpp => nhghandler.cpp} (71%) create mode 100644 orchagent/nhghandler.h create mode 100644 orchagent/nhgmaporch.cpp create mode 100644 orchagent/nhgmaporch.h diff --git a/doc/Configuration.md b/doc/Configuration.md index 0c91e16db5..039e749d37 100644 --- a/doc/Configuration.md +++ b/doc/Configuration.md @@ -1129,6 +1129,8 @@ name as object key and member list as attribute. "pfc_enable": "3,4", "pfc_to_queue_map": "AZURE", "dscp_to_tc_map": "AZURE", + "dscp_to_fc_map": "AZURE", + "exp_to_fc_map": "AZURE", "scheduler": "scheduler.port" } } diff --git a/doc/swss-schema.md b/doc/swss-schema.md index 47fc9cd32c..ca109a4ec2 100644 --- a/doc/swss-schema.md +++ b/doc/swss-schema.md @@ -192,6 +192,28 @@ and reflects the LAG ports into the redis under: `LAG_TABLE::port` mpls_nh = STRING ; Comma-separated list of MPLS NH info. weight = weight_list ; List of weights. +--------------------------------------------- +### CLASS_BASED_NEXT_HOP_GROUP_TABLE + ;Stores a list of groups of one or more next hop groups used for class based forwarding + ;Status: Mandatory + key = CLASS_BASED_NEXT_HOP_GROUP_TABLE:string ; arbitrary index for the next hop group + members = NEXT_HOP_GROUP_TABLE.key ; one or more separated by "," + selection_map = FC_TO_NHG_INDEX_MAP_TABLE.key ; the NHG map to use for this CBF NHG + +--------------------------------------------- +### FC_TO_NHG_INDEX_MAP_TABLE + ; FC to Next hop group index map + key = "FC_TO_NHG_INDEX_MAP_TABLE:"name + fc_num = 1*DIGIT ;value + nh_index = 1*DIGIT; index of NH inside NH group + + Example: + 127.0.0.1:6379> hgetall "FC_TO_NHG_INDEX_MAP_TABLE:AZURE" + 1) "0" ;fc_num + 2) "0" ;nhg_index + 3) "1" + 4) "0" + --------------------------------------------- ### NEIGH_TABLE ; Stores the neighbors or next hop IP address and output port or @@ -304,6 +326,41 @@ and reflects the LAG ports into the redis under: `LAG_TABLE::port` 9) "4" 10) "8" +### DSCP_TO_FC_TABLE_NAME + ; dscp to FC map + ;SAI mapping - qos_map object with SAI_QOS_MAP_ATTR_TYPE == sai_qos_map_type_t::SAI_QOS_MAP_TYPE_DSCP_TO_FORWARDING_CLASS + key = "DSCP_TO_FC_MAP_TABLE:"name + ;field value + dscp_value = 1*DIGIT + fc_value = 1*DIGIT + + Example: + 127.0.0.1:6379> hgetall "DSCP_TO_FC_MAP_TABLE:AZURE" + 1) "0" ;dscp + 2) "1" ;fc + 3) "1" + 4) "1" + 5) "2" + 6) "3" + 7) +--------------------------------------------- +### EXP_TO_FC_MAP_TABLE + ; dscp to FC map + ;SAI mapping - qos_map object with SAI_QOS_MAP_ATTR_TYPE == sai_qos_map_type_t::SAI_QOS_MAP_TYPE_MPLS_EXP_TO_FORWARDING_CLASS + key = "EXP_TO_FC_MAP_TABLE:"name + ;field value + mpls_exp_value = 1*DIGIT + fc_value = 1*DIGIT + + Example: + 127.0.0.1:6379> hgetall "EXP_TO_FC_MAP_TABLE:AZURE" + 1) "0" ;mpls_exp + 2) "1" ;fc + 3) "1" + 4) "1" + 5) "2" + 6) "3" + --------------------------------------------- ### SCHEDULER_TABLE ; Scheduler table diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index bc58bbbbf9..1482127145 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -40,7 +40,10 @@ orchagent_SOURCES = \ orchdaemon.cpp \ orch.cpp \ notifications.cpp \ - nhgorch.cpp \ + nhgbase.cpp \ + nhghandler.cpp \ + cbfnhghandler.cpp \ + nhgmaporch.cpp \ routeorch.cpp \ mplsrouteorch.cpp \ neighorch.cpp \ diff --git a/orchagent/cbfnhghandler.cpp b/orchagent/cbfnhghandler.cpp new file mode 100644 index 0000000000..621895a36d --- /dev/null +++ b/orchagent/cbfnhghandler.cpp @@ -0,0 +1,788 @@ +#include "cbfnhghandler.h" +#include "crmorch.h" +#include "bulker.h" +#include "tokenize.h" +#include "nhgorch.h" +#include "nhgmaporch.h" +#include "routeorch.h" + +extern sai_object_id_t gSwitchId; + +extern NhgOrch *gNhgOrch; +extern CrmOrch *gCrmOrch; +extern NhgMapOrch *gNhgMapOrch; +extern RouteOrch *gRouteOrch; + +extern sai_next_hop_group_api_t* sai_next_hop_group_api; + +extern size_t gMaxBulkSize; + +/* + * Purpose: Perform the operations requested by APPL_DB users. + * + * Description: Iterate over the untreated operations list and resolve them. + * The operations supported are SET and DEL. If an operation + * could not be resolved, it will either remain in the list, or be + * removed, depending on the case. + * + * Params: IN consumer - The cosumer object. + * + * Returns: Nothing. + */ +void CbfNhgHandler::doTask(Consumer& consumer) +{ + SWSS_LOG_ENTER(); + + auto it = consumer.m_toSync.begin(); + + while (it != consumer.m_toSync.end()) + { + swss::KeyOpFieldsValuesTuple t = it->second; + + string index = kfvKey(t); + string op = kfvOp(t); + + bool success; + const auto &cbf_nhg_it = m_syncedNextHopGroups.find(index); + + if (op == SET_COMMAND) + { + string members; + string selection_map; + + /* + * Get CBF group's members and selection map. + */ + for (const auto &i : kfvFieldsValues(t)) + { + if (fvField(i) == "members") + { + members = fvValue(i); + } + else if (fvField(i) == "selection_map") + { + selection_map = fvValue(i); + } + } + + /* + * Validate the data. + */ + auto p = getMembers(members); + + if (!p.first) + { + SWSS_LOG_ERROR("CBF next hop group %s data is invalid.", + index.c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + + /* + * If the CBF group does not exist, create it. + */ + if (cbf_nhg_it == m_syncedNextHopGroups.end()) + { + /* + * If we reached the NHG limit, postpone the creation. + */ + if (gRouteOrch->getNhgCount() + NhgBase::getSyncedCount() >= gRouteOrch->getMaxNhgCount()) + { + SWSS_LOG_WARN("Reached next hop group limit. Postponing creation."); + success = false; + } + else + { + auto cbf_nhg = CbfNhg(index, p.second, selection_map); + success = cbf_nhg.sync(); + + if (success) + { + /* + * If the CBF NHG contains temporary NHGs as members, + * we have to keep checking for updates. + */ + if (cbf_nhg.hasTemps()) + { + success = false; + } + + m_syncedNextHopGroups.emplace(index, NhgEntry(move(cbf_nhg))); + } + } + } + /* + * If the CBF group exists, update it. + */ + else + { + success = cbf_nhg_it->second.nhg.update(p.second, selection_map); + + /* + * If the CBF NHG has temporary NHGs synced, we need to keep + * checking this group in case they are promoted. + */ + if (cbf_nhg_it->second.nhg.hasTemps()) + { + success = false; + } + } + } + else if (op == DEL_COMMAND) + { + /* + * If there is a pending SET after this DEL operation, skip the + * DEL operation to perform the update instead. Otherwise, in the + * scenario where the DEL operation may be blocked by the ref + * counter, we'd end up deleting the object after the SET operation + * is performed, which would not reflect the desired state of the + * object. + */ + if (consumer.m_toSync.count(it->first) > 1) + { + success = true; + } + /* If the group doesn't exist, do nothing. */ + else if (cbf_nhg_it == m_syncedNextHopGroups.end()) + { + SWSS_LOG_WARN("Deleting inexistent CBF NHG %s", index.c_str()); + /* + * Mark it as a success to remove the task from the consumer. + */ + success = true; + } + /* If the group does exist but is still referenced, skip.*/ + else if (cbf_nhg_it->second.ref_count > 0) + { + SWSS_LOG_WARN("Skipping removal of CBF next hop group %s which" + " is still referenced", index.c_str()); + success = false; + } + /* Otherwise, delete it. */ + else + { + success = cbf_nhg_it->second.nhg.remove(); + + if (success) + { + m_syncedNextHopGroups.erase(cbf_nhg_it); + } + } + } + else + { + SWSS_LOG_WARN("Unknown operation type %s", op.c_str()); + /* Mark the operation as successful to consume it. */ + success = true; + } + + /* Depending on the operation success, consume it or skip it. */ + if (success) + { + it = consumer.m_toSync.erase(it); + } + else + { + ++it; + } + } +} + +/* + * Purpose: Validate the CBF members. + * + * Params: IN members - The members string to validate. + * + * Returns: Pair where: + * - the first element is a bool, representing if the members are + * valid or not + * - the second element is a vector of members + */ +pair> CbfNhgHandler::getMembers(const string &members) +{ + SWSS_LOG_ENTER(); + + auto members_vec = swss::tokenize(members, ','); + set members_set(members_vec.begin(), members_vec.end()); + bool success = true; + + /* + * Verify that the members list is not empty + */ + if (members_set.empty()) + { + SWSS_LOG_ERROR("CBF next hop group members list is empty."); + success = false; + } + /* + * Verify that the members are unique. + */ + else if (members_set.size() != members_vec.size()) + { + SWSS_LOG_ERROR("CBF next hop group members are not unique."); + success = false; + } + + return {success, members_vec}; +} + +/* + * Purpose: Constructor. + * + * Params: IN index - The index of the CBF NHG. + * IN members - The members of the CBF NHG. + * IN selection_map - The selection map index of the CBF NHG. + * + * Returns: Nothing. + */ +CbfNhg::CbfNhg(const string &index, + const vector &members, + const string &selection_map) : + NhgCommon(index), + m_selection_map(selection_map) +{ + SWSS_LOG_ENTER(); + + uint8_t idx = 0; + for (const auto &member : members) + { + m_members.emplace(member, CbfNhgMember(member, idx++)); + } +} + +/* + * Purpose: Move constructor. + * + * Params: IN cbf_nhg - The temporary object to construct from. + * + * Returns: Nothing. + */ +CbfNhg::CbfNhg(CbfNhg &&cbf_nhg) : + NhgCommon(move(cbf_nhg)), + m_selection_map(move(cbf_nhg.m_selection_map)), + m_temp_nhgs(move(cbf_nhg.m_temp_nhgs)) +{ + SWSS_LOG_ENTER(); +} + +/* + * Purpose: Sync the CBF NHG over SAI, getting a SAI ID. + * + * Params: None. + * + * Returns: true, if the operation was successful, + * false, otherwise. + */ +bool CbfNhg::sync() +{ + SWSS_LOG_ENTER(); + + /* If the group is already synced, exit. */ + if (isSynced()) + { + return true; + } + + /* Create the CBF next hop group over SAI. */ + sai_attribute_t nhg_attr; + vector nhg_attrs; + + nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_TYPE; + nhg_attr.value.u32 = SAI_NEXT_HOP_GROUP_TYPE_CLASS_BASED; + nhg_attrs.push_back(move(nhg_attr)); + + /* Add the number of members. */ + nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_CONFIGURED_SIZE; + assert(m_members.size() <= UINT32_MAX); + nhg_attr.value.u32 = static_cast(m_members.size()); + nhg_attrs.push_back(move(nhg_attr)); + + /* Add the selection map to the attributes. */ + nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP; + nhg_attr.value.oid = gNhgMapOrch->getMapId(m_selection_map); + + if (nhg_attr.value.oid == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_WARN("FC to NHG map index %s does not exist", m_selection_map.c_str()); + return false; + } + + nhg_attrs.push_back(move(nhg_attr)); + + auto status = sai_next_hop_group_api->create_next_hop_group( + &m_id, + gSwitchId, + static_cast(nhg_attrs.size()), + nhg_attrs.data()); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create CBF next hop group %s, rv %d", + m_key.c_str(), + status); + task_process_status handle_status = gNhgOrch->handleSaiCreateStatus(SAI_API_NEXT_HOP_GROUP, status); + if (handle_status != task_success) + { + return gNhgOrch->parseHandleSaiStatusFailure(handle_status); + } + } + + /* Increase the reference counter for the selection map. */ + gNhgMapOrch->incRefCount(m_selection_map); + + /* + * Increment the amount of programmed next hop groups. */ + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); + incSyncedCount(); + + /* Sync the group members. */ + set members; + + for (const auto &member : m_members) + { + members.insert(member.first); + } + + if (!syncMembers(members)) + { + SWSS_LOG_ERROR("Failed to sync CBF next hop group %s", m_key.c_str()); + return false; + } + + return true; +} + +/* + * Purpose: Remove the CBF NHG over SAI, removing the SAI ID. + * + * Params: None. + * + * Returns: true, if the operation was successful, + * false, otherwise. + */ +bool CbfNhg::remove() +{ + SWSS_LOG_ENTER(); + + bool is_synced = isSynced(); + + bool success = NhgCommon::remove(); + + if (success && is_synced) + { + gNhgMapOrch->decRefCount(m_selection_map); + } + + return success; +} + +/* + * Purpose: Check if the CBF NHG has the same members and in the same order as + * the ones given. + * + * Params: IN members - The members to compare with. + * + * Returns: true, if the current members are the same with the given one, + * false, otherwise. + */ +bool CbfNhg::hasSameMembers(const vector &members) const +{ + SWSS_LOG_ENTER(); + + /* The size should be the same. */ + if (m_members.size() != members.size()) + { + return false; + } + + /* + * Check that the members are the same and the index is preserved. + */ + uint8_t index = 0; + + for (const auto &member : members) + { + auto mbr_it = m_members.find(member); + + if (mbr_it == m_members.end()) + { + return false; + } + + if (mbr_it->second.getIndex() != index++) + { + return false; + } + } + + return true; +} + +/* + * Purpose: Update a CBF next hop group. + * + * Params: IN members - The new members. + * IN selection_map - The new selection map. + * + * Returns: true, if the update was successful, + * false, otherwise. + */ +bool CbfNhg::update(const vector &members, const string &selection_map) +{ + SWSS_LOG_ENTER(); + + /* + * Check if we're just checking if the temporary NHG members were updated. + * This would happen only if the given members are the same with the + * existing ones and in the same order. In this scenario, we'll update + * just the NEXT_HOP attribute of the temporary members if necessary. + */ + if (!m_temp_nhgs.empty() && hasSameMembers(members)) + { + /* Iterate over the temporary NHGs and check if they were updated. */ + for (auto member = m_temp_nhgs.begin(); member != m_temp_nhgs.end();) + { + const auto &nhg = gNhgOrch->getNhg(member->first); + + /* + * If the NHG ID has changed since storing the first occurence, + * we have to update the CBF NHG member's attribute. + */ + if (nhg.getId() != member->second) + { + if (!m_members.at(member->first).updateNhAttr()) + { + SWSS_LOG_ERROR("Failed to update temporary next hop group " + "member %s of CBF next hop group %s", + member->first.c_str(), m_key.c_str()); + return false; + } + + /* If the NHG was promoted, remove it from the temporary NHG map. */ + if (!nhg.isTemp()) + { + member = m_temp_nhgs.erase(member); + } + /* + * If the NHG was just updated, update its current NHG ID value + * in the map. + */ + else + { + member->second = nhg.getId(); + ++member; + } + } + else + { + ++member; + } + } + } + /* If the members are different, update the whole members list. */ + else + { + /* + * Because the INDEX attribute is CREATE_ONLY, we have to remove all + * the existing members and sync the new ones back as the order of the + * members can change due to some of them being removed, which would + * invalidate all the other members following them, or inserting a new + * one somewhere in the front of the existing members which would also + * invalidate them. + */ + set members_set; + + for (const auto &member : m_members) + { + members_set.insert(member.first); + } + + /* Remove the existing members. */ + if (!removeMembers(members_set)) + { + SWSS_LOG_ERROR("Failed to remove members of CBF next hop group %s", + m_key.c_str()); + return false; + } + m_members.clear(); + m_temp_nhgs.clear(); + + /* Add the new members. */ + uint8_t index = 0; + for (const auto &member : members) + { + m_members.emplace(member, CbfNhgMember(member, index++)); + } + + /* Sync the new members. */ + if (!syncMembers({members.begin(), members.end()})) + { + SWSS_LOG_ERROR("Failed to sync members of CBF next hop group %s", + m_key.c_str()); + return false; + } + } + + /* Update the group map. */ + if (m_selection_map != selection_map) + { + sai_attribute_t nhg_attr; + nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP; + nhg_attr.value.oid = gNhgMapOrch->getMapId(selection_map); + + if (nhg_attr.value.oid == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_WARN("NHG map %s does not exist", selection_map.c_str()); + return false; + } + + auto status = sai_next_hop_group_api->set_next_hop_group_attribute( m_id, &nhg_attr); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to update CBF next hop group %s, rv %d", + m_key.c_str(), + status); + return false; + } + + /* Update the selection map and update the previous and new map ref count. */ + gNhgMapOrch->decRefCount(m_selection_map); + m_selection_map = selection_map; + gNhgMapOrch->incRefCount(m_selection_map); + } + + return true; +} + +/* + * Purpose: Sync the given CBF group members. + * + * Params: IN members - The members to sync. + * + * Returns: true, if the operation was successful, + * false, otherwise. + */ +bool CbfNhg::syncMembers(const set &members) +{ + SWSS_LOG_ENTER(); + + /* The group should be synced at this point. */ + if (!isSynced()) + { + SWSS_LOG_ERROR("Trying to sync members of CBF next hop group %s which is not synced", + m_key.c_str()); + throw logic_error("Syncing members of unsynced CBF next hop group"); + } + + /* + * Sync all the given members. If a NHG does not exist, is not yet synced + * or is temporary, stop immediately. + */ + ObjectBulker bulker(sai_next_hop_group_api, + gSwitchId, + gMaxBulkSize); + unordered_map nhgm_ids; + + for (const auto &key : members) + { + auto &nhgm = m_members.at(key); + + /* A next hop group member can't be already synced if it was passed into the syncMembers() method. */ + if (nhgm.isSynced()) + { + SWSS_LOG_ERROR("Trying to sync already synced CBF NHG member %s in group %s", + nhgm.to_string().c_str(), to_string().c_str()); + throw std::logic_error("Syncing already synced NHG member"); + } + + /* + * Check if the group exists in NhgOrch. + */ + if (!gNhgOrch->nhgHandler.hasNhg(key)) + { + SWSS_LOG_ERROR("Next hop group %s in CBF next hop group %s does " + "not exist", key.c_str(), m_key.c_str()); + return false; + } + + const auto &nhg = gNhgOrch->nhgHandler.getNhg(key); + + /* + * Check if the group is synced. + */ + if (!nhg.isSynced()) + { + SWSS_LOG_ERROR("Next hop group %s in CBF next hop group %s is not" + " synced", + key.c_str(), m_key.c_str()); + return false; + } + + /* Create the SAI attributes for syncing the NHG as a member. */ + auto attrs = createNhgmAttrs(nhgm); + + bulker.create_entry(&nhgm_ids[key], + (uint32_t)attrs.size(), + attrs.data()); + } + + /* Flush the bulker to perform the sync. */ + bulker.flush(); + + /* Iterate over the synced members and set their SAI ID. */ + bool success = true; + + for (const auto &member : nhgm_ids) + { + if (member.second == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_ERROR("Failed to create CBF next hop group %s member %s", + m_key.c_str(), member.first.c_str()); + success = false; + } + else + { + m_members.at(member.first).sync(member.second); + + const auto &nhg = gNhgOrch->getNhg(member.first); + /* + * If the member is temporary, store it in order to check if it is + * promoted at some point. + */ + if (nhg.isTemp()) + { + m_temp_nhgs.emplace(member.first, nhg.getId()); + } + } + } + + return success; +} + +/* + * Purpose: Create a vector with the SAI attributes for syncing a next hop + * group member over SAI. The caller is reponsible of filling in the + * index attribute. + * + * Params: IN nhgm - The next hop group member to sync. + * + * Returns: The vector containing the SAI attributes. + */ +vector CbfNhg::createNhgmAttrs(const CbfNhgMember &nhgm) const +{ + SWSS_LOG_ENTER(); + + if (!isSynced() || (nhgm.getNhgId() == SAI_NULL_OBJECT_ID)) + { + SWSS_LOG_ERROR("CBF next hop group %s or next hop group %s are not synced", + to_string().c_str(), nhgm.to_string().c_str()); + throw logic_error("CBF next hop group member attributes data is insufficient"); + } + + vector attrs; + sai_attribute_t attr; + + /* Fill in the group ID. */ + attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID; + attr.value.oid = m_id; + attrs.push_back(attr); + + /* Fill in the next hop ID. */ + attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID; + attr.value.oid = nhgm.getNhgId(); + attrs.push_back(attr); + + /* Fill in the index. */ + attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_INDEX; + attr.value.oid = nhgm.getIndex(); + attrs.push_back(attr); + + return attrs; +} + +/* + * Purpose: Sync the member, setting its SAI ID and incrementing the necessary + * ref counters. + * + * Params: IN gm_id - The SAI ID to set. + * + * Returns: Nothing. + */ +void CbfNhgMember::sync(sai_object_id_t gm_id) +{ + SWSS_LOG_ENTER(); + + NhgMember::sync(gm_id); + gNhgOrch->nhgHandler.incNhgRefCount(m_key); +} + +/* + * Purpose: Update the next hop attribute of the member. + * + * Params: None. + * + * Returns: true, if the operation was successful, + * false, otherwise. + */ +bool CbfNhgMember::updateNhAttr() +{ + SWSS_LOG_ENTER(); + + if (!isSynced()) + { + SWSS_LOG_ERROR("Trying to update next hop attribute of CBF NHG member %s that is not synced", + to_string().c_str()); + throw logic_error("Trying to update attribute of unsynced object."); + } + + /* + * Fill in the attribute. + */ + sai_attribute_t attr; + attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID; + attr.value.oid = getNhgId(); + + /* + * Set the attribute over SAI. + */ + auto status = sai_next_hop_group_api->set_next_hop_group_member_attribute(m_id, &attr); + + return status == SAI_STATUS_SUCCESS; +} + +/* + * Purpose: Remove the member, reseting its SAI ID and decrementing the NHG ref + * counter. + * + * Params: None. + * + * Returns: Nothing. + */ +void CbfNhgMember::remove() +{ + SWSS_LOG_ENTER(); + + NhgMember::remove(); + gNhgOrch->nhgHandler.decNhgRefCount(m_key); +} + +/* + * Purpose: Get the NHG ID of this member. + * + * Params: None. + * + * Returns: The SAI ID of the NHG it references or SAI_NULL_OBJECT_ID if it + * doesn't exist. + */ +sai_object_id_t CbfNhgMember::getNhgId() const +{ + SWSS_LOG_ENTER(); + + if (!gNhgOrch->hasNhg(m_key)) + { + return SAI_NULL_OBJECT_ID; + } + + return gNhgOrch->getNhg(m_key).getId(); +} diff --git a/orchagent/cbfnhghandler.h b/orchagent/cbfnhghandler.h new file mode 100644 index 0000000000..af2dbb30af --- /dev/null +++ b/orchagent/cbfnhghandler.h @@ -0,0 +1,96 @@ +#pragma once + +#include "nhgbase.h" +#include "nhghandler.h" + +using namespace std; + +class CbfNhgMember : public NhgMember +{ +public: + CbfNhgMember(const string &key, uint8_t index) : + NhgMember(key), m_index(index) { SWSS_LOG_ENTER(); } + + /* Sync the member, setting its SAI ID and incrementing the necessary ref counters. */ + void sync(sai_object_id_t gm_id) override; + + /* Remove the member, reseting its SAI ID and decrementing the necessary ref counters. */ + void remove() override; + + /* Get the NHG ID of this member. */ + sai_object_id_t getNhgId() const; + + /* Update the NEXT_HOP attribute of this member. */ + bool updateNhAttr(); + + /* Get the index of this group member. */ + uint8_t getIndex() const { return m_index; } + + /* Get a string representation of this member. */ + string to_string() const override { return m_key; } + +private: + /* The index of this member in the group's member list. */ + uint8_t m_index; +}; + +class CbfNhg : public NhgCommon +{ +public: + /* Constructors. */ + CbfNhg(const string &index, + const vector &members, + const string &selection_map); + CbfNhg(CbfNhg &&cbf_nhg); + + /* Destructor. */ + ~CbfNhg() { SWSS_LOG_ENTER(); remove(); } + + /* Create the CBF group over SAI. */ + bool sync() override; + + /* Remove the CBF group over SAI. */ + bool remove() override; + + /* CBF groups can never be temporary. */ + inline bool isTemp() const override { SWSS_LOG_ENTER(); return false; } + + /* Check if the CBF next hop group contains synced temporary NHGs. */ + inline bool hasTemps() const + { SWSS_LOG_ENTER(); return !m_temp_nhgs.empty(); } + + /* CBF groups do not have a NextHopGroupkey. */ + inline NextHopGroupKey getNhgKey() const override { return {}; } + + /* Update the CBF group, including the SAI programming. */ + bool update(const vector &members, const string &selection_map); + + string to_string() const override { return m_key; } + +private: + string m_selection_map; + + /* + * Map of synced temporary NHGs contained in this next hop group along with + * the NHG ID at the time of sync. + */ + unordered_map m_temp_nhgs; + + /* Sync the given members over SAI. */ + bool syncMembers(const set &members) override; + + /* Get the SAI attributes for creating the members over SAI. */ + vector + createNhgmAttrs(const CbfNhgMember &nhg) const override; + /* Check if the CBF NHG has the same members and in the same order as the ones given. */ + bool hasSameMembers(const vector &members) const; +}; + +class CbfNhgHandler : public NhgHandlerCommon +{ +public: + void doTask(Consumer &consumer); + +private: + static pair> getMembers(const string &members); +}; diff --git a/orchagent/mplsrouteorch.cpp b/orchagent/mplsrouteorch.cpp index 5aebf70cbe..86b7eea75e 100644 --- a/orchagent/mplsrouteorch.cpp +++ b/orchagent/mplsrouteorch.cpp @@ -253,8 +253,8 @@ void RouteOrch::doLabelTask(Consumer& consumer) { try { - const NextHopGroup& nh_group = gNhgOrch->getNhg(nhg_index); - ctx.nhg = nh_group.getKey(); + const auto &nh_group = gNhgOrch->getNhg(nhg_index); + ctx.nhg = nh_group.getNhgKey(); ctx.using_temp_nhg = nh_group.isTemp(); } catch (const std::out_of_range& e) @@ -308,7 +308,7 @@ void RouteOrch::doLabelTask(Consumer& consumer) // If already exhaust the nexthop groups, and there are pending removing routes in bulker, // flush the bulker and possibly collect some released nexthop groups - if (gNhgOrch->getNhgCount() >= gNhgOrch->getMaxNhgCount() && + if (m_nextHopGroupCount + gNhgOrch->getSyncedNhgCount() >= m_maxNextHopGroupCount && gLabelRouteBulker.removing_entries_count() > 0) { break; @@ -478,7 +478,7 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey { try { - const NextHopGroup& nhg = gNhgOrch->getNhg(ctx.nhg_index); + const auto &nhg = gNhgOrch->getNhg(ctx.nhg_index); next_hop_id = nhg.getId(); } catch(const std::out_of_range& e) diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 23d7a38b3b..a899c1450c 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -334,7 +334,7 @@ bool NeighOrch::setNextHopFlag(const NextHopKey &nexthop, const uint32_t nh_flag { case NHFLAGS_IFDOWN: rc = gRouteOrch->invalidnexthopinNextHopGroup(nexthop, count); - rc &= gNhgOrch->invalidateNextHop(nexthop); + rc &= gNhgOrch->nhgHandler.invalidateNextHop(nexthop); break; default: assert(0); @@ -364,7 +364,7 @@ bool NeighOrch::clearNextHopFlag(const NextHopKey &nexthop, const uint32_t nh_fl { case NHFLAGS_IFDOWN: rc = gRouteOrch->validnexthopinNextHopGroup(nexthop, count); - rc &= gNhgOrch->validateNextHop(nexthop); + rc &= gNhgOrch->nhgHandler.validateNextHop(nexthop); break; default: assert(0); diff --git a/orchagent/nhgbase.cpp b/orchagent/nhgbase.cpp new file mode 100644 index 0000000000..390d38ccfb --- /dev/null +++ b/orchagent/nhgbase.cpp @@ -0,0 +1,45 @@ +#include "nhgbase.h" +#include "vector" +#include "rediscommand.h" + +extern sai_object_id_t gSwitchId; + +unsigned NhgBase::m_syncedCount = 0; + +/* + * Purpose: Destructor. + * + * Params: None. + * + * Returns: Nothing. + */ +NhgBase::~NhgBase() +{ + SWSS_LOG_ENTER(); + + if (isSynced()) + { + SWSS_LOG_ERROR("Destroying next hop group with SAI ID %lu which is still synced.", m_id); + assert(false); + } +} + +/* + * Purpose: Decrease the count of synced next hop group objects. + * + * Params: None. + * + * Returns: Nothing. + */ +void NhgBase::decSyncedCount() +{ + SWSS_LOG_ENTER(); + + if (m_syncedCount == 0) + { + SWSS_LOG_ERROR("Decreasing next hop groups count while already 0"); + throw logic_error("Decreasing next hop groups count while already 0"); + } + + --m_syncedCount; +} diff --git a/orchagent/nhgbase.h b/orchagent/nhgbase.h new file mode 100644 index 0000000000..f718b76f1a --- /dev/null +++ b/orchagent/nhgbase.h @@ -0,0 +1,465 @@ +#pragma once + +#include "string" +#include "logger.h" +#include "saitypes.h" +#include "unordered_map" +#include "dbconnector.h" +#include "set" +#include "orch.h" +#include "crmorch.h" +#include "nexthopgroupkey.h" +#include "bulker.h" + +using namespace std; + +extern sai_object_id_t gSwitchId; + +extern CrmOrch *gCrmOrch; + +extern sai_next_hop_group_api_t* sai_next_hop_group_api; +extern size_t gMaxBulkSize; + +/* + * Base class for next hop groups, containing the common interface that every + * next hop group should have based on what RouteOrch needs when working with + * next hop groups. + */ +class NhgBase +{ +public: + NhgBase() : m_id(SAI_NULL_OBJECT_ID) { SWSS_LOG_ENTER(); } + + NhgBase(NhgBase &&nhg) : m_id(nhg.m_id) + { SWSS_LOG_ENTER(); nhg.m_id = SAI_NULL_OBJECT_ID; } + + NhgBase& operator=(NhgBase &&nhg) + { SWSS_LOG_ENTER(); swap(m_id, nhg.m_id); return *this; } + + /* + * Prevent copying. + */ + NhgBase(const NhgBase&) = delete; + void operator=(const NhgBase&) = delete; + + virtual ~NhgBase(); + + /* + * Getters. + */ + inline sai_object_id_t getId() const { SWSS_LOG_ENTER(); return m_id; } + static inline unsigned getSyncedCount() + { SWSS_LOG_ENTER(); return m_syncedCount; } + + /* + * Check if the next hop group is synced or not. + */ + inline bool isSynced() const { return m_id != SAI_NULL_OBJECT_ID; } + + /* + * Check if the next hop group is temporary. + */ + virtual bool isTemp() const = 0; + + /* + * Get the NextHopGroupKey of this object. + */ + virtual NextHopGroupKey getNhgKey() const = 0; + + /* Increment the number of existing groups. */ + static inline void incSyncedCount() { SWSS_LOG_ENTER(); ++m_syncedCount; } + + /* Decrement the number of existing groups. */ + static void decSyncedCount(); + +protected: + /* + * The SAI ID of this object. + */ + sai_object_id_t m_id; + + /* + * Number of synced NHGs. Incremented when an object is synced and + * decremented when an object is removed. This will also account for the + * groups created by RouteOrch. + */ + static unsigned m_syncedCount; +}; + +/* + * NhgMember class representing the common templated base class between + * WeightedNhgMember and CbfNhgMember classes. + */ +template +class NhgMember +{ +public: + explicit NhgMember(const Key &key) : + m_key(key), m_id(SAI_NULL_OBJECT_ID) { SWSS_LOG_ENTER(); } + + NhgMember(NhgMember &&nhgm) : m_key(move(nhgm.m_key)), m_id(nhgm.m_id) + { SWSS_LOG_ENTER(); nhgm.m_id = SAI_NULL_OBJECT_ID; } + + NhgMember& operator=(NhgMember &&nhgm) + { + SWSS_LOG_ENTER(); + + swap(m_key, nhgm.m_key); + swap(m_id, nhgm.m_id); + + return *this; + } + + /* + * Prevent copying. + */ + NhgMember(const NhgMember&) = delete; + void operator=(const NhgMember&) = delete; + + virtual ~NhgMember() + { + SWSS_LOG_ENTER(); + + if (isSynced()) + { + SWSS_LOG_ERROR("Deleting next hop group member which is still synced"); + assert(false); + } + } + + /* + * Sync the NHG member, setting its SAI ID. + */ + virtual void sync(sai_object_id_t gm_id) + { + SWSS_LOG_ENTER(); + + /* + * The SAI ID should be updated from invalid to something valid. + */ + if ((m_id != SAI_NULL_OBJECT_ID) || (gm_id == SAI_NULL_OBJECT_ID)) + { + SWSS_LOG_ERROR("Setting invalid SAI ID %lu to next hop group " + "membeer %s, with current SAI ID %lu", + gm_id, to_string().c_str(), m_id); + throw logic_error("Invalid SAI ID assigned to next hop group member"); + } + + m_id = gm_id; + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); + } + + /* + * Remove the group member, resetting its SAI ID. + */ + virtual void remove() + { + SWSS_LOG_ENTER(); + + /* + * If the membeer is not synced, exit. + */ + if (!isSynced()) + { + return; + } + + m_id = SAI_NULL_OBJECT_ID; + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); + } + + /* + * Getters. + */ + inline Key getKey() const { return m_key; } + inline sai_object_id_t getId() const { return m_id; } + + /* + * Check whether the group is synced. + */ + inline bool isSynced() const { return m_id != SAI_NULL_OBJECT_ID; } + + /* + * Get a string form of the member. + */ + virtual string to_string() const = 0; + +protected: + /* + * The index / key of this NHG member. + */ + Key m_key; + + /* + * The SAI ID of this NHG member. + */ + sai_object_id_t m_id; +}; + +/* + * NhgCommon class representing the common templated base class between + * Nhg and CbfNhg classes. + */ +template +class NhgCommon : public NhgBase +{ +public: + /* + * Constructors. + */ + explicit NhgCommon(const Key &key) : m_key(key) { SWSS_LOG_ENTER(); } + + NhgCommon(NhgCommon &&nhg) : NhgBase(move(nhg)), + m_key(move(nhg.m_key)), + m_members(move(nhg.m_members)) + { SWSS_LOG_ENTER(); } + + NhgCommon& operator=(NhgCommon &&nhg) + { + SWSS_LOG_ENTER(); + + swap(m_key, nhg.m_key); + swap(m_members, nhg.m_members); + + NhgBase::operator=(move(nhg)); + + return *this; + } + + /* + * Check if the group contains the given member. + */ + inline bool hasMember(const MbrKey &key) const + { SWSS_LOG_ENTER(); return m_members.find(key) != m_members.end(); } + + /* + * Getters. + */ + inline Key getKey() const { SWSS_LOG_ENTER(); return m_key; } + inline size_t getSize() const + { SWSS_LOG_ENTER(); return m_members.size(); } + + /* + * Sync the group, generating a SAI ID. + */ + virtual bool sync() = 0; + + /* + * Remove the group, releasing the SAI ID. + */ + virtual bool remove() + { + SWSS_LOG_ENTER(); + + /* + * If the group is already removed, there is nothing to be done. + */ + if (!isSynced()) + { + return true; + } + + /* + * Remove the group members. + */ + set members; + + for (const auto &member : m_members) + { + members.insert(member.first); + } + + if (!removeMembers(members)) + { + SWSS_LOG_ERROR("Failed to remove next hop group %s members", + to_string().c_str()); + return false; + } + + /* + * Remove the NHG over SAI. + */ + auto status = sai_next_hop_group_api->remove_next_hop_group(m_id); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to remove next hop group %s, rv: %d", + to_string().c_str(), status); + return false; + } + + /* + * Decrease the number of programmed NHGs. + */ + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); + decSyncedCount(); + + /* + * Reset the group ID. + */ + m_id = SAI_NULL_OBJECT_ID; + + return true; + } + + /* + * Get a string representation of this next hop group. + */ + virtual string to_string() const = 0; + +protected: + /* + * The key indexing this object. + */ + Key m_key; + + /* + * The members of this group. + */ + map m_members; + + /* + * Sync the given members in the group. + */ + virtual bool syncMembers(const set &member_keys) = 0; + + /* + * Remove the given members from the group. + */ + virtual bool removeMembers(const set &member_keys) + { + SWSS_LOG_ENTER(); + + /* + * Remove all the given members from the group. + */ + ObjectBulker bulker(sai_next_hop_group_api, + gSwitchId, + gMaxBulkSize); + map statuses; + + for (const auto &key : member_keys) + { + const auto &nhgm = m_members.at(key); + + if (nhgm.isSynced()) + { + bulker.remove_entry(&statuses[key], nhgm.getId()); + } + } + + /* + * Flush the bulker to remove the members. + */ + bulker.flush(); + + /* + * Iterate over the returned statuses and check if the removal was + * successful. If it was, remove the member, otherwise log an error + * message. + */ + bool success = true; + + for (const auto &status : statuses) + { + auto &member = m_members.at(status.first); + + if (status.second == SAI_STATUS_SUCCESS) + { + member.remove(); + } + else + { + SWSS_LOG_ERROR("Failed to remove next hop group member %s, rv: %d", + member.to_string().c_str(), + status.second); + success = false; + } + } + + return success; + } + + /* + * Get the SAI attributes for creating a next hop group member over SAI. + */ + virtual vector createNhgmAttrs(const Mbr &member) + const = 0; +}; + +/* + * Structure describing a next hop group which NhgHandler owns. Beside having + * a next hop group, we also want to keep a ref count so we don't delete + * objects that are still referenced. + */ +template +struct NhgEntry +{ + /* The next hop group object in this entry. */ + NhgClass nhg; + + /* Number of external objects referencing this next hop group. */ + unsigned ref_count; + + NhgEntry() = default; + explicit NhgEntry(NhgClass&& _nhg, unsigned int _ref_count = 0) : + nhg(move(_nhg)), ref_count(_ref_count) { SWSS_LOG_ENTER(); } +}; + +/* + * Class providing the common functionality shared by all NhgHandler classes. + */ +template +class NhgHandlerCommon +{ +public: + /* + * Check if the given next hop group index exists. + */ + inline bool hasNhg(const string &index) const + { + SWSS_LOG_ENTER(); + return m_syncedNextHopGroups.find(index) != m_syncedNextHopGroups.end(); + } + + /* + * Get the next hop group with the given index. If the index does not + * exist in the map, a out_of_range eexception will be thrown. + */ + inline const NhgClass& getNhg(const string &index) const + { SWSS_LOG_ENTER(); return m_syncedNextHopGroups.at(index).nhg; } + + /* Increase the ref count for a NHG given by it's index. */ + void incNhgRefCount(const string& index) + { + SWSS_LOG_ENTER(); + + auto& nhg_entry = m_syncedNextHopGroups.at(index); + ++nhg_entry.ref_count; + } + + /* Dencrease the ref count for a NHG given by it's index. */ + void decNhgRefCount(const string& index) + { + SWSS_LOG_ENTER(); + + auto& nhg_entry = m_syncedNextHopGroups.at(index); + + /* Sanity check so we don't overflow. */ + if (nhg_entry.ref_count == 0) + { + SWSS_LOG_ERROR("Trying to decrement next hop group %s reference " + "count while none are left.", + nhg_entry.nhg.to_string().c_str()); + throw logic_error("Decreasing ref count which is already 0"); + } + + --nhg_entry.ref_count; + } + +protected: + /* + * Map of synced next hop groups. + */ + unordered_map> m_syncedNextHopGroups; +}; diff --git a/orchagent/nhgorch.cpp b/orchagent/nhghandler.cpp similarity index 71% rename from orchagent/nhgorch.cpp rename to orchagent/nhghandler.cpp index 4b2084e82a..510ff5a763 100644 --- a/orchagent/nhgorch.cpp +++ b/orchagent/nhghandler.cpp @@ -1,15 +1,14 @@ +#include "nhghandler.h" #include "nhgorch.h" #include "neighorch.h" -#include "crmorch.h" #include "routeorch.h" +#include "crmorch.h" #include "bulker.h" #include "logger.h" #include "swssnet.h" extern sai_object_id_t gSwitchId; -extern PortsOrch *gPortsOrch; -extern CrmOrch *gCrmOrch; extern NeighOrch *gNeighOrch; extern RouteOrch *gRouteOrch; extern NhgOrch *gNhgOrch; @@ -18,15 +17,6 @@ extern size_t gMaxBulkSize; extern sai_next_hop_group_api_t* sai_next_hop_group_api; extern sai_next_hop_api_t* sai_next_hop_api; -extern sai_switch_api_t* sai_switch_api; - -unsigned int NextHopGroup::m_count = 0; - -NhgOrch::NhgOrch(DBConnector *db, string tableName) : - Orch(db, tableName) -{ - SWSS_LOG_ENTER(); -} /* * Purpose: Perform the operations requested by APPL_DB users. @@ -37,15 +27,10 @@ NhgOrch::NhgOrch(DBConnector *db, string tableName) : * Params: IN consumer - The cosumer object. * Returns: Nothing. */ -void NhgOrch::doTask(Consumer& consumer) +void NhgHandler::doTask(Consumer& consumer) { SWSS_LOG_ENTER(); - if (!gPortsOrch->allPortsReady()) - { - return; - } - auto it = consumer.m_toSync.begin(); while (it != consumer.m_toSync.end()) @@ -56,7 +41,7 @@ void NhgOrch::doTask(Consumer& consumer) string op = kfvOp(t); bool success = false; - const auto& nhg_it = m_syncdNextHopGroups.find(index); + const auto& nhg_it = m_syncedNextHopGroups.find(index); if (op == SET_COMMAND) { @@ -102,7 +87,7 @@ void NhgOrch::doTask(Consumer& consumer) NextHopGroupKey nhg_key = NextHopGroupKey(nhg_str, weights); /* If the group does not exist, create one. */ - if (nhg_it == m_syncdNextHopGroups.end()) + if (nhg_it == m_syncedNextHopGroups.end()) { /* * If we've reached the NHG limit, we're going to create a temporary @@ -111,16 +96,16 @@ void NhgOrch::doTask(Consumer& consumer) * to be kept in the sync list so we keep trying to create the * actual group when there are enough resources. */ - if (gRouteOrch->getNhgCount() + NextHopGroup::getCount() >= gRouteOrch->getMaxNhgCount()) + if (gRouteOrch->getNhgCount() + Nhg::getSyncedCount() >= gRouteOrch->getMaxNhgCount()) { SWSS_LOG_DEBUG("Next hop group count reached its limit."); try { - auto nhg = std::make_unique(createTempNhg(nhg_key)); - if (nhg->sync()) + auto nhg = createTempNhg(nhg_key); + if (nhg.sync()) { - m_syncdNextHopGroups.emplace(index, NhgEntry(std::move(nhg))); + m_syncedNextHopGroups.emplace(index, NhgEntry(std::move(nhg))); } else { @@ -138,19 +123,19 @@ void NhgOrch::doTask(Consumer& consumer) } else { - auto nhg = std::make_unique(nhg_key, false); - success = nhg->sync(); + auto nhg = Nhg(nhg_key, false); + success = nhg.sync(); if (success) { - m_syncdNextHopGroups.emplace(index, NhgEntry(std::move(nhg))); + m_syncedNextHopGroups.emplace(index, NhgEntry(std::move(nhg))); } } } /* If the group exists, update it. */ else { - const auto& nhg_ptr = nhg_it->second.nhg; + auto& nhg = nhg_it->second.nhg; /* * If the update would mandate promoting a temporary next hop @@ -158,8 +143,8 @@ void NhgOrch::doTask(Consumer& consumer) * resources yet, we have to skip it until we have enough * resources. */ - if (nhg_ptr->isTemp() && - (gRouteOrch->getNhgCount() + NextHopGroup::getCount() >= gRouteOrch->getMaxNhgCount())) + if (nhg.isTemp() && + (gRouteOrch->getNhgCount() + Nhg::getSyncedCount() >= gRouteOrch->getMaxNhgCount())) { /* * If the group was updated in such way that the previously @@ -168,12 +153,12 @@ void NhgOrch::doTask(Consumer& consumer) * the new key. Otherwise, this will be a no-op as we have * to wait for resources in order to promote the group. */ - if (!nhg_key.contains(nhg_ptr->getKey())) + if (!nhg_key.contains(nhg.getKey())) { try { /* Create the new temporary next hop group. */ - auto new_nhg = std::make_unique(createTempNhg(nhg_key)); + auto new_nhg = createTempNhg(nhg_key); /* * If we successfully sync the new group, update @@ -181,7 +166,7 @@ void NhgOrch::doTask(Consumer& consumer) * don't mess up the reference counter, as other * objects may already reference it. */ - if (new_nhg->sync()) + if (new_nhg.sync()) { nhg_it->second.nhg = std::move(new_nhg); } @@ -204,10 +189,10 @@ void NhgOrch::doTask(Consumer& consumer) * If the group is temporary but can now be promoted, create and sync a new group for * the desired next hops. */ - else if (nhg_ptr->isTemp()) + else if (nhg.isTemp()) { - auto nhg = std::make_unique(nhg_key, false); - success = nhg->sync(); + auto nhg = Nhg(nhg_key, false); + success = nhg.sync(); if (success) { @@ -221,14 +206,26 @@ void NhgOrch::doTask(Consumer& consumer) /* Common update, when all the requirements are met. */ else { - success = nhg_ptr->update(nhg_key); + success = nhg.update(nhg_key); } } } else if (op == DEL_COMMAND) { + /* + * If there is a pending SET after this DEL operation, skip the + * DEL operation to perform the update instead. Otherwise, in the + * scenario where the DEL operation may be blocked by the ref + * counter, we'd end up deleting the object after the SET operation + * is performed, which would not reflect the desired state of the + * object. + */ + if (consumer.m_toSync.count(it->first) > 1) + { + success = true; + } /* If the group does not exist, do nothing. */ - if (nhg_it == m_syncdNextHopGroups.end()) + else if (nhg_it == m_syncedNextHopGroups.end()) { SWSS_LOG_INFO("Unable to find group with key %s to remove", index.c_str()); /* Mark the operation as successful to consume it. */ @@ -242,19 +239,19 @@ void NhgOrch::doTask(Consumer& consumer) /* Else, if the group is no more referenced, remove it. */ else { - const auto& nhg = nhg_it->second.nhg; + auto& nhg = nhg_it->second.nhg; - success = nhg->remove(); + success = nhg.remove(); if (success) { - m_syncdNextHopGroups.erase(nhg_it); + m_syncedNextHopGroups.erase(nhg_it); } } } else { - SWSS_LOG_ERROR("Unknown operation type %s\n", op.c_str()); + SWSS_LOG_WARN("Unknown operation type %s", op.c_str()); /* Mark the operation as successful to consume it. */ success = true; } @@ -280,7 +277,7 @@ void NhgOrch::doTask(Consumer& consumer) * containing groups; * false, otherwise. */ -bool NhgOrch::validateNextHop(const NextHopKey& nh_key) +bool NhgHandler::validateNextHop(const NextHopKey& nh_key) { SWSS_LOG_ENTER(); @@ -288,17 +285,17 @@ bool NhgOrch::validateNextHop(const NextHopKey& nh_key) * Iterate through all groups and validate the next hop in those who * contain it. */ - for (auto& it : m_syncdNextHopGroups) + for (auto& it : m_syncedNextHopGroups) { auto& nhg = it.second.nhg; - if (nhg->hasNextHop(nh_key)) + if (nhg.hasMember(nh_key)) { /* * If sync fails, exit right away, as we expect it to be due to a * raeson for which any other future validations will fail too. */ - if (!nhg->validateNextHop(nh_key)) + if (!nhg.validateNextHop(nh_key)) { SWSS_LOG_ERROR("Failed to validate next hop %s in group %s", nh_key.to_string().c_str(), @@ -320,7 +317,7 @@ bool NhgOrch::validateNextHop(const NextHopKey& nh_key) * containing groups; * false, otherwise. */ -bool NhgOrch::invalidateNextHop(const NextHopKey& nh_key) +bool NhgHandler::invalidateNextHop(const NextHopKey& nh_key) { SWSS_LOG_ENTER(); @@ -328,14 +325,14 @@ bool NhgOrch::invalidateNextHop(const NextHopKey& nh_key) * Iterate through all groups and invalidate the next hop from those who * contain it. */ - for (auto& it : m_syncdNextHopGroups) + for (auto& it : m_syncedNextHopGroups) { auto& nhg = it.second.nhg; - if (nhg->hasNextHop(nh_key)) + if (nhg.hasMember(nh_key)) { /* If the remove fails, exit right away. */ - if (!nhg->invalidateNextHop(nh_key)) + if (!nhg.invalidateNextHop(nh_key)) { SWSS_LOG_WARN("Failed to invalidate next hop %s from group %s", nh_key.to_string().c_str(), @@ -348,37 +345,6 @@ bool NhgOrch::invalidateNextHop(const NextHopKey& nh_key) return true; } -/* - * Purpose: Increase the ref count for a next hop group. - * Description: Increment the ref count for a next hop group by 1. - * Params: IN index - The index of the next hop group. - * Returns: Nothing. - */ -void NhgOrch::incNhgRefCount(const std::string& index) -{ - SWSS_LOG_ENTER(); - - NhgEntry& nhg_entry = m_syncdNextHopGroups.at(index); - ++nhg_entry.ref_count; -} - -/* - * Purpose: Decrease the ref count for a next hop group. - * Description: Decrement the ref count for a next hop group by 1. - * Params: IN index - The index of the next hop group. - * Returns: Nothing. - */ -void NhgOrch::decNhgRefCount(const std::string& index) -{ - SWSS_LOG_ENTER(); - - NhgEntry& nhg_entry = m_syncdNextHopGroups.at(index); - - /* Sanity check so we don't overflow. */ - assert(nhg_entry.ref_count > 0); - --nhg_entry.ref_count; -} - /* * Purpose: Get the next hop ID of the member. * Description: Get the SAI ID of the next hop from NeighOrch. @@ -386,15 +352,15 @@ void NhgOrch::decNhgRefCount(const std::string& index) * Returns: The SAI ID of the next hop, or SAI_NULL_OBJECT_ID if the next * hop is not valid. */ -sai_object_id_t NextHopGroupMember::getNhId() const +sai_object_id_t WeightedNhgMember::getNhId() const { SWSS_LOG_ENTER(); sai_object_id_t nh_id = SAI_NULL_OBJECT_ID; - if (gNeighOrch->hasNextHop(m_nh_key)) + if (gNeighOrch->hasNextHop(m_key)) { - nh_id = gNeighOrch->getNextHopId(m_nh_key); + nh_id = gNeighOrch->getNextHopId(m_key); } /* * If the next hop is labeled and the IP next hop exists, create the @@ -403,35 +369,19 @@ sai_object_id_t NextHopGroupMember::getNhId() const * after the object is created and would never create the labeled next hop * afterwards. */ - else if (isLabeled() && gNeighOrch->isNeighborResolved(m_nh_key)) + else if (isLabeled() && gNeighOrch->isNeighborResolved(m_key)) { - gNeighOrch->addNextHop(m_nh_key); - nh_id = gNeighOrch->getNextHopId(m_nh_key); + gNeighOrch->addNextHop(m_key); + nh_id = gNeighOrch->getNextHopId(m_key); } else { - gNeighOrch->resolveNeighbor(m_nh_key); + gNeighOrch->resolveNeighbor(m_key); } return nh_id; } -/* - * Purpose: Move assignment operator. - * Description: Perform member-wise swap. - * Params: IN nhgm - The next hop group member to swap. - * Returns: Reference to this object. - */ -NextHopGroupMember& NextHopGroupMember::operator=(NextHopGroupMember&& nhgm) -{ - SWSS_LOG_ENTER(); - - std::swap(m_nh_key, nhgm.m_nh_key); - std::swap(m_gm_id, nhgm.m_gm_id); - - return *this; -} - /* * Purpose: Update the weight of a member. * Description: Set the new member's weight and if the member is synced, update @@ -440,21 +390,21 @@ NextHopGroupMember& NextHopGroupMember::operator=(NextHopGroupMember&& nhgm) * Returns: true, if the operation was successful; * false, otherwise. */ -bool NextHopGroupMember::updateWeight(uint32_t weight) +bool WeightedNhgMember::updateWeight(uint32_t weight) { SWSS_LOG_ENTER(); bool success = true; - m_nh_key.weight = weight; + m_key.weight = weight; if (isSynced()) { sai_attribute_t nhgm_attr; nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT; - nhgm_attr.value.s32 = m_nh_key.weight; + nhgm_attr.value.s32 = m_key.weight; - sai_status_t status = sai_next_hop_group_api->set_next_hop_group_member_attribute(m_gm_id, &nhgm_attr); + sai_status_t status = sai_next_hop_group_api->set_next_hop_group_member_attribute(m_id, &nhgm_attr); success = status == SAI_STATUS_SUCCESS; } @@ -468,16 +418,12 @@ bool NextHopGroupMember::updateWeight(uint32_t weight) * Params: IN gm_id - The group member SAI ID to set. * Returns: Nothing. */ -void NextHopGroupMember::sync(sai_object_id_t gm_id) +void WeightedNhgMember::sync(sai_object_id_t gm_id) { SWSS_LOG_ENTER(); - /* The SAI ID should be updated from invalid to something valid. */ - assert((m_gm_id == SAI_NULL_OBJECT_ID) && (gm_id != SAI_NULL_OBJECT_ID)); - - m_gm_id = gm_id; - gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); - gNeighOrch->increaseNextHopRefCount(m_nh_key); + NhgMember::sync(gm_id); + gNeighOrch->increaseNextHopRefCount(m_key); } /* @@ -487,22 +433,12 @@ void NextHopGroupMember::sync(sai_object_id_t gm_id) * Params: None. * Returns: Nothing. */ -void NextHopGroupMember::remove() +void WeightedNhgMember::remove() { SWSS_LOG_ENTER(); - /* - * If the member is already removed, exit so we don't decrement the ref - * counters more than once. - */ - if (!isSynced()) - { - return; - } - - m_gm_id = SAI_NULL_OBJECT_ID; - gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); - gNeighOrch->decreaseNextHopRefCount(m_nh_key); + NhgMember::remove(); + gNeighOrch->decreaseNextHopRefCount(m_key); } /* @@ -512,15 +448,10 @@ void NextHopGroupMember::remove() * Params: None. * Returns: Nothing. */ -NextHopGroupMember::~NextHopGroupMember() +WeightedNhgMember::~WeightedNhgMember() { SWSS_LOG_ENTER(); - /* - * The group member should be removed from its group before destroying it. - */ - assert(!isSynced()); - /* * If the labeled next hop is unreferenced, remove it from NeighOrch as * NhgOrch and RouteOrch are the ones controlling it's lifetime. They both @@ -529,10 +460,10 @@ NextHopGroupMember::~NextHopGroupMember() * next hop. */ if (isLabeled() && - gNeighOrch->hasNextHop(m_nh_key) && - (gNeighOrch->getNextHopRefCount(m_nh_key) == 0)) + gNeighOrch->hasNextHop(m_key) && + (gNeighOrch->getNextHopRefCount(m_key) == 0)) { - gNeighOrch->removeMplsNextHop(m_nh_key); + gNeighOrch->removeMplsNextHop(m_key); } } @@ -542,53 +473,31 @@ NextHopGroupMember::~NextHopGroupMember() * Params: IN key - The next hop group's key. * Returns: Nothing. */ -NextHopGroup::NextHopGroup(const NextHopGroupKey& key, bool is_temp) : - m_key(key), - m_id(SAI_NULL_OBJECT_ID), - m_is_temp(is_temp) +Nhg::Nhg(const NextHopGroupKey& key, bool is_temp) : NhgCommon(key), m_is_temp(is_temp) { SWSS_LOG_ENTER(); /* Parse the key and create the members. */ - for (const auto& it : m_key.getNextHops()) + for (const auto &it : m_key.getNextHops()) { - m_members.emplace(it, NextHopGroupMember(it)); + m_members.emplace(it, WeightedNhgMember(it)); } } -/* - * Purpose: Move constructor. - * Description: Initialize the members by doing member-wise move construct. - * Params: IN nhg - The rvalue object to initialize from. - * Returns: Nothing. - */ -NextHopGroup::NextHopGroup(NextHopGroup&& nhg) : - m_key(std::move(nhg.m_key)), - m_id(std::move(nhg.m_id)), - m_members(std::move(nhg.m_members)), - m_is_temp(nhg.m_is_temp) -{ - SWSS_LOG_ENTER(); - - /* Invalidate the rvalue SAI ID. */ - nhg.m_id = SAI_NULL_OBJECT_ID; -} - /* * Purpose: Move assignment operator. * Description: Perform member-wise swap. * Params: IN nhg - The rvalue object to swap with. * Returns: Referene to this object. */ -NextHopGroup& NextHopGroup::operator=(NextHopGroup&& nhg) +Nhg& Nhg::operator=(Nhg&& nhg) { SWSS_LOG_ENTER(); - std::swap(m_key, nhg.m_key); - std::swap(m_id, nhg.m_id); - std::swap(m_members, nhg.m_members); m_is_temp = nhg.m_is_temp; + NhgCommon::operator=(std::move(nhg)); + return *this; } @@ -602,7 +511,7 @@ NextHopGroup& NextHopGroup::operator=(NextHopGroup&& nhg) * Returns: true, if the operation was successful; * false, otherwise. */ -bool NextHopGroup::sync() +bool Nhg::sync() { SWSS_LOG_ENTER(); @@ -618,12 +527,12 @@ bool NextHopGroup::sync() */ if (m_is_temp) { - const NextHopGroupMember& nhgm = m_members.begin()->second; + const WeightedNhgMember& nhgm = m_members.begin()->second; sai_object_id_t nhid = nhgm.getNhId(); if (nhid == SAI_NULL_OBJECT_ID) { - SWSS_LOG_WARN("Next hop %s is not synced", nhgm.getNhKey().to_string().c_str()); + SWSS_LOG_WARN("Next hop %s is not synced", nhgm.getKey().to_string().c_str()); return false; } else @@ -667,7 +576,7 @@ bool NextHopGroup::sync() gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); /* Increment the number of synced NHGs. */ - ++m_count; + ++m_syncedCount; /* * Try creating the next hop group's members over SAI. @@ -693,7 +602,7 @@ bool NextHopGroup::sync() * Params: IN index - The CP index of the next hop group. * Returns: The created temporary next hop group. */ -NextHopGroup NhgOrch::createTempNhg(const NextHopGroupKey& nhg_key) +Nhg NhgHandler::createTempNhg(const NextHopGroupKey& nhg_key) { SWSS_LOG_ENTER(); @@ -726,7 +635,7 @@ NextHopGroup NhgOrch::createTempNhg(const NextHopGroupKey& nhg_key) advance(it, rand() % valid_nhs.size()); /* Create the temporary group. */ - NextHopGroup nhg(NextHopGroupKey(it->to_string()), true); + Nhg nhg(NextHopGroupKey(it->to_string()), true); return nhg; } @@ -739,51 +648,18 @@ NextHopGroup NhgOrch::createTempNhg(const NextHopGroupKey& nhg_key) * Returns: true, if the operation was successful; * false, otherwise */ -bool NextHopGroup::remove() +bool Nhg::remove() { SWSS_LOG_ENTER(); - /* If the group is already removed, or is temporary, there is nothing to be done - - * just reset the ID. - */ - if (!isSynced() || m_is_temp) + // If the group is temporary, there is nothing to be done - just reset the ID. + if (m_is_temp) { m_id = SAI_NULL_OBJECT_ID; return true; } - /* Remove group's members. If we failed to remove the members, exit. */ - if (!removeMembers(m_key.getNextHops())) - { - SWSS_LOG_ERROR("Failed to remove group %s members", to_string().c_str()); - return false; - } - - /* Remove the group. */ - sai_status_t status = sai_next_hop_group_api-> - remove_next_hop_group(m_id); - - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to remove next hop group %s, rv: %d", - m_key.to_string().c_str(), status); - - task_process_status handle_status = gNhgOrch->handleSaiRemoveStatus(SAI_API_NEXT_HOP_GROUP, status); - if (handle_status != task_success) - { - return gNhgOrch->parseHandleSaiStatusFailure(handle_status); - } - } - - /* If the operation is successful, release the resources. */ - gCrmOrch->decCrmResUsedCounter( - CrmResourceType::CRM_NEXTHOP_GROUP); - --m_count; - - /* Reset the group ID. */ - m_id = SAI_NULL_OBJECT_ID; - - return true; + return NhgCommon::remove(); } /* @@ -796,7 +672,7 @@ bool NextHopGroup::remove() * Returns: true, if the members were added succesfully; * false, otherwise. */ -bool NextHopGroup::syncMembers(const std::set& nh_keys) +bool Nhg::syncMembers(const std::set& nh_keys) { SWSS_LOG_ENTER(); @@ -813,10 +689,10 @@ bool NextHopGroup::syncMembers(const std::set& nh_keys) for (const auto& nh_key : nh_keys) { - NextHopGroupMember& nhgm = m_members.at(nh_key); + WeightedNhgMember& nhgm = m_members.at(nh_key); /* If the member is already synced, continue. */ - if (nhgm.getGmId() != SAI_NULL_OBJECT_ID) + if (nhgm.isSynced()) { continue; } @@ -873,66 +749,6 @@ bool NextHopGroup::syncMembers(const std::set& nh_keys) return success; } -/* - * Purpose: Remove the given group's members over the SAI API. - * Description: Go through the given members and remove them. - * Params: IN nh_keys - The next hop keys of the members to remove. - * Returns: true, if the operation was successful; - * false, otherwise - */ -bool NextHopGroup::removeMembers(const std::set& nh_keys) -{ - SWSS_LOG_ENTER(); - - ObjectBulker nextHopGroupMemberBulker( - sai_next_hop_group_api, gSwitchId, gMaxBulkSize); - - /* - * Iterate through the given group members add them to be removed. - * - * Keep track of the SAI remove statuses in case one of them returns an - * error. We assume that removal should always succeed. If for some - * reason it doesn't, there's nothing we can do, but we'll log an error - * later. - */ - std::map statuses; - - for (const auto& nh_key : nh_keys) - { - const NextHopGroupMember& nhgm = m_members.at(nh_key); - - if (nhgm.isSynced()) - { - nextHopGroupMemberBulker.remove_entry(&statuses[nh_key], nhgm.getGmId()); - } - } - - /* Flush the bulker to apply the changes. */ - nextHopGroupMemberBulker.flush(); - - /* - * Iterate over the statuses map and check if the removal was successful. - * If it was, decrement the Crm counter and reset the member's ID. If it - * wasn't, log an error message. - */ - bool success = true; - - for (const auto& status : statuses) - { - if (status.second == SAI_STATUS_SUCCESS) - { - m_members.at(status.first).remove(); - } - else - { - SWSS_LOG_ERROR("Could not remove next hop group member %s, rv: %d", - status.first.to_string().c_str(), status.second); - success = false; - } - } - - return success; -} /* * Purpose: Update the next hop group based on a new next hop group key. @@ -945,7 +761,7 @@ bool NextHopGroup::removeMembers(const std::set& nh_keys) * Returns: true, if the operation was successful; * false, otherwise. */ -bool NextHopGroup::update(const NextHopGroupKey& nhg_key) +bool Nhg::update(const NextHopGroupKey& nhg_key) { SWSS_LOG_ENTER(); @@ -1001,7 +817,7 @@ bool NextHopGroup::update(const NextHopGroupKey& nhg_key) /* Add any new members to the group. */ for (const auto& it : new_nh_keys) { - m_members.emplace(it, NextHopGroupMember(it)); + m_members.emplace(it, WeightedNhgMember(it)); } /* @@ -1024,7 +840,7 @@ bool NextHopGroup::update(const NextHopGroupKey& nhg_key) * Params: IN nhgm - The next hop group member. * Returns: The attributes vector for the given next hop. */ -vector NextHopGroup::createNhgmAttrs(const NextHopGroupMember& nhgm) const +vector Nhg::createNhgmAttrs(const WeightedNhgMember& nhgm) const { SWSS_LOG_ENTER(); @@ -1056,7 +872,7 @@ vector NextHopGroup::createNhgmAttrs(const NextHopGroupMember& * Returns: true, if the operation was successful; * false, otherwise. */ -bool NextHopGroup::validateNextHop(const NextHopKey& nh_key) +bool Nhg::validateNextHop(const NextHopKey& nh_key) { SWSS_LOG_ENTER(); @@ -1070,7 +886,7 @@ bool NextHopGroup::validateNextHop(const NextHopKey& nh_key) * Returns: true, if the operation was successful; * false, otherwise. */ -bool NextHopGroup::invalidateNextHop(const NextHopKey& nh_key) +bool Nhg::invalidateNextHop(const NextHopKey& nh_key) { SWSS_LOG_ENTER(); diff --git a/orchagent/nhghandler.h b/orchagent/nhghandler.h new file mode 100644 index 0000000000..301ef398a4 --- /dev/null +++ b/orchagent/nhghandler.h @@ -0,0 +1,116 @@ +#pragma once + +#include "nhgbase.h" + +using namespace std; + +class WeightedNhgMember : public NhgMember +{ +public: + /* Constructors / Assignment operators. */ + WeightedNhgMember(const NextHopKey& nh_key) : + NhgMember(nh_key) {} + + WeightedNhgMember(WeightedNhgMember&& nhgm) : + NhgMember(move(nhgm)) {} + + /* Destructor. */ + ~WeightedNhgMember(); + + /* Update member's weight and update the SAI attribute as well. */ + bool updateWeight(uint32_t weight); + + /* Sync / Remove. */ + void sync(sai_object_id_t gm_id) override; + void remove() override; + + /* Getters / Setters. */ + inline uint32_t getWeight() const { return m_key.weight; } + sai_object_id_t getNhId() const; + + /* Check if the next hop is labeled. */ + inline bool isLabeled() const { return !m_key.label_stack.empty(); } + + /* Convert member's details to string. */ + string to_string() const override + { + return m_key.to_string() + + ", SAI ID: " + std::to_string(m_id); + } +}; + +/* + * Nhg class representing a next hop group object. + */ +class Nhg : public NhgCommon +{ +public: + /* Constructors. */ + explicit Nhg(const NextHopGroupKey& key, bool is_temp); + + Nhg(Nhg&& nhg) : + NhgCommon(move(nhg)), m_is_temp(nhg.m_is_temp) + { SWSS_LOG_ENTER(); } + + Nhg& operator=(Nhg&& nhg); + + ~Nhg() { SWSS_LOG_ENTER(); remove(); } + + /* Sync the group, creating the group's and members SAI IDs. */ + bool sync() override; + + /* Remove the group, reseting the group's and members SAI IDs. */ + bool remove() override; + + /* + * Update the group based on a new next hop group key. This will also + * perform any sync / remove necessary. + */ + bool update(const NextHopGroupKey& nhg_key); + + /* Validate a next hop in the group, syncing it. */ + bool validateNextHop(const NextHopKey& nh_key); + + /* Invalidate a next hop in the group, removing it. */ + bool invalidateNextHop(const NextHopKey& nh_key); + + /* Getters / Setters. */ + inline bool isTemp() const override { return m_is_temp; } + inline void setTemp(bool is_temp) { m_is_temp = is_temp; } + + NextHopGroupKey getNhgKey() const override { return m_key; } + + /* Convert NHG's details to a string. */ + string to_string() const override + { + return m_key.to_string() + ", SAI ID: " + std::to_string(m_id); + } + +private: + /* Whether the group is temporary or not. */ + bool m_is_temp; + + /* Add group's members over the SAI API for the given keys. */ + bool syncMembers(const set& nh_keys) override; + + /* Create the attributes vector for a next hop group member. */ + vector createNhgmAttrs( + const WeightedNhgMember& nhgm) const override; +}; + +/* + * Next Hop Group Orchestrator class that handles NEXT_HOP_GROUP_TABLE + * updates. + */ +class NhgHandler : public NhgHandlerCommon +{ +public: + /* Add a temporary next hop group when resources are exhausted. */ + Nhg createTempNhg(const NextHopGroupKey& nhg_key); + + /* Validate / Invalidate a next hop. */ + bool validateNextHop(const NextHopKey& nh_key); + bool invalidateNextHop(const NextHopKey& nh_key); + + void doTask(Consumer &consumer); +}; diff --git a/orchagent/nhgmaporch.cpp b/orchagent/nhgmaporch.cpp new file mode 100644 index 0000000000..45604dd8a7 --- /dev/null +++ b/orchagent/nhgmaporch.cpp @@ -0,0 +1,366 @@ +#include "nhgmaporch.h" +#include "climits" + +extern sai_object_id_t gSwitchId; +extern sai_next_hop_group_api_t* sai_next_hop_group_api; +extern sai_switch_api_t *sai_switch_api; + +uint64_t NhgMapOrch::m_max_nhg_map_count = 0; + +NhgMapEntry::NhgMapEntry(sai_object_id_t _id, uint32_t _ref_count) : id(_id), ref_count(_ref_count) +{ + SWSS_LOG_ENTER(); +} + +NhgMapOrch::NhgMapOrch(swss::DBConnector *db, const string &table_name) : Orch(db, table_name) +{ + SWSS_LOG_ENTER(); + + /* + * Get the maximum number of NHG maps. + */ + if (sai_object_type_get_availability(gSwitchId, + SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MAP, + 0, + nullptr, + &m_max_nhg_map_count) != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("Switch does not support NHG maps"); + m_max_nhg_map_count = 0; + } + + SWSS_LOG_INFO("Maximum number of NHG maps: %lu", m_max_nhg_map_count); +} + +void NhgMapOrch::doTask(Consumer &consumer) +{ + SWSS_LOG_ENTER(); + + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) + { + swss::KeyOpFieldsValuesTuple t = it->second; + string index = kfvKey(t); + string op = kfvOp(t); + bool success = true; + + auto fc_map_it = m_syncdMaps.find(index); + + /* + * Set operation. + */ + if (op == SET_COMMAND) + { + /* + * Extract the map from the values. + */ + auto p = getMap(kfvFieldsValues(t)); + success = p.first; + + /* + * If the map can't be extracted, erase the work item as it's useless to retry unless the user updates the + * wrong values. We achieve the erase by setting the success value to "true". + * + */ + if (!success) + { + SWSS_LOG_ERROR("Failed to extract NHG map %s", index.c_str()); + success = true; + } + else + { + /* + * Create the SAI map. + */ + auto *fc_map = new sai_map_t[p.second.size()]; + uint32_t ii = 0; + + for (const auto &fc_nh_idx : p.second) + { + fc_map[ii].key = fc_nh_idx.first; + fc_map[ii++].value = fc_nh_idx.second; + } + + sai_map_list_t fc_map_list; + assert(p.second.size() <= UINT32_MAX); + fc_map_list.count = static_cast(p.second.size()); + fc_map_list.list = fc_map; + + /* + * Create the map. + */ + if (fc_map_it == m_syncdMaps.end()) + { + /* + * Check if we have enough resources for a new map. + */ + if (m_syncdMaps.size() >= m_max_nhg_map_count) + { + SWSS_LOG_WARN("No more resources left for new next hop group map %s", index.c_str()); + success = false; + } + else + { + /* + * Set the map attributes. + */ + sai_attribute_t attr; + vector attrs; + + attr.id = SAI_NEXT_HOP_GROUP_MAP_ATTR_TYPE; + attr.value.u32 = SAI_NEXT_HOP_GROUP_MAP_TYPE_FORWARDING_CLASS_TO_INDEX; + attrs.push_back(move(attr)); + + attr.id = SAI_NEXT_HOP_GROUP_MAP_ATTR_MAP_TO_VALUE_LIST; + attr.value.maplist = fc_map_list; + attrs.push_back(move(attr)); + + /* + * Create the map over SAI. + */ + sai_object_id_t nhg_map_id; + sai_status_t status = sai_next_hop_group_api->create_next_hop_group_map(&nhg_map_id, + gSwitchId, + static_cast(attrs.size()), + attrs.data()); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create NHG map %s, rv %d", index.c_str(), status); + success = false; + } + else + { + assert(nhg_map_id != SAI_NULL_OBJECT_ID); + m_syncdMaps.emplace(move(index), nhg_map_id); + } + } + } + /* + * Update the map. + */ + else + { + /* + * Update the map attribute. + */ + sai_attribute_t attr; + attr.id = SAI_NEXT_HOP_GROUP_MAP_ATTR_MAP_TO_VALUE_LIST; + attr.value.maplist = fc_map_list; + + sai_status_t status = sai_next_hop_group_api->set_next_hop_group_map_attribute(fc_map_it->second.id, + &attr); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to update NHG map %s, rv %d", index.c_str(), status); + success = false; + } + } + + /* + * Free the allocated memory. + */ + delete[] fc_map; + } + } + /* + * Del operation. + */ + else if (op == DEL_COMMAND) + { + /* + * If there is a pending SET after this DEL operation, skip the + * DEL operation to perform the update instead. Otherwise, in the + * scenario where the DEL operation may be blocked by the ref + * counter, we'd end up deleting the object after the SET operation + * is performed, which would not reflect the desired state of the + * object. + */ + if (consumer.m_toSync.count(it->first) > 1) + { + success = true; + } + else if (fc_map_it == m_syncdMaps.end()) + { + SWSS_LOG_WARN("NHG map %s does not exist to deleted", index.c_str()); + } + else if (fc_map_it->second.ref_count > 0) + { + SWSS_LOG_WARN("Can not delete referenced NHG map %s", index.c_str()); + success = false; + } + else + { + sai_status_t status = sai_next_hop_group_api->remove_next_hop_group_map(fc_map_it->second.id); + + if (status == SAI_STATUS_SUCCESS) + { + m_syncdMaps.erase(fc_map_it); + } + else + { + SWSS_LOG_ERROR("Failed to remove NHG map %s, rv %d", index.c_str(), status); + success = false; + } + } + } + else + { + SWSS_LOG_WARN("Unknown operation type %s", op.c_str()); + + /* + * Mark the operation as a success to remove the task. + */ + success = true; + } + + /* + * Depending on the operation success, remove the task or skip it. + */ + if (success) + { + it = consumer.m_toSync.erase(it); + } + else + { + ++it; + } + } +} + +/* + * Return the SAI ID for the map indexed by "index". If it does not exist, return SAI_NULL_OBJECT_ID. + */ +sai_object_id_t NhgMapOrch::getMapId(const string &index) const +{ + SWSS_LOG_ENTER(); + + auto it = m_syncdMaps.find(index); + + return it == m_syncdMaps.end() ? SAI_NULL_OBJECT_ID : it->second.id; +} + +/* + * Increase reference counter for a map. + */ +void NhgMapOrch::incRefCount(const string &index) +{ + SWSS_LOG_ENTER(); + + ++m_syncdMaps.at(index).ref_count; +} + +/* + * Decrease reference counter for a map. + */ +void NhgMapOrch::decRefCount(const string &index) +{ + SWSS_LOG_ENTER(); + + auto &nhg_map = m_syncdMaps.at(index); + + if (nhg_map.ref_count == 0) + { + SWSS_LOG_ERROR("Decreasing reference counter beyond 0 for NHG map %s", index.c_str()); + throw std::runtime_error("Decreasing reference counter beyond 0"); + } + + --nhg_map.ref_count; +} + +/* + * Get the max FC value supported by the switch. + */ +sai_uint8_t NhgMapOrch::getMaxFcVal() +{ + SWSS_LOG_ENTER(); + + static int max_fc_val = -1; + + /* + * Get the maximum value allowed for FC if it wasn't already initialized. + */ + if (max_fc_val == -1) + { + sai_attribute_t attr; + attr.id = SAI_SWITCH_ATTR_MAX_NUMBER_OF_FORWARDING_CLASSES; + + if (sai_switch_api->get_switch_attribute(gSwitchId, 1, &attr) == SAI_STATUS_SUCCESS) + { + max_fc_val = attr.value.u8; + } + else + { + SWSS_LOG_WARN("Switch does not support FCs"); + max_fc_val = 0; + } + } + + return static_cast(max_fc_val); +} + +/* + * Extract the NHG map from the FV tuples + */ +pair> NhgMapOrch::getMap(const vector &values) +{ + SWSS_LOG_ENTER(); + + bool success = true; + + /* + * If the map is empty, return error + */ + if (values.empty()) + { + SWSS_LOG_ERROR("NHG map is empty"); + success = false; + } + + unordered_map fc_map; + sai_uint8_t max_fc_val = getMaxFcVal(); + + /* + * Create the map while validating that the values are positive + */ + for (auto it = values.begin(); it != values.end(); ++it) + { + try + { + /* + * Check the FC value is valid. + */ + auto fc = stoi(fvField(*it)); + + if ((fc < 0) || (fc > max_fc_val)) + { + SWSS_LOG_ERROR("FC value %d is either negative or greater than max value %d", fc, max_fc_val); + success = false; + break; + } + + /* + * Check the NH index value is valid. + */ + auto nh_idx = stoi(fvValue(*it)); + + if (nh_idx < 0) + { + SWSS_LOG_ERROR("NH index %d is negative", nh_idx); + success = false; + break; + } + + fc_map.emplace(fc, nh_idx).second; + } + catch(const invalid_argument& e) + { + SWSS_LOG_ERROR("Got exception during conversion: %s", e.what()); + success = false; + break; + } + } + + return {success, fc_map}; +} diff --git a/orchagent/nhgmaporch.h b/orchagent/nhgmaporch.h new file mode 100644 index 0000000000..8e5d2b976a --- /dev/null +++ b/orchagent/nhgmaporch.h @@ -0,0 +1,61 @@ +#pragma once + +#include "orch.h" +#include "dbconnector.h" +#include + +using namespace std; + +/* + * Structure describing a NHG map entry in the NHG map map. + */ +struct NhgMapEntry +{ + /* The next hop group map SAI ID. */ + sai_object_id_t id; + + /* Number of external objects referencing this next hop group map. */ + uint32_t ref_count; + + explicit NhgMapEntry(sai_object_id_t id, uint32_t _ref_count = 0); +}; + +class NhgMapOrch : public Orch +{ +public: + NhgMapOrch(swss::DBConnector *db, const string &table_name); + + void doTask(Consumer &consumer) override; + + /* + * Return the SAI ID for the map indexed by "index". If it does not exist, return SAI_NULL_OBJECT_ID. + */ + sai_object_id_t getMapId(const string &key) const; + + /* + * Increase / Decrease reference counter for a map. + */ + void incRefCount(const string &key); + void decRefCount(const string &key); + + /* + * Get the max FC value supported by the switch. + */ + static sai_uint8_t getMaxFcVal(); + +private: + /* + * Map of synced NHG maps over SAI. + */ + unordered_map m_syncdMaps; + + /* + * Maximum number of NHG maps supported by the switch. + */ + static uint64_t m_max_nhg_map_count; + + /* + * Extract the NHG map from the FV tuples + */ + static pair> getMap(const vector &values); +}; diff --git a/orchagent/nhgorch.h b/orchagent/nhgorch.h index ce99ef85e3..20369c9a0e 100644 --- a/orchagent/nhgorch.h +++ b/orchagent/nhgorch.h @@ -1,229 +1,138 @@ #pragma once -#include "orch.h" -#include "nexthopgroupkey.h" +#include "cbfnhghandler.h" +#include "nhghandler.h" +#include "switchorch.h" +#include "vector" +#include "portsorch.h" -class NextHopGroupMember -{ -public: - /* Constructors / Assignment operators. */ - NextHopGroupMember(const NextHopKey& nh_key) : - m_nh_key(nh_key), - m_gm_id(SAI_NULL_OBJECT_ID) {} - - NextHopGroupMember(NextHopGroupMember&& nhgm) : - m_nh_key(std::move(nhgm.m_nh_key)), - m_gm_id(nhgm.m_gm_id) - { nhgm.m_gm_id = SAI_NULL_OBJECT_ID; } - - NextHopGroupMember& operator=(NextHopGroupMember&& nhgm); - - /* - * Prevent object copying so we don't end up having multiple objects - * referencing the same SAI objects. - */ - NextHopGroupMember(const NextHopGroupMember&) = delete; - void operator=(const NextHopGroupMember&) = delete; - - /* Destructor. */ - virtual ~NextHopGroupMember(); - - /* Update member's weight and update the SAI attribute as well. */ - bool updateWeight(uint32_t weight); +using namespace std; - /* Sync / Remove. */ - void sync(sai_object_id_t gm_id); - void remove(); +extern PortsOrch *gPortsOrch; - /* Getters / Setters. */ - inline const NextHopKey& getNhKey() const { return m_nh_key; } - inline uint32_t getWeight() const { return m_nh_key.weight; } - sai_object_id_t getNhId() const; - inline sai_object_id_t getGmId() const { return m_gm_id; } - inline bool isSynced() const { return m_gm_id != SAI_NULL_OBJECT_ID; } - - /* Check if the next hop is labeled. */ - inline bool isLabeled() const { return !m_nh_key.label_stack.empty(); } - - /* Convert member's details to string. */ - std::string to_string() const - { - return m_nh_key.to_string() + ", SAI ID: " + std::to_string(m_gm_id); - } - -private: - /* The key of the next hop of this member. */ - NextHopKey m_nh_key; - - /* The group member SAI ID for this member. */ - sai_object_id_t m_gm_id; -}; - -/* Map indexed by NextHopKey, containing the SAI ID of the group member. */ -typedef std::map NhgMembers; - -/* - * NextHopGroup class representing a next hop group object. - */ -class NextHopGroup +class NhgOrch : public Orch { public: - /* Constructors. */ - explicit NextHopGroup(const NextHopGroupKey& key, bool is_temp); - NextHopGroup(NextHopGroup&& nhg); - NextHopGroup& operator=(NextHopGroup&& nhg); - - /* Destructor. */ - virtual ~NextHopGroup() { remove(); } - - /* Sync the group, creating the group's and members SAI IDs. */ - bool sync(); + NhgOrch(DBConnector *db, const vector &table_names) : + Orch(db, table_names) + { + SWSS_LOG_ENTER(); + } - /* Remove the group, reseting the group's and members SAI IDs. */ - bool remove(); + /* + * Get the maximum number of ECMP groups allowed by the switch. + */ + static inline unsigned getMaxNhgCount() + { SWSS_LOG_ENTER(); return m_maxNhgCount; } /* - * Update the group based on a new next hop group key. This will also - * perform any sync / remove necessary. + * Get the number of next hop groups that are synced. */ - bool update(const NextHopGroupKey& nhg_key); + static inline unsigned getSyncedNhgCount() + { SWSS_LOG_ENTER(); return NhgBase::getSyncedCount(); } - /* Check if the group contains the given next hop. */ - inline bool hasNextHop(const NextHopKey& nh_key) const + /* Increase the number of synced next hop groups. */ + static void incSyncedNhgCount() { - return m_members.find(nh_key) != m_members.end(); - } - - /* Validate a next hop in the group, syncing it. */ - bool validateNextHop(const NextHopKey& nh_key); - - /* Invalidate a next hop in the group, removing it. */ - bool invalidateNextHop(const NextHopKey& nh_key); + SWSS_LOG_ENTER(); - /* Increment the number of existing groups. */ - static inline void incCount() { ++m_count; } + if (getSyncedNhgCount() >= m_maxNhgCount) + { + SWSS_LOG_ERROR("Incresing synced next hop group count beyond " + "switch's capabilities"); + throw logic_error("Next hop groups exceed switch's " + "capabilities"); + } - /* Decrement the number of existing groups. */ - static inline void decCount() { assert(m_count > 0); --m_count; } + NhgBase::incSyncedCount(); + } - /* Getters / Setters. */ - inline const NextHopGroupKey& getKey() const { return m_key; } - inline sai_object_id_t getId() const { return m_id; } - static inline unsigned int getCount() { return m_count; } - inline bool isTemp() const { return m_is_temp; } - inline bool isSynced() const { return m_id != SAI_NULL_OBJECT_ID; } - inline size_t getSize() const { return m_members.size(); } + /* Decrease the number of next hop groups. */ + static inline void decSyncedNhgCount() + { SWSS_LOG_ENTER(); NhgBase::decSyncedCount(); } - /* Convert NHG's details to a string. */ - std::string to_string() const + /* + * Check if the next hop group with the given index exists. + */ + inline bool hasNhg(const string &index) const { - return m_key.to_string() + ", SAI ID: " + std::to_string(m_id); + SWSS_LOG_ENTER(); + return nhgHandler.hasNhg(index) || cbfNhgHandler.hasNhg(index); } -private: - - /* The next hop group key of this group. */ - NextHopGroupKey m_key; - - /* The SAI ID of the group. */ - sai_object_id_t m_id; - - /* Members of this next hop group. */ - NhgMembers m_members; - - /* Whether the group is temporary or not. */ - bool m_is_temp; - /* - * Number of existing groups. Incremented when an object is created and - * decremented when an object is destroyed. This will also account for the - * groups created by RouteOrch. + * Get the next hop group with the given index. */ - static unsigned int m_count; - - /* Add group's members over the SAI API for the given keys. */ - bool syncMembers(const std::set& nh_keys); - - /* Remove group's members the SAI API from the given keys. */ - bool removeMembers(const std::set& nh_keys); - - /* Create the attributes vector for a next hop group member. */ - vector createNhgmAttrs(const NextHopGroupMember& nhgm) const; -}; - -/* - * Structure describing a next hop group which NhgOrch owns. Beside having a - * unique pointer to that next hop group, we also want to keep a ref count so - * NhgOrch knows how many other objects reference the next hop group in order - * not to remove them while still being referenced. - */ -struct NhgEntry -{ - /* Pointer to the next hop group. NhgOrch is the sole owner of it. */ - std::unique_ptr nhg; - - /* Number of external objects referencing this next hop group. */ - unsigned int ref_count; - - NhgEntry() = default; - explicit NhgEntry(std::unique_ptr&& _nhg, - unsigned int _ref_count = 0) : - nhg(std::move(_nhg)), ref_count(_ref_count) {} -}; - -/* - * Map indexed by next hop group's CP ID, containing the next hop group for - * that ID and the number of objects referencing it. - */ -typedef std::unordered_map NhgTable; + const NhgBase& getNhg(const string &index) const + { + SWSS_LOG_ENTER(); + + try + { + return nhgHandler.getNhg(index); + } + catch(const std::out_of_range &e) + { + return cbfNhgHandler.getNhg(index); + } + } -/* - * Next Hop Group Orchestrator class that handles NEXTHOP_GROUP_TABLE - * updates. - */ -class NhgOrch : public Orch -{ -public: /* - * Constructor. + * Increase the reference counter for the next hop group with the given + * index. */ - NhgOrch(DBConnector *db, string tableName); - - /* Check if the next hop group given by it's index exists. */ - inline bool hasNhg(const std::string& index) const + void incNhgRefCount(const string &index) { - return m_syncdNextHopGroups.find(index) != m_syncdNextHopGroups.end(); + SWSS_LOG_ENTER(); + + if (nhgHandler.hasNhg(index)) + { + nhgHandler.incNhgRefCount(index); + } + else + { + cbfNhgHandler.incNhgRefCount(index); + } } /* - * Get the next hop group given by it's index. If the index does not exist - * in map, a std::out_of_range exception will be thrown. + * Decrease the reference counter for the next hop group with the given + * index. */ - inline const NextHopGroup& getNhg(const std::string& index) const - { return *m_syncdNextHopGroups.at(index).nhg; } - - /* Add a temporary next hop group when resources are exhausted. */ - NextHopGroup createTempNhg(const NextHopGroupKey& nhg_key); - - /* Getters / Setters. */ - inline unsigned int getMaxNhgCount() const { return m_maxNhgCount; } - static inline unsigned int getNhgCount() { return NextHopGroup::getCount(); } - - /* Validate / Invalidate a next hop. */ - bool validateNextHop(const NextHopKey& nh_key); - bool invalidateNextHop(const NextHopKey& nh_key); - - /* Increase / Decrease the number of next hop groups. */ - inline void incNhgCount() + void decNhgRefCount(const string &index) { - assert(NextHopGroup::getCount() < m_maxNhgCount); - NextHopGroup::incCount(); + SWSS_LOG_ENTER(); + + if (nhgHandler.hasNhg(index)) + { + nhgHandler.decNhgRefCount(index); + } + else + { + cbfNhgHandler.decNhgRefCount(index); + } } - inline void decNhgCount() { NextHopGroup::decCount(); } - /* Increase / Decrease ref count for a NHG given by it's index. */ - void incNhgRefCount(const std::string& index); - void decNhgRefCount(const std::string& index); + void doTask(Consumer &consumer) override + { + SWSS_LOG_ENTER(); + + if (!gPortsOrch->allPortsReady()) + { + return; + } + + string table_name = consumer.getTableName(); + + if (table_name == APP_NEXTHOP_GROUP_TABLE_NAME) + { + nhgHandler.doTask(consumer); + } + else if (table_name == APP_CLASS_BASED_NEXT_HOP_GROUP_TABLE_NAME) + { + cbfNhgHandler.doTask(consumer); + } + } /* Handling SAI status*/ task_process_status handleSaiCreateStatus(sai_api_t api, sai_status_t status, void *context = nullptr) @@ -233,17 +142,15 @@ class NhgOrch : public Orch bool parseHandleSaiStatusFailure(task_process_status status) { return Orch::parseHandleSaiStatusFailure(status); } -private: - /* - * Switch's maximum number of next hop groups capacity. + * Handlers dealing with the (non) CBF operations. */ - unsigned int m_maxNhgCount; + NhgHandler nhgHandler; + CbfNhgHandler cbfNhgHandler; +private: /* - * The next hop group table. + * Switch's maximum number of next hop groups capacity. */ - NhgTable m_syncdNextHopGroups; - - void doTask(Consumer& consumer); + static unsigned m_maxNhgCount; }; diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 9ef6cb07f3..285ae4a789 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -21,6 +21,8 @@ extern sai_switch_api_t* sai_switch_api; extern sai_object_id_t gSwitchId; extern bool gSaiRedisLogRotate; +unsigned NhgOrch::m_maxNhgCount = 0; + extern void syncd_apply_view(); /* * Global orch daemon variables @@ -32,6 +34,7 @@ IntfsOrch *gIntfsOrch; NeighOrch *gNeighOrch; RouteOrch *gRouteOrch; NhgOrch *gNhgOrch; +NhgMapOrch *gNhgMapOrch; FgNhgOrch *gFgNhgOrch; AclOrch *gAclOrch; PbhOrch *gPbhOrch; @@ -165,8 +168,8 @@ bool OrchDaemon::init() { APP_ROUTE_TABLE_NAME, routeorch_pri }, { APP_LABEL_ROUTE_TABLE_NAME, routeorch_pri } }; + gNhgOrch = new NhgOrch(m_applDb, {APP_NEXTHOP_GROUP_TABLE_NAME, APP_CLASS_BASED_NEXT_HOP_GROUP_TABLE_NAME}); gRouteOrch = new RouteOrch(m_applDb, route_tables, gSwitchOrch, gNeighOrch, gIntfsOrch, vrf_orch, gFgNhgOrch); - gNhgOrch = new NhgOrch(m_applDb, APP_NEXTHOP_GROUP_TABLE_NAME); CoppOrch *copp_orch = new CoppOrch(m_applDb, APP_COPP_TABLE_NAME); TunnelDecapOrch *tunnel_decap_orch = new TunnelDecapOrch(m_applDb, APP_TUNNEL_DECAP_TABLE_NAME); @@ -194,7 +197,9 @@ bool OrchDaemon::init() CFG_WRED_PROFILE_TABLE_NAME, CFG_TC_TO_PRIORITY_GROUP_MAP_TABLE_NAME, CFG_PFC_PRIORITY_TO_PRIORITY_GROUP_MAP_TABLE_NAME, - CFG_PFC_PRIORITY_TO_QUEUE_MAP_TABLE_NAME + CFG_PFC_PRIORITY_TO_QUEUE_MAP_TABLE_NAME, + CFG_DSCP_TO_FC_MAP_TABLE_NAME, + CFG_EXP_TO_FC_MAP_TABLE_NAME }; QosOrch *qos_orch = new QosOrch(m_configDb, qos_tables); @@ -294,6 +299,8 @@ bool OrchDaemon::init() TableConnector stateDbBfdSessionTable(m_stateDb, STATE_BFD_SESSION_TABLE_NAME); gBfdOrch = new BfdOrch(m_applDb, APP_BFD_SESSION_TABLE_NAME, stateDbBfdSessionTable); + gNhgMapOrch = new NhgMapOrch(m_applDb, APP_FC_TO_NHG_INDEX_MAP_TABLE_NAME); + /* * The order of the orch list is important for state restore of warm start and * the queued processing in m_toSync map after gPortsOrch->allPortsReady() is set. @@ -302,7 +309,7 @@ bool OrchDaemon::init() * when iterating ConsumerMap. This is ensured implicitly by the order of keys in ordered map. * For cases when Orch has to process tables in specific order, like PortsOrch during warm start, it has to override Orch::doTask() */ - m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, mux_orch, mux_cb_orch, gIntfsOrch, gNeighOrch, gNhgOrch, gRouteOrch, copp_orch, qos_orch, wm_orch, policer_orch, tunnel_decap_orch, sflow_orch, debug_counter_orch, gMacsecOrch, gBfdOrch}; + m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, mux_orch, mux_cb_orch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gRouteOrch, copp_orch, qos_orch, wm_orch, policer_orch, tunnel_decap_orch, sflow_orch, debug_counter_orch, gMacsecOrch, gBfdOrch}; bool initialize_dtel = false; if (platform == BFN_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING) diff --git a/orchagent/orchdaemon.h b/orchagent/orchdaemon.h index 2e88447ce7..989a39c902 100644 --- a/orchagent/orchdaemon.h +++ b/orchagent/orchdaemon.h @@ -12,6 +12,7 @@ #include "neighorch.h" #include "routeorch.h" #include "nhgorch.h" +#include "nhgmaporch.h" #include "copporch.h" #include "tunneldecaporch.h" #include "qosorch.h" diff --git a/orchagent/qosorch.cpp b/orchagent/qosorch.cpp index 1430aceef1..fbb8c4a6b5 100644 --- a/orchagent/qosorch.cpp +++ b/orchagent/qosorch.cpp @@ -3,12 +3,14 @@ #include "logger.h" #include "crmorch.h" #include "sai_serialize.h" +#include "nhgmaporch.h" #include #include #include #include #include +#include using namespace std; @@ -51,7 +53,9 @@ map qos_to_attr_map = { {tc_to_pg_map_field_name, SAI_PORT_ATTR_QOS_TC_TO_PRIORITY_GROUP_MAP}, {pfc_to_pg_map_name, SAI_PORT_ATTR_QOS_PFC_PRIORITY_TO_PRIORITY_GROUP_MAP}, {pfc_to_queue_map_name, SAI_PORT_ATTR_QOS_PFC_PRIORITY_TO_QUEUE_MAP}, - {scheduler_field_name, SAI_PORT_ATTR_QOS_SCHEDULER_PROFILE_ID} + {scheduler_field_name, SAI_PORT_ATTR_QOS_SCHEDULER_PROFILE_ID}, + {dscp_to_fc_field_name, SAI_PORT_ATTR_QOS_DSCP_TO_FORWARDING_CLASS_MAP}, + {exp_to_fc_field_name, SAI_PORT_ATTR_QOS_MPLS_EXP_TO_FORWARDING_CLASS_MAP} }; map scheduler_meter_map = { @@ -70,7 +74,9 @@ type_map QosOrch::m_qos_maps = { {CFG_QUEUE_TABLE_NAME, new object_reference_map()}, {CFG_TC_TO_PRIORITY_GROUP_MAP_TABLE_NAME, new object_reference_map()}, {CFG_PFC_PRIORITY_TO_PRIORITY_GROUP_MAP_TABLE_NAME, new object_reference_map()}, - {CFG_PFC_PRIORITY_TO_QUEUE_MAP_TABLE_NAME, new object_reference_map()} + {CFG_PFC_PRIORITY_TO_QUEUE_MAP_TABLE_NAME, new object_reference_map()}, + {CFG_DSCP_TO_FC_MAP_TABLE_NAME, new object_reference_map()}, + {CFG_EXP_TO_FC_MAP_TABLE_NAME, new object_reference_map()}, }; map qos_to_ref_table_map = { @@ -82,9 +88,13 @@ map qos_to_ref_table_map = { {pfc_to_pg_map_name, CFG_PFC_PRIORITY_TO_PRIORITY_GROUP_MAP_TABLE_NAME}, {pfc_to_queue_map_name, CFG_PFC_PRIORITY_TO_QUEUE_MAP_TABLE_NAME}, {scheduler_field_name, CFG_SCHEDULER_TABLE_NAME}, - {wred_profile_field_name, CFG_WRED_PROFILE_TABLE_NAME} + {wred_profile_field_name, CFG_WRED_PROFILE_TABLE_NAME}, + {dscp_to_fc_field_name, CFG_DSCP_TO_FC_MAP_TABLE_NAME}, + {exp_to_fc_field_name, CFG_EXP_TO_FC_MAP_TABLE_NAME} }; +#define DSCP_MAX_VAL 63 +#define EXP_MAX_VAL 7 task_process_status QosMapHandler::processWorkItem(Consumer& consumer) { @@ -794,6 +804,180 @@ sai_object_id_t PfcToQueueHandler::addQosItem(const vector &att } +bool DscpToFcMapHandler::convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &tuple, vector &attributes) +{ + SWSS_LOG_ENTER(); + + sai_uint8_t max_fc_val = NhgMapOrch::getMaxFcVal(); + + sai_attribute_t list_attr; + list_attr.id = SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST; + list_attr.value.qosmap.count = (uint32_t)kfvFieldsValues(tuple).size(); + list_attr.value.qosmap.list = new sai_qos_map_t[list_attr.value.qosmap.count](); + uint32_t ind = 0; + + for (auto i = kfvFieldsValues(tuple).begin(); i != kfvFieldsValues(tuple).end(); i++, ind++) + { + try + { + auto value = stoi(fvField(*i)); + if (value < 0) + { + SWSS_LOG_ERROR("DSCP value %d is negative", value); + return false; + } + else if (value > DSCP_MAX_VAL) + { + SWSS_LOG_ERROR("DSCP value %d is greater than max value %d", value, DSCP_MAX_VAL); + return false; + } + list_attr.value.qosmap.list[ind].key.dscp = static_cast(value); + + value = stoi(fvValue(*i)); + if ((value < 0) || (value > max_fc_val)) + { + SWSS_LOG_ERROR("FC value %d is either negative, or bigger than max value %d", value, max_fc_val); + return false; + } + list_attr.value.qosmap.list[ind].value.fc = static_cast(value); + + SWSS_LOG_DEBUG("key.dscp:%d, value.fc:%d", + list_attr.value.qosmap.list[ind].key.dscp, + list_attr.value.qosmap.list[ind].value.fc); + } + catch(const invalid_argument& e) + { + SWSS_LOG_ERROR("Got exception during conversion: %s", e.what()); + return false; + } + } + attributes.push_back(list_attr); + return true; +} + +sai_object_id_t DscpToFcMapHandler::addQosItem(const vector &attributes) +{ + SWSS_LOG_ENTER(); + sai_status_t sai_status; + sai_object_id_t sai_object; + vector qos_map_attrs; + + sai_attribute_t qos_map_attr; + qos_map_attr.id = SAI_QOS_MAP_ATTR_TYPE; + qos_map_attr.value.u32 = SAI_QOS_MAP_TYPE_DSCP_TO_FORWARDING_CLASS; + qos_map_attrs.push_back(qos_map_attr); + + qos_map_attr.id = SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST; + qos_map_attr.value.qosmap.count = attributes[0].value.qosmap.count; + qos_map_attr.value.qosmap.list = attributes[0].value.qosmap.list; + qos_map_attrs.push_back(qos_map_attr); + + sai_status = sai_qos_map_api->create_qos_map(&sai_object, + gSwitchId, + (uint32_t)qos_map_attrs.size(), + qos_map_attrs.data()); + if (SAI_STATUS_SUCCESS != sai_status) + { + SWSS_LOG_ERROR("Failed to create dscp_to_fc map. status:%d", sai_status); + return SAI_NULL_OBJECT_ID; + } + SWSS_LOG_DEBUG("created QosMap object:%" PRIx64, sai_object); + return sai_object; +} + +task_process_status QosOrch::handleDscpToFcTable(Consumer& consumer) +{ + SWSS_LOG_ENTER(); + DscpToFcMapHandler dscp_fc_handler; + return dscp_fc_handler.processWorkItem(consumer); +} + +bool ExpToFcMapHandler::convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &tuple, + vector &attributes) +{ + SWSS_LOG_ENTER(); + + sai_uint8_t max_fc_val = NhgMapOrch::getMaxFcVal(); + + sai_attribute_t list_attr; + list_attr.id = SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST; + list_attr.value.qosmap.count = (uint32_t)kfvFieldsValues(tuple).size(); + list_attr.value.qosmap.list = new sai_qos_map_t[list_attr.value.qosmap.count](); + uint32_t ind = 0; + + for (auto i = kfvFieldsValues(tuple).begin(); i != kfvFieldsValues(tuple).end(); i++, ind++) + { + try + { + auto value = stoi(fvField(*i)); + if (value < 0) + { + SWSS_LOG_ERROR("EXP value %d is negative", value); + return false; + } + else if (value > EXP_MAX_VAL) + { + SWSS_LOG_ERROR("EXP value %d is greater than max value %d", value, EXP_MAX_VAL); + return false; + } + list_attr.value.qosmap.list[ind].key.mpls_exp = static_cast(value); + + value = stoi(fvValue(*i)); + if ((value < 0) || (value > max_fc_val)) + { + SWSS_LOG_ERROR("FC value %d is either negative, or bigger than max value %hu", value, max_fc_val); + return false; + } + list_attr.value.qosmap.list[ind].value.fc = static_cast(value); + + SWSS_LOG_DEBUG("key.mpls_exp:%d, value.fc:%d", + list_attr.value.qosmap.list[ind].key.mpls_exp, + list_attr.value.qosmap.list[ind].value.fc); + } + catch(const invalid_argument& e) + { + SWSS_LOG_ERROR("Got exception during conversion: %s", e.what()); + return false; + } + } + attributes.push_back(list_attr); + return true; +} + +sai_object_id_t ExpToFcMapHandler::addQosItem(const vector &attributes) +{ + SWSS_LOG_ENTER(); + sai_status_t sai_status; + sai_object_id_t sai_object; + vector qos_map_attrs; + + sai_attribute_t qos_map_attr; + qos_map_attr.id = SAI_QOS_MAP_ATTR_TYPE; + qos_map_attr.value.u32 = SAI_QOS_MAP_TYPE_MPLS_EXP_TO_FORWARDING_CLASS; + qos_map_attrs.push_back(qos_map_attr); + + qos_map_attr.id = SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST; + qos_map_attr.value.qosmap.count = attributes[0].value.qosmap.count; + qos_map_attr.value.qosmap.list = attributes[0].value.qosmap.list; + qos_map_attrs.push_back(qos_map_attr); + + sai_status = sai_qos_map_api->create_qos_map(&sai_object, gSwitchId, (uint32_t)qos_map_attrs.size(), qos_map_attrs.data()); + if (SAI_STATUS_SUCCESS != sai_status) + { + SWSS_LOG_ERROR("Failed to create exp_to_fc map. status:%d", sai_status); + return SAI_NULL_OBJECT_ID; + } + SWSS_LOG_DEBUG("created QosMap object:%" PRIx64, sai_object); + return sai_object; +} + +task_process_status QosOrch::handleExpToFcTable(Consumer& consumer) +{ + SWSS_LOG_ENTER(); + ExpToFcMapHandler exp_fc_handler; + return exp_fc_handler.processWorkItem(consumer); +} + task_process_status QosOrch::handlePfcToQueueTable(Consumer& consumer) { SWSS_LOG_ENTER(); @@ -825,6 +1009,8 @@ void QosOrch::initTableHandlers() m_qos_handler_map.insert(qos_handler_pair(CFG_QUEUE_TABLE_NAME, &QosOrch::handleQueueTable)); m_qos_handler_map.insert(qos_handler_pair(CFG_PORT_QOS_MAP_TABLE_NAME, &QosOrch::handlePortQosMapTable)); m_qos_handler_map.insert(qos_handler_pair(CFG_WRED_PROFILE_TABLE_NAME, &QosOrch::handleWredProfileTable)); + m_qos_handler_map.insert(qos_handler_pair(CFG_DSCP_TO_FC_MAP_TABLE_NAME, &QosOrch::handleDscpToFcTable)); + m_qos_handler_map.insert(qos_handler_pair(CFG_EXP_TO_FC_MAP_TABLE_NAME, &QosOrch::handleExpToFcTable)); m_qos_handler_map.insert(qos_handler_pair(CFG_TC_TO_PRIORITY_GROUP_MAP_TABLE_NAME, &QosOrch::handleTcToPgTable)); m_qos_handler_map.insert(qos_handler_pair(CFG_PFC_PRIORITY_TO_PRIORITY_GROUP_MAP_TABLE_NAME, &QosOrch::handlePfcPrioToPgTable)); diff --git a/orchagent/qosorch.h b/orchagent/qosorch.h index e65c3fa028..5e8f41e7c6 100644 --- a/orchagent/qosorch.h +++ b/orchagent/qosorch.h @@ -25,6 +25,8 @@ const string green_min_threshold_field_name = "green_min_threshold"; const string red_drop_probability_field_name = "red_drop_probability"; const string yellow_drop_probability_field_name = "yellow_drop_probability"; const string green_drop_probability_field_name = "green_drop_probability"; +const string dscp_to_fc_field_name = "dscp_to_fc_map"; +const string exp_to_fc_field_name = "exp_to_fc_map"; const string wred_profile_field_name = "wred_profile"; const string wred_red_enable_field_name = "wred_red_enable"; @@ -127,6 +129,20 @@ class PfcToQueueHandler : public QosMapHandler sai_object_id_t addQosItem(const vector &attributes); }; +class DscpToFcMapHandler : public QosMapHandler +{ +public: + bool convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &tuple, vector &attributes) override; + sai_object_id_t addQosItem(const vector &attributes) override; +}; + +class ExpToFcMapHandler : public QosMapHandler +{ +public: + bool convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &tuple, vector &attributes) override; + sai_object_id_t addQosItem(const vector &attributes) override; +}; + class QosOrch : public Orch { public: @@ -155,6 +171,8 @@ class QosOrch : public Orch task_process_status handleSchedulerTable(Consumer& consumer); task_process_status handleQueueTable(Consumer& consumer); task_process_status handleWredProfileTable(Consumer& consumer); + task_process_status handleDscpToFcTable(Consumer& consumer); + task_process_status handleExpToFcTable(Consumer& consumer); sai_object_id_t getSchedulerGroup(const Port &port, const sai_object_id_t queue_id); diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index e8ebf9a6c5..4b90f44480 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -721,8 +721,8 @@ void RouteOrch::doTask(Consumer& consumer) { try { - const NextHopGroup& nh_group = gNhgOrch->getNhg(nhg_index); - nhg = nh_group.getKey(); + const auto &nh_group = gNhgOrch->getNhg(nhg_index); + nhg = nh_group.getNhgKey(); ctx.using_temp_nhg = nh_group.isTemp(); } catch (const std::out_of_range& e) @@ -791,7 +791,7 @@ void RouteOrch::doTask(Consumer& consumer) // If already exhaust the nexthop groups, and there are pending removing routes in bulker, // flush the bulker and possibly collect some released nexthop groups - if (m_nextHopGroupCount + gNhgOrch->getNhgCount() >= m_maxNextHopGroupCount && + if (m_nextHopGroupCount + gNhgOrch->getSyncedNhgCount() >= m_maxNextHopGroupCount && gRouteBulker.removing_entries_count() > 0) { break; @@ -1053,7 +1053,7 @@ bool RouteOrch::createFineGrainedNextHopGroup(sai_object_id_t &next_hop_group_id { SWSS_LOG_ENTER(); - if (m_nextHopGroupCount + gNhgOrch->getNhgCount() >= m_maxNextHopGroupCount) + if (m_nextHopGroupCount + gNhgOrch->getSyncedNhgCount() >= m_maxNextHopGroupCount) { SWSS_LOG_DEBUG("Failed to create new next hop group. \ Reaching maximum number of next hop groups."); @@ -1108,7 +1108,7 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops) assert(!hasNextHopGroup(nexthops)); - if (m_nextHopGroupCount + gNhgOrch->getNhgCount() >= m_maxNextHopGroupCount) + if (m_nextHopGroupCount + gNhgOrch->getSyncedNhgCount() >= m_maxNextHopGroupCount) { SWSS_LOG_DEBUG("Failed to create new next hop group. \ Reaching maximum number of next hop groups."); @@ -1556,7 +1556,7 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) { try { - const NextHopGroup& nhg = gNhgOrch->getNhg(ctx.nhg_index); + const auto &nhg = gNhgOrch->getNhg(ctx.nhg_index); next_hop_id = nhg.getId(); } catch(const std::out_of_range& e) diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index c75a9ad3ca..807c9ce64b 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -39,7 +39,10 @@ tests_SOURCES = aclorch_ut.cpp \ $(top_srcdir)/orchagent/routeorch.cpp \ $(top_srcdir)/orchagent/mplsrouteorch.cpp \ $(top_srcdir)/orchagent/fgnhgorch.cpp \ - $(top_srcdir)/orchagent/nhgorch.cpp \ + $(top_srcdir)/orchagent/nhgbase.cpp \ + $(top_srcdir)/orchagent/nhghandler.cpp \ + $(top_srcdir)/orchagent/cbfnhghandler.cpp \ + $(top_srcdir)/orchagent/nhgmaporch.cpp \ $(top_srcdir)/orchagent/neighorch.cpp \ $(top_srcdir)/orchagent/intfsorch.cpp \ $(top_srcdir)/orchagent/portsorch.cpp \ diff --git a/tests/test_nhg.py b/tests/test_nhg.py index f1b600a22b..b57940021d 100644 --- a/tests/test_nhg.py +++ b/tests/test_nhg.py @@ -11,6 +11,7 @@ class TestNextHopGroupBase(object): ASIC_NHS_STR = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP" ASIC_NHG_STR = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP" + ASIC_NHG_MAP_STR = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MAP" ASIC_NHGM_STR = ASIC_NHG_STR + "_MEMBER" ASIC_RT_STR = "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY" ASIC_INSEG_STR = "ASIC_STATE:SAI_OBJECT_TYPE_INSEG_ENTRY" @@ -74,6 +75,45 @@ def get_nhgm_ids(self, nhg_index): return nhgms + def get_nhg_map_id(self, nhg_map_index): + nhg_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, swsscommon.APP_NEXTHOP_GROUP_TABLE_NAME) + cbf_nhg_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, + swsscommon.APP_CLASS_BASED_NEXT_HOP_GROUP_TABLE_NAME) + + asic_nhgs_count = len(self.asic_db.get_keys(self.ASIC_NHG_STR)) + + # Create a NHG + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), ('ifname', 'Ethernet0')]) + nhg_ps.set('testnhg', fvs) + + # Add a CBF NHG pointing to the given map + fvs = swsscommon.FieldValuePairs([('members', 'testnhg'), ('selection_map', nhg_map_index)]) + cbf_nhg_ps.set('testcbfnhg', fvs) + + # If the CBF NHG can't be created, the provided NHG map index is invalid + try: + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) + except: + # Remove the added NHGs + cbf_nhg_ps._del('testcbfnhg') + nhg_ps._del('testnhg') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count) + return None + + # Get the CBF NHG ID + cbf_nhg_id = self.get_nhg_id('testcbfnhg') + assert cbf_nhg_id != None + + # Get the NHG map id + nhg_map_id = self.asic_db.get_entry(self.ASIC_NHG_STR, cbf_nhg_id)['SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP'] + + # Remove the added NHGs + cbf_nhg_ps._del('testcbfnhg') + nhg_ps._del('testnhg') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count) + + return nhg_map_id + def port_name(self, i): return "Ethernet" + str(i * 4) @@ -116,6 +156,11 @@ def init_test(self, dvs, num_intfs): self.nhg_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, swsscommon.APP_NEXTHOP_GROUP_TABLE_NAME) self.rt_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, swsscommon.APP_ROUTE_TABLE_NAME) self.lr_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, swsscommon.APP_LABEL_ROUTE_TABLE_NAME) + self.cbf_nhg_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, swsscommon.APP_CLASS_BASED_NEXT_HOP_GROUP_TABLE_NAME) + self.fc_to_nhg_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, swsscommon.APP_FC_TO_NHG_INDEX_MAP_TABLE_NAME) + + # Set switch FC capability to 63 + self.dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_MAX_NUMBER_OF_FORWARDING_CLASSES', '63') for i in range(num_intfs): self.config_intf(i) @@ -125,11 +170,18 @@ def init_test(self, dvs, num_intfs): self.asic_insgs_count = len(self.asic_db.get_keys(self.ASIC_INSEG_STR)) self.asic_nhs_count = len(self.asic_db.get_keys(self.ASIC_NHS_STR)) self.asic_rts_count = len(self.asic_db.get_keys(self.ASIC_RT_STR)) + self.asic_nhg_maps_count = len(self.asic_db.get_keys(self.ASIC_NHG_MAP_STR)) def nhg_exists(self, nhg_index): return self.get_nhg_id(nhg_index) is not None -class TestNextHopGroupExhaust(TestNextHopGroupBase): + def route_exists(self, rt_prefix): + return self.get_route_id(rt_prefix) is not None + + def nhg_map_exists(self, nhg_map_index): + return self.get_nhg_map_id(nhg_map_index) is not None + +class TestNhgExhaustBase(TestNextHopGroupBase): MAX_ECMP_COUNT = 512 MAX_PORT_COUNT = 10 @@ -162,28 +214,200 @@ def gen_valid_binary(self): return binary gen_valid_binary.fmt = '{{0:0{}b}}'.format(MAX_PORT_COUNT) - def test_nhgorch_nhg_exhaust(self, dvs, testlog): - def gen_nhg_index(nhg_number): - return "group{}".format(nhg_number) - def create_temp_nhg(): +class TestRotueOrchNhgExhaust(TestNhgExhaustBase): + def test_route_nhg_exhaust(self, dvs, testlog): + """ + Test the situation of exhausting ECMP group, assume SAI_SWITCH_ATTR_NUMBER_OF_ECMP_GROUPS is 512 + + In order to achieve that, we will config + 1. 9 ports + 2. 512 routes with different nexthop group + + See Also + -------- + SwitchStateBase::set_number_of_ecmp_groups() + https://github.com/Azure/sonic-sairedis/blob/master/vslib/src/SwitchStateBase.cpp + + """ + + # TODO: check ECMP 512 + + def gen_ipprefix(r): + """ Construct route like 2.X.X.0/24 """ + ip = ipaddress.IPv4Address(IP_INTEGER_BASE + r * 256) + ip = str(ip) + ipprefix = ip + "/24" + return ipprefix + + def asic_route_nhg_fvs(k): + fvs = self.asic_db.get_entry(self.ASIC_RT_STR, k) + if not fvs: + return None + + nhgid = fvs.get("SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID") + if nhgid is None: + return None + + fvs = self.asic_db.get_entry(self.ASIC_NHG_STR, nhgid) + return fvs + + if sys.version_info < (3, 0): + IP_INTEGER_BASE = int(ipaddress.IPv4Address(unicode("2.2.2.0"))) + else: + IP_INTEGER_BASE = int(ipaddress.IPv4Address(str("2.2.2.0"))) + + self.init_test(dvs) + + # Add first batch of routes with unique nexthop groups in AppDB + route_count = 0 + while route_count < self.MAX_ECMP_COUNT: + binary = self.gen_valid_binary() + fvs = self.gen_nhg_fvs(binary) + route_ipprefix = gen_ipprefix(route_count) + self.rt_ps.set(route_ipprefix, fvs) + route_count += 1 + + # Wait and check ASIC DB the count of nexthop groups used + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Wait and check ASIC DB the count of routes + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + self.MAX_ECMP_COUNT) + self.asic_rts_count += self.MAX_ECMP_COUNT + + # Add a route with labeled NHs + self.asic_nhs_count = len(self.asic_db.get_keys(self.ASIC_NHS_STR)) + route_ipprefix = gen_ipprefix(route_count) + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('mpls_nh', 'push1,push3'), + ('ifname', 'Ethernet0,Ethernet4')]) + self.rt_ps.set(route_ipprefix, fvs) + route_count += 1 + + # A temporary route should be created + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) + + # A NH should be elected as the temporary NHG and it should be created + # as it doesn't exist. + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 1) + + # Delete the route. The route and the added labeled NH should be + # removed. + self.rt_ps._del(route_ipprefix) + route_count -= 1 + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count) + + # Add second batch of routes with unique nexthop groups in AppDB + # Add more routes with new nexthop group in AppDBdd + route_ipprefix = gen_ipprefix(route_count) + base_ipprefix = route_ipprefix + base = route_count + route_count = 0 + while route_count < 10: + binary = self.gen_valid_binary() + fvs = self.gen_nhg_fvs(binary) + route_ipprefix = gen_ipprefix(base + route_count) + self.rt_ps.set(route_ipprefix, fvs) + route_count += 1 + last_ipprefix = route_ipprefix + + # Wait until we get expected routes and check ASIC DB on the count of nexthop groups used, and it should not increase + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 10) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Check the route points to next hop group + # Note: no need to wait here + k = self.get_route_id("2.2.2.0/24") + assert k is not None + fvs = asic_route_nhg_fvs(k) + assert fvs is not None + + # Check the second batch does not point to next hop group + k = self.get_route_id(base_ipprefix) + assert k is not None + fvs = asic_route_nhg_fvs(k) + assert not(fvs) + + # Remove first batch of routes with unique nexthop groups in AppDB + route_count = 0 + self.r = 0 + while route_count < self.MAX_ECMP_COUNT: + route_ipprefix = gen_ipprefix(route_count) + self.rt_ps._del(route_ipprefix) + route_count += 1 + self.asic_rts_count -= self.MAX_ECMP_COUNT + + # Wait and check the second batch points to next hop group + # Check ASIC DB on the count of nexthop groups used, and it should not increase or decrease + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, 10) + k = self.get_route_id(base_ipprefix) + assert k is not None + fvs = asic_route_nhg_fvs(k) + assert fvs is not None + k = self.get_route_id(last_ipprefix) + assert k is not None + fvs = asic_route_nhg_fvs(k) + assert fvs is not None + + # Cleanup + + # Remove second batch of routes + for i in range(10): + route_ipprefix = gen_ipprefix(self.MAX_ECMP_COUNT + i) + self.rt_ps._del(route_ipprefix) + + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, 0) + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + + +class TestNhgOrchNhgExhaust(TestNhgExhaustBase): + + def init_test(self, dvs): + super().init_test(dvs) + + self.nhg_count = self.asic_nhgs_count + self.first_valid_nhg = self.nhg_count + self.asic_nhgs = {} + + # Add first batch of next hop groups to reach the NHG limit + while self.nhg_count < self.MAX_ECMP_COUNT: binary = self.gen_valid_binary() nhg_fvs = self.gen_nhg_fvs(binary) - nhg_index = gen_nhg_index(self.nhg_count) + nhg_index = self.gen_nhg_index(self.nhg_count) self.nhg_ps.set(nhg_index, nhg_fvs) + + # Save the NHG index/ID pair + self.asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index) + + # Increase the number of NHGs in ASIC DB self.nhg_count += 1 + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + def gen_nhg_index(self, nhg_number): + return "group{}".format(nhg_number) - return nhg_index, binary + def create_temp_nhg(self): + binary = self.gen_valid_binary() + nhg_fvs = self.gen_nhg_fvs(binary) + nhg_index = self.gen_nhg_index(self.nhg_count) + self.nhg_ps.set(nhg_index, nhg_fvs) + self.nhg_count += 1 - def delete_nhg(): - del_nhg_index = gen_nhg_index(self.first_valid_nhg) - del_nhg_id = self.asic_nhgs[del_nhg_index] + return nhg_index, binary - self.nhg_ps._del(del_nhg_index) - self.asic_nhgs.pop(del_nhg_index) - self.first_valid_nhg += 1 + def delete_nhg(self): + del_nhg_index = self.gen_nhg_index(self.first_valid_nhg) + del_nhg_id = self.asic_nhgs[del_nhg_index] - return del_nhg_id + self.nhg_ps._del(del_nhg_index) + self.asic_nhgs.pop(del_nhg_index) + self.first_valid_nhg += 1 + + return del_nhg_id + + + def test_nhgorch_nhg_exhaust(self, dvs, testlog): # Test scenario: # - create a NHG and assert a NHG object doesn't get added to ASIC DB @@ -191,7 +415,7 @@ def delete_nhg(): def temporary_group_promotion_test(): # Add a new next hop group - it should create a temporary one instead prev_nhgs = self.asic_db.get_keys(self.ASIC_NHG_STR) - nhg_index, _ = create_temp_nhg() + nhg_index, _ = self.create_temp_nhg() # Save the temporary NHG's SAI ID time.sleep(1) @@ -204,7 +428,7 @@ def temporary_group_promotion_test(): assert prev_nhgs == self.asic_db.get_keys(self.ASIC_NHG_STR) # Delete an existing next hop group - del_nhg_id = delete_nhg() + del_nhg_id = self.delete_nhg() # Wait for the key to be deleted self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) @@ -225,7 +449,7 @@ def group_update_test(): # Update a group binary = self.gen_valid_binary() nhg_fvs = self.gen_nhg_fvs(binary) - nhg_index = gen_nhg_index(self.first_valid_nhg) + nhg_index = self.gen_nhg_index(self.first_valid_nhg) # Save the previous members prev_nhg_members = self.get_nhgm_ids(nhg_index) @@ -241,7 +465,7 @@ def group_update_test(): # - create and delete a NHG while the ASIC DB is full and assert nothing changes def create_delete_temporary_test(): # Create a new temporary group - nhg_index, _ = create_temp_nhg() + nhg_index, _ = self.create_temp_nhg() time.sleep(1) # Delete the temporary group @@ -259,7 +483,7 @@ def create_delete_temporary_test(): # - delete a NHG and assert the new one is added and it has the updated number of members def update_temporary_group_test(): # Create a new temporary group - nhg_index, binary = create_temp_nhg() + nhg_index, binary = self.create_temp_nhg() # Save the number of group members binary_count = binary.count('1') @@ -275,7 +499,7 @@ def update_temporary_group_test(): self.nhg_ps.set(nhg_index, nhg_fvs) # Delete a group - del_nhg_id = delete_nhg() + del_nhg_id = self.delete_nhg() # Wait for the group to be deleted self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) @@ -296,7 +520,7 @@ def update_temporary_group_test(): # - delete a NHG and assert the temporary NHG is promoted and its SAI ID also changes def route_nhg_update_test(): # Add a route - nhg_index = gen_nhg_index(self.first_valid_nhg) + nhg_index = self.gen_nhg_index(self.first_valid_nhg) rt_fvs = swsscommon.FieldValuePairs([('nexthop_group', nhg_index)]) self.rt_ps.set('2.2.2.0/24', rt_fvs) @@ -307,7 +531,7 @@ def route_nhg_update_test(): prev_nhg_id = self.asic_nhgs[nhg_index] # Create a new temporary group - nhg_index, binary = create_temp_nhg() + nhg_index, binary = self.create_temp_nhg() # Get the route ID rt_id = self.get_route_id('2.2.2.0/24') @@ -349,7 +573,7 @@ def route_nhg_update_test(): {'SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID': prev_nhg_id}) # Delete a NHG. - del_nhg_id = delete_nhg() + del_nhg_id = self.delete_nhg() # Wait for the NHG to be deleted self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) @@ -376,7 +600,7 @@ def labeled_nhg_temporary_promotion_test(): fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('mpls_nh', 'push1,push3'), ('ifname', 'Ethernet0,Ethernet4')]) - nhg_index = gen_nhg_index(self.nhg_count) + nhg_index = self.gen_nhg_index(self.nhg_count) self.nhg_ps.set(nhg_index, fvs) self.nhg_count += 1 @@ -385,7 +609,7 @@ def labeled_nhg_temporary_promotion_test(): self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 1) # Delete a next hop group - delete_nhg() + self.delete_nhg() # The group should be promoted and the other labeled NH should also get # created @@ -409,7 +633,7 @@ def back_compatibility_test(): self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) # Delete a next hop group - del_nhg_id = delete_nhg() + del_nhg_id = self.delete_nhg() self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) # The temporary group should be promoted @@ -425,7 +649,7 @@ def invalid_temporary_test(): # Create a temporary NHG that contains only NHs that do not exist nhg_fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.21,10.0.0.23'), ('ifname', 'Ethernet40,Ethernet44')]) - nhg_index = gen_nhg_index(self.nhg_count) + nhg_index = self.gen_nhg_index(self.nhg_count) self.nhg_count += 1 self.nhg_ps.set(nhg_index, nhg_fvs) @@ -456,24 +680,6 @@ def invalid_temporary_test(): self.init_test(dvs) - self.nhg_count = self.asic_nhgs_count - self.first_valid_nhg = self.nhg_count - self.asic_nhgs = {} - - # Add first batch of next hop groups to reach the NHG limit - while self.nhg_count < self.MAX_ECMP_COUNT: - binary = self.gen_valid_binary() - nhg_fvs = self.gen_nhg_fvs(binary) - nhg_index = gen_nhg_index(self.nhg_count) - self.nhg_ps.set(nhg_index, nhg_fvs) - - # Save the NHG index/ID pair - self.asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index) - - # Increase the number of NHGs in ASIC DB - self.nhg_count += 1 - self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) - temporary_group_promotion_test() group_update_test() create_delete_temporary_test() @@ -495,149 +701,188 @@ def invalid_temporary_test(): self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) - def test_route_nhg_exhaust(self, dvs, testlog): - """ - Test the situation of exhausting ECMP group, assume SAI_SWITCH_ATTR_NUMBER_OF_ECMP_GROUPS is 512 - - In order to achieve that, we will config - 1. 9 ports - 2. 512 routes with different nexthop group - - See Also - -------- - SwitchStateBase::set_number_of_ecmp_groups() - https://github.com/Azure/sonic-sairedis/blob/master/vslib/src/SwitchStateBase.cpp - - """ - - # TODO: check ECMP 512 - - def gen_ipprefix(r): - """ Construct route like 2.X.X.0/24 """ - ip = ipaddress.IPv4Address(IP_INTEGER_BASE + r * 256) - ip = str(ip) - ipprefix = ip + "/24" - return ipprefix - - def asic_route_nhg_fvs(k): - fvs = self.asic_db.get_entry(self.ASIC_RT_STR, k) - if not fvs: - return None - - nhgid = fvs.get("SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID") - if nhgid is None: - return None - - fvs = self.asic_db.get_entry(self.ASIC_NHG_STR, nhgid) - return fvs - - if sys.version_info < (3, 0): - IP_INTEGER_BASE = int(ipaddress.IPv4Address(unicode("2.2.2.0"))) - else: - IP_INTEGER_BASE = int(ipaddress.IPv4Address(str("2.2.2.0"))) - + def test_cbf_nhg_exhaust(self, dvs, testlog): self.init_test(dvs) - # Add first batch of routes with unique nexthop groups in AppDB - route_count = 0 - while route_count < self.MAX_ECMP_COUNT: - binary = self.gen_valid_binary() - fvs = self.gen_nhg_fvs(binary) - route_ipprefix = gen_ipprefix(route_count) - self.rt_ps.set(route_ipprefix, fvs) - route_count += 1 - - # Wait and check ASIC DB the count of nexthop groups used - self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) - - # Wait and check ASIC DB the count of routes - self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + self.MAX_ECMP_COUNT) - self.asic_rts_count += self.MAX_ECMP_COUNT + # Create an FC to NH index selection map + fvs = swsscommon.FieldValuePairs([('0', '0')]) + self.fc_to_nhg_ps.set('cbfnhgmap1', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count + 1) - # Add a route with labeled NHs - self.asic_nhs_count = len(self.asic_db.get_keys(self.ASIC_NHS_STR)) - route_ipprefix = gen_ipprefix(route_count) - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), - ('mpls_nh', 'push1,push3'), - ('ifname', 'Ethernet0,Ethernet4')]) - self.rt_ps.set(route_ipprefix, fvs) - route_count += 1 + # Create a CBF NHG group - it should fail + fvs = swsscommon.FieldValuePairs([('members', 'group2'), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + assert(not self.nhg_exists('cbfgroup1')) - # A temporary route should be created - self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) + # Delete a NHG + del_nhg_id = self.delete_nhg() + self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) - # A NH should be elected as the temporary NHG and it should be created - # as it doesn't exist. - self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 1) + # The CBF NHG should be created + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + assert(self.nhg_exists('cbfgroup1')) + nhg_id = self.get_nhg_id('cbfgroup1') - # Delete the route. The route and the added labeled NH should be - # removed. - self.rt_ps._del(route_ipprefix) - route_count -= 1 - self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) - self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count) + # Create a temporary NHG + nhg_index, _ = self.create_temp_nhg() - # Add second batch of routes with unique nexthop groups in AppDB - # Add more routes with new nexthop group in AppDBdd - route_ipprefix = gen_ipprefix(route_count) - base_ipprefix = route_ipprefix - base = route_count - route_count = 0 - while route_count < 10: - binary = self.gen_valid_binary() - fvs = self.gen_nhg_fvs(binary) - route_ipprefix = gen_ipprefix(base + route_count) - self.rt_ps.set(route_ipprefix, fvs) - route_count += 1 - last_ipprefix = route_ipprefix + # Update the CBF NHG to contain the temporary NHG + fvs = swsscommon.FieldValuePairs([('members', nhg_index), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + nhgm_id = self.get_nhgm_ids('cbfgroup1')[0] + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_id) + assert(fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID'] != self.get_nhg_id('group2')) + + # Coverage testing: CbfNhgHandler::hasSameMembers() returns false + # + # Update the group while it has a temporary NHG to contain one more member. + # + # Update the group keeping the same number of members, but changing the fully fledged NHG with another one. + # + # Update the group reversing the order of the 2 members. + # + # Update the CBF NHG back to its original form to resume testing. + fvs = swsscommon.FieldValuePairs([('members', '{},group3'.format(nhg_index)), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + fvs = swsscommon.FieldValuePairs([('members', '{},group4'.format(nhg_index)), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + fvs = swsscommon.FieldValuePairs([('members', 'group4,{}'.format(nhg_index)), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + fvs = swsscommon.FieldValuePairs([('members', nhg_index), ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + + # Delete a NHG + nh_id = self.get_nhg_id(nhg_index) + del_nhg_id = self.delete_nhg() + self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + + # The NHG should be promoted and the CBF NHG member updated + nhgm_id = self.get_nhgm_ids('cbfgroup1')[0] + self.asic_db.wait_for_field_negative_match(self.ASIC_NHGM_STR, + nhgm_id, + {'SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID': nh_id}) + self.asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index) + + # Create a couple more temporary NHGs + nhg_index1, _ = self.create_temp_nhg() + nhg_index2, _ = self.create_temp_nhg() + + # Update the CBF NHG to contain both of them as well + fvs = swsscommon.FieldValuePairs([('members', '{},{},{}'.format(nhg_index, nhg_index1, nhg_index2)), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + + # The previous CBF NHG member should be deleted + self.asic_db.wait_for_deleted_entry(self.ASIC_NHGM_STR, nhgm_id) + + # Create an FC to NH index selection map + fvs = swsscommon.FieldValuePairs([('0', '0')]) + self.fc_to_nhg_ps.set('cbfnhgmap2', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count + 2) + + # Update the selection map of the group + old_map = self.asic_db.get_entry(self.ASIC_NHG_STR, nhg_id)['SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP'] + nhgm_ids = self.get_nhgm_ids('cbfgroup1') + members = {nhgm_id: self.asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_id) for nhgm_id in nhgm_ids} + fvs = swsscommon.FieldValuePairs([('members', '{},{},{}'.format(nhg_index, nhg_index1, nhg_index2)), + ('selection_map', 'cbfnhgmap2')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + + # Check that the map was updates, but the members weren't + self.asic_db.wait_for_field_negative_match(self.ASIC_NHG_STR, + nhg_id, + {'SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP': old_map}) + values = {} + for nhgm_id in nhgm_ids: + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_id) + assert(members[nhgm_id] == fvs) + values[nhgm_id] = fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID'] + + # Gradually delete temporary NHGs and check that the NHG members are + # updated + for i in range(2): + # Delete a temporary NHG + del_nhg_id = self.delete_nhg() + self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + time.sleep(1) - # Wait until we get expected routes and check ASIC DB on the count of nexthop groups used, and it should not increase - self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 10) - self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + # Sleep 1 second to allow CBF NHG to update its member NH ID + time.sleep(1) - # Check the route points to next hop group - # Note: no need to wait here - k = self.get_route_id("2.2.2.0/24") - assert k is not None - fvs = asic_route_nhg_fvs(k) - assert fvs is not None + # Check that one of the temporary NHGs has been updated + diff_count = 0 + for nhgm_id, nh_id in values.items(): + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_id) + if nh_id != fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID']: + values[nhgm_id] = fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID'] + diff_count += 1 + assert(diff_count == 1) - # Check the second batch does not point to next hop group - k = self.get_route_id(base_ipprefix) - assert k is not None - fvs = asic_route_nhg_fvs(k) - assert not(fvs) + for index in [nhg_index1, nhg_index2]: + self.asic_nhgs[index] = self.get_nhg_id(index) - # Remove first batch of routes with unique nexthop groups in AppDB - route_count = 0 - self.r = 0 - while route_count < self.MAX_ECMP_COUNT: - route_ipprefix = gen_ipprefix(route_count) - self.rt_ps._del(route_ipprefix) - route_count += 1 - self.asic_rts_count -= self.MAX_ECMP_COUNT + # Create a temporary NHG + nhg_index, binary = self.create_temp_nhg() - # Wait and check the second batch points to next hop group - # Check ASIC DB on the count of nexthop groups used, and it should not increase or decrease - self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, 10) - k = self.get_route_id(base_ipprefix) - assert k is not None - fvs = asic_route_nhg_fvs(k) - assert fvs is not None - k = self.get_route_id(last_ipprefix) - assert k is not None - fvs = asic_route_nhg_fvs(k) - assert fvs is not None + # Update the CBF NHG to point to the new NHG + nhgm_ids = self.get_nhgm_ids('cbfgroup1') + fvs = swsscommon.FieldValuePairs([('members', nhg_index), ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + self.asic_db.wait_for_deleted_keys(self.ASIC_NHGM_STR, nhgm_ids) - # Cleanup + # Update the temporary NHG to force using a new designated NH + new_binary = [] - # Remove second batch of routes - for i in range(10): - route_ipprefix = gen_ipprefix(self.MAX_ECMP_COUNT + i) - self.rt_ps._del(route_ipprefix) + for i in range(len(binary)): + if binary[i] == '1': + new_binary.append('0') + else: + new_binary.append('1') + + binary = ''.join(new_binary) + assert binary.count('1') > 1 + + nh_id = self.get_nhg_id(nhg_index) + nhg_fvs = self.gen_nhg_fvs(binary) + self.nhg_ps.set(nhg_index, nhg_fvs) + + # The CBF NHG member's NH attribute should be updated + nhgm_id = self.get_nhgm_ids('cbfgroup1')[0] + self.asic_db.wait_for_field_negative_match(self.ASIC_NHGM_STR, + nhgm_id, + {'SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID': nh_id}) + + # Delete a NHG + del_nhg_id = self.delete_nhg() + self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + + # The temporary group should be promoted + nh_id = self.get_nhg_id(nhg_index) + self.asic_db.wait_for_field_match(self.ASIC_NHGM_STR, + nhgm_id, + {'SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID': nh_id}) + self.asic_nhgs[nhg_index] = nh_id + + # Delete the NHGs + self.cbf_nhg_ps._del('cbfgroup1') + for k in self.asic_nhgs: + self.nhg_ps._del(k) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) - self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, 0) - self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + # Delete the NHG maps + self.fc_to_nhg_ps._del('cbfnhgmap1') + self.fc_to_nhg_ps._del('cbfnhgmap2') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count) class TestNextHopGroup(TestNextHopGroupBase): @@ -956,9 +1201,9 @@ def mainline_labeled_nhs_test(): def routeorch_nhgorch_interop_test(): # Create a route with labeled NHs fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), - ('mpls_nh', 'push1,push3'), - ('ifname', 'Ethernet0,Ethernet4'), - ('weight', '2,4')]) + ('mpls_nh', 'push1,push3'), + ('ifname', 'Ethernet0,Ethernet4'), + ('weight', '2,4')]) self.rt_ps.set('2.2.2.0/24', fvs) self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) @@ -1379,6 +1624,495 @@ def test_nhgorch_label_route(self, dvs, testlog): self.nhg_ps._del("group1") self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) +class TestCbfNextHopGroup(TestNextHopGroupBase): + MAX_NHG_MAP_COUNT = 512 + + def test_cbf_nhgorch(self, dvs, testlog): + + # Test scenario: + # - create two NHGs and a NHG map + # - create a CBF NHG and assert it has the expected details + # - delete the CBF NHG + def mainline_cbf_nhg_test(): + # Create two non-CBF NHGs + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5'), + ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) + self.nhg_ps.set("group1", fvs) + + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('ifname', 'Ethernet0,Ethernet4')]) + self.nhg_ps.set('group2', fvs) + + # Wait for the groups to appear in ASIC DB + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + + # Create an FC to NH index selection map + nhg_map = [(str(i), '0' if i < 4 else '1') for i in range(8)] + fvs = swsscommon.FieldValuePairs(nhg_map) + self.fc_to_nhg_ps.set('cbfnhgmap1', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count + 1) + + # Create a CBF NHG + fvs = swsscommon.FieldValuePairs([('members', 'group1,group2'), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + + # Check if the group makes it to ASIC DB + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 3) + nhgid = self.get_nhg_id('cbfgroup1') + assert(nhgid != None) + fvs = self.asic_db.get_entry(self.ASIC_NHG_STR, nhgid) + assert(fvs['SAI_NEXT_HOP_GROUP_ATTR_TYPE'] == 'SAI_NEXT_HOP_GROUP_TYPE_CLASS_BASED') + + # Check if the next hop group members get created + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 7) + nhgm_ids = self.get_nhgm_ids('cbfgroup1') + assert(len(nhgm_ids) == 2) + nhg_ids = dict(zip([self.get_nhg_id(x) for x in ['group1', 'group2']], ['0', '1'])) + for nhgm_id in nhgm_ids: + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_id) + nh_id = fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID'] + assert(fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_INDEX'] == nhg_ids[nh_id]) + nhg_ids.pop(nh_id) + + # Delete the CBF next hop group + self.cbf_nhg_ps._del('cbfgroup1') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + + # Test scenario: + # - try creating invalid NHG maps and assert they're not being created + def data_validation_test(): + # Test the data validation + fv_values = [ + ('', 'cbfnhgmap1'), # empty members + ('group1,group2', ''), # non-existent selection map + ('group1,group1', 'cbfnhgmap1'), # non-unique members + ] + + for members, selection_map in fv_values: + fvs = swsscommon.FieldValuePairs([('members', members), + ('selection_map', selection_map)]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + assert(len(self.asic_db.get_keys(self.ASIC_NHG_STR)) == self.asic_nhgs_count + 2) + + # Test scenario: + # - create a CBF NHG + # - try deleting one of its members and assert it fails as it is being referenced + # - update the group replacing the to-be-deleted member with another NHG and assert it is deleted + # - update the CBF NHG reordering the members and assert the new details match + def update_cbf_nhg_members_test(): + # Create a NHG with a single next hop + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), + ("ifname", "Ethernet0")]) + self.nhg_ps.set("group3", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 3) + + # Create a CBF NHG + fvs = swsscommon.FieldValuePairs([('members', 'group1,group3'), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 4) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 8) + + # Remove group1 while being referenced - should not work + self.nhg_ps._del('group1') + time.sleep(1) + assert(self.nhg_exists('group1')) + + # Update the CBF NHG replacing group1 with group2 + nhgm_ids = self.get_nhgm_ids('cbfgroup1') + fvs = swsscommon.FieldValuePairs([('members', 'group2,group3'), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + self.asic_db.wait_for_deleted_keys(self.ASIC_NHGM_STR, nhgm_ids) + # group1 should be deleted and the updated members added + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 3) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 5) + + # Update the CBF NHG changing the order of the members + nhgm_ids = self.get_nhgm_ids('cbfgroup1') + fvs = swsscommon.FieldValuePairs([('members', 'group3,group2'), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + self.asic_db.wait_for_deleted_keys(self.ASIC_NHGM_STR, nhgm_ids) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 5) + assert(len(self.asic_db.get_keys(self.ASIC_NHG_STR)) == self.asic_nhgs_count + 3) + + indexes = { + self.get_nhg_id('group3'): '0', + self.get_nhg_id('group2'): '1' + } + nhgm_ids = self.get_nhgm_ids('cbfgroup1') + for nhgm_id in nhgm_ids: + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_id) + nh_id = fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID'] + assert(fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_INDEX'] == indexes[nh_id]) + indexes.pop(nh_id) + + # Test scenario: + # - update the selection map of the CBF NHG and assert it changes in ASIC DB + def update_cbf_nhg_map_test(): + # Create an FC to NH index selection map + fvs = swsscommon.FieldValuePairs([('0', '1'), ('1', '0')]) + self.fc_to_nhg_ps.set('cbfnhgmap2', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count + 2) + + # Update the CBF NHG class map + nhg_id = self.get_nhg_id('cbfgroup1') + old_map = self.get_nhg_map_id('cbfnhgmap1') + fvs = swsscommon.FieldValuePairs([('members', 'group3,group2'), + ('selection_map', 'cbfnhgmap2')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + self.asic_db.wait_for_field_negative_match(self.ASIC_NHG_STR, + nhg_id, + {'SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP': old_map}) + assert(len(self.asic_db.get_keys(self.ASIC_NHG_STR)) == self.asic_nhgs_count + 3) + + # Test scenario: + # - create a route pointing to the CBF NHG + # - try deleting the CBF NHG and assert it fails as it is being referenced + # - delete the route and assert the CBF NHG also gets deleted + def delete_referenced_cbf_nhg_test(): + # Create a route pointing to the CBF NHG + fvs = swsscommon.FieldValuePairs([('nexthop_group', 'cbfgroup1')]) + self.rt_ps.set('2.2.2.0/24', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) + + # Try deleting the CBF NHG - should not work + self.cbf_nhg_ps._del('cbfgroup1') + time.sleep(1) + assert(self.nhg_exists('cbfgroup1')) + + # Delete the route - the CBF NHG should also get deleted + self.rt_ps._del('2.2.2.0/24') + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 3) + + # Test scenario: + # - create a route pointing to a CBF NHG that doesn't exist and assert it isn't created + # - create the CBF NHG and assert the route also gets created + def create_route_inexistent_cbf_nhg_test(): + # Create a route pointing to a CBF NHG that does not yet exist + fvs = swsscommon.FieldValuePairs([('nexthop_group', 'cbfgroup1')]) + self.rt_ps.set('2.2.2.0/24', fvs) + time.sleep(1) + assert(not self.route_exists('2.2.2.0/24')) + + # Create the CBF NHG + fvs = swsscommon.FieldValuePairs([('members', 'group3,group2'), + ('selection_map', 'cbfnhgmap2')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 3) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 5) + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) + + # Test scenario: + # - try deleting the CBF NHG and assert it fails as it is being referenced + # - update the CBF NHG + # - delete the route pointing to the CBF NHG and assert the CBF NHG doesn't get deleted + def update_deleting_cbf_nhg_test(): + # Try deleting the CBF group + self.cbf_nhg_ps._del('cbfgroup1') + time.sleep(1) + assert(self.nhg_exists('cbfgroup1')) + + # Update the CBF group + nhgm_ids = self.get_nhgm_ids('cbfgroup1') + nhg_id = self.get_nhg_id('cbfgroup1') + old_map = self.asic_db.get_entry(self.ASIC_NHG_STR, nhg_id)['SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP'] + fvs = swsscommon.FieldValuePairs([('members', 'group2'), ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + self.asic_db.wait_for_deleted_keys(self.ASIC_NHGM_STR, nhgm_ids) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 4) + nhgm_id = self.get_nhgm_ids('cbfgroup1')[0] + self.asic_db.wait_for_field_match(self.ASIC_NHGM_STR, + nhgm_id, + {'SAI_NEXT_HOP_GROUP_MEMBER_ATTR_INDEX': '0'}) + self.asic_db.wait_for_field_negative_match(self.ASIC_NHG_STR, + nhg_id, + {'SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP': old_map}) + + # Delete the route + self.rt_ps._del('2.2.2.0/24') + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + # The update should have overwritten the delete for CBF NHG + assert(self.nhg_exists('cbfgroup1')) + + # Test scenario: + # - try updating the CBF NHG with a member that doesn't exist and assert the CBF NHG's + # - create the missing NHG and assert the CBF NHG's member also gets created + def update_cbf_nhg_inexistent_member_test(): + # Update the CBF NHG referencing an NHG that doesn't exist. In the end, create the NHG and + # make sure everything works fine. + fvs = swsscommon.FieldValuePairs([('members', 'group2'), ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) + assert(len(self.asic_db.get_keys(self.ASIC_NHGM_STR)) == self.asic_nhgms_count) + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('ifname', 'Ethernet0,Ethernet4')]) + self.nhg_ps.set('group2', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 3) + + # Test scenario: + # - try updating the CBF NHG with a NHG map that doesn't exist and assert it does't change + # - create the missing NHG map and assert the selection map also gets updated + def update_cbf_nhg_inexistent_map_test(): + # Update the CBF NHG with an NHG map that doesn't exist, then update it with one that does exist. + nhg_id = self.get_nhg_id('cbfgroup1') + smap_id = self.asic_db.get_entry(self.ASIC_NHG_STR, nhg_id)['SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP'] + fvs = swsscommon.FieldValuePairs([('members', 'group2'), ('selection_map', 'a')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + assert(self.asic_db.get_entry(self.ASIC_NHG_STR, nhg_id)['SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP'] == smap_id) + fvs = swsscommon.FieldValuePairs([('members', 'group2'), ('selection_map', 'cbfnhgmap2')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + self.asic_db.wait_for_field_negative_match(self.ASIC_NHG_STR, + nhg_id, + {'SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP': smap_id}) + + self.init_test(dvs, 4) + + mainline_cbf_nhg_test() + data_validation_test() + update_cbf_nhg_members_test() + update_cbf_nhg_map_test() + delete_referenced_cbf_nhg_test() + create_route_inexistent_cbf_nhg_test() + update_deleting_cbf_nhg_test() + + # Delete the NHGs + self.cbf_nhg_ps._del('cbfgroup1') + self.nhg_ps._del('group2') + self.nhg_ps._del('group3') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count) + + update_cbf_nhg_inexistent_member_test() + update_cbf_nhg_inexistent_map_test() + + # Coverage testing: Update the CBF NHG with a member that does not exist. + fvs = swsscommon.FieldValuePairs([('members', 'group3'), ('selection_map', 'cbfnhgmap2')]) + self.cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + + # Delete the NHGs + self.cbf_nhg_ps._del('cbfgroup1') + self.nhg_ps._del('group2') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) + + # Delete the NHG maps + self.fc_to_nhg_ps._del('cbfnhgmap1') + self.fc_to_nhg_ps._del('cbfnhgmap2') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count) + + # Coverage testing: Delete inexistent CBF NHG + self.cbf_nhg_ps._del('cbfgroup1') + + def test_nhg_map(self, dvs, testlog): + + # Test scenario: + # - create a NHG map and assert the expected details + # - delete the map + def mainline_nhg_map_test(): + # Create an FC to NH index map + fvs = swsscommon.FieldValuePairs([('0', '0')]) + self.fc_to_nhg_ps.set('cbfnhgmap1', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count + 1) + + # Check the NHG map attributes + nhg_map_id = self.get_nhg_map_id('cbfnhgmap1') + assert nhg_map_id != None + + self.asic_db.wait_for_field_match(self.ASIC_NHG_MAP_STR, + nhg_map_id, + {'SAI_NEXT_HOP_GROUP_MAP_ATTR_TYPE': 'SAI_NEXT_HOP_GROUP_MAP_TYPE_FORWARDING_CLASS_TO_INDEX', + 'SAI_NEXT_HOP_GROUP_MAP_ATTR_MAP_TO_VALUE_LIST': '{\"count\":1,\"list\":[{\"key\":0,\"value\":0}]}'}) + + # Delete the map + self.fc_to_nhg_ps._del('cbfnhgmap1') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count) + + # Test scenario: + # - try creating different NHG maps with invalid data and assert they don't get created + def data_validation_test(): + # Test validation errors + nhg_maps = [ + ('-1', '0'), # negative FC + ('64', '0'), # greater than max FC value + ('a', '0'), # non-integer FC + ('0', '-1'), # negative NH index + ('0', 'a'), # non-integer NH index + ] + + # Check that no such NHG map gets created + for nhg_map in nhg_maps: + fvs = swsscommon.FieldValuePairs([nhg_map]) + self.fc_to_nhg_ps.set('cbfnhgmap1', fvs) + time.sleep(1) + assert(not self.nhg_map_exists('cbfnhgmap1')) + + # Test scenario: + # - create two CBF NHG and a NHG map + # - try deleting the NHG map and assert it fails as it is being referenced + # - delete a CBF NHG and assert the NHG map is still not deleted as it's still being referenced + # - create a new NHG map and update the CBF NHG to point to it and assert the previous NHG map gets deleted + def delete_referenced_nhg_map_test(): + # Create two non-CBF NHGs + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5'), + ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) + self.nhg_ps.set("group1", fvs) + + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('ifname', 'Ethernet0,Ethernet4')]) + self.nhg_ps.set('group2', fvs) + + # Wait for the groups to appear in ASIC DB + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + + # Create an FC to NH index map + fvs = swsscommon.FieldValuePairs([('0', '0')]) + self.fc_to_nhg_ps.set('cbfnhgmap1', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count + 1) + + # Create a couple of CBF NHGs + for i in range(2): + fvs = swsscommon.FieldValuePairs([('members', 'group{}'.format(i + 1)), + ('selection_map', 'cbfnhgmap1')]) + self.cbf_nhg_ps.set('cbfgroup{}'.format(i), fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 4) + # Try deleting the NHG map. It should fail as it is referenced. + self.fc_to_nhg_ps._del('cbfnhgmap1') + time.sleep(1) + assert(self.nhg_map_exists('cbfnhgmap1')) + + # Delete a CBF NHG + self.cbf_nhg_ps._del('cbfgroup1') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 3) + + # The NHG map is still referenced so shouldn't be deleted + assert(self.nhg_map_exists('cbfnhgmap1')) + + # Create a new FC to NH index map + fvs = swsscommon.FieldValuePairs([('0', '0')]) + self.fc_to_nhg_ps.set('cbfnhgmap2', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count + 2) + + # Update the second CBF NHG to point to the new map + fvs = swsscommon.FieldValuePairs([('members', 'group1'), ('selection_map', 'cbfnhgmap2')]) + self.cbf_nhg_ps.set('cbfgroup0', fvs) + + # The NHG map is no longer referenced and should have been deleted + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count + 1) + + # Test scenario: + # - delete the NHG map and assert it fails as it's being referenced + # - update the NHG map and assert for the new details + # - delete the referencing CBF NHG and assert the NHG map isn't deleted as it was updated + # - delete the NHG map + def update_override_delete_test(): + # Delete the second NHG map. It fails as it is referenced. + self.fc_to_nhg_ps._del('cbfnhgmap2') + time.sleep(1) + assert(self.nhg_map_exists('cbfnhgmap2')) + + # Update the NHG map + nhg_map_id = self.get_nhg_map_id('cbfnhgmap2') + fvs = swsscommon.FieldValuePairs([('1', '0'), ('2', '0')]) + self.fc_to_nhg_ps.set('cbfnhgmap2', fvs) + self.asic_db.wait_for_field_match(self.ASIC_NHG_MAP_STR, + nhg_map_id, + {'SAI_NEXT_HOP_GROUP_MAP_ATTR_TYPE': 'SAI_NEXT_HOP_GROUP_MAP_TYPE_FORWARDING_CLASS_TO_INDEX', + 'SAI_NEXT_HOP_GROUP_MAP_ATTR_MAP_TO_VALUE_LIST': '{\"count\":2,\"list\":[{\"key\":2,\"value\":0},{\"key\":1,\"value\":0}]}'}) + + # Delete the second CBF NHG + self.cbf_nhg_ps._del('cbfgroup0') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + + # The NHG map should not be deleted as it was updated + time.sleep(1) + assert(self.nhg_map_exists('cbfnhgmap2')) + + # Delete the NHG map + self.fc_to_nhg_ps._del('cbfnhgmap2') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count) + + # Test scenario: + # - fill the ASIC with NHG maps + # - try creating a new NHG map and assert it fails as the ASIC is full + # - delete a NHG map and assert the to-be-created one is actually created + def nhg_map_exhaust_test(): + # Create the maximum number of NHG maps allowed by the VS switch (512) + fvs = swsscommon.FieldValuePairs([('0', '0')]) + for i in range(512 - self.asic_nhg_maps_count): + nhg_maps.append('cbfnhgmap{}'.format(i)) + self.fc_to_nhg_ps.set('cbfnhgmap{}'.format(i), fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.MAX_NHG_MAP_COUNT) + + # Try creating a new NHG map. It should fail as there is no more space. + nhg_maps.append('cbfnhgmap512') + self.fc_to_nhg_ps.set('cbfnhgmap512', fvs) + time.sleep(1) + assert(not self.nhg_map_exists('cbfnhgmap512')) + + # Delete an existing NHG map. The pending NHG map should now be created + del_nhg_map_index = nhg_maps.pop(0) + del_nhg_map_id = self.get_nhg_map_id(del_nhg_map_index) + self.fc_to_nhg_ps._del(del_nhg_map_index) + self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_MAP_STR, del_nhg_map_id) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.MAX_NHG_MAP_COUNT) + assert(self.nhg_map_exists('cbfnhgmap512')) + + # Test scenario: + # - create a NHG map while the ASIC is full + # - create a CBF NHG pointing to this NHG map and assert it isn't created + # - delete a NHG map and assert the new NHG map and the CBF NHG map is created + # - delete the CBF NHG + def create_cbf_nhg_inexistent_map_test(): + # Create a new NHG map. It should remain pending. + nhg_maps.append('cbfnhgmap513') + fvs = swsscommon.FieldValuePairs([('0', '0')]) + self.fc_to_nhg_ps.set('cbfnhgmap513', fvs) + + # Create a CBF NHG which should reference this NHG map. It should fail creating it. + fvs = swsscommon.FieldValuePairs([('members', 'group1'), ('selection_map', 'cbfnhgmap513')]) + self.cbf_nhg_ps.set('testcbfnhg', fvs) + time.sleep(1) + assert(not self.nhg_exists('testcbfnhg')) + + # Delete an existing NHG map. The new map and the CBF NHG should be created. + self.fc_to_nhg_ps._del(nhg_maps.pop(0)) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 3) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.MAX_NHG_MAP_COUNT) + + self.cbf_nhg_ps._del('testcbfnhg') + + self.init_test(dvs, 4) + nhg_maps = [] + + mainline_nhg_map_test() + data_validation_test() + delete_referenced_nhg_map_test() + update_override_delete_test() + nhg_map_exhaust_test() + create_cbf_nhg_inexistent_map_test() + + # Cleanup + + # Delete the NHGs + for i in range(2): + self.nhg_ps._del('group{}'.format(i + 1)) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) + + # Delete all the NHG maps + while len(nhg_maps) > 0: + self.fc_to_nhg_ps._del(nhg_maps.pop()) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_MAP_STR, self.asic_nhg_maps_count) + + # Add Dummy always-pass test at end as workaroud # for issue when Flaky fail on final test it invokes module tear-down before retrying def test_nonflaky_dummy(): diff --git a/tests/test_qos_map.py b/tests/test_qos_map.py index 51304d8b1f..b51d5cbb49 100644 --- a/tests/test_qos_map.py +++ b/tests/test_qos_map.py @@ -47,7 +47,6 @@ def create_dot1p_profile(self): tbl.set(CFG_DOT1P_TO_TC_MAP_KEY, fvs) time.sleep(1) - def find_dot1p_profile(self): found = False dot1p_tc_map_raw = None @@ -86,7 +85,7 @@ def test_dot1p_cfg(self, dvs): self.create_dot1p_profile() _, dot1p_tc_map_raw = self.find_dot1p_profile() - dot1p_tc_map = json.loads(dot1p_tc_map_raw); + dot1p_tc_map = json.loads(dot1p_tc_map_raw) for dot1p2tc in dot1p_tc_map['list']: dot1p = str(dot1p2tc['key']['dot1p']) tc = str(dot1p2tc['value']['tc']) @@ -115,6 +114,158 @@ def test_port_dot1p(self, dvs): port_cnt = len(swsscommon.Table(self.config_db, CFG_PORT_TABLE_NAME).getKeys()) assert port_cnt == cnt +class TestCbf(object): + ASIC_QOS_MAP_STR = "ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP" + ASIC_PORT_STR = "ASIC_STATE:SAI_OBJECT_TYPE_PORT" + + def init_test(self, dvs): + self.dvs = dvs + self.asic_db = dvs.get_asic_db() + self.config_db = dvs.get_config_db() + self.asic_qos_map_ids = self.asic_db.get_keys(self.ASIC_QOS_MAP_STR) + self.asic_qos_map_count = len(self.asic_qos_map_ids) + self.dscp_ps = swsscommon.Table(self.config_db.db_connection, swsscommon.CFG_DSCP_TO_FC_MAP_TABLE_NAME) + self.exp_ps = swsscommon.Table(self.config_db.db_connection, swsscommon.CFG_EXP_TO_FC_MAP_TABLE_NAME) + + # Set switch FC capability to 63 + dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_MAX_NUMBER_OF_FORWARDING_CLASSES', '63') + + def get_qos_id(self): + diff = set(self.asic_db.get_keys(self.ASIC_QOS_MAP_STR)) - set(self.asic_qos_map_ids) + assert len(diff) <= 1 + return None if len(diff) == 0 else diff.pop() + + def test_dscp_to_fc(self, dvs): + self.init_test(dvs) + + # Create a DSCP_TO_FC map + dscp_map = [(str(i), str(i)) for i in range(0, 64)] + self.dscp_ps.set("AZURE", swsscommon.FieldValuePairs(dscp_map)) + + self.asic_db.wait_for_n_keys(self.ASIC_QOS_MAP_STR, self.asic_qos_map_count + 1) + + # Get the DSCP map ID + dscp_map_id = self.get_qos_id() + assert(dscp_map_id is not None) + + # Assert the expected values + fvs = self.asic_db.get_entry(self.ASIC_QOS_MAP_STR, dscp_map_id) + assert(fvs.get("SAI_QOS_MAP_ATTR_TYPE") == "SAI_QOS_MAP_TYPE_DSCP_TO_FORWARDING_CLASS") + + # Apply port configuration + # self.port_qos_map_profile_test('dscp_to_fc_map', swsscommon.CFG_DSCP_TO_FC_MAP_TABLE_NAME, 'SAI_PORT_ATTR_QOS_DSCP_TO_FORWARDING_CLASS_MAP') + + # Delete the map + self.dscp_ps._del("AZURE") + self.asic_db.wait_for_deleted_entry(self.ASIC_QOS_MAP_STR, dscp_map_id) + + # Test validation + maps = [ + ('-1', '0'), # negative DSCP + ('64', '0'), # DSCP greater than max value + ('0', '-1'), # negative FC + ('0', '64'), # FC greater than max value + ('a', '0'), # non-integer DSCP + ('0', 'a'), # non-integet FC + ] + + for fvs in maps: + dscp_ps.set('AZURE', swsscommon.FieldValuePairs([fvs])) + time.sleep(1) + assert(asic_qos_map_count == len(asic_db.get_keys('ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP'))) + dscp_ps._del("AZURE") + + # Delete a map that does not exist. Nothing should happen + self.dscp_ps._del("AZURE") + time.sleep(1) + assert(len(self.asic_db.get_keys(self.ASIC_QOS_MAP_STR)) == self.asic_qos_map_count) + + def test_exp_to_fc(self, dvs): + self.init_test(dvs) + + # Create a EXP_TO_FC map + exp_map = [(str(i), str(i)) for i in range(0, 8)] + self.exp_ps.set("AZURE", swsscommon.FieldValuePairs(exp_map)) + + self.asic_db.wait_for_n_keys(self.ASIC_QOS_MAP_STR, self.asic_qos_map_count + 1) + + # Get the EXP map ID + exp_map_id = self.get_qos_id() + + # Assert the expected values + fvs = self.asic_db.get_entry(self.ASIC_QOS_MAP_STR, exp_map_id) + assert(fvs.get("SAI_QOS_MAP_ATTR_TYPE") == "SAI_QOS_MAP_TYPE_MPLS_EXP_TO_FORWARDING_CLASS") + + # Delete the map + self.exp_ps._del("AZURE") + self.asic_db.wait_for_deleted_entry(self.ASIC_QOS_MAP_STR, exp_map_id) + + # Test validation + maps = [ + ('-1', '0'), # negative EXP + ('8', '0'), # EXP greater than max value + ('0', '-1'), # negative FC + ('0', '64'), # FC greater than max value + ('a', '0'), # non-integer EXP + ('0', 'a'), # non-integet FC + ] + + for fvs in maps: + exp_ps.set('AZURE', swsscommon.FieldValuePairs([fvs])) + time.sleep(1) + assert(asic_qos_map_count == len(asic_db.get_keys('ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP'))) + exp_ps._del("AZURE") + + # Update the map with valid values + exp_map = [(str(i), str(i + 10)) for i in range(0, 8)] + self.exp_ps.set('AZURE', swsscommon.FieldValuePairs(exp_map)) + self.asic_db.wait_for_n_keys(self.ASIC_QOS_MAP_STR, self.asic_qos_map_count + 1) + + # Delete the map + exp_map_id = self.get_qos_id() + self.exp_ps._del("AZURE") + self.asic_db.wait_for_deleted_entry(self.ASIC_QOS_MAP_STR, exp_map_id) + + # Delete a map that does not exist. Nothing should happen + self.exp_ps._del("AZURE") + time.sleep(1) + assert(len(self.asic_db.get_keys(self.ASIC_QOS_MAP_STR)) == self.asic_qos_map_count) + + def test_per_port_cbf_binding(self, dvs): + self.init_test(dvs) + + # Create a DSCP_TO_FC map + dscp_map = [(str(i), str(i)) for i in range(0, 64)] + self.dscp_ps.set("AZURE", swsscommon.FieldValuePairs(dscp_map)) + self.asic_db.wait_for_n_keys(self.ASIC_QOS_MAP_STR, self.asic_qos_map_count + 1) + dscp_map_id = self.get_qos_id() + self.asic_qos_map_ids = self.asic_db.get_keys(self.ASIC_QOS_MAP_STR) + + # Create a EXP_TO_FC map + exp_map = [(str(i), str(i)) for i in range(0, 8)] + self.exp_ps.set("AZURE", swsscommon.FieldValuePairs(exp_map)) + self.asic_db.wait_for_n_keys(self.ASIC_QOS_MAP_STR, self.asic_qos_map_count + 2) + exp_map_id = self.get_qos_id() + + tbl = swsscommon.Table(self.config_db.db_connection, swsscommon.CFG_PORT_QOS_MAP_TABLE_NAME) + fvs = swsscommon.FieldValuePairs([('dscp_to_fc_map', "AZURE"), + ('exp_to_fc_map', "AZURE")]) + keys = self.config_db.get_keys(swsscommon.CFG_PORT_TABLE_NAME) + for key in keys: + tbl.set(key, fvs) + time.sleep(1) + + dscp_cnt = 0 + exp_cnt = 0 + for key in self.asic_db.get_keys(self.ASIC_PORT_STR): + fvs = self.asic_db.get_entry(self.ASIC_PORT_STR, key) + if 'SAI_PORT_ATTR_QOS_DSCP_TO_FORWARDING_CLASS_MAP' in fvs: + assert fvs['SAI_PORT_ATTR_QOS_DSCP_TO_FORWARDING_CLASS_MAP'] == dscp_map_id + dscp_cnt += 1 + if 'SAI_PORT_ATTR_QOS_MPLS_EXP_TO_FORWARDING_CLASS_MAP' in fvs: + assert fvs['SAI_PORT_ATTR_QOS_MPLS_EXP_TO_FORWARDING_CLASS_MAP'] == exp_map_id + exp_cnt += 1 + assert dscp_cnt == exp_cnt == len(keys) class TestMplsTc(object): def connect_dbs(self, dvs):