Skip to content

Commit f871bfc

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

File tree

4 files changed

+88
-128
lines changed

4 files changed

+88
-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

+13-41
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import time
2+
import pytest
3+
from pytest import *
24

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

119
bind_ports = ["Ethernet0", "Ethernet4"]
1210
self.dvs_acl.create_acl_table("test", "L3", bind_ports)
@@ -16,9 +14,7 @@ def test_AclTableCreation(self, dvs):
1614
self.dvs_acl.verify_acl_group_member(acl_group_ids, self.dvs_acl.get_acl_table_id())
1715
self.dvs_acl.verify_acl_port_binding(bind_ports)
1816

19-
#fails
2017
def test_AclRuleL4SrcPort(self, dvs):
21-
self.setup_dvs_acl(dvs)
2218

2319
config_qualifiers = {"L4_SRC_PORT": "65000"}
2420
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_L4_SRC_PORT": self.dvs_acl.get_simple_qualifier_comparator("65000&mask:0xffff")}
@@ -29,9 +25,7 @@ def test_AclRuleL4SrcPort(self, dvs):
2925
self.dvs_acl.remove_acl_rule("test", "acl_test_rule")
3026
self.dvs_acl.verify_no_acl_rules()
3127

32-
#fails
3328
def test_AclRuleInOutPorts(self, dvs):
34-
self.setup_dvs_acl(dvs)
3529

