From 2fa988ecc21e2e8473f97173a4e7f8f474b71d67 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Wed, 20 Oct 2021 01:51:23 +0300 Subject: [PATCH 01/35] [aclorch] Add ACL_TABLE_TYPE configuration Added an API to create a table with configurable ACL table type (matches, bpoints, actions). Implemented a handler for new ACL_TABLE_TYPE CONFIG DB table. Implemented UT for the above. Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 1745 +++++++++++++++---------------- orchagent/aclorch.h | 162 +-- orchagent/acltable.h | 24 +- orchagent/muxorch.cpp | 18 +- orchagent/muxorch.h | 2 +- orchagent/orchdaemon.cpp | 10 +- orchagent/pbh/pbhrule.cpp | 10 +- orchagent/pbhorch.cpp | 13 +- orchagent/pfcactionhandler.cpp | 25 +- orchagent/pfcactionhandler.h | 2 +- tests/mock_tests/aclorch_ut.cpp | 421 ++++++-- tests/test_acl.py | 11 +- 12 files changed, 1373 insertions(+), 1070 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index c7cfe200a1..bc157ab11d 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -35,6 +35,9 @@ extern CrmOrch *gCrmOrch; #define MIN_VLAN_ID 1 // 0 is a reserved VLAN ID #define MAX_VLAN_ID 4095 // 4096 is a reserved VLAN ID +#define STATE_DB_ACL_ACTION_FIELD_IS_ACTION_LIST_MANDATORY "is_action_list_mandatory" +#define STATE_DB_ACL_ACTION_FIELD_ACTION_LIST "action_list" + const int TCP_PROTOCOL_NUM = 6; // TCP protocol number acl_rule_attr_lookup_t aclMatchLookup = @@ -68,6 +71,12 @@ acl_rule_attr_lookup_t aclMatchLookup = { MATCH_INNER_L4_DST_PORT, SAI_ACL_ENTRY_ATTR_FIELD_INNER_L4_DST_PORT } }; +static acl_bind_point_type_lookup_t aclBindPointTypeLookup = +{ + { BIND_POINT_TYPE_PORT, SAI_ACL_BIND_POINT_TYPE_PORT }, + { BIND_POINT_TYPE_PORTCHANNEL, SAI_ACL_BIND_POINT_TYPE_LAG }, +}; + static acl_rule_attr_lookup_t aclL3ActionLookup = { { ACTION_PACKET_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION }, @@ -105,20 +114,6 @@ static acl_dtel_flow_op_type_lookup_t aclDTelFlowOpTypeLookup = { DTEL_FLOW_OP_IOAM, SAI_ACL_DTEL_FLOW_OP_IOAM } }; -static acl_table_type_lookup_t aclTableTypeLookUp = -{ - { TABLE_TYPE_L3, ACL_TABLE_L3 }, - { TABLE_TYPE_L3V6, ACL_TABLE_L3V6 }, - { TABLE_TYPE_MIRROR, ACL_TABLE_MIRROR }, - { TABLE_TYPE_MIRRORV6, ACL_TABLE_MIRRORV6 }, - { TABLE_TYPE_MIRROR_DSCP, ACL_TABLE_MIRROR_DSCP }, - { TABLE_TYPE_CTRLPLANE, ACL_TABLE_CTRLPLANE }, - { TABLE_TYPE_DTEL_FLOW_WATCHLIST, ACL_TABLE_DTEL_FLOW_WATCHLIST }, - { TABLE_TYPE_DTEL_DROP_WATCHLIST, ACL_TABLE_DTEL_DROP_WATCHLIST }, - { TABLE_TYPE_MCLAG, ACL_TABLE_MCLAG }, - { TABLE_TYPE_DROP, ACL_TABLE_DROP } -}; - static acl_stage_type_lookup_t aclStageLookUp = { {STAGE_INGRESS, ACL_STAGE_INGRESS }, @@ -129,16 +124,24 @@ static const acl_capabilities_t defaultAclActionsSupported = { { ACL_STAGE_INGRESS, + AclActionCapabilities { - SAI_ACL_ACTION_TYPE_PACKET_ACTION, - SAI_ACL_ACTION_TYPE_MIRROR_INGRESS, - SAI_ACL_ACTION_TYPE_NO_NAT + { + SAI_ACL_ACTION_TYPE_PACKET_ACTION, + SAI_ACL_ACTION_TYPE_MIRROR_INGRESS, + SAI_ACL_ACTION_TYPE_NO_NAT + }, + false } }, { ACL_STAGE_EGRESS, + AclActionCapabilities { - SAI_ACL_ACTION_TYPE_PACKET_ACTION + { + SAI_ACL_ACTION_TYPE_PACKET_ACTION + }, + false } } }; @@ -157,18 +160,271 @@ static acl_ip_type_lookup_t aclIpTypeLookup = { IP_TYPE_ARP_REPLY, SAI_ACL_IP_TYPE_ARP_REPLY } }; -AclRule::AclRule(AclOrch *pAclOrch, string rule, string table, acl_table_type_t type, bool createCounter) : +static bool isAclEntryFieldAttribute(sai_acl_entry_attr_t attr) +{ + return attr >= SAI_ACL_ENTRY_ATTR_FIELD_START && attr <= SAI_ACL_ENTRY_ATTR_FIELD_END; +} + +static bool isAclEntryActionAttribute(sai_acl_entry_attr_t attr) +{ + return attr >= SAI_ACL_ENTRY_ATTR_ACTION_START && attr <= SAI_ACL_ENTRY_ATTR_ACTION_END; +} + +static sai_acl_table_attr_t AclEntryFieldToAclTableField(sai_acl_entry_attr_t attr) +{ + if (!isAclEntryFieldAttribute(attr)) + { + SWSS_LOG_THROW("ACL entry attribute is not a in a range of SAI_ACL_ENTRY_ATTR_FIELD_* attribute: %d", attr); + } + return static_cast(SAI_ACL_TABLE_ATTR_FIELD_START + (attr - SAI_ACL_ENTRY_ATTR_FIELD_START)); +} + +static sai_acl_action_type_t AclEntryActionToAclAction(sai_acl_entry_attr_t attr) +{ + if (!isAclEntryActionAttribute(attr)) + { + SWSS_LOG_THROW("ACL entry attribute is not a in a range of SAI_ACL_ENTRY_ATTR_ACTION_* attribute: %d", attr); + } + return static_cast(attr - SAI_ACL_ENTRY_ATTR_ACTION_START); +} + +static bool isAttributeValueEnum(sai_object_type_t objectType, sai_attr_id_t attr) +{ + const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, attr); + if (!meta) + { + SWSS_LOG_THROW("Metadata null pointer returned by sai_metadata_get_attr_metadata"); + } + return meta->isenum; +} + +string getAttributeIdName(sai_object_type_t objectType, sai_attr_id_t attr) +{ + const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, attr); + if (!meta) + { + SWSS_LOG_THROW("Metadata null pointer returned by sai_metadata_get_attr_metadata"); + } + return meta->attridname; +} + +string AclTableType::getName() const +{ + return m_name; +} + +const set& AclTableType::getBindPointTypes() const +{ + return m_bpointTypes; +} + +const set& AclTableType::getMatches() const +{ + return m_enabledMatches; +} + +const set& AclTableType::getRangeTypes() const +{ + return m_rangeTypes; +} + +const set& AclTableType::getActions() const +{ + return m_aclAcitons; +} + +AclTableTypeBuilder& AclTableTypeBuilder::withName(string name) +{ + m_tableType.m_name = name; + return *this; +} + +AclTableTypeBuilder& AclTableTypeBuilder::withBindPointType(sai_acl_bind_point_type_t bpointType) +{ + m_tableType.m_bpointTypes.insert(bpointType); + return *this; +} + +AclTableTypeBuilder& AclTableTypeBuilder::withMatch(sai_acl_table_attr_t matchField) +{ + if (!(matchField >= SAI_ACL_TABLE_ATTR_FIELD_START && matchField <= SAI_ACL_TABLE_ATTR_FIELD_END)) + { + SWSS_LOG_THROW("Invalid match table attribute %d", matchField); + } + + const auto meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_TABLE, matchField); + if (!meta) + { + SWSS_LOG_THROW("Failed to get metadata for attribute for SAI_OBJECT_TYPE_ACL_TABLE: attribute %d", matchField); + } + + if (meta->attrvaluetype != SAI_ATTR_VALUE_TYPE_BOOL) + { + SWSS_LOG_THROW("This API does not allow to set match with a non boolean value type"); + } + + m_tableType.m_enabledMatches.insert(matchField); + return *this; +} + +AclTableTypeBuilder& AclTableTypeBuilder::withAction(sai_acl_action_type_t action) +{ + m_tableType.m_aclAcitons.insert(action); + return *this; +} + +AclTableTypeBuilder& AclTableTypeBuilder::withRangeMatch(sai_acl_range_type_t rangeType) +{ + m_tableType.m_rangeTypes.insert(rangeType); + return *this; +} + +AclTableType AclTableTypeBuilder::build() +{ + auto tableType = m_tableType; + m_tableType = AclTableType(); + return tableType; +} + +bool AclTableTypeParser::parse(const std::string& key, + const vector& fieldValues, + AclTableTypeBuilder& builder) +{ + builder.withName(key); + + for (const auto& fieldValue: fieldValues) + { + auto field = to_upper(fvField(fieldValue)); + auto value = to_upper(fvValue(fieldValue)); + if (field == ACL_TABLE_TYPE_MATCHES) + { + if (!parseAclTableTypeMatches(value, builder)) + { + return false; + } + } + else if (field == ACL_TABLE_TYPE_ACTIONS) + { + if (!parseAclTableTypeActions(value, builder)) + { + return false; + } + } + else if (field == ACL_TABLE_TYPE_BPOINT_TYPES) + { + if (!parseAclTableTypeBindPointTypes(value, builder)) + { + return false; + } + } + else + { + SWSS_LOG_ERROR("Unknown field %s: value %s", field.c_str(), value.c_str()); + return false; + } + } + + return true; +} + +bool AclTableTypeParser::parseAclTableTypeMatches(const std::string& value, AclTableTypeBuilder& builder) +{ + auto matches = tokenize(value, comma); + for (const auto& match: matches) + { + auto matchIt = aclMatchLookup.find(match); + + if (matchIt == aclMatchLookup.end()) + { + SWSS_LOG_ERROR("Unknown match %s", match.c_str()); + return false; + } + + auto saiMatchAttr = matchIt->second; + if (isAclEntryFieldAttribute(saiMatchAttr)) + { + auto tableAttrId = AclEntryFieldToAclTableField(saiMatchAttr); + builder.withMatch(tableAttrId); + } + else /* range type */ + { + auto saiRangeType = static_cast(matchIt->second); + builder.withRangeMatch(saiRangeType); + } + } + + return true; +} + +bool AclTableTypeParser::parseAclTableTypeActions(const std::string& value, AclTableTypeBuilder& builder) +{ + auto actions = tokenize(value, comma); + for (const auto& action: actions) + { + sai_acl_entry_attr_t saiActionAttr = SAI_ACL_ENTRY_ATTR_ACTION_END; + + auto l3Action = aclL3ActionLookup.find(action); + auto mirrorAction = aclMirrorStageLookup.find(action); + auto dtelAction = aclDTelActionLookup.find(action); + + if (l3Action != aclL3ActionLookup.end()) + { + saiActionAttr = l3Action->second; + } + else if (mirrorAction != aclMirrorStageLookup.end()) + { + saiActionAttr = mirrorAction->second; + } + else if (dtelAction != aclDTelActionLookup.end()) + { + saiActionAttr = dtelAction->second; + } + else + { + SWSS_LOG_ERROR("Unknown action %s", action.c_str()); + return false; + } + + builder.withAction(AclEntryActionToAclAction(saiActionAttr)); + } + + return true; +} + +bool AclTableTypeParser::parseAclTableTypeBindPointTypes(const std::string& value, AclTableTypeBuilder& builder) +{ + auto bpointTypes = tokenize(value, comma); + for (const auto& bpointType: bpointTypes) + { + auto bpointIt = aclBindPointTypeLookup.find(bpointType); + if (bpointIt == aclBindPointTypeLookup.end()) + { + SWSS_LOG_ERROR("Unknown bind point %s", bpointType.c_str()); + return false; + } + + auto saiBpointType = bpointIt->second; + builder.withBindPointType(saiBpointType); + } + + return true; +} + + +AclRule::AclRule(AclOrch *pAclOrch, string rule, string table, bool createCounter) : m_pAclOrch(pAclOrch), m_id(rule), - m_tableId(table), - m_tableType(type), - m_tableOid(SAI_NULL_OBJECT_ID), m_ruleOid(SAI_NULL_OBJECT_ID), m_counterOid(SAI_NULL_OBJECT_ID), m_priority(0), m_createCounter(createCounter) { - m_tableOid = pAclOrch->getTableById(m_tableId); + auto tableOid = pAclOrch->getTableById(table); + m_pTable = pAclOrch->getTableByOid(tableOid); + if (!m_pTable) + { + SWSS_LOG_THROW("Failed to find ACL table %s. ACL table must exist at the time of creating AclRule", table.c_str()); + } } bool AclRule::validateAddPriority(string attr_name, string attr_value) @@ -408,7 +664,7 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) // TODO: For backwards compatibility, users can substitute IP_PROTOCOL for NEXT_HEADER. // This should be removed in a future release. - if ((m_tableType == ACL_TABLE_MIRRORV6 || m_tableType == ACL_TABLE_L3V6) + if ((m_pTable->type.getName() == TABLE_TYPE_MIRRORV6 || m_pTable->type.getName() == TABLE_TYPE_L3V6) && attr_name == MATCH_IP_PROTOCOL) { SWSS_LOG_WARN("Support for IP protocol on IPv6 tables will be removed in a future release, please switch to using NEXT_HEADER instead!"); @@ -422,34 +678,6 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) bool AclRule::validateAddAction(string attr_name, string attr_value) { - for (const auto& it: m_actions) - { - if (!AclRule::isActionSupported(it.first)) - { - SWSS_LOG_ERROR("Action %s:%s is not supported by ASIC", - attr_name.c_str(), attr_value.c_str()); - return false; - } - - // check if ACL action attribute entry parameter is an enum value - const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, it.first); - if (meta == nullptr) - { - SWSS_LOG_THROW("Metadata null pointer returned by sai_metadata_get_attr_metadata for action %s", - attr_name.c_str()); - } - if (meta->isenum) - { - // if ACL action attribute requires enum value check if value is supported by the ASIC - if (!m_pAclOrch->isAclActionEnumValueSupported(AclOrch::getAclActionFromAclEntry(it.first), - it.second.aclaction.parameter)) - { - SWSS_LOG_ERROR("Action %s:%s is not supported by ASIC", - attr_name.c_str(), attr_value.c_str()); - return false; - } - } - } return true; } @@ -469,11 +697,35 @@ bool AclRule::processIpType(string type, sai_uint32_t &ip_type) return true; } +bool AclRule::validate() +{ + for (auto matchPair: m_matches) + { + sai_attribute_t attr; + tie(attr.id, attr.value) = matchPair; + if (!m_pTable->validateAclRuleMatch(attr)) + { + return false; + } + } + + for (auto actionPair: m_actions) + { + sai_attribute_t attr; + tie(attr.id, attr.value) = actionPair; + if (!m_pTable->validateAclRuleAction(attr)) + { + return false; + } + } + + return true; +} + bool AclRule::create() { SWSS_LOG_ENTER(); - sai_object_id_t table_oid = m_pAclOrch->getTableById(m_tableId); vector rule_attrs; sai_object_id_t range_objects[2]; sai_object_list_t range_object_list = {0, range_objects}; @@ -488,7 +740,7 @@ bool AclRule::create() // store table oid this rule belongs to attr.id = SAI_ACL_ENTRY_ATTR_TABLE_ID; - attr.value.oid = table_oid; + attr.value.oid = m_pTable->getOid(); rule_attrs.push_back(attr); attr.id = SAI_ACL_ENTRY_ATTR_PRIORITY; @@ -564,7 +816,7 @@ bool AclRule::create() decreaseNextHopRefCount(); } - gCrmOrch->incCrmAclTableUsedCounter(CrmResourceType::CRM_ACL_ENTRY, m_tableOid); + gCrmOrch->incCrmAclTableUsedCounter(CrmResourceType::CRM_ACL_ENTRY, m_pTable->getOid()); return (status == SAI_STATUS_SUCCESS); } @@ -599,17 +851,6 @@ void AclRule::decreaseNextHopRefCount() return; } -bool AclRule::isActionSupported(sai_acl_entry_attr_t action) const -{ - auto action_type = AclOrch::getAclActionFromAclEntry(action); - const auto* pTable = m_pAclOrch->getTableByOid(m_tableOid); - if (pTable == nullptr) - { - SWSS_LOG_THROW("ACL table does not exist for oid %" PRIu64, m_tableOid); - } - return m_pAclOrch->isAclActionSupported(pTable->stage, action_type); -} - bool AclRule::remove() { SWSS_LOG_ENTER(); @@ -621,7 +862,7 @@ bool AclRule::remove() return false; } - gCrmOrch->decCrmAclTableUsedCounter(CrmResourceType::CRM_ACL_ENTRY, m_tableOid); + gCrmOrch->decCrmAclTableUsedCounter(CrmResourceType::CRM_ACL_ENTRY, m_pTable->getOid()); m_ruleOid = SAI_NULL_OBJECT_ID; @@ -672,94 +913,58 @@ AclRuleCounters AclRule::getCounters() return AclRuleCounters(counter_attr[0].value.u64, counter_attr[1].value.u64); } -shared_ptr AclRule::makeShared(acl_table_type_t type, AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple& data) +string AclRule::getId() { - string action; - bool action_found = false; - /* Find action configured by user. Based on action type create rule. */ - for (const auto& itr : kfvFieldsValues(data)) - { - string attr_name = to_upper(fvField(itr)); - string attr_value = fvValue(itr); - if (aclL3ActionLookup.find(attr_name) != aclL3ActionLookup.cend() || - aclMirrorStageLookup.find(attr_name) != aclMirrorStageLookup.cend() || - /* handle "MIRROR_ACTION" key without mirror stage specified for backward compatibility */ - attr_name == ACTION_MIRROR_ACTION || - aclDTelActionLookup.find(attr_name) != aclDTelActionLookup.cend()) - { - action_found = true; - action = attr_name; - break; - } - } + return m_id; +} - if (!action_found) - { - throw runtime_error("ACL rule action is not found in rule " + rule); - } +string AclRule::getTableId() +{ + return m_pTable->getId(); +} - if (type != ACL_TABLE_L3 && - type != ACL_TABLE_L3V6 && - type != ACL_TABLE_MIRROR && - type != ACL_TABLE_MIRRORV6 && - type != ACL_TABLE_MIRROR_DSCP && - type != ACL_TABLE_DTEL_FLOW_WATCHLIST && - type != ACL_TABLE_DTEL_DROP_WATCHLIST && - type != ACL_TABLE_MCLAG && - type != ACL_TABLE_DROP) - { - throw runtime_error("Unknown table type"); - } +shared_ptr AclRule::makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple& data) +{ + shared_ptr aclRule; - /* Mirror rules can exist in both tables */ - if (aclMirrorStageLookup.find(action) != aclMirrorStageLookup.cend() || - action == ACTION_MIRROR_ACTION /* implicitly ingress in old schema */) - { - return make_shared(acl, mirror, rule, table, type); - } - /* L3 rules can exist only in L3 table */ - else if (type == ACL_TABLE_L3) + for (const auto& itr: kfvFieldsValues(data)) { - return make_shared(acl, rule, table, type); - } - /* L3V6 rules can exist only in L3V6 table */ - else if (type == ACL_TABLE_L3V6) - { - return make_shared(acl, rule, table, type); - } - /* Pfcwd rules can exist only in PFCWD table */ - else if (type == ACL_TABLE_PFCWD) - { - return make_shared(acl, rule, table, type); - } - else if (type == ACL_TABLE_DTEL_FLOW_WATCHLIST) - { - if (dtel) + auto action = to_upper(fvField(itr)); + if (aclMirrorStageLookup.find(action) != aclMirrorStageLookup.cend() || + action == ACTION_MIRROR_ACTION /* implicitly ingress in old schema */) { - return make_shared(acl, dtel, rule, table, type); - } else { - throw runtime_error("DTel feature is not enabled. Watchlists cannot be configured"); + return make_shared(acl, mirror, rule, table); } - } - else if (type == ACL_TABLE_DTEL_DROP_WATCHLIST) - { - if (dtel) + else if (aclL3ActionLookup.find(action) != aclL3ActionLookup.cend()) { - return make_shared(acl, dtel, rule, table, type); - } else { - throw runtime_error("DTel feature is not enabled. Watchlists cannot be configured"); + return make_shared(acl, rule, table); + } + else if (aclDTelFlowOpTypeLookup.find(action) != aclDTelFlowOpTypeLookup.cend()) + { + if (!dtel) + { + throw runtime_error("DTel feature is not enabled. Watchlists cannot be configured"); + } + + if (action == ACTION_DTEL_DROP_REPORT_ENABLE || + action == ACTION_DTEL_TAIL_DROP_REPORT_ENABLE || + action == ACTION_DTEL_REPORT_ALL_PACKETS) + { + return make_shared(acl, dtel, rule, table); + } + else + { + return make_shared(acl, dtel, rule, table); + } } } - else if (type == ACL_TABLE_MCLAG) - { - return make_shared(acl, rule, table, type); - } - else if (type == ACL_TABLE_DROP) + + if (!aclRule) { - return make_shared(acl, rule, table, type); + throw runtime_error("ACL rule action is not found in rule " + rule); } - throw runtime_error("Wrong combination of table type and action in rule " + rule); + return aclRule; } bool AclRule::enableCounter() @@ -773,7 +978,7 @@ bool AclRule::enableCounter() if (m_ruleOid == SAI_NULL_OBJECT_ID) { - SWSS_LOG_ERROR("ACL rule %s doesn't exist in ACL table %s", m_id.c_str(), m_tableId.c_str()); + SWSS_LOG_ERROR("ACL rule %s doesn't exist in ACL table %s", m_id.c_str(), m_pTable->getId().c_str()); return false; } @@ -791,7 +996,7 @@ bool AclRule::enableCounter() sai_status_t status = sai_acl_api->set_acl_entry_attribute(m_ruleOid, &attr); if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to enable counter for ACL rule %s in ACL table %s", m_id.c_str(), m_tableId.c_str()); + SWSS_LOG_ERROR("Failed to enable counter for ACL rule %s in ACL table %s", m_id.c_str(), m_pTable->getId().c_str()); removeCounter(); return false; } @@ -810,7 +1015,7 @@ bool AclRule::disableCounter() if (m_ruleOid == SAI_NULL_OBJECT_ID) { - SWSS_LOG_ERROR("ACL rule %s doesn't exist in ACL table %s", m_id.c_str(), m_tableId.c_str()); + SWSS_LOG_ERROR("ACL rule %s doesn't exist in ACL table %s", m_id.c_str(), m_pTable->getId().c_str()); return false; } @@ -823,7 +1028,7 @@ bool AclRule::disableCounter() sai_status_t status = sai_acl_api->set_acl_entry_attribute(m_ruleOid, &attr); if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to disable counter for ACL rule %s in ACL table %s", m_id.c_str(), m_tableId.c_str()); + SWSS_LOG_ERROR("Failed to disable counter for ACL rule %s in ACL table %s", m_id.c_str(), m_pTable->getId().c_str()); return false; } @@ -848,7 +1053,7 @@ bool AclRule::createCounter() } attr.id = SAI_ACL_COUNTER_ATTR_TABLE_ID; - attr.value.oid = m_tableOid; + attr.value.oid = m_pTable->getOid(); counter_attrs.push_back(attr); attr.id = SAI_ACL_COUNTER_ATTR_ENABLE_BYTE_COUNT; @@ -861,13 +1066,13 @@ bool AclRule::createCounter() if (sai_acl_api->create_acl_counter(&m_counterOid, gSwitchId, (uint32_t)counter_attrs.size(), counter_attrs.data()) != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to create counter for the rule %s in table %s", m_id.c_str(), m_tableId.c_str()); + SWSS_LOG_ERROR("Failed to create counter for the rule %s in table %s", m_id.c_str(), m_pTable->getId().c_str()); return false; } - gCrmOrch->incCrmAclTableUsedCounter(CrmResourceType::CRM_ACL_COUNTER, m_tableOid); + gCrmOrch->incCrmAclTableUsedCounter(CrmResourceType::CRM_ACL_COUNTER, m_pTable->getOid()); - SWSS_LOG_INFO("Created counter for the rule %s in table %s", m_id.c_str(), m_tableId.c_str()); + SWSS_LOG_INFO("Created counter for the rule %s in table %s", m_id.c_str(), m_pTable->getId().c_str()); return true; } @@ -897,28 +1102,28 @@ bool AclRule::removeCounter() if (sai_acl_api->remove_acl_counter(m_counterOid) != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to remove ACL counter for rule %s in table %s", m_id.c_str(), m_tableId.c_str()); + SWSS_LOG_ERROR("Failed to remove ACL counter for rule %s in table %s", m_id.c_str(), m_pTable->getId().c_str()); return false; } - gCrmOrch->decCrmAclTableUsedCounter(CrmResourceType::CRM_ACL_COUNTER, m_tableOid); + gCrmOrch->decCrmAclTableUsedCounter(CrmResourceType::CRM_ACL_COUNTER, m_pTable->getOid()); SWSS_LOG_INFO("Removing record about the counter %" PRIx64 " from the DB", m_counterOid); AclOrch::getCountersTable().del(getTableId() + ":" + getId()); m_counterOid = SAI_NULL_OBJECT_ID; - SWSS_LOG_INFO("Removed counter for the rule %s in table %s", m_id.c_str(), m_tableId.c_str()); + SWSS_LOG_INFO("Removed counter for the rule %s in table %s", m_id.c_str(), m_pTable->getId().c_str()); return true; } -AclRuleL3::AclRuleL3(AclOrch *aclOrch, string rule, string table, acl_table_type_t type, bool createCounter) : - AclRule(aclOrch, rule, table, type, createCounter) +AclRulePacket::AclRulePacket(AclOrch *aclOrch, string rule, string table, bool createCounter) : + AclRule(aclOrch, rule, table, createCounter) { } -bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) +bool AclRulePacket::validateAddAction(string attr_name, string _attr_value) { SWSS_LOG_ENTER(); @@ -996,7 +1201,7 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) } // This method should return sai attribute id of the redirect destination -sai_object_id_t AclRuleL3::getRedirectObjectId(const string& redirect_value) +sai_object_id_t AclRulePacket::getRedirectObjectId(const string& redirect_value) { string target = redirect_value; @@ -1069,36 +1274,7 @@ sai_object_id_t AclRuleL3::getRedirectObjectId(const string& redirect_value) return SAI_NULL_OBJECT_ID; } -bool AclRuleL3::validateAddMatch(string attr_name, string attr_value) -{ - if (attr_name == MATCH_DSCP) - { - SWSS_LOG_ERROR("DSCP match is not supported for table type L3"); - return false; - } - - if (attr_name == MATCH_SRC_IPV6 || attr_name == MATCH_DST_IPV6) - { - SWSS_LOG_ERROR("IPv6 address match is not supported for table type L3"); - return false; - } - - if (attr_name == MATCH_ICMPV6_TYPE || attr_name == MATCH_ICMPV6_CODE) - { - SWSS_LOG_ERROR("ICMPv6 match is not supported for table type L3"); - return false; - } - - if (attr_name == MATCH_NEXT_HEADER) - { - SWSS_LOG_ERROR("IPv6 Next Header match is not supported for table type L3"); - return false; - } - - return AclRule::validateAddMatch(attr_name, attr_value); -} - -bool AclRuleL3::validate() +bool AclRulePacket::validate() { SWSS_LOG_ENTER(); @@ -1107,75 +1283,16 @@ bool AclRuleL3::validate() return false; } - return true; + return AclRule::validate(); } -void AclRuleL3::update(SubjectType, void *) +void AclRulePacket::update(SubjectType, void *) { // Do nothing } -AclRulePfcwd::AclRulePfcwd(AclOrch *aclOrch, string rule, string table, acl_table_type_t type, bool createCounter) : - AclRuleL3(aclOrch, rule, table, type, createCounter) -{ -} - -bool AclRulePfcwd::validateAddMatch(string attr_name, string attr_value) -{ - return AclRule::validateAddMatch(attr_name, attr_value); -} - -AclRuleMux::AclRuleMux(AclOrch *aclOrch, string rule, string table, acl_table_type_t type, bool createCounter) : - AclRuleL3(aclOrch, rule, table, type, createCounter) -{ -} - -bool AclRuleMux::validateAddMatch(string attr_name, string attr_value) -{ - return AclRule::validateAddMatch(attr_name, attr_value); -} - -AclRuleL3V6::AclRuleL3V6(AclOrch *aclOrch, string rule, string table, acl_table_type_t type) : - AclRuleL3(aclOrch, rule, table, type) -{ -} - - -bool AclRuleL3V6::validateAddMatch(string attr_name, string attr_value) -{ - if (attr_name == MATCH_DSCP) - { - SWSS_LOG_ERROR("DSCP match is not supported for table type L3V6"); - return false; - } - - if (attr_name == MATCH_SRC_IP || attr_name == MATCH_DST_IP) - { - SWSS_LOG_ERROR("IPv4 address match is not supported for table type L3V6"); - return false; - } - - if (attr_name == MATCH_ICMP_TYPE || attr_name == MATCH_ICMP_CODE) - { - SWSS_LOG_ERROR("ICMPv4 match is not supported for table type L3V6"); - return false; - } - - if (attr_name == MATCH_ETHER_TYPE) - { - SWSS_LOG_ERROR("Ethertype match is not supported for table type L3V6"); - return false; - } - - // TODO: For backwards compatibility, users can substitute IP_PROTOCOL for NEXT_HEADER. - // Should add a check for IP_PROTOCOL in a future release. - - return AclRule::validateAddMatch(attr_name, attr_value); -} - - -AclRuleMirror::AclRuleMirror(AclOrch *aclOrch, MirrorOrch *mirror, string rule, string table, acl_table_type_t type) : - AclRule(aclOrch, rule, table, type), +AclRuleMirror::AclRuleMirror(AclOrch *aclOrch, MirrorOrch *mirror, string rule, string table) : + AclRule(aclOrch, rule, table), m_state(false), m_pMirrorOrch(mirror) { @@ -1210,70 +1327,6 @@ bool AclRuleMirror::validateAddAction(string attr_name, string attr_value) return AclRule::validateAddAction(attr_name, attr_value); } -bool AclRuleMirror::validateAddMatch(string attr_name, string attr_value) -{ - if ((m_tableType == ACL_TABLE_L3 || m_tableType == ACL_TABLE_L3V6) - && attr_name == MATCH_DSCP) - { - SWSS_LOG_ERROR("DSCP match is not supported for the table of type L3"); - return false; - } - - if ((m_tableType == ACL_TABLE_MIRROR_DSCP && - aclMatchLookup.find(attr_name) != aclMatchLookup.end() && - attr_name != MATCH_DSCP)) - { - SWSS_LOG_ERROR("%s match is not supported for the table of type MIRROR_DSCP", - attr_name.c_str()); - return false; - } - - /* - * Type of Tables and Supported Match Types (Configuration) - * |---------------------------------------------------| - * | Match Type | TABLE_MIRROR | TABLE_MIRRORV6 | - * |---------------------------------------------------| - * | MATCH_SRC_IP | √ | | - * | MATCH_DST_IP | √ | | - * |---------------------------------------------------| - * | MATCH_ICMP_TYPE | √ | | - * | MATCH_ICMP_CODE | √ | | - * |---------------------------------------------------| - * | MATCH_ICMPV6_TYPE | | √ | - * | MATCH_ICMPV6_CODE | | √ | - * |---------------------------------------------------| - * | MATCH_SRC_IPV6 | | √ | - * | MATCH_DST_IPV6 | | √ | - * |---------------------------------------------------| - * | MARTCH_ETHERTYPE | √ | | - * |---------------------------------------------------| - */ - - if (m_tableType == ACL_TABLE_MIRROR && - (attr_name == MATCH_SRC_IPV6 || attr_name == MATCH_DST_IPV6 || - attr_name == MATCH_ICMPV6_TYPE || attr_name == MATCH_ICMPV6_CODE || - attr_name == MATCH_NEXT_HEADER)) - { - SWSS_LOG_ERROR("%s match is not supported for the table of type MIRROR", - attr_name.c_str()); - return false; - } - - // TODO: For backwards compatibility, users can substitute IP_PROTOCOL for NEXT_HEADER. - // This check should be expanded to include IP_PROTOCOL in a future release. - if (m_tableType == ACL_TABLE_MIRRORV6 && - (attr_name == MATCH_SRC_IP || attr_name == MATCH_DST_IP || - attr_name == MATCH_ICMP_TYPE || attr_name == MATCH_ICMP_CODE || - attr_name == MATCH_ETHER_TYPE)) - { - SWSS_LOG_ERROR("%s match is not supported for the table of type MIRRORv6", - attr_name.c_str()); - return false; - } - - return AclRule::validateAddMatch(attr_name, attr_value); -} - bool AclRuleMirror::validate() { SWSS_LOG_ENTER(); @@ -1283,7 +1336,7 @@ bool AclRuleMirror::validate() return false; } - return true; + return AclRule::validate(); } bool AclRuleMirror::create() @@ -1387,33 +1440,6 @@ void AclRuleMirror::update(SubjectType type, void *cntx) } } -AclRuleMclag::AclRuleMclag(AclOrch *aclOrch, string rule, string table, acl_table_type_t type, bool createCounter) : - AclRuleL3(aclOrch, rule, table, type, createCounter) -{ -} - -bool AclRuleMclag::validateAddMatch(string attr_name, string attr_value) -{ - if (attr_name != MATCH_IP_TYPE && attr_name != MATCH_OUT_PORTS) - { - return false; - } - - return AclRule::validateAddMatch(attr_name, attr_value); -} - -bool AclRuleMclag::validate() -{ - SWSS_LOG_ENTER(); - - if (m_matches.size() == 0) - { - return false; - } - - return true; -} - AclTable::AclTable(AclOrch *pAclOrch, string id) noexcept : m_pAclOrch(pAclOrch), id(id) { @@ -1424,20 +1450,11 @@ AclTable::AclTable(AclOrch *pAclOrch) noexcept : m_pAclOrch(pAclOrch) } -bool AclTable::validateAddType(const acl_table_type_t &value) +bool AclTable::validateAddType(const AclTableType &tableType) { SWSS_LOG_ENTER(); - if (value == ACL_TABLE_MIRROR || value == ACL_TABLE_MIRRORV6) - { - if (!m_pAclOrch->isAclMirrorTableSupported(value)) - { - SWSS_LOG_ERROR("Failed to validate type: mirror table is not supported"); - return false; - } - } - - type = value; + type = tableType; return true; } @@ -1493,318 +1510,153 @@ bool AclTable::validateAddPorts(const unordered_set &value) bool AclTable::validate() { - if (type == ACL_TABLE_CTRLPLANE) + if (type.getName() == TABLE_TYPE_CTRLPLANE) + { return true; + } - if (type == ACL_TABLE_UNKNOWN || stage == ACL_STAGE_UNKNOWN) - return false; - - return true; -} - -bool AclTable::create() -{ - SWSS_LOG_ENTER(); - - sai_attribute_t attr; - vector table_attrs; - vector bpoint_list; - - // PFC watch dog ACLs are only applied to port - if ((type == ACL_TABLE_PFCWD) || (type == ACL_TABLE_DROP)) + if (stage == ACL_STAGE_UNKNOWN) { - bpoint_list = { SAI_ACL_BIND_POINT_TYPE_PORT }; + return false; } - else + + if (m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage)) { - bpoint_list = { SAI_ACL_BIND_POINT_TYPE_PORT, SAI_ACL_BIND_POINT_TYPE_LAG }; + if (type.getActions().empty()) + { + SWSS_LOG_ERROR("Action list for table %s is mandatory", id.c_str()); + return false; + } } - attr.id = SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST; - attr.value.s32list.count = static_cast(bpoint_list.size()); - attr.value.s32list.list = bpoint_list.data(); - table_attrs.push_back(attr); + return true; +} - if (type == ACL_TABLE_PBH) +bool AclTable::validateAclRuleMatch(sai_attribute_t attr) const +{ + auto aclEntryAttr = static_cast(attr.id); + if (isAclEntryFieldAttribute(aclEntryAttr)) { - attr.id = SAI_ACL_TABLE_ATTR_ACL_STAGE; - attr.value.s32 = SAI_ACL_STAGE_INGRESS; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_GRE_KEY; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - sai_status_t status = sai_acl_api->create_acl_table(&m_oid, gSwitchId, (uint32_t)table_attrs.size(), table_attrs.data()); - - if (status == SAI_STATUS_SUCCESS) + if (!type.getMatches().count(AclEntryFieldToAclTableField(aclEntryAttr))) { - gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, SAI_ACL_STAGE_INGRESS, SAI_ACL_BIND_POINT_TYPE_PORT); - gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, SAI_ACL_STAGE_INGRESS, SAI_ACL_BIND_POINT_TYPE_LAG); + SWSS_LOG_ERROR("Match %s is not supported on table %s", + getAttributeIdName(SAI_OBJECT_TYPE_ACL_ENTRY, aclEntryAttr).c_str(), id.c_str()); + return false; } - - return status == SAI_STATUS_SUCCESS; } - - if ((type == ACL_TABLE_PFCWD) || (type == ACL_TABLE_DROP)) + else /* range type */ { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_TC; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_ACL_STAGE; - attr.value.s32 = (stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; - table_attrs.push_back(attr); - - if (stage == ACL_STAGE_INGRESS) + auto rangeType = static_cast(attr.id); + if (!type.getRangeTypes().count(rangeType)) { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS; - attr.value.booldata = true; - table_attrs.push_back(attr); + SWSS_LOG_ERROR("Range match %s is not supported on table %s", + sai_metadata_get_acl_range_type_name(rangeType), id.c_str()); + return false; } + } - sai_status_t status = sai_acl_api->create_acl_table(&m_oid, gSwitchId, (uint32_t)table_attrs.size(), table_attrs.data()); - - if (status == SAI_STATUS_SUCCESS) - { - gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, (sai_acl_stage_t) attr.value.s32, SAI_ACL_BIND_POINT_TYPE_PORT); - } + return true; +} - return status == SAI_STATUS_SUCCESS; +bool AclTable::validateAclRuleAction(sai_attribute_t attr) const +{ + auto aclEntryAttr = static_cast(attr.id); + auto action = AclEntryActionToAclAction(aclEntryAttr); + if (!m_pAclOrch->isAclActionSupported(stage, action)) + { + SWSS_LOG_ERROR("Action %s is not supported on table %s", + sai_metadata_get_acl_action_type_name(action), id.c_str()); } - if (type == ACL_TABLE_MIRROR_DSCP) + if (isAttributeValueEnum(SAI_OBJECT_TYPE_ACL_ENTRY, attr.id)) { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_DSCP; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_ACL_STAGE; - attr.value.s32 = (stage == ACL_STAGE_INGRESS) ? - SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; - table_attrs.push_back(attr); - - sai_status_t status = sai_acl_api->create_acl_table( - &m_oid, gSwitchId, (uint32_t)table_attrs.size(), table_attrs.data()); - - if (status == SAI_STATUS_SUCCESS) + if (!m_pAclOrch->isAclActionEnumValueSupported(action, attr.value.aclaction.parameter)) { - gCrmOrch->incCrmAclUsedCounter( - CrmResourceType::CRM_ACL_TABLE, (sai_acl_stage_t)attr.value.s32, SAI_ACL_BIND_POINT_TYPE_PORT); - gCrmOrch->incCrmAclUsedCounter( - CrmResourceType::CRM_ACL_TABLE, (sai_acl_stage_t)attr.value.s32, SAI_ACL_BIND_POINT_TYPE_LAG); + SWSS_LOG_ERROR("Action %s is not supported by ASIC", sai_metadata_get_acl_action_type_name(action)); + return false; } - - return status == SAI_STATUS_SUCCESS; - } - - if (type != ACL_TABLE_MIRRORV6 && type != ACL_TABLE_L3V6) - { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); } - attr.id = SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - /* - * Type of Tables and Supported Match Types (ASIC database) - * |------------------------------------------------------------------| - * | | TABLE_MIRROR | TABLE_MIRROR | TABLE_MIRRORV6 | - * | Match Type |----------------------------------------------| - * | | combined | separated | - * |------------------------------------------------------------------| - * | MATCH_SRC_IP | √ | √ | | - * | MATCH_DST_IP | √ | √ | | - * |------------------------------------------------------------------| - * | MATCH_ICMP_TYPE | √ | √ | | - * | MATCH_ICMP_CODE | √ | √ | | - * |------------------------------------------------------------------| - * | MATCH_SRC_IPV6 | √ | | √ | - * | MATCH_DST_IPV6 | √ | | √ | - * |------------------------------------------------------------------| - * | MATCH_ICMPV6_TYPE | √ | | √ | - * | MATCH_ICMPV6_CODE | √ | | √ | - * |------------------------------------------------------------------| - * | MATCH_IP_PROTOCOL | √ | √ | | - * | MATCH_NEXT_HEADER | √ | | √ | - * | -----------------------------------------------------------------| - * | MATCH_ETHERTYPE | √ | √ | | - * |------------------------------------------------------------------| - * | MATCH_IN_PORTS | √ | √ | | - * |------------------------------------------------------------------| - */ - - // FIXME: This section has become hard to maintain and should be refactored. - if (type == ACL_TABLE_MIRROR) + // This means ACL table can hold rules with any action. + if (!type.getActions().empty()) { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_SRC_IP; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_DST_IP; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS; - attr.value.booldata = true; - table_attrs.push_back(attr); - - // If the switch supports v6 and requires one single table - if (m_pAclOrch->m_mirrorTableCapabilities[ACL_TABLE_MIRRORV6] && - m_pAclOrch->m_isCombinedMirrorV6Table) + if (!type.getActions().count(action)) { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER; - attr.value.booldata = true; - table_attrs.push_back(attr); + SWSS_LOG_ERROR("Action %s is not supported on table %s", + sai_metadata_get_acl_action_type_name(action), id.c_str()); + return false; } } - else if (type == ACL_TABLE_L3V6 || type == ACL_TABLE_MIRRORV6) // v6 only - { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6; - attr.value.booldata = true; - table_attrs.push_back(attr); - attr.id = SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6; - attr.value.booldata = true; - table_attrs.push_back(attr); + return true; +} - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); +bool AclTable::create() +{ + SWSS_LOG_ENTER(); - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE; - attr.value.booldata = true; - table_attrs.push_back(attr); + sai_attribute_t attr; + vector table_attrs; + vector range_types_list; + vector action_types_list; + vector bpoint_list {type.getBindPointTypes().begin(), type.getBindPointTypes().end()}; - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER; - attr.value.booldata = true; - table_attrs.push_back(attr); - } - else // v4 only + attr.id = SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST; + attr.value.s32list.count = static_cast(bpoint_list.size()); + attr.value.s32list.list = bpoint_list.data(); + table_attrs.push_back(attr); + + for (const auto& enabledMatch: type.getMatches()) { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_SRC_IP; + attr.id = enabledMatch; attr.value.booldata = true; table_attrs.push_back(attr); + } - attr.id = SAI_ACL_TABLE_ATTR_FIELD_DST_IP; - attr.value.booldata = true; - table_attrs.push_back(attr); + for (const auto& rangeType: type.getRangeTypes()) + { + range_types_list.push_back(rangeType); + } - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE; - attr.value.booldata = true; + if (!range_types_list.empty()) + { + attr.id = SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE; + attr.value.s32list.count = static_cast(range_types_list.size()); + attr.value.s32list.list = range_types_list.data(); table_attrs.push_back(attr); + } - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE; - attr.value.booldata = true; - table_attrs.push_back(attr); + for (const auto& actionType: type.getActions()) + { + action_types_list.push_back(actionType); + } - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL; - attr.value.booldata = true; + if (!action_types_list.empty()) + { + attr.id= SAI_ACL_TABLE_ATTR_ACL_ACTION_TYPE_LIST; + attr.value.s32list.count = static_cast(action_types_list.size()); + attr.value.s32list.list = action_types_list.data(); table_attrs.push_back(attr); } - attr.id = SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS; - attr.value.booldata = true; - table_attrs.push_back(attr); - - int32_t range_types_list[] = { SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE }; - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE; - attr.value.s32list.count = (uint32_t)(sizeof(range_types_list) / sizeof(range_types_list[0])); - attr.value.s32list.list = range_types_list; - table_attrs.push_back(attr); - sai_acl_stage_t acl_stage; attr.id = SAI_ACL_TABLE_ATTR_ACL_STAGE; acl_stage = (stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; attr.value.s32 = acl_stage; table_attrs.push_back(attr); - if (type == ACL_TABLE_MIRROR || type == ACL_TABLE_MIRRORV6) - { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_DSCP; - attr.value.booldata = true; - table_attrs.push_back(attr); - } - - if (type == ACL_TABLE_MCLAG) + sai_status_t status = sai_acl_api->create_acl_table(&m_oid, gSwitchId, (uint32_t)table_attrs.size(), table_attrs.data()); + if (status != SAI_STATUS_SUCCESS) { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS; - attr.value.booldata = true; - table_attrs.push_back(attr); + return false; } - sai_status_t status = sai_acl_api->create_acl_table(&m_oid, gSwitchId, (uint32_t)table_attrs.size(), table_attrs.data()); - - if (status == SAI_STATUS_SUCCESS) + for (const auto& bpointType: type.getBindPointTypes()) { - gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, acl_stage, SAI_ACL_BIND_POINT_TYPE_PORT); - gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, acl_stage, SAI_ACL_BIND_POINT_TYPE_LAG); + gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, acl_stage, bpointType); } - return status == SAI_STATUS_SUCCESS; + return true; } void AclTable::update(SubjectType type, void *cntx) @@ -2026,8 +1878,8 @@ AclRuleCounters AclRuleMirror::getCounters() return cnt; } -AclRuleDTelFlowWatchListEntry::AclRuleDTelFlowWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table, acl_table_type_t type) : - AclRule(aclOrch, rule, table, type), +AclRuleDTelFlowWatchListEntry::AclRuleDTelFlowWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table) : + AclRule(aclOrch, rule, table), m_pDTelOrch(dtel) { } @@ -2130,7 +1982,7 @@ bool AclRuleDTelFlowWatchListEntry::validate() return false; } - return true; + return AclRule::validate(); } bool AclRuleDTelFlowWatchListEntry::create() @@ -2152,7 +2004,7 @@ bool AclRuleDTelFlowWatchListEntry::create() return false; } - return true; + return AclRule::validate(); } bool AclRuleDTelFlowWatchListEntry::remove() @@ -2241,8 +2093,8 @@ void AclRuleDTelFlowWatchListEntry::update(SubjectType type, void *cntx) } } -AclRuleDTelDropWatchListEntry::AclRuleDTelDropWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table, acl_table_type_t type) : - AclRule(aclOrch, rule, table, type), +AclRuleDTelDropWatchListEntry::AclRuleDTelDropWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table) : + AclRule(aclOrch, rule, table), m_pDTelOrch(dtel) { } @@ -2289,7 +2141,7 @@ bool AclRuleDTelDropWatchListEntry::validate() return false; } - return true; + return AclRule::validate(); } void AclRuleDTelDropWatchListEntry::update(SubjectType, void *) @@ -2440,24 +2292,24 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr { m_mirrorTableCapabilities = { - { ACL_TABLE_MIRROR, true }, - { ACL_TABLE_MIRRORV6, true }, + { TABLE_TYPE_MIRROR, true }, + { TABLE_TYPE_MIRRORV6, true }, }; } else { m_mirrorTableCapabilities = { - { ACL_TABLE_MIRROR, true }, - { ACL_TABLE_MIRRORV6, false }, + { TABLE_TYPE_MIRROR, true }, + { TABLE_TYPE_MIRRORV6, false }, }; } SWSS_LOG_NOTICE("%s switch capability:", platform.c_str()); - SWSS_LOG_NOTICE(" ACL_TABLE_MIRROR: %s", - m_mirrorTableCapabilities[ACL_TABLE_MIRROR] ? "yes" : "no"); - SWSS_LOG_NOTICE(" ACL_TABLE_MIRRORV6: %s", - m_mirrorTableCapabilities[ACL_TABLE_MIRRORV6] ? "yes" : "no"); + SWSS_LOG_NOTICE(" TABLE_TYPE_MIRROR: %s", + m_mirrorTableCapabilities[TABLE_TYPE_MIRROR] ? "yes" : "no"); + SWSS_LOG_NOTICE(" TABLE_TYPE_MIRRORV6: %s", + m_mirrorTableCapabilities[TABLE_TYPE_MIRRORV6] ? "yes" : "no"); // In Broadcom platform, V4 and V6 rules are stored in the same table if (platform == BRCM_PLATFORM_SUBSTRING || @@ -2479,16 +2331,17 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr for (auto const& it : m_mirrorTableCapabilities) { string value = it.second ? "true" : "false"; - switch (it.first) + if (it.first == TABLE_TYPE_MIRROR) + { + fvVector.emplace_back(TABLE_TYPE_MIRROR, value); + } + else if (it.first == TABLE_TYPE_MIRRORV6) { - case ACL_TABLE_MIRROR: - fvVector.emplace_back(TABLE_TYPE_MIRROR, value); - break; - case ACL_TABLE_MIRRORV6: - fvVector.emplace_back(TABLE_TYPE_MIRRORV6, value); - break; - default: - break; + fvVector.emplace_back(TABLE_TYPE_MIRRORV6, value); + } + else + { + // ignore } } m_switchOrch->set_switch_capability(fvVector); @@ -2521,6 +2374,8 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr m_mirrorV6TableId[stage] = ""; } + initDefaultTableTypes(); + // Attach observers m_mirrorOrch->attach(this); gPortsOrch->attach(this); @@ -2534,6 +2389,168 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr timer->start(); } +void AclOrch::initDefaultTableTypes() +{ + SWSS_LOG_ENTER(); + + AclTableTypeBuilder builder; + + addAclTableType( + builder.withName(TABLE_TYPE_L3) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .build() + ); + + addAclTableType( + builder.withName(TABLE_TYPE_L3V6) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .build() + ); + + addAclTableType( + builder.withName(TABLE_TYPE_MCLAG) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .build() + ); + + addAclTableType( + builder.withName(TABLE_TYPE_PFCWD) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TC) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) + .build() + ); + + addAclTableType( + builder.withName(TABLE_TYPE_DROP) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TC) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) + .build() + ); + + if (isAclMirrorV4Supported()) + { + addAclTableType( + builder.withName(TABLE_TYPE_MIRROR_DSCP) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DSCP) + .build() + ); + + builder.withName(TABLE_TYPE_MIRROR) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DSCP) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE); + + if (isAclMirrorV6Supported() && isCombinedMirrorV6Table()) + { + builder + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER); + } + addAclTableType(builder.build()); + } + + if (isAclMirrorV6Supported()) + { + addAclTableType( + builder.withName(TABLE_TYPE_MIRRORV6) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DSCP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .build() + ); + } + + // Placeholder for control plane tables + addAclTableType(builder.withName(TABLE_TYPE_CTRLPLANE).build()); +} + void AclOrch::queryAclActionCapability() { SWSS_LOG_ENTER(); @@ -2565,12 +2582,15 @@ void AclOrch::queryAclActionCapability() SWSS_LOG_INFO("Supported %s action count %d:", stage_str, attr.value.aclcapability.action_list.count); + auto& capabilities = m_aclCapabilities[stage]; + for (size_t i = 0; i < static_cast(attr.value.aclcapability.action_list.count); i++) { auto action = static_cast(action_list[i]); - m_aclCapabilities[stage].insert(action); + capabilities.actionList.insert(action); SWSS_LOG_INFO(" %s", sai_serialize_enum(action, &sai_metadata_enum_sai_acl_action_type_t).c_str()); } + capabilities.isActionListMandatoryOnTableCreation = attr.value.aclcapability.is_action_list_mandatory; } else { @@ -2618,10 +2638,12 @@ void AclOrch::putAclActionCapabilityInDB(acl_stage_type_t stage) auto stage_str = (stage == ACL_STAGE_INGRESS ? STAGE_INGRESS : STAGE_EGRESS); auto field = std::string("ACL_ACTIONS") + '|' + stage_str; - auto& acl_action_set = m_aclCapabilities[stage]; + auto& capabilities = m_aclCapabilities[stage]; + auto& acl_action_set = capabilities.actionList; string delimiter; ostringstream acl_action_value_stream; + ostringstream is_action_list_mandatory_stream; for (const auto& action_map: {aclL3ActionLookup, aclMirrorStageLookup, aclDTelActionLookup}) { @@ -2636,8 +2658,11 @@ void AclOrch::putAclActionCapabilityInDB(acl_stage_type_t stage) } } - fvVector.emplace_back(field, acl_action_value_stream.str()); - m_switchOrch->set_switch_capability(fvVector); + is_action_list_mandatory_stream << boolalpha << capabilities.isActionListMandatoryOnTableCreation; + + fvVector.emplace_back(STATE_DB_ACL_ACTION_FIELD_IS_ACTION_LIST_MANDATORY, is_action_list_mandatory_stream.str()); + fvVector.emplace_back(STATE_DB_ACL_ACTION_FIELD_ACTION_LIST, acl_action_value_stream.str()); + m_aclStageCapabilityTable.set(stage_str, fvVector); } void AclOrch::initDefaultAclActionCapabilities(acl_stage_type_t stage) @@ -2646,9 +2671,9 @@ void AclOrch::initDefaultAclActionCapabilities(acl_stage_type_t stage) SWSS_LOG_INFO("Assumed %s %zu actions to be supported:", stage == ACL_STAGE_INGRESS ? STAGE_INGRESS : STAGE_EGRESS, - m_aclCapabilities[stage].size()); + m_aclCapabilities[stage].actionList.size()); - for (auto action: m_aclCapabilities[stage]) + for (auto action: m_aclCapabilities[stage].actionList) { SWSS_LOG_INFO(" %s", sai_serialize_enum(action, &sai_metadata_enum_sai_acl_action_type_t).c_str()); } @@ -2752,9 +2777,10 @@ sai_acl_action_type_t AclOrch::getAclActionFromAclEntry(sai_acl_entry_attr_t att return static_cast(attr - SAI_ACL_ENTRY_ATTR_ACTION_START); }; -AclOrch::AclOrch(vector& connectors, SwitchOrch *switchOrch, +AclOrch::AclOrch(vector& connectors, DBConnector* stateDb, SwitchOrch *switchOrch, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch, DTelOrch *dtelOrch) : Orch(connectors), + m_aclStageCapabilityTable(stateDb, STATE_ACL_STAGE_CAPABILITY_TABLE_NAME), m_switchOrch(switchOrch), m_mirrorOrch(mirrorOrch), m_neighOrch(neighOrch), @@ -2839,6 +2865,10 @@ void AclOrch::doTask(Consumer &consumer) unique_lock lock(m_countersMutex); doAclRuleTask(consumer); } + else if (table_name == CFG_ACL_TABLE_TYPE_TABLE_NAME || table_name == APP_ACL_TABLE_TYPE_TABLE_NAME) + { + doAclTableTypeTask(consumer); + } else { SWSS_LOG_ERROR("Invalid table %s", table_name.c_str()); @@ -2993,7 +3023,7 @@ bool AclOrch::addAclTable(AclTable &newTable) SWSS_LOG_ENTER(); string table_id = newTable.id; - if (newTable.type == ACL_TABLE_CTRLPLANE) + if (newTable.type.getName() == TABLE_TYPE_CTRLPLANE) { m_ctrlAclTables.emplace(table_id, newTable); SWSS_LOG_NOTICE("Created control plane ACL table %s", newTable.id.c_str()); @@ -3018,15 +3048,15 @@ bool AclOrch::addAclTable(AclTable &newTable) // If ACL table is new, check for the existence of current mirror tables // Note: only one table per mirror type can be created auto table_type = newTable.type; - if (table_type == ACL_TABLE_MIRROR || table_type == ACL_TABLE_MIRRORV6) + if (table_type.getName() == TABLE_TYPE_MIRROR || table_type.getName() == TABLE_TYPE_MIRRORV6) { string mirror_type; - if (table_type == ACL_TABLE_MIRROR && !m_mirrorTableId[table_stage].empty()) + if (table_type.getName() == TABLE_TYPE_MIRROR && !m_mirrorTableId[table_stage].empty()) { mirror_type = TABLE_TYPE_MIRROR; } - if (table_type == ACL_TABLE_MIRRORV6 && !m_mirrorV6TableId[table_stage].empty()) + if (table_type.getName() == TABLE_TYPE_MIRRORV6 && !m_mirrorV6TableId[table_stage].empty()) { mirror_type = TABLE_TYPE_MIRRORV6; } @@ -3044,7 +3074,7 @@ bool AclOrch::addAclTable(AclTable &newTable) } // Check if a separate mirror table is needed or not based on the platform - if (newTable.type == ACL_TABLE_MIRROR || newTable.type == ACL_TABLE_MIRRORV6) + if (newTable.type.getName() == TABLE_TYPE_MIRROR || newTable.type.getName() == TABLE_TYPE_MIRRORV6) { if (m_isCombinedMirrorV6Table && (!m_mirrorTableId[table_stage].empty() || @@ -3078,11 +3108,11 @@ bool AclOrch::addAclTable(AclTable &newTable) newTable.id.c_str(), table_oid); // Mark the existence of the mirror table - if (newTable.type == ACL_TABLE_MIRROR) + if (newTable.type.getName() == TABLE_TYPE_MIRROR) { m_mirrorTableId[table_stage] = table_id; } - else if (newTable.type == ACL_TABLE_MIRRORV6) + else if (newTable.type.getName() == TABLE_TYPE_MIRRORV6) { m_mirrorV6TableId[table_stage] = table_id; } @@ -3117,11 +3147,9 @@ bool AclOrch::removeAclTable(string table_id) auto type = m_AclTables[table_oid].type; sai_acl_stage_t sai_stage = (stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; - gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, sai_stage, SAI_ACL_BIND_POINT_TYPE_PORT, table_oid); - - if (type != ACL_TABLE_PFCWD) + for (const auto& bpointType: type.getBindPointTypes()) { - gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, sai_stage, SAI_ACL_BIND_POINT_TYPE_LAG, table_oid); + gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, sai_stage, bpointType, table_oid); } SWSS_LOG_NOTICE("Successfully deleted ACL table %s", table_id.c_str()); @@ -3156,6 +3184,45 @@ bool AclOrch::removeAclTable(string table_id) } } +bool AclOrch::addAclTableType(const AclTableType& tableType) +{ + SWSS_LOG_ENTER(); + + if (tableType.getName().empty()) + { + SWSS_LOG_ERROR("Received table type without a name"); + return false; + } + + if (m_AclTableTypes.find(tableType.getName()) != m_AclTableTypes.end()) + { + SWSS_LOG_ERROR("Table type %s already exists", tableType.getName().c_str()); + return false; + } + + m_AclTableTypes.emplace(tableType.getName(), tableType); + return true; +} + +bool AclOrch::removeAclTableType(const string& tableTypeName) +{ + // It is Ok to remove table type that is in use by AclTable. + // AclTable holds a copy of AclTableType and there is no + // SAI object associated with AclTableType. + // So it is no harm to remove it without validation. + // The upper layer can although ensure that + // user does not remove table type that is referenced + // by an ACL table. + auto erased = m_AclTableTypes.erase(tableTypeName); + + if (!erased) + { + SWSS_LOG_ERROR("Unknown table type %s", tableTypeName.c_str()); + } + + return erased; +} + bool AclOrch::addAclRule(shared_ptr newRule, string table_id) { sai_object_id_t table_oid = getTableById(table_id); @@ -3329,7 +3396,7 @@ bool AclOrch::isCombinedMirrorV6Table() return m_isCombinedMirrorV6Table; } -bool AclOrch::isAclMirrorTableSupported(acl_table_type_t type) const +bool AclOrch::isAclMirrorTableSupported(string type) const { const auto &cit = m_mirrorTableCapabilities.find(type); if (cit == m_mirrorTableCapabilities.cend()) @@ -3340,6 +3407,26 @@ bool AclOrch::isAclMirrorTableSupported(acl_table_type_t type) const return cit->second; } +bool AclOrch::isAclMirrorV4Supported() const +{ + return isAclMirrorTableSupported(TABLE_TYPE_MIRROR); +} + +bool AclOrch::isAclMirrorV6Supported() const +{ + return isAclMirrorTableSupported(TABLE_TYPE_MIRRORV6); +} + +bool AclOrch::isAclActionListMandatoryOnTableCreation(acl_stage_type_t stage) const +{ + const auto& it = m_aclCapabilities.find(stage); + if (it == m_aclCapabilities.cend()) + { + return false; + } + return it->second.isActionListMandatoryOnTableCreation; +} + bool AclOrch::isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const { const auto& it = m_aclCapabilities.find(stage); @@ -3347,7 +3434,8 @@ bool AclOrch::isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t { return false; } - return it->second.find(action) != it->second.cend(); + const auto& actionCapability = it->second; + return actionCapability.actionList.find(action) != actionCapability.actionList.cend(); } bool AclOrch::isAclActionEnumValueSupported(sai_acl_action_type_t action, sai_acl_action_parameter_t param) const @@ -3378,6 +3466,7 @@ void AclOrch::doAclTableTask(Consumer &consumer) if (op == SET_COMMAND) { AclTable newTable(this); + string tableTypeName; bool bAllAttributesOk = true; newTable.id = table_id; @@ -3395,7 +3484,7 @@ void AclOrch::doAclTableTask(Consumer &consumer) } else if (attr_name == ACL_TABLE_TYPE) { - if (!processAclTableType(attr_value, newTable.type)) + if (!processAclTableType(attr_value, tableTypeName)) { SWSS_LOG_ERROR("Failed to process ACL table %s type", table_id.c_str()); @@ -3436,6 +3525,15 @@ void AclOrch::doAclTableTask(Consumer &consumer) } } + auto tableType = getAclTableType(tableTypeName); + if (!tableType) + { + it++; + continue; + } + + newTable.validateAddType(*tableType); + // validate and create/update ACL Table if (bAllAttributesOk && newTable.validate()) { @@ -3445,7 +3543,7 @@ void AclOrch::doAclTableTask(Consumer &consumer) sai_object_id_t table_oid = getTableById(table_id); if (table_oid != SAI_NULL_OBJECT_ID && - !isAclTableTypeUpdated(newTable.type, + !isAclTableTypeUpdated(newTable.type.getName(), m_AclTables[table_oid]) && !isAclTableStageUpdated(newTable.stage, m_AclTables[table_oid])) @@ -3526,8 +3624,7 @@ void AclOrch::doAclRuleTask(Consumer &consumer) sai_object_id_t table_oid = getTableById(table_id); /* ACL table is not yet created or ACL table is a control plane table */ - /* TODO: Remove ACL_TABLE_UNKNOWN as a table with this type cannot be successfully created */ - if (table_oid == SAI_NULL_OBJECT_ID || m_AclTables[table_oid].type == ACL_TABLE_UNKNOWN) + if (table_oid == SAI_NULL_OBJECT_ID) { /* Skip the control plane rules */ @@ -3543,17 +3640,17 @@ void AclOrch::doAclRuleTask(Consumer &consumer) continue; } - auto type = m_AclTables[table_oid].type; + auto type = m_AclTables[table_oid].type.getName(); auto stage = m_AclTables[table_oid].stage; - if (type == ACL_TABLE_MIRROR || type == ACL_TABLE_MIRRORV6) + if (type == TABLE_TYPE_MIRROR || type == TABLE_TYPE_MIRRORV6) { - type = table_id == m_mirrorTableId[stage] ? ACL_TABLE_MIRROR : ACL_TABLE_MIRRORV6; + type = table_id == m_mirrorTableId[stage] ? TABLE_TYPE_MIRROR : TABLE_TYPE_MIRRORV6; } try { - newRule = AclRule::makeShared(type, this, m_mirrorOrch, m_dTelOrch, rule_id, table_id, t); + newRule = AclRule::makeShared(this, m_mirrorOrch, m_dTelOrch, rule_id, table_id, t); } catch (exception &e) { @@ -3601,7 +3698,7 @@ void AclOrch::doAclRuleTask(Consumer &consumer) if (bHasTCPFlag && !bHasIPProtocol) { string attr_name; - if (type == ACL_TABLE_MIRRORV6 || type == ACL_TABLE_L3V6) + if (type == TABLE_TYPE_MIRRORV6 || type == TABLE_TYPE_L3V6) { attr_name = MATCH_NEXT_HEADER; } @@ -3650,6 +3747,42 @@ void AclOrch::doAclRuleTask(Consumer &consumer) } } +void AclOrch::doAclTableTypeTask(Consumer &consumer) +{ + SWSS_LOG_ENTER(); + + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) + { + auto keyOpFieldValues = it->second; + auto key = kfvKey(keyOpFieldValues); + auto op = kfvOp(keyOpFieldValues); + + if (op == SET_COMMAND) + { + AclTableTypeBuilder builder; + if (!AclTableTypeParser().parse(key, kfvFieldsValues(keyOpFieldValues), builder)) + { + SWSS_LOG_ERROR("Failed to parse ACL table type configuration %s", key.c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + + addAclTableType(builder.build()); + } + else if (op == DEL_COMMAND) + { + removeAclTableType(key); + } + else + { + SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); + } + + it = consumer.m_toSync.erase(it); + } +} + bool AclOrch::processAclTablePorts(string portList, AclTable &aclTable) { SWSS_LOG_ENTER(); @@ -3683,39 +3816,26 @@ bool AclOrch::processAclTablePorts(string portList, AclTable &aclTable) return true; } -bool AclOrch::isAclTableTypeUpdated(acl_table_type_t table_type, AclTable &t) +bool AclOrch::isAclTableTypeUpdated(string table_type, AclTable &t) { - if (m_isCombinedMirrorV6Table && (table_type == ACL_TABLE_MIRROR || table_type == ACL_TABLE_MIRRORV6)) + if (m_isCombinedMirrorV6Table && (table_type == TABLE_TYPE_MIRROR || table_type == TABLE_TYPE_MIRRORV6)) { - // ACL_TABLE_MIRRORV6 and ACL_TABLE_MIRROR should be treated as same type in combined scenario - return !(t.type == ACL_TABLE_MIRROR || t.type == ACL_TABLE_MIRRORV6); + // TABLE_TYPE_MIRRORV6 and TABLE_TYPE_MIRROR should be treated as same type in combined scenario + return !(t.type.getName() == TABLE_TYPE_MIRROR || t.type.getName() == TABLE_TYPE_MIRRORV6); } - return (table_type != t.type); + return (table_type != t.type.getName()); } -bool AclOrch::processAclTableType(string type, acl_table_type_t &table_type) +bool AclOrch::processAclTableType(string type, string &out_table_type) { SWSS_LOG_ENTER(); - auto iter = aclTableTypeLookUp.find(to_upper(type)); - - if (iter == aclTableTypeLookUp.end()) + if (type.empty()) { return false; } - table_type = iter->second; - - // Mirror table check procedure - if (table_type == ACL_TABLE_MIRROR || table_type == ACL_TABLE_MIRRORV6) - { - // Check the switch capability - if (!m_mirrorTableCapabilities[table_type]) - { - SWSS_LOG_ERROR("Mirror table type %s is not supported", type.c_str()); - return false; - } - } + out_table_type = type; return true; } @@ -3742,6 +3862,18 @@ bool AclOrch::processAclTableStage(string stage, acl_stage_type_t &acl_stage) return true; } +const AclTableType* AclOrch::getAclTableType(const string& tableTypeName) const +{ + auto it = m_AclTableTypes.find(to_upper(tableTypeName)); + if (it == m_AclTableTypes.end()) + { + SWSS_LOG_INFO("Failed to find ACL table type %s", tableTypeName.c_str()); + return nullptr; + } + + return &it->second; +} + sai_object_id_t AclOrch::getTableById(string table_id) { SWSS_LOG_ENTER(); @@ -3867,220 +3999,61 @@ sai_status_t AclOrch::bindAclTable(AclTable &aclTable, bool bind) return status; } -sai_status_t AclOrch::createDTelWatchListTables() +void AclOrch::createDTelWatchListTables() { SWSS_LOG_ENTER(); - AclTable flowWLTable, dropWLTable; - sai_object_id_t table_oid; - - sai_status_t status; - sai_attribute_t attr; - vector table_attrs; - - /* Create Flow watchlist ACL table */ - - flowWLTable.id = TABLE_TYPE_DTEL_FLOW_WATCHLIST; - flowWLTable.type = ACL_TABLE_DTEL_FLOW_WATCHLIST; - flowWLTable.description = "Dataplane Telemetry Flow Watchlist table"; - - attr.id = SAI_ACL_TABLE_ATTR_ACL_STAGE; - attr.value.s32 = SAI_ACL_STAGE_INGRESS; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST; - vector bpoint_list; - bpoint_list.push_back(SAI_ACL_BIND_POINT_TYPE_SWITCH); - attr.value.s32list.count = 1; - attr.value.s32list.list = bpoint_list.data(); - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_SRC_IP; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_DST_IP; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_TUNNEL_VNI; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_INNER_SRC_IP; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_INNER_DST_IP; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_ACL_ACTION_TYPE_LIST; - int32_t acl_action_list[4]; - acl_action_list[0] = SAI_ACL_ACTION_TYPE_ACL_DTEL_FLOW_OP; - acl_action_list[1] = SAI_ACL_ACTION_TYPE_DTEL_INT_SESSION; - acl_action_list[2] = SAI_ACL_ACTION_TYPE_DTEL_REPORT_ALL_PACKETS; - acl_action_list[3] = SAI_ACL_ACTION_TYPE_DTEL_FLOW_SAMPLE_PERCENT; - attr.value.s32list.count = 4; - attr.value.s32list.list = acl_action_list; - table_attrs.push_back(attr); - - status = sai_acl_api->create_acl_table(&table_oid, gSwitchId, (uint32_t)table_attrs.size(), table_attrs.data()); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to create table %s", flowWLTable.description.c_str()); - if (handleSaiCreateStatus(SAI_API_ACL, status) != task_success) - { - return status; - } - } - - gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, SAI_ACL_STAGE_INGRESS, SAI_ACL_BIND_POINT_TYPE_SWITCH); - m_AclTables[table_oid] = flowWLTable; - SWSS_LOG_INFO("Successfully created ACL table %s, oid: %" PRIx64, flowWLTable.description.c_str(), table_oid); - - /* Create Drop watchlist ACL table */ - - table_attrs.clear(); - - dropWLTable.id = TABLE_TYPE_DTEL_DROP_WATCHLIST; - dropWLTable.type = ACL_TABLE_DTEL_DROP_WATCHLIST; - dropWLTable.description = "Dataplane Telemetry Drop Watchlist table"; - - attr.id = SAI_ACL_TABLE_ATTR_ACL_STAGE; - attr.value.s32 = SAI_ACL_STAGE_INGRESS; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST; - bpoint_list.clear(); - bpoint_list.push_back(SAI_ACL_BIND_POINT_TYPE_SWITCH); - attr.value.s32list.count = 1; - attr.value.s32list.list = bpoint_list.data(); - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_SRC_IP; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_DST_IP; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT; - attr.value.booldata = true; - table_attrs.push_back(attr); + AclTableTypeBuilder builder; - attr.id = SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL; - attr.value.booldata = true; - table_attrs.push_back(attr); - - attr.id = SAI_ACL_TABLE_ATTR_ACL_ACTION_TYPE_LIST; - acl_action_list[0] = SAI_ACL_ACTION_TYPE_DTEL_DROP_REPORT_ENABLE; - acl_action_list[1] = SAI_ACL_ACTION_TYPE_DTEL_TAIL_DROP_REPORT_ENABLE; - attr.value.s32list.count = 2; - attr.value.s32list.list = acl_action_list; - table_attrs.push_back(attr); + AclTable flowWLTable(this, TABLE_TYPE_DTEL_FLOW_WATCHLIST); + AclTable dropWLTable(this, TABLE_TYPE_DTEL_DROP_WATCHLIST); - status = sai_acl_api->create_acl_table(&table_oid, gSwitchId, (uint32_t)table_attrs.size(), table_attrs.data()); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to create table %s", dropWLTable.description.c_str()); - if (handleSaiCreateStatus(SAI_API_ACL, status) != task_success) - { - return status; - } - } + flowWLTable.validateAddStage(ACL_STAGE_INGRESS); + flowWLTable.validateAddType(builder + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_SWITCH) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TUNNEL_VNI) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_INNER_SRC_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_INNER_DST_IP) + .withAction(SAI_ACL_ACTION_TYPE_ACL_DTEL_FLOW_OP) + .withAction(SAI_ACL_ACTION_TYPE_DTEL_INT_SESSION) + .withAction(SAI_ACL_ACTION_TYPE_DTEL_REPORT_ALL_PACKETS) + .withAction(SAI_ACL_ACTION_TYPE_DTEL_FLOW_SAMPLE_PERCENT) + .build() + ); + flowWLTable.setDescription("Dataplane Telemetry Flow Watchlist table"); - gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, SAI_ACL_STAGE_INGRESS, SAI_ACL_BIND_POINT_TYPE_SWITCH); - m_AclTables[table_oid] = dropWLTable; - SWSS_LOG_INFO("Successfully created ACL table %s, oid: %" PRIx64, dropWLTable.description.c_str(), table_oid); + dropWLTable.validateAddStage(ACL_STAGE_INGRESS); + dropWLTable.validateAddType(builder + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_SWITCH) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) + .withAction(SAI_ACL_ACTION_TYPE_DTEL_DROP_REPORT_ENABLE) + .withAction(SAI_ACL_ACTION_TYPE_DTEL_TAIL_DROP_REPORT_ENABLE) + .build() + ); + dropWLTable.setDescription("Dataplane Telemetry Drop Watchlist table"); - return SAI_STATUS_SUCCESS; + addAclTable(flowWLTable); + addAclTable(dropWLTable); } -sai_status_t AclOrch::deleteDTelWatchListTables() +void AclOrch::deleteDTelWatchListTables() { SWSS_LOG_ENTER(); - AclTable flowWLTable(this), dropWLTable(this); - sai_object_id_t table_oid; - string table_id = TABLE_TYPE_DTEL_FLOW_WATCHLIST; - - sai_status_t status; - - table_oid = getTableById(table_id); - - if (table_oid == SAI_NULL_OBJECT_ID) - { - SWSS_LOG_INFO("Failed to find ACL table %s", table_id.c_str()); - return SAI_STATUS_FAILURE; - } - - status = sai_acl_api->remove_acl_table(table_oid); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to delete table %s", table_id.c_str()); - if (handleSaiRemoveStatus(SAI_API_ACL, status) != task_success) - { - return status; - } - } - - gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, SAI_ACL_STAGE_INGRESS, SAI_ACL_BIND_POINT_TYPE_SWITCH, table_oid); - m_AclTables.erase(table_oid); - - table_id = TABLE_TYPE_DTEL_DROP_WATCHLIST; - - table_oid = getTableById(table_id); - - if (table_oid == SAI_NULL_OBJECT_ID) - { - SWSS_LOG_INFO("Failed to find ACL table %s", table_id.c_str()); - return SAI_STATUS_FAILURE; - } - - status = sai_acl_api->remove_acl_table(table_oid); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to delete table %s", table_id.c_str()); - if (handleSaiRemoveStatus(SAI_API_ACL, status) != task_success) - { - return status; - } - } - - gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, SAI_ACL_STAGE_INGRESS, SAI_ACL_BIND_POINT_TYPE_SWITCH, table_oid); - m_AclTables.erase(table_oid); - - return SAI_STATUS_SUCCESS; + removeAclTable(TABLE_TYPE_DTEL_FLOW_WATCHLIST); + removeAclTable(TABLE_TYPE_DTEL_DROP_WATCHLIST); } bool AclOrch::getAclBindPortId(Port &port, sai_object_id_t &port_id) diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index f2a5e0826b..c6fcf4b178 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -52,6 +52,9 @@ #define MATCH_INNER_L4_SRC_PORT "INNER_L4_SRC_PORT" #define MATCH_INNER_L4_DST_PORT "INNER_L4_DST_PORT" +#define BIND_POINT_TYPE_PORT "PORT" +#define BIND_POINT_TYPE_PORTCHANNEL "PORTCHANNEL" + #define ACTION_PACKET_ACTION "PACKET_ACTION" #define ACTION_REDIRECT_ACTION "REDIRECT_ACTION" #define ACTION_DO_NOT_NAT_ACTION "DO_NOT_NAT_ACTION" @@ -94,14 +97,67 @@ #define RULE_OPER_ADD 0 #define RULE_OPER_DELETE 1 +struct AclActionCapabilities +{ + set actionList; + bool isActionListMandatoryOnTableCreation {false}; +}; + typedef map acl_rule_attr_lookup_t; +typedef map acl_bind_point_type_lookup_t; typedef map acl_ip_type_lookup_t; typedef map acl_dtel_flow_op_type_lookup_t; typedef map acl_packet_action_lookup_t; typedef tuple acl_range_properties_t; -typedef map> acl_capabilities_t; +typedef map acl_capabilities_t; typedef map> acl_action_enum_values_capabilities_t; +class AclTableType +{ +public: + string getName() const; + const set& getBindPointTypes() const; + const set& getMatches() const; + const set& getRangeTypes() const; + const set& getActions() const; + +private: + friend class AclTableTypeBuilder; + + string m_name; + set m_bpointTypes; + set m_enabledMatches; + set m_rangeTypes; + set m_aclAcitons; +}; + +class AclTableTypeBuilder +{ +public: + AclTableTypeBuilder& withName(string name); + AclTableTypeBuilder& withBindPointType(sai_acl_bind_point_type_t bpointType); + AclTableTypeBuilder& withMatch(sai_acl_table_attr_t matchField); + AclTableTypeBuilder& withAction(sai_acl_action_type_t action); + AclTableTypeBuilder& withRangeMatch(sai_acl_range_type_t rangeType); + AclTableType build(); + +private: + AclTableType m_tableType; +}; + +class AclTableTypeParser +{ +public: + bool parse( + const string& key, + const vector& fieldValues, + AclTableTypeBuilder& builder); +private: + bool parseAclTableTypeMatches(const string& value, AclTableTypeBuilder& builder); + bool parseAclTableTypeActions(const string& value, AclTableTypeBuilder& builder); + bool parseAclTableTypeBindPointTypes(const string& value, AclTableTypeBuilder& builder); +}; + class AclOrch; class AclRange @@ -151,14 +207,16 @@ struct AclRuleCounters } }; +class AclTable; + class AclRule { public: - AclRule(AclOrch *pAclOrch, string rule, string table, acl_table_type_t type, bool createCounter = true); + AclRule(AclOrch *pAclOrch, string rule, string table, bool createCounter = true); virtual bool validateAddPriority(string attr_name, string attr_value); virtual bool validateAddMatch(string attr_name, string attr_value); virtual bool validateAddAction(string attr_name, string attr_value); - virtual bool validate() = 0; + virtual bool validate(); bool processIpType(string type, sai_uint32_t &ip_type); inline static void setRulePriorities(sai_uint32_t min, sai_uint32_t max) { @@ -168,6 +226,7 @@ class AclRule virtual bool create(); virtual bool remove(); + virtual void update(SubjectType, void *) = 0; virtual void updateInPorts(); @@ -175,27 +234,20 @@ class AclRule virtual bool disableCounter(); virtual AclRuleCounters getCounters(); - string getId() - { - return m_id; - } - - string getTableId() - { - return m_tableId; - } + string getId(); + string getTableId(); sai_object_id_t getCounterOid() { return m_counterOid; } - vector getInPorts() + vector getInPorts() { return m_inPorts; } - static shared_ptr makeShared(acl_table_type_t type, AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple&); + static shared_ptr makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple&); virtual ~AclRule() {} protected: @@ -211,9 +263,7 @@ class AclRule static sai_uint32_t m_maxPriority; AclOrch *m_pAclOrch; string m_id; - string m_tableId; - acl_table_type_t m_tableType; - sai_object_id_t m_tableOid; + const AclTable* m_pTable {nullptr}; sai_object_id_t m_ruleOid; sai_object_id_t m_counterOid; uint32_t m_priority; @@ -229,46 +279,23 @@ class AclRule bool m_createCounter; }; -class AclRuleL3: public AclRule +class AclRulePacket: public AclRule { public: - AclRuleL3(AclOrch *m_pAclOrch, string rule, string table, acl_table_type_t type, bool createCounter = true); + AclRulePacket(AclOrch *m_pAclOrch, string rule, string table, bool createCounter = true); bool validateAddAction(string attr_name, string attr_value); - bool validateAddMatch(string attr_name, string attr_value); bool validate(); void update(SubjectType, void *); protected: sai_object_id_t getRedirectObjectId(const string& redirect_param); }; -class AclRuleL3V6: public AclRuleL3 -{ -public: - AclRuleL3V6(AclOrch *m_pAclOrch, string rule, string table, acl_table_type_t type); - bool validateAddMatch(string attr_name, string attr_value); -}; - -class AclRulePfcwd: public AclRuleL3 -{ -public: - AclRulePfcwd(AclOrch *m_pAclOrch, string rule, string table, acl_table_type_t type, bool createCounter = false); - bool validateAddMatch(string attr_name, string attr_value); -}; - -class AclRuleMux: public AclRuleL3 -{ -public: - AclRuleMux(AclOrch *m_pAclOrch, string rule, string table, acl_table_type_t type, bool createCounter = false); - bool validateAddMatch(string attr_name, string attr_value); -}; - class AclRuleMirror: public AclRule { public: - AclRuleMirror(AclOrch *m_pAclOrch, MirrorOrch *m_pMirrorOrch, string rule, string table, acl_table_type_t type); + AclRuleMirror(AclOrch *m_pAclOrch, MirrorOrch *m_pMirrorOrch, string rule, string table); bool validateAddAction(string attr_name, string attr_value); - bool validateAddMatch(string attr_name, string attr_value); bool validate(); bool create(); bool remove(); @@ -285,7 +312,7 @@ class AclRuleMirror: public AclRule class AclRuleDTelFlowWatchListEntry: public AclRule { public: - AclRuleDTelFlowWatchListEntry(AclOrch *m_pAclOrch, DTelOrch *m_pDTelOrch, string rule, string table, acl_table_type_t type); + AclRuleDTelFlowWatchListEntry(AclOrch *m_pAclOrch, DTelOrch *m_pDTelOrch, string rule, string table); bool validateAddAction(string attr_name, string attr_value); bool validate(); bool create(); @@ -302,7 +329,7 @@ class AclRuleDTelFlowWatchListEntry: public AclRule class AclRuleDTelDropWatchListEntry: public AclRule { public: - AclRuleDTelDropWatchListEntry(AclOrch *m_pAclOrch, DTelOrch *m_pDTelOrch, string rule, string table, acl_table_type_t type); + AclRuleDTelDropWatchListEntry(AclOrch *m_pAclOrch, DTelOrch *m_pDTelOrch, string rule, string table); bool validateAddAction(string attr_name, string attr_value); bool validate(); void update(SubjectType, void *); @@ -311,14 +338,6 @@ class AclRuleDTelDropWatchListEntry: public AclRule DTelOrch *m_pDTelOrch; }; -class AclRuleMclag: public AclRuleL3 -{ -public: - AclRuleMclag(AclOrch *m_pAclOrch, string rule, string table, acl_table_type_t type, bool createCounter = false); - bool validateAddMatch(string attr_name, string attr_value); - bool validate(); -}; - class AclTable { public: @@ -328,18 +347,21 @@ class AclTable AclTable() = default; ~AclTable() = default; - sai_object_id_t getOid() { return m_oid; } - string getId() { return id; } + sai_object_id_t getOid() const { return m_oid; } + string getId() const { return id; } void setDescription(const string &value) { description = value; } const string& getDescription() const { return description; } - bool validateAddType(const acl_table_type_t &value); + bool validateAddType(const AclTableType &tableType); bool validateAddStage(const acl_stage_type_t &value); bool validateAddPorts(const unordered_set &value); bool validate(); bool create(); + bool validateAclRuleMatch(sai_attribute_t attr) const; + bool validateAclRuleAction(sai_attribute_t attr) const; + // Bind the ACL table to a port which is already linked bool bind(sai_object_id_t portOid); // Unbind the ACL table to a port which is already linked @@ -365,7 +387,7 @@ class AclTable string id; string description; - acl_table_type_t type = ACL_TABLE_UNKNOWN; + AclTableType type; acl_stage_type_t stage = ACL_STAGE_INGRESS; // Map port oid to group member oid @@ -386,6 +408,7 @@ class AclOrch : public Orch, public Observer { public: AclOrch(vector& connectors, + DBConnector *m_stateDb, SwitchOrch *m_switchOrch, PortsOrch *portOrch, MirrorOrch *mirrorOrch, @@ -397,6 +420,7 @@ class AclOrch : public Orch, public Observer sai_object_id_t getTableById(string table_id); const AclTable* getTableByOid(sai_object_id_t oid) const; + const AclTableType* getAclTableType(const std::string& tableTypeName) const; static swss::Table& getCountersTable() { @@ -411,6 +435,8 @@ class AclOrch : public Orch, public Observer bool addAclTable(AclTable &aclTable); bool removeAclTable(string table_id); + bool addAclTableType(const AclTableType& tableType); + bool removeAclTableType(const string& tableTypeName); bool updateAclTable(AclTable ¤tTable, AclTable &newTable); bool updateAclTable(string table_id, AclTable &table); bool addAclRule(shared_ptr aclRule, string table_id); @@ -420,15 +446,18 @@ class AclOrch : public Orch, public Observer AclRule* getAclRule(string table_id, string rule_id); bool isCombinedMirrorV6Table(); - bool isAclMirrorTableSupported(acl_table_type_t type) const; + bool isAclMirrorV6Supported() const; + bool isAclMirrorV4Supported() const; + bool isAclMirrorTableSupported(string type) const; + bool isAclActionListMandatoryOnTableCreation(acl_stage_type_t stage) const; bool isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const; bool isAclActionEnumValueSupported(sai_acl_action_type_t action, sai_acl_action_parameter_t param) const; bool m_isCombinedMirrorV6Table = true; - map m_mirrorTableCapabilities; + map m_mirrorTableCapabilities; static sai_acl_action_type_t getAclActionFromAclEntry(sai_acl_entry_attr_t attr); - + // Get the OID for the ACL bind point for a given port static bool getAclBindPortId(Port& port, sai_object_id_t& port_id); @@ -443,8 +472,10 @@ class AclOrch : public Orch, public Observer void doTask(Consumer &consumer); void doAclTableTask(Consumer &consumer); void doAclRuleTask(Consumer &consumer); + void doAclTableTypeTask(Consumer &consumer); void doTask(SelectableTimer &timer); void init(vector& connectors, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch); + void initDefaultTableTypes(); void queryMirrorTableCapability(); void queryAclActionCapability(); @@ -462,10 +493,10 @@ class AclOrch : public Orch, public Observer sai_status_t bindAclTable(AclTable &aclTable, bool bind = true); sai_status_t deleteUnbindAclTable(sai_object_id_t table_oid); - bool isAclTableTypeUpdated(acl_table_type_t table_type, AclTable &aclTable); - bool processAclTableType(string type, acl_table_type_t &table_type); + bool isAclTableTypeUpdated(string table_type, AclTable &aclTable); bool isAclTableStageUpdated(acl_stage_type_t acl_stage, AclTable &aclTable); bool processAclTableStage(string stage, acl_stage_type_t &acl_stage); + bool processAclTableType(string type, string &out_table_type); bool processAclTablePorts(string portList, AclTable &aclTable); bool validateAclTable(AclTable &aclTable); bool updateAclTablePorts(AclTable &newTable, AclTable &curTable); @@ -473,12 +504,13 @@ class AclOrch : public Orch, public Observer AclTable &curT, set &addSet, set &delSet); - sai_status_t createDTelWatchListTables(); - sai_status_t deleteDTelWatchListTables(); + void createDTelWatchListTables(); + void deleteDTelWatchListTables(); map m_AclTables; // TODO: Move all ACL tables into one map: name -> instance map m_ctrlAclTables; + map m_AclTableTypes; static mutex m_countersMutex; static condition_variable m_sleepGuard; @@ -486,6 +518,8 @@ class AclOrch : public Orch, public Observer static DBConnector m_db; static Table m_countersTable; + Table m_aclStageCapabilityTable; + map m_mirrorTableId; map m_mirrorV6TableId; diff --git a/orchagent/acltable.h b/orchagent/acltable.h index 44d6ea4dbf..081170984f 100644 --- a/orchagent/acltable.h +++ b/orchagent/acltable.h @@ -15,6 +15,10 @@ extern "C" { #define ACL_TABLE_PORTS "PORTS" #define ACL_TABLE_SERVICES "SERVICES" +#define ACL_TABLE_TYPE_MATCHES "MATCHES" +#define ACL_TABLE_TYPE_BPOINT_TYPES "BIND_POINTS" +#define ACL_TABLE_TYPE_ACTIONS "ACTIONS" + #define STAGE_INGRESS "INGRESS" #define STAGE_EGRESS "EGRESS" @@ -39,23 +43,3 @@ typedef enum } acl_stage_type_t; typedef std::unordered_map acl_stage_type_lookup_t; - -typedef enum -{ - ACL_TABLE_UNKNOWN, - ACL_TABLE_L3, - ACL_TABLE_L3V6, - ACL_TABLE_MIRROR, - ACL_TABLE_MIRRORV6, - ACL_TABLE_MIRROR_DSCP, - ACL_TABLE_PFCWD, - ACL_TABLE_CTRLPLANE, - ACL_TABLE_DTEL_FLOW_WATCHLIST, - ACL_TABLE_DTEL_DROP_WATCHLIST, - ACL_TABLE_MCLAG, - ACL_TABLE_MUX, - ACL_TABLE_DROP, - ACL_TABLE_PBH -} acl_table_type_t; - -typedef std::unordered_map acl_table_type_lookup_t; diff --git a/orchagent/muxorch.cpp b/orchagent/muxorch.cpp index eeae6b4cb2..e727fc92de 100644 --- a/orchagent/muxorch.cpp +++ b/orchagent/muxorch.cpp @@ -702,7 +702,6 @@ MuxAclHandler::MuxAclHandler(sai_object_id_t port, string alias) SWSS_LOG_ENTER(); // There is one handler instance per MUX port - acl_table_type_t table_type = ACL_TABLE_DROP; string table_name = MUX_ACL_TABLE_NAME; string rule_name = MUX_ACL_RULE_NAME; @@ -716,8 +715,8 @@ MuxAclHandler::MuxAclHandler(sai_object_id_t port, string alias) // First time handling of Mux Table, create ACL table, and bind createMuxAclTable(port, table_name); - shared_ptr newRule = - make_shared(gAclOrch, rule_name, table_name, table_type); + shared_ptr newRule = + make_shared(gAclOrch, rule_name, table_name); createMuxAclRule(newRule, table_name); } else @@ -727,8 +726,8 @@ MuxAclHandler::MuxAclHandler(sai_object_id_t port, string alias) AclRule* rule = gAclOrch->getAclRule(table_name, rule_name); if (rule == nullptr) { - shared_ptr newRule = - make_shared(gAclOrch, rule_name, table_name, table_type); + shared_ptr newRule = + make_shared(gAclOrch, rule_name, table_name); createMuxAclRule(newRule, table_name); } else @@ -769,7 +768,7 @@ void MuxAclHandler::createMuxAclTable(sai_object_id_t port, string strTable) auto inserted = acl_table_.emplace(piecewise_construct, std::forward_as_tuple(strTable), - std::forward_as_tuple()); + std::forward_as_tuple(gAclOrch, strTable)); assert(inserted.second); @@ -784,14 +783,15 @@ void MuxAclHandler::createMuxAclTable(sai_object_id_t port, string strTable) return; } - acl_table.type = ACL_TABLE_DROP; - acl_table.id = strTable; + auto dropType = gAclOrch->getAclTableType(TABLE_TYPE_DROP); + assert(dropType); + acl_table.validateAddType(*dropType); acl_table.link(port); acl_table.stage = ACL_STAGE_INGRESS; gAclOrch->addAclTable(acl_table); } -void MuxAclHandler::createMuxAclRule(shared_ptr rule, string strTable) +void MuxAclHandler::createMuxAclRule(shared_ptr rule, string strTable) { SWSS_LOG_ENTER(); diff --git a/orchagent/muxorch.h b/orchagent/muxorch.h index ed1f5bd4b1..59620b425b 100644 --- a/orchagent/muxorch.h +++ b/orchagent/muxorch.h @@ -43,7 +43,7 @@ class MuxAclHandler private: void createMuxAclTable(sai_object_id_t port, string strTable); - void createMuxAclRule(shared_ptr rule, string strTable); + void createMuxAclRule(shared_ptr rule, string strTable); // class shared dict: ACL table name -> ACL table static std::map acl_table_; diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 814729ee0c..e6d192a0c6 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -213,15 +213,19 @@ bool OrchDaemon::init() gMirrorOrch = new MirrorOrch(stateDbMirrorSession, confDbMirrorSession, gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch, policer_orch); TableConnector confDbAclTable(m_configDb, CFG_ACL_TABLE_TABLE_NAME); + TableConnector confDbAclTableType(m_configDb, CFG_ACL_TABLE_TYPE_TABLE_NAME); TableConnector confDbAclRuleTable(m_configDb, CFG_ACL_RULE_TABLE_NAME); TableConnector appDbAclTable(m_applDb, APP_ACL_TABLE_TABLE_NAME); + TableConnector appDbAclTableType(m_applDb, APP_ACL_TABLE_TYPE_TABLE_NAME); TableConnector appDbAclRuleTable(m_applDb, APP_ACL_RULE_TABLE_NAME); vector acl_table_connectors = { + confDbAclTableType, confDbAclTable, confDbAclRuleTable, appDbAclTable, - appDbAclRuleTable + appDbAclRuleTable, + appDbAclTableType, }; vector dtel_tables = { @@ -329,7 +333,9 @@ bool OrchDaemon::init() dtel_orch = new DTelOrch(m_configDb, dtel_tables, gPortsOrch); m_orchList.push_back(dtel_orch); } - gAclOrch = new AclOrch(acl_table_connectors, gSwitchOrch, gPortsOrch, gMirrorOrch, gNeighOrch, gRouteOrch, dtel_orch); + + gAclOrch = new AclOrch(acl_table_connectors, m_stateDb, + gSwitchOrch, gPortsOrch, gMirrorOrch, gNeighOrch, gRouteOrch, dtel_orch); vector mlag_tables = { { CFG_MCLAG_TABLE_NAME }, diff --git a/orchagent/pbh/pbhrule.cpp b/orchagent/pbh/pbhrule.cpp index 52812e35b6..2ae143ad53 100644 --- a/orchagent/pbh/pbhrule.cpp +++ b/orchagent/pbh/pbhrule.cpp @@ -3,7 +3,7 @@ #include "pbhrule.h" AclRulePbh::AclRulePbh(AclOrch *pAclOrch, string rule, string table, bool createCounter) : - AclRule(pAclOrch, rule, table, ACL_TABLE_PBH, createCounter) + AclRule(pAclOrch, rule, table, createCounter) { } @@ -83,12 +83,6 @@ bool AclRulePbh::validateAddAction(const sai_attribute_t &attr) return false; } - if (!AclRule::isActionSupported(attrId)) - { - SWSS_LOG_ERROR("Action %s is not supported by ASIC", attrName.c_str()); - return false; - } - m_actions[attrId] = attr.value; return true; @@ -104,7 +98,7 @@ bool AclRulePbh::validate() return false; } - return true; + return AclRule::validate(); } void AclRulePbh::update(SubjectType, void *) diff --git a/orchagent/pbhorch.cpp b/orchagent/pbhorch.cpp index 3d0b43ce01..343d66f161 100644 --- a/orchagent/pbhorch.cpp +++ b/orchagent/pbhorch.cpp @@ -180,7 +180,18 @@ bool PbhOrch::createPbhTable(const PbhTable &table) AclTable pbhTable(this->aclOrch, table.name); - if (!pbhTable.validateAddType(acl_table_type_t::ACL_TABLE_PBH)) + static const auto pbhTableType = AclTableTypeBuilder() + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_GRE_KEY) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE) + .build(); + + if (!pbhTable.validateAddType(pbhTableType)) { SWSS_LOG_ERROR("Failed to configure PBH table(%s) type", table.key.c_str()); return false; diff --git a/orchagent/pfcactionhandler.cpp b/orchagent/pfcactionhandler.cpp index cf43f57d08..9589574858 100644 --- a/orchagent/pfcactionhandler.cpp +++ b/orchagent/pfcactionhandler.cpp @@ -225,20 +225,20 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, { SWSS_LOG_ENTER(); - acl_table_type_t table_type; + string table_type; string queuestr = to_string(queueId); m_strRule = "Rule_PfcWdAclHandler_" + queuestr; // Ingress table/rule creation - table_type = ACL_TABLE_DROP; + table_type = TABLE_TYPE_DROP; m_strIngressTable = INGRESS_TABLE_DROP; auto found = m_aclTables.find(m_strIngressTable); if (found == m_aclTables.end()) { // First time of handling PFC for this queue, create ACL table, and bind createPfcAclTable(port, m_strIngressTable, true); - shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strIngressTable, table_type); + shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strIngressTable); createPfcAclRule(newRule, queueId, m_strIngressTable, port); } else @@ -246,7 +246,7 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, AclRule* rule = gAclOrch->getAclRule(m_strIngressTable, m_strRule); if (rule == nullptr) { - shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strIngressTable, table_type); + shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strIngressTable); createPfcAclRule(newRule, queueId, m_strIngressTable, port); } else @@ -256,14 +256,14 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, } // Egress table/rule creation - table_type = ACL_TABLE_PFCWD; + table_type = TABLE_TYPE_PFCWD; m_strEgressTable = "EgressTable_PfcWdAclHandler_" + queuestr; found = m_aclTables.find(m_strEgressTable); if (found == m_aclTables.end()) { // First time of handling PFC for this queue, create ACL table, and bind createPfcAclTable(port, m_strEgressTable, false); - shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strEgressTable, table_type); + shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strEgressTable); createPfcAclRule(newRule, queueId, m_strEgressTable, port); } else @@ -316,7 +316,7 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b auto inserted = m_aclTables.emplace(piecewise_construct, std::forward_as_tuple(strTable), - std::forward_as_tuple()); + std::forward_as_tuple(gAclOrch, strTable)); assert(inserted.second); @@ -332,23 +332,26 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b } aclTable.link(port); - aclTable.id = strTable; if (ingress) { - aclTable.type = ACL_TABLE_DROP; + auto dropType = gAclOrch->getAclTableType(TABLE_TYPE_DROP); + assert(dropType); + aclTable.validateAddType(*dropType); aclTable.stage = ACL_STAGE_INGRESS; } else { - aclTable.type = ACL_TABLE_PFCWD; + auto pfcwdType = gAclOrch->getAclTableType(TABLE_TYPE_PFCWD); + assert(pfcwdType); + aclTable.validateAddType(*pfcwdType); aclTable.stage = ACL_STAGE_EGRESS; } gAclOrch->addAclTable(aclTable); } -void PfcWdAclHandler::createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, sai_object_id_t portOid) +void PfcWdAclHandler::createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, sai_object_id_t portOid) { SWSS_LOG_ENTER(); diff --git a/orchagent/pfcactionhandler.h b/orchagent/pfcactionhandler.h index e381a798c6..7c5e07ae56 100644 --- a/orchagent/pfcactionhandler.h +++ b/orchagent/pfcactionhandler.h @@ -111,7 +111,7 @@ class PfcWdAclHandler: public PfcWdLossyHandler string m_strEgressTable; string m_strRule; void createPfcAclTable(sai_object_id_t port, string strTable, bool ingress); - void createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, sai_object_id_t port); + void createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, sai_object_id_t port); void updatePfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, vector port); }; diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 4cd7688883..48dac39c95 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -98,8 +98,27 @@ namespace aclorch_test TEST_F(AclTest, Create_L3_Acl_Table) { - AclTable acltable; - acltable.type = ACL_TABLE_L3; + AclTable acltable(nullptr); /* this test won't trigger a call to gAclOrch so it's nullptr */ + AclTableTypeBuilder builder; + auto l3TableType = builder.withName(TABLE_TYPE_L3) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) + .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .build(); + acltable.validateAddType(l3TableType); auto res = createAclTable(acltable); ASSERT_TRUE(res->ret_val); @@ -138,7 +157,7 @@ namespace aclorch_test vector acl_table_connectors = { confDbAclTable, confDbAclRuleTable }; - m_aclOrch = new AclOrch(acl_table_connectors, switchOrch, portsOrch, mirrorOrch, + m_aclOrch = new AclOrch(acl_table_connectors, state_db, switchOrch, portsOrch, mirrorOrch, neighOrch, routeOrch); } @@ -152,6 +171,15 @@ namespace aclorch_test return m_aclOrch; } + void doAclTableTypeTask(const deque &entries) + { + auto consumer = unique_ptr(new Consumer( + new swss::ConsumerStateTable(config_db, CFG_ACL_TABLE_TYPE_TABLE_NAME, 1, 1), m_aclOrch, CFG_ACL_TABLE_TYPE_TABLE_NAME)); + + consumer->addToSync(entries); + static_cast(m_aclOrch)->doTask(*consumer); + } + void doAclTableTask(const deque &entries) { auto consumer = unique_ptr(new Consumer( @@ -175,6 +203,27 @@ namespace aclorch_test return m_aclOrch->getTableById(table_id); } + const AclRule* getAclRule(string tableName, string ruleName) + { + return m_aclOrch->getAclRule(tableName, ruleName); + } + + const AclTable* getTableByOid(sai_object_id_t oid) + { + return m_aclOrch->getTableByOid(oid); + } + + const AclTable* getAclTable(string tableName) + { + auto oid = m_aclOrch->getTableById(tableName); + return getTableByOid(oid); + } + + const AclTableType* getAclTableType(string tableTypeName) + { + return m_aclOrch->getAclTableType(tableTypeName); + } + const map &getAclTables() const { return Portal::AclOrchInternal::getAclTables(m_aclOrch); @@ -438,24 +487,22 @@ namespace aclorch_test fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS", "true" }); fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE", "2:SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE,SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE" }); - switch (acl_table.type) + if (acl_table.type.getName() == TABLE_TYPE_L3) { - case ACL_TABLE_L3: - fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE", "true" }); - fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_SRC_IP", "true" }); - fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_DST_IP", "true" }); - fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL", "true" }); - break; - - case ACL_TABLE_L3V6: - fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6", "true" }); - fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6", "true" }); - fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER", "true" }); - break; - - default: - // We shouldn't get here. Will continue to add more test cases ...; - ; + fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE", "true" }); + fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_SRC_IP", "true" }); + fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_DST_IP", "true" }); + fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL", "true" }); + } + else if (acl_table.type.getName() == TABLE_TYPE_L3V6) + { + fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6", "true" }); + fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6", "true" }); + fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER", "true" }); + } + else + { + // We shouldn't get here. Will continue to add more test cases ...; } if (ACL_STAGE_INGRESS == acl_table.stage) @@ -482,19 +529,17 @@ namespace aclorch_test fields.push_back({ "SAI_ACL_ENTRY_ATTR_ADMIN_STATE", "true" }); fields.push_back({ "SAI_ACL_ENTRY_ATTR_ACTION_COUNTER", counter_id }); - switch (acl_table.type) + if (acl_table.type.getName() == TABLE_TYPE_L3) { - case ACL_TABLE_L3: - fields.push_back({ "SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP", "1.2.3.4&mask:255.255.255.255" }); - break; - - case ACL_TABLE_L3V6: - fields.push_back({ "SAI_ACL_ENTRY_ATTR_FIELD_SRC_IPV6", "::1.2.3.4&mask:ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff" }); - break; - - default: - // We shouldn't get here. Will continue to add more test cases ... - ; + fields.push_back({ "SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP", "1.2.3.4&mask:255.255.255.255" }); + } + if (acl_table.type.getName() == TABLE_TYPE_L3V6) + { + fields.push_back({ "SAI_ACL_ENTRY_ATTR_FIELD_SRC_IPV6", "::1.2.3.4&mask:ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff" }); + } + else + { + // We shouldn't get here. Will continue to add more test cases ... } return shared_ptr(new SaiAttributeList(objecttype, fields, false)); @@ -559,13 +604,17 @@ namespace aclorch_test return true; } - bool validateAclTable(sai_object_id_t acl_table_oid, const AclTable &acl_table) + bool validateAclTable(sai_object_id_t acl_table_oid, const AclTable &acl_table, shared_ptr expAttrList = nullptr) { const sai_object_type_t objecttype = SAI_OBJECT_TYPE_ACL_TABLE; - auto exp_attrlist_2 = getAclTableAttributeList(objecttype, acl_table); + if (!expAttrList) + { + expAttrList = getAclTableAttributeList(objecttype, acl_table); + + } { - auto &exp_attrlist = *exp_attrlist_2; + auto &exp_attrlist = *expAttrList; vector act_attr; @@ -631,23 +680,16 @@ namespace aclorch_test auto const &resourceMap = Portal::CrmOrchInternal::getResourceMap(crmOrch); // Verify the ACL tables - uint32_t crmAclTableBindingCount = 0; + ssize_t crmAclTableBindingCount = 0; for (auto const &kv: resourceMap.at(CrmResourceType::CRM_ACL_TABLE).countersMap) { crmAclTableBindingCount += kv.second.usedCounter; } - uint32_t aclorchAclTableBindingCount = 0; + ssize_t aclorchAclTableBindingCount = 0; for (auto const &kv: Portal::AclOrchInternal::getAclTables(aclOrch)) { - if (kv.second.type == ACL_TABLE_PFCWD) - { - aclorchAclTableBindingCount += 1; // port binding only - } - else - { - aclorchAclTableBindingCount += 2; // port + LAG binding - } + aclorchAclTableBindingCount += kv.second.type.getBindPointTypes().size(); } if (crmAclTableBindingCount != aclorchAclTableBindingCount) @@ -760,21 +802,7 @@ namespace aclorch_test { if (fv.first == ACL_TABLE_TYPE) { - if (fv.second == TABLE_TYPE_L3) - { - if (acl_table.type != ACL_TABLE_L3) - { - return false; - } - } - else if (fv.second == TABLE_TYPE_L3V6) - { - if (acl_table.type != ACL_TABLE_L3V6) - { - return false; - } - } - else + if (acl_table.type.getName() != fv.second) { return false; } @@ -1307,4 +1335,277 @@ namespace aclorch_test ASSERT_EQ(tableIt, orch->getAclTables().end()); } + TEST_F(AclOrchTest, AclTableType_Configuration) + { + const string aclTableTypeName = "TEST_TYPE"; + const string aclTableName = "TEST_TABLE"; + const string aclRuleName = "TEST_RULE"; + + auto orch = createAclOrch(); + + auto tableKofvt = deque( + { + { + aclTableName, + SET_COMMAND, + { + { ACL_TABLE_DESCRIPTION, "Test table" }, + { ACL_TABLE_TYPE, aclTableTypeName}, + { ACL_TABLE_STAGE, STAGE_INGRESS }, + { ACL_TABLE_PORTS, "1,2" } + } + } + } + ); + + orch->doAclTableTask(tableKofvt); + + // Table not created without table type + ASSERT_FALSE(orch->getAclTable(aclTableName)); + + orch->doAclTableTypeTask( + deque( + { + { + aclTableTypeName, + SET_COMMAND, + { + { + ACL_TABLE_TYPE_MATCHES, + string(MATCH_SRC_IP) + comma + MATCH_ETHER_TYPE + comma + MATCH_L4_SRC_PORT_RANGE + }, + { + ACL_TABLE_TYPE_BPOINT_TYPES, + string(BIND_POINT_TYPE_PORT) + comma + BIND_POINT_TYPE_PORTCHANNEL + }, + } + } + } + ) + ); + + orch->doAclTableTask(tableKofvt); + + // Table is created now + ASSERT_TRUE(orch->getAclTable(aclTableName)); + + auto fvs = vector{ + { "SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST", "2:SAI_ACL_BIND_POINT_TYPE_PORT,SAI_ACL_BIND_POINT_TYPE_LAG" }, + { "SAI_ACL_TABLE_ATTR_FIELD_SRC_IP", "true" }, + { "SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE", "true" }, + { "SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE", "1:SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE" }, + }; + + ASSERT_TRUE(validateAclTable( + orch->getAclTable(aclTableName)->getOid(), + *orch->getAclTable(aclTableName), + make_shared(SAI_OBJECT_TYPE_ACL_TABLE, fvs, false)) + ); + + orch->doAclRuleTask( + deque( + { + { + aclTableName + "|" + aclRuleName, + SET_COMMAND, + { + { MATCH_SRC_IP, "1.1.1.1/32" }, + { MATCH_L4_DST_PORT_RANGE, "80..100" }, + { ACTION_PACKET_ACTION, PACKET_ACTION_DROP }, + } + } + } + ) + ); + + // L4_DST_PORT_RANGE is not in the table type + ASSERT_FALSE(orch->getAclRule(aclTableName, aclRuleName)); + + orch->doAclRuleTask( + deque( + { + { + aclTableName + "|" + aclRuleName, + SET_COMMAND, + { + { MATCH_SRC_IP, "1.1.1.1/32" }, + { MATCH_DST_IP, "2.2.2.2/32" }, + { ACTION_PACKET_ACTION, PACKET_ACTION_DROP }, + } + } + } + ) + ); + + // DST_IP is not in the table type + ASSERT_FALSE(orch->getAclRule(aclTableName, aclRuleName)); + + orch->doAclRuleTask( + deque( + { + { + aclTableName + "|" + aclRuleName, + SET_COMMAND, + { + { MATCH_SRC_IP, "1.1.1.1/32" }, + { ACTION_PACKET_ACTION, PACKET_ACTION_DROP }, + } + } + } + ) + ); + + // Now it is valid for this table. + ASSERT_TRUE(orch->getAclRule(aclTableName, aclRuleName)); + + orch->doAclRuleTask( + deque( + { + { + aclTableName + "|" + aclRuleName, + DEL_COMMAND, + {} + } + } + ) + ); + + ASSERT_FALSE(orch->getAclRule(aclTableName, aclRuleName)); + + orch->doAclTableTypeTask( + deque( + { + { + aclTableTypeName, + DEL_COMMAND, + {} + } + } + ) + ); + + // Table still exists + ASSERT_TRUE(orch->getAclTable(aclTableName)); + ASSERT_FALSE(orch->getAclTableType(aclTableTypeName)); + + orch->doAclTableTask( + deque( + { + { + aclTableName, + DEL_COMMAND, + {} + } + } + ) + ); + + // Table is removed + ASSERT_FALSE(orch->getAclTable(aclTableName)); + } + + TEST_F(AclOrchTest, AclTableType_ActionValidation) + { + const string aclTableTypeName = "TEST_TYPE"; + const string aclTableName = "TEST_TABLE"; + const string aclRuleName = "TEST_RULE"; + + auto orch = createAclOrch(); + + orch->doAclTableTypeTask( + deque( + { + { + aclTableTypeName, + SET_COMMAND, + { + { + ACL_TABLE_TYPE_MATCHES, + string(MATCH_ETHER_TYPE) + comma + MATCH_L4_SRC_PORT_RANGE + comma + MATCH_L4_SRC_PORT_RANGE + }, + { + ACL_TABLE_TYPE_BPOINT_TYPES, + BIND_POINT_TYPE_PORTCHANNEL + }, + { + ACL_TABLE_TYPE_ACTIONS, + ACTION_MIRROR_INGRESS_ACTION + } + } + } + } + ) + ); + + orch->doAclTableTask( + deque( + { + { + aclTableName, + SET_COMMAND, + { + { ACL_TABLE_DESCRIPTION, "Test table" }, + { ACL_TABLE_TYPE, aclTableTypeName}, + { ACL_TABLE_STAGE, STAGE_INGRESS }, + { ACL_TABLE_PORTS, "1,2" } + } + } + } + ) + ); + + ASSERT_TRUE(orch->getAclTable(aclTableName)); + + auto fvs = vector{ + { "SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST", "1:SAI_ACL_BIND_POINT_TYPE_LAG" }, + { "SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE", "true" }, + { "SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE", "2:SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE,SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE" }, + { "SAI_ACL_TABLE_ATTR_ACL_ACTION_TYPE_LIST", "1:SAI_ACL_ACTION_TYPE_MIRROR_INGRESS" }, + }; + + ASSERT_TRUE(validateAclTable( + orch->getAclTable(aclTableName)->getOid(), + *orch->getAclTable(aclTableName), + make_shared(SAI_OBJECT_TYPE_ACL_TABLE, fvs, false)) + ); + + orch->doAclRuleTask( + deque( + { + { + aclTableName + "|" + aclRuleName, + SET_COMMAND, + { + { MATCH_ETHER_TYPE, "2048" }, + { ACTION_PACKET_ACTION, PACKET_ACTION_DROP }, + } + } + } + ) + ); + + // Packet action is not supported on this table + ASSERT_FALSE(orch->getAclRule(aclTableName, aclRuleName)); + + const auto testSessionName = "test_session"; + gMirrorOrch->createEntry(testSessionName, {}); + orch->doAclRuleTask( + deque( + { + { + aclTableName + "|" + aclRuleName, + SET_COMMAND, + { + { MATCH_ETHER_TYPE, "2048" }, + { ACTION_MIRROR_INGRESS_ACTION, testSessionName }, + } + } + } + ) + ); + + // Mirror action is supported on this table + ASSERT_TRUE(orch->getAclRule(aclTableName, aclRuleName)); + } + } // namespace nsAclOrchTest diff --git a/tests/test_acl.py b/tests/test_acl.py index d4b317bf55..c3b0fdaaf0 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -550,15 +550,12 @@ def test_ValidateAclTableBindingCrmUtilization(self, dvs, dvs_acl): class TestAclRuleValidation: """Test class for cases that check if orchagent corectly validates ACL rules input.""" - SWITCH_CAPABILITY_TABLE = "SWITCH_CAPABILITY" + ACL_STAGE_CAPABILITY_TABLE_NAME = "ACL_STAGE_CAPABILITY_TABLE" + ACL_ACTION_LIST_FIELD_NAME = "action_list" def get_acl_actions_supported(self, dvs_acl, stage): - switch_id = dvs_acl.state_db.wait_for_n_keys(self.SWITCH_CAPABILITY_TABLE, 1)[0] - switch = dvs_acl.state_db.wait_for_entry(self.SWITCH_CAPABILITY_TABLE, switch_id) - - field = "ACL_ACTIONS|{}".format(stage.upper()) - - supported_actions = switch.get(field, None) + switch = dvs_acl.state_db.wait_for_entry(self.ACL_STAGE_CAPABILITY_TABLE_NAME, stage.upper()) + supported_actions = switch.get(self.ACL_ACTION_LIST_FIELD_NAME, None) if supported_actions: supported_actions = supported_actions.split(",") From 0e85da1fd8e04bf75c14021886b8bb60d9a84148 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Fri, 22 Oct 2021 02:37:19 +0300 Subject: [PATCH 02/35] refactor acl range match validation Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 577 +++++++++++++++++--------------- orchagent/aclorch.h | 58 +++- orchagent/pbh/pbhrule.cpp | 18 +- orchagent/pbhorch.cpp | 12 +- orchagent/saihelper.h | 3 + tests/mock_tests/aclorch_ut.cpp | 34 +- 6 files changed, 402 insertions(+), 300 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index bc157ab11d..f548d63365 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -62,8 +62,8 @@ acl_rule_attr_lookup_t aclMatchLookup = { MATCH_ICMP_CODE, SAI_ACL_ENTRY_ATTR_FIELD_ICMP_CODE }, { MATCH_ICMPV6_TYPE, SAI_ACL_ENTRY_ATTR_FIELD_ICMPV6_TYPE }, { MATCH_ICMPV6_CODE, SAI_ACL_ENTRY_ATTR_FIELD_ICMPV6_CODE }, - { MATCH_L4_SRC_PORT_RANGE, (sai_acl_entry_attr_t)SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE }, - { MATCH_L4_DST_PORT_RANGE, (sai_acl_entry_attr_t)SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE }, + { MATCH_L4_SRC_PORT_RANGE, SAI_ACL_ENTRY_ATTR_FIELD_ACL_RANGE_TYPE }, + { MATCH_L4_DST_PORT_RANGE, SAI_ACL_ENTRY_ATTR_FIELD_ACL_RANGE_TYPE }, { MATCH_TUNNEL_VNI, SAI_ACL_ENTRY_ATTR_FIELD_TUNNEL_VNI }, { MATCH_INNER_ETHER_TYPE, SAI_ACL_ENTRY_ATTR_FIELD_INNER_ETHER_TYPE }, { MATCH_INNER_IP_PROTOCOL, SAI_ACL_ENTRY_ATTR_FIELD_INNER_IP_PROTOCOL }, @@ -71,6 +71,12 @@ acl_rule_attr_lookup_t aclMatchLookup = { MATCH_INNER_L4_DST_PORT, SAI_ACL_ENTRY_ATTR_FIELD_INNER_L4_DST_PORT } }; +static acl_range_type_lookup_t aclRangeTypeLookup = +{ + { MATCH_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE }, + { MATCH_L4_DST_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE }, +}; + static acl_bind_point_type_lookup_t aclBindPointTypeLookup = { { BIND_POINT_TYPE_PORT, SAI_ACL_BIND_POINT_TYPE_PORT }, @@ -160,19 +166,9 @@ static acl_ip_type_lookup_t aclIpTypeLookup = { IP_TYPE_ARP_REPLY, SAI_ACL_IP_TYPE_ARP_REPLY } }; -static bool isAclEntryFieldAttribute(sai_acl_entry_attr_t attr) -{ - return attr >= SAI_ACL_ENTRY_ATTR_FIELD_START && attr <= SAI_ACL_ENTRY_ATTR_FIELD_END; -} - -static bool isAclEntryActionAttribute(sai_acl_entry_attr_t attr) -{ - return attr >= SAI_ACL_ENTRY_ATTR_ACTION_START && attr <= SAI_ACL_ENTRY_ATTR_ACTION_END; -} - static sai_acl_table_attr_t AclEntryFieldToAclTableField(sai_acl_entry_attr_t attr) { - if (!isAclEntryFieldAttribute(attr)) + if (!IS_ATTR_ID_IN_RANGE(attr, ACL_ENTRY, FIELD)) { SWSS_LOG_THROW("ACL entry attribute is not a in a range of SAI_ACL_ENTRY_ATTR_FIELD_* attribute: %d", attr); } @@ -181,33 +177,110 @@ static sai_acl_table_attr_t AclEntryFieldToAclTableField(sai_acl_entry_attr_t at static sai_acl_action_type_t AclEntryActionToAclAction(sai_acl_entry_attr_t attr) { - if (!isAclEntryActionAttribute(attr)) + if (!IS_ATTR_ID_IN_RANGE(attr, ACL_ENTRY, ACTION)) { SWSS_LOG_THROW("ACL entry attribute is not a in a range of SAI_ACL_ENTRY_ATTR_ACTION_* attribute: %d", attr); } return static_cast(attr - SAI_ACL_ENTRY_ATTR_ACTION_START); } -static bool isAttributeValueEnum(sai_object_type_t objectType, sai_attr_id_t attr) +string getAttributeIdName(sai_object_type_t objectType, sai_attr_id_t attr) { const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, attr); if (!meta) { SWSS_LOG_THROW("Metadata null pointer returned by sai_metadata_get_attr_metadata"); } - return meta->isenum; + return meta->attridname; } -string getAttributeIdName(sai_object_type_t objectType, sai_attr_id_t attr) +AclTableMatchInterface::AclTableMatchInterface(sai_acl_table_attr_t matchField): + m_matchField(matchField) { - const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, attr); + if (!IS_ATTR_ID_IN_RANGE(m_matchField, ACL_TABLE, FIELD)) + { + SWSS_LOG_THROW("Invalid match table attribute %d", m_matchField); + } +} + +sai_acl_table_attr_t AclTableMatchInterface::getId() const +{ + return m_matchField; +} + +AclTableMatch::AclTableMatch(sai_acl_table_attr_t matchField): + AclTableMatchInterface(matchField) +{ + const auto meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_TABLE, getId()); if (!meta) { - SWSS_LOG_THROW("Metadata null pointer returned by sai_metadata_get_attr_metadata"); + SWSS_LOG_THROW("Failed to get metadata for attribute for SAI_OBJECT_TYPE_ACL_TABLE: attribute %d", getId()); } - return meta->attridname; + + if (meta->attrvaluetype != SAI_ATTR_VALUE_TYPE_BOOL) + { + SWSS_LOG_THROW("This API does not allow to set match with a non boolean value type"); + } +} + +sai_attribute_t AclTableMatch::toSaiAttribute() +{ + return sai_attribute_t{ + .id = getId(), + .value = { + .booldata = true, + }, + }; +} + +bool AclTableMatch::validateAclRuleMatch(const AclRule& rule) const +{ + // no need to validate rule configuration + return true; +} + +AclTableRangeMatch::AclTableRangeMatch(set rangeTypes): + AclTableMatchInterface(SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE), + m_rangeList(rangeTypes.begin(), rangeTypes.end()) +{ } +sai_attribute_t AclTableRangeMatch::toSaiAttribute() +{ + return sai_attribute_t{ + .id = getId(), + .value = { + .s32list = { + .count = static_cast(m_rangeList.size()), + .list = m_rangeList.data(), + }, + }, + }; +} + +bool AclTableRangeMatch::validateAclRuleMatch(const AclRule& rule) const +{ + const auto& ruleMatches = rule.getMatches(); + for (const auto& rangeIt: aclRangeTypeLookup) + { + auto rangeType = rangeIt.second; + if (ruleMatches.find(static_cast(rangeType)) == ruleMatches.end()) + { + continue; + } + + if (find(m_rangeList.begin(), m_rangeList.end(), rangeType) == m_rangeList.end()) + { + SWSS_LOG_ERROR("Range match %s is not supported on table %s", + sai_metadata_get_acl_range_type_name(rangeType), rule.getTableId().c_str()); + return false; + } + } + + return true; +} + + string AclTableType::getName() const { return m_name; @@ -218,14 +291,9 @@ const set& AclTableType::getBindPointTypes() const return m_bpointTypes; } -const set& AclTableType::getMatches() const -{ - return m_enabledMatches; -} - -const set& AclTableType::getRangeTypes() const +const map>& AclTableType::getMatches() const { - return m_rangeTypes; + return m_matches; } const set& AclTableType::getActions() const @@ -245,25 +313,9 @@ AclTableTypeBuilder& AclTableTypeBuilder::withBindPointType(sai_acl_bind_point_t return *this; } -AclTableTypeBuilder& AclTableTypeBuilder::withMatch(sai_acl_table_attr_t matchField) +AclTableTypeBuilder& AclTableTypeBuilder::withMatch(shared_ptr match) { - if (!(matchField >= SAI_ACL_TABLE_ATTR_FIELD_START && matchField <= SAI_ACL_TABLE_ATTR_FIELD_END)) - { - SWSS_LOG_THROW("Invalid match table attribute %d", matchField); - } - - const auto meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_TABLE, matchField); - if (!meta) - { - SWSS_LOG_THROW("Failed to get metadata for attribute for SAI_OBJECT_TYPE_ACL_TABLE: attribute %d", matchField); - } - - if (meta->attrvaluetype != SAI_ATTR_VALUE_TYPE_BOOL) - { - SWSS_LOG_THROW("This API does not allow to set match with a non boolean value type"); - } - - m_tableType.m_enabledMatches.insert(matchField); + m_tableType.m_matches.emplace(match->getId(), match); return *this; } @@ -273,12 +325,6 @@ AclTableTypeBuilder& AclTableTypeBuilder::withAction(sai_acl_action_type_t actio return *this; } -AclTableTypeBuilder& AclTableTypeBuilder::withRangeMatch(sai_acl_range_type_t rangeType) -{ - m_tableType.m_rangeTypes.insert(rangeType); - return *this; -} - AclTableType AclTableTypeBuilder::build() { auto tableType = m_tableType; @@ -296,6 +342,9 @@ bool AclTableTypeParser::parse(const std::string& key, { auto field = to_upper(fvField(fieldValue)); auto value = to_upper(fvValue(fieldValue)); + + SWSS_LOG_DEBUG("field %s, value %s", field.c_str(), value.c_str()); + if (field == ACL_TABLE_TYPE_MATCHES) { if (!parseAclTableTypeMatches(value, builder)) @@ -330,29 +379,35 @@ bool AclTableTypeParser::parse(const std::string& key, bool AclTableTypeParser::parseAclTableTypeMatches(const std::string& value, AclTableTypeBuilder& builder) { auto matches = tokenize(value, comma); + set saiRangeTypes; + for (const auto& match: matches) { auto matchIt = aclMatchLookup.find(match); + auto rangeMatchIt = aclRangeTypeLookup.find(match); - if (matchIt == aclMatchLookup.end()) + if (rangeMatchIt != aclRangeTypeLookup.end()) { - SWSS_LOG_ERROR("Unknown match %s", match.c_str()); - return false; + auto rangeType = rangeMatchIt->second; + saiRangeTypes.insert(rangeType); } - - auto saiMatchAttr = matchIt->second; - if (isAclEntryFieldAttribute(saiMatchAttr)) + else if (matchIt != aclMatchLookup.end()) { - auto tableAttrId = AclEntryFieldToAclTableField(saiMatchAttr); - builder.withMatch(tableAttrId); + auto saiMatch = AclEntryFieldToAclTableField(matchIt->second); + builder.withMatch(make_unique(saiMatch)); } - else /* range type */ + else { - auto saiRangeType = static_cast(matchIt->second); - builder.withRangeMatch(saiRangeType); + SWSS_LOG_ERROR("Unknown match %s", match.c_str()); + return false; } } + if (!saiRangeTypes.empty()) + { + builder.withMatch(make_unique(saiRangeTypes)); + } + return true; } @@ -671,13 +726,62 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) attr_name = MATCH_NEXT_HEADER; } - m_matches[aclMatchLookup[attr_name]] = value; + const auto ruleMatchId = aclMatchLookup[attr_name]; + const auto& rangeTypeIt = aclRangeTypeLookup.find(attr_name); - return true; + // TODO: Refactor AclRule to save range configuration in a different data structure. + // It is weird to find ACL range type inside m_matches of type sai_acl_entry_attr_t. + if (rangeTypeIt != aclRangeTypeLookup.end()) + { + m_matches[static_cast(rangeTypeIt->second)] = value; + } + else + { + m_matches[aclMatchLookup[attr_name]] = value; + } + + return m_pTable->validateAclRuleMatch(ruleMatchId, *this); } bool AclRule::validateAddAction(string attr_name, string attr_value) { + for (const auto& it: m_actions) + { + auto attrId = it.first; + auto value = it.second; + auto actionType = AclEntryActionToAclAction(attrId); + + if (!isActionSupported(attrId)) + { + SWSS_LOG_ERROR("Action %s:%s is not supported by ASIC", + attr_name.c_str(), attr_value.c_str()); + return false; + } + + // check if ACL action attribute entry parameter is an enum value + const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, attrId); + if (meta == nullptr) + { + SWSS_LOG_THROW("Metadata null pointer returned by sai_metadata_get_attr_metadata for action %s", + attr_name.c_str()); + } + if (meta->isenum) + { + // if ACL action attribute requires enum value check if value is supported by the ASIC + if (!m_pAclOrch->isAclActionEnumValueSupported(actionType, value.aclaction.parameter)) + { + SWSS_LOG_ERROR("Action %s:%s is not supported by ASIC", + attr_name.c_str(), attr_value.c_str()); + return false; + } + } + + if (!m_pTable->validateAclRuleAction(attrId, *this)) + { + return false; + } + } + return true; } @@ -697,31 +801,6 @@ bool AclRule::processIpType(string type, sai_uint32_t &ip_type) return true; } -bool AclRule::validate() -{ - for (auto matchPair: m_matches) - { - sai_attribute_t attr; - tie(attr.id, attr.value) = matchPair; - if (!m_pTable->validateAclRuleMatch(attr)) - { - return false; - } - } - - for (auto actionPair: m_actions) - { - sai_attribute_t attr; - tie(attr.id, attr.value) = actionPair; - if (!m_pTable->validateAclRuleAction(attr)) - { - return false; - } - } - - return true; -} - bool AclRule::create() { SWSS_LOG_ENTER(); @@ -851,6 +930,11 @@ void AclRule::decreaseNextHopRefCount() return; } +bool AclRule::isActionSupported(sai_acl_entry_attr_t action) const +{ + return m_pAclOrch->isAclActionSupported(m_pTable->stage, AclEntryActionToAclAction(action)); +} + bool AclRule::remove() { SWSS_LOG_ENTER(); @@ -913,16 +997,21 @@ AclRuleCounters AclRule::getCounters() return AclRuleCounters(counter_attr[0].value.u64, counter_attr[1].value.u64); } -string AclRule::getId() +string AclRule::getId() const { return m_id; } -string AclRule::getTableId() +string AclRule::getTableId() const { return m_pTable->getId(); } +const map& AclRule::getMatches() const +{ + return m_matches; +} + shared_ptr AclRule::makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple& data) { shared_ptr aclRule; @@ -1283,7 +1372,7 @@ bool AclRulePacket::validate() return false; } - return AclRule::validate(); + return true; } void AclRulePacket::update(SubjectType, void *) @@ -1336,7 +1425,7 @@ bool AclRuleMirror::validate() return false; } - return AclRule::validate(); + return true; } bool AclRuleMirror::create() @@ -1529,28 +1618,12 @@ bool AclTable::validate() } } - return true; -} - -bool AclTable::validateAclRuleMatch(sai_attribute_t attr) const -{ - auto aclEntryAttr = static_cast(attr.id); - if (isAclEntryFieldAttribute(aclEntryAttr)) - { - if (!type.getMatches().count(AclEntryFieldToAclTableField(aclEntryAttr))) - { - SWSS_LOG_ERROR("Match %s is not supported on table %s", - getAttributeIdName(SAI_OBJECT_TYPE_ACL_ENTRY, aclEntryAttr).c_str(), id.c_str()); - return false; - } - } - else /* range type */ + for (const auto& action: type.getActions()) { - auto rangeType = static_cast(attr.id); - if (!type.getRangeTypes().count(rangeType)) + if (!m_pAclOrch->isAclActionSupported(stage, action)) { - SWSS_LOG_ERROR("Range match %s is not supported on table %s", - sai_metadata_get_acl_range_type_name(rangeType), id.c_str()); + SWSS_LOG_ERROR("Action %s is not supported on table %s", + sai_metadata_get_acl_action_type_name(action), id.c_str()); return false; } } @@ -1558,28 +1631,37 @@ bool AclTable::validateAclRuleMatch(sai_attribute_t attr) const return true; } -bool AclTable::validateAclRuleAction(sai_attribute_t attr) const +bool AclTable::validateAclRuleMatch(sai_acl_entry_attr_t matchId, const AclRule& rule) const { - auto aclEntryAttr = static_cast(attr.id); - auto action = AclEntryActionToAclAction(aclEntryAttr); - if (!m_pAclOrch->isAclActionSupported(stage, action)) + const auto& tableMatches = type.getMatches(); + const auto tableMatchId = AclEntryFieldToAclTableField(matchId); + const auto tableMatchIt = tableMatches.find(tableMatchId); + if (tableMatchIt == tableMatches.end()) { - SWSS_LOG_ERROR("Action %s is not supported on table %s", - sai_metadata_get_acl_action_type_name(action), id.c_str()); + SWSS_LOG_ERROR("Match %s in rule %s is not supported by table %s", + getAttributeIdName(SAI_OBJECT_TYPE_ACL_ENTRY, matchId).c_str(), + rule.getId().c_str(), id.c_str()); + return false; } - if (isAttributeValueEnum(SAI_OBJECT_TYPE_ACL_ENTRY, attr.id)) + const auto& tableMatchAttrObject = tableMatchIt->second; + if (!tableMatchAttrObject->validateAclRuleMatch(rule)) { - if (!m_pAclOrch->isAclActionEnumValueSupported(action, attr.value.aclaction.parameter)) - { - SWSS_LOG_ERROR("Action %s is not supported by ASIC", sai_metadata_get_acl_action_type_name(action)); - return false; - } + SWSS_LOG_ERROR("Match %s in rule configuration %s is invalid %s", + getAttributeIdName(SAI_OBJECT_TYPE_ACL_ENTRY, matchId).c_str(), + rule.getId().c_str(), id.c_str()); + return false; } + return true; +} + +bool AclTable::validateAclRuleAction(sai_acl_entry_attr_t actionId, const AclRule& rule) const +{ // This means ACL table can hold rules with any action. if (!type.getActions().empty()) { + auto action = AclEntryActionToAclAction(actionId); if (!type.getActions().count(action)) { SWSS_LOG_ERROR("Action %s is not supported on table %s", @@ -1597,7 +1679,6 @@ bool AclTable::create() sai_attribute_t attr; vector table_attrs; - vector range_types_list; vector action_types_list; vector bpoint_list {type.getBindPointTypes().begin(), type.getBindPointTypes().end()}; @@ -1606,24 +1687,9 @@ bool AclTable::create() attr.value.s32list.list = bpoint_list.data(); table_attrs.push_back(attr); - for (const auto& enabledMatch: type.getMatches()) - { - attr.id = enabledMatch; - attr.value.booldata = true; - table_attrs.push_back(attr); - } - - for (const auto& rangeType: type.getRangeTypes()) + for (const auto& matchPair: type.getMatches()) { - range_types_list.push_back(rangeType); - } - - if (!range_types_list.empty()) - { - attr.id = SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE; - attr.value.s32list.count = static_cast(range_types_list.size()); - attr.value.s32list.list = range_types_list.data(); - table_attrs.push_back(attr); + table_attrs.push_back(matchPair.second->toSaiAttribute()); } for (const auto& actionType: type.getActions()) @@ -1982,7 +2048,7 @@ bool AclRuleDTelFlowWatchListEntry::validate() return false; } - return AclRule::validate(); + return true; } bool AclRuleDTelFlowWatchListEntry::create() @@ -2004,7 +2070,7 @@ bool AclRuleDTelFlowWatchListEntry::create() return false; } - return AclRule::validate(); + return true; } bool AclRuleDTelFlowWatchListEntry::remove() @@ -2141,7 +2207,7 @@ bool AclRuleDTelDropWatchListEntry::validate() return false; } - return AclRule::validate(); + return true; } void AclRuleDTelDropWatchListEntry::update(SubjectType, void *) @@ -2399,22 +2465,21 @@ void AclOrch::initDefaultTableTypes() builder.withName(TABLE_TYPE_L3) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS)) + .withMatch(make_shared(set{ + {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})) .build() ); @@ -2422,22 +2487,21 @@ void AclOrch::initDefaultTableTypes() builder.withName(TABLE_TYPE_L3V6) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS)) + .withMatch(make_shared(set{ + {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})) .build() ); @@ -2445,37 +2509,36 @@ void AclOrch::initDefaultTableTypes() builder.withName(TABLE_TYPE_MCLAG) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS)) + .withMatch(make_shared(set{ + {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})) .build() ); addAclTableType( builder.withName(TABLE_TYPE_PFCWD) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TC) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TC)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) .build() ); addAclTableType( builder.withName(TABLE_TYPE_DROP) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TC) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TC)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) .build() ); @@ -2485,38 +2548,37 @@ void AclOrch::initDefaultTableTypes() builder.withName(TABLE_TYPE_MIRROR_DSCP) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DSCP) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DSCP)) .build() ); builder.withName(TABLE_TYPE_MIRROR) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DSCP) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE); + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DSCP)) + .withMatch(make_shared(set{ + {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})); if (isAclMirrorV6Supported() && isCombinedMirrorV6Table()) { builder - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER); + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER)); } addAclTableType(builder.build()); } @@ -2527,22 +2589,21 @@ void AclOrch::initDefaultTableTypes() builder.withName(TABLE_TYPE_MIRRORV6) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DSCP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DSCP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) + .withMatch(make_shared(set{ + {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})) .build() ); } @@ -2649,7 +2710,7 @@ void AclOrch::putAclActionCapabilityInDB(acl_stage_type_t stage) { for (const auto& it: action_map) { - auto saiAction = getAclActionFromAclEntry(it.second); + auto saiAction = AclEntryActionToAclAction(it.second); if (acl_action_set.find(saiAction) != acl_action_set.cend()) { acl_action_value_stream << delimiter << it.first; @@ -2688,7 +2749,7 @@ void AclOrch::queryAclActionAttrEnumValues(const string &action_name, { vector fvVector; auto acl_attr = ruleAttrLookupMap.at(action_name); - auto acl_action = getAclActionFromAclEntry(acl_attr); + auto acl_action = AclEntryActionToAclAction(acl_attr); /* if the action is not supported then no need to do secondary query for * supported values @@ -2767,16 +2828,6 @@ void AclOrch::queryAclActionAttrEnumValues(const string &action_name, m_switchOrch->set_switch_capability(fvVector); } -sai_acl_action_type_t AclOrch::getAclActionFromAclEntry(sai_acl_entry_attr_t attr) -{ - if (attr < SAI_ACL_ENTRY_ATTR_ACTION_START || attr > SAI_ACL_ENTRY_ATTR_ACTION_END) - { - SWSS_LOG_THROW("Invalid ACL entry attribute passed in: %d", attr); - } - - return static_cast(attr - SAI_ACL_ENTRY_ATTR_ACTION_START); -}; - AclOrch::AclOrch(vector& connectors, DBConnector* stateDb, SwitchOrch *switchOrch, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch, DTelOrch *dtelOrch) : Orch(connectors), @@ -4011,16 +4062,16 @@ void AclOrch::createDTelWatchListTables() flowWLTable.validateAddStage(ACL_STAGE_INGRESS); flowWLTable.validateAddType(builder .withBindPointType(SAI_ACL_BIND_POINT_TYPE_SWITCH) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TUNNEL_VNI) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_INNER_SRC_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_INNER_DST_IP) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TUNNEL_VNI)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_SRC_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_DST_IP)) .withAction(SAI_ACL_ACTION_TYPE_ACL_DTEL_FLOW_OP) .withAction(SAI_ACL_ACTION_TYPE_DTEL_INT_SESSION) .withAction(SAI_ACL_ACTION_TYPE_DTEL_REPORT_ALL_PACKETS) @@ -4032,12 +4083,12 @@ void AclOrch::createDTelWatchListTables() dropWLTable.validateAddStage(ACL_STAGE_INGRESS); dropWLTable.validateAddType(builder .withBindPointType(SAI_ACL_BIND_POINT_TYPE_SWITCH) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL)) .withAction(SAI_ACL_ACTION_TYPE_DTEL_DROP_REPORT_ENABLE) .withAction(SAI_ACL_ACTION_TYPE_DTEL_TAIL_DROP_REPORT_ENABLE) .build() diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index c6fcf4b178..1010ee26e1 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -104,6 +104,7 @@ struct AclActionCapabilities }; typedef map acl_rule_attr_lookup_t; +typedef map acl_range_type_lookup_t; typedef map acl_bind_point_type_lookup_t; typedef map acl_ip_type_lookup_t; typedef map acl_dtel_flow_op_type_lookup_t; @@ -112,12 +113,46 @@ typedef tuple acl_range_properties_t; typedef map acl_capabilities_t; typedef map> acl_action_enum_values_capabilities_t; +class AclRule; + +class AclTableMatchInterface +{ +public: + AclTableMatchInterface(sai_acl_table_attr_t matchField); + + sai_acl_table_attr_t getId() const; + virtual sai_attribute_t toSaiAttribute() = 0; + virtual bool validateAclRuleMatch(const AclRule& rule) const = 0; +private: + sai_acl_table_attr_t m_matchField; +}; + +class AclTableMatch: public AclTableMatchInterface +{ +public: + AclTableMatch(sai_acl_table_attr_t matchField); + + sai_attribute_t toSaiAttribute() override; + bool validateAclRuleMatch(const AclRule& rule) const override; +}; + +class AclTableRangeMatch: public AclTableMatchInterface +{ +public: + AclTableRangeMatch(set rangeTypes); + + sai_attribute_t toSaiAttribute() override; + bool validateAclRuleMatch(const AclRule& rule) const override; + +private: + vector m_rangeList; +}; class AclTableType { public: string getName() const; const set& getBindPointTypes() const; - const set& getMatches() const; + const map>& getMatches() const; const set& getRangeTypes() const; const set& getActions() const; @@ -126,8 +161,7 @@ class AclTableType string m_name; set m_bpointTypes; - set m_enabledMatches; - set m_rangeTypes; + map> m_matches; set m_aclAcitons; }; @@ -136,9 +170,8 @@ class AclTableTypeBuilder public: AclTableTypeBuilder& withName(string name); AclTableTypeBuilder& withBindPointType(sai_acl_bind_point_type_t bpointType); - AclTableTypeBuilder& withMatch(sai_acl_table_attr_t matchField); + AclTableTypeBuilder& withMatch(shared_ptr match); AclTableTypeBuilder& withAction(sai_acl_action_type_t action); - AclTableTypeBuilder& withRangeMatch(sai_acl_range_type_t rangeType); AclTableType build(); private: @@ -216,7 +249,7 @@ class AclRule virtual bool validateAddPriority(string attr_name, string attr_value); virtual bool validateAddMatch(string attr_name, string attr_value); virtual bool validateAddAction(string attr_name, string attr_value); - virtual bool validate(); + virtual bool validate() = 0; bool processIpType(string type, sai_uint32_t &ip_type); inline static void setRulePriorities(sai_uint32_t min, sai_uint32_t max) { @@ -234,8 +267,8 @@ class AclRule virtual bool disableCounter(); virtual AclRuleCounters getCounters(); - string getId(); - string getTableId(); + string getId() const; + string getTableId() const; sai_object_id_t getCounterOid() { @@ -247,6 +280,7 @@ class AclRule return m_inPorts; } + const map& getMatches() const; static shared_ptr makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple&); virtual ~AclRule() {} @@ -359,8 +393,10 @@ class AclTable bool validate(); bool create(); - bool validateAclRuleMatch(sai_attribute_t attr) const; - bool validateAclRuleAction(sai_attribute_t attr) const; + // 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 + bool validateAclRuleAction(sai_acl_entry_attr_t actionId, const AclRule& rule) const; // Bind the ACL table to a port which is already linked bool bind(sai_object_id_t portOid); @@ -456,8 +492,6 @@ class AclOrch : public Orch, public Observer bool m_isCombinedMirrorV6Table = true; map m_mirrorTableCapabilities; - static sai_acl_action_type_t getAclActionFromAclEntry(sai_acl_entry_attr_t attr); - // Get the OID for the ACL bind point for a given port static bool getAclBindPortId(Port& port, sai_object_id_t& port_id); diff --git a/orchagent/pbh/pbhrule.cpp b/orchagent/pbh/pbhrule.cpp index 2ae143ad53..4a4b5a6d5c 100644 --- a/orchagent/pbh/pbhrule.cpp +++ b/orchagent/pbh/pbhrule.cpp @@ -52,6 +52,11 @@ bool AclRulePbh::validateAddMatch(const sai_attribute_t &attr) return false; } + if (!m_pTable->validateAclRuleMatch(attrId, *this)) + { + return false; + } + m_matches[attrId] = attr.value; return true; @@ -83,6 +88,17 @@ bool AclRulePbh::validateAddAction(const sai_attribute_t &attr) return false; } + if (!isActionSupported(attrId)) + { + SWSS_LOG_ERROR("Action %s is not supported by ASIC", attrName.c_str()); + return false; + } + + if (!m_pTable->validateAclRuleAction(attrId, *this)) + { + return false; + } + m_actions[attrId] = attr.value; return true; @@ -98,7 +114,7 @@ bool AclRulePbh::validate() return false; } - return AclRule::validate(); + return true; } void AclRulePbh::update(SubjectType, void *) diff --git a/orchagent/pbhorch.cpp b/orchagent/pbhorch.cpp index 343d66f161..83a1e80bd0 100644 --- a/orchagent/pbhorch.cpp +++ b/orchagent/pbhorch.cpp @@ -183,12 +183,12 @@ bool PbhOrch::createPbhTable(const PbhTable &table) static const auto pbhTableType = AclTableTypeBuilder() .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_GRE_KEY) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE) + .withMatch(std::make_shared(SAI_ACL_TABLE_ATTR_FIELD_GRE_KEY)) + .withMatch(std::make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) + .withMatch(std::make_shared(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL)) + .withMatch(std::make_shared(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER)) + .withMatch(std::make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(std::make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE)) .build(); if (!pbhTable.validateAddType(pbhTableType)) diff --git a/orchagent/saihelper.h b/orchagent/saihelper.h index 450acf8b69..a0b2aa2fac 100644 --- a/orchagent/saihelper.h +++ b/orchagent/saihelper.h @@ -4,6 +4,9 @@ #include +#define IS_ATTR_ID_IN_RANGE(attrId, objectType, attrPrefix) \ + ((attrId) >= SAI_ ## objectType ## _ATTR_ ## attrPrefix ## _START && (attrId) <= SAI_ ## objectType ## _ATTR_ ## attrPrefix ## _END) + void initSaiApi(); void initSaiRedis(const std::string &record_location, const std::string &record_filename); sai_status_t initSaiPhyApi(swss::gearbox_phy_t *phy); diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 48dac39c95..1740575b2a 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -98,27 +98,25 @@ namespace aclorch_test TEST_F(AclTest, Create_L3_Acl_Table) { - AclTable acltable(nullptr); /* this test won't trigger a call to gAclOrch so it's nullptr */ + AclTable acltable; /* this test shouldn't trigger a call to gAclOrch because it's nullptr */ AclTableTypeBuilder builder; - auto l3TableType = builder.withName(TABLE_TYPE_L3) + auto l3TableType = builder .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_DST_IP) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withMatch(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) - .withRangeMatch(SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) + .withMatch(make_shared(set({SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}))) .build(); - acltable.validateAddType(l3TableType); + acltable.type = l3TableType; auto res = createAclTable(acltable); ASSERT_TRUE(res->ret_val); @@ -1521,7 +1519,7 @@ namespace aclorch_test { { ACL_TABLE_TYPE_MATCHES, - string(MATCH_ETHER_TYPE) + comma + MATCH_L4_SRC_PORT_RANGE + comma + MATCH_L4_SRC_PORT_RANGE + string(MATCH_ETHER_TYPE) + comma + MATCH_L4_SRC_PORT_RANGE + comma + MATCH_L4_DST_PORT_RANGE }, { ACL_TABLE_TYPE_BPOINT_TYPES, From 99eaca99af71c4e17c2575dac750b641ecc30a66 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Fri, 22 Oct 2021 16:21:54 +0300 Subject: [PATCH 03/35] internal review comments Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 24 +++++++++--------------- tests/mock_tests/aclorch_ut.cpp | 4 ++-- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index f548d63365..52c462ff54 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1372,7 +1372,7 @@ bool AclRulePacket::validate() return false; } - return true; + return true; } void AclRulePacket::update(SubjectType, void *) @@ -1425,7 +1425,7 @@ bool AclRuleMirror::validate() return false; } - return true; + return true; } bool AclRuleMirror::create() @@ -1679,7 +1679,7 @@ bool AclTable::create() sai_attribute_t attr; vector table_attrs; - vector action_types_list; + vector action_types_list {type.getActions().begin(), type.getActions().end()}; vector bpoint_list {type.getBindPointTypes().begin(), type.getBindPointTypes().end()}; attr.id = SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST; @@ -1692,11 +1692,6 @@ bool AclTable::create() table_attrs.push_back(matchPair.second->toSaiAttribute()); } - for (const auto& actionType: type.getActions()) - { - action_types_list.push_back(actionType); - } - if (!action_types_list.empty()) { attr.id= SAI_ACL_TABLE_ATTR_ACL_ACTION_TYPE_LIST; @@ -2048,7 +2043,7 @@ bool AclRuleDTelFlowWatchListEntry::validate() return false; } - return true; + return true; } bool AclRuleDTelFlowWatchListEntry::create() @@ -2070,7 +2065,7 @@ bool AclRuleDTelFlowWatchListEntry::create() return false; } - return true; + return true; } bool AclRuleDTelFlowWatchListEntry::remove() @@ -2207,7 +2202,7 @@ bool AclRuleDTelDropWatchListEntry::validate() return false; } - return true; + return true; } void AclRuleDTelDropWatchListEntry::update(SubjectType, void *) @@ -3264,14 +3259,13 @@ bool AclOrch::removeAclTableType(const string& tableTypeName) // The upper layer can although ensure that // user does not remove table type that is referenced // by an ACL table. - auto erased = m_AclTableTypes.erase(tableTypeName); - - if (!erased) + if (!m_AclTableTypes.erase(tableTypeName)) { SWSS_LOG_ERROR("Unknown table type %s", tableTypeName.c_str()); + return false; } - return erased; + return true; } bool AclOrch::addAclRule(shared_ptr newRule, string table_id) diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 1740575b2a..387cdbf8a0 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -678,13 +678,13 @@ namespace aclorch_test auto const &resourceMap = Portal::CrmOrchInternal::getResourceMap(crmOrch); // Verify the ACL tables - ssize_t crmAclTableBindingCount = 0; + size_t crmAclTableBindingCount = 0; for (auto const &kv: resourceMap.at(CrmResourceType::CRM_ACL_TABLE).countersMap) { crmAclTableBindingCount += kv.second.usedCounter; } - ssize_t aclorchAclTableBindingCount = 0; + size_t aclorchAclTableBindingCount = 0; for (auto const &kv: Portal::AclOrchInternal::getAclTables(aclOrch)) { aclorchAclTableBindingCount += kv.second.type.getBindPointTypes().size(); From 1a34188f6a2f8b68df3050a734569abef53295b5 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Fri, 22 Oct 2021 17:37:14 +0300 Subject: [PATCH 04/35] make getAttributeIdName static Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 52c462ff54..7f54481b72 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -184,7 +184,7 @@ static sai_acl_action_type_t AclEntryActionToAclAction(sai_acl_entry_attr_t attr return static_cast(attr - SAI_ACL_ENTRY_ATTR_ACTION_START); } -string getAttributeIdName(sai_object_type_t objectType, sai_attr_id_t attr) +static string getAttributeIdName(sai_object_type_t objectType, sai_attr_id_t attr) { const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, attr); if (!meta) From c1b39474e86b0158e256542f7d4f0b8770e7549b Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 25 Oct 2021 16:15:00 +0300 Subject: [PATCH 05/35] [aclorch] remove IN_PORTS, OUT_PORTS from L3 tables. Fix VS tests as they should not use IN_PORTS, OUT_PORTS on L3 tables Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 5 --- tests/test_acl.py | 74 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 7f54481b72..422d3f8f8b 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2471,8 +2471,6 @@ void AclOrch::initDefaultTableTypes() .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS)) .withMatch(make_shared(set{ {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})) .build() @@ -2493,8 +2491,6 @@ void AclOrch::initDefaultTableTypes() .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORTS)) .withMatch(make_shared(set{ {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})) .build() @@ -2596,7 +2592,6 @@ void AclOrch::initDefaultTableTypes() .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DSCP)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) .withMatch(make_shared(set{ {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})) .build() diff --git a/tests/test_acl.py b/tests/test_acl.py index c3b0fdaaf0..9bb72c48cd 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -10,6 +10,15 @@ L3V6_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8"] L3V6_RULE_NAME = "L3V6_TEST_RULE" +MCLAG_TABLE_TYPE = "MCLAG" +MCLAG_TABLE_NAME = "MCLAG_TEST" +MCLAG_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"] +MCLAG_RULE_NAME = "MCLAG_TEST_RULE" + +MIRROR_TABLE_TYPE = "MIRROR" +MIRROR_TABLE_NAME = "MIRROR_TEST" +MIRROR_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"] +MIRROR_RULE_NAME = "MIRROR_TEST_RULE" class TestAcl: @pytest.yield_fixture @@ -32,6 +41,24 @@ def l3v6_acl_table(self, dvs_acl): dvs_acl.remove_acl_table(L3V6_TABLE_NAME) dvs_acl.verify_acl_table_count(0) + @pytest.yield_fixture + def mclag_acl_table(self, dvs_acl): + try: + dvs_acl.create_acl_table(MCLAG_TABLE_NAME, MCLAG_TABLE_TYPE, MCLAG_BIND_PORTS) + yield dvs_acl.get_acl_table_ids(1)[0] + finally: + dvs_acl.remove_acl_table(MCLAG_TABLE_NAME) + dvs_acl.verify_acl_table_count(0) + + @pytest.yield_fixture + def mirror_acl_table(self, dvs_acl): + try: + dvs_acl.create_acl_table(MIRROR_TABLE_NAME, MIRROR_TABLE_TYPE, MIRROR_BIND_PORTS) + yield dvs_acl.get_acl_table_ids(1)[0] + finally: + dvs_acl.remove_acl_table(MIRROR_TABLE_NAME) + dvs_acl.verify_acl_table_count(0) + @pytest.yield_fixture def setup_teardown_neighbor(self, dvs): try: @@ -133,42 +160,65 @@ def test_V6AclRuleNextHeaderAppendedForTCPFlags(self, dvs_acl, l3v6_acl_table): dvs_acl.remove_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME) dvs_acl.verify_no_acl_rules() - def test_AclRuleInOutPorts(self, dvs_acl, l3_acl_table): + def test_AclRuleInOutPorts(self, dvs_acl, mclag_acl_table, mirror_acl_table): + """ + Verify IN_PORTS, OUT_PORTS matches on ACL rule. + Using MIRROR table type for IN_PORTS matches. + Using MCLAG table type for OUT_PORTS matches. + """ config_qualifiers = { - "IN_PORTS": "Ethernet0,Ethernet4", - "OUT_PORTS": "Ethernet8,Ethernet12" + "IN_PORTS": "Ethernet8,Ethernet12", + } + + expected_sai_qualifiers = { + "SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS": dvs_acl.get_port_list_comparator(["Ethernet8", "Ethernet12"]) + } + + dvs_acl.create_acl_rule(MIRROR_TABLE_NAME, MIRROR_RULE_NAME, config_qualifiers) + dvs_acl.verify_acl_rule(expected_sai_qualifiers) + + dvs_acl.remove_acl_rule(MIRROR_TABLE_NAME, MIRROR_RULE_NAME) + dvs_acl.verify_no_acl_rules() + + config_qualifiers = { + "OUT_PORTS": "Ethernet8,Ethernet12", } expected_sai_qualifiers = { - "SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS": dvs_acl.get_port_list_comparator(["Ethernet0", "Ethernet4"]), "SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORTS": dvs_acl.get_port_list_comparator(["Ethernet8", "Ethernet12"]) } - dvs_acl.create_acl_rule(L3_TABLE_NAME, L3_RULE_NAME, config_qualifiers) + dvs_acl.create_acl_rule(MCLAG_TABLE_NAME, MCLAG_RULE_NAME, config_qualifiers) dvs_acl.verify_acl_rule(expected_sai_qualifiers) - dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME) + dvs_acl.remove_acl_rule(MCLAG_TABLE_NAME, MCLAG_RULE_NAME) dvs_acl.verify_no_acl_rules() - def test_AclRuleInPortsNonExistingInterface(self, dvs_acl, l3_acl_table): + def test_AclRuleInPortsNonExistingInterface(self, dvs_acl, mirror_acl_table): + """ + Using MIRROR table type as it has IN_PORTS matches. + """ config_qualifiers = { "IN_PORTS": "FOO_BAR_BAZ" } - dvs_acl.create_acl_rule(L3_TABLE_NAME, L3_RULE_NAME, config_qualifiers) + dvs_acl.create_acl_rule(MIRROR_TABLE_NAME, MIRROR_RULE_NAME, config_qualifiers) dvs_acl.verify_no_acl_rules() - dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME) + dvs_acl.remove_acl_rule(MIRROR_TABLE_NAME, MIRROR_RULE_NAME) - def test_AclRuleOutPortsNonExistingInterface(self, dvs_acl, l3_acl_table): + def test_AclRuleOutPortsNonExistingInterface(self, dvs_acl, mclag_acl_table): + """ + Using MCLAG table type as it has OUT_PORTS matches. + """ config_qualifiers = { "OUT_PORTS": "FOO_BAR_BAZ" } - dvs_acl.create_acl_rule(L3_TABLE_NAME, L3_RULE_NAME, config_qualifiers) + dvs_acl.create_acl_rule(MCLAG_TABLE_NAME, MCLAG_RULE_NAME, config_qualifiers) dvs_acl.verify_no_acl_rules() - dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME) + dvs_acl.remove_acl_rule(MCLAG_TABLE_NAME, MCLAG_RULE_NAME) def test_AclRuleVlanId(self, dvs_acl, l3_acl_table): config_qualifiers = {"VLAN_ID": "100"} From 743027b840d4c82917d735937d07114b5b66f302 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Fri, 29 Oct 2021 12:41:33 +0300 Subject: [PATCH 06/35] update doc Signed-off-by: Stepan Blyshchak --- doc/Configuration.md | 40 ++++++++++++++++++++++++++++++++++++++++ doc/swss-schema.md | 24 +++++++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/doc/Configuration.md b/doc/Configuration.md index a619531f70..355164abb1 100644 --- a/doc/Configuration.md +++ b/doc/Configuration.md @@ -286,6 +286,46 @@ and migration plan ``` +***ACL table type configuration example*** +``` +{ + "ACL_TABLE_TYPE": { + "CUSTOM_L3": { + "MATCHES": [ + "IN_PORTS", + "OUT_PORTS", + "SRC_IP" + ], + "ACTIONS": [ + "PACKET_ACTION", + "MIRROR_INGRESS_ACTION" + ], + "BIND_POINTS": [ + "PORT", + "LAG" + ] + } + }, + "ACL_TABLE": { + "DATAACL": { + "STAGE": "INGRESS", + "TYPE": "CUSTOM_L3", + "PORTS": [ + "Ethernet0", + "PortChannel1" + ] + } + }, + "ACL_RULE": { + "DATAACL|RULE0": { + "PRIORITY": "999", + "PACKET_ACTION": "DROP", + "SRC_IP": "1.1.1.1/32", + } + } +} +``` + ### BGP Sessions BGP session configuration is defined in **BGP_NEIGHBOR** table. BGP diff --git a/doc/swss-schema.md b/doc/swss-schema.md index 3ccc7b74af..81263e4f40 100644 --- a/doc/swss-schema.md +++ b/doc/swss-schema.md @@ -443,15 +443,37 @@ It's possible to create separate configuration files for different ASIC platform ---------------------------------------------- +### ACL\_TABLE\_TYPE +Stores a definition of table - set of matches, actions and bind point types. ACL_TABLE references a key inside this table in "type" field. + +``` +key: ACL_TABLE_TYPE:name ; key of the ACL table type entry. The name is arbitary name user chooses. +; field = value +matches = match-list ; list of matches for this table, matches are same as in ACL_RULE table. +actions = action-list ; list of actions for this table, actions are same as in ACL_RULE table. +bind_points = bind-points-list ; list of bind point types for this table. + +; values annotation +match = 1*64VCHAR +match-list = [1-max-matches]*match +action = 1*64VCHAR +action-list = [1-max-actions]*action +bind-point = port/lag +bind-points-list = [1-max-bind-points]*bind-point +``` + ### ACL\_TABLE Stores information about ACL tables on the switch. Port names are defined in [port_config.ini](../portsyncd/port_config.ini). key = ACL_TABLE:name ; acl_table_name must be unique ;field = value policy_desc = 1*255VCHAR ; name of the ACL policy table description - type = "mirror"/"l3"/"l3v6" ; type of acl table, every type of + type = 1*255VCHAR ; type of acl table, every type of ; table defines the match/action a ; specific set of match and actions. + ; There are pre-defined table types like + ; "MIRROR", "MIRRORV6", "MIRROR_DSCP", + ; "L3", "L3V6", "MCLAG", "PFCWD", "DROP". ports = [0-max_ports]*port_name ; the ports to which this ACL ; table is applied, can be emtry ; value annotations From 823986dd85b9133a2e914c88f4a4ee920851cef7 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Wed, 10 Nov 2021 20:38:21 +0200 Subject: [PATCH 07/35] JUST FOR TEST Signed-off-by: Stepan Blyschak --- orchagent/aclorch.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index dd7fab1def..7bf277b631 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -505,6 +505,8 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) { SWSS_LOG_ENTER(); + SWSS_LOG_THROW("ADDED JUST FOR TEST"); + sai_attribute_value_t value; try From ea5acfebca562156275eb97f9c4ee37a4cbe90d8 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 11 Nov 2021 11:45:11 +0200 Subject: [PATCH 08/35] Revert "JUST FOR TEST" This reverts commit 823986dd85b9133a2e914c88f4a4ee920851cef7. --- orchagent/aclorch.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 28b66541e2..9ae2c4f8fa 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -511,8 +511,6 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) { SWSS_LOG_ENTER(); - SWSS_LOG_THROW("ADDED JUST FOR TEST"); - sai_attribute_value_t value; try From a8508c1880e82abc13bfbe7aa7aedeb64fdcd968 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 11 Nov 2021 11:46:03 +0200 Subject: [PATCH 09/35] split in out ports tests Signed-off-by: Stepan Blyschak --- tests/test_acl.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_acl.py b/tests/test_acl.py index 9bb72c48cd..fb8aecb0ea 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -160,11 +160,10 @@ def test_V6AclRuleNextHeaderAppendedForTCPFlags(self, dvs_acl, l3v6_acl_table): dvs_acl.remove_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME) dvs_acl.verify_no_acl_rules() - def test_AclRuleInOutPorts(self, dvs_acl, mclag_acl_table, mirror_acl_table): + def test_AclRuleInPorts(self, dvs_acl, mirror_acl_table): """ - Verify IN_PORTS, OUT_PORTS matches on ACL rule. + Verify IN_PORTS matches on ACL rule. Using MIRROR table type for IN_PORTS matches. - Using MCLAG table type for OUT_PORTS matches. """ config_qualifiers = { "IN_PORTS": "Ethernet8,Ethernet12", @@ -180,6 +179,11 @@ def test_AclRuleInOutPorts(self, dvs_acl, mclag_acl_table, mirror_acl_table): dvs_acl.remove_acl_rule(MIRROR_TABLE_NAME, MIRROR_RULE_NAME) dvs_acl.verify_no_acl_rules() + def test_AclRuleOutPorts(self, dvs_acl, mclag_acl_table): + """ + Verify OUT_PORTS matches on ACL rule. + Using MCLAG table type for OUT_PORTS matches. + """ config_qualifiers = { "OUT_PORTS": "Ethernet8,Ethernet12", } From 14341405db9485c405d583b8cc0b2eadc3175fb9 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 11 Nov 2021 12:53:22 +0200 Subject: [PATCH 10/35] [temporary] print git status and commit hash before build to check if the same as when running tests locally Signed-off-by: Stepan Blyschak --- .azure-pipelines/build-template.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index d351d5703a..0dc4bd7f3b 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -136,6 +136,8 @@ jobs: export ENABLE_GCOV=y fi ./autogen.sh + git status + git rev-parse HEAD dpkg-buildpackage -us -uc -b -j$(nproc) && cp ../*.deb . displayName: "Compile sonic swss" - publish: $(System.DefaultWorkingDirectory)/ From 4caf56ee53b6948c74df793d50ceafaafd0327f7 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 11 Nov 2021 13:22:19 +0200 Subject: [PATCH 11/35] Revert "[temporary] print git status and commit hash before build to check if the same as when running tests locally" This reverts commit 14341405db9485c405d583b8cc0b2eadc3175fb9. --- .azure-pipelines/build-template.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index 0dc4bd7f3b..d351d5703a 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -136,8 +136,6 @@ jobs: export ENABLE_GCOV=y fi ./autogen.sh - git status - git rev-parse HEAD dpkg-buildpackage -us -uc -b -j$(nproc) && cp ../*.deb . displayName: "Compile sonic swss" - publish: $(System.DefaultWorkingDirectory)/ From 01cfa909a7842535a91e7394dd7ef664e36f845e Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 11 Nov 2021 13:37:54 +0200 Subject: [PATCH 12/35] [temporary] add throw message for debug on ci Signed-off-by: Stepan Blyschak --- orchagent/aclorch.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 9ae2c4f8fa..e6755daaea 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -3618,6 +3618,7 @@ void AclOrch::doAclTableTask(Consumer &consumer) auto tableType = getAclTableType(tableTypeName); if (!tableType) { + SWSS_LOG_THROW("Temporary: failed to find table type %s", tableTypeName.c_str()); it++; continue; } From ebe36ee40c1a19a2732a1ad0655dd513b29cd662 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 11 Nov 2021 13:55:53 +0200 Subject: [PATCH 13/35] Revert "[temporary] add throw message for debug on ci" This reverts commit 01cfa909a7842535a91e7394dd7ef664e36f845e. --- orchagent/aclorch.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index e6755daaea..9ae2c4f8fa 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -3618,7 +3618,6 @@ void AclOrch::doAclTableTask(Consumer &consumer) auto tableType = getAclTableType(tableTypeName); if (!tableType) { - SWSS_LOG_THROW("Temporary: failed to find table type %s", tableTypeName.c_str()); it++; continue; } From 8c54a141f3bd908bc1cee16632de060f28b2a6eb Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 11 Nov 2021 13:56:40 +0200 Subject: [PATCH 14/35] temporary disable the check for action list Signed-off-by: Stepan Blyschak --- orchagent/aclorch.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 9ae2c4f8fa..e5500bd357 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1649,14 +1649,14 @@ bool AclTable::validate() return false; } - if (m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage)) - { - if (type.getActions().empty()) - { - SWSS_LOG_ERROR("Action list for table %s is mandatory", id.c_str()); - return false; - } - } + // if (m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage)) + // { + // if (type.getActions().empty()) + // { + // SWSS_LOG_ERROR("Action list for table %s is mandatory", id.c_str()); + // return false; + // } + // } for (const auto& action: type.getActions()) { From e35b5a928fc2bee4c3f0d1b284262757b563a49b Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 11 Nov 2021 14:26:06 +0200 Subject: [PATCH 15/35] Revert "temporary disable the check for action list" This reverts commit 8c54a141f3bd908bc1cee16632de060f28b2a6eb. --- orchagent/aclorch.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index e5500bd357..9ae2c4f8fa 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1649,14 +1649,14 @@ bool AclTable::validate() return false; } - // if (m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage)) - // { - // if (type.getActions().empty()) - // { - // SWSS_LOG_ERROR("Action list for table %s is mandatory", id.c_str()); - // return false; - // } - // } + if (m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage)) + { + if (type.getActions().empty()) + { + SWSS_LOG_ERROR("Action list for table %s is mandatory", id.c_str()); + return false; + } + } for (const auto& action: type.getActions()) { From 7c96d4e179cb6c7cf1f07d0475a39b3365355111 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Wed, 17 Nov 2021 03:59:27 +0200 Subject: [PATCH 16/35] temporary disable check Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 9ae2c4f8fa..585a04f22b 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1649,6 +1649,7 @@ bool AclTable::validate() return false; } +#if 0 // uncomment when vslib from sonic-sairedis is updated if (m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage)) { if (type.getActions().empty()) @@ -1657,6 +1658,7 @@ bool AclTable::validate() return false; } } +#endif for (const auto& action: type.getActions()) { From 566fbbdf53478e68ecda0ec9da1819dd7091f61b Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Wed, 17 Nov 2021 05:49:35 +0200 Subject: [PATCH 17/35] add missing fields in L3 table Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 585a04f22b..1ab039a203 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2501,6 +2501,12 @@ void AclOrch::initDefaultTableTypes() .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TUNNEL_VNI)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_IP_PROTOCOL)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TC)) .withMatch(make_shared(set{ {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})) .build() From fbbb6f28054bf9a7be641060de0fb36d147e0ec6 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Wed, 17 Nov 2021 11:36:39 +0200 Subject: [PATCH 18/35] remove ETHER_TYPE from V6 table types Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 1ab039a203..8bd075afba 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2516,7 +2516,6 @@ void AclOrch::initDefaultTableTypes() builder.withName(TABLE_TYPE_L3V6) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6)) @@ -2616,7 +2615,6 @@ void AclOrch::initDefaultTableTypes() builder.withName(TABLE_TYPE_MIRRORV6) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6)) From 03a9d61db0188beeba5c7ac695b2454bebda1885 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Wed, 17 Nov 2021 11:41:45 +0200 Subject: [PATCH 19/35] remove a test case test_AclBindMirrorV6WrongConfig because IPv6 rules can be inserted into MIRROR table in combined mode Signed-off-by: Stepan Blyshchak --- tests/test_mirror_ipv6_combined.py | 79 ------------------------------ 1 file changed, 79 deletions(-) diff --git a/tests/test_mirror_ipv6_combined.py b/tests/test_mirror_ipv6_combined.py index cafa5eaf47..e75f6af72d 100644 --- a/tests/test_mirror_ipv6_combined.py +++ b/tests/test_mirror_ipv6_combined.py @@ -473,85 +473,6 @@ def test_AclBindMirrorV6Reorder2(self, dvs, testlog): self.set_interface_status("Ethernet32", "down") - # Test case - create ACL rules associated with wrong table - # 0. predefine the VS platform: mellanox platform - # 1. create a mirror session - # 2. create the ipv4 ACL table - # 3. create the ipv6 ACL rule associated with ipv4 table - # 4. create the ipv6 ACL table - # 5. create the ipv4 ACL rule associated with ipv6 table - # 6. verify two rules are inserted successfully - def test_AclBindMirrorV6WrongConfig(self, dvs, testlog): - """ - This test verifies IPv6 rules cannot be inserted into MIRROR table - """ - self.setup_db(dvs) - - session = "MIRROR_SESSION" - acl_table = "MIRROR_TABLE" - acl_table_v6 = "MIRROR_TABLE_V6" - acl_rule_1 = "MIRROR_RULE_1" - acl_rule_2 = "MIRROR_RULE_2" - - # bring up port; assign ip; create neighbor; create route - self.set_interface_status("Ethernet32", "up") - self.add_ip_address("Ethernet32", "20.0.0.0/31") - self.add_neighbor("Ethernet32", "20.0.0.1", "02:04:06:08:10:12") - self.add_route(dvs, "4.4.4.4", "20.0.0.1") - - # create mirror session - self.create_mirror_session(session, "3.3.3.3", "4.4.4.4", "0x6558", "8", "100", "0") - assert self.get_mirror_session_state(session)["status"] == "active" - - # assert mirror session in asic database - tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION") - assert len(tbl.getKeys()) == 1 - mirror_session_oid = tbl.getKeys()[0] - - # create acl table ipv4 - self.create_acl_table(acl_table, ["Ethernet0", "Ethernet4"], "MIRROR") - - # create WRONG acl rule with IPv6 addresses - self.create_mirror_acl_ipv6_rule(acl_table, acl_rule_2, session) - - # assert acl rule is not created - tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY") - rule_entries = [k for k in tbl.getKeys() if k not in dvs.asicdb.default_acl_entries] - assert len(rule_entries) == 0 - - # create acl table ipv6 - self.create_acl_table(acl_table_v6, ["Ethernet0", "Ethernet4"], "MIRRORV6") - - # create WRONG acl rule with IPv4 addresses - self.create_mirror_acl_ipv4_rule(acl_table_v6, acl_rule_1, session) - - # assert acl rules are created - tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY") - rule_entries = [k for k in tbl.getKeys() if k not in dvs.asicdb.default_acl_entries] - assert len(rule_entries) == 0 - - # remove acl rule - self.remove_mirror_acl_rule(acl_table, acl_rule_1) - self.remove_mirror_acl_rule(acl_table_v6, acl_rule_2) - - # remove acl table - self.remove_acl_table(acl_table) - self.remove_acl_table(acl_table_v6) - - # remove mirror session - self.remove_mirror_session(session) - - # assert no mirror session in asic database - tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION") - assert len(tbl.getKeys()) == 0 - - # remove route; remove neighbor; remove ip; bring down port - self.remove_route(dvs, "4.4.4.4") - self.remove_neighbor("Ethernet32", "20.0.0.1") - self.remove_ip_address("Ethernet32", "20.0.0.0/31") - self.set_interface_status("Ethernet32", "down") - - # Add Dummy always-pass test at end as workaroud # for issue when Flaky fail on final test it invokes module tear-down before retrying def test_nonflaky_dummy(): From 381a73d4fd6eeac1cdc628327346de50b0c6684d Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Wed, 17 Nov 2021 15:15:28 +0200 Subject: [PATCH 20/35] Revert "temporary disable check" This reverts commit 7c96d4e179cb6c7cf1f07d0475a39b3365355111. --- orchagent/aclorch.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 8bd075afba..89f561e550 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1649,7 +1649,6 @@ bool AclTable::validate() return false; } -#if 0 // uncomment when vslib from sonic-sairedis is updated if (m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage)) { if (type.getActions().empty()) @@ -1658,7 +1657,6 @@ bool AclTable::validate() return false; } } -#endif for (const auto& action: type.getActions()) { From 056cbd98fd427311c196ec8d60321bd44d098152 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Wed, 17 Nov 2021 15:19:42 +0200 Subject: [PATCH 21/35] Revert "add missing fields in L3 table" This reverts commit 566fbbdf53478e68ecda0ec9da1819dd7091f61b. --- orchagent/aclorch.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 89f561e550..3f78d84004 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2499,12 +2499,6 @@ void AclOrch::initDefaultTableTypes() .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_L4_DST_PORT)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TUNNEL_VNI)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_L4_SRC_PORT)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_INNER_IP_PROTOCOL)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TC)) .withMatch(make_shared(set{ {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}})) .build() From 5667e64affeef9a18cb4aec3fd5cac9187d96e00 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Wed, 17 Nov 2021 15:42:19 +0200 Subject: [PATCH 22/35] change test_acl_egress_table to work with table types Signed-off-by: Stepan Blyshchak --- tests/dvslib/dvs_acl.py | 29 +++++++++++++++++++++++++++++ tests/test_acl_egress_table.py | 19 ++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/tests/dvslib/dvs_acl.py b/tests/dvslib/dvs_acl.py index dbf9791b53..9111de7a8e 100644 --- a/tests/dvslib/dvs_acl.py +++ b/tests/dvslib/dvs_acl.py @@ -6,6 +6,7 @@ class DVSAcl: """Manage ACL tables and rules on the virtual switch.""" CDB_ACL_TABLE_NAME = "ACL_TABLE" + CDB_ACL_TABLE_TYPE_NAME = "ACL_TABLE_TYPE" CDB_MIRROR_ACTION_LOOKUP = { "ingress": "MIRROR_INGRESS_ACTION", @@ -48,6 +49,26 @@ def __init__(self, asic_db, config_db, state_db, counters_db): self.state_db = state_db self.counters_db = counters_db + def create_acl_table_type( + self, + name: str, + matches: List[str], + bpoint_types: List[str] + ) -> None: + """Create a new ACL table type in Config DB. + + Args: + name: The name for the new ACL table type. + matches: A list of matches to use in ACL table. + bpoint_types: A list of bind point types to use in ACL table. + """ + table_type_attrs = { + "matches@": ",".join(matches), + "bind_points@": ",".join(bpoint_types) + } + + self.config_db.create_entry(self.CDB_ACL_TABLE_TYPE_NAME, name, table_type_attrs) + def create_acl_table( self, table_name: str, @@ -111,6 +132,14 @@ def remove_acl_table(self, table_name: str) -> None: """ self.config_db.delete_entry(self.CDB_ACL_TABLE_NAME, table_name) + def remove_acl_table_type(self, name: str) -> None: + """Remove an ACL table type from Config DB. + + Args: + name: The name of the ACL table type to delete. + """ + self.config_db.delete_entry(self.CDB_ACL_TABLE_TYPE_NAME, name) + def get_acl_table_ids(self, expected: int) -> List[str]: """Get all of the ACL table IDs in ASIC DB. diff --git a/tests/test_acl_egress_table.py b/tests/test_acl_egress_table.py index f2b917ebc6..01800d6b20 100644 --- a/tests/test_acl_egress_table.py +++ b/tests/test_acl_egress_table.py @@ -1,6 +1,19 @@ import pytest -TABLE_TYPE = "L3" +TABLE_TYPE = "CUSTOM_L3" +CUSTOM_TABLE_TYPE_MATCHES = [ + "L4_SRC_PORT_RANGE", + "L4_DST_PORT_RANGE", + "ETHER_TYPE", + "TUNNEL_VNI", + "TC", + "INNER_IP_PROTOCOL", + "INNER_ETHER_TYPE", + "INNER_L4_SRC_PORT", + "INNER_L4_DST_PORT", + "VLAN_ID" +] +CUSTOM_TABLE_TYPE_BPOINT_TYPES = ["PORT","PORTCHANNEL"] TABLE_NAME = "EGRESS_TEST" BIND_PORTS = ["Ethernet0", "Ethernet4"] RULE_NAME = "EGRESS_TEST_RULE" @@ -10,14 +23,17 @@ class TestEgressAclTable: @pytest.yield_fixture def egress_acl_table(self, dvs_acl): try: + dvs_acl.create_acl_table_type(TABLE_TYPE, CUSTOM_TABLE_TYPE_MATCHES, CUSTOM_TABLE_TYPE_BPOINT_TYPES) dvs_acl.create_acl_table(TABLE_NAME, TABLE_TYPE, BIND_PORTS, stage="egress") yield dvs_acl.get_acl_table_ids(1)[0] finally: dvs_acl.remove_acl_table(TABLE_NAME) + dvs_acl.remove_acl_table_type(TABLE_TYPE) dvs_acl.verify_acl_table_count(0) def test_EgressAclTableCreationDeletion(self, dvs_acl): try: + dvs_acl.create_acl_table_type(TABLE_TYPE, CUSTOM_TABLE_TYPE_MATCHES, CUSTOM_TABLE_TYPE_BPOINT_TYPES) dvs_acl.create_acl_table(TABLE_NAME, TABLE_TYPE, BIND_PORTS, stage="egress") acl_table_id = dvs_acl.get_acl_table_ids(1)[0] @@ -27,6 +43,7 @@ def test_EgressAclTableCreationDeletion(self, dvs_acl): dvs_acl.verify_acl_table_port_binding(acl_table_id, BIND_PORTS, 1, stage="egress") finally: dvs_acl.remove_acl_table(TABLE_NAME) + dvs_acl.remove_acl_table_type(TABLE_TYPE) dvs_acl.verify_acl_table_count(0) def test_EgressAclRuleL4SrcPortRange(self, dvs_acl, egress_acl_table): From 2a4d63e51129567a7ae0b42a2cdfbb6d8a0a1056 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Thu, 18 Nov 2021 14:54:45 +0200 Subject: [PATCH 23/35] add a table with matches and mirror table types Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 3f78d84004..b53f1ed897 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2560,6 +2560,35 @@ void AclOrch::initDefaultTableTypes() .build() ); + + /* + * Type of Tables and Supported Match Types (ASIC database) + * |------------------------------------------------------------------| + * | | TABLE_MIRROR | TABLE_MIRROR | TABLE_MIRRORV6 | + * | Match Type |----------------------------------------------| + * | | combined | separated | + * |------------------------------------------------------------------| + * | MATCH_SRC_IP | √ | √ | | + * | MATCH_DST_IP | √ | √ | | + * |------------------------------------------------------------------| + * | MATCH_ICMP_TYPE | √ | √ | | + * | MATCH_ICMP_CODE | √ | √ | | + * |------------------------------------------------------------------| + * | MATCH_SRC_IPV6 | √ | | √ | + * | MATCH_DST_IPV6 | √ | | √ | + * |------------------------------------------------------------------| + * | MATCH_ICMPV6_TYPE | √ | | √ | + * | MATCH_ICMPV6_CODE | √ | | √ | + * |------------------------------------------------------------------| + * | MATCH_IP_PROTOCOL | √ | √ | | + * | MATCH_NEXT_HEADER | √ | | √ | + * | -----------------------------------------------------------------| + * | MATCH_ETHERTYPE | √ | √ | | + * |------------------------------------------------------------------| + * | MATCH_IN_PORTS | √ | √ | | + * |------------------------------------------------------------------| + */ + if (isAclMirrorV4Supported()) { addAclTableType( From fe09424ea8f4503bc3808c4bc4089d0bfffb91fb Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Thu, 18 Nov 2021 15:29:30 +0200 Subject: [PATCH 24/35] add libgmock-dev Signed-off-by: Stepan Blyshchak --- lgtm.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/lgtm.yml b/lgtm.yml index 68a2fc7586..59f2e812af 100644 --- a/lgtm.yml +++ b/lgtm.yml @@ -21,6 +21,7 @@ extraction: - swig3.0 - libpython2.7-dev - libgtest-dev + - libgmock-dev - dh-exec - doxygen - cdbs From e2b1d3e78552faec9549aeee700832225f3a8d1e Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Thu, 18 Nov 2021 17:24:16 +0200 Subject: [PATCH 25/35] Revert "add libgmock-dev" This reverts commit fe09424ea8f4503bc3808c4bc4089d0bfffb91fb. --- lgtm.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/lgtm.yml b/lgtm.yml index 59f2e812af..68a2fc7586 100644 --- a/lgtm.yml +++ b/lgtm.yml @@ -21,7 +21,6 @@ extraction: - swig3.0 - libpython2.7-dev - libgtest-dev - - libgmock-dev - dh-exec - doxygen - cdbs From 49f9dbd8027b9166764196ec54b7d1ceca5ea377 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 18:57:43 +0200 Subject: [PATCH 26/35] fix conflicts Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index d8941c8bc3..9afa508018 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -273,7 +273,7 @@ bool AclTableRangeMatch::validateAclRuleMatch(const AclRule& rule) const if (find(m_rangeList.begin(), m_rangeList.end(), range.rangeType) == m_rangeList.end()) { SWSS_LOG_ERROR("Range match %s is not supported on table %s", - sai_metadata_get_acl_range_type_name(rangeType), rule.getTableId().c_str()); + sai_metadata_get_acl_range_type_name(range.rangeType), rule.getTableId().c_str()); return false; } } @@ -1150,7 +1150,7 @@ bool AclRule::setAction(sai_acl_entry_attr_t actionId, sai_acl_action_data_t act if (meta->isenum) { // if ACL action attribute requires enum value check if value is supported by the ASIC - if (!m_pAclOrch->isAclActionEnumValueSupported(AclOrch::getAclActionFromAclEntry(actionId), actionData.parameter)) + if (!m_pAclOrch->isAclActionEnumValueSupported(AclEntryActionToAclAction(actionId), actionData.parameter)) { SWSS_LOG_ERROR("Action %s is not supported by ASIC", getAttributeIdName(SAI_OBJECT_TYPE_ACL_ENTRY, actionId).c_str()); @@ -1175,7 +1175,7 @@ bool AclRule::setMatch(sai_acl_entry_attr_t matchId, sai_acl_field_data_t matchD m_matches[matchId] = SaiAttrWrapper(SAI_OBJECT_TYPE_ACL_ENTRY, attr); - if (!m_pTable->validateAclRuleMatch(attrId, *this)) + if (!m_pTable->validateAclRuleMatch(matchId, *this)) { return false; } @@ -1246,11 +1246,6 @@ bool AclRule::hasCounter() const return getCounterOid() != SAI_NULL_OBJECT_ID; } -vector AclRule::getInPorts() -{ - return m_inPorts; -} - const vector& AclRule::getRangeConfig() const { return m_rangeConfig; @@ -1612,11 +1607,6 @@ bool AclRulePacket::validate() return true; } -void AclRuleL3::onUpdate(SubjectType, void *) -{ - // Do nothing -} - AclRuleMirror::AclRuleMirror(AclOrch *aclOrch, MirrorOrch *mirror, string rule, string table) : AclRule(aclOrch, rule, table), m_state(false), @@ -2446,7 +2436,7 @@ bool AclRuleDTelFlowWatchListEntry::update(const AclRule& rule) } AclRuleDTelDropWatchListEntry::AclRuleDTelDropWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table) : - AclRule(aclOrch, rule, table, type), + AclRule(aclOrch, rule, table), m_pDTelOrch(dtel) { } From a01ec617c6cc65be5b56976f9b4d8effce6bd3ed Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 18:58:30 +0200 Subject: [PATCH 27/35] fix conflicts Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 4e9f25cada..9e6db3919c 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -253,7 +253,7 @@ class AclRule sai_object_id_t getOid() const; sai_object_id_t getCounterOid() const; bool hasCounter() const; - vector getInPorts(); + vector getInPorts() const; const vector& getRangeConfig() const; static shared_ptr makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple&); From 1717a7005a3d0f61cea60dd80a4efa3fefcc3bf3 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 19:00:49 +0200 Subject: [PATCH 28/35] fix conflicts Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 9afa508018..43a5fd2aea 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1607,6 +1607,11 @@ bool AclRulePacket::validate() return true; } +void AclRulePacket::onUpdate(SubjectType, void *) +{ + // Do nothing +} + AclRuleMirror::AclRuleMirror(AclOrch *aclOrch, MirrorOrch *mirror, string rule, string table) : AclRule(aclOrch, rule, table), m_state(false), From be3256138f9bed0cbaa55e8240f6a536da2a5966 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 19:03:44 +0200 Subject: [PATCH 29/35] fix conflicts Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 43a5fd2aea..7509238864 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1747,6 +1747,19 @@ bool AclRuleMirror::deactivate() return true; } +bool AclRuleMirror::update(const AclRule& rule) +{ + auto mirrorRule = dynamic_cast(&rule); + if (!mirrorRule) + { + SWSS_LOG_ERROR("Cannot update mirror rule with a rule of a different type"); + return false; + } + + SWSS_LOG_ERROR("Updating mirror rule is currently not implemented"); + return false; +} + void AclRuleMirror::onUpdate(SubjectType type, void *cntx) { if (type != SUBJECT_TYPE_MIRROR_SESSION_CHANGE) From bf41eb4a5475bfc43afd4bf7f28944d167e6cb7f Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 19:07:09 +0200 Subject: [PATCH 30/35] fix conflicts Signed-off-by: Stepan Blyshchak --- tests/mock_tests/aclorch_ut.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 87f8bfa299..126b777943 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -1633,6 +1633,8 @@ namespace aclorch_test // Mirror action is supported on this table ASSERT_TRUE(orch->getAclRule(aclTableName, aclRuleName)); + } + TEST_F(AclOrchTest, AclRuleUpdate) { string acl_table_id = "acl_table_1"; From 869cd7eb8cf07a5f1e2ad7a6d7573e3830c3cda9 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 19:08:39 +0200 Subject: [PATCH 31/35] fix conflicts Signed-off-by: Stepan Blyshchak --- tests/mock_tests/aclorch_ut.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 126b777943..8fb5809970 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -1661,7 +1661,7 @@ namespace aclorch_test auto it_table = acl_tables.find(acl_table_oid); ASSERT_NE(it_table, acl_tables.end()); - class AclRuleTest : public AclRuleL3 + class AclRuleTest : public AclRulePacket { public: AclRuleTest(AclOrch* orch, string rule, string table): From aabdd8c822f3b6c037199ab518c39cf1e2b9be96 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 19:09:05 +0200 Subject: [PATCH 32/35] fix conflicts Signed-off-by: Stepan Blyshchak --- tests/mock_tests/aclorch_ut.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 8fb5809970..18abdf5852 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -1665,7 +1665,7 @@ namespace aclorch_test { public: AclRuleTest(AclOrch* orch, string rule, string table): - AclRuleL3(orch, rule, table, ACL_TABLE_L3, true) + AclRulePacket(orch, rule, table, ACL_TABLE_L3, true) {} void setCounterEnabled(bool enabled) From f49fb54613ac34873fe78b7b232f977bab9434b7 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 19:09:41 +0200 Subject: [PATCH 33/35] fix conflicts Signed-off-by: Stepan Blyshchak --- tests/mock_tests/aclorch_ut.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 18abdf5852..c0d7399570 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -1665,7 +1665,7 @@ namespace aclorch_test { public: AclRuleTest(AclOrch* orch, string rule, string table): - AclRulePacket(orch, rule, table, ACL_TABLE_L3, true) + AclRulePacket(orch, rule, table, true) {} void setCounterEnabled(bool enabled) From c7e8ac0769633ecfeec47438070c4f90ad852f09 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 19:14:51 +0200 Subject: [PATCH 34/35] fix conflicts Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 7509238864..30670bc4d7 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1164,6 +1164,11 @@ bool AclRule::setAction(sai_acl_entry_attr_t actionId, sai_acl_action_data_t act m_actions[actionId] = SaiAttrWrapper(SAI_OBJECT_TYPE_ACL_ENTRY, attr); + if (!m_pTable->validateAclRuleAction(attrId, *this)) + { + return false; + } + return true; } From 70d2c7da0b5ed0da9b11f8a5b0939a0ea8fcdbe9 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Mon, 22 Nov 2021 19:15:40 +0200 Subject: [PATCH 35/35] fix conflicts Signed-off-by: Stepan Blyshchak --- orchagent/aclorch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 30670bc4d7..3d8e847e84 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1164,7 +1164,7 @@ bool AclRule::setAction(sai_acl_entry_attr_t actionId, sai_acl_action_data_t act m_actions[actionId] = SaiAttrWrapper(SAI_OBJECT_TYPE_ACL_ENTRY, attr); - if (!m_pTable->validateAclRuleAction(attrId, *this)) + if (!m_pTable->validateAclRuleAction(actionId, *this)) { return false; }