diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 4c6583a217b..165cdd49ad9 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -32,7 +32,9 @@ orchagent_SOURCES = \ orchdaemon.cpp \ orch.cpp \ notifications.cpp \ - nhgorch.cpp \ + nexthopgroup.cpp \ + nhghandler.cpp \ + cbfnhghandler.cpp \ routeorch.cpp \ mplsrouteorch.cpp \ neighorch.cpp \ diff --git a/orchagent/cbfnhghandler.cpp b/orchagent/cbfnhghandler.cpp new file mode 100644 index 00000000000..60238572afc --- /dev/null +++ b/orchagent/cbfnhghandler.cpp @@ -0,0 +1,992 @@ +#include "cbfnhghandler.h" +#include "crmorch.h" +#include "bulker.h" +#include "tokenize.h" +#include "nhgorch.h" + +extern sai_object_id_t gSwitchId; + +extern NhgOrch *gNhgOrch; +extern CrmOrch *gCrmOrch; + +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); + + SWSS_LOG_INFO("CBF next hop group key %s, op %s", + index.c_str(), op.c_str()); + + bool success; + const auto &cbf_nhg_it = m_syncedNhgs.find(index); + + if (op == SET_COMMAND) + { + string members; + string class_map; + + /* + * Get CBF group's members and class map. + */ + for (const auto &i : kfvFieldsValues(t)) + { + if (fvField(i) == "members") + { + members = fvValue(i); + } + else if (fvField(i) == "class_map") + { + class_map = fvValue(i); + } + } + + SWSS_LOG_INFO("CBF NHG has members %s, class map %s", + members.c_str(), class_map.c_str()); + + /* + * Validate the data. + */ + auto t = validateData(members, class_map); + + if (!get<0>(t)) + { + 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_syncedNhgs.end()) + { + SWSS_LOG_INFO("Creating the CBF next hop group"); + + /* + * If we reached the NHG limit, postpone the creation. + */ + if (NhgBase::getSyncedCount() >= NhgOrch::getMaxNhgCount()) + { + SWSS_LOG_WARN("Reached next hop group limit. Postponing " + "creation."); + success = false; + } + else + { + auto cbf_nhg = CbfNhg(index, get<1>(t), get<2>(t)); + success = cbf_nhg.sync(); + + if (success) + { + SWSS_LOG_INFO("CBF NHG successfully synced."); + + /* + * If the CBF NHG contains temporary NHGs as members, + * we have to keep checking for updates. + */ + if (cbf_nhg.hasTemps()) + { + SWSS_LOG_INFO("CBF NHG contains temporary NHGs"); + success = false; + } + + m_syncedNhgs.emplace(index, + NhgEntry(move(cbf_nhg))); + } + } + + } + /* + * If the CBF group exists, update it. + */ + else + { + SWSS_LOG_INFO("Updating the CBF next hop group"); + success = cbf_nhg_it->second.nhg.update(get<1>(t), get<2>(t)); + + /* + * 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()) + { + SWSS_LOG_INFO("The CBF NHG references temporary NHGs"); + success = false; + } + } + } + else if (op == DEL_COMMAND) + { + SWSS_LOG_INFO("Deleting CBF next hop group %s", index.c_str()); + + /* + * 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) + { + SWSS_LOG_INFO("There is a pending SET operation - skipping " + "delete operation"); + success = true; + } + /* + * If the group doesn't exist, do nothing. + */ + else if (cbf_nhg_it == m_syncedNhgs.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 + { + SWSS_LOG_INFO("Removing CBF next hop group"); + success = cbf_nhg_it->second.nhg.desync(); + + if (success) + { + SWSS_LOG_INFO("Successfully desynced CBF next hop group"); + m_syncedNhgs.erase(cbf_nhg_it); + } + } + } + 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; + } + } +} + +/* + * Purpose: Validate the CBF data. + * + * Params: IN members - The members string to validate. + * IN class_map - The class map string to validate. + * + * Returns: Tuple where: + * - the first element is a bool, representing if the data is valid or + * not + * - the second element is a vector of members + * - the third element is an unordered map of the class map + */ +tuple, unordered_map> + CbfNhgHandler::validateData(const string &members, const string &class_map) +{ + SWSS_LOG_ENTER(); + + auto members_vec = swss::tokenize(members, ','); + auto class_map_vec = swss::tokenize(class_map, ','); + + /* + * Verify that the members and class map are not empty and are of + * the same size. + */ + if (members_vec.empty() || class_map_vec.empty()) + { + SWSS_LOG_ERROR("CBF next hop group data is empty."); + return make_tuple(false, + vector(), + unordered_map()); + } + + /* + * Verify that the members are unique. + */ + set members_set(members_vec.begin(), members_vec.end()); + + if (members_set.size() != members_vec.size()) + { + SWSS_LOG_ERROR("CBF next hop group members are not unique."); + return make_tuple(false, + vector(), + unordered_map()); + } + + /* + * Verify that the class map contains valid data. The FC should be greater + * than 0, and the index should be between 0 and the size of members + * (exclusive). Also, the FC values should be unique (the same FC can't be + * mapped more than once). + */ + unordered_map class_map_map; + + for (const auto &i : class_map_vec) + { + /* + * Check that the mapping is correctly formed. + */ + auto tokens = swss::tokenize(i, ':'); + + if (tokens.size() != 2) + { + SWSS_LOG_ERROR("CBF next hop group class map is ill-formed"); + return make_tuple(false, + vector(), + unordered_map()); + } + + try + { + /* + * Check that the FC value is valid. + */ + auto fc = stoi(tokens[0]); + + if (fc < 0) + { + SWSS_LOG_ERROR("CBF next hop group class map contains negative" + " FC %d", fc); + return make_tuple(false, + vector(), + unordered_map()); + } + + /* + * Check that the index value is valid. + */ + auto index = stoi(tokens[1]); + + if ((index < 0) || (index >= (int)members_vec.size())) + { + SWSS_LOG_ERROR("CBF next hop group class map contains invalid " + "index %d", index); + return make_tuple(false, + vector(), + unordered_map()); + } + + /* + * Check that the mapping is unique. + */ + auto rc = class_map_map.emplace(static_cast(fc), + static_cast(index)).second; + if (!rc) + { + SWSS_LOG_ERROR("CBF next hop group class map maps FC %d more " + "than once", fc); + return make_tuple(false, + vector(), + unordered_map()); + } + } + catch(const exception& e) + { + SWSS_LOG_ERROR("Failed to convert CBF next hop group FC or index " + "to uint8_t."); + return make_tuple(false, + vector(), + unordered_map()); + } + } + + return make_tuple(true, members_vec, class_map_map); +} + +/* + * Purpose: Get the non CBF NHG with the given index. + * + * Params: IN index - The index of the non CBF NHG. + * + * Returns: Reference to the non CBF NHG. + */ +const Nhg& CbfNhgHandler::getNonCbfNhg(const string &index) +{ + SWSS_LOG_ENTER(); + return gNhgOrch->nhgHandler.getNhg(index); +} + +/* + * Purpose: Constructor. + * + * Params: IN index - The index of the CBF NHG. + * IN members - The members of the CBF NHG. + * IN class_map - The class map of the CBF NHG. + * + * Returns: Nothing. + */ +CbfNhg::CbfNhg(const string &index, + const vector &members, + const unordered_map &class_map) : + NhgCommon(index), + m_class_map(class_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_class_map(move(cbf_nhg.m_class_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()) + { + SWSS_LOG_INFO("Group %s is already synced", m_key.c_str()); + 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.s32 = SAI_NEXT_HOP_GROUP_TYPE_CLASS_BASED; + nhg_attrs.push_back(nhg_attr); + + /* + * Add the class map to the attributes. + */ + nhg_attrs.push_back(getClassMapAttr()); + + auto status = sai_next_hop_group_api->create_next_hop_group( + &m_id, + gSwitchId, + static_cast(nhg_attrs.size()), + nhg_attrs.data()); + + /* + * Free the class map attribute resources. + */ + delete[] nhg_attrs[1].value.maplist.list; + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create CBF next hop group %s, rv %d", + m_key.c_str(), + status); + return false; + } + + /* + * 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: 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()) + { + SWSS_LOG_INFO("The size of the current members are different than the " + "size of the given members."); + return false; + } + + /* + * Check that the members are the same and the index is preserved. + */ + uint8_t index = 0; + + for (const auto &member : members) + { + SWSS_LOG_DEBUG("Checking member %s", member.c_str()); + + auto mbr_it = m_members.find(member); + + if (mbr_it == m_members.end()) + { + SWSS_LOG_INFO("Member %s does not exist in the current members", + member.c_str()); + return false; + } + + if (mbr_it->second.getIndex() != index++) + { + SWSS_LOG_INFO("Member %s has a different index value than the " + "existing member", member.c_str()); + return false; + } + } + + return true; +} + +/* + * Purpose: Update a CBF next hop group. + * + * Params: IN members - The new members. + * IN class_map - The new class map. + * + * Returns: true, if the update was successful, + * false, otherwise. + */ +bool CbfNhg::update(const vector &members, + const unordered_map &class_map) +{ + SWSS_LOG_ENTER(); + + SWSS_LOG_INFO("Updating CBF next hop group %s", m_key.c_str()); + + /* + * 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)) + { + SWSS_LOG_INFO("Check if any temporary NHGs has been promoted"); + + /* + * Iterate over the temporary NHGs and check if they were updated. + */ + for (auto member = m_temp_nhgs.begin(); member != m_temp_nhgs.end();) + { + SWSS_LOG_DEBUG("Checking temporary CBF NHG member %s", + member->first.c_str()); + + 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) + { + SWSS_LOG_INFO("CBF NHG member %s changed it's SAI ID from %lu" + " to %lu", + member->first.c_str(), member->second, nhg.getId()); + + 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()) + { + SWSS_LOG_INFO("CBF NHG member %s was promoted", + member->first.c_str()); + member = m_temp_nhgs.erase(member); + } + /* + * If the NHG was just updated, update its current NHG ID value + * in the map. + */ + else + { + SWSS_LOG_INFO("CBF NHG member %s is still temporary", + member->first.c_str()); + member->second = nhg.getId(); + ++member; + } + } + else + { + SWSS_LOG_DEBUG("CBF temporary NHG member %s hasn't been " + "updated", member->first.c_str()); + ++member; + } + } + } + /* + * If the members are different, update the whole members list. + */ + else + { + SWSS_LOG_INFO("Update CBF NHG's members"); + + /* + * Because the INDEX attribute is CREATE_ONLY, we have to desync 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 (!desyncMembers(members_set)) + { + SWSS_LOG_ERROR("Failed to desync 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) + { + SWSS_LOG_INFO("Adding CBF next hop group member %s", + member.c_str()); + 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_class_map != class_map) + { + SWSS_LOG_INFO("Updating CBF NHG's class map"); + + m_class_map = class_map; + + sai_attribute_t nhg_attr = getClassMapAttr(); + + auto status = sai_next_hop_group_api->set_next_hop_group_attribute( + m_id, &nhg_attr); + + /* + * Free the class map attribute resources. + */ + delete[] nhg_attr.value.maplist.list; + + 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; + } + } + + 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(); + + SWSS_LOG_INFO("Syncing CBF next hop group %s members", m_key.c_str()); + + /* + * 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) + { + SWSS_LOG_INFO("Checking next hop group member %s", key.c_str()); + + auto &nhgm = m_members.at(key); + + /* + * If the member is already synced, it means an update occurred for + * which we need to update the member's index. + */ + if (nhgm.isSynced()) + { + SWSS_LOG_INFO("CBF NHG member is already synced"); + continue; + } + + /* + * 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 = CbfNhgHandler::getNonCbfNhg(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) + { + SWSS_LOG_DEBUG("CBF next hop group member %s has SAI ID %lu", + member.first.c_str(), member.second); + + 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 + { + SWSS_LOG_DEBUG("Successfully synced CBF next hop group member %s", + member.first.c_str()); + 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()) + { + SWSS_LOG_INFO("CBF NHG member is temporary"); + m_temp_nhgs.emplace(member.first, nhg.getId()); + } + } + } + + SWSS_LOG_DEBUG("Returning %d", success); + + 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", m_key.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: Build the SAI attribute for the class map. The caller is + * responsible for freeing the resources when finished using the + * attribute. + * + * Params: None. + * + * Returns: The SAI attribute. + */ +sai_attribute_t CbfNhg::getClassMapAttr() const +{ + SWSS_LOG_ENTER(); + + sai_attribute_t nhg_attr; + nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_FORWARDING_CLASS_TO_INDEX_MAP; + nhg_attr.value.maplist.count = static_cast(m_class_map.size()); + nhg_attr.value.maplist.list = + new sai_map_t[nhg_attr.value.maplist.count](); + uint32_t idx = 0; + for (const auto &it : m_class_map) + { + nhg_attr.value.maplist.list[idx].key = it.first; + nhg_attr.value.maplist.list[idx++].value = it.second; + } + + return nhg_attr; +} + +/* + * 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(); + + SWSS_LOG_INFO("Updating next hop attribute of CBF NHG member %s", + to_string().c_str()); + + 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: Desync the member, reseting its SAI ID and decrementing the NHG ref + * counter. + * + * Params: None. + * + * Returns: Nothing. + */ +void CbfNhgMember::desync() +{ + SWSS_LOG_ENTER(); + + NhgMember::desync(); + 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->nhgHandler.hasNhg(m_key)) + { + SWSS_LOG_INFO("NHG %s does not exist", to_string().c_str()); + return SAI_NULL_OBJECT_ID; + } + + return CbfNhgHandler::getNonCbfNhg(m_key).getId(); +} diff --git a/orchagent/cbfnhghandler.h b/orchagent/cbfnhghandler.h new file mode 100644 index 00000000000..cbdcce9262b --- /dev/null +++ b/orchagent/cbfnhghandler.h @@ -0,0 +1,143 @@ +#pragma once + +#include "nexthopgroup.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; + + /* + * Desync the member, reseting its SAI ID and decrementing the necessary + * ref counters. + */ + void desync() 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 unordered_map &class_map); + CbfNhg(CbfNhg &&cbf_nhg); + + /* + * Destructor. + */ + ~CbfNhg() { SWSS_LOG_ENTER(); desync(); } + + /* + * Create the CBF group over SAI. + */ + bool sync() 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 unordered_map &class_map); + + string to_string() const override { return m_key; } + +private: + unordered_map m_class_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; + + /* + * Build the SAI attribute for the class map. + */ + sai_attribute_t getClassMapAttr() const; + + /* + * 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); + + /* + * Get the non CBF NHG with the given index. + */ + static inline const Nhg& getNonCbfNhg(const string &index); + +private: + static tuple, unordered_map> + validateData(const string &members, const string &class_map); +}; diff --git a/orchagent/mplsrouteorch.cpp b/orchagent/mplsrouteorch.cpp index c22767fe6e9..3d0ea8174cf 100644 --- a/orchagent/mplsrouteorch.cpp +++ b/orchagent/mplsrouteorch.cpp @@ -248,8 +248,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.is_temp = nh_group.isTemp(); } catch (const std::out_of_range& e) @@ -304,7 +304,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 (gNhgOrch->getSyncedNhgCount() >= gNhgOrch->getMaxNhgCount() && gRouteBulker.removing_entries_count() > 0) { break; @@ -553,7 +553,7 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey ctx.nhg_index.c_str()); 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 4b0a020e834..0bffb1f17fb 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -324,7 +324,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); @@ -354,7 +354,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/nexthopgroup.cpp b/orchagent/nexthopgroup.cpp new file mode 100644 index 00000000000..00735eccdb4 --- /dev/null +++ b/orchagent/nexthopgroup.cpp @@ -0,0 +1,47 @@ +#include "nexthopgroup.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/nexthopgroup.h b/orchagent/nexthopgroup.h new file mode 100644 index 00000000000..a6decef8399 --- /dev/null +++ b/orchagent/nexthopgroup.h @@ -0,0 +1,495 @@ +#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; + +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(); + + SWSS_LOG_INFO("Syncing next hop group member %s", to_string().c_str()); + + /* + * 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); + } + + /* + * Desync the group member, resetting its SAI ID. + */ + virtual void desync() + { + 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; +}; + +/* + * 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 desynced. This will also account for the + * groups created by RouteOrch. + */ + static unsigned m_syncedCount; +}; + +/* + * 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; + + /* + * Desync the group, releasing the SAI ID. + */ + virtual bool desync() + { + SWSS_LOG_ENTER(); + + /* + * If the group is already desynced, there is nothing to be done. + */ + if (!isSynced()) + { + SWSS_LOG_INFO("Next hop group is already desynced"); + return true; + } + + /* + * Desync the group members. + */ + set members; + + for (const auto &member : m_members) + { + members.insert(member.first); + } + + if (!desyncMembers(members)) + { + SWSS_LOG_ERROR("Failed to desync 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; + + /* + * Desync the given members from the group. + */ + virtual bool desyncMembers(const set &member_keys) + { + SWSS_LOG_ENTER(); + + SWSS_LOG_INFO("Desyncing members of next hop group %s", + to_string().c_str()); + + /* + * Desync 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); + + SWSS_LOG_INFO("Desyncing next hop group member %s", + nhgm.to_string().c_str()); + + if (nhgm.isSynced()) + { + SWSS_LOG_DEBUG("Next hop group member is synced"); + bulker.remove_entry(&statuses[key], nhgm.getId()); + } + } + + /* + * Flush the bulker to desync the members. + */ + bulker.flush(); + + /* + * Iterate over the returned statuses and check if the removal was + * successful. If it was, desync the member, otherwise log an error + * message. + */ + bool success = true; + + for (const auto &status : statuses) + { + auto &member = m_members.at(status.first); + + SWSS_LOG_DEBUG("Verifying next hop group member %s status", + member.to_string().c_str()); + + if (status.second == SAI_STATUS_SUCCESS) + { + SWSS_LOG_DEBUG("Next hop group member was successfully " + "desynced"); + member.desync(); + } + else + { + SWSS_LOG_ERROR("Failed to desync next hop group member %s, rv: " + "%d", + member.to_string().c_str(), + status.second); + success = false; + } + } + + SWSS_LOG_DEBUG("Returning %d", success); + + 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 NhgOrch 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_syncedNhgs.find(index) != m_syncedNhgs.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_syncedNhgs.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_syncedNhgs.at(index); + + SWSS_LOG_INFO("Increment group %s ref count from %u to %u", + index.c_str(), + nhg_entry.ref_count, + nhg_entry.ref_count + 1); + + ++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_syncedNhgs.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"); + } + + SWSS_LOG_INFO("Decrement group %s ref count from %u to %u", + index.c_str(), + nhg_entry.ref_count, + nhg_entry.ref_count - 1); + + --nhg_entry.ref_count; + } + +protected: + /* + * Map of synced next hop groups. + */ + unordered_map> m_syncedNhgs; +}; diff --git a/orchagent/nhgorch.cpp b/orchagent/nhghandler.cpp similarity index 68% rename from orchagent/nhgorch.cpp rename to orchagent/nhghandler.cpp index bf83d1e3936..6741a51a689 100644 --- a/orchagent/nhgorch.cpp +++ b/orchagent/nhghandler.cpp @@ -1,82 +1,19 @@ +#include "nhghandler.h" #include "nhgorch.h" -#include "switchorch.h" #include "neighorch.h" #include "crmorch.h" -#include "routeorch.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 SwitchOrch *gSwitchOrch; -extern RouteOrch *gRouteOrch; 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; - -/* Default maximum number of next hop groups */ -#define DEFAULT_NUMBER_OF_ECMP_GROUPS 128 -#define DEFAULT_MAX_ECMP_GROUP_SIZE 32 - -NhgOrch::NhgOrch(DBConnector *db, string tableName) : - Orch(db, tableName) -{ - SWSS_LOG_ENTER(); - - /* Get the switch's maximum next hop group capacity. */ - sai_attribute_t attr; - attr.id = SAI_SWITCH_ATTR_NUMBER_OF_ECMP_GROUPS; - - sai_status_t status = sai_switch_api->get_switch_attribute(gSwitchId, - 1, - &attr); - - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_WARN("Failed to get switch attribute number of ECMP groups. \ - Use default value. rv:%d", status); - m_maxNhgCount = DEFAULT_NUMBER_OF_ECMP_GROUPS; - } - else - { - m_maxNhgCount = attr.value.s32; - - /* - * ASIC specific workaround to re-calculate maximum ECMP groups - * according to diferent ECMP mode used. - * - * On Mellanox platform, the maximum ECMP groups returned is the value - * under the condition that the ECMP group size is 1. Deviding this - * number by DEFAULT_MAX_ECMP_GROUP_SIZE gets the maximum number of - * ECMP groups when the maximum ECMP group size is 32. - */ - char *platform = getenv("platform"); - if (platform && strstr(platform, MLNX_PLATFORM_SUBSTRING)) - { - SWSS_LOG_NOTICE("Mellanox platform - divide capacity by %d", - DEFAULT_MAX_ECMP_GROUP_SIZE); - m_maxNhgCount /= DEFAULT_MAX_ECMP_GROUP_SIZE; - } - } - - /* Set switch's next hop group capacity. */ - vector fvTuple; - fvTuple.emplace_back("MAX_NEXTHOP_GROUP_COUNT", - to_string(m_maxNhgCount)); - gSwitchOrch->set_switch_capability(fvTuple); - - SWSS_LOG_NOTICE("Maximum number of ECMP groups supported is %d", - m_maxNhgCount); -} /* * Purpose: Perform the operations requested by APPL_DB users. @@ -90,15 +27,10 @@ NhgOrch::NhgOrch(DBConnector *db, string tableName) : * * 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()) @@ -112,7 +44,7 @@ void NhgOrch::doTask(Consumer& consumer) index.c_str(), op.c_str()); bool success; - const auto& nhg_it = m_syncdNextHopGroups.find(index); + const auto& nhg_it = m_syncedNhgs.find(index); if (op == SET_COMMAND) { @@ -158,9 +90,9 @@ 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_syncedNhgs.end()) { - SWSS_LOG_NOTICE("Adding next hop group %s with %s", + SWSS_LOG_INFO("Adding next hop group %s with %s", index.c_str(), nhg_str.c_str()); /* @@ -172,21 +104,20 @@ void NhgOrch::doTask(Consumer& consumer) * actual group when there are enough resources. */ if ((nhg_key.getSize() > 1) && - (NextHopGroup::getCount() >= m_maxNhgCount)) + (Nhg::getSyncedCount() >= NhgOrch::getMaxNhgCount())) { SWSS_LOG_WARN("Next hop group count reached its limit."); try { - auto nhg = std::make_unique( - createTempNhg(nhg_key)); - SWSS_LOG_NOTICE("Adding temp next hop group with %s", - nhg->to_string().c_str()); - if (nhg->sync()) + auto nhg = createTempNhg(nhg_key); + SWSS_LOG_INFO("Adding temp next hop group with %s", + nhg.to_string().c_str()); + if (nhg.sync()) { SWSS_LOG_INFO("Temporary NHG successfully synced"); - m_syncdNextHopGroups.emplace(index, - NhgEntry(std::move(nhg))); + m_syncedNhgs.emplace(index, + NhgEntry(std::move(nhg))); } else { @@ -212,26 +143,25 @@ void NhgOrch::doTask(Consumer& consumer) } else { - auto nhg = std::make_unique(nhg_key); - success = nhg->sync(); + auto nhg = Nhg(nhg_key); + success = nhg.sync(); if (success) { SWSS_LOG_INFO("NHG successfully synced"); - m_syncdNextHopGroups.emplace(index, - NhgEntry(std::move(nhg))); + m_syncedNhgs.emplace(index, + NhgEntry(std::move(nhg))); } } } /* If the group exists, update it. */ else { - SWSS_LOG_NOTICE("Update next hop group %s with %s", + SWSS_LOG_INFO("Update next hop group %s with %s", index.c_str(), nhg_str.c_str()); - const auto& nhg_ptr = nhg_it->second.nhg; - + auto& nhg = nhg_it->second.nhg; /* * A NHG update should never change the SAI ID of the NHG if it @@ -240,8 +170,8 @@ void NhgOrch::doTask(Consumer& consumer) * rule is for the temporary NHGs, as the referencing objects * will keep querying the NhgOrch for any SAI ID updates. */ - if (!nhg_ptr->isTemp() && - ((nhg_key.getSize() == 1) || (nhg_ptr->getSize() == 1)) && + if (!nhg.isTemp() && + ((nhg_key.getSize() == 1) || (nhg.getSize() == 1)) && (nhg_it->second.ref_count > 0)) { SWSS_LOG_WARN("Next hop group %s update would change SAI " @@ -255,9 +185,9 @@ void NhgOrch::doTask(Consumer& consumer) * resources yet, we have to skip it until we have enough * resources. */ - else if (nhg_ptr->isTemp() && + else if (nhg.isTemp() && (nhg_key.getSize() > 1) && - (NextHopGroup::getCount() >= m_maxNhgCount)) + (Nhg::getSyncedCount() >= NhgOrch::getMaxNhgCount())) { /* * If the group was updated in such way that the previously @@ -265,7 +195,7 @@ void NhgOrch::doTask(Consumer& consumer) * update the temporary group to choose a new next hop from * the new key. */ - if (!nhg_key.contains(nhg_ptr->getKey())) + if (!nhg_key.contains(nhg.getKey())) { SWSS_LOG_NOTICE("Updating temporary group %s to %s", index.c_str(), @@ -274,8 +204,7 @@ void NhgOrch::doTask(Consumer& consumer) 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 @@ -283,7 +212,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()) { SWSS_LOG_INFO( "Temporary NHG successfully synced"); @@ -316,16 +245,30 @@ 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) { - SWSS_LOG_NOTICE("Deleting next hop group %s", index.c_str()); + SWSS_LOG_INFO("Deleting next hop group %s", index.c_str()); + /* + * 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) + { + SWSS_LOG_INFO("There is a pending SET operation - skipping " + "delete operation"); + success = true; + } /* If the group does not exist, do nothing. */ - if (nhg_it == m_syncdNextHopGroups.end()) + else if (nhg_it == m_syncedNhgs.end()) { SWSS_LOG_WARN("Unable to find group with key %s to remove", index.c_str()); @@ -343,19 +286,19 @@ void NhgOrch::doTask(Consumer& consumer) /* Else, if the group is no more referenced, delete it. */ else { - const auto& nhg = nhg_it->second.nhg; + auto& nhg = nhg_it->second.nhg; - success = nhg->desync(); + success = nhg.desync(); if (success) { - m_syncdNextHopGroups.erase(nhg_it); + m_syncedNhgs.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; } @@ -384,7 +327,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(); SWSS_LOG_INFO("Validating next hop %s", nh_key.to_string().c_str()); @@ -393,14 +336,14 @@ 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_syncedNhgs) { auto& nhg = it.second.nhg; SWSS_LOG_INFO("Check if next hop in group %s", it.first.c_str()); - if (nhg->hasNextHop(nh_key)) + if (nhg.hasMember(nh_key)) { SWSS_LOG_INFO("Group has next hop"); @@ -408,7 +351,7 @@ bool NhgOrch::validateNextHop(const NextHopKey& 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(), @@ -433,7 +376,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(); SWSS_LOG_INFO("Invalidating next hop %s", nh_key.to_string().c_str()); @@ -442,19 +385,19 @@ 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_syncedNhgs) { auto& nhg = it.second.nhg; SWSS_LOG_INFO("Check if next hop is in group %s", it.first.c_str()); - if (nhg->hasNextHop(nh_key)) + if (nhg.hasMember(nh_key)) { SWSS_LOG_INFO("Group has next hop"); /* If the desync 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(), @@ -467,55 +410,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); - - SWSS_LOG_INFO("Increment group %s ref count from %u to %u", - index.c_str(), - nhg_entry.ref_count, - nhg_entry.ref_count + 1); - - ++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); - - SWSS_LOG_INFO("Decrement group %s ref count from %u to %u", - index.c_str(), - nhg_entry.ref_count, - nhg_entry.ref_count - 1); - - --nhg_entry.ref_count; -} - /* * Purpose: Get the next hop ID of the member. * @@ -526,17 +420,17 @@ 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)) { SWSS_LOG_INFO("NeighOrch has next hop %s", - m_nh_key.to_string().c_str()); - nh_id = gNeighOrch->getNextHopId(m_nh_key); + m_key.to_string().c_str()); + nh_id = gNeighOrch->getNextHopId(m_key); } /* * If the next hop is labeled and the IP next hop exists, create the @@ -545,12 +439,12 @@ 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->hasNextHop(m_nh_key.ipKey())) + else if (isLabeled() && gNeighOrch->hasNextHop(m_key.ipKey())) { SWSS_LOG_INFO("Create labeled next hop %s", - m_nh_key.to_string().c_str()); - gNeighOrch->addNextHop(m_nh_key); - nh_id = gNeighOrch->getNextHopId(m_nh_key); + m_key.to_string().c_str()); + gNeighOrch->addNextHop(m_key); + nh_id = gNeighOrch->getNextHopId(m_key); } return nh_id; @@ -565,12 +459,11 @@ sai_object_id_t NextHopGroupMember::getNhId() const * * Returns: Reference to this object. */ -NextHopGroupMember& NextHopGroupMember::operator=(NextHopGroupMember&& nhgm) +WeightedNhgMember& WeightedNhgMember::operator=(WeightedNhgMember&& nhgm) { SWSS_LOG_ENTER(); - std::swap(m_nh_key, nhgm.m_nh_key); - std::swap(m_gm_id, nhgm.m_gm_id); + NhgMember::operator=(std::move(nhgm)); return *this; } @@ -586,17 +479,17 @@ 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(); SWSS_LOG_INFO("Update group member %s weight from %u to %u", - m_nh_key.to_string().c_str(), - m_nh_key.weight, + m_key.to_string().c_str(), + m_key.weight, weight); bool success = true; - m_nh_key.weight = weight; + m_key.weight = weight; if (isSynced()) { @@ -604,10 +497,10 @@ bool NextHopGroupMember::updateWeight(uint32_t weight) 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); + set_next_hop_group_member_attribute(m_id, &nhgm_attr); success = status == SAI_STATUS_SUCCESS; } @@ -625,16 +518,12 @@ bool NextHopGroupMember::updateWeight(uint32_t weight) * * 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); } /* @@ -647,24 +536,12 @@ void NextHopGroupMember::sync(sai_object_id_t gm_id) * * Returns: Nothing. */ -void NextHopGroupMember::desync() +void WeightedNhgMember::desync() { SWSS_LOG_ENTER(); - /* - * If the member is already desynced, exit so we don't decrement the ref - * counters more than once. - */ - if (!isSynced()) - { - SWSS_LOG_INFO("Next hop group member %s is already desynced", - m_nh_key.to_string().c_str()); - return; - } - - m_gm_id = SAI_NULL_OBJECT_ID; - gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); - gNeighOrch->decreaseNextHopRefCount(m_nh_key); + NhgMember::desync(); + gNeighOrch->decreaseNextHopRefCount(m_key); } /* @@ -677,16 +554,10 @@ void NextHopGroupMember::desync() * * Returns: Nothing. */ -NextHopGroupMember::~NextHopGroupMember() +WeightedNhgMember::~WeightedNhgMember() { SWSS_LOG_ENTER(); - /* - * The group member should be desynced from it's group before destroying - * it. - */ - assert(!isSynced()); - /* * If the labeled next hop is unreferenced, delete it from NeighOrch as * NhgOrch and RouteOrch are the ones controlling it's lifetime. They both @@ -695,12 +566,11 @@ 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)) { - SWSS_LOG_INFO("Delete labeled next hop %s", - m_nh_key.to_string().c_str()); - gNeighOrch->removeMplsNextHop(m_nh_key); + SWSS_LOG_INFO("Delete labeled next hop %s", m_key.to_string().c_str()); + gNeighOrch->removeMplsNextHop(m_key); } } @@ -713,44 +583,17 @@ NextHopGroupMember::~NextHopGroupMember() * * Returns: Nothing. */ -NextHopGroup::NextHopGroup(const NextHopGroupKey& key) : - m_key(key), - m_id(SAI_NULL_OBJECT_ID), - m_is_temp(false) +Nhg::Nhg(const NextHopGroupKey& key) : NhgCommon(key), m_is_temp(false) { SWSS_LOG_ENTER(); - SWSS_LOG_INFO("Creating next hop group %s", m_key.to_string().c_str()); /* Parse the key and create the members. */ - for (const auto& it : m_key.getNextHops()) + for (const auto &it : m_key.getNextHops()) { - SWSS_LOG_INFO("Adding next hop %s to the group", - it.to_string().c_str()); - 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. * @@ -760,15 +603,14 @@ NextHopGroup::NextHopGroup(NextHopGroup&& nhg) : * * 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; } @@ -785,7 +627,7 @@ NextHopGroup& NextHopGroup::operator=(NextHopGroup&& nhg) * Returns: true, if the operation was successful; * false, otherwise. */ -bool NextHopGroup::sync() +bool Nhg::sync() { SWSS_LOG_ENTER(); @@ -802,12 +644,12 @@ bool NextHopGroup::sync() */ if (m_members.size() == 1) { - const NextHopGroupMember& nhgm = m_members.begin()->second; + const WeightedNhgMember& nhgm = m_members.begin()->second; if (nhgm.getNhId() == SAI_NULL_OBJECT_ID) { SWSS_LOG_WARN("Next hop %s is not synced", - nhgm.getNhKey().to_string().c_str()); + nhgm.getKey().to_string().c_str()); return false; } else @@ -847,7 +689,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. @@ -876,7 +718,7 @@ bool NextHopGroup::sync() * * Returns: The created temporary next hop group. */ -NextHopGroup NhgOrch::createTempNhg(const NextHopGroupKey& nhg_key) +Nhg NhgHandler::createTempNhg(const NextHopGroupKey& nhg_key) { SWSS_LOG_ENTER(); SWSS_LOG_INFO("Syncing temporary group %s", nhg_key.to_string().c_str()); @@ -893,7 +735,7 @@ NextHopGroup NhgOrch::createTempNhg(const NextHopGroupKey& nhg_key) */ if (gNeighOrch->hasNextHop(nh_key.ipKey())) { - SWSS_LOG_INFO("Next hop %s is a candidate for temporary group %s", + SWSS_LOG_DEBUG("Next hop %s is a candidate for temporary group %s", nh_key.to_string().c_str(), nhg_key.to_string().c_str()); valid_nhs.push_back(nh_key); @@ -913,7 +755,9 @@ NextHopGroup NhgOrch::createTempNhg(const NextHopGroupKey& nhg_key) advance(it, rand() % valid_nhs.size()); /* Create the temporary group. */ - NextHopGroup nhg(NextHopGroupKey(it->to_string())); + SWSS_LOG_INFO("Using next hop %s for the temporary NHG", + it->to_string().c_str()); + Nhg nhg(NextHopGroupKey(it->to_string())); nhg.setTemp(true); return nhg; @@ -930,54 +774,22 @@ NextHopGroup NhgOrch::createTempNhg(const NextHopGroupKey& nhg_key) * Returns: true, if the operation was successful; * false, otherwise */ -bool NextHopGroup::desync() +bool Nhg::desync() { SWSS_LOG_ENTER(); - SWSS_LOG_INFO("Desyncing group %s", to_string().c_str()); - - /* If the group is already desynced, there is nothing to be done. */ - if (!isSynced()) - { - SWSS_LOG_INFO("Group %s is already desynced", - m_key.to_string().c_str()); - return true; - } + SWSS_LOG_INFO("Desyncing non CBF group %s", to_string().c_str()); /* - * If the group has more than one members, desync it's members, then the - * group. + * If the group has just one member, simply reset it's SAI ID. */ - if (m_members.size() > 1) + if (m_members.size() == 1) { - /* Desync group's members. If we failed to desync the members, exit. */ - if (!desyncMembers(m_key.getNextHops())) - { - SWSS_LOG_ERROR("Failed to desync group %s members", - to_string().c_str()); - return false; - } - - /* Desync 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); - return false; - } - - /* If the operation is successful, release the resources. */ - gCrmOrch->decCrmResUsedCounter( - CrmResourceType::CRM_NEXTHOP_GROUP); - --m_count; + SWSS_LOG_INFO("Group has just one member"); + m_id = SAI_NULL_OBJECT_ID; + return true; } - /* Reset the group ID. */ - m_id = SAI_NULL_OBJECT_ID; - - return true; + return NhgCommon::desync(); } /* @@ -993,7 +805,7 @@ bool NextHopGroup::desync() * 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(); SWSS_LOG_INFO("Adding next hop group %s members", @@ -1019,10 +831,10 @@ bool NextHopGroup::syncMembers(const std::set& nh_keys) SWSS_LOG_INFO("Checking if next hop %s is valid", nh_key.to_string().c_str()); - 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()) { SWSS_LOG_INFO("Update member"); continue; @@ -1085,85 +897,6 @@ bool NextHopGroup::syncMembers(const std::set& nh_keys) return success; } -/* - * Purpose: Desync the given group's members over the SAI API. - * - * Description: Go through the given members and desync them. - * - * Params: IN nh_keys - The next hop keys of the members to desync. - * - * Returns: true, if the operation was successful; - * false, otherwise - */ -bool NextHopGroup::desyncMembers(const std::set& nh_keys) -{ - SWSS_LOG_ENTER(); - SWSS_LOG_INFO("Removing members of group %s", - to_string().c_str()); - - /* This method should not be called for groups with only one NH. */ - assert(m_members.size() > 1); - - ObjectBulker nextHopGroupMemberBulker( - sai_next_hop_group_api, gSwitchId, gMaxBulkSize); - - /* - * Iterate through the given group members add them to be desynced. - * - * Keep track of the SAI delete statuses in case one of them returns an - * error. We assume that deletion 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) - { - SWSS_LOG_INFO("Desyncing member %s", nh_key.to_string().c_str()); - - const NextHopGroupMember& nhgm = m_members.at(nh_key); - - if (nhgm.isSynced()) - { - SWSS_LOG_INFO("Removing next hop group member %s", - nh_key.to_string().c_str()); - 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) - { - SWSS_LOG_INFO("Check member's %s status", - status.first.to_string().c_str()); - - if (status.second == SAI_STATUS_SUCCESS) - { - SWSS_LOG_INFO("Member was successfully desynced"); - m_members.at(status.first).desync(); - } - else - { - SWSS_LOG_ERROR("Could not remove next hop group member %s, rv: %d", - status.first.to_string().c_str(), status.second); - success = false; - } - } - - SWSS_LOG_INFO("Returning %d", success); - - return success; -} /* * Purpose: Update the next hop group based on a new next hop group key. @@ -1179,7 +912,7 @@ bool NextHopGroup::desyncMembers(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(); SWSS_LOG_INFO("Update group %s with %s", @@ -1202,7 +935,7 @@ bool NextHopGroup::update(const NextHopGroupKey& nhg_key) SWSS_LOG_INFO("Updating group without preserving it's SAI ID"); bool was_synced = isSynced(); - *this = NextHopGroup(nhg_key); + *this = Nhg(nhg_key); /* Sync the group only if it was synced before. */ return (was_synced ? sync() : true); @@ -1271,7 +1004,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)); } /* @@ -1299,8 +1032,8 @@ bool NextHopGroup::update(const NextHopGroupKey& nhg_key) * * 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(); @@ -1335,7 +1068,7 @@ vector NextHopGroup::createNhgmAttrs( * 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(); SWSS_LOG_INFO("Validate NH %s in group %s", @@ -1365,7 +1098,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(); SWSS_LOG_INFO("Invalidate NH %s in group %s", diff --git a/orchagent/nhghandler.h b/orchagent/nhghandler.h new file mode 100644 index 00000000000..dc485ca0bff --- /dev/null +++ b/orchagent/nhghandler.h @@ -0,0 +1,118 @@ +#pragma once + +#include "nexthopgroup.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)) {} + + WeightedNhgMember& operator=(WeightedNhgMember&& nhgm); + + /* Destructor. */ + ~WeightedNhgMember(); + + /* Update member's weight and update the SAI attribute as well. */ + bool updateWeight(uint32_t weight); + + /* Sync / Desync. */ + void sync(sai_object_id_t gm_id) override; + void desync() 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); + + Nhg(Nhg&& nhg) : + NhgCommon(move(nhg)), m_is_temp(nhg.m_is_temp) + { SWSS_LOG_ENTER(); } + + Nhg& operator=(Nhg&& nhg); + + ~Nhg() { SWSS_LOG_ENTER(); desync(); } + + /* Sync the group, creating the group's and members SAI IDs. */ + bool sync() override; + + /* Desync the group, reseting the group's and members SAI IDs. */ + bool desync() override; + + /* + * Update the group based on a new next hop group key. This will also + * perform any sync / desync 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, desyncing 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/nhgorch.h b/orchagent/nhgorch.h index f7e4bcd4b6b..ac5e0522d15 100644 --- a/orchagent/nhgorch.h +++ b/orchagent/nhgorch.h @@ -1,242 +1,210 @@ #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; } +using namespace std; - NextHopGroupMember& operator=(NextHopGroupMember&& nhgm); +/* Default maximum number of next hop groups */ +#define DEFAULT_NUMBER_OF_ECMP_GROUPS 128 +#define DEFAULT_MAX_ECMP_GROUP_SIZE 32 - /* - * 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; +extern sai_object_id_t gSwitchId; - /* Destructor. */ - virtual ~NextHopGroupMember(); +extern SwitchOrch *gSwitchOrch; +extern PortsOrch *gPortsOrch; - /* Update member's weight and update the SAI attribute as well. */ - bool updateWeight(uint32_t weight); +extern sai_switch_api_t *sai_switch_api; - /* Sync / Desync. */ - void sync(sai_object_id_t gm_id); - void desync(); - - /* 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); - NextHopGroup(NextHopGroup&& nhg); - NextHopGroup& operator=(NextHopGroup&& nhg); - - /* Destructor. */ - virtual ~NextHopGroup() { desync(); } - - /* Sync the group, creating the group's and members SAI IDs. */ - bool sync(); - - /* Desync the group, reseting the group's and members SAI IDs. */ - bool desync(); + NhgOrch(DBConnector *db, const vector &table_names) : + Orch(db, table_names) + { + SWSS_LOG_ENTER(); + + /* Get the switch's maximum next hop group capacity. */ + sai_attribute_t attr; + attr.id = SAI_SWITCH_ATTR_NUMBER_OF_ECMP_GROUPS; + + sai_status_t status = sai_switch_api->get_switch_attribute(gSwitchId, + 1, + &attr); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("Failed to get switch attribute number of ECMP " + "groups. Use default value. rv:%d", status); + m_maxNhgCount = DEFAULT_NUMBER_OF_ECMP_GROUPS; + } + else + { + m_maxNhgCount = attr.value.s32; + + /* + * ASIC specific workaround to re-calculate maximum ECMP groups + * according to diferent ECMP mode used. + * + * On Mellanox platform, the maximum ECMP groups returned is the value + * under the condition that the ECMP group size is 1. Deviding this + * number by DEFAULT_MAX_ECMP_GROUP_SIZE gets the maximum number of + * ECMP groups when the maximum ECMP group size is 32. + */ + char *platform = getenv("platform"); + if (platform && strstr(platform, MLNX_PLATFORM_SUBSTRING)) + { + SWSS_LOG_NOTICE("Mellanox platform - divide capacity by %d", + DEFAULT_MAX_ECMP_GROUP_SIZE); + m_maxNhgCount /= DEFAULT_MAX_ECMP_GROUP_SIZE; + } + } + + /* Set switch's next hop group capacity. */ + std::vector fvTuple; + fvTuple.emplace_back("MAX_NEXTHOP_GROUP_COUNT", + std::to_string(m_maxNhgCount)); + gSwitchOrch->set_switch_capability(fvTuple); + + SWSS_LOG_NOTICE("Maximum number of ECMP groups supported is %d", + m_maxNhgCount); + } /* - * Update the group based on a new next hop group key. This will also - * perform any sync / desync necessary. + * Get the maximum number of ECMP groups allowed by the switch. */ - bool update(const NextHopGroupKey& nhg_key); - - /* Check if the group contains the given next hop. */ - inline bool hasNextHop(const NextHopKey& nh_key) const { - return m_members.find(nh_key) != m_members.end(); } + static inline unsigned getMaxNhgCount() + { SWSS_LOG_ENTER(); return m_maxNhgCount; } - /* Validate a next hop in the group, syncing it. */ - bool validateNextHop(const NextHopKey& nh_key); + /* + * Get the number of next hop groups that are synced. + */ + static inline unsigned getSyncedNhgCount() + { SWSS_LOG_ENTER(); return NhgBase::getSyncedCount(); } - /* Invalidate a next hop in the group, desyncing it. */ - bool invalidateNextHop(const NextHopKey& nh_key); + /* Increase the number of synced next hop groups. */ + static void incSyncedNhgCount() + { + 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 void setTemp(bool is_temp) { m_is_temp = 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 desyncMembers(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 delete 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 - { return m_syncdNextHopGroups.find(index) != - m_syncdNextHopGroups.end(); } + void incNhgRefCount(const string &index) + { + SWSS_LOG_ENTER(); + + if (nhgHandler.hasNhg(index)) + { + nhgHandler.incNhgRefCount(index); + } + else if (cbfNhgHandler.hasNhg(index)) + { + cbfNhgHandler.incNhgRefCount(index); + } + else + { + throw std::out_of_range("Next hop group index not found."); + } + } /* - * 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 if (cbfNhgHandler.hasNhg(index)) + { + cbfNhgHandler.decNhgRefCount(index); + } + else + { + throw std::out_of_range("Next hop group index not found."); + } } - 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); - -private: + 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); + } + } /* - * 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/orch.cpp b/orchagent/orch.cpp index b4ef2466623..34106187082 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -892,4 +892,3 @@ void Orch2::doTask(Consumer &consumer) } } } - diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 5233385cac1..1e0d125539b 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 @@ -163,8 +165,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, 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); @@ -193,7 +195,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); diff --git a/orchagent/qosorch.cpp b/orchagent/qosorch.cpp index c2e15aa7631..c2e5d8fd8d2 100644 --- a/orchagent/qosorch.cpp +++ b/orchagent/qosorch.cpp @@ -9,6 +9,7 @@ #include #include #include +#include using namespace std; @@ -68,9 +69,15 @@ 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()}, }; + +#define DSCP_MAX_VAL 63 +#define EXP_MAX_VAL 7 + task_process_status QosMapHandler::processWorkItem(Consumer& consumer) { SWSS_LOG_ENTER(); @@ -724,6 +731,168 @@ sai_object_id_t PfcToQueueHandler::addQosItem(const vector &att } +bool DscpToFcMapHandler::convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &tuple, vector &attributes) +{ + SWSS_LOG_ENTER(); + 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 > UCHAR_MAX)) + { + SWSS_LOG_ERROR("FC value %d is either negative, or bigger than max value %d", value, UCHAR_MAX); + 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_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 > UCHAR_MAX)) + { + SWSS_LOG_ERROR("FC value %d is either negative, or bigger than max value %d", value, UCHAR_MAX); + 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(); @@ -754,6 +923,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 d15f4e3a735..4520eacb1ea 100644 --- a/orchagent/qosorch.h +++ b/orchagent/qosorch.h @@ -119,6 +119,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: @@ -146,6 +160,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 cb7847d1e23..d35f161db9a 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -697,8 +697,8 @@ void RouteOrch::doTask(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.is_temp = nh_group.isTemp(); } catch (const std::out_of_range& e) @@ -771,7 +771,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 (gNhgOrch->getNhgCount() >= gNhgOrch->getMaxNhgCount() && + if (gNhgOrch->getSyncedNhgCount() >= gNhgOrch->getMaxNhgCount() && gRouteBulker.removing_entries_count() > 0) { break; @@ -1035,7 +1035,7 @@ bool RouteOrch::createFineGrainedNextHopGroup(sai_object_id_t &next_hop_group_id { SWSS_LOG_ENTER(); - if (gNhgOrch->getNhgCount() >= gNhgOrch->getMaxNhgCount()) + if (gNhgOrch->getSyncedNhgCount() >= gNhgOrch->getMaxNhgCount()) { SWSS_LOG_DEBUG("Failed to create new next hop group. \ Reaching maximum number of next hop groups."); @@ -1057,7 +1057,7 @@ bool RouteOrch::createFineGrainedNextHopGroup(sai_object_id_t &next_hop_group_id } gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); - gNhgOrch->incNhgCount(); + gNhgOrch->incSyncedNhgCount(); return true; } @@ -1079,7 +1079,7 @@ bool RouteOrch::removeFineGrainedNextHopGroup(sai_object_id_t &next_hop_group_id } gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); - gNhgOrch->decNhgCount(); + gNhgOrch->decSyncedNhgCount(); return true; } @@ -1090,7 +1090,7 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops) assert(!hasNextHopGroup(nexthops)); - if (gNhgOrch->getNhgCount() >= gNhgOrch->getMaxNhgCount()) + if (gNhgOrch->getSyncedNhgCount() >= gNhgOrch->getMaxNhgCount()) { SWSS_LOG_WARN("Reached maximum next hop groups of %u", gNhgOrch->getMaxNhgCount()); @@ -1165,8 +1165,8 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops) } } - gNhgOrch->incNhgCount(); - SWSS_LOG_NOTICE("Create next hop group %s", nexthops.to_string().c_str()); + gNhgOrch->incSyncedNhgCount(); + SWSS_LOG_INFO("Create next hop group %s", nexthops.to_string().c_str()); gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); @@ -1267,7 +1267,7 @@ bool RouteOrch::removeNextHopGroup(const NextHopGroupKey &nexthops) } next_hop_group_id = next_hop_group_entry->second.next_hop_group_id; - SWSS_LOG_NOTICE("Delete next hop group %s", nexthops.to_string().c_str()); + SWSS_LOG_INFO("Delete next hop group %s", nexthops.to_string().c_str()); vector next_hop_ids; auto& nhgm = next_hop_group_entry->second.nhopgroup_members; @@ -1319,7 +1319,7 @@ bool RouteOrch::removeNextHopGroup(const NextHopGroupKey &nexthops) } } - gNhgOrch->decNhgCount(); + gNhgOrch->decSyncedNhgCount(); gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); set next_hop_set = nexthops.getNextHops(); @@ -1629,7 +1629,7 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) ctx.nhg_index.c_str()); 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) @@ -1716,9 +1716,9 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) } } - SWSS_LOG_NOTICE("Added route %s with next hop(s) %s", - ipPrefix.to_string().c_str(), - nextHops.to_string().c_str()); + SWSS_LOG_INFO("Added route %s with next hop(s) %s", + ipPrefix.to_string().c_str(), + nextHops.to_string().c_str()); return false; } @@ -1893,7 +1893,7 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey gNhgOrch->incNhgRefCount(ctx.nhg_index); } - SWSS_LOG_NOTICE("Post create route %s with next hop(s) %s", + SWSS_LOG_INFO("Post create route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); } else @@ -1947,7 +1947,7 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey } else if (ol_nextHops.is_overlay_nexthop()) { - SWSS_LOG_NOTICE("Update overlay Nexthop %s", ol_nextHops.to_string().c_str()); + SWSS_LOG_INFO("Update overlay Nexthop %s", ol_nextHops.to_string().c_str()); m_bulkNhgReducedRefCnt.emplace(ol_nextHops, vrf_id); } } @@ -1984,7 +1984,7 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey gNhgOrch->incNhgRefCount(ctx.nhg_index); } - SWSS_LOG_NOTICE("Post set route %s with next hop(s) %s", + SWSS_LOG_INFO("Post set route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); } diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index bbe2d46e6f7..833b08cac74 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -39,7 +39,9 @@ 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/nexthopgroup.cpp \ + $(top_srcdir)/orchagent/nhghandler.cpp \ + $(top_srcdir)/orchagent/cbfnhghandler.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 fb1ffbb120b..1441f5d2e62 100644 --- a/tests/test_nhg.py +++ b/tests/test_nhg.py @@ -6,7 +6,7 @@ import pytest import ipaddress -from swsscommon import swsscommon +from swsscommon import swsscommon as swss class TestNextHopGroup(object): @@ -37,9 +37,9 @@ def get_nhg_id(self, nhg_index, dvs): asic_db = dvs.get_asic_db() asic_rts_count = len(asic_db.get_keys(self.ASIC_RT_STR)) - fvs = swsscommon.FieldValuePairs([('nexthop_group', nhg_index)]) + fvs = swss.FieldValuePairs([('nexthop_group', nhg_index)]) prefix = '255.255.255.255/24' - ps = swsscommon.ProducerStateTable(dvs.get_app_db().db_connection, + ps = swss.ProducerStateTable(dvs.get_app_db().db_connection, "ROUTE_TABLE") ps.set(prefix, fvs) @@ -63,8 +63,11 @@ def get_nhg_id(self, nhg_index, dvs): # Return the NHGID return nhgid - def get_nhgm_ids(self, nhg_index, dvs): - nhgid = self.get_nhg_id(nhg_index, dvs) + def get_nhgm_ids(self, nhg_index, dvs, nh_id=None): + if nh_id is None: + nhgid = self.get_nhg_id(nhg_index, dvs) + else: + nhgid = nh_id nhgms = [] asic_db = dvs.get_asic_db() @@ -123,14 +126,14 @@ def test_route_nhg(self, dvs, dvs_route, testlog): rtprefix = "2.2.2.0/24" app_db = dvs.get_app_db() - ps = swsscommon.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") + ps = swss.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") asic_db = dvs.get_asic_db() dvs_route.check_asicdb_deleted_route_entries([rtprefix]) # nexthop group without weight - fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), + fvs = swss.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) ps.set(rtprefix, fvs) @@ -292,13 +295,13 @@ def test_label_route_nhg(self, dvs, testlog): self.config_intf(i, dvs) app_db = dvs.get_app_db() - lr_ps = swsscommon.ProducerStateTable(app_db.db_connection, "LABEL_ROUTE_TABLE") + lr_ps = swss.ProducerStateTable(app_db.db_connection, "LABEL_ROUTE_TABLE") asic_db = dvs.get_asic_db() asic_insegs_count = len(asic_db.get_keys(self.ASIC_INSEG_STR)) # add label route - fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), + fvs = swss.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) lr_ps.set("10", fvs) @@ -359,13 +362,13 @@ def test_nhgorch_labeled_nhs(self, dvs, testlog): app_db = dvs.get_app_db() asic_db = dvs.get_asic_db() - nhg_ps = swsscommon.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") + nhg_ps = swss.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") asic_nhgs_count = len(asic_db.get_keys(self.ASIC_NHG_STR)) asic_nhgms_count = len(asic_db.get_keys(self.ASIC_NHGM_STR)) asic_nhs_count = len(asic_db.get_keys(self.ASIC_NHS_STR)) # Add a group containing labeled weighted NHs - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('mpls_nh', 'push1,push3'), ('ifname', 'Ethernet0,Ethernet4'), ('weight', '2,4')]) @@ -385,7 +388,7 @@ def test_nhgorch_labeled_nhs(self, dvs, testlog): assert set(weights) == set(['2', '4']) # Create a new single next hop with the same label - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1'), ('mpls_nh', 'push1'), ('ifname', 'Ethernet0')]) nhg_ps.set('group2', fvs) @@ -395,7 +398,7 @@ def test_nhgorch_labeled_nhs(self, dvs, testlog): assert len(asic_db.get_keys(self.ASIC_NHS_STR)) == asic_nhs_count + 2 # Create a new single next hop with a different label - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1'), ('mpls_nh', 'push2'), ('ifname', 'Ethernet0')]) nhg_ps.set('group3', fvs) @@ -418,7 +421,7 @@ def test_nhgorch_labeled_nhs(self, dvs, testlog): asic_db.wait_for_n_keys(self.ASIC_NHS_STR, asic_nhs_count + 2) # Update group1 with no weights and both labeled and unlabeled NHs - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('mpls_nh', 'push2,na'), ('ifname', 'Ethernet0,Ethernet4')]) nhg_ps.set('group1', fvs) @@ -451,8 +454,8 @@ def test_nhgorch_excp_group_cases(self, dvs, testlog): app_db = dvs.get_app_db() asic_db = dvs.get_asic_db() - nhg_ps = swsscommon.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") - rt_ps = swsscommon.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") + nhg_ps = swss.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") + rt_ps = swss.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") def get_nhg_keys(): return asic_db.get_keys(self.ASIC_NHG_STR) @@ -475,7 +478,7 @@ def get_rt_keys(): assert len(get_nhg_keys()) == asic_nhgs_count # Create a next hop group with a member that does not exist - should fail - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.63'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.63'), ("ifname", "Ethernet0,Ethernet4,Ethernet124")]) nhg_ps.set("group1", fvs) time.sleep(1) @@ -484,7 +487,7 @@ def get_rt_keys(): # Issue an update for this next hop group that doesn't yet exist, # which contains only valid NHs. This will overwrite the previous # operation and create the group. - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.5'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.5'), ("ifname", "Ethernet0,Ethernet8")]) nhg_ps.set("group1", fvs) asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 1) @@ -504,7 +507,7 @@ def get_rt_keys(): # Add a route referencing the new group asic_rts_count = len(get_rt_keys()) - fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group1')]) + fvs = swss.FieldValuePairs([('nexthop_group', 'group1')]) rt_ps.set('2.2.2.0/24', fvs) asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_rts_count + 1) @@ -521,14 +524,14 @@ def get_rt_keys(): assert len(get_nhg_keys()) == asic_nhgs_count + 1 # Create a new group - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('ifname', 'Ethernet0,Ethernet4')]) nhg_ps.set("group2", fvs) asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 4) # Update the route to point to the new group - fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group2')]) + fvs = swss.FieldValuePairs([('nexthop_group', 'group2')]) rt_ps.set('2.2.2.0/24', fvs) # The first group should have got deleted @@ -539,7 +542,7 @@ def get_rt_keys(): # Update the route with routeOrch's owned next hop group nhgid = asic_db.get_entry(self.ASIC_RT_STR, rt_key)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('ifname', 'Ethernet0,Ethernet4')]) rt_ps.set('2.2.2.0/24', fvs) @@ -552,7 +555,7 @@ def get_rt_keys(): nhgid = asic_db.get_entry(self.ASIC_RT_STR, rt_key)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] # Update the route to point back to group2 - fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group2')]) + fvs = swss.FieldValuePairs([('nexthop_group', 'group2')]) rt_ps.set('2.2.2.0/24', fvs) # The routeOrch's owned next hop group should get deleted @@ -564,24 +567,39 @@ def get_rt_keys(): # Create a new group with the same members as group2 nhgid = asic_db.get_entry(self.ASIC_RT_STR, rt_key)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('ifname', 'Ethernet0,Ethernet4')]) nhg_ps.set("group1", fvs) asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 4) # Update the route to point to the new group - fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group1')]) + fvs = swss.FieldValuePairs([('nexthop_group', 'group1')]) rt_ps.set('2.2.2.0/24', fvs) time.sleep(1) # Assert the next hop group ID changed assert asic_db.get_entry(self.ASIC_RT_STR, rt_key)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] != nhgid + # Delete group1 while being referenced + nhg_ps._del('group1') + time.sleep(1) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) + + # Update group1 + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.5'), + ('ifname', 'Ethernet4,Ethernet8')]) + nhg_ps.set("group1", fvs) + time.sleep(1) + # Remove the route rt_ps._del('2.2.2.0/24') asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_rts_count) + # The group1 update should have overwritten the delete + time.sleep(1) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) + # Remove the groups nhg_ps._del('group1') nhg_ps._del('group2') @@ -590,7 +608,7 @@ def get_rt_keys(): asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count) # Create a route with labeled NHs - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('mpls_nh', 'push1,push3'), ('ifname', 'Ethernet0,Ethernet4'), ('weight', '2,4')]) @@ -608,7 +626,7 @@ def get_rt_keys(): assert len(asic_db.get_keys(self.ASIC_NHS_STR)) == asic_nhs_count + 2 # Update the group with a different NH - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('mpls_nh', 'push2,push3'), ('ifname', 'Ethernet0,Ethernet4'), ('weight', '2,4')]) @@ -631,7 +649,7 @@ def get_rt_keys(): asic_db.wait_for_n_keys(self.ASIC_NHS_STR, asic_nhs_count) # Add a route with a NHG that does not exist - fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group1')]) + fvs = swss.FieldValuePairs([('nexthop_group', 'group1')]) rt_ps.set('2.2.2.0/24', fvs) time.sleep(1) assert asic_rts_count == len(asic_db.get_keys(self.ASIC_RT_STR)) @@ -673,7 +691,7 @@ def gen_nhg_fvs(binary): nexthop = ','.join(nexthop) ifname = ','.join(ifname) - fvs = swsscommon.FieldValuePairs([("nexthop", nexthop), ("ifname", ifname)]) + fvs = swss.FieldValuePairs([("nexthop", nexthop), ("ifname", ifname)]) return fvs def asic_route_nhg_fvs(k): @@ -701,7 +719,7 @@ def asic_route_nhg_fvs(k): app_db = dvs.get_app_db() asic_db = dvs.get_asic_db() - ps = swsscommon.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") + ps = swss.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") dvs.runcmd('swssloglevel -c orchagent -l INFO') @@ -729,9 +747,10 @@ def asic_route_nhg_fvs(k): asic_routes_count += MAX_ECMP_COUNT # Add a route with labeled NHs + time.sleep(1) asic_nhs_count = len(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'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('mpls_nh', 'push1,push3'), ('ifname', 'Ethernet0,Ethernet4')]) ps.set(route_ipprefix, fvs) @@ -827,8 +846,8 @@ def asic_route_nhg_fvs(k): def test_nhgorch_nhg_exhaust(self, dvs, testlog): app_db = dvs.get_app_db() asic_db = dvs.get_asic_db() - nhg_ps = swsscommon.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") - rt_ps = swsscommon.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") + nhg_ps = swss.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") + rt_ps = swss.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") MAX_ECMP_COUNT = 512 MAX_PORT_COUNT = 10 @@ -850,7 +869,7 @@ def gen_nhg_fvs(binary): nexthop = ','.join(nexthop) ifname = ','.join(ifname) - fvs = swsscommon.FieldValuePairs([("nexthop", nexthop), ("ifname", ifname)]) + fvs = swss.FieldValuePairs([("nexthop", nexthop), ("ifname", ifname)]) return fvs @@ -912,7 +931,7 @@ def nhg_exists(nhg_index): nhg_ps.set(nhg_index, nhg_fvs) # Wait for the group to be added - asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + nhg_count + 1) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, nhg_count + 1) # Save the NHG index/ID pair asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index, dvs) @@ -1006,7 +1025,7 @@ def nhg_exists(nhg_index): # Add a route asic_rts_count = len(asic_db.get_keys(self.ASIC_RT_STR)) - rt_fvs = swsscommon.FieldValuePairs([('nexthop_group', nhg_index)]) + rt_fvs = swss.FieldValuePairs([('nexthop_group', nhg_index)]) rt_ps.set('2.2.2.0/24', rt_fvs) # Assert the route is created @@ -1023,7 +1042,7 @@ def nhg_exists(nhg_index): assert rt_id != None # Update the route to point to the temporary NHG - rt_fvs = swsscommon.FieldValuePairs([('nexthop_group', nhg_index)]) + rt_fvs = swss.FieldValuePairs([('nexthop_group', nhg_index)]) rt_ps.set('2.2.2.0/24', rt_fvs) # Wait for the route to change it's NHG ID @@ -1078,7 +1097,7 @@ def nhg_exists(nhg_index): # Try updating the promoted NHG to a single NHG. Should fail as it no # longer is a temporary NHG nhg_id = self.get_nhg_id(nhg_index, dvs) - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1'), ('ifname', 'Ethernet0')]) nhg_ps.set(nhg_index, fvs) time.sleep(1) @@ -1088,12 +1107,18 @@ def nhg_exists(nhg_index): prev_nhg_id = nhg_id # Create a temporary next hop groups + dvs.runcmd('swssloglevel -c orchagent -l INFO') nhg_index, binary = create_temp_nhg() - nhg_id = self.get_nhg_id(nhg_index, dvs) # Update the route to point to the temporary group - fvs = swsscommon.FieldValuePairs([('nexthop_group', nhg_index)]) + fvs = swss.FieldValuePairs([('nexthop_group', nhg_index)]) rt_ps.set('2.2.2.0/24', fvs) + asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, prev_nhg_id) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, MAX_ECMP_COUNT) + nhg_id = self.get_nhg_id(nhg_index, dvs) + asic_db.wait_for_field_match(self.ASIC_RT_STR, + rt_id, + {'SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID': nhg_id}) # The previous group should be updated to a single NHG now, freeing a # NHG slot in ASIC and promoting the temporary one @@ -1106,11 +1131,11 @@ def nhg_exists(nhg_index): nhg_id = self.get_nhg_id(nhg_index, dvs) # Update the route to point to the temporary group - fvs = swsscommon.FieldValuePairs([('nexthop_group', nhg_index)]) + fvs = swss.FieldValuePairs([('nexthop_group', nhg_index)]) rt_ps.set('2.2.2.0/24', fvs) # Update the group to a single NHG - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1'), ('ifname', 'Ethernet0')]) nhg_ps.set(nhg_index, fvs) nhg_id = self.get_nhg_id(nhg_index, dvs) @@ -1121,7 +1146,7 @@ def nhg_exists(nhg_index): {'SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID': nhg_id}) # Try to update the nexthop group to another one - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.3'), ('ifname', 'Ethernet4')]) nhg_ps.set(nhg_index, fvs) @@ -1137,7 +1162,7 @@ def nhg_exists(nhg_index): # Create a next hop group that contains labeled NHs that do not exist # in NeighOrch asic_nhs_count = len(asic_db.get_keys(self.ASIC_NHS_STR)) - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ('mpls_nh', 'push1,push3'), ('ifname', 'Ethernet0,Ethernet4')]) nhg_index = gen_nhg_index(nhg_count) @@ -1189,7 +1214,7 @@ def nhg_exists(nhg_index): asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index, dvs) # 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'), + nhg_fvs = swss.FieldValuePairs([('nexthop', '10.0.0.21,10.0.0.23'), ('ifname', 'Ethernet40,Ethernet44')]) nhg_index = gen_nhg_index(nhg_count) nhg_count += 1 @@ -1208,7 +1233,7 @@ def nhg_exists(nhg_index): assert nhg_id is not None # Update the temporary NHG to an invalid one again - nhg_fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.21,10.0.0.23'), + nhg_fvs = swss.FieldValuePairs([('nexthop', '10.0.0.21,10.0.0.23'), ('ifname', 'Ethernet40,Ethernet44')]) nhg_ps.set(nhg_index, nhg_fvs) @@ -1239,13 +1264,13 @@ def test_nhgorch_multi_nh_group(self, dvs, testlog): # create next hop group in APPL DB - nhg_ps = swsscommon.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") + nhg_ps = swss.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") prev_nhg_keys = asic_db.get_keys(self.ASIC_NHG_STR) asic_nhgs_count = len(prev_nhg_keys) asic_nhgms_count = len(asic_db.get_keys(self.ASIC_NHGM_STR)) - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5'), ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) nhg_ps.set("group1", fvs) @@ -1279,11 +1304,11 @@ def test_nhgorch_multi_nh_group(self, dvs, testlog): # create route in APPL DB - rt_ps = swsscommon.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") + rt_ps = swss.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") asic_routes_count = len(asic_db.get_keys(self.ASIC_RT_STR)) - fvs = swsscommon.FieldValuePairs([("nexthop_group", "group1")]) + fvs = swss.FieldValuePairs([("nexthop_group", "group1")]) rt_ps.set("2.2.2.0/24", fvs) # check if route was propagated to ASIC DB @@ -1328,7 +1353,7 @@ def test_nhgorch_multi_nh_group(self, dvs, testlog): asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, 2) # Create a group that contains a NH that uses the down link - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ("ifname", "Ethernet0,Ethernet4")]) nhg_ps.set('group2', fvs) @@ -1338,7 +1363,7 @@ def test_nhgorch_multi_nh_group(self, dvs, testlog): asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 3) # Update the NHG with one interface down - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.1'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.1'), ("ifname", "Ethernet4,Ethernet0")]) nhg_ps.set("group1", fvs) @@ -1358,7 +1383,7 @@ def test_nhgorch_multi_nh_group(self, dvs, testlog): asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 2) # Create group2 with a NH that does not exist - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.63'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.63'), ("ifname", "Ethernet4,Ethernet124")]) nhg_ps.set("group2", fvs) @@ -1367,7 +1392,7 @@ def test_nhgorch_multi_nh_group(self, dvs, testlog): asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 1) # Update group1 with a NH that does not exist - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.63'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.63'), ("ifname", "Ethernet4,Ethernet124")]) nhg_ps.set("group1", fvs) @@ -1390,7 +1415,7 @@ def test_nhgorch_multi_nh_group(self, dvs, testlog): asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 1) # Update the NHG, adding two new members - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5,10.0.0.7'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5,10.0.0.7'), ("ifname", "Ethernet0,Ethernet4,Ethernet8,Ethernet12")]) nhg_ps.set("group1", fvs) @@ -1399,7 +1424,7 @@ def test_nhgorch_multi_nh_group(self, dvs, testlog): # Update the group to one NH only - orchagent should fail as it is # referenced - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), ("ifname", "Ethernet0")]) + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1'), ("ifname", "Ethernet0")]) nhg_ps.set("group1", fvs) time.sleep(1) @@ -1431,8 +1456,8 @@ def test_nhgorch_single_nh_group(self, dvs, testlog): asic_nhgs_count = len(asic_db.get_keys(self.ASIC_NHG_STR)) # Create single next hop group in APPL DB - nhg_ps = swsscommon.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), ("ifname", "Ethernet0")]) + nhg_ps = swss.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1'), ("ifname", "Ethernet0")]) nhg_ps.set("group1", fvs) time.sleep(1) @@ -1451,11 +1476,11 @@ def test_nhgorch_single_nh_group(self, dvs, testlog): # create route in APPL DB - rt_ps = swsscommon.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") + rt_ps = swss.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") asic_routes_count = len(asic_db.get_keys(self.ASIC_RT_STR)) - fvs = swsscommon.FieldValuePairs([("nexthop_group", "group1")]) + fvs = swss.FieldValuePairs([("nexthop_group", "group1")]) rt_ps.set("2.2.2.0/24", fvs) # check if route was propagated to ASIC DB @@ -1484,12 +1509,12 @@ def test_nhgorch_single_nh_group(self, dvs, testlog): self.flap_intf(1, 'down', dvs) # Create group2 pointing to the NH which's link is down - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.3'), ("ifname", "Ethernet4")]) + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.3'), ("ifname", "Ethernet4")]) nhg_ps.set("group2", fvs) # The group should be created. To test this, add a route pointing to # it. If the group exists, the route will be created as well. - fvs = swsscommon.FieldValuePairs([("nexthop_group", "group2")]) + fvs = swss.FieldValuePairs([("nexthop_group", "group2")]) rt_ps.set('2.2.4.0/24', fvs) asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_routes_count + 2) @@ -1501,12 +1526,12 @@ def test_nhgorch_single_nh_group(self, dvs, testlog): self.flap_intf(1, 'up', dvs) # Create group2 pointing to a NH that does not exist - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.61'), ("ifname", "Ethernet120")]) + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.61'), ("ifname", "Ethernet120")]) nhg_ps.set("group2", fvs) # The group should fail to be created. To test this, we add a route # pointing to it. The route should not be created. - fvs = swsscommon.FieldValuePairs([("nexthop_group", "group2")]) + fvs = swss.FieldValuePairs([("nexthop_group", "group2")]) rt_ps.set('2.2.4.0/24', fvs) asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_routes_count + 1) @@ -1525,7 +1550,7 @@ def test_nhgorch_single_nh_group(self, dvs, testlog): # Update the group to a multiple NH group - should fail as the group # is referenced - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ("ifname", "Ethernet0,Ethernet4")]) nhg_ps.set("group1", fvs) time.sleep(1) @@ -1534,7 +1559,7 @@ def test_nhgorch_single_nh_group(self, dvs, testlog): # Update group1 to point to another NH - should fail as the group is # referenced nhgid = self.get_nhg_id('group1', dvs) - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.3'), ("ifname", "Ethernet4")]) + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.3'), ("ifname", "Ethernet4")]) nhg_ps.set("group1", fvs) assert nhgid == self.get_nhg_id('group1', dvs) @@ -1549,7 +1574,7 @@ def test_nhgorch_single_nh_group(self, dvs, testlog): # Update the group to a multiple NH group - should work as the group is # not referenced - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ("ifname", "Ethernet0,Ethernet4")]) nhg_ps.set("group1", fvs) asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 1) @@ -1566,13 +1591,13 @@ def test_nhgorch_label_route(self, dvs, testlog): # create next hop group in APPL DB - nhg_ps = swsscommon.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") + nhg_ps = swss.ProducerStateTable(app_db.db_connection, "NEXTHOP_GROUP_TABLE") prev_nhg_keys = asic_db.get_keys(self.ASIC_NHG_STR) asic_nhgs_count = len(prev_nhg_keys) asic_nhgms_count = len(asic_db.get_keys(self.ASIC_NHGM_STR)) - fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5'), + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5'), ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) nhg_ps.set("group1", fvs) @@ -1606,11 +1631,11 @@ def test_nhgorch_label_route(self, dvs, testlog): # create prefix route in APPL DB - rt_ps = swsscommon.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") + rt_ps = swss.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") asic_routes_count = len(asic_db.get_keys(self.ASIC_RT_STR)) - fvs = swsscommon.FieldValuePairs([("nexthop_group", "group1")]) + fvs = swss.FieldValuePairs([("nexthop_group", "group1")]) rt_ps.set("2.2.2.0/24", fvs) # check if route was propagated to ASIC DB @@ -1625,11 +1650,11 @@ def test_nhgorch_label_route(self, dvs, testlog): # create label route in APPL DB pointing to the same next hop group - lrt_ps = swsscommon.ProducerStateTable(app_db.db_connection, "LABEL_ROUTE_TABLE") + lrt_ps = swss.ProducerStateTable(app_db.db_connection, "LABEL_ROUTE_TABLE") asic_insegs_count = len(asic_db.get_keys(self.ASIC_INSEG_STR)) - fvs = swsscommon.FieldValuePairs([("nexthop_group", "group1")]) + fvs = swss.FieldValuePairs([("nexthop_group", "group1")]) lrt_ps.set("20", fvs) # check if label route was propagated to ASIC DB @@ -1690,11 +1715,11 @@ def test_nhgorch_label_route(self, dvs, testlog): # create label route in APPL DBwith no properties (so just popping the label) - lrt_ps = swsscommon.ProducerStateTable(app_db.db_connection, "LABEL_ROUTE_TABLE") + lrt_ps = swss.ProducerStateTable(app_db.db_connection, "LABEL_ROUTE_TABLE") asic_insegs_count = len(asic_db.get_keys(self.ASIC_INSEG_STR)) - fvs = swsscommon.FieldValuePairs([("ifname","")]) + fvs = swss.FieldValuePairs([("ifname","")]) lrt_ps.set("30", fvs) # check if label route was propagated to ASIC DB @@ -1714,6 +1739,436 @@ def test_nhgorch_label_route(self, dvs, testlog): # Wait for route 20 to be removed asic_db.wait_for_n_keys(self.ASIC_INSEG_STR, asic_insegs_count) + def test_cbf_nhgorch(self, dvs, testlog): + for i in range(4): + self.config_intf(i, dvs) + + app_db = dvs.get_app_db() + asic_db = dvs.get_asic_db() + cbf_nhg_ps = swss.ProducerStateTable(app_db.db_connection, "CLASS_BASED_NEXT_HOP_GROUP_TABLE") + nhg_ps = swss.ProducerStateTable(app_db.db_connection, "NEXT_HOP_GROUP_TABLE") + + asic_nhgs_count = len(asic_db.get_keys(self.ASIC_NHG_STR)) + asic_nhgms_count = len(asic_db.get_keys(self.ASIC_NHGM_STR)) + + # Create two non-CBF nhgs + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5'), + ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) + nhg_ps.set("group1", fvs) + + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('ifname', 'Ethernet0,Ethernet4')]) + nhg_ps.set('group2', fvs) + + # Wait for the groups to appear in ASIC DB + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) + + # Create a CBF NHG + fvs = swss.FieldValuePairs([('members', 'group1,group2'), + ('class_map', '0:0,1:0,2:0,3:0,4:1,5:1,6:1,7:1')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + + # Check if the group makes it to ASIC DB + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 3) + nhgid = self.get_nhg_id('cbfgroup1', dvs) + assert(nhgid != None) + fvs = 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 + asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 7) + nhgm_ids = self.get_nhgm_ids('cbfgroup1', dvs) + assert(len(nhgm_ids) == 2) + nhg_ids = dict(zip([self.get_nhg_id(x, dvs) for x in ['group1', 'group2']], ['0', '1'])) + for nhgm_id in nhgm_ids: + fvs = 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 + cbf_nhg_ps._del('cbfgroup1') + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) + + # Test the data validation + fv_values = [ + ('', '0:0,1:1'), # empty members + ('group1,group2', ''), # empty class map + ('group1,group1', '0:0,1:1'), # non-unique members + ('group1,group2', '0;0,1:1'), # ill-formed class map + ('group1,group2', '-1:0,1:1'), # negative FC value + ('group1,group2', '0:-1,1:1'), # negative indeex + ('group1,group2', '0:2,1:1'), # index out of range + ('group1,group2', '0:0,0:1'), # non-unique class map + ('group1,group2', 'asd:1,0:0'), # FC not integer + ('group1,group2', '0:asd,1:1'), # index not integer + ('group1;group2', '0:0,1:1'), # invalid members separator + ('group1,group2', '0:0;1:1'), # invalid class map separator + ] + + for members, class_map in fv_values: + fvs = swss.FieldValuePairs([('members', members), + ('class_map', class_map)]) + cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + assert(len(asic_db.get_keys(self.ASIC_NHG_STR)) == asic_nhgs_count + 2) + + # Create a NHG with a single next hop + fvs = swss.FieldValuePairs([('nexthop', '10.0.0.1'), + ("ifname", "Ethernet0")]) + nhg_ps.set("group3", fvs) + time.sleep(1) + + # Create a CBF NHG + fvs = swss.FieldValuePairs([('members', 'group1,group3'), + ('class_map', '0:0,1:1')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 3) + asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 7)\ + + # Remove group1 while being referenced - should not work + nhg_ps._del('group1') + time.sleep(1) + assert(len(asic_db.get_keys(self.ASIC_NHG_STR)) == asic_nhgs_count + 3) + + # Update the CBF NHG replacing group1 with group2 + nhgm_ids = self.get_nhgm_ids('cbfgroup1', dvs) + fvs = swss.FieldValuePairs([('members', 'group2,group3'), + ('class_map', '0:0,1:1')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + asic_db.wait_for_deleted_keys(self.ASIC_NHGM_STR, nhgm_ids) + # group1 should be deleted and the updated members added + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) + asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 4) + + # Update the CBF NHG changing the order of the members + nhgm_ids = self.get_nhgm_ids('cbfgroup1', dvs) + fvs = swss.FieldValuePairs([('members', 'group3,group2'), + ('class_map', '0:0,1:1')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + asic_db.wait_for_deleted_keys(self.ASIC_NHGM_STR, nhgm_ids) + asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 4) + + indexes = { + self.get_nhg_id('group3', dvs): '0', + self.get_nhg_id('group2', dvs): '1' + } + nhgm_ids = self.get_nhgm_ids('cbfgroup1', dvs) + for nhgm_id in nhgm_ids: + fvs = 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) + + # Update the CBF NHG class map + nhg_id = self.get_nhg_id('cbfgroup1', dvs) + old_map = asic_db.get_entry(self.ASIC_NHG_STR, nhg_id)['SAI_NEXT_HOP_GROUP_ATTR_FORWARDING_CLASS_TO_INDEX_MAP'] + fvs = swss.FieldValuePairs([('members', 'group3,group2'), + ('class_map', '0:1,1:0')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + asic_db.wait_for_field_negative_match(self.ASIC_NHG_STR, + nhg_id, + {'SAI_NEXT_HOP_GROUP_ATTR_FORWARDING_CLASS_TO_INDEX_MAP': old_map}) + + # Create a route pointing to the CBF NHG + asic_rts_count = len(asic_db.get_keys(self.ASIC_RT_STR)) + rt_ps = swss.ProducerStateTable(app_db.db_connection, swss.APP_ROUTE_TABLE_NAME) + fvs = swss.FieldValuePairs([('nexthop_group', 'cbfgroup1')]) + rt_ps.set('2.2.2.0/24', fvs) + asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_rts_count + 1) + + # Try deleting thee CBF NHG - should not work + cbf_nhg_ps._del('cbfgroup1') + time.sleep(1) + assert(len(asic_db.get_keys(self.ASIC_NHG_STR)) == asic_nhgs_count + 2) + + # Delete the route - the CBF NHG should also get deleted + rt_ps._del('2.2.2.0/24') + asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_rts_count) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 1) + asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 2) + + # Create a route pointing to a CBF NHG that does not yet exist + fvs = swss.FieldValuePairs([('nexthop_group', 'cbfgroup1')]) + rt_ps.set('2.2.2.0/24', fvs) + time.sleep(1) + asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_rts_count) + + # Create the CBF NHG + fvs = swss.FieldValuePairs([('members', 'group3,group2'), + ('class_map', '0:0,1:1')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) + asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 4) + asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_rts_count + 1) + + # Try deleting the CBF group + cbf_nhg_ps._del('cbfgroup1') + time.sleep(1) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) + + # Update the CBF group + nhgm_ids = self.get_nhgm_ids('cbfgroup1', dvs) + nhg_id = self.get_nhg_id('cbfgroup1', dvs) + old_map = asic_db.get_entry(self.ASIC_NHG_STR, nhg_id)['SAI_NEXT_HOP_GROUP_ATTR_FORWARDING_CLASS_TO_INDEX_MAP'] + fvs = swss.FieldValuePairs([('members', 'group2'), ('class_map', '0:0')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + asic_db.wait_for_deleted_keys(self.ASIC_NHGM_STR, nhgm_ids) + asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count + 3) + nhgm_id = self.get_nhgm_ids('cbfgroup1', dvs)[0] + asic_db.wait_for_field_match(self.ASIC_NHGM_STR, + nhgm_id, + {'SAI_NEXT_HOP_GROUP_MEMBER_ATTR_INDEX': '0'}) + asic_db.wait_for_field_negative_match(self.ASIC_NHG_STR, + nhg_id, + {'SAI_NEXT_HOP_GROUP_ATTR_FORWARDING_CLASS_TO_INDEX_MAP': old_map}) + + # Delete the route + rt_ps._del('2.2.2.0/24') + asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_rts_count) + # The update should have overwritten the delete for CBF NHG + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count + 2) + + # Delete the NHGs + cbf_nhg_ps._del('cbfgroup1') + nhg_ps._del('group2') + nhg_ps._del('group3') + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_count) + asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, asic_nhgms_count) + + def test_cbf_nhg_exhaust(self, dvs, testlog): + app_db = dvs.get_app_db() + asic_db = dvs.get_asic_db() + nhg_ps = swss.ProducerStateTable(app_db.db_connection, swss.APP_NEXT_HOP_GROUP_TABLE_NAME) + cbf_nhg_ps = swss.ProducerStateTable(app_db.db_connection, swss.APP_CLASS_BASED_NEXT_HOP_GROUP_TABLE_NAME) + rt_ps = swss.ProducerStateTable(app_db.db_connection, swss.APP_ROUTE_TABLE_NAME) + + MAX_ECMP_COUNT = 512 + MAX_PORT_COUNT = 10 + + r = 0 + fmt = '{{0:0{}b}}'.format(MAX_PORT_COUNT) + asic_nhgs_count = len(asic_db.get_keys(self.ASIC_NHG_STR)) + nhg_count = asic_nhgs_count + first_valid_nhg = nhg_count + + def gen_nhg_fvs(binary): + nexthop = [] + ifname = [] + + for i in range(MAX_PORT_COUNT): + if binary[i] == '1': + nexthop.append(self.peer_ip(i)) + ifname.append(self.port_name(i)) + + nexthop = ','.join(nexthop) + ifname = ','.join(ifname) + fvs = swss.FieldValuePairs([("nexthop", nexthop), ("ifname", ifname)]) + + return fvs + + def gen_nhg_index(nhg_number): + return "group{}".format(nhg_number) + + def gen_valid_binary(): + nonlocal r + + while True: + r += 1 + binary = fmt.format(r) + # We need at least 2 ports for a nexthop group + if binary.count('1') <= 1: + continue + return binary + + def create_temp_nhg(): + nonlocal nhg_count + + binary = gen_valid_binary() + nhg_fvs = gen_nhg_fvs(binary) + nhg_index = gen_nhg_index(nhg_count) + nhg_ps.set(nhg_index, nhg_fvs) + nhg_count += 1 + + return nhg_index, binary + + def delete_nhg(): + nonlocal first_valid_nhg + + del_nhg_index = gen_nhg_index(first_valid_nhg) + del_nhg_id = asic_nhgs[del_nhg_index] + + nhg_ps._del(del_nhg_index) + asic_nhgs.pop(del_nhg_index) + first_valid_nhg += 1 + + return del_nhg_id + + def nhg_exists(nhg_index): + return self.get_nhg_id(nhg_index, dvs) is not None + + # Create interfaces + for i in range(MAX_PORT_COUNT): + self.config_intf(i, dvs) + + asic_nhgs = {} + + # Add first batch of next hop groups to reach the NHG limit + while nhg_count < MAX_ECMP_COUNT: + r += 1 + binary = fmt.format(r) + # We need at least 2 ports for a nexthop group + if binary.count('1') <= 1: + continue + nhg_fvs = gen_nhg_fvs(binary) + nhg_index = gen_nhg_index(nhg_count) + nhg_ps.set(nhg_index, nhg_fvs) + + # Wait for the group to be added + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, nhg_count + 1) + + # Save the NHG index/ID pair + asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index, dvs) + + nhg_count += 1 + + # Create a CBF NHG group - it should fail + fvs = swss.FieldValuePairs([('members', 'group2'), + ('class_map', '0:0')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, MAX_ECMP_COUNT) + + # Delete a NHG + del_nhg_id = delete_nhg() + asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + + # The CBF NHG should be created + assert(nhg_exists('cbfgroup1')) + nhg_id = self.get_nhg_id('cbfgroup1', dvs) + + # Create a temporary NHG + nhg_index, _ = create_temp_nhg() + + # Update the NHG to contain the temporary NHG + fvs = swss.FieldValuePairs([('members', '{}'.format(nhg_index)), + ('class_map', '0:0')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + time.sleep(1) + nhgm_id = self.get_nhgm_ids(None, dvs, nhg_id)[0] + fvs = 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', dvs)) + + # Delete a temporary NHG + nh_id = self.get_nhg_id(nhg_index, dvs) + del_nhg_id = delete_nhg() + asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + + # The NHG should be promoted and the CBF NHG member updated + asic_db.wait_for_field_negative_match(self.ASIC_NHGM_STR, + nhgm_id, + {'SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID': nh_id}) + asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index, dvs) + + # Create a couple more temporary NHGs + nhg_index1, _ = create_temp_nhg() + nhg_index2, _ = create_temp_nhg() + + # Update the CBF NhG to contain both of them as well + fvs = swss.FieldValuePairs([('members', '{},{},{}'.format(nhg_index, nhg_index1, nhg_index2)), + ('class_map', '0:0,1:1,2:2')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + + # The previous CBF NHG member should be deleted + asic_db.wait_for_deleted_entry(self.ASIC_NHGM_STR, nhgm_id) + nhgm_ids = self.get_nhgm_ids(None, dvs, nhg_id) + values = {} + for nhgm_id in nhgm_ids: + fvs = asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_id) + values[nhgm_id] = fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID'] + + # Update the class map of the group + old_map = asic_db.get_entry(self.ASIC_NHG_STR, nhg_id)['SAI_NEXT_HOP_GROUP_ATTR_FORWARDING_CLASS_TO_INDEX_MAP'] + members = {nhgm_id: asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_id) for nhgm_id in self.get_nhgm_ids(None, dvs, nhg_id)} + fvs = swss.FieldValuePairs([('members', '{},{},{}'.format(nhg_index, nhg_index1, nhg_index2)), + ('class_map', '0:1,1:0,2:2')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + + # Check that the map was updates, but the members weren't + asic_db.wait_for_field_negative_match(self.ASIC_NHG_STR, + nhg_id, + {'SAI_NEXT_HOP_GROUP_ATTR_FORWARDING_CLASS_TO_INDEX_MAP': old_map}) + for nhgm_id in self.get_nhgm_ids(None, dvs, nhg_id): + assert(members[nhgm_id] == asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_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 = delete_nhg() + asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + + # Check that one of the temporary NHGs has been updated + diff_count = 0 + for nhgm_id, nh_id in values.items(): + fvs = 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) + + for index in [nhg_index1, nhg_index2]: + asic_nhgs[index] = self.get_nhg_id(index, dvs) + + # Create a temporary NHG + nhg_index, binary = create_temp_nhg() + + # Update the CBF NHG to point to the new NHG + nhgm_ids = self.get_nhgm_ids(None, dvs, nhg_id) + fvs = swss.FieldValuePairs([('members', nhg_index), ('class_map', '0:0')]) + cbf_nhg_ps.set('cbfgroup1', fvs) + asic_db.wait_for_deleted_keys(self.ASIC_NHGM_STR, nhgm_ids) + + # Update the temporary NHG to force using a new designated NH + new_binary = [] + + 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, dvs) + nhg_fvs = gen_nhg_fvs(binary) + nhg_ps.set(nhg_index, nhg_fvs) + + # The CBF NHG member's NH attribute should be updated + nhgm_id = self.get_nhgm_ids(None, dvs, nhg_id)[0] + 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 = delete_nhg() + 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, dvs) + asic_db.wait_for_field_match(self.ASIC_NHGM_STR, + nhgm_id, + {'SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID': nh_id}) + asic_nhgs[nhg_index] = nh_id + + # Delete the NHGs + cbf_nhg_ps._del('cbfgroup1') + for k in asic_nhgs: + nhg_ps._del(k) + asic_db.wait_for_n_keys(self.ASIC_NHG_STR, asic_nhgs_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 32a8b396aa4..b814b17e85c 100644 --- a/tests/test_qos_map.py +++ b/tests/test_qos_map.py @@ -3,9 +3,8 @@ import sys import time -from swsscommon import swsscommon +from swsscommon import swsscommon as swss -CFG_DOT1P_TO_TC_MAP_TABLE_NAME = "DOT1P_TO_TC_MAP" CFG_DOT1P_TO_TC_MAP_KEY = "AZURE" DOT1P_TO_TC_MAP = { "0": "0", @@ -18,29 +17,26 @@ "7": "7", } -CFG_PORT_QOS_MAP_TABLE_NAME = "PORT_QOS_MAP" CFG_PORT_QOS_MAP_FIELD = "dot1p_to_tc_map" -CFG_PORT_TABLE_NAME = "PORT" class TestDot1p(object): def connect_dbs(self, dvs): - self.asic_db = swsscommon.DBConnector(1, dvs.redis_sock, 0) - self.config_db = swsscommon.DBConnector(4, dvs.redis_sock, 0) + self.asic_db = swss.DBConnector(swss.ASIC_DB, dvs.redis_sock, 0) + self.config_db = swss.DBConnector(swss.CONFIG_DB, dvs.redis_sock, 0) def create_dot1p_profile(self): - tbl = swsscommon.Table(self.config_db, CFG_DOT1P_TO_TC_MAP_TABLE_NAME) - fvs = swsscommon.FieldValuePairs(list(DOT1P_TO_TC_MAP.items())) + tbl = swss.Table(self.config_db, swss.CFG_DOT1P_TO_TC_MAP_TABLE_NAME) + fvs = swss.FieldValuePairs(list(DOT1P_TO_TC_MAP.items())) tbl.set(CFG_DOT1P_TO_TC_MAP_KEY, fvs) time.sleep(1) - def find_dot1p_profile(self): found = False dot1p_tc_map_raw = None dot1p_tc_map_key = None - tbl = swsscommon.Table(self.asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP") + tbl = swss.Table(self.asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP") keys = tbl.getKeys() for key in keys: (status, fvs) = tbl.get(key) @@ -62,9 +58,9 @@ def find_dot1p_profile(self): def apply_dot1p_profile_on_all_ports(self): - tbl = swsscommon.Table(self.config_db, CFG_PORT_QOS_MAP_TABLE_NAME) - fvs = swsscommon.FieldValuePairs([(CFG_PORT_QOS_MAP_FIELD, "[" + CFG_DOT1P_TO_TC_MAP_TABLE_NAME + "|" + CFG_DOT1P_TO_TC_MAP_KEY + "]")]) - ports = swsscommon.Table(self.config_db, CFG_PORT_TABLE_NAME).getKeys() + tbl = swss.Table(self.config_db, swss.CFG_PORT_QOS_MAP_TABLE_NAME) + fvs = swss.FieldValuePairs([(CFG_PORT_QOS_MAP_FIELD, "[" + swss.CFG_DOT1P_TO_TC_MAP_TABLE_NAME + "|" + CFG_DOT1P_TO_TC_MAP_KEY + "]")]) + ports = swss.Table(self.config_db, swss.CFG_PORT_TABLE_NAME).getKeys() for port in ports: tbl.set(port, fvs) @@ -76,7 +72,7 @@ def test_dot1p_cfg(self, dvs): self.create_dot1p_profile() oid, 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']) @@ -91,7 +87,7 @@ def test_port_dot1p(self, dvs): self.apply_dot1p_profile_on_all_ports() cnt = 0 - tbl = swsscommon.Table(self.asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + tbl = swss.Table(self.asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") keys = tbl.getKeys() for key in keys: (status, fvs) = tbl.get(key) @@ -102,9 +98,121 @@ def test_port_dot1p(self, dvs): cnt += 1 assert fv[1] == oid - port_cnt = len(swsscommon.Table(self.config_db, CFG_PORT_TABLE_NAME).getKeys()) + port_cnt = len(swss.Table(self.config_db, swss.CFG_PORT_TABLE_NAME).getKeys()) assert port_cnt == cnt + def test_cbf(self, dvs): + def get_qos_id(): + nonlocal asic_db + nonlocal asic_qos_map_ids + + dscp_map_id = None + for qos_id in asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP"): + if qos_id not in asic_qos_map_ids: + dscp_map_id = qos_id + break + return dscp_map_id + + cfg_db = dvs.get_config_db() + asic_db = dvs.get_asic_db() + + dscp_ps = swss.Table(cfg_db.db_connection, swss.CFG_DSCP_TO_FC_MAP_TABLE_NAME) + exp_ps = swss.Table(cfg_db.db_connection, swss.CFG_EXP_TO_FC_MAP_TABLE_NAME) + + asic_qos_map_ids = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP") + asic_qos_map_count = len(asic_qos_map_ids) + + # Create a DSCP_TO_FC map + dscp_map = [(str(i), str(i)) for i in range(0, 64)] + dscp_ps.set("AZURE", swss.FieldValuePairs(dscp_map)) + + asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP", asic_qos_map_count + 1) + + # Get the DSCP map ID + dscp_map_id = get_qos_id() + assert(dscp_map_id is not None) + + # Assert the expected values + fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP", dscp_map_id) + assert(fvs.get("SAI_QOS_MAP_ATTR_TYPE") == "SAI_QOS_MAP_TYPE_DSCP_TO_FORWARDING_CLASS") + + # Delete the map + dscp_ps._del("AZURE") + asic_db.wait_for_deleted_entry("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP", dscp_map_id) + + # Create a map with invalid DSCP values + dscp_map = [(str(i), str(i)) for i in range(0, 65)] + dscp_ps.set('AZURE', swss.FieldValuePairs(dscp_map)) + time.sleep(1) + assert(asic_qos_map_count == len(asic_db.get_keys('ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP'))) + + # Delete the map + dscp_ps._del("AZURE") + + # Delete a map that does not exist. Nothing should happen + dscp_ps._del("AZURE") + time.sleep(1) + assert(len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP")) == asic_qos_map_count) + + # Create a EXP_TO_FC map + exp_map = [(str(i), str(i)) for i in range(0, 8)] + exp_ps.set("AZURE", swss.FieldValuePairs(exp_map)) + + asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP", asic_qos_map_count + 1) + + # Get the EXP map ID + exp_map_id = get_qos_id() + + # Assert the expected values + fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP", exp_map_id) + assert(fvs.get("SAI_QOS_MAP_ATTR_TYPE") == "SAI_QOS_MAP_TYPE_MPLS_EXP_TO_FORWARDING_CLASS") + + # Delete the map + exp_ps._del("AZURE") + asic_db.wait_for_deleted_entry("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP", exp_map_id) + + # Create a map with invalid EXP values + exp_map = [(str(i), str(i)) for i in range(1, 9)] + exp_ps.set('AZURE', swss.FieldValuePairs(exp_map)) + time.sleep(1) + assert(asic_qos_map_count == len(asic_db.get_keys('ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP'))) + + # Delete the map + exp_ps._del("AZURE") + + # Update the map with valid EXP values but invalid FC + exp_map = [(str(i), '-1') for i in range(0, 8)] + exp_ps.set('AZURE', swss.FieldValuePairs(exp_map)) + time.sleep(1) + assert(asic_qos_map_count == len(asic_db.get_keys('ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP'))) + + # Delete the map + exp_ps._del("AZURE") + + # Update the map with bigger than unsigned char FC values + exp_map = [(str(i), '256') for i in range(0, 8)] + exp_ps.set('AZURE', swss.FieldValuePairs(exp_map)) + time.sleep(1) + assert(asic_qos_map_count == len(asic_db.get_keys('ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP'))) + + # Delete the map + exp_ps._del("AZURE") + + # Update the map with valid values + exp_map = [(str(i), str(i + 10)) for i in range(0, 8)] + exp_ps.set('AZURE', swss.FieldValuePairs(exp_map)) + asic_db.wait_for_n_keys('ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP', asic_qos_map_count + 1) + + # Delete the map + exp_map_id = get_qos_id() + exp_ps._del("AZURE") + asic_db.wait_for_deleted_entry("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP", exp_map_id) + + # Delete a map that does not exist. Nothing should happen + exp_ps._del("AZURE") + time.sleep(1) + assert(len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP")) == asic_qos_map_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