Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ACL] Support stage particular match fields #2341

Merged
merged 2 commits into from
Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 68 additions & 2 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,38 @@ static acl_table_action_list_lookup_t defaultAclActionList =
}
};

// The match fields for certain ACL table type are not exactly the same between INGRESS and EGRESS.
// For example, we can only match IN_PORT for PFCWD table type at INGRESS.
// Hence we need to specify stage particular matching fields in stageMandatoryMatchFields
static acl_table_match_field_lookup_t stageMandatoryMatchFields =
yxieca marked this conversation as resolved.
Show resolved Hide resolved
{
{
// TABLE_TYPE_PFCWD
TABLE_TYPE_PFCWD,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS
}
}
}
},
{
// TABLE_TYPE_DROP
TABLE_TYPE_DROP,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS
}
}
}
}

};

static acl_ip_type_lookup_t aclIpTypeLookup =
{
{ IP_TYPE_ANY, SAI_ACL_IP_TYPE_ANY },
Expand Down Expand Up @@ -477,6 +509,12 @@ bool AclTableType::addAction(sai_acl_action_type_t action)
return true;
}

bool AclTableType::addMatch(shared_ptr<AclTableMatchInterface> match)
{
m_matches.emplace(match->getId(), match);
return true;
}

AclTableTypeBuilder& AclTableTypeBuilder::withName(string name)
{
m_tableType.m_name = name;
Expand Down Expand Up @@ -2020,6 +2058,34 @@ bool AclTable::addMandatoryActions()
return true;
}

bool AclTable::addStageMandatoryMatchFields()
{
SWSS_LOG_ENTER();

if (stage == ACL_STAGE_UNKNOWN)
{
return false;
}

if (stageMandatoryMatchFields.count(type.getName()) != 0)
{
auto &fields_for_stage = stageMandatoryMatchFields[type.getName()];
if (fields_for_stage.count(stage) != 0)
{
// Add the stage particular matching fields
for (auto match : fields_for_stage[stage])
{
type.addMatch(make_shared<AclTableMatch>(match));
SWSS_LOG_INFO("Added mandatory match field %s for table type %s stage %d",
sai_serialize_enum(match, &sai_metadata_enum_sai_acl_table_attr_t).c_str(),
type.getName().c_str(), stage);
}
}
}

return true;
}

bool AclTable::validateAddType(const AclTableType &tableType)
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -2983,15 +3049,13 @@ void AclOrch::initDefaultTableTypes()
builder.withName(TABLE_TYPE_PFCWD)
.withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS))
.build()
);

addAclTableType(
builder.withName(TABLE_TYPE_DROP)
.withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS))
.build()
);

Expand Down Expand Up @@ -4108,6 +4172,8 @@ void AclOrch::doAclTableTask(Consumer &consumer)

newTable.validateAddType(*tableType);

newTable.addStageMandatoryMatchFields();

newTable.addMandatoryActions();

// validate and create/update ACL Table
Expand Down
7 changes: 7 additions & 0 deletions orchagent/aclorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ typedef map<sai_acl_action_type_t, set<int32_t>> acl_action_enum_values_capabili
typedef map<acl_stage_type_t, set<sai_acl_action_type_t> > acl_stage_action_list_t;
typedef map<string, acl_stage_action_list_t> acl_table_action_list_lookup_t;

typedef map<acl_stage_type_t, set<sai_acl_table_attr_t> > acl_stage_match_field_t;
typedef map<string, acl_stage_match_field_t> acl_table_match_field_lookup_t;

class AclRule;

class AclTableMatchInterface
Expand Down Expand Up @@ -160,6 +163,7 @@ class AclTableType
const set<sai_acl_action_type_t>& getActions() const;

bool addAction(sai_acl_action_type_t action);
bool addMatch(shared_ptr<AclTableMatchInterface> match);

private:
friend class AclTableTypeBuilder;
Expand Down Expand Up @@ -384,6 +388,9 @@ class AclTable
// Add actions to ACL table if mandatory action list is required on table creation.
bool addMandatoryActions();

// Add stage mandatory matching fields to ACL table
bool addStageMandatoryMatchFields();

// validate AclRule match attribute against rule and table configuration
bool validateAclRuleMatch(sai_acl_entry_attr_t matchId, const AclRule& rule) const;
// validate AclRule action attribute against rule and table configuration
Expand Down
31 changes: 29 additions & 2 deletions tests/test_acl.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import pytest
from requests import request

L3_TABLE_TYPE = "L3"
L3_TABLE_NAME = "L3_TEST"
Expand All @@ -20,6 +21,9 @@
MIRROR_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"]
MIRROR_RULE_NAME = "MIRROR_TEST_RULE"

PFCWD_TABLE_TYPE = "PFCWD"
PFCWD_TABLE_NAME = "PFCWD_TEST"
PFCWD_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"]
class TestAcl:
@pytest.yield_fixture
def l3_acl_table(self, dvs_acl):
Expand Down Expand Up @@ -59,6 +63,15 @@ def mirror_acl_table(self, dvs_acl):
dvs_acl.remove_acl_table(MIRROR_TABLE_NAME)
dvs_acl.verify_acl_table_count(0)

@pytest.fixture(params=['ingress', 'egress'])
def pfcwd_acl_table(self, dvs_acl, request):
try:
dvs_acl.create_acl_table(PFCWD_TABLE_NAME, PFCWD_TABLE_TYPE, PFCWD_BIND_PORTS, request.param)
yield dvs_acl.get_acl_table_ids(1)[0], request.param
finally:
dvs_acl.remove_acl_table(PFCWD_TABLE_NAME)
dvs_acl.verify_acl_table_count(0)

@pytest.yield_fixture
def setup_teardown_neighbor(self, dvs):
try:
Expand Down Expand Up @@ -548,8 +561,22 @@ def test_AclRuleRedirect(self, dvs, dvs_acl, l3_acl_table, setup_teardown_neighb

dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME)
dvs_acl.verify_no_acl_rules()



def test_AclTableMandatoryMatchFields(self, dvs, pfcwd_acl_table):
"""
The test case is to verify stage particular matching fields is applied
"""
table_oid, stage = pfcwd_acl_table
match_in_ports = False
entry = dvs.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", table_oid)
for k, v in entry.items():
if k == "SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS" and v == "true":
match_in_ports = True

if stage == "ingress":
assert match_in_ports
else:
assert not match_in_ports
class TestAclCrmUtilization:
@pytest.fixture(scope="class", autouse=True)
def configure_crm_polling_interval_for_test(self, dvs):
Expand Down