From 52591a6cedfb14abfcfa0067eef0f2f85461fc1a Mon Sep 17 00:00:00 2001 From: bingwang Date: Thu, 2 Sep 2021 20:04:42 -0700 Subject: [PATCH 1/2] Cherry-pick PR 1761 after resolving conflict Signed-off-by: bingwang --- orchagent/aclorch.cpp | 12 +++++------ tests/test_mirror.py | 47 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index d347567bf0..9df574b7eb 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1089,13 +1089,6 @@ bool AclRuleMirror::create() throw runtime_error("Failed to get mirror session state"); } - // Increase session reference count regardless of state to deny - // attempt to remove mirror session with attached ACL rules. - if (!m_pMirrorOrch->increaseRefCount(m_sessionName)) - { - throw runtime_error("Failed to increase mirror session reference count"); - } - if (!state) { return true; @@ -1118,6 +1111,11 @@ bool AclRuleMirror::create() return false; } + if (!m_pMirrorOrch->increaseRefCount(m_sessionName)) + { + throw runtime_error("Failed to increase mirror session reference count for session"); + } + m_state = true; return true; diff --git a/tests/test_mirror.py b/tests/test_mirror.py index 38c94f6a66..56869871de 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -707,12 +707,11 @@ def remove_mirror_acl_dscp_rule(self, table, rule): time.sleep(1) - 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" @@ -722,16 +721,18 @@ def test_AclBindMirror(self, dvs, testlog): self.set_interface_status("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"]) @@ -739,10 +740,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 @@ -789,3 +805,16 @@ def test_AclBindMirror(self, dvs, testlog): self.remove_neighbor("Ethernet32", "20.0.0.1") self.remove_ip_address("Ethernet32", "20.0.0.0/31") self.set_interface_status("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 +def test_nonflaky_dummy(): + pass + From 71dd816a91e08f6a0cdd0ae8534701a4eee0ea52 Mon Sep 17 00:00:00 2001 From: bingwang Date: Sun, 5 Sep 2021 18:00:34 -0700 Subject: [PATCH 2/2] Fix test issue Signed-off-by: bingwang --- tests/test_mirror.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/test_mirror.py b/tests/test_mirror.py index 56869871de..5ef92b979c 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -132,7 +132,7 @@ def test_MirrorAddRemove(self, dvs, testlog): # add route to mirror destination via 10.0.0.1 self.add_route(dvs, "2.2.2.2", "10.0.0.1") assert self.get_mirror_session_state(session)["status"] == "active" - assert self.get_mirror_session_state(session)["monitor_port"] == dvs.asicdb.portnamemap["Ethernet16"] + assert self.get_mirror_session_state(session)["monitor_port"] == "Ethernet16" assert self.get_mirror_session_state(session)["dst_mac"] == "02:04:06:08:10:12" assert self.get_mirror_session_state(session)["route_prefix"] == "2.2.2.2/32" @@ -812,9 +812,3 @@ def test_AclBindMirror(self, dvs, testlog): 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 -def test_nonflaky_dummy(): - pass -