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.

Sign up for GitHub

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] Add default action_list for default ACL table type #2298

Merged
merged 6 commits into from
May 27, 2022
Merged

[ACL] Add default action_list for default ACL table type #2298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2f38e3d
Add mandatory ACL actions when creating mirror ACL table
ysmanman Mar 28, 2022
a4fb759
Add acl action to TABLE_TYPE_MIRROR_DSCP and also fix typo
ysmanman Apr 23, 2022
9b1b9bb
Add test coverage.
ysmanman May 5, 2022
51c9b43
Merge remote-tracking branch 'mirror_update/acl-actions'
bingwang-ms May 27, 2022
c429c73
Add default action_list for default ACL table type
bingwang-ms May 27, 2022
02e4439
Address comment
bingwang-ms May 27, 2022
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
223 changes: 223 additions & 0 deletions orchagent/aclorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,176 @@ static const acl_capabilities_t defaultAclActionsSupported =
}
};

static acl_table_action_list_lookup_t defaultAclActionList =
{
{
// L3
TABLE_TYPE_L3,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION,
SAI_ACL_ACTION_TYPE_REDIRECT
}
},
{
ACL_STAGE_EGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION,
SAI_ACL_ACTION_TYPE_REDIRECT
}
}
}
},
{
// L3V6
TABLE_TYPE_L3V6,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION,
SAI_ACL_ACTION_TYPE_REDIRECT
}
},
{
ACL_STAGE_EGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION,
SAI_ACL_ACTION_TYPE_REDIRECT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redirect action should be only for the ingress and not egress.

}
}
}
},
{
// MIRROR
TABLE_TYPE_MIRROR,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_ACTION_TYPE_MIRROR_INGRESS
}
},
{
ACL_STAGE_EGRESS,
{
SAI_ACL_ACTION_TYPE_MIRROR_EGRESS
}
}
}
},
{
// MIRRORV6
TABLE_TYPE_MIRRORV6,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_ACTION_TYPE_MIRROR_INGRESS
}
},
{
ACL_STAGE_EGRESS,
{
SAI_ACL_ACTION_TYPE_MIRROR_EGRESS
}
}
}
},
{
// MIRROR_DSCP
TABLE_TYPE_MIRROR_DSCP,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_ACTION_TYPE_MIRROR_INGRESS
}
},
{
ACL_STAGE_EGRESS,
{
SAI_ACL_ACTION_TYPE_MIRROR_EGRESS
}
}
}
},
{
// TABLE_TYPE_PFCWD
TABLE_TYPE_PFCWD,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION
}
},
{
ACL_STAGE_EGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION
}
}
}
},
{
// MCLAG
TABLE_TYPE_MCLAG,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION
}
},
{
ACL_STAGE_EGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION
}
}
}
},
{
// MUX
TABLE_TYPE_MUX,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION
}
},
{
ACL_STAGE_EGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION
}
}
}
},
{
// DROP
TABLE_TYPE_DROP,
{
{
ACL_STAGE_INGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION
}
},
{
ACL_STAGE_EGRESS,
{
SAI_ACL_ACTION_TYPE_PACKET_ACTION
}
}
}
}
};

static acl_ip_type_lookup_t aclIpTypeLookup =
{
{ IP_TYPE_ANY, SAI_ACL_IP_TYPE_ANY },
Expand Down Expand Up @@ -301,6 +471,12 @@ const set<sai_acl_action_type_t>& AclTableType::getActions() const
return m_aclAcitons;
}

bool AclTableType::addAction(sai_acl_action_type_t action)
{
m_aclAcitons.insert(action);
return true;
}

AclTableTypeBuilder& AclTableTypeBuilder::withName(string name)
{
m_tableType.m_name = name;
Expand Down Expand Up @@ -1808,6 +1984,51 @@ AclTable::AclTable(AclOrch *pAclOrch) noexcept : m_pAclOrch(pAclOrch)

}

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

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

if (!m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage))
{
// No op if action list is not mandatory on table creation.
return true;
}
if (!type.getActions().empty())
{
// No change if action_list is provided
return true;
}

sai_acl_action_type_t acl_action = SAI_ACL_ACTION_TYPE_COUNTER;
if (m_pAclOrch->isAclActionSupported(stage, acl_action))
{
SWSS_LOG_INFO("Add counter acl action");
type.addAction(acl_action);
}

if (defaultAclActionList.count(type.getName()) != 0)
{
// Add the default action list
for (auto action : defaultAclActionList[type.getName()][stage])
{
if (m_pAclOrch->isAclActionSupported(stage, acl_action))
{
SWSS_LOG_INFO("Added default action for table type %s stage %s",
type.getName().c_str(),
((stage == ACL_STAGE_INGRESS)? "INGRESS":"EGRESS"));
type.addAction(action);
}
}
}

return true;
}

bool AclTable::validateAddType(const AclTableType &tableType)
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -3949,6 +4170,8 @@ void AclOrch::doAclTableTask(Consumer &consumer)

newTable.validateAddType(*tableType);

newTable.addMandatoryActions();

