diff --git a/doc/swss-schema.md b/doc/swss-schema.md index befe0c248c..ac7e25ce5e 100644 --- a/doc/swss-schema.md +++ b/doc/swss-schema.md @@ -459,7 +459,17 @@ Stores rules associated with a specific ACL table on the switch. : next-hop ip address Example: "10.0.0.1" : next-hop group set of addresses Example: "10.0.0.1,10.0.0.3" - mirror_action = 1*255VCHAR ; refer to the mirror session + redirect_action = 1*255CHAR ; redirect parameter + ; This parameter defines a destination for redirected packets + ; it could be: + : name of physical port. Example: "Ethernet10" + : name of LAG port Example: "PortChannel5" + : next-hop ip address Example: "10.0.0.1" + : next-hop group set of addresses Example: "10.0.0.1,10.0.0.3" + + mirror_action = 1*255VCHAR ; refer to the mirror session (by default it will be ingress mirror action) + mirror_ingress_action = 1*255VCHAR ; refer to the mirror session + mirror_egress_action = 1*255VCHAR ; refer to the mirror session ether_type = h16 ; Ethernet type field diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 22600af304..3327079a28 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -56,7 +56,7 @@ orchagent_SOURCES = \ orchagent_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) orchagent_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) -orchagent_LDADD = -lnl-3 -lnl-route-3 -lpthread -lsairedis -lswsscommon -lsaimetadata +orchagent_LDADD = -lnl-3 -lnl-route-3 -lpthread -lsairedis -lswsscommon -lsaimeta -lsaimetadata routeresync_SOURCES = routeresync.cpp routeresync_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 780b38f84c..d224b29f20 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -10,6 +10,7 @@ #include "tokenize.h" #include "timer.h" #include "crmorch.h" +#include "sai_serialize.h" using namespace std; using namespace swss; @@ -60,14 +61,19 @@ acl_rule_attr_lookup_t aclMatchLookup = { MATCH_INNER_L4_DST_PORT, SAI_ACL_ENTRY_ATTR_FIELD_INNER_L4_DST_PORT } }; -acl_rule_attr_lookup_t aclL3ActionLookup = +static acl_rule_attr_lookup_t aclL3ActionLookup = { - { PACKET_ACTION_FORWARD, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION }, - { PACKET_ACTION_DROP, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION }, - { PACKET_ACTION_REDIRECT, SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT } + { ACTION_PACKET_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION }, + { ACTION_REDIRECT_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT }, }; -acl_rule_attr_lookup_t aclDTelActionLookup = +static acl_rule_attr_lookup_t aclMirrorStageLookup = +{ + { ACTION_MIRROR_INGRESS_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS}, + { ACTION_MIRROR_EGRESS_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_EGRESS}, +}; + +static acl_rule_attr_lookup_t aclDTelActionLookup = { { ACTION_DTEL_FLOW_OP, SAI_ACL_ENTRY_ATTR_ACTION_ACL_DTEL_FLOW_OP }, { ACTION_DTEL_INT_SESSION, SAI_ACL_ENTRY_ATTR_ACTION_DTEL_INT_SESSION }, @@ -77,7 +83,13 @@ acl_rule_attr_lookup_t aclDTelActionLookup = { ACTION_DTEL_REPORT_ALL_PACKETS, SAI_ACL_ENTRY_ATTR_ACTION_DTEL_REPORT_ALL_PACKETS } }; -acl_dtel_flow_op_type_lookup_t aclDTelFlowOpTypeLookup = +static acl_packet_action_lookup_t aclPacketActionLookup = +{ + { PACKET_ACTION_FORWARD, SAI_PACKET_ACTION_FORWARD }, + { PACKET_ACTION_DROP, SAI_PACKET_ACTION_DROP }, +}; + +static acl_dtel_flow_op_type_lookup_t aclDTelFlowOpTypeLookup = { { DTEL_FLOW_OP_NOP, SAI_ACL_DTEL_FLOW_OP_NOP }, { DTEL_FLOW_OP_POSTCARD, SAI_ACL_DTEL_FLOW_OP_POSTCARD }, @@ -99,8 +111,8 @@ static acl_table_type_lookup_t aclTableTypeLookUp = static acl_stage_type_lookup_t aclStageLookUp = { - {TABLE_INGRESS, ACL_STAGE_INGRESS }, - {TABLE_EGRESS, ACL_STAGE_EGRESS } + {STAGE_INGRESS, ACL_STAGE_INGRESS }, + {STAGE_EGRESS, ACL_STAGE_EGRESS } }; static acl_ip_type_lookup_t aclIpTypeLookup = @@ -379,6 +391,39 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) return true; } +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; +} + bool AclRule::processIpType(string type, sai_uint32_t &ip_type) { SWSS_LOG_ENTER(); @@ -527,6 +572,17 @@ 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 %lu", m_tableOid); + } + return m_pAclOrch->isAclActionSupported(pTable->stage, action_type); +} + bool AclRule::remove() { SWSS_LOG_ENTER(); @@ -584,12 +640,11 @@ shared_ptr AclRule::makeShared(acl_table_type_t type, AclOrch *acl, Mir { string attr_name = to_upper(fvField(itr)); string attr_value = fvValue(itr); - if (attr_name == ACTION_PACKET_ACTION || attr_name == ACTION_MIRROR_ACTION || - attr_name == ACTION_DTEL_FLOW_OP || attr_name == ACTION_DTEL_INT_SESSION || - attr_name == ACTION_DTEL_DROP_REPORT_ENABLE || - attr_name == ACTION_DTEL_TAIL_DROP_REPORT_ENABLE || - attr_name == ACTION_DTEL_FLOW_SAMPLE_PERCENT || - attr_name == ACTION_DTEL_REPORT_ALL_PACKETS) + 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; @@ -614,7 +669,8 @@ shared_ptr AclRule::makeShared(acl_table_type_t type, AclOrch *acl, Mir } /* Mirror rules can exist in both tables */ - if (action == ACTION_MIRROR_ACTION) + if (aclMirrorStageLookup.find(action) != aclMirrorStageLookup.cend() || + action == ACTION_MIRROR_ACTION /* implicitly ingress in old schema */) { return make_shared(acl, mirror, rule, table, type); } @@ -736,24 +792,37 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) string attr_value = to_upper(_attr_value); sai_attribute_value_t value; - if (attr_name != ACTION_PACKET_ACTION) - { - return false; - } + auto action_str = attr_name; - if (attr_value == PACKET_ACTION_FORWARD) + if (attr_name == ACTION_PACKET_ACTION) { - value.aclaction.parameter.s32 = SAI_PACKET_ACTION_FORWARD; - } - else if (attr_value == PACKET_ACTION_DROP) - { - value.aclaction.parameter.s32 = SAI_PACKET_ACTION_DROP; + const auto it = aclPacketActionLookup.find(attr_value); + if (it != aclPacketActionLookup.cend()) + { + value.aclaction.parameter.s32 = it->second; + } + // handle PACKET_ACTION_REDIRECT in ACTION_PACKET_ACTION for backward compatibility + else if (attr_value.find(PACKET_ACTION_REDIRECT) != string::npos) + { + // resize attr_value to remove argument, _attr_value still has the argument + attr_value.resize(string(PACKET_ACTION_REDIRECT).length()); + + sai_object_id_t param_id = getRedirectObjectId(_attr_value); + if (param_id == SAI_NULL_OBJECT_ID) + { + return false; + } + value.aclaction.parameter.oid = param_id; + + action_str = ACTION_REDIRECT_ACTION; + } + else + { + return false; + } } - else if (attr_value.find(PACKET_ACTION_REDIRECT) != string::npos) + else if (attr_name == ACTION_REDIRECT_ACTION) { - // resize attr_value to remove argument, _attr_value still has the argument - attr_value.resize(string(PACKET_ACTION_REDIRECT).length()); - sai_object_id_t param_id = getRedirectObjectId(_attr_value); if (param_id == SAI_NULL_OBJECT_ID) { @@ -765,11 +834,12 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) { return false; } + value.aclaction.enable = true; - m_actions[aclL3ActionLookup[attr_value]] = value; + m_actions[aclL3ActionLookup[action_str]] = value; - return true; + return AclRule::validateAddAction(attr_name, attr_value); } // This method should return sai attribute id of the redirect destination @@ -957,19 +1027,35 @@ bool AclRuleMirror::validateAddAction(string attr_name, string attr_value) { SWSS_LOG_ENTER(); - if (attr_name != ACTION_MIRROR_ACTION) + sai_acl_entry_attr_t action; + + const auto it = aclMirrorStageLookup.find(attr_name); + if (it != aclMirrorStageLookup.cend()) + { + action = it->second; + } + // handle ACTION_MIRROR_ACTION as ingress by default for backward compatibility + else if (attr_name == ACTION_MIRROR_ACTION) + { + action = SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS; + } + else { return false; } - if (!m_pMirrorOrch->sessionExists(attr_value)) + m_sessionName = attr_value; + + if (!m_pMirrorOrch->sessionExists(m_sessionName)) { + SWSS_LOG_ERROR("Mirror rule reference mirror session that does not exists %s", m_sessionName.c_str()); return false; } - m_sessionName = attr_value; + // insert placeholder value, we'll set the session oid in AclRuleMirror::create() + m_actions[action] = sai_attribute_value_t{}; - return true; + return AclRule::validateAddAction(attr_name, attr_value); } bool AclRuleMirror::validateAddMatch(string attr_name, string attr_value) @@ -1049,20 +1135,19 @@ bool AclRuleMirror::create() { SWSS_LOG_ENTER(); - sai_attribute_value_t value; - bool state = false; sai_object_id_t oid = SAI_NULL_OBJECT_ID; + bool state = false; if (!m_pMirrorOrch->getSessionStatus(m_sessionName, state)) { - throw runtime_error("Failed to get mirror session state"); + SWSS_LOG_THROW("Failed to get mirror session state for session %s", m_sessionName.c_str()); } // Increase session reference count regardless of state to deny // attempt to remove mirror session with attached ACL rules. if (!m_pMirrorOrch->increaseRefCount(m_sessionName)) { - throw runtime_error("Failed to increase mirror session reference count"); + SWSS_LOG_THROW("Failed to increase mirror session reference count for session %s", m_sessionName.c_str()); } if (!state) @@ -1072,15 +1157,15 @@ bool AclRuleMirror::create() if (!m_pMirrorOrch->getSessionOid(m_sessionName, oid)) { - throw runtime_error("Failed to get mirror session OID"); + SWSS_LOG_THROW("Failed to get mirror session OID for session %s", m_sessionName.c_str()); } - value.aclaction.enable = true; - value.aclaction.parameter.objlist.list = &oid; - value.aclaction.parameter.objlist.count = 1; - - m_actions.clear(); - m_actions[SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS] = value; + for (auto& it: m_actions) + { + it.second.aclaction.enable = true; + it.second.aclaction.parameter.objlist.list = &oid; + it.second.aclaction.parameter.objlist.count = 1; + } if (!AclRule::create()) { @@ -1658,7 +1743,7 @@ bool AclRuleDTelFlowWatchListEntry::validateAddAction(string attr_name, string a m_actions[aclDTelActionLookup[attr_name]] = value; - return true; + return AclRule::validateAddAction(attr_name, attr_value); } bool AclRuleDTelFlowWatchListEntry::validate() @@ -1817,7 +1902,7 @@ bool AclRuleDTelDropWatchListEntry::validateAddAction(string attr_name, string a m_actions[aclDTelActionLookup[attr_name]] = value; - return true; + return AclRule::validateAddAction(attr_name, attr_value); } bool AclRuleDTelDropWatchListEntry::validate() @@ -2048,6 +2133,8 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr throw "AclOrch initialization failure"; } + queryAclActionCapability(); + // Attach observers m_mirrorOrch->attach(this); gPortsOrch->attach(this); @@ -2061,6 +2148,181 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr timer->start(); } +void AclOrch::queryAclActionCapability() +{ + SWSS_LOG_ENTER(); + + sai_status_t status {SAI_STATUS_FAILURE}; + sai_attribute_t attr; + vector action_list; + vector fvVector; + + attr.id = SAI_SWITCH_ATTR_MAX_ACL_ACTION_COUNT; + status = sai_switch_api->get_switch_attribute(gSwitchId, 1, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_THROW("AclOrch initialization failed: " + "failed to query maximum ACL action count"); + } + + const auto max_action_count = attr.value.u32; + + for (auto stage_attr: {SAI_SWITCH_ATTR_ACL_STAGE_INGRESS, SAI_SWITCH_ATTR_ACL_STAGE_EGRESS}) + { + auto stage = (stage_attr == SAI_SWITCH_ATTR_ACL_STAGE_INGRESS ? ACL_STAGE_INGRESS : ACL_STAGE_EGRESS); + auto stage_str = (stage_attr == SAI_SWITCH_ATTR_ACL_STAGE_INGRESS ? STAGE_INGRESS : STAGE_EGRESS); + action_list.resize(static_cast(max_action_count)); + + attr.id = stage_attr; + attr.value.aclcapability.action_list.list = action_list.data(); + attr.value.aclcapability.action_list.count = max_action_count; + + status = sai_switch_api->get_switch_attribute(gSwitchId, 1, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_THROW("AclOrch initialization failed: " + "failed to query supported %s ACL actions", stage_str); + } + + SWSS_LOG_INFO("Supported %s action count %d:", stage_str, + attr.value.aclcapability.action_list.count); + + 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); + SWSS_LOG_INFO(" %s", sai_serialize_enum(action, &sai_metadata_enum_sai_acl_action_type_t).c_str()); + } + + // put capabilities in state DB + + auto field = std::string("ACL_ACTIONS") + '|' + stage_str; + auto& acl_action_set = m_aclCapabilities[stage]; + + string delimiter; + ostringstream acl_action_value_stream; + + for (const auto& action_map: {aclL3ActionLookup, aclMirrorStageLookup, aclDTelActionLookup}) + { + for (const auto& it: action_map) + { + auto saiAction = getAclActionFromAclEntry(it.second); + if (acl_action_set.find(saiAction) != acl_action_set.cend()) + { + acl_action_value_stream << delimiter << it.first; + delimiter = comma; + } + } + } + + fvVector.emplace_back(field, acl_action_value_stream.str()); + m_switchTable.set("switch", fvVector); + } + + /* For those ACL action entry attributes for which acl parameter is enumeration (metadata->isenum == true) + * we can query enum values which are implemented by vendor SAI. + * For this purpose we may want to use "sai_query_attribute_enum_values_capability" + * from SAI object API call (saiobject.h). + * However, right now libsairedis does not support SAI object API, so we will just + * put all values as supported for now. + */ + + queryAclActionAttrEnumValues(ACTION_PACKET_ACTION, + aclL3ActionLookup, + aclPacketActionLookup); + queryAclActionAttrEnumValues(ACTION_DTEL_FLOW_OP, + aclDTelActionLookup, + aclDTelFlowOpTypeLookup); +} + +template +void AclOrch::queryAclActionAttrEnumValues(const string &action_name, + const acl_rule_attr_lookup_t& ruleAttrLookupMap, + const AclActionAttrLookupT lookupMap) +{ + vector fvVector; + auto acl_attr = ruleAttrLookupMap.at(action_name); + auto acl_action = getAclActionFromAclEntry(acl_attr); + + /* if the action is not supported then no need to do secondary query for + * supported values + */ + if (isAclActionSupported(ACL_STAGE_INGRESS, acl_action) || + isAclActionSupported(ACL_STAGE_EGRESS, acl_action)) + { + string delimiter; + ostringstream acl_action_value_stream; + auto field = std::string("ACL_ACTION") + '|' + action_name; + + const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, acl_attr); + if (meta == nullptr) + { + SWSS_LOG_THROW("Metadata null pointer returned by sai_metadata_get_attr_metadata for action %s", + action_name.c_str()); + } + + if (!meta->isenum) + { + SWSS_LOG_THROW("%s is not an enum", action_name.c_str()); + } + + // TODO: once sai object api is available make this code compile +#ifdef SAIREDIS_SUPPORT_OBJECT_API + vector values_list(meta->enummetadata->valuescount); + sai_s32_list_t values; + values.count = static_cast(values_list.size()); + values.list = values_list.data(); + + auto status = sai_query_attribute_enum_values_capability(gSwitchId, + SAI_OBJECT_TYPE_ACL_ENTRY, + acl_attr, + &values); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_THROW("sai_query_attribute_enum_values_capability failed for %s", + action_name.c_str()); + } + + for (size_t i = 0; i < values.count; i++) + { + m_aclEnumActionCapabilities[acl_action].insert(values.list[i]); + } +#else + /* assume all enum values are supported untill sai object api is available */ + for (size_t i = 0; i < meta->enummetadata->valuescount; i++) + { + m_aclEnumActionCapabilities[acl_action].insert(meta->enummetadata->values[i]); + } +#endif + + // put supported values in DB + for (const auto& it: lookupMap) + { + const auto foundIt = m_aclEnumActionCapabilities[acl_action].find(it.second); + if (foundIt == m_aclEnumActionCapabilities[acl_action].cend()) + { + continue; + } + acl_action_value_stream << delimiter << it.first; + delimiter = comma; + } + + fvVector.emplace_back(field, acl_action_value_stream.str()); + } + + m_switchTable.set("switch", 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, TableConnector switchTable, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch, DTelOrch *dtelOrch) : Orch(connectors), @@ -2313,6 +2575,26 @@ bool AclOrch::isCombinedMirrorV6Table() return m_isCombinedMirrorV6Table; } +bool AclOrch::isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const +{ + const auto& it = m_aclCapabilities.find(stage); + if (it == m_aclCapabilities.cend()) + { + return false; + } + return it->second.find(action) != it->second.cend(); +} + +bool AclOrch::isAclActionEnumValueSupported(sai_acl_action_type_t action, sai_acl_action_parameter_t param) const +{ + const auto& it = m_aclEnumActionCapabilities.find(action); + if (it == m_aclEnumActionCapabilities.cend()) + { + return false; + } + return it->second.find(param.s32) != it->second.cend(); +} + void AclOrch::doAclTableTask(Consumer &consumer) { SWSS_LOG_ENTER(); @@ -2651,6 +2933,16 @@ sai_object_id_t AclOrch::getTableById(string table_id) return SAI_NULL_OBJECT_ID; } +const AclTable *AclOrch::getTableByOid(sai_object_id_t oid) const +{ + const auto& it = m_AclTables.find(oid); + if (it == m_AclTables.cend()) + { + return nullptr; + } + return &it->second; +} + bool AclOrch::createBindAclTable(AclTable &aclTable, sai_object_id_t &table_oid) { SWSS_LOG_ENTER(); diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 558a032645..45cfd675e7 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -62,7 +62,10 @@ #define MATCH_INNER_L4_DST_PORT "INNER_L4_DST_PORT" #define ACTION_PACKET_ACTION "PACKET_ACTION" +#define ACTION_REDIRECT_ACTION "REDIRECT_ACTION" #define ACTION_MIRROR_ACTION "MIRROR_ACTION" +#define ACTION_MIRROR_INGRESS_ACTION "MIRROR_INGRESS_ACTION" +#define ACTION_MIRROR_EGRESS_ACTION "MIRROR_EGRESS_ACTION" #define ACTION_DTEL_FLOW_OP "FLOW_OP" #define ACTION_DTEL_INT_SESSION "INT_SESSION" #define ACTION_DTEL_DROP_REPORT_ENABLE "DROP_REPORT_ENABLE" @@ -113,7 +116,10 @@ typedef map acl_table_type_lookup_t; typedef map acl_rule_attr_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_action_enum_values_capabilities_t; class AclOrch; @@ -170,7 +176,7 @@ class AclRule AclRule(AclOrch *m_pAclOrch, string rule, string table, acl_table_type_t type, 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) = 0; + virtual bool validateAddAction(string attr_name, string attr_value); 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) @@ -209,6 +215,8 @@ class AclRule void decreaseNextHopRefCount(); + bool isActionSupported(sai_acl_entry_attr_t) const; + static sai_uint32_t m_minPriority; static sai_uint32_t m_maxPriority; AclOrch *m_pAclOrch; @@ -272,10 +280,10 @@ class AclRuleMirror: public AclRule AclRuleCounters getCounters(); protected: - bool m_state; + bool m_state {false}; string m_sessionName; AclRuleCounters counters; - MirrorOrch *m_pMirrorOrch; + MirrorOrch *m_pMirrorOrch {nullptr}; }; class AclRuleDTelFlowWatchListEntry: public AclRule @@ -386,6 +394,7 @@ class AclOrch : public Orch, public Observer void update(SubjectType, void *); sai_object_id_t getTableById(string table_id); + const AclTable* getTableByOid(sai_object_id_t oid) const; static swss::Table& getCountersTable() { @@ -406,10 +415,14 @@ class AclOrch : public Orch, public Observer bool removeAclRule(string table_id, string rule_id); bool isCombinedMirrorV6Table(); + 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; + static sai_acl_action_type_t getAclActionFromAclEntry(sai_acl_entry_attr_t attr); + private: void doTask(Consumer &consumer); void doAclTableTask(Consumer &consumer); @@ -418,6 +431,12 @@ class AclOrch : public Orch, public Observer void init(vector& connectors, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch); void queryMirrorTableCapability(); + void queryAclActionCapability(); + + template + void queryAclActionAttrEnumValues(const string& action_name, + const acl_rule_attr_lookup_t& ruleAttrLookupMap, + const AclActionAttrLookupT lookupMap); static void collectCountersThread(AclOrch *pAclOrch); @@ -444,6 +463,9 @@ class AclOrch : public Orch, public Observer string m_mirrorTableId; string m_mirrorV6TableId; + + acl_capabilities_t m_aclCapabilities; + acl_action_enum_values_capabilities_t m_aclEnumActionCapabilities; }; #endif /* SWSS_ACLORCH_H */ diff --git a/orchagent/acltable.h b/orchagent/acltable.h index 097ead33ae..4244678843 100644 --- a/orchagent/acltable.h +++ b/orchagent/acltable.h @@ -14,8 +14,10 @@ using namespace std; /* TODO: move all acltable and aclrule implementation out of aclorch */ -#define TABLE_INGRESS "INGRESS" -#define TABLE_EGRESS "EGRESS" +#define STAGE_INGRESS "INGRESS" +#define STAGE_EGRESS "EGRESS" +#define TABLE_INGRESS STAGE_INGRESS +#define TABLE_EGRESS STAGE_EGRESS #define TABLE_STAGE "STAGE" typedef enum diff --git a/tests/test_acl.py b/tests/test_acl.py index 197026d548..5cfeb49b4e 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -3,18 +3,25 @@ import re import json -class TestAcl(object): + +class BaseTestAcl(object): + """ base class with helpers for Test classes """ def setup_db(self, dvs): self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) self.cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) self.sdb = swsscommon.DBConnector(6, dvs.redis_sock, 0) - def create_acl_table(self, table, type, ports): + def create_acl_table(self, table, type, ports, stage=None): tbl = swsscommon.Table(self.cdb, "ACL_TABLE") - fvs = swsscommon.FieldValuePairs([("policy_desc", "test"), - ("type", type), - ("ports", ",".join(ports))]) + table_props = [("policy_desc", "test"), + ("type", type), + ("ports", ",".join(ports))] + + if stage is not None: + table_props += [("stage", stage)] + + fvs = swsscommon.FieldValuePairs(table_props) tbl.set(table, fvs) time.sleep(1) @@ -124,6 +131,61 @@ def verify_acl_port_binding(self, dvs, adb, bind_ports): assert len(port_groups) == len(bind_ports) assert set(port_groups) == set(acl_table_groups) + def check_rule_existence(self, entry, rules, verifs): + """ helper function to verify if rule exists """ + + for rule in rules: + ruleD = dict(rule) + # find the rule to match with based on priority + if ruleD["PRIORITY"] == entry['SAI_ACL_ENTRY_ATTR_PRIORITY']: + ruleIndex = rules.index(rule) + # use verification dictionary to match entry to rule + for key in verifs[ruleIndex]: + assert verifs[ruleIndex][key] == entry[key] + return True + return False + + def create_acl_rule(self, table, rule, field, value): + tbl = swsscommon.Table(self.cdb, "ACL_RULE") + fvs = swsscommon.FieldValuePairs([("priority", "666"), + ("PACKET_ACTION", "FORWARD"), + (field, value)]) + tbl.set(table + "|" + rule, fvs) + time.sleep(1) + + def remove_acl_rule(self, table, rule): + tbl = swsscommon.Table(self.cdb, "ACL_RULE") + tbl._del(table + "|" + rule) + time.sleep(1) + + def verify_acl_rule(self, dvs, field, value): + acl_table_id = self.get_acl_table_id(dvs) + + tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY") + acl_entries = [k for k in tbl.getKeys() if k not in dvs.asicdb.default_acl_entries] + assert len(acl_entries) == 1 + + (status, fvs) = tbl.get(acl_entries[0]) + assert status == True + assert len(fvs) == 6 + for fv in fvs: + if fv[0] == "SAI_ACL_ENTRY_ATTR_TABLE_ID": + assert fv[1] == acl_table_id + elif fv[0] == "SAI_ACL_ENTRY_ATTR_ADMIN_STATE": + assert fv[1] == "true" + elif fv[0] == "SAI_ACL_ENTRY_ATTR_PRIORITY": + assert fv[1] == "666" + elif fv[0] == "SAI_ACL_ENTRY_ATTR_ACTION_COUNTER": + assert True + elif fv[0] == "SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION": + assert fv[1] == "SAI_PACKET_ACTION_FORWARD" + elif fv[0] == field: + assert fv[1] == value + else: + assert False + + +class TestAcl(BaseTestAcl): def test_AclTableCreation(self, dvs, testlog): self.setup_db(dvs) db = swsscommon.DBConnector(4, dvs.redis_sock, 0) @@ -923,21 +985,6 @@ def test_V6AclTableDeletion(self, dvs, testlog): # only the default table was left assert len(keys) >= 1 - #helper function to verify if rule exists - def check_rule_existence(self, entry, rules, verifs): - for rule in rules: - ruleD = dict(rule) - #find the rule to match with based on priority - if ruleD["PRIORITY"] == entry['SAI_ACL_ENTRY_ATTR_PRIORITY']: - ruleIndex = rules.index(rule) - #use verification dictionary to match entry to rule - for key in verifs[ruleIndex]: - assert verifs[ruleIndex][key] == entry[key] - #found the rule - return True - #did not find the rule - return False - def test_InsertAclRuleBetweenPriorities(self, dvs, testlog): self.setup_db(dvs) db = swsscommon.DBConnector(4, dvs.redis_sock, 0) @@ -1123,45 +1170,6 @@ def test_RulesWithDiffMaskLengths(self, dvs, testlog): keys = atbl.getKeys() assert len(keys) >= 1 - def create_acl_rule(self, table, rule, field, value): - tbl = swsscommon.Table(self.cdb, "ACL_RULE") - fvs = swsscommon.FieldValuePairs([("priority", "666"), - ("PACKET_ACTION", "FORWARD"), - (field, value)]) - tbl.set(table + "|" + rule, fvs) - time.sleep(1) - - def remove_acl_rule(self, table, rule): - tbl = swsscommon.Table(self.cdb, "ACL_RULE") - tbl._del(table + "|" + rule) - time.sleep(1) - - def verify_acl_rule(self, dvs, field, value): - acl_table_id = self.get_acl_table_id(dvs) - - tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY") - acl_entries = [k for k in tbl.getKeys() if k not in dvs.asicdb.default_acl_entries] - assert len(acl_entries) == 1 - - (status, fvs) = tbl.get(acl_entries[0]) - assert status == True - assert len(fvs) == 6 - for fv in fvs: - if fv[0] == "SAI_ACL_ENTRY_ATTR_TABLE_ID": - assert fv[1] == acl_table_id - elif fv[0] == "SAI_ACL_ENTRY_ATTR_ADMIN_STATE": - assert fv[1] == "true" - elif fv[0] == "SAI_ACL_ENTRY_ATTR_PRIORITY": - assert fv[1] == "666" - elif fv[0] == "SAI_ACL_ENTRY_ATTR_ACTION_COUNTER": - assert True - elif fv[0] == "SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION": - assert fv[1] == "SAI_PACKET_ACTION_FORWARD" - elif fv[0] == field: - assert fv[1] == value - else: - assert False - def test_AclRuleIcmp(self, dvs, testlog): self.setup_db(dvs) @@ -1206,3 +1214,98 @@ def test_AclRuleIcmpV6(self, dvs, testlog): self.remove_acl_rule(acl_table, acl_rule) self.remove_acl_table(acl_table) + + +class TestAclRuleValidation(BaseTestAcl): + """ Test class for cases that check if orchagent corectly validates + ACL rules input + """ + + SWITCH_CAPABILITY_TABLE = "SWITCH_CAPABILITY" + + def get_acl_actions_supported(self, stage): + capability_table = swsscommon.Table(self.sdb, self.SWITCH_CAPABILITY_TABLE) + keys = capability_table.getKeys() + # one switch available + assert len(keys) == 1 + status, fvs = capability_table.get(keys[0]) + assert status == True + + field = "ACL_ACTIONS|{}".format(stage.upper()) + fvs = dict(fvs) + + values_list = fvs.get(field, None) + + if values_list is not None: + values_list = values_list.split(",") + + return values_list + + def test_AclActionValidation(self, dvs, testlog): + """ The test overrides R/O SAI_SWITCH_ATTR_ACL_STAGE_INGRESS/EGRESS switch attributes + to check the case when orchagent refuses to process rules with action that is not supported + by the ASIC + """ + + self.setup_db(dvs) + + stage_name_map = { + "ingress": "SAI_SWITCH_ATTR_ACL_STAGE_INGRESS", + "egress": "SAI_SWITCH_ATTR_ACL_STAGE_EGRESS", + } + + for stage in stage_name_map: + action_values = self.get_acl_actions_supported(stage) + + # virtual switch supports all actions + assert action_values is not None + assert "PACKET_ACTION" in action_values + + sai_acl_stage = stage_name_map[stage] + + # mock switch attribute in VS so only REDIRECT action is supported on this stage + dvs.setReadOnlyAttr("SAI_OBJECT_TYPE_SWITCH", + sai_acl_stage, + # FIXME: here should use sai_serialize_value() for acl_capability_t + # but it is not available in VS testing infrastructure + "false:1:SAI_ACL_ACTION_TYPE_REDIRECT") + + # restart SWSS so orchagent will query updated switch attributes + dvs.stop_swss() + dvs.start_swss() + # wait for daemons to start + time.sleep(2) + # reinit ASIC DB validator object + dvs.init_asicdb_validator() + + action_values = self.get_acl_actions_supported(stage) + # now, PACKET_ACTION is not supported + # and REDIRECT_ACTION is supported + assert "PACKET_ACTION" not in action_values + assert "REDIRECT_ACTION" in action_values + + # try to create a forward rule + + acl_table = "TEST_TABLE" + acl_rule = "TEST_RULE" + + bind_ports = ["Ethernet0", "Ethernet4"] + + self.create_acl_table(acl_table, "L3", bind_ports, stage=stage) + self.create_acl_rule(acl_table, acl_rule, "ICMP_TYPE", "8") + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY") + keys = atbl.getKeys() + + # verify there are no non-default ACL rules created + acl_entry = [k for k in keys if k not in dvs.asicdb.default_acl_entries] + assert len(acl_entry) == 0 + + self.remove_acl_table(acl_table) + # remove rules from CFG DB + self.remove_acl_rule(acl_table, acl_rule) + + dvs.runcmd("supervisorctl restart syncd") + dvs.stop_swss() + dvs.start_swss() + time.sleep(5) diff --git a/tests/test_mirror.py b/tests/test_mirror.py index bcb5b62236..c3da54d43d 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -701,10 +701,18 @@ def remove_acl_table(self, table): tbl._del(table) time.sleep(1) - def create_mirror_acl_dscp_rule(self, table, rule, dscp, session): + def create_mirror_acl_dscp_rule(self, table, rule, dscp, session, stage=None): + action_name = "mirror_action" + action_name_map = { + "ingress": "MIRROR_INGRESS_ACTION", + "egress": "MIRROR_EGRESS_ACTION" + } + if stage is not None: # else it should be treated as ingress by default in orchagent + assert stage in ('ingress', 'egress'), "invalid stage input {}".format(stage) + action_name = action_name_map[stage] tbl = swsscommon.Table(self.cdb, "ACL_RULE") fvs = swsscommon.FieldValuePairs([("priority", "1000"), - ("mirror_action", session), + (action_name, session), ("DSCP", dscp)]) tbl.set(table + "|" + rule, fvs) time.sleep(1) @@ -714,6 +722,74 @@ def remove_mirror_acl_dscp_rule(self, table, rule): tbl._del(table + "|" + rule) time.sleep(1) + def test_AclBindMirrorPerStage(self, dvs, testlog): + """ + This test configures mirror rules with specifying explicitely + the mirror action stage (ingress, egress) and verifies ASIC db + entry set with correct mirror action + """ + self.setup_db(dvs) + + session = "MIRROR_SESSION" + acl_table = "MIRROR_TABLE" + acl_rule = "MIRROR_RULE" + + # bring up port; assign ip; create neighbor; create route + self.set_interface_status(dvs, "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 + self.create_acl_table(acl_table, ["Ethernet0", "Ethernet4"]) + + for stage, asic_attr in (("ingress", "SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS"), + ("egress", "SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_EGRESS")): + # create acl rule with dscp value 48 + self.create_mirror_acl_dscp_rule(acl_table, acl_rule, "48", session, stage=stage) + + # assert acl rule is 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) == 1 + + (status, fvs) = tbl.get(rule_entries[0]) + assert status == True + + asic_attr_found = False + for fv in fvs: + if fv[0] == asic_attr: + asic_attr_found = True + + assert asic_attr_found == True + + # remove acl rule + self.remove_mirror_acl_dscp_rule(acl_table, acl_rule) + + # remove acl table + self.remove_acl_table(acl_table) + + # 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(dvs, "Ethernet32", "down") def test_AclBindMirror(self, dvs, testlog): """