3630
config_qualifiers = {
3731
"IN_PORTS": "Ethernet0,Ethernet4",
@@ -50,7 +44,6 @@ def test_AclRuleInOutPorts(self, dvs):
5044
self.dvs_acl.verify_no_acl_rules()
5145

5246
def test_AclRuleInPortsNonExistingInterface(self, dvs):
53-
self.setup_dvs_acl(dvs)
5447

5548
config_qualifiers = {
5649
"IN_PORTS": "FOO_BAR_BAZ"
@@ -62,7 +55,6 @@ def test_AclRuleInPortsNonExistingInterface(self, dvs):
6255
self.dvs_acl.remove_acl_rule("test", "acl_test_rule")
6356

6457
def test_AclRuleOutPortsNonExistingInterface(self, dvs):
65-
self.setup_dvs_acl(dvs)
6658

6759
config_qualifiers = {
6860
"OUT_PORTS": "FOO_BAR_BAZ"
@@ -74,13 +66,11 @@ def test_AclRuleOutPortsNonExistingInterface(self, dvs):
7466
self.dvs_acl.remove_acl_rule("test", "acl_test_rule")
7567

7668
def test_AclTableDeletion(self, dvs):
77-
self.setup_dvs_acl(dvs)
7869

7970
self.dvs_acl.remove_acl_table("test")
80-
self.dvs_acl.verify_no_acl_tables()
71+
self.dvs_acl.verify_acl_table_count(0)
8172

8273
def test_V6AclTableCreation(self, dvs):
83-
self.setup_dvs_acl(dvs)
8474

8575
bind_ports = ["Ethernet0", "Ethernet4", "Ethernet8"]
8676
self.dvs_acl.create_acl_table("test_aclv6", "L3V6", bind_ports)
@@ -90,9 +80,7 @@ def test_V6AclTableCreation(self, dvs):
9080
self.dvs_acl.verify_acl_group_member(acl_group_ids, self.dvs_acl.get_acl_table_id())
9181
self.dvs_acl.verify_acl_port_binding(bind_ports)
9282

93-
# fails
9483
def test_V6AclRuleIPv6Any(self, dvs):
95-
self.setup_dvs_acl(dvs)
9684

9785
config_qualifiers = {"IP_TYPE": "IPv6ANY"}
9886
expected_sai_qualifiers = {
@@ -105,9 +93,7 @@ def test_V6AclRuleIPv6Any(self, dvs):
10593
self.dvs_acl.remove_acl_rule("test_aclv6", "acl_test_rule")
10694
self.dvs_acl.verify_no_acl_rules()
10795

108-
# fails
10996
def test_V6AclRuleIPv6AnyDrop(self, dvs):
110-
self.setup_dvs_acl(dvs)
11197

11298
config_qualifiers = {"IP_TYPE": "IPv6ANY"}
11399
expected_sai_qualifiers = {
@@ -121,7 +107,6 @@ def test_V6AclRuleIPv6AnyDrop(self, dvs):
121107
self.dvs_acl.verify_no_acl_rules()
122108

123109
def test_V6AclRuleIpProtocol(self, dvs):
124-
self.setup_dvs_acl(dvs)
125110

126111
config_qualifiers = {"IP_PROTOCOL": "6"}
127112
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_IP_PROTOCOL": self.dvs_acl.get_simple_qualifier_comparator("6&mask:0xff")}
@@ -133,7 +118,6 @@ def test_V6AclRuleIpProtocol(self, dvs):
133118
self.dvs_acl.verify_no_acl_rules()
134119

135120
def test_V6AclRuleSrcIPv6(self, dvs):
136-
self.setup_dvs_acl(dvs)
137121

138122
config_qualifiers = {"SRC_IPV6": "2777::0/64"}
139123
expected_sai_qualifiers = {
@@ -147,7 +131,6 @@ def test_V6AclRuleSrcIPv6(self, dvs):
147131
self.dvs_acl.verify_no_acl_rules()
148132

149133
def test_V6AclRuleDstIPv6(self, dvs):
150-
self.setup_dvs_acl(dvs)
151134

152135
config_qualifiers = {"DST_IPV6": "2002::2/128"}
153136
expected_sai_qualifiers = {
@@ -161,7 +144,6 @@ def test_V6AclRuleDstIPv6(self, dvs):
161144
self.dvs_acl.verify_no_acl_rules()
162145

163146
def test_V6AclRuleL4SrcPort(self, dvs):
164-
self.setup_dvs_acl(dvs)
165147

166148
config_qualifiers = {"L4_SRC_PORT": "65000"}
167149
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_L4_SRC_PORT": self.dvs_acl.get_simple_qualifier_comparator("65000&mask:0xffff")}
@@ -173,7 +155,6 @@ def test_V6AclRuleL4SrcPort(self, dvs):
173155
self.dvs_acl.verify_no_acl_rules()
174156

175157
def test_V6AclRuleL4DstPort(self, dvs):
176-
self.setup_dvs_acl(dvs)
177158

178159
config_qualifiers = {"L4_DST_PORT": "65001"}
179160
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT": self.dvs_acl.get_simple_qualifier_comparator("65001&mask:0xffff")}
@@ -185,7 +166,6 @@ def test_V6AclRuleL4DstPort(self, dvs):
185166
self.dvs_acl.verify_no_acl_rules()
186167

187168
def test_V6AclRuleTCPFlags(self, dvs):
188-
self.setup_dvs_acl(dvs)
189169

190170
config_qualifiers = {"TCP_FLAGS": "0x07/0x3f"}
191171
expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_TCP_FLAGS": self.dvs_acl.get_simple_qualifier_comparator("7&mask:0x3f")}
@@ -197,7 +177,6 @@ def test_V6AclRuleTCPFlags(self, dvs):
197177
self.dvs_acl.verify_no_acl_rules()
198178

199179
def test_V6AclRuleL4SrcPortRange(self, dvs):
200-
self.setup_dvs_acl(dvs)
201180

202181
config_qualifiers = {"L4_SRC_PORT_RANGE": "1-100"}
203182
expected_sai_qualifiers = {
@@ -211,7 +190,6 @@ def test_V6AclRuleL4SrcPortRange(self, dvs):
211190
self.dvs_acl.verify_no_acl_rules()
212191

213192
def test_V6AclRuleL4DstPortRange(self, dvs):
214-
self.setup_dvs_acl(dvs)
215193

216194
config_qualifiers = {"L4_DST_PORT_RANGE": "101-200"}
217195
expected_sai_qualifiers = {
@@ -225,13 +203,11 @@ def test_V6AclRuleL4DstPortRange(self, dvs):
225203
self.dvs_acl.verify_no_acl_rules()
226204

227205
def test_V6AclTableDeletion(self, dvs):
228-
self.setup_dvs_acl(dvs)
229206

230207
self.dvs_acl.remove_acl_table("test_aclv6")
231-
self.dvs_acl.verify_no_acl_tables()
208+
self.dvs_acl.verify_acl_table_count(0)
232209

233210
def test_InsertAclRuleBetweenPriorities(self, dvs):
234-
self.setup_dvs_acl(dvs)
235211

236212
bind_ports = ["Ethernet0", "Ethernet4"]
237213
self.dvs_acl.create_acl_table("test_priorities", "L3", bind_ports)
@@ -283,10 +259,9 @@ def test_InsertAclRuleBetweenPriorities(self, dvs):
283259
self.dvs_acl.verify_no_acl_rules()
284260

285261
self.dvs_acl.remove_acl_table("test_priorities")
286-
self.dvs_acl.verify_no_acl_tables()
262+
self.dvs_acl.verify_acl_table_count(0)
287263

288264
def test_RulesWithDiffMaskLengths(self, dvs):
289-
self.setup_dvs_acl(dvs)
290265

291266
bind_ports = ["Ethernet0", "Ethernet4"]
292267
self.dvs_acl.create_acl_table("test_masks", "L3", bind_ports)
@@ -331,10 +306,9 @@ def test_RulesWithDiffMaskLengths(self, dvs):
331306
self.dvs_acl.verify_no_acl_rules()
332307

333308
self.dvs_acl.remove_acl_table("test_masks")
334-
self.dvs_acl.verify_no_acl_tables()
309+
self.dvs_acl.verify_acl_table_count(0)
335310

336311
def test_AclRuleIcmp(self, dvs):
337-
self.setup_dvs_acl(dvs)
338312

339313
bind_ports = ["Ethernet0", "Ethernet4"]
340314
self.dvs_acl.create_acl_table("test_icmp", "L3", bind_ports)
@@ -356,10 +330,9 @@ def test_AclRuleIcmp(self, dvs):
356330
self.dvs_acl.verify_no_acl_rules()
357331

358332
self.dvs_acl.remove_acl_table("test_icmp")
359-
self.dvs_acl.verify_no_acl_tables()
333+
self.dvs_acl.verify_acl_table_count(0)
360334

361335
def test_AclRuleIcmpV6(self, dvs):
362-
self.setup_dvs_acl(dvs)
363336

364337
bind_ports = ["Ethernet0", "Ethernet4"]
365338
self.dvs_acl.create_acl_table("test_icmpv6", "L3V6", bind_ports)
@@ -381,13 +354,12 @@ def test_AclRuleIcmpV6(self, dvs):
381354
self.dvs_acl.verify_no_acl_rules()
382355

383356
self.dvs_acl.remove_acl_table("test_icmpv6")
384-
self.dvs_acl.verify_no_acl_tables()
357+
self.dvs_acl.verify_acl_table_count(0)
385358

386359
def test_AclRuleRedirectToNextHop(self, dvs):
387360
# NOTE: set_interface_status has a dependency on cdb within dvs,
388361
# so we still need to setup the db. This should be refactored.
389362
dvs.setup_db()
390-
self.setup_dvs_acl(dvs)
391363

392364
# Bring up an IP interface with a neighbor
393365
dvs.set_interface_status("Ethernet4", "up")
@@ -413,15 +385,16 @@ def test_AclRuleRedirectToNextHop(self, dvs):
413385
self.dvs_acl.verify_no_acl_rules()
414386

415387
self.dvs_acl.remove_acl_table("test_redirect")
416-
self.dvs_acl.verify_no_acl_tables()
388+
self.dvs_acl.verify_acl_table_count(0)
417389

418390
# Clean up the IP interface and neighbor
419391
dvs.remove_neighbor("Ethernet4", "10.0.0.2")
420392
dvs.remove_ip_address("Ethernet4", "10.0.0.1/24")
421393
dvs.set_interface_status("Ethernet4", "down")
422394

423395

424-
class TestAclRuleValidation(BaseTestAcl):
396+
@pytest.mark.usefixtures('dvs_acl_manager')
397+
class TestAclRuleValidation():
425398
"""
426399
Test class for cases that check if orchagent corectly validates
427400
ACL rules input
@@ -450,7 +423,6 @@ def test_AclActionValidation(self, dvs):
450423
to check the case when orchagent refuses to process rules with action that is not
451424
supported by the ASIC.
452425
"""
453-
self.setup_dvs_acl(dvs)
454426

455427
stage_name_map = {
456428
"ingress": "SAI_SWITCH_ATTR_ACL_STAGE_INGRESS",

0 commit comments

Comments
 (0)