[aclorch] Add support for creating ingress and egress MIRROR tables concurrently (#1286)

daall · web-flow · commit a9dbc8386794 · 2020-05-27T14:27:50.000-07:00
Signed-off-by: Danny Allen &lt;daall@microsoft.com&gt;
diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp
@@ -2213,6 +2213,12 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr
 
     queryAclActionCapability();
 
+    for (auto stage: {ACL_STAGE_INGRESS, ACL_STAGE_EGRESS})
+    {
+        m_mirrorTableId[stage] = "";
+        m_mirrorV6TableId[stage] = "";
+    }
+
     // Attach observers
     m_mirrorOrch->attach(this);
     gPortsOrch->attach(this);
@@ -2666,6 +2672,7 @@ bool AclOrch::addAclTable(AclTable &newTable)
     }
 
     sai_object_id_t table_oid = getTableById(table_id);
+    auto table_stage = newTable.stage;
 
     if (table_oid != SAI_NULL_OBJECT_ID)
     {
@@ -2685,19 +2692,23 @@ bool AclOrch::addAclTable(AclTable &newTable)
         if (table_type == ACL_TABLE_MIRROR || table_type == ACL_TABLE_MIRRORV6)
         {
             string mirror_type;
-            if ((table_type == ACL_TABLE_MIRROR && !m_mirrorTableId.empty()))
+            if (table_type == ACL_TABLE_MIRROR && !m_mirrorTableId[table_stage].empty())
             {
                 mirror_type = TABLE_TYPE_MIRROR;
             }
 
-            if (table_type == ACL_TABLE_MIRRORV6 && !m_mirrorV6TableId.empty())
+            if (table_type == ACL_TABLE_MIRRORV6 && !m_mirrorV6TableId[table_stage].empty())
             {
                 mirror_type = TABLE_TYPE_MIRRORV6;
             }
 
             if (!mirror_type.empty())
             {
-                SWSS_LOG_ERROR("Mirror table %s has already been created", mirror_type.c_str());
+                string stage_str = table_stage == ACL_STAGE_INGRESS ? "INGRESS" : "EGRESS";
+                SWSS_LOG_ERROR(
+                    "Mirror table %s (%s) has already been created",
+                    mirror_type.c_str(),
+                    stage_str.c_str());
                 return false;
             }
         }
@@ -2706,23 +2717,22 @@ bool AclOrch::addAclTable(AclTable &newTable)
     // Check if a separate mirror table is needed or not based on the platform
     if (newTable.type == ACL_TABLE_MIRROR || newTable.type == ACL_TABLE_MIRRORV6)
     {
-
         if (m_isCombinedMirrorV6Table &&
-                (!m_mirrorTableId.empty() || !m_mirrorV6TableId.empty())) {
-
+                (!m_mirrorTableId[table_stage].empty() ||
+                !m_mirrorV6TableId[table_stage].empty())) {
             string orig_table_name;
 
             // If v4 table is created, mark v6 table is created
-            if (!m_mirrorTableId.empty())
+            if (!m_mirrorTableId[table_stage].empty())
             {
-                orig_table_name = m_mirrorTableId;
-                m_mirrorV6TableId = newTable.id;
+                orig_table_name = m_mirrorTableId[table_stage];
+                m_mirrorV6TableId[table_stage] = newTable.id;
             }
             // If v6 table is created, mark v4 table is created
             else
             {
-                orig_table_name = m_mirrorV6TableId;
-                m_mirrorTableId = newTable.id;
+                orig_table_name = m_mirrorV6TableId[table_stage];
+                m_mirrorTableId[table_stage] = newTable.id;
             }
 
             SWSS_LOG_NOTICE("Created ACL table %s as a sibling of %s",
@@ -2741,11 +2751,11 @@ bool AclOrch::addAclTable(AclTable &newTable)
         // Mark the existence of the mirror table
         if (newTable.type == ACL_TABLE_MIRROR)
         {
-            m_mirrorTableId = table_id;
+            m_mirrorTableId[table_stage] = table_id;
         }
         else if (newTable.type == ACL_TABLE_MIRRORV6)
         {
-            m_mirrorV6TableId = table_id;
+            m_mirrorV6TableId[table_stage] = table_id;
         }
 
         return true;
@@ -2774,29 +2784,30 @@ bool AclOrch::removeAclTable(string table_id)
 
     if (deleteUnbindAclTable(table_oid) == SAI_STATUS_SUCCESS)
     {
-        sai_acl_stage_t stage = (m_AclTables[table_oid].stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS;
-        gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, stage, SAI_ACL_BIND_POINT_TYPE_PORT, table_oid);
+        auto stage = m_AclTables[table_oid].stage;
+        sai_acl_stage_t sai_stage = (stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS;
+        gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, sai_stage, SAI_ACL_BIND_POINT_TYPE_PORT, table_oid);
 
         SWSS_LOG_NOTICE("Successfully deleted ACL table %s", table_id.c_str());
         m_AclTables.erase(table_oid);
 
         // Clear mirror table information
         // If the v4 and v6 ACL mirror tables are combined together,
         // remove both of them.
-        if (table_id == m_mirrorTableId)
+        if (m_mirrorTableId[stage] == table_id)
         {
-            m_mirrorTableId.clear();
+            m_mirrorTableId[stage].clear();
             if (m_isCombinedMirrorV6Table)
             {
-                m_mirrorV6TableId.clear();
+                m_mirrorV6TableId[stage].clear();
             }
         }
-        else if (table_id == m_mirrorV6TableId)
+        else if (m_mirrorV6TableId[stage] == table_id)
         {
-            m_mirrorV6TableId.clear();
+            m_mirrorV6TableId[stage].clear();
             if (m_isCombinedMirrorV6Table)
             {
-                m_mirrorTableId.clear();
+                m_mirrorTableId[stage].clear();
             }
         }
 
@@ -3042,9 +3053,10 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
             }
 
             auto type = m_AclTables[table_oid].type;
+            auto stage = m_AclTables[table_oid].stage;
             if (type == ACL_TABLE_MIRROR || type == ACL_TABLE_MIRRORV6)
             {
-                type = table_id == m_mirrorTableId ? ACL_TABLE_MIRROR : ACL_TABLE_MIRRORV6;
+                type = table_id == m_mirrorTableId[stage] ? ACL_TABLE_MIRROR : ACL_TABLE_MIRRORV6;
             }
 
 
@@ -3212,18 +3224,20 @@ sai_object_id_t AclOrch::getTableById(string table_id)
     }
 
     // Check if the table is a mirror table and a sibling mirror table is created
-    if (m_isCombinedMirrorV6Table &&
-            (table_id == m_mirrorTableId || table_id == m_mirrorV6TableId))
-    {
-        // If the table is v4, the corresponding v6 table is already created
-        if (table_id == m_mirrorTableId)
-        {
-            return getTableById(m_mirrorV6TableId);
-        }
-        // If the table is v6, the corresponding v4 table is already created
-        else
+    for (auto stage: {ACL_STAGE_INGRESS, ACL_STAGE_EGRESS}) {
+        if (m_isCombinedMirrorV6Table &&
+                (table_id == m_mirrorTableId[stage] || table_id == m_mirrorV6TableId[stage]))
         {
-            return getTableById(m_mirrorTableId);
+            // If the table is v4, the corresponding v6 table is already created
+            if (table_id == m_mirrorTableId[stage])
+            {
+                return getTableById(m_mirrorV6TableId[stage]);
+            }
+            // If the table is v6, the corresponding v4 table is already created
+            else
+            {
+                return getTableById(m_mirrorTableId[stage]);
+            }
         }
     }
 
diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h
@@ -477,8 +477,8 @@ class AclOrch : public Orch, public Observer
     static DBConnector m_db;
     static Table m_countersTable;
 
-    string m_mirrorTableId;
-    string m_mirrorV6TableId;
+    map<acl_stage_type_t, string> m_mirrorTableId;
+    map<acl_stage_type_t, string> m_mirrorV6TableId;
 
     acl_capabilities_t m_aclCapabilities;
     acl_action_enum_values_capabilities_t m_aclEnumActionCapabilities;
diff --git a/tests/test_mirror_ipv6_separate.py b/tests/test_mirror_ipv6_separate.py
@@ -99,11 +99,15 @@ def get_mirror_session_state(self, name):
         assert len(fvs) > 0
         return { fv[0]: fv[1] for fv in fvs }
 
-    def create_acl_table(self, table, interfaces, type):
+    def create_acl_table(self, table, interfaces, type, stage=None):
+        attrs = [("policy_desc", "mirror_test"),
+                 ("type", type),
+                 ("ports", ",".join(interfaces))]
+        if stage:
+            attrs.append(("stage", stage))
+
         tbl = swsscommon.Table(self.cdb, "ACL_TABLE")
-        fvs = swsscommon.FieldValuePairs([("policy_desc", "mirror_test"),
-                                          ("type", type),
-                                          ("ports", ",".join(interfaces))])
+        fvs = swsscommon.FieldValuePairs(attrs)
         tbl.set(table, fvs)
         time.sleep(1)
 
@@ -268,6 +272,56 @@ def test_MirrorV6TableCreation(self, dvs, testlog):
         # Delete the V6 table
         self.remove_acl_table(acl_table_v6)
 
+    def test_CreateMirrorIngressAndEgress(self, dvs, testlog):
+        self.setup_db(dvs)
+        asic_db = dvs.get_asic_db()
+
+        ingress_table = "INGRESS_TABLE"
+        duplicate_ingress_table = "INGRESS_TABLE_2"
+        ports = ["Ethernet0", "Ethernet4"]
+
+        # Create the table
+        self.create_acl_table(ingress_table, ports, "MIRROR")
+
+        # Check that the table has been created
+        table_ids = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", 
+                                            len(asic_db.default_acl_tables) + 1)
+        table_entries = [oid for oid in table_ids if oid not in asic_db.default_acl_tables]
+        original_entry = table_entries[0]
+
+        # Attempt to create another MIRROR table with ingress ACLs
+        self.create_acl_table(duplicate_ingress_table, ports, "MIRROR")
+
+        # Check that there is still only one table, and that it is the original table
+        table_ids = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", 
+                                            len(asic_db.default_acl_tables) + 1)
+        table_entries = [oid for oid in table_ids if oid not in asic_db.default_acl_tables]
+        assert table_entries[0] == original_entry
+
+        egress_table = "EGRESS_TABLE"
+        duplicate_egress_table = "EGRESS_TABLE_2"
+
+        # Create the egress table
+        self.create_acl_table(egress_table, ports, "MIRROR", "egress")
+
+        # Check that there are two tables
+        asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", 
+                                len(asic_db.default_acl_tables) + 2)
+
+        # Attempt to create another MIRROR table with egress ACLs
+        self.create_acl_table(duplicate_egress_table, ports, "MIRROR", "egress")
+
+        # Check that there are still only two tables
+        asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", 
+                                len(asic_db.default_acl_tables) + 2)
+
+        self.remove_acl_table(ingress_table)
+        self.remove_acl_table(egress_table)
+        self.remove_acl_table(duplicate_ingress_table)
+        self.remove_acl_table(duplicate_egress_table)
+
+        asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", len(asic_db.default_acl_tables))
+
     # Test case - create a MIRROR table and a MIRRORV6 table in separated mode
     # 0. predefine the VS platform: mellanox platform
     # 1. create a mirror session

