Skip to content

Commit f2d2fb3

Browse files
L3 / L3 V6 Egress ACL table creation failure (sonic-net#2561)
* In L3/L3V6 Egress ACL, Range type qualifier is not supported for Broadcom Platforms. Check is added to ignore the range type qualifier for Broadcom platforms alone.
1 parent 577f696 commit f2d2fb3

File tree

3 files changed

+89
-13
lines changed

3 files changed

+89
-13
lines changed

orchagent/aclorch.cpp

+68-9
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,41 @@ static acl_table_match_field_lookup_t stageMandatoryMatchFields =
356356
}
357357
}
358358
}
359+
},
360+
{
361+
TABLE_TYPE_L3,
362+
{
363+
{
364+
ACL_STAGE_INGRESS,
365+
{
366+
SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE
367+
}
368+
},
369+
{
370+
ACL_STAGE_EGRESS,
371+
{
372+
SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE
373+
}
374+
}
375+
}
376+
},
377+
{
378+
TABLE_TYPE_L3V6,
379+
{
380+
{
381+
ACL_STAGE_INGRESS,
382+
{
383+
SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE
384+
}
385+
},
386+
{
387+
ACL_STAGE_EGRESS,
388+
{
389+
SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE
390+
}
391+
}
392+
}
359393
}
360-
361394
};
362395

363396
static acl_ip_type_lookup_t aclIpTypeLookup =
@@ -2091,6 +2124,29 @@ bool AclTable::addMandatoryActions()
20912124
return true;
20922125
}
20932126

2127+
bool AclTable::addStageMandatoryRangeFields()
2128+
{
2129+
SWSS_LOG_ENTER();
2130+
2131+
string platform = getenv("platform") ? getenv("platform") : "";
2132+
auto match = SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE;
2133+
2134+
if ((platform == BRCM_PLATFORM_SUBSTRING) &&
2135+
(stage == ACL_STAGE_EGRESS))
2136+
{
2137+
return false;
2138+
}
2139+
2140+
type.addMatch(make_shared<AclTableRangeMatch>(set<sai_acl_range_type_t>{
2141+
{SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}}));
2142+
SWSS_LOG_INFO("Added mandatory match field %s for table type %s stage %d",
2143+
sai_serialize_enum(match, &sai_metadata_enum_sai_acl_table_attr_t).c_str(),
2144+
type.getName().c_str(), stage);
2145+
2146+
return true;
2147+
}
2148+
2149+
20942150
bool AclTable::addStageMandatoryMatchFields()
20952151
{
20962152
SWSS_LOG_ENTER();
@@ -2108,10 +2164,17 @@ bool AclTable::addStageMandatoryMatchFields()
21082164
// Add the stage particular matching fields
21092165
for (auto match : fields_for_stage[stage])
21102166
{
2111-
type.addMatch(make_shared<AclTableMatch>(match));
2112-
SWSS_LOG_INFO("Added mandatory match field %s for table type %s stage %d",
2113-
sai_serialize_enum(match, &sai_metadata_enum_sai_acl_table_attr_t).c_str(),
2114-
type.getName().c_str(), stage);
2167+
if (match != SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE)
2168+
{
2169+
type.addMatch(make_shared<AclTableMatch>(match));
2170+
SWSS_LOG_INFO("Added mandatory match field %s for table type %s stage %d",
2171+
sai_serialize_enum(match, &sai_metadata_enum_sai_acl_table_attr_t).c_str(),
2172+
type.getName().c_str(), stage);
2173+
}
2174+
else
2175+
{
2176+
addStageMandatoryRangeFields();
2177+
}
21152178
}
21162179
}
21172180
}
@@ -3035,8 +3098,6 @@ void AclOrch::initDefaultTableTypes()
30353098
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT))
30363099
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT))
30373100
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS))
3038-
.withMatch(make_shared<AclTableRangeMatch>(set<sai_acl_range_type_t>{
3039-
{SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}}))
30403101
.build()
30413102
);
30423103

@@ -3054,8 +3115,6 @@ void AclOrch::initDefaultTableTypes()
30543115
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT))
30553116
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT))
30563117
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS))
3057-
.withMatch(make_shared<AclTableRangeMatch>(set<sai_acl_range_type_t>{
3058-
{SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}}))
30593118
.build()
30603119
);
30613120

orchagent/aclorch.h

+3
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,9 @@ class AclTable
393393
// Add stage mandatory matching fields to ACL table
394394
bool addStageMandatoryMatchFields();
395395

396+
// Add stage mandatory range fields to ACL table
397+
bool addStageMandatoryRangeFields();
398+
396399
// validate AclRule match attribute against rule and table configuration
397400
bool validateAclRuleMatch(sai_acl_entry_attr_t matchId, const AclRule& rule) const;
398401
// validate AclRule action attribute against rule and table configuration

tests/test_acl.py

+18-4
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@
2525
PFCWD_TABLE_NAME = "PFCWD_TEST"
2626
PFCWD_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"]
2727
class TestAcl:
28-
@pytest.fixture
29-
def l3_acl_table(self, dvs_acl):
28+
@pytest.fixture(params=['ingress', 'egress'])
29+
def l3_acl_table(self, dvs_acl, request):
3030
try:
31-
dvs_acl.create_acl_table(L3_TABLE_NAME, L3_TABLE_TYPE, L3_BIND_PORTS)
32-
yield dvs_acl.get_acl_table_ids(1)[0]
31+
dvs_acl.create_acl_table(L3_TABLE_NAME, L3_TABLE_TYPE, L3_BIND_PORTS, stage=request.param)
32+
yield dvs_acl.get_acl_table_ids(1)[0], request.param
3333
finally:
3434
dvs_acl.remove_acl_table(L3_TABLE_NAME)
3535
dvs_acl.verify_acl_table_count(0)
@@ -577,6 +577,20 @@ def test_AclTableMandatoryMatchFields(self, dvs, pfcwd_acl_table):
577577
assert match_in_ports
578578
else:
579579
assert not match_in_ports
580+
581+
def test_AclTableMandatoryRangeFields(self, dvs, l3_acl_table):
582+
"""
583+
The test case is to verify range qualifier is not applied for egress ACL
584+
"""
585+
table_oid, stage = l3_acl_table
586+
match_range_qualifier = False
587+
entry = dvs.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", table_oid)
588+
for k, v in entry.items():
589+
if k == "SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE" and v == "true":
590+
match_range_qualifier = True
591+
592+
assert not match_range_qualifier
593+
580594
class TestAclCrmUtilization:
581595
@pytest.fixture(scope="class", autouse=True)
582596
def configure_crm_polling_interval_for_test(self, dvs):

0 commit comments

Comments
 (0)