From 60698a27b7816662b301ee1a99314fcdbb6e04c4 Mon Sep 17 00:00:00 2001 From: "wenda.ni" Date: Mon, 31 May 2021 17:51:25 +0800 Subject: [PATCH 1/2] Add vs test for testing creation sequence Signed-off-by: Wenda Ni --- tests/test_mirror.py | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/tests/test_mirror.py b/tests/test_mirror.py index 472d33fef2..d0e7380331 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -790,12 +790,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" @@ -805,16 +804,18 @@ 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"]) @@ -822,10 +823,25 @@ def test_AclBindMirror(self, dvs, testlog): # 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 @@ -873,6 +889,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 From 262785b9473c2264068b999920a2362b1cdb0cd0 Mon Sep 17 00:00:00 2001 From: "wenda.ni" Date: Mon, 31 May 2021 17:53:53 +0800 Subject: [PATCH 2/2] Fix mirror session ref count at acl rule attachement Signed-off-by: wenda.ni --- orchagent/aclorch.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 01b6a8e0bc..7f2ccfb50c 100644 --- 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;