Original file line number	Diff line number	Diff line change
`@@ -2213,6 +2213,12 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr`
`2213`	`2213`
`2214`	`2214`	`queryAclActionCapability();`
`2215`	`2215`
	`2216`	`+ for (auto stage: {ACL_STAGE_INGRESS, ACL_STAGE_EGRESS})`
	`2217`	`+ {`
	`2218`	`+ m_mirrorTableId[stage] = "";`
	`2219`	`+ m_mirrorV6TableId[stage] = "";`
	`2220`	`+ }`
	`2221`	`+`
`2216`	`2222`	`// Attach observers`
`2217`	`2223`	`m_mirrorOrch->attach(this);`
`2218`	`2224`	`gPortsOrch->attach(this);`
`@@ -2666,6 +2672,7 @@ bool AclOrch::addAclTable(AclTable &newTable)`
`2666`	`2672`	`}`
`2667`	`2673`
`2668`	`2674`	`sai_object_id_t table_oid = getTableById(table_id);`
	`2675`	`+ auto table_stage = newTable.stage;`
`2669`	`2676`
`2670`	`2677`	`if (table_oid != SAI_NULL_OBJECT_ID)`
`2671`	`2678`	`{`
`@@ -2685,19 +2692,23 @@ bool AclOrch::addAclTable(AclTable &newTable)`
`2685`	`2692`	`if (table_type == ACL_TABLE_MIRROR \|\| table_type == ACL_TABLE_MIRRORV6)`
`2686`	`2693`	`{`
`2687`	`2694`	`string mirror_type;`
`2688`		`- if ((table_type == ACL_TABLE_MIRROR && !m_mirrorTableId.empty()))`
	`2695`	`+ if (table_type == ACL_TABLE_MIRROR && !m_mirrorTableId[table_stage].empty())`