// validate and create/update ACL Table
if (bAllAttributesOk && newTable.validate())
{
Expand Down
8 changes: 8 additions & 0 deletions orchagent/aclorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ typedef tuple<sai_acl_range_type_t, int, int> acl_range_properties_t;
typedef map<acl_stage_type_t, AclActionCapabilities> acl_capabilities_t;
typedef map<sai_acl_action_type_t, set<int32_t>> acl_action_enum_values_capabilities_t;

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;

class AclRule;

class AclTableMatchInterface
Expand Down Expand Up @@ -156,6 +159,8 @@ class AclTableType
const set<sai_acl_range_type_t>& getRangeTypes() const;
const set<sai_acl_action_type_t>& getActions() const;

bool addAction(sai_acl_action_type_t action);

private:
friend class AclTableTypeBuilder;

Expand Down Expand Up @@ -387,6 +392,9 @@ class AclTable
bool validate();
bool create();

// Add actions to ACL table if mandatory action list is required on table creation.
bool addMandatoryActions();

// 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
92 changes: 92 additions & 0 deletions tests/mock_tests/aclorch_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1755,4 +1755,96 @@ namespace aclorch_test
// try to delete non existing acl rule
ASSERT_TRUE(orch->m_aclOrch->removeAclRule(tableId, ruleId));
}

sai_switch_api_t *old_sai_switch_api;

// The following function is used to override SAI API get_switch_attribute to request passing
// mandatory ACL actions to SAI when creating mirror ACL table.
sai_status_t getSwitchAttribute(_In_ sai_object_id_t switch_id,_In_ uint32_t attr_count,
_Inout_ sai_attribute_t *attr_list)
{
if (attr_count == 1)
{
switch(attr_list[0].id)
{
case SAI_SWITCH_ATTR_MAX_ACL_ACTION_COUNT:
attr_list[0].value.u32 = 2;
return SAI_STATUS_SUCCESS;
case SAI_SWITCH_ATTR_ACL_STAGE_INGRESS:
case SAI_SWITCH_ATTR_ACL_STAGE_EGRESS:
attr_list[0].value.aclcapability.action_list.count = 2;
attr_list[0].value.aclcapability.action_list.list[0]= SAI_ACL_ACTION_TYPE_COUNTER;
attr_list[0].value.aclcapability.action_list.list[1]=
attr_list[0].id == SAI_SWITCH_ATTR_ACL_STAGE_INGRESS ?
SAI_ACL_ACTION_TYPE_MIRROR_INGRESS : SAI_ACL_ACTION_TYPE_MIRROR_EGRESS;
attr_list[0].value.aclcapability.is_action_list_mandatory = true;
return SAI_STATUS_SUCCESS;
}
}
return old_sai_switch_api->get_switch_attribute(switch_id, attr_count, attr_list);
}

TEST_F(AclOrchTest, AclTableCreationWithMandatoryActions)
{
// Override SAI API get_switch_attribute to request passing mandatory ACL actions to SAI
// when creating mirror ACL table.
old_sai_switch_api = sai_switch_api;
sai_switch_api_t new_sai_switch_api = *sai_switch_api;
sai_switch_api = &new_sai_switch_api;
sai_switch_api->get_switch_attribute = getSwitchAttribute;

// Set platform env to enable support of MIRRORV6 ACL table.
bool unset_platform_env = false;
if (!getenv("platform"))
{
setenv("platform", VS_PLATFORM_SUBSTRING, 0);
unset_platform_env = true;
}

auto orch = createAclOrch();

for (const auto &acl_table_type : { TABLE_TYPE_MIRROR, TABLE_TYPE_MIRRORV6, TABLE_TYPE_MIRROR_DSCP })
{
for (const auto &acl_table_stage : { STAGE_INGRESS, STAGE_EGRESS })
{
// Create ACL table.
string acl_table_id = "mirror_acl_table";
auto kvfAclTable = deque<KeyOpFieldsValuesTuple>(
{ { acl_table_id,
SET_COMMAND,
{ { ACL_TABLE_DESCRIPTION, acl_table_type },
{ ACL_TABLE_TYPE, acl_table_type },
{ ACL_TABLE_STAGE, acl_table_stage },
{ ACL_TABLE_PORTS, "1,2" } } } });
orch->doAclTableTask(kvfAclTable);
auto acl_table = orch->getAclTable(acl_table_id);
ASSERT_NE(acl_table, nullptr);

// Verify mandaotry ACL actions has been added.
auto acl_actions = acl_table->type.getActions();
ASSERT_NE(acl_actions.find(SAI_ACL_ACTION_TYPE_COUNTER), acl_actions.end());
sai_acl_action_type_t action = strcmp(acl_table_stage, STAGE_INGRESS) == 0 ?
SAI_ACL_ACTION_TYPE_MIRROR_INGRESS : SAI_ACL_ACTION_TYPE_MIRROR_EGRESS;
ASSERT_NE(acl_actions.find(action), acl_actions.end());

// Delete ACL table.
kvfAclTable = deque<KeyOpFieldsValuesTuple>(
{ { acl_table_id,
DEL_COMMAND,
{} } });
orch->doAclTableTask(kvfAclTable);
acl_table = orch->getAclTable(acl_table_id);
ASSERT_EQ(acl_table, nullptr);
}
}

// Unset platform env.
if (unset_platform_env)
{
unsetenv("platform");
}

// Restore sai_switch_api.
sai_switch_api = old_sai_switch_api;
}
} // namespace nsAclOrchTest