Skip to content

Commit 0878d5e

Browse files
author
Vasant
committed
Addressed code-review comments and LGTM errors
1 parent 6fce5d3 commit 0878d5e

File tree

4 files changed

+87
-128
lines changed

4 files changed

+87
-128
lines changed

‎tests/conftest.py

+7-1
Original file line numberDiff line numberDiff line change
@@ -989,7 +989,6 @@ def get_dvs_acl(self):
989989
self.get_counters_db())
990990
return self.dvs_acl
991991

992-
993992
@pytest.yield_fixture(scope="module")
994993
def dvs(request):
995994
name = request.config.getoption("--dvsname")
@@ -1010,6 +1009,13 @@ def testlog(request, dvs):
10101009
yield testlog
10111010
dvs.runcmd("logger === finish test %s ===" % request.node.name)
10121011

1012+
@pytest.yield_fixture(scope="class")
1013+
def dvs_acl_manager(request, dvs):
1014+
request.cls.dvs_acl = dvs_acl.DVSAcl(dvs.get_asic_db(),
1015+
dvs.get_config_db(),
1016+
dvs.get_state_db(),
1017+
dvs.get_counters_db())
1018+
10131019
##################### DPB fixtures ###########################################
10141020
@pytest.yield_fixture(scope="module")
10151021
def create_dpb_config_file(dvs):

‎tests/dvslib/dvs_acl.py

+1-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
import time
2-
from swsscommon import swsscommon
3-
41
class DVSAcl(object):
52
def __init__(self, adb, cdb, sdb, cntrdb):
63
self.asic_db = adb
@@ -47,7 +44,7 @@ def get_acl_table_id(self):
4744
acl_tables = self.get_acl_table_ids()
4845
return acl_tables[0]
4946

50-
def verify_acl_tables(self, expt):
47+
def verify_acl_table_count(self, expt):
5148
num_keys = len(self.asic_db.default_acl_tables) + expt
5249
keys = self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", num_keys)
5350
for k in self.asic_db.default_acl_tables:
@@ -57,11 +54,6 @@ def verify_acl_tables(self, expt):
5754

5855
assert len(acl_tables) == expt
5956

60-
def verify_no_acl_tables(self):
61-
num_keys = len(self.asic_db.default_acl_tables)
62-
keys = self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", num_keys)
63-
assert set(keys) == set(self.asic_db.default_acl_tables)
64-
6557
def verify_acl_group_num(self, expt):
6658
acl_table_groups = self.get_acl_table_group_ids(expt)
6759

‎tests/test_acl.py

+12-41
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
import time
2+
import pytest
23

3-
class BaseTestAcl(object):
4-
def setup_dvs_acl(self, dvs):
5-
self.dvs_acl = dvs.get_dvs_acl()
6-
7-
class TestAcl(BaseTestAcl):
4+
@pytest.mark.usefixtures('dvs_acl_manager')
5+
class TestAcl():
86
def test_AclTableCreation(self, dvs):
9-
self.setup_dvs_acl(dvs)
107

118
bind_ports = ["Ethernet0", "Ethernet4"]
129
self.dvs_acl.create_acl_table("test", "L3", bind_ports)
@@ -16,9 +13,7 @@ def test_AclTableCreation(self, dvs):
1613
self.dvs_acl.verify_acl_group_member(acl_group_ids, self.dvs_acl.get_acl_table_id())
1714
self.dvs_acl.verify_acl_port_binding(bind_ports)
1815

19-
#fails
2016
def test_AclRuleL4SrcPort(self, dvs):
21-
self.setup_dvs_acl(dvs)
2217

2318
config_qualifiers = {"L4_SRC_PORT": "65000"}
2419
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_L4_SRC_PORT": self.dvs_acl.get_simple_qualifier_comparator("65000&mask:0xffff")}
@@ -29,9 +24,7 @@ def test_AclRuleL4SrcPort(self, dvs):
2924
self.dvs_acl.remove_acl_rule("test", "acl_test_rule")
3025
self.dvs_acl.verify_no_acl_rules()
3126