`2689`	`2696`	`{`
`2690`	`2697`	`mirror_type = TABLE_TYPE_MIRROR;`
`2691`	`2698`	`}`
`2692`	`2699`
`2693`		`- if (table_type == ACL_TABLE_MIRRORV6 && !m_mirrorV6TableId.empty())`
	`2700`	`+ if (table_type == ACL_TABLE_MIRRORV6 && !m_mirrorV6TableId[table_stage].empty())`
`2694`	`2701`	`{`
`2695`	`2702`	`mirror_type = TABLE_TYPE_MIRRORV6;`
`2696`	`2703`	`}`
`2697`	`2704`
`2698`	`2705`	`if (!mirror_type.empty())`
`2699`	`2706`	`{`
`2700`		`- SWSS_LOG_ERROR("Mirror table %s has already been created", mirror_type.c_str());`
	`2707`	`+ string stage_str = table_stage == ACL_STAGE_INGRESS ? "INGRESS" : "EGRESS";`
	`2708`	`+ SWSS_LOG_ERROR(`
	`2709`	`+ "Mirror table %s (%s) has already been created",`
	`2710`	`+ mirror_type.c_str(),`
	`2711`	`+ stage_str.c_str());`
`2701`	`2712`	`return false;`
`2702`	`2713`	`}`
`2703`	`2714`	`}`
`@@ -2706,23 +2717,22 @@ bool AclOrch::addAclTable(AclTable &newTable)`
`2706`	`2717`	`// Check if a separate mirror table is needed or not based on the platform`
`2707`	`2718`	`if (newTable.type == ACL_TABLE_MIRROR \|\| newTable.type == ACL_TABLE_MIRRORV6)`
`2708`	`2719`	`{`
`2709`		`-`
`2710`	`2720`	`if (m_isCombinedMirrorV6Table &&`
`2711`		`- (!m_mirrorTableId.empty() \|\| !m_mirrorV6TableId.empty())) {`
`2712`		`-`
	`2721`	`+ (!m_mirrorTableId[table_stage].empty() \|\|`
	`2722`	`+ !m_mirrorV6TableId[table_stage].empty())) {`
