[copporch]: fix the error when removing the copp group table.

orchagent/copporch.cpp

@@ -212,6 +212,30 @@ bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id,
             return false;
         }
         m_syncdTrapIds[trap_id] = trap_group_id;
+	m_trapIdObjectIds[trap_id] = hostif_trap_id;
+    }
+
+    return true;
+}
+
+bool CoppOrch::removeTrapIds(const vector<sai_hostif_trap_type_t> &trap_id_list)
+{
+    sai_object_id_t trap_id_object = SAI_NULL_OBJECT_ID;
+
+    for (auto trap_id : trap_id_list)
+    {
+        if (m_trapIdObjectIds.find(trap_id) != m_trapIdObjectIds.end())
+        {
+            trap_id_object = m_trapIdObjectIds[trap_id];
+            sai_status_t status = sai_hostif_api->remove_hostif_trap(trap_id_object);
+            if (status != SAI_STATUS_SUCCESS)
+            {
+                SWSS_LOG_ERROR("Failed to remove trap %d, rv:%d", trap_id, status);
+                return false;
+            }
+            m_trapIdObjectIds.erase(trap_id);
+            m_syncdTrapIds.erase(trap_id);
+        }
     }
 
     return true;
@@ -550,21 +574,10 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer)
                 trap_ids_to_reset.push_back(it.first);
             }
         }
-
-        sai_attribute_t attr;
-        vector<sai_attribute_t> default_trap_attrs;
-
-        attr.id = SAI_HOSTIF_TRAP_ATTR_PACKET_ACTION;
-        attr.value.s32 = SAI_PACKET_ACTION_FORWARD;
-        default_trap_attrs.push_back(attr);
-
-        attr.id = SAI_HOSTIF_TRAP_ATTR_TRAP_GROUP;
-        attr.value.oid = m_trap_group_map[default_trap_group];
-        default_trap_attrs.push_back(attr);
-
-        if (!applyAttributesToTrapIds(m_trap_group_map[default_trap_group], trap_ids_to_reset, default_trap_attrs))
+
+        if ( !removeTrapIds(trap_ids_to_reset))
         {
-            SWSS_LOG_ERROR("Failed to reset traps to default trap group with default attributes");
+            SWSS_LOG_ERROR("Failed to remove trap id with group %s",trap_group_name.c_str());
             return task_process_status::task_failed;
         }
 

orchagent/copporch.h

@@ -28,6 +28,9 @@ const string copp_policer_action_yellow_field = "yellow_action";
 typedef map<sai_object_id_t, sai_object_id_t> TrapGroupPolicerTable;
 /* TrapIdTrapGroupTable: trap ID, trap group ID */
 typedef map<sai_hostif_trap_type_t, sai_object_id_t> TrapIdTrapGroupTable;
