From 45ee52a84debc0cf5b0dea46d85f62bb9339dddf Mon Sep 17 00:00:00 2001 From: Ravi Minnikanti Date: Tue, 15 Apr 2025 18:09:04 +0530 Subject: [PATCH 1/3] [orchagent] CoPP neighbor miss trap and enhancements What I did * Added neighbor_miss trap type support * enum capability query for hostif trap type * Added trap hw_status field to state_db * Added UT and mock tests HLD: sonic-net/SONiC#1943 Signed-off-by: Ravi Minnikanti --- orchagent/copporch.cpp | 183 +++++++++++++++++++++++++++++-- orchagent/copporch.h | 17 ++- orchagent/debugcounterorch.cpp | 2 +- tests/mock_tests/copp_cfg.json | 15 +++ tests/mock_tests/copporch_ut.cpp | 16 ++- tests/mock_tests/portal.h | 5 + tests/test_copp.py | 51 ++++++++- 7 files changed, 277 insertions(+), 12 deletions(-) diff --git a/orchagent/copporch.cpp b/orchagent/copporch.cpp index 3295d401087..05080c8ddcc 100644 --- a/orchagent/copporch.cpp +++ b/orchagent/copporch.cpp @@ -1,3 +1,9 @@ +extern "C" { +#include +#include +#include +} + #include "sai.h" #include "copporch.h" #include "portsorch.h" @@ -89,9 +95,60 @@ static map trap_id_map = { {"dest_nat_miss", SAI_HOSTIF_TRAP_TYPE_DNAT_MISS}, {"ldp", SAI_HOSTIF_TRAP_TYPE_LDP}, {"bfd_micro", SAI_HOSTIF_TRAP_TYPE_BFD_MICRO}, - {"bfdv6_micro", SAI_HOSTIF_TRAP_TYPE_BFDV6_MICRO} + {"bfdv6_micro", SAI_HOSTIF_TRAP_TYPE_BFDV6_MICRO}, + {"neighbor_miss", SAI_HOSTIF_TRAP_TYPE_NEIGHBOR_MISS} }; +/* + * List of default supported traps used as a fallback when the vendor SAI + * does not support capability query for the HOSTIF object. + */ +const vector default_supported_trap_ids = { + SAI_HOSTIF_TRAP_TYPE_STP, + SAI_HOSTIF_TRAP_TYPE_LACP, + SAI_HOSTIF_TRAP_TYPE_EAPOL, + SAI_HOSTIF_TRAP_TYPE_LLDP, + SAI_HOSTIF_TRAP_TYPE_PVRST, + SAI_HOSTIF_TRAP_TYPE_IGMP_TYPE_QUERY, + SAI_HOSTIF_TRAP_TYPE_IGMP_TYPE_LEAVE, + SAI_HOSTIF_TRAP_TYPE_IGMP_TYPE_V1_REPORT, + SAI_HOSTIF_TRAP_TYPE_IGMP_TYPE_V2_REPORT, + SAI_HOSTIF_TRAP_TYPE_IGMP_TYPE_V3_REPORT, + SAI_HOSTIF_TRAP_TYPE_SAMPLEPACKET, + SAI_HOSTIF_TRAP_TYPE_SWITCH_CUSTOM_RANGE_BASE, + SAI_HOSTIF_TRAP_TYPE_ARP_REQUEST, + SAI_HOSTIF_TRAP_TYPE_ARP_RESPONSE, + SAI_HOSTIF_TRAP_TYPE_DHCP, + SAI_HOSTIF_TRAP_TYPE_OSPF, + SAI_HOSTIF_TRAP_TYPE_PIM, + SAI_HOSTIF_TRAP_TYPE_VRRP, + SAI_HOSTIF_TRAP_TYPE_BGP, + SAI_HOSTIF_TRAP_TYPE_DHCPV6, + SAI_HOSTIF_TRAP_TYPE_OSPFV6, + SAI_HOSTIF_TRAP_TYPE_ISIS, + SAI_HOSTIF_TRAP_TYPE_VRRPV6, + SAI_HOSTIF_TRAP_TYPE_BGPV6, + SAI_HOSTIF_TRAP_TYPE_IPV6_NEIGHBOR_DISCOVERY, + SAI_HOSTIF_TRAP_TYPE_IPV6_MLD_V1_V2, + SAI_HOSTIF_TRAP_TYPE_IPV6_MLD_V1_REPORT, + SAI_HOSTIF_TRAP_TYPE_IPV6_MLD_V1_DONE, + SAI_HOSTIF_TRAP_TYPE_MLD_V2_REPORT, + SAI_HOSTIF_TRAP_TYPE_IP2ME, + SAI_HOSTIF_TRAP_TYPE_SSH, + SAI_HOSTIF_TRAP_TYPE_SNMP, + SAI_HOSTIF_TRAP_TYPE_ROUTER_CUSTOM_RANGE_BASE, + SAI_HOSTIF_TRAP_TYPE_L3_MTU_ERROR, + SAI_HOSTIF_TRAP_TYPE_TTL_ERROR, + SAI_HOSTIF_TRAP_TYPE_UDLD, + SAI_HOSTIF_TRAP_TYPE_BFD, + SAI_HOSTIF_TRAP_TYPE_BFDV6, + SAI_HOSTIF_TRAP_TYPE_SNAT_MISS, + SAI_HOSTIF_TRAP_TYPE_DNAT_MISS, + SAI_HOSTIF_TRAP_TYPE_LDP, + SAI_HOSTIF_TRAP_TYPE_BFD_MICRO, + SAI_HOSTIF_TRAP_TYPE_BFDV6_MICRO + /* This list is intended to remain static and should not be updated with new traps. */ +}; std::string get_trap_name_by_type(sai_hostif_trap_type_t trap_type) { @@ -104,7 +161,13 @@ std::string get_trap_name_by_type(sai_hostif_trap_type_t trap_type) } } - return trap_name_to_id_map.at(trap_type); + auto it = trap_name_to_id_map.find(trap_type); + if (it == trap_name_to_id_map.end()) + { + return ""; + } + + return it->second; } static map packet_action_map = { @@ -122,15 +185,19 @@ const string default_trap_group = "default"; const vector default_trap_ids = { SAI_HOSTIF_TRAP_TYPE_TTL_ERROR }; + const uint HOSTIF_TRAP_COUNTER_POLLING_INTERVAL_MS = 10000; CoppOrch::CoppOrch(DBConnector* db, string tableName) : Orch(db, tableName), m_counter_db(std::shared_ptr(new DBConnector("COUNTERS_DB", 0))), m_asic_db(std::shared_ptr(new DBConnector("ASIC_DB", 0))), + m_state_db(std::shared_ptr(new DBConnector("STATE_DB", 0))), m_counter_table(std::unique_ptr(new Table(m_counter_db.get(), COUNTERS_TRAP_NAME_MAP))), m_vidToRidTable(std::unique_ptr
(new Table(m_asic_db.get(), "VIDTORID"))), - m_trap_counter_manager(HOSTIF_TRAP_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, HOSTIF_TRAP_COUNTER_POLLING_INTERVAL_MS, false) + m_trap_counter_manager(HOSTIF_TRAP_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, HOSTIF_TRAP_COUNTER_POLLING_INTERVAL_MS, false), + m_trapCapabilityTable(std::unique_ptr
(new Table(m_state_db.get(), STATE_COPP_TRAP_CAPABILITY_TABLE_NAME))), + m_trapTable(std::unique_ptr
(new Table(m_state_db.get(), STATE_COPP_TRAP_TABLE_NAME))) { SWSS_LOG_ENTER(); auto intervT = timespec { .tv_sec = FLEX_COUNTER_UPD_INTERVAL , .tv_nsec = 0 }; @@ -138,11 +205,100 @@ CoppOrch::CoppOrch(DBConnector* db, string tableName) : auto executorT = new ExecutableTimer(m_FlexCounterUpdTimer, this, "FLEX_COUNTER_UPD_TIMER"); Orch::addExecutor(executorT); + /* Query SAI for supported trap IDs and publish to STATE_DB */ + publishTrapIdsCapability(); + initDefaultHostIntfTable(); initDefaultTrapGroup(); initDefaultTrapIds(); + }; +bool CoppOrch::isTrapIdSupported(sai_hostif_trap_type_t trap_id) const +{ + return supported_trap_ids.find(trap_id) != supported_trap_ids.end(); +} + +void CoppOrch::updateTrapOperStatus(sai_hostif_trap_type_t trap_type, const string& hw_status) +{ + SWSS_LOG_ENTER(); + + string trap_name = get_trap_name_by_type(trap_type); + if (trap_name.empty()) + { + SWSS_LOG_ERROR("Failed to get trap name for type %d", trap_type); + return; + } + + // Update or add the hw_status field in the table + vector hwStatusFvs; + hwStatusFvs.emplace_back("hw_status", hw_status); + m_trapTable->set(trap_name, hwStatusFvs); +} + +// Query SAI for trap IDs capability and publish to the COPP_TRAP_CAPABILITY_TABLE +void CoppOrch::publishTrapIdsCapability() +{ + SWSS_LOG_ENTER(); + + sai_s32_list_t enum_values_capability; + + supported_trap_ids.clear(); + + const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_HOSTIF_TRAP, SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE); + if (!meta || !meta->isenum) + { + SWSS_LOG_WARN("sai_metadata_get_attr_metadata for SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE Failed"); + return; + } + + vector values_list(meta->enummetadata->valuescount); + enum_values_capability.count = static_cast(values_list.size()); + enum_values_capability.list = values_list.data(); + + sai_status_t status = sai_query_attribute_enum_values_capability(gSwitchId, + SAI_OBJECT_TYPE_HOSTIF_TRAP, + SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE, + &enum_values_capability); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_NOTICE("SAI capability query for SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE is failed," + " falling back to default supported trap IDs"); + // Populate enum_values_capability with default_supported_trap_ids + enum_values_capability.count = static_cast(default_supported_trap_ids.size()); + values_list.assign(default_supported_trap_ids.begin(), default_supported_trap_ids.end()); + enum_values_capability.list = values_list.data(); + } + + SWSS_LOG_NOTICE("Number of supported trap IDs: %u", enum_values_capability.count); + + string trap_id_list_str; + + for (uint32_t i = 0; i < enum_values_capability.count; i++) + { + + auto trap_str = get_trap_name_by_type(static_cast(enum_values_capability.list[i])); + if (trap_str.empty()) + { + SWSS_LOG_NOTICE("Unknown trap id enum value: %d", enum_values_capability.list[i]); + continue; + } + + supported_trap_ids.insert(static_cast(enum_values_capability.list[i])); + + if (!trap_id_list_str.empty()) + { + trap_id_list_str += ","; + } + trap_id_list_str += trap_str; + } + + SWSS_LOG_NOTICE("Publishing supported trap IDs to STATE_DB"); + vector trapCapabilityFvs; + trapCapabilityFvs.push_back(FieldValueTuple("trap_ids", trap_id_list_str)); + m_trapCapabilityTable->set("traps", trapCapabilityFvs); +} + void CoppOrch::initDefaultHostIntfTable() { SWSS_LOG_ENTER(); @@ -248,6 +404,14 @@ void CoppOrch::getTrapIdList(vector &trap_id_name_list, vector tmp_trap_ids = trap_ids; + if(m_trap_group_map.find(trap_group_name) == m_trap_group_map.end()) { add_trap_ids = trap_ids; @@ -867,7 +1034,7 @@ bool CoppOrch::trapGroupProcessTrapIdChange (string trap_group_name, { if (m_syncdTrapIds.find(i)!= m_syncdTrapIds.end()) { - if (!removeTrap(m_syncdTrapIds[i].trap_obj)) + if (!removeTrap(m_syncdTrapIds[i].trap_obj, i)) { return false; } @@ -912,7 +1079,7 @@ bool CoppOrch::trapGroupProcessTrapIdChange (string trap_group_name, */ if (m_syncdTrapIds[i].trap_group_obj == m_trap_group_map[trap_group_name]) { - if (!removeTrap(m_syncdTrapIds[i].trap_obj)) + if (!removeTrap(m_syncdTrapIds[i].trap_obj, i)) { return false; } @@ -956,7 +1123,7 @@ bool CoppOrch::processTrapGroupDel (string trap_group_name) if (it.second.trap_group_obj == m_trap_group_map[trap_group_name]) { trap_ids_to_reset.push_back(it.first); - if (!removeTrap(it.second.trap_obj)) + if (!removeTrap(it.second.trap_obj, it.first)) { return false; } @@ -1227,7 +1394,7 @@ void CoppOrch::initTrapRatePlugin() m_trap_rate_plugin_loaded = true; } -bool CoppOrch::removeTrap(sai_object_id_t hostif_trap_id) +bool CoppOrch::removeTrap(sai_object_id_t hostif_trap_id, sai_hostif_trap_type_t trap_type) { unbindTrapCounter(hostif_trap_id); @@ -1242,6 +1409,8 @@ bool CoppOrch::removeTrap(sai_object_id_t hostif_trap_id) return parseHandleSaiStatusFailure(handle_status); } } + + updateTrapOperStatus(trap_type, "not-installed"); return true; } diff --git a/orchagent/copporch.h b/orchagent/copporch.h index fa49324a88b..76755cf3a32 100644 --- a/orchagent/copporch.h +++ b/orchagent/copporch.h @@ -1,9 +1,16 @@ #ifndef SWSS_COPPORCH_H #define SWSS_COPPORCH_H +extern "C" { +#include +#include +#include +} + #include #include #include +#include #include "dbconnector.h" #include "orch.h" #include "flex_counter_manager.h" @@ -99,8 +106,13 @@ class CoppOrch : public Orch std::shared_ptr m_counter_db; std::shared_ptr m_asic_db; + std::shared_ptr m_state_db; std::unique_ptr
m_counter_table; std::unique_ptr
m_vidToRidTable; + std::unique_ptr
m_trapCapabilityTable; + std::unique_ptr
m_trapTable; + + std::unordered_set supported_trap_ids; FlexCounterManager m_trap_counter_manager; @@ -112,6 +124,9 @@ class CoppOrch : public Orch void initDefaultTrapGroup(); void initDefaultTrapIds(); void initTrapRatePlugin(); + bool isTrapIdSupported(sai_hostif_trap_type_t trap_id) const; + void updateTrapOperStatus(sai_hostif_trap_type_t trap_type, const std::string& hw_status); + void publishTrapIdsCapability(); task_process_status processCoppRule(Consumer& consumer); bool isValidList(std::vector &trap_id_list, std::vector &all_items) const; @@ -148,7 +163,7 @@ class CoppOrch : public Orch bool trapGroupUpdatePolicer (std::string trap_group_name, std::vector &policer_attribs); - bool removeTrap(sai_object_id_t hostif_trap_id); + bool removeTrap(sai_object_id_t hostif_trap_id, sai_hostif_trap_type_t trap_type); bool bindTrapCounter(sai_object_id_t hostif_trap_id, sai_hostif_trap_type_t trap_type); void unbindTrapCounter(sai_object_id_t hostif_trap_id); diff --git a/orchagent/debugcounterorch.cpp b/orchagent/debugcounterorch.cpp index 882eaaf39b7..56d072e2551 100644 --- a/orchagent/debugcounterorch.cpp +++ b/orchagent/debugcounterorch.cpp @@ -227,7 +227,7 @@ void DebugCounterOrch::doTask(Consumer& consumer) // publishDropCounterCapabilities queries the SAI for available drop counter // capabilities on this device and publishes the information to the -// DROP_COUNTER_CAPABILITIES table in STATE_DB. +// DEBUG_COUNTER_CAPABILITIES table in STATE_DB. void DebugCounterOrch::publishDropCounterCapabilities() { supported_ingress_drop_reasons = DropCounter::getSupportedDropReasons(SAI_DEBUG_COUNTER_ATTR_IN_DROP_REASON_LIST); diff --git a/tests/mock_tests/copp_cfg.json b/tests/mock_tests/copp_cfg.json index 46d921b8276..f23228ed549 100644 --- a/tests/mock_tests/copp_cfg.json +++ b/tests/mock_tests/copp_cfg.json @@ -48,6 +48,16 @@ "cbs":"600", "red_action":"drop" }, + "queue1_group3": { + "trap_action":"trap", + "trap_priority":"1", + "queue": "1", + "meter_type":"packets", + "mode":"sr_tcm", + "cir":"200", + "cbs":"200", + "red_action":"drop" + }, "queue2_group1": { "cbs": "1000", "cir": "1000", @@ -106,6 +116,11 @@ "sflow": { "trap_group": "queue2_group1", "trap_ids": "sample_packet" + }, + "neighbor_miss": { + "trap_ids": "neighbor_miss", + "trap_group": "queue1_group3", + "always_enabled": "true" } } } diff --git a/tests/mock_tests/copporch_ut.cpp b/tests/mock_tests/copporch_ut.cpp index 9f94e586342..926c3b4779e 100644 --- a/tests/mock_tests/copporch_ut.cpp +++ b/tests/mock_tests/copporch_ut.cpp @@ -293,6 +293,15 @@ namespace copporch_test std::vector resourcesList; }; + TEST_F(CoppOrchTest, VerifySupportedTrapIds) + { + MockCoppOrch coppOrch; + + const auto &supportedTrapIds = Portal::CoppOrchInternal::getSupportedTrapIds(coppOrch.get()); + EXPECT_TRUE(supportedTrapIds.find(SAI_HOSTIF_TRAP_TYPE_IP2ME) != supportedTrapIds.end()); + EXPECT_TRUE(supportedTrapIds.find(SAI_HOSTIF_TRAP_TYPE_NEIGHBOR_MISS) == supportedTrapIds.end()); + } + TEST_F(CoppOrchTest, TrapGroup_AddRemove) { const std::string trapGroupName = "queue4_group1"; @@ -457,7 +466,7 @@ namespace copporch_test TEST_F(CoppOrchTest, Trap_AddRemove) { const std::string trapGroupName = "queue4_group1"; - const std::string trapNameList = "bgp,bgpv6"; + const std::string trapNameList = "bgp,bgpv6,neighbor_miss"; const std::set trapIDSet = { SAI_HOSTIF_TRAP_TYPE_BGP, SAI_HOSTIF_TRAP_TYPE_BGPV6 @@ -490,6 +499,11 @@ namespace copporch_test const auto &tgOid = cit->second; const auto &tidList = Portal::CoppOrchInternal::getTrapIdsFromTrapGroup(coppOrch.get(), tgOid); const auto &tidSet = std::set(tidList.begin(), tidList.end()); + + // Verify that neighbor_miss is not installed + EXPECT_TRUE(tidSet.find(SAI_HOSTIF_TRAP_TYPE_NEIGHBOR_MISS) == tidSet.end()); + + // Verify that bgp and bgpv6 are installed EXPECT_TRUE(trapIDSet == tidSet); } diff --git a/tests/mock_tests/portal.h b/tests/mock_tests/portal.h index 31fa4ac4b7e..cfc78ed45de 100644 --- a/tests/mock_tests/portal.h +++ b/tests/mock_tests/portal.h @@ -87,6 +87,11 @@ struct Portal { return obj.processCoppRule(processCoppRule); } + + static const std::unordered_set& getSupportedTrapIds(const CoppOrch &obj) + { + return obj.supported_trap_ids; + } }; struct SflowOrchInternal diff --git a/tests/test_copp.py b/tests/test_copp.py index e7d86c41d74..686c851a685 100644 --- a/tests/test_copp.py +++ b/tests/test_copp.py @@ -71,7 +71,8 @@ "dest_nat_miss": "SAI_HOSTIF_TRAP_TYPE_DNAT_MISS", "ldp": "SAI_HOSTIF_TRAP_TYPE_LDP", "bfd_micro": "SAI_HOSTIF_TRAP_TYPE_BFD_MICRO", - "bfdv6_micro": "SAI_HOSTIF_TRAP_TYPE_BFDV6_MICRO" + "bfdv6_micro": "SAI_HOSTIF_TRAP_TYPE_BFDV6_MICRO", + "neighbor_miss": "SAI_HOSTIF_TRAP_TYPE_NEIGHBOR_MISS" } copp_group_default = { @@ -128,6 +129,17 @@ "red_action":"drop" } +copp_group_queue1_group3 = { + "trap_action":"trap", + "trap_priority":"1", + "queue": "1", + "meter_type":"packets", + "mode":"sr_tcm", + "cir":"200", + "cbs":"200", + "red_action":"drop" +} + copp_group_queue2_group1 = { "cbs": "1000", "cir": "1000", @@ -162,7 +174,8 @@ "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] + "ttl": ["ttl_error", copp_group_default], + "neighbor_miss": ["neighbor_miss", copp_group_queue1_group3, "always_enabled"] } disabled_traps = ["sample_packet"] @@ -194,6 +207,7 @@ class TestCopp(object): def setup_copp(self, dvs): self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) self.cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) + self.sdb = swsscommon.DBConnector(6, dvs.redis_sock, 0) self.trap_atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF_TRAP") self.trap_group_atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF_TRAP_GROUP") self.policer_atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_POLICER") @@ -202,10 +216,24 @@ 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") + self.state_stbl = swsscommon.Table(self.sdb, "COPP_TRAP_TABLE") + self.capability_stbl = swsscommon.Table(self.sdb, "COPP_TRAP_CAPABILITY_TABLE") fvs = swsscommon.FieldValuePairs([("state", "disabled")]) self.feature_tbl.set("sflow", fvs) time.sleep(2) + def validate_trap_hw_status(self, trap_id, trap_status): + (status, fvs) = self.state_stbl.get(trap_id) + if trap_status == True: + assert status == True + + if status: + for fv in fvs: + if fv[0] == "hw_status": + if trap_status == True: + assert fv[1] == "installed" + else: + assert fv[1] == "not-installed" def validate_policer(self, policer_oid, field, value): (status, fvs) = self.policer_atbl.get(policer_oid) @@ -337,6 +365,15 @@ def test_defaults(self, dvs, testlog): if trap_id not in disabled_traps: assert trap_found == True + (status, fvs) = self.capability_stbl.get("traps") + assert status == True + trap_list = [] + for fv in fvs: + if fv[0] == "trap_ids": + trap_list = fv[1].split(",") + break + + assert len(trap_list) != 0 def test_restricted_trap_sflow(self, dvs, testlog): self.setup_copp(dvs) @@ -477,6 +514,7 @@ def test_trap_ids_set(self, dvs, testlog): assert trap_found == True elif trap_id == "bgpv6": assert trap_found == False + self.validate_trap_hw_status(trap_id, trap_found) traps = "bgp,bgpv6" fvs = swsscommon.FieldValuePairs([("trap_ids", traps)]) @@ -501,6 +539,8 @@ def test_trap_ids_set(self, dvs, testlog): self.validate_trap_group(key,trap_group) break assert trap_found == True + self.validate_trap_hw_status(trap_id, trap_found) + def test_trap_action_set(self, dvs, testlog): self.setup_copp(dvs) @@ -565,6 +605,7 @@ def test_new_trap_add(self, dvs, testlog): break if trap_id not in disabled_traps: assert trap_found == True + self.validate_trap_hw_status(trap_id, trap_found) def test_new_trap_del(self, dvs, testlog): self.setup_copp(dvs) @@ -602,6 +643,7 @@ def test_new_trap_del(self, dvs, testlog): break if trap_id not in disabled_traps: assert trap_found == False + self.validate_trap_hw_status(trap_id, trap_found) def test_new_trap_group_add(self, dvs, testlog): self.setup_copp(dvs) @@ -641,6 +683,7 @@ def test_new_trap_group_add(self, dvs, testlog): break if trap_id not in disabled_traps: assert trap_found == True + self.validate_trap_hw_status(trap_id, trap_found) def test_new_trap_group_del(self, dvs, testlog): self.setup_copp(dvs) @@ -682,6 +725,7 @@ def test_new_trap_group_del(self, dvs, testlog): break if trap_id not in disabled_traps: assert trap_found != True + self.validate_trap_hw_status(trap_id, trap_found) def test_override_trap_grp_cfg_del (self, dvs, testlog): self.setup_copp(dvs) @@ -751,6 +795,7 @@ def test_override_trap_cfg_del(self, dvs, testlog): assert trap_found == True elif trap_id == "ssh": assert trap_found == False + self.validate_trap_hw_status(trap_id, trap_found) def test_empty_trap_cfg(self, dvs, testlog): self.setup_copp(dvs) @@ -776,6 +821,7 @@ def test_empty_trap_cfg(self, dvs, testlog): self.validate_trap_group(key,trap_group) break assert trap_found == False + self.validate_trap_hw_status(trap_id, trap_found) self.trap_ctbl._del("ip2me") time.sleep(2) @@ -795,6 +841,7 @@ def test_empty_trap_cfg(self, dvs, testlog): self.validate_trap_group(key,trap_group) break assert trap_found == True + self.validate_trap_hw_status(trap_id, trap_found) def test_disabled_feature_always_enabled_trap(self, dvs, testlog): From 4d3b1dea078c4751d785f64aa146f37afbb09520 Mon Sep 17 00:00:00 2001 From: Ravi Minnikanti Date: Sun, 22 Jun 2025 23:03:42 +0530 Subject: [PATCH 2/3] Fix mock tests libvssai has HOSTIF_TRAP_TYPE object support merged which returns all the trap types as supported. With that default_supported_trap_ids is no longer used by copporch.cpp as part of mock_Tests Signed-off-by: Ravi Minnikanti --- tests/mock_tests/copporch_ut.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/mock_tests/copporch_ut.cpp b/tests/mock_tests/copporch_ut.cpp index 926c3b4779e..8e91df04c06 100644 --- a/tests/mock_tests/copporch_ut.cpp +++ b/tests/mock_tests/copporch_ut.cpp @@ -299,7 +299,7 @@ namespace copporch_test const auto &supportedTrapIds = Portal::CoppOrchInternal::getSupportedTrapIds(coppOrch.get()); EXPECT_TRUE(supportedTrapIds.find(SAI_HOSTIF_TRAP_TYPE_IP2ME) != supportedTrapIds.end()); - EXPECT_TRUE(supportedTrapIds.find(SAI_HOSTIF_TRAP_TYPE_NEIGHBOR_MISS) == supportedTrapIds.end()); + EXPECT_TRUE(supportedTrapIds.find(SAI_HOSTIF_TRAP_TYPE_NEIGHBOR_MISS) != supportedTrapIds.end()); } TEST_F(CoppOrchTest, TrapGroup_AddRemove) @@ -469,7 +469,8 @@ namespace copporch_test const std::string trapNameList = "bgp,bgpv6,neighbor_miss"; const std::set trapIDSet = { SAI_HOSTIF_TRAP_TYPE_BGP, - SAI_HOSTIF_TRAP_TYPE_BGPV6 + SAI_HOSTIF_TRAP_TYPE_BGPV6, + SAI_HOSTIF_TRAP_TYPE_NEIGHBOR_MISS }; MockCoppOrch coppOrch; @@ -500,10 +501,7 @@ namespace copporch_test const auto &tidList = Portal::CoppOrchInternal::getTrapIdsFromTrapGroup(coppOrch.get(), tgOid); const auto &tidSet = std::set(tidList.begin(), tidList.end()); - // Verify that neighbor_miss is not installed - EXPECT_TRUE(tidSet.find(SAI_HOSTIF_TRAP_TYPE_NEIGHBOR_MISS) == tidSet.end()); - - // Verify that bgp and bgpv6 are installed + // Verify that bgp, bgpv6 and neighbor_miss are installed EXPECT_TRUE(trapIDSet == tidSet); } From b5ebb4f40e468fcbe24146850cf35a9cd7e229b4 Mon Sep 17 00:00:00 2001 From: Ravi Minnikanti Date: Tue, 15 Jul 2025 09:36:32 +0530 Subject: [PATCH 3/3] Remove neighbor_miss from default traps Signed-off-by: Ravi Minnikanti --- tests/test_copp.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_copp.py b/tests/test_copp.py index 686c851a685..c9dd514013f 100644 --- a/tests/test_copp.py +++ b/tests/test_copp.py @@ -174,8 +174,7 @@ "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], - "neighbor_miss": ["neighbor_miss", copp_group_queue1_group3, "always_enabled"] + "ttl": ["ttl_error", copp_group_default] } disabled_traps = ["sample_packet"]