[acl mirror action] Mirror session ref count fix at acl rule attachme…

…nt (sonic-net#1761) * Fix mirror session ref count at acl rule attachement Signed-off-by: wenda.ni <[email protected]>
theasianpianist · Jul 3, 2021 · d594c47 · d594c47
1 parent d3aa660
commit d594c47
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 16 deletions.
diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp
@@ -1225,13 +1225,6 @@ bool AclRuleMirror::create()
         SWSS_LOG_THROW("Failed to get mirror session state for session %s", m_sessionName.c_str());
     }
 
-    // Increase session reference count regardless of state to deny
-    // attempt to remove mirror session with attached ACL rules.
-    if (!m_pMirrorOrch->increaseRefCount(m_sessionName))
-    {
-        SWSS_LOG_THROW("Failed to increase mirror session reference count for session %s", m_sessionName.c_str());
-    }
-
     if (!state)
     {
         return true;
@@ -1254,6 +1247,11 @@ bool AclRuleMirror::create()
         return false;
     }
 
+    if (!m_pMirrorOrch->increaseRefCount(m_sessionName))
+    {
+        SWSS_LOG_THROW("Failed to increase mirror session reference count for session %s", m_sessionName.c_str());
+    }
+
     m_state = true;
 
     return true;

diff --git a/tests/test_mirror.py b/tests/test_mirror.py
@@ -805,12 +805,11 @@ def test_AclBindMirrorPerStage(self, dvs, testlog):
         self.remove_ip_address("Ethernet32", "20.0.0.0/31")
         self.set_interface_status(dvs, "Ethernet32", "down")
 
-    def test_AclBindMirror(self, dvs, testlog):
+    def _test_AclBindMirror(self, dvs, testlog, create_seq_test=False):
         """
         This test tests ACL associated with mirror session with DSCP value
         The DSCP value is tested on both with mask and without mask
         """
-        self.setup_db(dvs)
 
         session = "MIRROR_SESSION"
         acl_table = "MIRROR_TABLE"
@@ -820,27 +819,44 @@ def test_AclBindMirror(self, dvs, testlog):
         self.set_interface_status(dvs, "Ethernet32", "up")
         self.add_ip_address("Ethernet32", "20.0.0.0/31")
         self.add_neighbor("Ethernet32", "20.0.0.1", "02:04:06:08:10:12")
-        self.add_route(dvs, "4.4.4.4", "20.0.0.1")
+        if create_seq_test == False:
+            self.add_route(dvs, "4.4.4.4", "20.0.0.1")
 
         # create mirror session
         self.create_mirror_session(session, "3.3.3.3", "4.4.4.4", "0x6558", "8", "100", "0")
-        assert self.get_mirror_session_state(session)["status"] == "active"
+        assert self.get_mirror_session_state(session)["status"] == ("active" if create_seq_test == False else "inactive")
 
-        # assert mirror session in asic database
+        # check mirror session in asic database
         tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION")
-        assert len(tbl.getKeys()) == 1
-        mirror_session_oid = tbl.getKeys()[0]
+        assert len(tbl.getKeys()) == (1 if create_seq_test == False else 0)
+        if create_seq_test == False:
+            mirror_session_oid = tbl.getKeys()[0]
 
         # create acl table
         self.create_acl_table(acl_table, ["Ethernet0", "Ethernet4"])
 
         # create acl rule with dscp value 48
         self.create_mirror_acl_dscp_rule(acl_table, acl_rule, "48", session)
 
-        # assert acl rule is created
+        # acl rule creation check
         tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY")
         rule_entries = [k for k in tbl.getKeys() if k not in dvs.asicdb.default_acl_entries]
-        assert len(rule_entries) == 1
+        assert len(rule_entries) == (1 if create_seq_test == False else 0)
+
+        if create_seq_test == True:
+            self.add_route(dvs, "4.4.4.4", "20.0.0.1")
+
+            assert self.get_mirror_session_state(session)["status"] == "active"
+
+            # assert mirror session in asic database
+            tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION")
+            assert len(tbl.getKeys()) == 1
+            mirror_session_oid = tbl.getKeys()[0]
+
+            # assert acl rule is created
+            tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY")
+            rule_entries = [k for k in tbl.getKeys() if k not in dvs.asicdb.default_acl_entries]
+            assert len(rule_entries) == 1
 
         (status, fvs) = tbl.get(rule_entries[0])
         assert status == True
@@ -888,6 +904,12 @@ def test_AclBindMirror(self, dvs, testlog):
         self.remove_ip_address("Ethernet32", "20.0.0.0/31")
         self.set_interface_status(dvs, "Ethernet32", "down")
 
+    def test_AclBindMirror(self, dvs, testlog):
+        self.setup_db(dvs)
+
+        self._test_AclBindMirror(dvs, testlog)
+        self._test_AclBindMirror(dvs, testlog, create_seq_test=True)
+
 
 # Add Dummy always-pass test at end as workaroud
 # for issue when Flaky fail on final test it invokes module tear-down before retrying