`2713`	`2723`	`string orig_table_name;`
`2714`	`2724`
`2715`	`2725`	`// If v4 table is created, mark v6 table is created`
`2716`		`- if (!m_mirrorTableId.empty())`
	`2726`	`+ if (!m_mirrorTableId[table_stage].empty())`
`2717`	`2727`	`{`
`2718`		`- orig_table_name = m_mirrorTableId;`
`2719`		`- m_mirrorV6TableId = newTable.id;`
	`2728`	`+ orig_table_name = m_mirrorTableId[table_stage];`
	`2729`	`+ m_mirrorV6TableId[table_stage] = newTable.id;`
`2720`	`2730`	`}`
`2721`	`2731`	`// If v6 table is created, mark v4 table is created`
`2722`	`2732`	`else`
`2723`	`2733`	`{`
`2724`		`- orig_table_name = m_mirrorV6TableId;`
`2725`		`- m_mirrorTableId = newTable.id;`
	`2734`	`+ orig_table_name = m_mirrorV6TableId[table_stage];`
	`2735`	`+ m_mirrorTableId[table_stage] = newTable.id;`
`2726`	`2736`	`}`
`2727`	`2737`
`2728`	`2738`	`SWSS_LOG_NOTICE("Created ACL table %s as a sibling of %s",`
`@@ -2741,11 +2751,11 @@ bool AclOrch::addAclTable(AclTable &newTable)`
`2741`	`2751`	`// Mark the existence of the mirror table`
`2742`	`2752`	`if (newTable.type == ACL_TABLE_MIRROR)`
`2743`	`2753`	`{`
`2744`		`- m_mirrorTableId = table_id;`
	`2754`	`+ m_mirrorTableId[table_stage] = table_id;`
`2745`	`2755`	`}`
`2746`	`2756`	`else if (newTable.type == ACL_TABLE_MIRRORV6)`
`2747`	`2757`	`{`
`2748`		`- m_mirrorV6TableId = table_id;`
	`2758`	`+ m_mirrorV6TableId[table_stage] = table_id;`
`2749`	`2759`	`}`
`2750`	`2760`
`2751`	`2761`	`return true;`
`@@ -2774,29 +2784,30 @@ bool AclOrch::removeAclTable(string table_id)`
`2774`	`2784`
`2775`	`2785`	`if (deleteUnbindAclTable(table_oid) == SAI_STATUS_SUCCESS)`
`2776`	`2786`	`{`
`2777`		`- sai_acl_stage_t stage = (m_AclTables[table_oid].stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS;`
`2778`		`- gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, stage, SAI_ACL_BIND_POINT_TYPE_PORT, table_oid);`
	`2787`	`+ auto stage = m_AclTables[table_oid].stage;`
	`2788`	`+ sai_acl_stage_t sai_stage = (stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS;`
	`2789`	`+ gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, sai_stage, SAI_ACL_BIND_POINT_TYPE_PORT, table_oid);`
