[aclorch]: Store control plane ACL tables in orchagent (sonic-net#708)

To prevent control plane ACL rules from being stored in the pending
task set, store the control plane ACL table in orchagent first and
check if the ACL rule is associated with control plane ACL table and
ignore it.

The next step is to combine the data structure of control plane ACL
tables and data plane ACL tables to make the code more efficient.

Add unit test to ensure control plane ACL table won't be added
to the ASIC database.

Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>

orchagent/aclorch.cpp

@@ -1051,10 +1051,14 @@ void AclRuleMirror::update(SubjectType type, void *cntx)
 
 bool AclTable::validate()
 {
-    // Control plane ACLs are handled by a separate process
-    if (type == ACL_TABLE_UNKNOWN || type == ACL_TABLE_CTRLPLANE) return false;
-    if (stage == ACL_STAGE_UNKNOWN) return false;
-    if (portSet.empty() && pendingPortSet.empty()) return false;
+    if (type == ACL_TABLE_CTRLPLANE)
+        return true;
+
+    if (type == ACL_TABLE_UNKNOWN || stage == ACL_STAGE_UNKNOWN)
+        return false;
+
+    if (portSet.empty() && pendingPortSet.empty())
+        return false;
 
     return true;
 }
@@ -1902,6 +1906,13 @@ bool AclOrch::addAclTable(AclTable &newTable, string table_id)
 {
     SWSS_LOG_ENTER();
 
+    if (newTable.type == ACL_TABLE_CTRLPLANE)
+    {
+        m_ctrlAclTables.emplace(table_id, newTable);
+        SWSS_LOG_NOTICE("Created control plane ACL table %s", newTable.id.c_str());
+        return true;
+    }
+
     sai_object_id_t table_oid = getTableById(table_id);
 
     if (table_oid != SAI_NULL_OBJECT_ID)
@@ -2048,13 +2059,19 @@ void AclOrch::doAclTableTask(Consumer &consumer)
                        break;
                    }
                 }
+                else if (attr_name == TABLE_SERVICES)
+                {
+                    // TODO: validate control plane ACL table has this attribute
+                    continue;
+                }
                 else
                 {
                     SWSS_LOG_ERROR("Unknown table attribute '%s'", attr_name.c_str());
                     bAllAttributesOk = false;
                     break;
                 }
             }
+
             // validate and create ACL Table
             if (bAllAttributesOk && newTable.validate())
             {
@@ -2107,10 +2124,18 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
             shared_ptr<AclRule> newRule;
             sai_object_id_t table_oid = getTableById(table_id);
 
-            /* ACL table is not yet created */
+            /* ACL table is not yet created or ACL table is a control plane table */
             /* TODO: Remove ACL_TABLE_UNKNOWN as a table with this type cannot be successfully created */
             if (table_oid == SAI_NULL_OBJECT_ID || m_AclTables[table_oid].type == ACL_TABLE_UNKNOWN)
             {
+                /* Skip the control plane rules */
+                if (m_ctrlAclTables.find(table_id) != m_ctrlAclTables.end())
+                {
+                    SWSS_LOG_INFO("Skip control plane ACL rule %s", key.c_str());
+                    it = consumer.m_toSync.erase(it);
+                    continue;
+                }
+
                 SWSS_LOG_INFO("Wait for ACL table %s to be created", table_id.c_str());
                 it++;
                 continue;

orchagent/aclorch.h

@@ -22,6 +22,7 @@
 #define TABLE_DESCRIPTION "POLICY_DESC"
 #define TABLE_TYPE        "TYPE"
 #define TABLE_PORTS       "PORTS"
+#define TABLE_SERVICES    "SERVICES"
 
 #define TABLE_TYPE_L3        "L3"
 #define TABLE_TYPE_L3V6      "L3V6"
@@ -403,7 +404,9 @@ class AclOrch : public Orch, public Observer
     sai_status_t deleteDTelWatchListTables();
 
     //vector <AclTable> m_AclTables;
-    map <sai_object_id_t, AclTable> m_AclTables;
+    map<sai_object_id_t, AclTable> m_AclTables;
+    // TODO: Move all ACL tables into one map: name -> instance
+    map<string, AclTable> m_ctrlAclTables;
 
     static mutex m_countersMutex;
     static condition_variable m_sleepGuard;

tests/test_acl_ctrl.py

@@ -0,0 +1,69 @@
+from swsscommon import swsscommon
+
+import time
+
+class TestPortChannelAcl(object):
+    def setup_db(self, dvs):
+        self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0)
+        self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0)
+        self.cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0)
+
+    def create_acl_table(self, dvs):
+        tbl = swsscommon.Table(self.cdb, "ACL_TABLE")
+        fvs = swsscommon.FieldValuePairs([("POLICY_DESC", "CTRL_ACL_TEST"),
+                                          ("TYPE", "CTRLPLANE"),
+                                          ("SERVICES@", "SNMP")])
+        tbl.set("CTRL_ACL_TABLE", fvs)
+        time.sleep(1)
+
+    def remove_acl_table(self, dvs):
+        tbl = swsscommon.Table(self.cdb, "ACL_TABLE")
+        tbl._del("CTRL_ACL_TABLE")
+        time.sleep(1)
+
+    def create_acl_rule(self, dvs):
+        tbl = swsscommon.Table(self.cdb, "ACL_RULE")
+        fvs = swsscommon.FieldValuePairs([("PRIORITY", "88"),
+                                          ("PACKET_ACTION", "FORWARD"),
+                                          ("L4_SRC_PORT", "8888")])
+        tbl.set("CTRL_ACL_TABLE|CTRL_ACL_RULE", fvs)
+        time.sleep(1)
+
+    def remove_acl_rule(self, dvs):
+        tbl = swsscommon.Table(self.cdb, "ACL_RULE")
+        tbl._del("CTRL_ACL_TABLE|CTRL_ACL_RULE")
+        time.sleep(1)
+
+    def check_asic_table_absent(self, dvs):
+        tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE")
+        acl_tables = tbl.getKeys()
+        for key in dvs.asicdb.default_acl_tables:
+            assert key in acl_tables
+        acl_tables = [k for k in acl_tables if k not in dvs.asicdb.default_acl_tables]
+
+        assert len(acl_tables) == 0
+
+    def check_asic_rule_absent(self, dvs):
+        tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY")
+        acl_entries = tbl.getKeys()
+        for key in dvs.asicdb.default_acl_entries:
+            assert key in acl_entries
+        acl_entries = [k for k in acl_entries if k not in dvs.asicdb.default_acl_entries]
+
+        assert len(acl_entries) == 0
+
+    def test_AclCtrl(self, dvs):
+        self.setup_db(dvs)
+
+        # create ACL table and ACL rule
+        self.create_acl_table(dvs)
+        self.create_acl_rule(dvs)
+
+        # check ASIC table
+        self.check_asic_table_absent(dvs)
+        self.check_asic_rule_absent(dvs)
+
+        # remove ACL table
+        self.remove_acl_table(dvs)
+        self.remove_acl_rule(dvs)
+