32-
#fails
3327
def test_AclRuleInOutPorts(self, dvs):
34-
self.setup_dvs_acl(dvs)
3528

3629
config_qualifiers = {
3730
"IN_PORTS": "Ethernet0,Ethernet4",
@@ -50,7 +43,6 @@ def test_AclRuleInOutPorts(self, dvs):
5043
self.dvs_acl.verify_no_acl_rules()
5144

5245
def test_AclRuleInPortsNonExistingInterface(self, dvs):
53-
self.setup_dvs_acl(dvs)
5446

5547
config_qualifiers = {
5648
"IN_PORTS": "FOO_BAR_BAZ"
@@ -62,7 +54,6 @@ def test_AclRuleInPortsNonExistingInterface(self, dvs):
6254
self.dvs_acl.remove_acl_rule("test", "acl_test_rule")
6355

6456
def test_AclRuleOutPortsNonExistingInterface(self, dvs):
65-
self.setup_dvs_acl(dvs)
6657

6758
config_qualifiers = {
6859
"OUT_PORTS": "FOO_BAR_BAZ"
@@ -74,13 +65,11 @@ def test_AclRuleOutPortsNonExistingInterface(self, dvs):
7465
self.dvs_acl.remove_acl_rule("test", "acl_test_rule")
7566

7667
def test_AclTableDeletion(self, dvs):
77-
self.setup_dvs_acl(dvs)
7868

7969
self.dvs_acl.remove_acl_table("test")
80-
self.dvs_acl.verify_no_acl_tables()
70+
self.dvs_acl.verify_acl_table_count(0)
8171

8272
def test_V6AclTableCreation(self, dvs):
83-
self.setup_dvs_acl(dvs)
8473

8574
bind_ports = ["Ethernet0", "Ethernet4", "Ethernet8"]
8675
self.dvs_acl.create_acl_table("test_aclv6", "L3V6", bind_ports)
@@ -90,9 +79,7 @@ def test_V6AclTableCreation(self, dvs):
9079
self.dvs_acl.verify_acl_group_member(acl_group_ids, self.dvs_acl.get_acl_table_id())
9180
self.dvs_acl.verify_acl_port_binding(bind_ports)
9281

93-
# fails
9482
def test_V6AclRuleIPv6Any(self, dvs):
95-
self.setup_dvs_acl(dvs)
9683

9784
config_qualifiers = {"IP_TYPE": "IPv6ANY"}
9885
expected_sai_qualifiers = {
@@ -105,9 +92,7 @@ def test_V6AclRuleIPv6Any(self, dvs):
10592
self.dvs_acl.remove_acl_rule("test_aclv6", "acl_test_rule")
10693
self.dvs_acl.verify_no_acl_rules()
10794

108-
# fails
10995
def test_V6AclRuleIPv6AnyDrop(self, dvs):
110-
self.setup_dvs_acl(dvs)
11196

11297
config_qualifiers = {"IP_TYPE": "IPv6ANY"}
11398
expected_sai_qualifiers = {
@@ -121,7 +106,6 @@ def test_V6AclRuleIPv6AnyDrop(self, dvs):
121106
self.dvs_acl.verify_no_acl_rules()
122107

123108
def test_V6AclRuleIpProtocol(self, dvs):
124-
self.setup_dvs_acl(dvs)
125109

126110
config_qualifiers = {"IP_PROTOCOL": "6"}
127111
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_IP_PROTOCOL": self.dvs_acl.get_simple_qualifier_comparator("6&mask:0xff")}
@@ -133,7 +117,6 @@ def test_V6AclRuleIpProtocol(self, dvs):
133117
self.dvs_acl.verify_no_acl_rules()
134118

135119
def test_V6AclRuleSrcIPv6(self, dvs):
136-
self.setup_dvs_acl(dvs)
137120

138121
config_qualifiers = {"SRC_IPV6": "2777::0/64"}
139122
expected_sai_qualifiers = {
@@ -147,7 +130,6 @@ def test_V6AclRuleSrcIPv6(self, dvs):
147130
self.dvs_acl.verify_no_acl_rules()
148131

149132
def test_V6AclRuleDstIPv6(self, dvs):
150-
self.setup_dvs_acl(dvs)
151133

152134
config_qualifiers = {"DST_IPV6": "2002::2/128"}
153135
expected_sai_qualifiers = {
@@ -161,7 +143,6 @@ def test_V6AclRuleDstIPv6(self, dvs):
161143
self.dvs_acl.verify_no_acl_rules()
162144

163145
def test_V6AclRuleL4SrcPort(self, dvs):
164-
self.setup_dvs_acl(dvs)
165146

166147
config_qualifiers = {"L4_SRC_PORT": "65000"}
167148
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_L4_SRC_PORT": self.dvs_acl.get_simple_qualifier_comparator("65000&mask:0xffff")}
@@ -173,7 +154,6 @@ def test_V6AclRuleL4SrcPort(self, dvs):
173154
self.dvs_acl.verify_no_acl_rules()
174155

175156
def test_V6AclRuleL4DstPort(self, dvs):
176-
self.setup_dvs_acl(dvs)
177157

178158
config_qualifiers = {"L4_DST_PORT": "65001"}
179159
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT": self.dvs_acl.get_simple_qualifier_comparator("65001&mask:0xffff")}
@@ -185,7 +165,6 @@ def test_V6AclRuleL4DstPort(self, dvs):
185165
self.dvs_acl.verify_no_acl_rules()
186166

187167
def test_V6AclRuleTCPFlags(self, dvs):
188-
self.setup_dvs_acl(dvs)
189168

190169
config_qualifiers = {"TCP_FLAGS": "0x07/0x3f"}
191170
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_TCP_FLAGS": self.dvs_acl.get_simple_qualifier_comparator("7&mask:0x3f")}
@@ -197,7 +176,6 @@ def test_V6AclRuleTCPFlags(self, dvs):
197176
self.dvs_acl.verify_no_acl_rules()
198177

199178
def test_V6AclRuleL4SrcPortRange(self, dvs):
200-
self.setup_dvs_acl(dvs)
201179

202180
config_qualifiers = {"L4_SRC_PORT_RANGE": "1-100"}
203181
expected_sai_qualifiers = {
@@ -211,7 +189,6 @@ def test_V6AclRuleL4SrcPortRange(self, dvs):
211189
self.dvs_acl.verify_no_acl_rules()
212190

213191
def test_V6AclRuleL4DstPortRange(self, dvs):
214-
self.setup_dvs_acl(dvs)
215192

216193
config_qualifiers = {"L4_DST_PORT_RANGE": "101-200"}
217194
expected_sai_qualifiers = {
@@ -225,13 +202,11 @@ def test_V6AclRuleL4DstPortRange(self, dvs):
225202
self.dvs_acl.verify_no_acl_rules()
226203

227204
def test_V6AclTableDeletion(self, dvs):
228-
self.setup_dvs_acl(dvs)
229205

230206
self.dvs_acl.remove_acl_table("test_aclv6")
231-
self.dvs_acl.verify_no_acl_tables()
207+
self.dvs_acl.verify_acl_table_count(0)
232208

233209
def test_InsertAclRuleBetweenPriorities(self, dvs):
234-
self.setup_dvs_acl(dvs)
235210

236211
bind_ports = ["Ethernet0", "Ethernet4"]
237212
self.dvs_acl.create_acl_table("test_priorities", "L3", bind_ports)
@@ -283,10 +258,9 @@ def test_InsertAclRuleBetweenPriorities(self, dvs):
283258
self.dvs_acl.verify_no_acl_rules()
284259

285260
self.dvs_acl.remove_acl_table("test_priorities")
286-
self.dvs_acl.verify_no_acl_tables()
261+
self.dvs_acl.verify_acl_table_count(0)
287262

288263
def test_RulesWithDiffMaskLengths(self, dvs):
289-
self.setup_dvs_acl(dvs)
290264

291265
bind_ports = ["Ethernet0", "Ethernet4"]
292266
self.dvs_acl.create_acl_table("test_masks", "L3", bind_ports)
@@ -331,10 +305,9 @@ def test_RulesWithDiffMaskLengths(self, dvs):
331305
self.dvs_acl.verify_no_acl_rules()
332306

333307
self.dvs_acl.remove_acl_table("test_masks")
334-
self.dvs_acl.verify_no_acl_tables()
308+
self.dvs_acl.verify_acl_table_count(0)
335309

336310
def test_AclRuleIcmp(self, dvs):
337-
self.setup_dvs_acl(dvs)
338311

339312
bind_ports = ["Ethernet0", "Ethernet4"]
340313
self.dvs_acl.create_acl_table("test_icmp", "L3", bind_ports)
@@ -356,10 +329,9 @@ def test_AclRuleIcmp(self, dvs):
356329
self.dvs_acl.verify_no_acl_rules()
357330

358331
self.dvs_acl.remove_acl_table("test_icmp")
359-
self.dvs_acl.verify_no_acl_tables()
332+
self.dvs_acl.verify_acl_table_count(0)
360333

361334
def test_AclRuleIcmpV6(self, dvs):
362-
self.setup_dvs_acl(dvs)
363335

364336
bind_ports = ["Ethernet0", "Ethernet4"]
365337
self.dvs_acl.create_acl_table("test_icmpv6", "L3V6", bind_ports)
@@ -381,13 +353,12 @@ def test_AclRuleIcmpV6(self, dvs):
381353
self.dvs_acl.verify_no_acl_rules()
382354

383355
self.dvs_acl.remove_acl_table("test_icmpv6")
384-
self.dvs_acl.verify_no_acl_tables()
356+
self.dvs_acl.verify_acl_table_count(0)
385357

386358
def test_AclRuleRedirectToNextHop(self, dvs):
387359
# NOTE: set_interface_status has a dependency on cdb within dvs,
388360
# so we still need to setup the db. This should be refactored.
389361
dvs.setup_db()
390-
self.setup_dvs_acl(dvs)
391362

392363
# Bring up an IP interface with a neighbor
393364
dvs.set_interface_status("Ethernet4", "up")
@@ -413,15 +384,16 @@ def test_AclRuleRedirectToNextHop(self, dvs):
413384
self.dvs_acl.verify_no_acl_rules()
414385

415386
self.dvs_acl.remove_acl_table("test_redirect")
416-
self.dvs_acl.verify_no_acl_tables()
387+
self.dvs_acl.verify_acl_table_count(0)
417388

418389
# Clean up the IP interface and neighbor
419390
dvs.remove_neighbor("Ethernet4", "10.0.0.2")
420391
dvs.remove_ip_address("Ethernet4", "10.0.0.1/24")
421392
dvs.set_interface_status("Ethernet4", "down")
422393

423394

424-
class TestAclRuleValidation(BaseTestAcl):
395+
@pytest.mark.usefixtures('dvs_acl_manager')
396+
class TestAclRuleValidation():
425397
"""
426398
Test class for cases that check if orchagent corectly validates
427399
ACL rules input
@@ -450,7 +422,6 @@ def test_AclActionValidation(self, dvs):
450422
to check the case when orchagent refuses to process rules with action that is not
451423
supported by the ASIC.
452424
"""
453-
self.setup_dvs_acl(dvs)
454425

455426
stage_name_map = {
456427
"ingress": "SAI_SWITCH_ATTR_ACL_STAGE_INGRESS",

0 commit comments

Comments
 (0)