`2779`	`2790`
`2780`	`2791`	`SWSS_LOG_NOTICE("Successfully deleted ACL table %s", table_id.c_str());`
`2781`	`2792`	`m_AclTables.erase(table_oid);`
`2782`	`2793`
`2783`	`2794`	`// Clear mirror table information`
`2784`	`2795`	`// If the v4 and v6 ACL mirror tables are combined together,`
`2785`	`2796`	`// remove both of them.`
`2786`		`- if (table_id == m_mirrorTableId)`
	`2797`	`+ if (m_mirrorTableId[stage] == table_id)`
`2787`	`2798`	`{`
`2788`		`- m_mirrorTableId.clear();`
	`2799`	`+ m_mirrorTableId[stage].clear();`
`2789`	`2800`	`if (m_isCombinedMirrorV6Table)`
`2790`	`2801`	`{`
`2791`		`- m_mirrorV6TableId.clear();`
	`2802`	`+ m_mirrorV6TableId[stage].clear();`
`2792`	`2803`	`}`
`2793`	`2804`	`}`
`2794`		`- else if (table_id == m_mirrorV6TableId)`
	`2805`	`+ else if (m_mirrorV6TableId[stage] == table_id)`
`2795`	`2806`	`{`
`2796`		`- m_mirrorV6TableId.clear();`
	`2807`	`+ m_mirrorV6TableId[stage].clear();`
`2797`	`2808`	`if (m_isCombinedMirrorV6Table)`
`2798`	`2809`	`{`
`2799`		`- m_mirrorTableId.clear();`
	`2810`	`+ m_mirrorTableId[stage].clear();`
`2800`	`2811`	`}`
`2801`	`2812`	`}`
`2802`	`2813`
`@@ -3042,9 +3053,10 @@ void AclOrch::doAclRuleTask(Consumer &consumer)`
`3042`	`3053`	`}`
`3043`	`3054`
`3044`	`3055`	`auto type = m_AclTables[table_oid].type;`
	`3056`	`+ auto stage = m_AclTables[table_oid].stage;`
`3045`	`3057`	`if (type == ACL_TABLE_MIRROR \|\| type == ACL_TABLE_MIRRORV6)`
`3046`	`3058`	`{`
`3047`		`- type = table_id == m_mirrorTableId ? ACL_TABLE_MIRROR : ACL_TABLE_MIRRORV6;`
	`3059`	`+ type = table_id == m_mirrorTableId[stage] ? ACL_TABLE_MIRROR : ACL_TABLE_MIRRORV6;`
`3048`	`3060`	`}`
`3049`	`3061`
`3050`	`3062`
`@@ -3212,18 +3224,20 @@ sai_object_id_t AclOrch::getTableById(string table_id)`
`3212`	`3224`	`}`
`3213`	`3225`
`3214`	`3226`	`// Check if the table is a mirror table and a sibling mirror table is created`
`3215`		`- if (m_isCombinedMirrorV6Table &&`
`3216`		`- (table_id == m_mirrorTableId \|\| table_id == m_mirrorV6TableId))`
`3217`		`- {`
`3218`		`- // If the table is v4, the corresponding v6 table is already created`
`3219`		`- if (table_id == m_mirrorTableId)`
`3220`		`- {`
`3221`		`- return getTableById(m_mirrorV6TableId);`
`3222`		`- }`
`3223`		`- // If the table is v6, the corresponding v4 table is already created`
`3224`		`- else`
	`3227`	`+ for (auto stage: {ACL_STAGE_INGRESS, ACL_STAGE_EGRESS}) {`
	`3228`	`+ if (m_isCombinedMirrorV6Table &&`
	`3229`	`+ (table_id == m_mirrorTableId[stage] \|\| table_id == m_mirrorV6TableId[stage]))`
`3225`	`3230`	`{`
`3226`		`- return getTableById(m_mirrorTableId);`
	`3231`	`+ // If the table is v4, the corresponding v6 table is already created`
	`3232`	`+ if (table_id == m_mirrorTableId[stage])`
	`3233`	`+ {`
	`3234`	`+ return getTableById(m_mirrorV6TableId[stage]);`
	`3235`	`+ }`
	`3236`	`+ // If the table is v6, the corresponding v4 table is already created`
	`3237`	`+ else`
	`3238`	`+ {`
	`3239`	`+ return getTableById(m_mirrorTableId[stage]);`
	`3240`	`+ }`
`3227`	`3241`	`}`
`3228`	`3242`	`}`
`3229`	`3243`