+/* TrapIdTrapObjectTable: trap ID,  object id   */
+typedef map<sai_hostif_trap_type_t, sai_object_id_t> TrapIdTrapObjectTable;
+
 
 class CoppOrch : public Orch
 {
@@ -38,6 +41,7 @@ class CoppOrch : public Orch
 
     TrapGroupPolicerTable m_trap_group_policer_map;
     TrapIdTrapGroupTable m_syncdTrapIds;
+    TrapIdTrapObjectTable m_trapIdObjectIds;
 
     void initDefaultHostIntfTable();
     void initDefaultTrapGroup();
@@ -51,7 +55,7 @@ class CoppOrch : public Orch
 
     bool createPolicer(string trap_group, vector<sai_attribute_t> &policer_attribs);
     bool removePolicer(string trap_group_name);
-
+    bool removeTrapIds(const vector<sai_hostif_trap_type_t> &trap_id_list);
     sai_object_id_t getPolicer(string trap_group_name);
 
     virtual void doTask(Consumer& consumer);

tests/test_copp.py

@@ -0,0 +1,100 @@
+from swsscommon import swsscommon
+import time
+import re
+import json
+
+class TestCopp(object):
+    def setup_db(self, dvs):
+        self.pdb = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
+        self.adb = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
+
+    def create_copp_group_table(self, dvs, testlog):
+        self.setup_db(dvs)
+        group_name = "trap.group.test"
+        tbl = swsscommon.ProducerStateTable(self.pdb, "COPP_TABLE")
+        fvs = swsscommon.FieldValuePairs([("trap_ids", "arp_resp"),
+                                          ("trap_action", "copy"),
+                                          ("trap_priority", "4"),
+                                          ("queue","4"),
+                                          ("meter_type", "packets"),
+                                          ("mode", "tr_tcm"),
+                                          ("cir", "600"),
+                                          ("cbs", "600"),
+                                          ("pir", "600"),
+                                          ("pbs", "600"),
+                                          ("red_action", "drop")])
+        tbl.set(group_name, fvs)
+        time.sleep(1)
+
+    def remove_copp_group_table(self, dvs, testlog):
+        self.setup_db(dvs)
+        group_name = "trap.group.test"
+        tbl = swsscommon.ProducerStateTable(self.pdb, "COPP_TABLE")
+        tbl._del(group_name)
+        time.sleep(1)
+
+    def verify_create_copp_group_table(self, dvs, testlog):
+        self.setup_db(dvs)
+        tbl_trap_group = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF_TRAP_GROUP")
+        keys = tbl_trap_group.getKeys()
+
+        assert len(keys) == 2
+        for k in keys:
+            status, fvs = tbl_trap_group.get(k)
+            for (field, value) in fvs:
+                #ignore the default group
+                if field == 'SAI_HOSTIF_TRAP_GROUP_ATTR_QUEUE':
+                    assert value == '4'
+                    test_group_key = k
+                elif field == 'SAI_HOSTIF_TRAP_GROUP_ATTR_POLICER':
+                    assert value != 'NULL'
+                else:
+                    continue
+        # Trap ids
+        tbl_trap_ids = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF_TRAP")
+        keys = tbl_trap_ids.getKeys()
+        assert len(keys) == 2
+        for k in keys:
+            status, fvs = tbl_trap_ids.get(k)
+            for (field, value) in fvs:
+                if field == 'SAI_HOSTIF_TRAP_ATTR_TRAP_GROUP':
+                    if value == test_group_key:
+                        test_trap_table_fvs = fvs
+                        break
+
+        for (field, value) in test_trap_table_fvs:
+            if field == 'SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE':
+                assert value == 'SAI_HOSTIF_TRAP_TYPE_ARP_RESPONSE'
+            elif field == 'SAI_HOSTIF_TRAP_ATTR_PACKET_ACTION':
+                assert value == 'SAI_PACKET_ACTION_COPY'
+            elif field == 'SAI_HOSTIF_TRAP_ATTR_TRAP_PRIORITY':
+                assert value == '4'
+            else:
+                continue
+
+    def verify_remove_copp_group_table(self, dvs, testlog):
+        self.setup_db(dvs)
+        tbl_trap_group = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF_TRAP_GROUP")
+        keys = tbl_trap_group.getKeys()
+        #only the defaut group
+        assert len(keys) == 1 
+        for k in keys:
+            status, fvs = tbl_trap_group.get(k)
+            for (field, value) in fvs:
+                assert field == 'NULL'
+                assert value == 'NULL'
+        # Trap ids
+        tbl_trap_ids = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF_TRAP")
+        keys = tbl_trap_ids.getKeys()
+        assert len(keys) == 1
+
+
+    def test_add_copp_grout_table(self, dvs, testlog):
+        self.create_copp_group_table(dvs, testlog)
+        self.verify_create_copp_group_table(dvs, testlog)
+
+    def test_remove_copp_grout_table(self, dvs, testlog):
+        self.create_copp_group_table(dvs, testlog)
+        self.verify_create_copp_group_table(dvs, testlog)
+        self.remove_copp_group_table(dvs, testlog)
+        self.verify_remove_copp_group_table(dvs, testlog)