diff --git a/cfgmgr/coppmgr.cpp b/cfgmgr/coppmgr.cpp index 834b2c5ff0b2..1721cc8593f8 100644 --- a/cfgmgr/coppmgr.cpp +++ b/cfgmgr/coppmgr.cpp @@ -78,31 +78,42 @@ bool CoppMgr::checkTrapGroupPending(string trap_group_name) /* Feature name and CoPP Trap table name must match */ void CoppMgr::setFeatureTrapIdsStatus(string feature, bool enable) { - bool disabled_trap = (m_coppDisabledTraps.find(feature) != m_coppDisabledTraps.end()); - - if ((enable && !disabled_trap) || (!enable && disabled_trap)) + bool disabled_trap {true}; + string always_enabled; + if (m_coppTrapConfMap.find(feature) != m_coppTrapConfMap.end()) { - return; + always_enabled = m_coppTrapConfMap[feature].is_always_enabled; + } + if (always_enabled == "true" || isFeatureEnabled(feature)) + { + disabled_trap = false; } - if (m_coppTrapConfMap.find(feature) == m_coppTrapConfMap.end()) + if ((enable && !disabled_trap) || (!enable && disabled_trap)) { - if (!enable) - { - m_coppDisabledTraps.insert(feature); - } return; } + string trap_group = m_coppTrapConfMap[feature].trap_group; bool prev_group_state = checkTrapGroupPending(trap_group); - if (!enable) + // update features cache + auto state = "disabled"; + if (enable) { - m_coppDisabledTraps.insert(feature); + state = "enabled"; } - else + if (m_featuresCfgTable.find(feature) != m_featuresCfgTable.end()) { - m_coppDisabledTraps.erase(feature); + auto vect = m_featuresCfgTable[feature]; + for (long unsigned int i=0; i < vect.size(); i++) + { + if (vect[i].first == "state") + { + vect[i].second = state; + } + } + m_featuresCfgTable.at(feature) = vect; } /* Trap group moved to pending state when feature is disabled. Remove trap group @@ -140,25 +151,46 @@ void CoppMgr::setFeatureTrapIdsStatus(string feature, bool enable) } } -bool CoppMgr::isTrapIdDisabled(string trap_id) +bool CoppMgr::isFeatureEnabled(std::string feature) { - for (auto &m: m_coppDisabledTraps) + if (m_featuresCfgTable.find(feature) != m_featuresCfgTable.end()) { - if (m_coppTrapConfMap.find(m) == m_coppTrapConfMap.end()) + std::vector feature_fvs = m_featuresCfgTable[feature]; + for (auto i: feature_fvs) { - continue; + if (fvField(i) == "state" && (fvValue(i) == "enabled" || fvValue(i) == "always_enabled")) + { + return true; + } } - vector trap_id_list; + } + return false; +} - trap_id_list = tokenize(m_coppTrapConfMap[m].trap_ids, list_item_delimiter); - if(std::find(trap_id_list.begin(), trap_id_list.end(), trap_id) != trap_id_list.end()) +bool CoppMgr::isTrapIdDisabled(string trap_id) +{ + // check if trap is always_enabled + string trap_name; + for (auto &t: m_coppTrapConfMap) + { + if (m_coppTrapConfMap[t.first].trap_ids.find(trap_id) != string::npos) { - return true; + trap_name = t.first; + if (m_coppTrapConfMap[t.first].is_always_enabled == "true") + { + return false; + } + break; } + } + if (isFeatureEnabled(trap_name)) + { + return false; } - return false; + return true; } + void CoppMgr::mergeConfig(CoppCfg &init_cfg, CoppCfg &m_cfg, std::vector &cfg_keys, Table &cfgTable) { /* Read the init configuration first. If the same key is present in @@ -254,14 +286,7 @@ CoppMgr::CoppMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c { std::vector feature_fvs; m_cfgFeatureTable.get(i, feature_fvs); - - for (auto j: feature_fvs) - { - if (fvField(j) == "state" && fvValue(j) == "disabled") - { - m_coppDisabledTraps.insert(i); - } - } + m_featuresCfgTable.emplace(i, feature_fvs); } mergeConfig(m_coppTrapInitCfg, trap_cfg, trap_cfg_keys, m_cfgCoppTrapTable); @@ -270,6 +295,7 @@ CoppMgr::CoppMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c { string trap_group; string trap_ids; + string is_always_enabled = "false"; std::vector trap_fvs = i.second; for (auto j: trap_fvs) @@ -282,13 +308,22 @@ CoppMgr::CoppMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c { trap_group = fvValue(j); } + else if (fvField(j) == COPP_ALWAYS_ENABLED_FIELD) + { + is_always_enabled = fvValue(j); + } } + if (!trap_group.empty() && !trap_ids.empty()) { addTrapIdsToTrapGroup(trap_group, trap_ids); m_coppTrapConfMap[i.first].trap_group = trap_group; m_coppTrapConfMap[i.first].trap_ids = trap_ids; - setCoppTrapStateOk(i.first); + m_coppTrapConfMap[i.first].is_always_enabled = is_always_enabled; + if (is_always_enabled == "true" || isFeatureEnabled(i.first)) + { + setCoppTrapStateOk(i.first); + } } } @@ -384,7 +419,6 @@ void CoppMgr::removeTrapIdsFromTrapGroup(string trap_group, string trap_ids) void CoppMgr::getTrapGroupTrapIds(string trap_group, string &trap_ids) { - trap_ids.clear(); for (auto it: m_coppTrapIdTrapGroupMap) { @@ -406,6 +440,36 @@ void CoppMgr::getTrapGroupTrapIds(string trap_group, string &trap_ids) } } +void CoppMgr::removeTrap(string key) +{ + string trap_ids; + std::vector fvs; + removeTrapIdsFromTrapGroup(m_coppTrapConfMap[key].trap_group, m_coppTrapConfMap[key].trap_ids); + getTrapGroupTrapIds(m_coppTrapConfMap[key].trap_group, trap_ids); + FieldValueTuple fv(COPP_TRAP_ID_LIST_FIELD, trap_ids); + fvs.push_back(fv); + if (!checkTrapGroupPending(m_coppTrapConfMap[key].trap_group)) + { + m_appCoppTable.set(m_coppTrapConfMap[key].trap_group, fvs); + setCoppGroupStateOk(m_coppTrapConfMap[key].trap_group); + } +} + +void CoppMgr::addTrap(string trap_ids, string trap_group) +{ + string trap_group_trap_ids; + std::vector fvs; + addTrapIdsToTrapGroup(trap_group, trap_ids); + getTrapGroupTrapIds(trap_group, trap_group_trap_ids); + FieldValueTuple fv1(COPP_TRAP_ID_LIST_FIELD, trap_group_trap_ids); + fvs.push_back(fv1); + if (!checkTrapGroupPending(trap_group)) + { + m_appCoppTable.set(trap_group, fvs); + setCoppGroupStateOk(trap_group); + } +} + void CoppMgr::doCoppTrapTask(Consumer &consumer) { auto it = consumer.m_toSync.begin(); @@ -418,12 +482,21 @@ void CoppMgr::doCoppTrapTask(Consumer &consumer) vector fvs; string trap_ids = ""; string trap_group = ""; + string is_always_enabled = ""; bool conf_present = false; if (m_coppTrapConfMap.find(key) != m_coppTrapConfMap.end()) { trap_ids = m_coppTrapConfMap[key].trap_ids; trap_group = m_coppTrapConfMap[key].trap_group; + if (m_coppTrapConfMap[key].is_always_enabled.empty()) + { + is_always_enabled = "false"; + } + else + { + is_always_enabled = m_coppTrapConfMap[key].is_always_enabled; + } conf_present = true; } @@ -441,6 +514,10 @@ void CoppMgr::doCoppTrapTask(Consumer &consumer) { trap_ids = fvValue(i); } + else if (fvField(i) == COPP_ALWAYS_ENABLED_FIELD) + { + is_always_enabled = fvValue(i); + } else if (fvField(i) == "NULL") { null_cfg = true; @@ -450,20 +527,9 @@ void CoppMgr::doCoppTrapTask(Consumer &consumer) { if (conf_present) { - SWSS_LOG_DEBUG("Deleting trap key %s", key.c_str()); - removeTrapIdsFromTrapGroup(m_coppTrapConfMap[key].trap_group, - m_coppTrapConfMap[key].trap_ids); - trap_ids.clear(); + removeTrap(key); setCoppTrapStateOk(key); - getTrapGroupTrapIds(m_coppTrapConfMap[key].trap_group, trap_ids); - fvs.clear(); - FieldValueTuple fv(COPP_TRAP_ID_LIST_FIELD, trap_ids); - fvs.push_back(fv); - if (!checkTrapGroupPending(m_coppTrapConfMap[key].trap_group)) - { - m_appCoppTable.set(m_coppTrapConfMap[key].trap_group, fvs); - setCoppGroupStateOk(m_coppTrapConfMap[key].trap_group); - } + m_coppTrapConfMap.erase(key); } it = consumer.m_toSync.erase(it); @@ -472,38 +538,126 @@ void CoppMgr::doCoppTrapTask(Consumer &consumer) /*Duplicate check*/ if (conf_present && (trap_group == m_coppTrapConfMap[key].trap_group) && - (trap_ids == m_coppTrapConfMap[key].trap_ids)) + (trap_ids == m_coppTrapConfMap[key].trap_ids) && + (is_always_enabled == m_coppTrapConfMap[key].is_always_enabled)) { it = consumer.m_toSync.erase(it); continue; } + /* Incomplete configuration. Do not process until both trap group * and trap_ids are available */ if (trap_group.empty() || trap_ids.empty()) { + if (is_always_enabled.empty()) + { + it = consumer.m_toSync.erase(it); + continue; + } + + if (is_always_enabled != m_coppTrapConfMap[key].is_always_enabled) + { + m_coppTrapConfMap[key].is_always_enabled = is_always_enabled; + if (is_always_enabled == "true") + { + if (m_coppTrapConfMap.find(key) != m_coppTrapConfMap.end()) + { + addTrap(m_coppTrapConfMap[key].trap_ids, m_coppTrapConfMap[key].trap_group); + } + // else if it has info in the init cfg map + else if (m_coppTrapInitCfg.find(key) != m_coppTrapInitCfg.end()) + { + auto fvs = m_coppTrapInitCfg[key]; + string init_trap_ids = ""; + string init_trap_group = ""; + for (auto i: fvs) + { + if (fvField(i) == COPP_TRAP_GROUP_FIELD) + { + init_trap_group = fvValue(i); + } + else if (fvField(i) == COPP_TRAP_ID_LIST_FIELD) + { + init_trap_ids = fvValue(i); + } + } + addTrap(init_trap_ids, init_trap_group); + } + } + else + { + /* if the value was changed from true to false, + check if there is a feature enabled. + if no, remove the trap. is yes, do nothing. */ + + m_coppTrapConfMap[key].is_always_enabled = is_always_enabled; + if (isFeatureEnabled(key)) + { + it = consumer.m_toSync.erase(it); + continue; + } + + removeTrap(key); + delCoppTrapStateOk(key); + } + it = consumer.m_toSync.erase(it); + continue; + } + } + /* if always_enabled field has been changed */ + if (conf_present && + (trap_group == m_coppTrapConfMap[key].trap_group) && + (trap_ids == m_coppTrapConfMap[key].trap_ids)) + { + m_coppTrapConfMap[key].is_always_enabled = is_always_enabled; + if (is_always_enabled == "true") + { + /* if the value was changed from false to true, + if the trap is not installed, install it. + otherwise, do nothing. */ + + // if the feature was not enabled, install the trap + if (!isFeatureEnabled(key)) + { + addTrap(trap_ids, trap_group); + } + + it = consumer.m_toSync.erase(it); + continue; + } + else + { + /* if the value was changed from true to false, + check if there is a feature enabled. + if no, remove the trap. is yes, do nothing. */ + + if (isFeatureEnabled(key)) + { + it = consumer.m_toSync.erase(it); + continue; + } + + removeTrap(key); + delCoppTrapStateOk(key); + } it = consumer.m_toSync.erase(it); continue; } + /* Remove the current trap IDs and add the new trap IDS to recompute the - * trap IDs for the trap group + * trap IDs for the trap group */ if (conf_present) { removeTrapIdsFromTrapGroup(m_coppTrapConfMap[key].trap_group, m_coppTrapConfMap[key].trap_ids); } - fvs.clear(); - string trap_group_trap_ids; - addTrapIdsToTrapGroup(trap_group, trap_ids); - getTrapGroupTrapIds(trap_group, trap_group_trap_ids); - FieldValueTuple fv1(COPP_TRAP_ID_LIST_FIELD, trap_group_trap_ids); - fvs.push_back(fv1); - if (!checkTrapGroupPending(trap_group)) - { - m_appCoppTable.set(trap_group, fvs); - setCoppGroupStateOk(trap_group); - } + + m_coppTrapConfMap[key].trap_group = trap_group; + m_coppTrapConfMap[key].trap_ids = trap_ids; + m_coppTrapConfMap[key].is_always_enabled = is_always_enabled; + addTrap(trap_ids, trap_group); /* When the trap table's trap group is changed, the old trap group * should also be reprogrammed as some of its associated traps got @@ -511,7 +665,7 @@ void CoppMgr::doCoppTrapTask(Consumer &consumer) */ if (conf_present && (trap_group != m_coppTrapConfMap[key].trap_group)) { - trap_group_trap_ids.clear(); + string trap_group_trap_ids; fvs.clear(); getTrapGroupTrapIds(m_coppTrapConfMap[key].trap_group, trap_group_trap_ids); FieldValueTuple fv2(COPP_TRAP_ID_LIST_FIELD, trap_group_trap_ids); @@ -524,6 +678,7 @@ void CoppMgr::doCoppTrapTask(Consumer &consumer) } m_coppTrapConfMap[key].trap_group = trap_group; m_coppTrapConfMap[key].trap_ids = trap_ids; + m_coppTrapConfMap[key].is_always_enabled = is_always_enabled; setCoppTrapStateOk(key); } else if (op == DEL_COMMAND) @@ -546,8 +701,9 @@ void CoppMgr::doCoppTrapTask(Consumer &consumer) setCoppGroupStateOk(m_coppTrapConfMap[key].trap_group); } } - if (conf_present) + if (conf_present && !m_coppTrapConfMap[key].trap_group.empty() && !m_coppTrapConfMap[key].trap_ids.empty()) { + m_coppTrapConfMap.erase(key); } delCoppTrapStateOk(key); @@ -559,6 +715,7 @@ void CoppMgr::doCoppTrapTask(Consumer &consumer) if (m_coppTrapInitCfg.find(key) != m_coppTrapInitCfg.end()) { auto fvs = m_coppTrapInitCfg[key]; + is_always_enabled.clear(); for (auto i: fvs) { if (fvField(i) == COPP_TRAP_GROUP_FIELD) @@ -569,21 +726,24 @@ void CoppMgr::doCoppTrapTask(Consumer &consumer) { trap_ids = fvValue(i); } + else if (fvField(i) == COPP_ALWAYS_ENABLED_FIELD) + { + is_always_enabled = fvValue(i); + } } - vector g_fvs; - string trap_group_trap_ids; - addTrapIdsToTrapGroup(trap_group, trap_ids); - getTrapGroupTrapIds(trap_group, trap_group_trap_ids); - FieldValueTuple fv1(COPP_TRAP_ID_LIST_FIELD, trap_group_trap_ids); - g_fvs.push_back(fv1); - if (!checkTrapGroupPending(trap_group)) + if (is_always_enabled.empty()) { - m_appCoppTable.set(trap_group, g_fvs); - setCoppGroupStateOk(trap_group); + is_always_enabled = "false"; } + m_coppTrapConfMap[key].trap_group = trap_group; m_coppTrapConfMap[key].trap_ids = trap_ids; - setCoppTrapStateOk(key); + m_coppTrapConfMap[key].is_always_enabled = is_always_enabled; + if (is_always_enabled == "true" || isFeatureEnabled(key)) + { + addTrap(trap_ids, trap_group); + setCoppTrapStateOk(key); + } } } it = consumer.m_toSync.erase(it); @@ -706,6 +866,7 @@ void CoppMgr::doCoppGroupTask(Consumer &consumer) } } + void CoppMgr::doFeatureTask(Consumer &consumer) { auto it = consumer.m_toSync.begin(); @@ -715,17 +876,20 @@ void CoppMgr::doFeatureTask(Consumer &consumer) string key = kfvKey(t); string op = kfvOp(t); - vector fvs; string trap_ids; if (op == SET_COMMAND) { + if (m_featuresCfgTable.find(key) == m_featuresCfgTable.end()) + { + m_featuresCfgTable.emplace(key, kfvFieldsValues(t)); + } for (auto i : kfvFieldsValues(t)) { if (fvField(i) == "state") { bool status = false; - if (fvValue(i) == "enabled") + if (fvValue(i) == "enabled" || fvValue(i) == "always_enabled") { status = true; } diff --git a/cfgmgr/coppmgr.h b/cfgmgr/coppmgr.h index b010489f2eee..1d53756fceba 100644 --- a/cfgmgr/coppmgr.h +++ b/cfgmgr/coppmgr.h @@ -14,6 +14,7 @@ namespace swss { /* COPP Trap Table Fields */ #define COPP_TRAP_ID_LIST_FIELD "trap_ids" #define COPP_TRAP_GROUP_FIELD "trap_group" +#define COPP_ALWAYS_ENABLED_FIELD "always_enabled" /* COPP Group Table Fields */ #define COPP_GROUP_QUEUE_FIELD "queue" @@ -42,6 +43,7 @@ struct CoppTrapConf { std::string trap_ids; std::string trap_group; + std::string is_always_enabled; }; /* TrapName to TrapConf map */ @@ -70,10 +72,10 @@ class CoppMgr : public Orch CoppTrapConfMap m_coppTrapConfMap; CoppTrapIdTrapGroupMap m_coppTrapIdTrapGroupMap; CoppGroupFvs m_coppGroupFvs; - std::set m_coppDisabledTraps; CoppCfg m_coppGroupInitCfg; CoppCfg m_coppTrapInitCfg; - + CoppCfg m_featuresCfgTable; + void doTask(Consumer &consumer); void doCoppGroupTask(Consumer &consumer); @@ -96,8 +98,13 @@ class CoppMgr : public Orch std::vector &modified_fvs); void parseInitFile(void); bool isTrapGroupInstalled(std::string key); + bool isFeatureEnabled(std::string feature); void mergeConfig(CoppCfg &init_cfg, CoppCfg &m_cfg, std::vector &cfg_keys, Table &cfgTable); + void removeTrap(std::string key); + void addTrap(std::string trap_ids, std::string trap_group); + + }; } diff --git a/tests/test_copp.py b/tests/test_copp.py index 19faac954f79..5885a489b52e 100644 --- a/tests/test_copp.py +++ b/tests/test_copp.py @@ -151,17 +151,18 @@ "trap_action": "trap", "trap_priority": "5" } + copp_trap = { - "bgp,bgpv6": copp_group_queue4_group1, - "lacp": copp_group_queue4_group1, - "arp_req,arp_resp,neigh_discovery":copp_group_queue4_group2, - "lldp":copp_group_queue4_group3, - "dhcp,dhcpv6":copp_group_queue4_group3, - "udld":copp_group_queue4_group3, - "ip2me":copp_group_queue1_group1, - "src_nat_miss,dest_nat_miss": copp_group_queue1_group2, - "sample_packet": copp_group_queue2_group1, - "ttl_error": copp_group_default + "bgp": ["bgp;bgpv6", copp_group_queue4_group1], + "lacp": ["lacp", copp_group_queue4_group1, "always_enabled"], + "arp": ["arp_req;arp_resp;neigh_discovery", copp_group_queue4_group2, "always_enabled"], + "lldp": ["lldp", copp_group_queue4_group3], + "dhcp": ["dhcp;dhcpv6", copp_group_queue4_group3], + "udld": ["udld", copp_group_queue4_group3, "always_enabled"], + "ip2me": ["ip2me", copp_group_queue1_group1, "always_enabled"], + "nat": ["src_nat_miss;dest_nat_miss", copp_group_queue1_group2], + "sflow": ["sample_packet", copp_group_queue2_group1], + "ttl": ["ttl_error", copp_group_default] } disabled_traps = ["sample_packet"] @@ -201,7 +202,7 @@ def setup_copp(self, dvs): self.trap_ctbl = swsscommon.Table(self.cdb, "COPP_TRAP") self.trap_group_ctbl = swsscommon.Table(self.cdb, "COPP_GROUP") self.feature_tbl = swsscommon.Table(self.cdb, "FEATURE") - fvs = swsscommon.FieldValuePairs([("state", "disbled")]) + fvs = swsscommon.FieldValuePairs([("state", "disabled")]) self.feature_tbl.set("sflow", fvs) time.sleep(2) @@ -306,8 +307,12 @@ def test_defaults(self, dvs, testlog): self.setup_copp(dvs) trap_keys = self.trap_atbl.getKeys() for traps in copp_trap: - trap_ids = traps.split(",") - trap_group = copp_trap[traps] + trap_info = copp_trap[traps] + trap_ids = trap_info[0].split(";") + trap_group = trap_info[1] + always_enabled = False + if len(trap_info) > 2: + always_enabled = True for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -325,6 +330,7 @@ def test_defaults(self, dvs, testlog): if trap_id not in disabled_traps: assert trap_found == True + def test_restricted_trap_sflow(self, dvs, testlog): self.setup_copp(dvs) fvs = swsscommon.FieldValuePairs([("state", "enabled")]) @@ -334,10 +340,14 @@ def test_restricted_trap_sflow(self, dvs, testlog): trap_keys = self.trap_atbl.getKeys() for traps in copp_trap: - trap_ids = traps.split(",") + trap_info = copp_trap[traps] + trap_ids = trap_info[0].split(";") + trap_group = trap_info[1] + always_enabled = False + if len(trap_info) > 2: + always_enabled = True if "sample_packet" not in trap_ids: continue - trap_group = copp_trap[traps] trap_found = False trap_type = traps_to_trap_type["sample_packet"] for key in trap_keys: @@ -363,10 +373,14 @@ def test_policer_set(self, dvs, testlog): trap_keys = self.trap_atbl.getKeys() for traps in copp_trap: - if copp_trap[traps] != copp_group_queue4_group2: + trap_info = copp_trap[traps] + trap_ids = trap_info[0].split(";") + trap_group = trap_info[1] + always_enabled = False + if len(trap_info) > 2: + always_enabled = True + if trap_group != copp_group_queue4_group2: continue - trap_ids = traps.split(",") - trap_group = copp_trap[traps] for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -390,12 +404,19 @@ def test_trap_group_set(self, dvs, testlog): traps = "bgp,bgpv6" fvs = swsscommon.FieldValuePairs([("trap_group", "queue1_group1")]) self.trap_ctbl.set("bgp", fvs) - copp_trap[traps] = copp_group_queue1_group1 + + for c_trap in copp_trap: + trap_info = copp_trap[c_trap] + ids = trap_info[0].replace(';', ',') + if traps == ids: + break + + trap_info[1] = copp_group_queue1_group1 time.sleep(2) trap_keys = self.trap_atbl.getKeys() trap_ids = traps.split(",") - trap_group = copp_trap[traps] + trap_group = trap_info[1] for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -423,8 +444,14 @@ def test_trap_ids_set(self, dvs, testlog): old_traps = "bgp,bgpv6" trap_keys = self.trap_atbl.getKeys() + for c_trap in copp_trap: + trap_info = copp_trap[c_trap] + ids = trap_info[0].replace(';', ',') + if old_traps == ids: + break + trap_ids = old_traps.split(",") - trap_group = copp_trap[old_traps] + trap_group = trap_info[1] for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -451,7 +478,7 @@ def test_trap_ids_set(self, dvs, testlog): trap_keys = self.trap_atbl.getKeys() trap_ids = traps.split(",") - trap_group = copp_trap[traps] + trap_group = trap_info[1] for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -478,10 +505,11 @@ def test_trap_action_set(self, dvs, testlog): trap_keys = self.trap_atbl.getKeys() for traps in copp_trap: - if copp_trap[traps] != copp_group_queue4_group1: + trap_info = copp_trap[traps] + if trap_info[1] != copp_group_queue4_group1: continue - trap_ids = traps.split(",") - trap_group = copp_trap[traps] + trap_ids = trap_info[0].split(";") + trap_group = trap_info[1] for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -499,18 +527,21 @@ def test_trap_action_set(self, dvs, testlog): if trap_id not in disabled_traps: assert trap_found == True + def test_new_trap_add(self, dvs, testlog): self.setup_copp(dvs) global copp_trap traps = "eapol,isis,bfd_micro,bfdv6_micro,ldp" - fvs = swsscommon.FieldValuePairs([("trap_group", "queue1_group2"),("trap_ids", traps)]) + fvs = swsscommon.FieldValuePairs([("trap_group", "queue1_group2"),("trap_ids", traps),("always_enabled", "true")]) self.trap_ctbl.set(traps, fvs) - copp_trap[traps] = copp_group_queue1_group2 + + + copp_trap["eapol"] = [traps, copp_group_queue1_group2, "always_enabled"] time.sleep(2) trap_keys = self.trap_atbl.getKeys() trap_ids = traps.split(",") - trap_group = copp_trap[traps] + trap_group = copp_group_queue1_group2 for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -534,13 +565,19 @@ def test_new_trap_del(self, dvs, testlog): traps = "eapol,isis,bfd_micro,bfdv6_micro,ldp" fvs = swsscommon.FieldValuePairs([("trap_group", "queue1_group2"),("trap_ids", traps)]) self.trap_ctbl.set(traps, fvs) - copp_trap[traps] = copp_group_queue1_group2 + for c_trap in copp_trap: + trap_info = copp_trap[c_trap] + ids = trap_info[0].replace(';', ',') + if traps == ids: + break + + trap_info[1] = copp_group_queue1_group2 time.sleep(2) self.trap_ctbl._del(traps) time.sleep(2) trap_ids = traps.split(",") - trap_group = copp_trap[traps] + trap_group = trap_info[1] trap_keys = self.trap_atbl.getKeys() for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] @@ -568,14 +605,19 @@ def test_new_trap_group_add(self, dvs, testlog): fvs = swsscommon.FieldValuePairs(list_val) self.trap_group_ctbl.set("queue5_group1", fvs) traps = "igmp_v1_report" - t_fvs = swsscommon.FieldValuePairs([("trap_group", "queue5_group1"),("trap_ids", "igmp_v1_report")]) + t_fvs = swsscommon.FieldValuePairs([("trap_group", "queue5_group1"),("trap_ids", "igmp_v1_report"),("always_enabled", "true")]) self.trap_ctbl.set(traps, t_fvs) - copp_trap[traps] = copp_group_queue5_group1 + for c_trap in copp_trap: + trap_info = copp_trap[c_trap] + ids = trap_info[0].replace(';', ',') + if traps == ids: + break + trap_info[1] = copp_group_queue5_group1 time.sleep(2) trap_keys = self.trap_atbl.getKeys() trap_ids = traps.split(",") - trap_group = copp_trap[traps] + trap_group = trap_info[1] for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -602,16 +644,21 @@ def test_new_trap_group_del(self, dvs, testlog): fvs = swsscommon.FieldValuePairs(list_val) self.trap_group_ctbl.set("queue5_group1", fvs) traps = "igmp_v1_report" - t_fvs = swsscommon.FieldValuePairs([("trap_group", "queue5_group1"),("trap_ids", "igmp_v1_report")]) + t_fvs = swsscommon.FieldValuePairs([("trap_group", "queue5_group1"),("trap_ids", "igmp_v1_report"),("always_enabled", "true")]) self.trap_ctbl.set(traps, t_fvs) - copp_trap[traps] = copp_group_queue5_group1 + for c_trap in copp_trap: + trap_info = copp_trap[c_trap] + ids = trap_info[0].replace(';', ',') + if traps == ids: + break + trap_info[1] = copp_group_queue5_group1 self.trap_group_ctbl._del("queue5_group1") time.sleep(2) trap_keys = self.trap_atbl.getKeys() trap_ids = traps.split(",") - trap_group = copp_trap[traps] + trap_group = trap_info[1] for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -643,10 +690,11 @@ def test_override_trap_grp_cfg_del (self, dvs, testlog): trap_keys = self.trap_atbl.getKeys() for traps in copp_trap: - if copp_trap[traps] != copp_group_queue1_group1: + trap_info = copp_trap[traps] + if trap_info[1] != copp_group_queue1_group1: continue - trap_ids = traps.split(",") - trap_group = copp_trap[traps] + trap_ids = trap_info[0].split(";") + trap_group = trap_info[1] for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -675,7 +723,7 @@ def test_override_trap_cfg_del(self, dvs, testlog): self.trap_ctbl._del("ip2me") time.sleep(2) trap_ids = traps.split(",") - trap_group = copp_trap["ip2me"] + trap_group = copp_trap["ip2me"][1] trap_keys = self.trap_atbl.getKeys() for trap_id in trap_ids: trap_type = traps_to_trap_type[trap_id] @@ -705,7 +753,7 @@ def test_empty_trap_cfg(self, dvs, testlog): time.sleep(2) trap_id = "ip2me" - trap_group = copp_trap["ip2me"] + trap_group = copp_trap["ip2me"][1] trap_keys = self.trap_atbl.getKeys() trap_type = traps_to_trap_type[trap_id] trap_found = False @@ -740,3 +788,56 @@ def test_empty_trap_cfg(self, dvs, testlog): self.validate_trap_group(key,trap_group) break assert trap_found == True + + + def test_disabled_feature_always_enabled_trap(self, dvs, testlog): + self.setup_copp(dvs) + fvs = swsscommon.FieldValuePairs([("trap_ids", "lldp"), ("trap_group", "queue4_group3"), ("always_enabled", "true")]) + self.trap_ctbl.set("lldp", fvs) + fvs = swsscommon.FieldValuePairs([("state", "disabled")]) + self.feature_tbl.set("lldp", fvs) + + time.sleep(2) + global copp_trap + + trap_keys = self.trap_atbl.getKeys() + for traps in copp_trap: + trap_info = copp_trap[traps] + trap_ids = trap_info[0].split(";") + trap_group = trap_info[1] + + if "lldp" not in trap_ids: + continue + + trap_found = False + trap_type = traps_to_trap_type["lldp"] + for key in trap_keys: + (status, fvs) = self.trap_atbl.get(key) + assert status == True + for fv in fvs: + if fv[0] == "SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE": + if fv[1] == trap_type: + trap_found = True + if trap_found: + self.validate_trap_group(key,trap_group) + break + assert trap_found == True + + # change always_enabled to be false and check the trap is not installed: + fvs = swsscommon.FieldValuePairs([("trap_ids", "lldp"), ("trap_group", "queue4_group3"), ("always_enabled", "false")]) + self.trap_ctbl.set("lldp", fvs) + time.sleep(2) + + table_found = True + for key in trap_keys: + (status, fvs) = self.trap_atbl.get(key) + if status == False: + table_found = False + + # teardown + fvs = swsscommon.FieldValuePairs([("trap_ids", "lldp"), ("trap_group", "queue4_group3")]) + self.trap_ctbl.set("lldp", fvs) + fvs = swsscommon.FieldValuePairs([("state", "enabled")]) + self.feature_tbl.set("lldp", fvs) + + assert table_found == False