[COPP]Fix coppmgr when there are multiple features using same trap

dgsudharsan · May 9, 2024 · 87615ca · 87615ca
1 parent 837e467
commit 87615ca
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 7 deletions.
diff --git a/cfgmgr/coppmgr.cpp b/cfgmgr/coppmgr.cpp
@@ -183,14 +183,13 @@ bool CoppMgr::isTrapIdDisabled(string trap_id)
             {
                 return false;
             }
-            break;
+            if (isFeatureEnabled(trap_name))
+            {
+                return false;
+            }
         }
     }
 
-    if (isFeatureEnabled(trap_name))
-    {
-        return false;
-    }
     return true;
 }
 
@@ -941,7 +940,9 @@ void CoppMgr::doFeatureTask(Consumer &consumer)
         {
             if (m_featuresCfgTable.find(key) == m_featuresCfgTable.end())
             {
-                m_featuresCfgTable.emplace(key, kfvFieldsValues(t));
+                // Init with empty feature state which will be updated  in setFeatureTrapIdsStatus
+                FieldValueTuple fv("state", "");
+                m_featuresCfgTable[key].push_back(fv);
             }
             for (auto i : kfvFieldsValues(t))
             {

diff --git a/orchagent/copporch.cpp b/orchagent/copporch.cpp
@@ -633,7 +633,7 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer)
         if (!trap_id_attribs.empty())
         {
             vector<sai_hostif_trap_type_t> group_trap_ids;
-            TrapIdAttribs trap_attr;
+            TrapIdAttribs trap_attr = m_trap_group_trap_id_attrs[trap_group_name];
             getTrapIdsFromTrapGroup(m_trap_group_map[trap_group_name],
                                     group_trap_ids);
             for (auto trap_id : group_trap_ids)

diff --git a/tests/test_copp.py b/tests/test_copp.py
@@ -841,3 +841,38 @@ def test_disabled_feature_always_enabled_trap(self, dvs, testlog):
         self.feature_tbl.set("lldp", fvs)
 
         assert table_found == False
+
+    def test_multi_feature_trap_add(self, dvs, testlog):
+        self.setup_copp(dvs)
+        global copp_trap
+        traps = "eapol"
+        fvs = swsscommon.FieldValuePairs([("state", "disbled")])
+        self.feature_tbl.set("macsec", fvs)
+        fvs = swsscommon.FieldValuePairs([("state", "enabled")])
+        self.feature_tbl.set("pac", fvs)
+        fvs = swsscommon.FieldValuePairs([("trap_group", "queue4_group1"),("trap_ids", traps)])
+        self.trap_ctbl.set("pac", fvs)
+
+
+        copp_trap["eapol"] = [traps, copp_group_queue4_group1]
+        time.sleep(2)
+
+        trap_keys = self.trap_atbl.getKeys()
+        trap_ids = traps.split(",")
+        trap_group = copp_group_queue4_group1
+        for trap_id in trap_ids:
+            trap_type = traps_to_trap_type[trap_id]
+            trap_found = False
+            trap_group_oid = ""
+            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
+            if trap_id not in disabled_traps:
+                assert trap_found == True