Remove AclTableGroup upon removal of port/lag/vlan (#751)

* Remove AclTableGroup upon removal of port/lag/vlan

Signed-off-by: Jipan Yang <jipan.yang@alibaba-inc.com>

Author: jipanyang

orchagent/portsorch.cpp

@@ -784,7 +784,7 @@ bool PortsOrch::bindAclTable(sai_object_id_t id, sai_object_id_t table_oid, sai_
 
         setPort(port.m_alias, port);
 
-        gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_GROUP, ingress ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS, SAI_ACL_BIND_POINT_TYPE_PORT);
+        gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_GROUP, ingress ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS, bind_type);
 
         switch (port.m_type)
         {
@@ -1264,7 +1264,7 @@ bool PortsOrch::removePort(sai_object_id_t port_id)
         SWSS_LOG_ERROR("Failed to remove port %lx, rv:%d", port_id, status);
         return false;
     }
-
+    removeAclTableGroup(p);
     SWSS_LOG_NOTICE("Remove port %lx", port_id);
 
     return true;
@@ -2602,6 +2602,8 @@ bool PortsOrch::removeVlan(Port vlan)
         return false;
     }
 
+    removeAclTableGroup(vlan);
+
     SWSS_LOG_NOTICE("Remove VLAN %s vid:%hu", vlan.m_alias.c_str(),
             vlan.m_vlan_info.vlan_id);
 
@@ -2778,6 +2780,8 @@ bool PortsOrch::removeLag(Port lag)
         return false;
     }
 
+    removeAclTableGroup(lag);
+
     SWSS_LOG_NOTICE("Remove LAG %s lid:%lx", lag.m_alias.c_str(), lag.m_lag_id);
 
     m_portList.erase(lag.m_alias);
@@ -3178,3 +3182,46 @@ bool PortsOrch::getPortOperStatus(const Port& port, sai_port_oper_status_t& stat
     return true;
 }
 
+bool PortsOrch::removeAclTableGroup(const Port &p)
+{
+    sai_acl_bind_point_type_t bind_type;
+    switch (p.m_type)
+    {
+        case Port::PHY:
+            bind_type = SAI_ACL_BIND_POINT_TYPE_PORT;
+            break;
+        case Port::LAG:
+            bind_type = SAI_ACL_BIND_POINT_TYPE_LAG;
+            break;
+        case Port::VLAN:
+            bind_type = SAI_ACL_BIND_POINT_TYPE_VLAN;
+            break;
+        default:
+            // Dealing with port, lag and vlan for now.
+            return true;
+    }
+    sai_status_t ret;
+    if (p.m_ingress_acl_table_group_id != 0)
+    {
+        ret = sai_acl_api->remove_acl_table_group(p.m_ingress_acl_table_group_id);
+        if (ret != SAI_STATUS_SUCCESS)
+        {
+            SWSS_LOG_ERROR("Failed to remove ingress acl table group for %s", p.m_alias.c_str());
+            return false;
+        }
+        gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_GROUP, SAI_ACL_STAGE_INGRESS, bind_type, p.m_ingress_acl_table_group_id);
+    }
+
+    if (p.m_egress_acl_table_group_id != 0)
+    {
+        ret = sai_acl_api->remove_acl_table_group(p.m_egress_acl_table_group_id);
+        if (ret != SAI_STATUS_SUCCESS)
+        {
+            SWSS_LOG_ERROR("Failed to remove egress acl table group for %s", p.m_alias.c_str());
+            return false;
+        }
+        gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_GROUP, SAI_ACL_STAGE_EGRESS, bind_type, p.m_egress_acl_table_group_id);
+    }
+    return true;
+}
+

orchagent/portsorch.h

@@ -81,7 +81,7 @@ class PortsOrch : public Orch, public Subject
     void generatePriorityGroupMap();
 
     void refreshPortStatus();
-
+    bool removeAclTableGroup(const Port &p);
 private:
     unique_ptr<Table> m_counterTable;
     unique_ptr<Table> m_portTable;

tests/test_acl_portchannel.py

@@ -127,15 +127,22 @@ def check_asic_table_absent(self, dvs):
     # Second create ACL table
     def test_PortChannelAfterAcl(self, dvs):
         self.setup_db(dvs)
+        dvs.runcmd("crm config polling interval 1")
+        time.sleep(2)
 
+        used_counter = dvs.getCrmCounterValue('ACL_STATS:INGRESS:LAG', 'crm_stats_acl_group_used')
         # create port channel
         self.create_port_channel(dvs, "PortChannel01")
 
         # create ACL table
         self.create_acl_table(dvs, "LAG_ACL_TABLE", "PortChannel01")
 
-        time.sleep(1)
+        time.sleep(2)
 
+        new_used_counter = dvs.getCrmCounterValue('ACL_STATS:INGRESS:LAG', 'crm_stats_acl_group_used')
+        if used_counter is None:
+            used_counter = 0
+        assert new_used_counter - used_counter == 1
         # check ASIC table
         self.check_asic_table_existed(dvs)
 
@@ -145,6 +152,14 @@ def test_PortChannelAfterAcl(self, dvs):
         # remove port channel
         self.remove_port_channel(dvs, "PortChannel01")
 
+        time.sleep(2)
+        new_new_used_counter = dvs.getCrmCounterValue('ACL_STATS:INGRESS:LAG', 'crm_stats_acl_group_used')
+        if new_new_used_counter is None:
+            new_new_used_counter = 0
+        assert new_used_counter - new_new_used_counter == 1
+        # slow down crm polling
+        dvs.runcmd("crm config polling interval 10000")
+
     # Frist create ACL table
     # Second create port channel
     def test_PortChannelBeforeAcl(self, dvs):