diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 364fff48538a..e0106f00cee4 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -64,6 +64,7 @@ orchagent_SOURCES = \ pbh/pbhrule.cpp \ pbhorch.cpp \ saihelper.cpp \ + saiattr.cpp \ switchorch.cpp \ pfcwdorch.cpp \ pfcactionhandler.cpp \ diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index d9dc26e99e94..e3b279a482db 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -60,8 +60,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 }, @@ -69,6 +69,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_rule_attr_lookup_t aclL3ActionLookup = { { ACTION_PACKET_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION }, @@ -164,6 +170,16 @@ static map aclCounterLookup = {SAI_ACL_COUNTER_ATTR_ENABLE_PACKET_COUNT, SAI_ACL_COUNTER_ATTR_PACKETS}, }; +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) + { + SWSS_LOG_THROW("Metadata null pointer returned by sai_metadata_get_attr_metadata"); + } + return meta->attridname; +} + AclRule::AclRule(AclOrch *pAclOrch, string rule, string table, acl_table_type_t type, bool createCounter) : m_pAclOrch(pAclOrch), m_id(rule), @@ -186,12 +202,11 @@ bool AclRule::validateAddPriority(string attr_name, string attr_value) { char *endp = NULL; errno = 0; - m_priority = (uint32_t)strtol(attr_value.c_str(), &endp, 0); + auto priority = (uint32_t)strtol(attr_value.c_str(), &endp, 0); // check conversion was successful and the value is within the allowed range status = (errno == 0) && (endp == attr_value.c_str() + attr_value.size()) && - (m_priority >= m_minPriority) && - (m_priority <= m_maxPriority); + setPriority(priority); } return status; @@ -201,7 +216,11 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) { SWSS_LOG_ENTER(); - sai_attribute_value_t value; + sai_acl_field_data_t matchData{}; + vector inPorts; + vector outPorts; + + matchData.enable = true; try { @@ -218,7 +237,6 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) return false; } - m_inPorts.clear(); for (auto alias : ports) { Port port; @@ -234,11 +252,11 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) return false; } - m_inPorts.push_back(port.m_port_id); + inPorts.push_back(port.m_port_id); } - value.aclfield.data.objlist.count = static_cast(m_inPorts.size()); - value.aclfield.data.objlist.list = m_inPorts.data(); + matchData.data.objlist.count = static_cast(inPorts.size()); + matchData.data.objlist.list = inPorts.data(); } else if (attr_name == MATCH_OUT_PORTS) { @@ -249,7 +267,6 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) return false; } - m_outPorts.clear(); for (auto alias : ports) { Port port; @@ -265,49 +282,49 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) return false; } - m_outPorts.push_back(port.m_port_id); + outPorts.push_back(port.m_port_id); } - value.aclfield.data.objlist.count = static_cast(m_outPorts.size()); - value.aclfield.data.objlist.list = m_outPorts.data(); + matchData.data.objlist.count = static_cast(outPorts.size()); + matchData.data.objlist.list = outPorts.data(); } else if (attr_name == MATCH_IP_TYPE) { - if (!processIpType(attr_value, value.aclfield.data.u32)) + if (!processIpType(attr_value, matchData.data.u32)) { SWSS_LOG_ERROR("Invalid IP type %s", attr_value.c_str()); return false; } - value.aclfield.mask.u32 = 0xFFFFFFFF; + matchData.mask.u32 = 0xFFFFFFFF; } else if (attr_name == MATCH_TCP_FLAGS) { // Support both exact value match and value/mask match auto flag_data = tokenize(attr_value, '/'); - value.aclfield.data.u8 = to_uint(flag_data[0], 0, 0x3F); + matchData.data.u8 = to_uint(flag_data[0], 0, 0x3F); if (flag_data.size() == 2) { - value.aclfield.mask.u8 = to_uint(flag_data[1], 0, 0x3F); + matchData.mask.u8 = to_uint(flag_data[1], 0, 0x3F); } else { - value.aclfield.mask.u8 = 0x3F; + matchData.mask.u8 = 0x3F; } } else if (attr_name == MATCH_ETHER_TYPE || attr_name == MATCH_L4_SRC_PORT || attr_name == MATCH_L4_DST_PORT) { - value.aclfield.data.u16 = to_uint(attr_value); - value.aclfield.mask.u16 = 0xFFFF; + matchData.data.u16 = to_uint(attr_value); + matchData.mask.u16 = 0xFFFF; } else if (attr_name == MATCH_VLAN_ID) { - value.aclfield.data.u16 = to_uint(attr_value); - value.aclfield.mask.u16 = 0xFFF; + matchData.data.u16 = to_uint(attr_value); + matchData.mask.u16 = 0xFFF; - if (value.aclfield.data.u16 < MIN_VLAN_ID || value.aclfield.data.u16 > MAX_VLAN_ID) + if (matchData.data.u16 < MIN_VLAN_ID || matchData.data.u16 > MAX_VLAN_ID) { SWSS_LOG_ERROR("Invalid VLAN ID: %s", attr_value.c_str()); return false; @@ -318,21 +335,21 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) /* Support both exact value match and value/mask match */ auto dscp_data = tokenize(attr_value, '/'); - value.aclfield.data.u8 = to_uint(dscp_data[0], 0, 0x3F); + matchData.data.u8 = to_uint(dscp_data[0], 0, 0x3F); if (dscp_data.size() == 2) { - value.aclfield.mask.u8 = to_uint(dscp_data[1], 0, 0x3F); + matchData.mask.u8 = to_uint(dscp_data[1], 0, 0x3F); } else { - value.aclfield.mask.u8 = 0x3F; + matchData.mask.u8 = 0x3F; } } else if (attr_name == MATCH_IP_PROTOCOL || attr_name == MATCH_NEXT_HEADER) { - value.aclfield.data.u8 = to_uint(attr_value); - value.aclfield.mask.u8 = 0xFF; + matchData.data.u8 = to_uint(attr_value); + matchData.mask.u8 = 0xFF; } else if (attr_name == MATCH_SRC_IP || attr_name == MATCH_DST_IP) { @@ -343,8 +360,8 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) SWSS_LOG_ERROR("IP type is not v4 type"); return false; } - value.aclfield.data.ip4 = ip.getIp().getV4Addr(); - value.aclfield.mask.ip4 = ip.getMask().getV4Addr(); + matchData.data.ip4 = ip.getIp().getV4Addr(); + matchData.mask.ip4 = ip.getMask().getV4Addr(); } else if (attr_name == MATCH_SRC_IPV6 || attr_name == MATCH_DST_IPV6) { @@ -354,52 +371,59 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) SWSS_LOG_ERROR("IP type is not v6 type"); return false; } - memcpy(value.aclfield.data.ip6, ip.getIp().getV6Addr(), 16); - memcpy(value.aclfield.mask.ip6, ip.getMask().getV6Addr(), 16); + memcpy(matchData.data.ip6, ip.getIp().getV6Addr(), 16); + memcpy(matchData.mask.ip6, ip.getMask().getV6Addr(), 16); } else if ((attr_name == MATCH_L4_SRC_PORT_RANGE) || (attr_name == MATCH_L4_DST_PORT_RANGE)) { - if (sscanf(attr_value.c_str(), "%d-%d", &value.u32range.min, &value.u32range.max) != 2) + AclRangeConfig rangeConfig{}; + if (sscanf(attr_value.c_str(), "%d-%d", &rangeConfig.min, &rangeConfig.max) != 2) { SWSS_LOG_ERROR("Range parse error. Attribute: %s, value: %s", attr_name.c_str(), attr_value.c_str()); return false; } + rangeConfig.rangeType = aclRangeTypeLookup[attr_name]; + // check boundaries - if ((value.u32range.min > USHRT_MAX) || - (value.u32range.max > USHRT_MAX) || - (value.u32range.min > value.u32range.max)) + if ((rangeConfig.min > USHRT_MAX) || + (rangeConfig.max > USHRT_MAX) || + (rangeConfig.min > rangeConfig.max)) { SWSS_LOG_ERROR("Range parse error. Invalid range value. Attribute: %s, value: %s", attr_name.c_str(), attr_value.c_str()); return false; } + + m_rangeConfig.push_back(rangeConfig); + + return true; } else if (attr_name == MATCH_TC) { - value.aclfield.data.u8 = to_uint(attr_value); - value.aclfield.mask.u8 = 0xFF; + matchData.data.u8 = to_uint(attr_value); + matchData.mask.u8 = 0xFF; } else if (attr_name == MATCH_ICMP_TYPE || attr_name == MATCH_ICMP_CODE || attr_name == MATCH_ICMPV6_TYPE || attr_name == MATCH_ICMPV6_CODE) { - value.aclfield.data.u8 = to_uint(attr_value); - value.aclfield.mask.u8 = 0xFF; + matchData.data.u8 = to_uint(attr_value); + matchData.mask.u8 = 0xFF; } else if (attr_name == MATCH_TUNNEL_VNI) { - value.aclfield.data.u32 = to_uint(attr_value); - value.aclfield.mask.u32 = 0xFFFFFFFF; + matchData.data.u32 = to_uint(attr_value); + matchData.mask.u32 = 0xFFFFFFFF; } else if (attr_name == MATCH_INNER_ETHER_TYPE || attr_name == MATCH_INNER_L4_SRC_PORT || attr_name == MATCH_INNER_L4_DST_PORT) { - value.aclfield.data.u16 = to_uint(attr_value); - value.aclfield.mask.u16 = 0xFFFF; + matchData.data.u16 = to_uint(attr_value); + matchData.mask.u16 = 0xFFFF; } else if (attr_name == MATCH_INNER_IP_PROTOCOL) { - value.aclfield.data.u8 = to_uint(attr_value); - value.aclfield.mask.u8 = 0xFF; + matchData.data.u8 = to_uint(attr_value); + matchData.mask.u8 = 0xFF; } } catch (exception &e) @@ -422,42 +446,7 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) attr_name = MATCH_NEXT_HEADER; } - m_matches[aclMatchLookup[attr_name]] = 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; + return setMatch(aclMatchLookup[attr_name], matchData); } bool AclRule::processIpType(string type, sai_uint32_t &ip_type) @@ -526,50 +515,41 @@ bool AclRule::createRule() rule_attrs.push_back(attr); } - // store matches - for (auto it : m_matches) + if (!m_rangeConfig.empty()) { - // collect ranges and add them later as a list - if (((sai_acl_range_type_t)it.first == SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) || - ((sai_acl_range_type_t)it.first == SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE)) + for (const auto& rangeConfig: m_rangeConfig) { - SWSS_LOG_INFO("Creating range object %u..%u", it.second.u32range.min, it.second.u32range.max); + SWSS_LOG_INFO("Creating range object %u..%u", rangeConfig.min, rangeConfig.max); - AclRange *range = AclRange::create((sai_acl_range_type_t)it.first, it.second.u32range.min, it.second.u32range.max); + AclRange *range = AclRange::create(rangeConfig.rangeType, rangeConfig.min, rangeConfig.max); if (!range) { // release already created range if any AclRange::remove(range_objects, range_object_list.count); return false; } - else - { - range_objects[range_object_list.count++] = range->getOid(); - } - } - else - { - attr.id = it.first; - attr.value = it.second; - attr.value.aclfield.enable = true; - rule_attrs.push_back(attr); + + m_ranges.push_back(range); + range_objects[range_object_list.count++] = range->getOid(); } - } - // store ranges if any - if (range_object_list.count > 0) - { attr.id = SAI_ACL_ENTRY_ATTR_FIELD_ACL_RANGE_TYPE; attr.value.aclfield.enable = true; attr.value.aclfield.data.objlist = range_object_list; rule_attrs.push_back(attr); } + // store matches + for (auto& it : m_matches) + { + attr = it.second.getSaiAttr(); + rule_attrs.push_back(attr); + } + // store actions - for (auto it : m_actions) + for (auto& it : m_actions) { - attr.id = it.first; - attr.value = it.second; + attr = it.second.getSaiAttr(); rule_attrs.push_back(attr); } @@ -671,11 +651,9 @@ bool AclRule::remove() void AclRule::updateInPorts() { SWSS_LOG_ENTER(); - sai_attribute_t attr; sai_status_t status; - attr.id = SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS; - attr.value = m_matches[SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS]; + auto attr = m_matches[SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS].getSaiAttr(); attr.value.aclfield.enable = true; status = sai_acl_api->set_acl_entry_attribute(m_ruleOid, &attr); @@ -685,6 +663,279 @@ void AclRule::updateInPorts() } } +bool AclRule::update(const AclRule& updatedRule) +{ + SWSS_LOG_ENTER(); + + if (!m_rangeConfig.empty() || !updatedRule.m_rangeConfig.empty()) + { + SWSS_LOG_ERROR("Updating range matches is currently not implemented"); + return false; + } + + if (!updateCounter(updatedRule)) + { + return false; + } + + if (!updatePriority(updatedRule)) + { + return false; + } + + if (!updateMatches(updatedRule)) + { + return false; + } + + if (!updateActions(updatedRule)) + { + return false; + } + + return true; +} + +bool AclRule::updateCounter(const AclRule& updatedRule) +{ + if (updatedRule.m_createCounter) + { + if (!enableCounter()) + { + return false; + } + } + else + { + if (!disableCounter()) + { + return false; + } + } + + m_createCounter = updatedRule.m_createCounter; + + return true; +} + +bool AclRule::updatePriority(const AclRule& updatedRule) +{ + if (m_priority == updatedRule.m_priority) + { + return true; + } + + sai_attribute_t attr {}; + attr.id = SAI_ACL_ENTRY_ATTR_PRIORITY; + attr.value.s32 = updatedRule.m_priority; + if (!setAttribute(attr)) + { + return false; + } + + m_priority = updatedRule.m_priority; + + return true; +} + +bool AclRule::updateMatches(const AclRule& updatedRule) +{ + vector> matchesUpdated; + vector> matchesDisabled; + + // Diff by value to get new matches and updated matches + // in a single set_difference pass. + set_difference( + updatedRule.m_matches.begin(), + updatedRule.m_matches.end(), + m_matches.begin(), m_matches.end(), + back_inserter(matchesUpdated) + ); + + // Diff by key only to get delete matches. Assuming that + // deleted matches mean setting a match attribute to disabled state. + set_difference( + m_matches.begin(), m_matches.end(), + updatedRule.m_matches.begin(), + updatedRule.m_matches.end(), + back_inserter(matchesDisabled), + [](auto& oldMatch, auto& newMatch) + { + return oldMatch.first < newMatch.first; + } + ); + + for (const auto& attrPair: matchesDisabled) + { + auto attr = attrPair.second.getSaiAttr(); + attr.value.aclfield.enable = false; + if (!setAttribute(attr)) + { + return false; + } + m_matches.erase(attrPair.first); + } + + for (const auto& attrPair: matchesUpdated) + { + auto attr = attrPair.second.getSaiAttr(); + if (!setAttribute(attr)) + { + return false; + } + setMatch(attrPair.first, attr.value.aclfield); + } + + return true; +} + +bool AclRule::updateActions(const AclRule& updatedRule) +{ + vector> actionsUpdated; + vector> actionsDisabled; + + // Diff by value to get new action and updated actions + // in a single set_difference pass. + set_difference( + updatedRule.m_actions.begin(), + updatedRule.m_actions.end(), + m_actions.begin(), m_actions.end(), + back_inserter(actionsUpdated) + ); + + // Diff by key only to get delete actions. Assuming that + // action matches mean setting a action attribute to disabled state. + set_difference( + m_actions.begin(), m_actions.end(), + updatedRule.m_actions.begin(), + updatedRule.m_actions.end(), + back_inserter(actionsDisabled), + [](auto& oldAction, auto& newAction) + { + return oldAction.first < newAction.first; + } + ); + + for (const auto& attrPair: actionsDisabled) + { + auto attr = attrPair.second.getSaiAttr(); + attr.value.aclaction.enable = false; + if (!setAttribute(attr)) + { + return false; + } + m_actions.erase(attrPair.first); + } + + for (const auto& attrPair: actionsUpdated) + { + auto attr = attrPair.second.getSaiAttr(); + if (!setAttribute(attr)) + { + return false; + } + setAction(attrPair.first, attr.value.aclaction); + } + + return true; +} + +bool AclRule::setPriority(const sai_uint32_t &value) +{ + if (!(value >= m_minPriority && value <= m_maxPriority)) + { + SWSS_LOG_ERROR("Priority value %d is out of supported priority range %d-%d", + value, m_minPriority, m_maxPriority); + return false; + } + m_priority = value; + return true; +} + +bool AclRule::setAction(sai_acl_entry_attr_t actionId, sai_acl_action_data_t actionData) +{ + if (!isActionSupported(actionId)) + { + SWSS_LOG_ERROR("Action attribute %s is not supported", + getAttributeIdName(SAI_OBJECT_TYPE_ACL_ENTRY, actionId).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, actionId); + if (meta == nullptr) + { + SWSS_LOG_THROW("Metadata null pointer returned by sai_metadata_get_attr_metadata for action %d", actionId); + } + 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)) + { + SWSS_LOG_ERROR("Action %s is not supported by ASIC", + getAttributeIdName(SAI_OBJECT_TYPE_ACL_ENTRY, actionId).c_str()); + return false; + } + } + + sai_attribute_t attr; + attr.id = actionId; + attr.value.aclaction = actionData; + + m_actions[actionId] = SaiAttrWrapper(SAI_OBJECT_TYPE_ACL_ENTRY, attr); + + return true; +} + +bool AclRule::setMatch(sai_acl_entry_attr_t matchId, sai_acl_field_data_t matchData) +{ + sai_attribute_t attr; + attr.id = matchId; + attr.value.aclfield = matchData; + + m_matches[matchId] = SaiAttrWrapper(SAI_OBJECT_TYPE_ACL_ENTRY, attr); + + return true; +} + +bool AclRule::setAttribute(sai_attribute_t attr) +{ + auto meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, attr.id); + if (!meta) + { + SWSS_LOG_THROW("Failed to get metadata for attribute id %d", attr.id); + } + auto status = sai_acl_api->set_acl_entry_attribute(m_ruleOid, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to update attribute %s on ACL rule %s in ACL table %s: %s", + meta->attridname, getId().c_str(), getTableId().c_str(), + sai_serialize_status(status).c_str()); + return false; + } + SWSS_LOG_INFO("Successfully updated action attribute %s on ACL rule %s in ACL table %s", + meta->attridname, getId().c_str(), getTableId().c_str()); + return true; +} + +vector AclRule::getInPorts() const +{ + vector inPorts; + auto it = m_matches.find(SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS); + if (it == m_matches.end()) + { + return inPorts; + } + auto attr = it->second.getSaiAttr(); + if (!attr.value.aclfield.enable) + { + return inPorts; + } + auto objlist = attr.value.aclfield.data.objlist; + inPorts = vector(objlist.list, objlist.list + objlist.count); + return inPorts; +} + 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 action; @@ -887,12 +1138,11 @@ bool AclRule::createCounter() bool AclRule::removeRanges() { SWSS_LOG_ENTER(); - for (auto it : m_matches) + for (const auto& rangeConfig: m_rangeConfig) { - if (((sai_acl_range_type_t)it.first == SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE) || - ((sai_acl_range_type_t)it.first == SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE)) + if (!AclRange::remove(rangeConfig.rangeType, rangeConfig.min, rangeConfig.max)) { - return AclRange::remove((sai_acl_range_type_t)it.first, it.second.u32range.min, it.second.u32range.max); + return false; } } return true; @@ -932,7 +1182,7 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) SWSS_LOG_ENTER(); string attr_value = to_upper(_attr_value); - sai_attribute_value_t value; + sai_acl_action_data_t actionData; auto action_str = attr_name; @@ -941,7 +1191,7 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) const auto it = aclPacketActionLookup.find(attr_value); if (it != aclPacketActionLookup.cend()) { - value.aclaction.parameter.s32 = it->second; + actionData.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) @@ -968,14 +1218,14 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) { return false; } - value.aclaction.parameter.oid = param_id; + actionData.parameter.oid = param_id; action_str = ACTION_REDIRECT_ACTION; } // handle PACKET_ACTION_DO_NOT_NAT in ACTION_PACKET_ACTION else if (attr_value == PACKET_ACTION_DO_NOT_NAT) { - value.aclaction.parameter.booldata = true; + actionData.parameter.booldata = true; action_str = ACTION_DO_NOT_NAT_ACTION; } else @@ -990,18 +1240,16 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) { return false; } - value.aclaction.parameter.oid = param_id; + actionData.parameter.oid = param_id; } else { return false; } - value.aclaction.enable = true; - - m_actions[aclL3ActionLookup[action_str]] = value; + actionData.enable = true; - return AclRule::validateAddAction(attr_name, attr_value); + return setAction(aclL3ActionLookup[action_str], actionData); } // This method should return sai attribute id of the redirect destination @@ -1111,7 +1359,7 @@ bool AclRuleL3::validate() { SWSS_LOG_ENTER(); - if (m_matches.size() == 0 || m_actions.size() != 1) + if ((m_rangeConfig.empty() && m_matches.empty()) || m_actions.size() != 1) { return false; } @@ -1119,7 +1367,7 @@ bool AclRuleL3::validate() return true; } -void AclRuleL3::update(SubjectType, void *) +void AclRuleL3::onUpdate(SubjectType, void *) { // Do nothing } @@ -1214,9 +1462,7 @@ bool AclRuleMirror::validateAddAction(string attr_name, string attr_value) m_sessionName = attr_value; // insert placeholder value, we'll set the session oid in AclRuleMirror::create() - m_actions[action] = sai_attribute_value_t{}; - - return AclRule::validateAddAction(attr_name, attr_value); + return setAction(action, sai_acl_action_data_t{}); } bool AclRuleMirror::validateAddMatch(string attr_name, string attr_value) @@ -1287,7 +1533,7 @@ bool AclRuleMirror::validate() { SWSS_LOG_ENTER(); - if (m_matches.size() == 0 || m_sessionName.empty()) + if ((m_rangeConfig.empty() && m_matches.empty()) || m_sessionName.empty()) { return false; } @@ -1337,9 +1583,11 @@ bool AclRuleMirror::activate() for (auto& it: m_actions) { - it.second.aclaction.enable = true; - it.second.aclaction.parameter.objlist.list = &oid; - it.second.aclaction.parameter.objlist.count = 1; + auto attr = it.second.getSaiAttr(); + attr.value.aclaction.enable = true; + attr.value.aclaction.parameter.objlist.list = &oid; + attr.value.aclaction.parameter.objlist.count = 1; + setAction(it.first, attr.value.aclaction); } if (!AclRule::createRule()) @@ -1382,7 +1630,7 @@ bool AclRuleMirror::deactivate() return true; } -void AclRuleMirror::update(SubjectType type, void *cntx) +void AclRuleMirror::onUpdate(SubjectType type, void *cntx) { if (type != SUBJECT_TYPE_MIRROR_SESSION_CHANGE) { @@ -1408,6 +1656,19 @@ void AclRuleMirror::update(SubjectType type, void *cntx) } } +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; +} + AclRuleMclag::AclRuleMclag(AclOrch *aclOrch, string rule, string table, acl_table_type_t type, bool createCounter) : AclRuleL3(aclOrch, rule, table, type, createCounter) { @@ -1828,7 +2089,7 @@ bool AclTable::create() return status == SAI_STATUS_SUCCESS; } -void AclTable::update(SubjectType type, void *cntx) +void AclTable::onUpdate(SubjectType type, void *cntx) { SWSS_LOG_ENTER(); @@ -2016,6 +2277,36 @@ bool AclTable::remove(string rule_id) } } +bool AclTable::updateRule(shared_ptr updatedRule) +{ + SWSS_LOG_ENTER(); + + if (!updatedRule) + { + return false; + } + + auto ruleId = updatedRule->getId(); + auto ruleIter = rules.find(ruleId); + if (ruleIter == rules.end()) + { + SWSS_LOG_ERROR("Failed to update ACL rule %s as it does not exist in %s", + ruleId.c_str(), id.c_str()); + return false; + } + + if (!ruleIter->second->update(*updatedRule)) + { + SWSS_LOG_ERROR("Failed to update ACL rule %s in table %s", + ruleId.c_str(), id.c_str()); + return false; + } + + SWSS_LOG_NOTICE("Successfully updated ACL rule %s in table %s", + ruleId.c_str(), id.c_str()); + return true; +} + bool AclTable::clear() { SWSS_LOG_ENTER(); @@ -2045,7 +2336,7 @@ bool AclRuleDTelFlowWatchListEntry::validateAddAction(string attr_name, string a { SWSS_LOG_ENTER(); - sai_attribute_value_t value; + sai_acl_action_data_t actionData; string attr_value = to_upper(attr_val); sai_object_id_t session_oid; @@ -2069,7 +2360,7 @@ bool AclRuleDTelFlowWatchListEntry::validateAddAction(string attr_name, string a return false; } - value.aclaction.parameter.s32 = it->second; + actionData.parameter.s32 = it->second; if (attr_value == DTEL_FLOW_OP_INT) { @@ -2088,7 +2379,7 @@ bool AclRuleDTelFlowWatchListEntry::validateAddAction(string attr_name, string a bool ret = m_pDTelOrch->getINTSessionOid(attr_value, session_oid); if (ret) { - value.aclaction.parameter.oid = session_oid; + actionData.parameter.oid = session_oid; // Increase session reference count regardless of state to deny // attempt to remove INT session with attached ACL rules. @@ -2107,22 +2398,20 @@ bool AclRuleDTelFlowWatchListEntry::validateAddAction(string attr_name, string a if (attr_name == ACTION_DTEL_FLOW_SAMPLE_PERCENT) { - value.aclaction.parameter.u8 = to_uint(attr_value); + actionData.parameter.u8 = to_uint(attr_value); } - value.aclaction.enable = true; + actionData.enable = true; if (attr_name == ACTION_DTEL_REPORT_ALL_PACKETS || attr_name == ACTION_DTEL_DROP_REPORT_ENABLE || attr_name == ACTION_DTEL_TAIL_DROP_REPORT_ENABLE) { - value.aclaction.parameter.booldata = (attr_value == DTEL_ENABLED) ? true : false; - value.aclaction.enable = (attr_value == DTEL_ENABLED) ? true : false; + actionData.parameter.booldata = (attr_value == DTEL_ENABLED) ? true : false; + actionData.enable = (attr_value == DTEL_ENABLED) ? true : false; } - m_actions[aclDTelActionLookup[attr_name]] = value; - - return AclRule::validateAddAction(attr_name, attr_value); + return setAction(aclDTelActionLookup[attr_name], actionData); } bool AclRuleDTelFlowWatchListEntry::validate() @@ -2134,7 +2423,7 @@ bool AclRuleDTelFlowWatchListEntry::validate() return false; } - if (m_matches.size() == 0 || m_actions.size() == 0) + if ((m_rangeConfig.empty() && m_matches.empty()) || m_actions.size() == 0) { return false; } @@ -2202,9 +2491,9 @@ bool AclRuleDTelFlowWatchListEntry::deactivate() return true; } -void AclRuleDTelFlowWatchListEntry::update(SubjectType type, void *cntx) +void AclRuleDTelFlowWatchListEntry::onUpdate(SubjectType type, void *cntx) { - sai_attribute_value_t value; + sai_acl_action_data_t actionData; sai_object_id_t session_oid = SAI_NULL_OBJECT_ID; if (!m_pDTelOrch) @@ -2235,8 +2524,8 @@ void AclRuleDTelFlowWatchListEntry::update(SubjectType type, void *cntx) return; } - value.aclaction.enable = true; - value.aclaction.parameter.oid = session_oid; + actionData.enable = true; + actionData.parameter.oid = session_oid; // Increase session reference count regardless of state to deny // attempt to remove INT session with attached ACL rules. @@ -2245,7 +2534,11 @@ void AclRuleDTelFlowWatchListEntry::update(SubjectType type, void *cntx) throw runtime_error("Failed to increase INT session reference count"); } - m_actions[SAI_ACL_ENTRY_ATTR_ACTION_DTEL_INT_SESSION] = value; + if (!setAction(SAI_ACL_ENTRY_ATTR_ACTION_DTEL_INT_SESSION, actionData)) + { + SWSS_LOG_ERROR("Failed to set action SAI_ACL_ENTRY_ATTR_ACTION_DTEL_INT_SESSION"); + return; + } INT_session_valid = true; @@ -2259,6 +2552,19 @@ void AclRuleDTelFlowWatchListEntry::update(SubjectType type, void *cntx) } } +bool AclRuleDTelFlowWatchListEntry::update(const AclRule& rule) +{ + auto dtelDropWathcListRule = dynamic_cast(&rule); + if (!dtelDropWathcListRule) + { + SWSS_LOG_ERROR("Cannot update DTEL flow watch list rule with a rule of a different type"); + return false; + } + + SWSS_LOG_ERROR("Updating DTEL flow watch list rule is currently not implemented"); + return false; +} + AclRuleDTelDropWatchListEntry::AclRuleDTelDropWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table, acl_table_type_t type) : AclRule(aclOrch, rule, table, type), m_pDTelOrch(dtel) @@ -2274,7 +2580,7 @@ bool AclRuleDTelDropWatchListEntry::validateAddAction(string attr_name, string a return false; } - sai_attribute_value_t value; + sai_acl_action_data_t actionData; string attr_value = to_upper(attr_val); if (attr_name != ACTION_DTEL_DROP_REPORT_ENABLE && @@ -2284,13 +2590,10 @@ bool AclRuleDTelDropWatchListEntry::validateAddAction(string attr_name, string a return false; } + actionData.parameter.booldata = (attr_value == DTEL_ENABLED) ? true : false; + actionData.enable = (attr_value == DTEL_ENABLED) ? true : false; - value.aclaction.parameter.booldata = (attr_value == DTEL_ENABLED) ? true : false; - value.aclaction.enable = (attr_value == DTEL_ENABLED) ? true : false; - - m_actions[aclDTelActionLookup[attr_name]] = value; - - return AclRule::validateAddAction(attr_name, attr_value); + return setAction(aclDTelActionLookup[attr_name], actionData); } bool AclRuleDTelDropWatchListEntry::validate() @@ -2302,7 +2605,7 @@ bool AclRuleDTelDropWatchListEntry::validate() return false; } - if (m_matches.size() == 0 || m_actions.size() == 0) + if ((m_rangeConfig.empty() && m_matches.empty()) || m_actions.size() == 0) { return false; } @@ -2310,7 +2613,7 @@ bool AclRuleDTelDropWatchListEntry::validate() return true; } -void AclRuleDTelDropWatchListEntry::update(SubjectType, void *) +void AclRuleDTelDropWatchListEntry::onUpdate(SubjectType, void *) { // Do nothing } @@ -2816,13 +3119,13 @@ void AclOrch::update(SubjectType type, void *cntx) { if (type == SUBJECT_TYPE_PORT_CHANGE) { - table.second.update(type, cntx); + table.second.onUpdate(type, cntx); } else { for (auto& rule : table.second.rules) { - rule.second->update(type, cntx); + rule.second->onUpdate(type, cntx); } } } @@ -3355,6 +3658,26 @@ bool AclOrch::updateAclRule(string table_id, string rule_id, bool enableCounter) return true; } +bool AclOrch::updateAclRule(shared_ptr updatedRule) +{ + SWSS_LOG_ENTER(); + + auto tableId = updatedRule->getTableId(); + sai_object_id_t tableOid = getTableById(tableId); + if (tableOid == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_ERROR("Failed to add ACL rule in ACL table %s. Table doesn't exist", tableId.c_str()); + return false; + } + + if (!m_AclTables[tableOid].updateRule(updatedRule)) + { + return false; + } + + return true; +} + bool AclOrch::isCombinedMirrorV6Table() { return m_isCombinedMirrorV6Table; diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index a34d06846e1d..9b1df1857016 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -19,6 +19,8 @@ #include "acltable.h" +#include "saiattr.h" + #define RULE_PRIORITY "PRIORITY" #define MATCH_IN_PORTS "IN_PORTS" #define MATCH_OUT_PORTS "OUT_PORTS" @@ -93,6 +95,7 @@ #define ACL_COUNTER_FLEX_COUNTER_GROUP "ACL_STAT_COUNTER" typedef map acl_rule_attr_lookup_t; +typedef map acl_range_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; @@ -102,6 +105,13 @@ typedef map> acl_action_enum_values_capabili class AclOrch; +struct AclRangeConfig +{ + sai_acl_range_type_t rangeType; + uint32_t min; + uint32_t max; +}; + class AclRange { public: @@ -130,7 +140,7 @@ class AclRule AclRule(AclOrch *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); + virtual bool validateAddAction(string attr_name, string attr_value) = 0; 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) @@ -140,8 +150,9 @@ class AclRule } virtual bool create(); + virtual bool update(const AclRule& updatedRule); virtual bool remove(); - virtual void update(SubjectType, void *) = 0; + virtual void onUpdate(SubjectType, void *) = 0; virtual void updateInPorts(); virtual bool enableCounter(); @@ -167,16 +178,13 @@ class AclRule return m_counterOid; } + vector getInPorts() const; + bool hasCounter() const { return getCounterOid() != SAI_NULL_OBJECT_ID; } - 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&); virtual ~AclRule() {} @@ -187,6 +195,17 @@ class AclRule virtual bool removeRanges(); virtual bool removeRule(); + virtual bool updatePriority(const AclRule& updatedRule); + virtual bool updateMatches(const AclRule& updatedRule); + virtual bool updateActions(const AclRule& updatedRule); + virtual bool updateCounter(const AclRule& updatedRule); + + virtual bool setPriority(const sai_uint32_t &value); + virtual bool setAction(sai_acl_entry_attr_t actionId, sai_acl_action_data_t actionData); + virtual bool setMatch(sai_acl_entry_attr_t matchId, sai_acl_field_data_t matchData); + + virtual bool setAttribute(sai_attribute_t attr); + void decreaseNextHopRefCount(); bool isActionSupported(sai_acl_entry_attr_t) const; @@ -201,13 +220,13 @@ class AclRule sai_object_id_t m_ruleOid; sai_object_id_t m_counterOid; uint32_t m_priority; - map m_matches; - map m_actions; + map m_actions; + map m_matches; string m_redirect_target_next_hop; string m_redirect_target_next_hop_group; - vector m_inPorts; - vector m_outPorts; + vector m_rangeConfig; + vector m_ranges; private: bool m_createCounter; @@ -221,7 +240,8 @@ class AclRuleL3: public AclRule bool validateAddAction(string attr_name, string attr_value); bool validateAddMatch(string attr_name, string attr_value); bool validate(); - void update(SubjectType, void *); + void onUpdate(SubjectType, void *) override; + protected: sai_object_id_t getRedirectObjectId(const string& redirect_param); }; @@ -256,11 +276,12 @@ class AclRuleMirror: public AclRule bool validate(); bool createRule(); bool removeRule(); - void update(SubjectType, void *); + void onUpdate(SubjectType, void *) override; bool activate(); bool deactivate(); + bool update(const AclRule& updatedRule) override; protected: bool m_state {false}; string m_sessionName; @@ -275,11 +296,12 @@ class AclRuleDTelFlowWatchListEntry: public AclRule bool validate(); bool createRule(); bool removeRule(); - void update(SubjectType, void *); + void onUpdate(SubjectType, void *) override; bool activate(); bool deactivate(); + bool update(const AclRule& updatedRule) override; protected: DTelOrch *m_pDTelOrch; string m_intSessionId; @@ -293,8 +315,7 @@ class AclRuleDTelDropWatchListEntry: public AclRule AclRuleDTelDropWatchListEntry(AclOrch *m_pAclOrch, DTelOrch *m_pDTelOrch, string rule, string table, acl_table_type_t type); bool validateAddAction(string attr_name, string attr_value); bool validate(); - void update(SubjectType, void *); - + void onUpdate(SubjectType, void *) override; protected: DTelOrch *m_pDTelOrch; }; @@ -342,12 +363,14 @@ class AclTable void unlink(sai_object_id_t portOid); // Add or overwrite a rule into the ACL table bool add(shared_ptr newRule); + // Update existing ACL rule + bool updateRule(shared_ptr updatedRule); // Remove a rule from the ACL table bool remove(string rule_id); // Remove all rules from the ACL table bool clear(); // Update table subject to changes - void update(SubjectType, void *); + void onUpdate(SubjectType, void *); public: string id; @@ -403,6 +426,7 @@ class AclOrch : public Orch, public Observer bool updateAclTable(string table_id, AclTable &table); bool addAclRule(shared_ptr aclRule, string table_id); bool removeAclRule(string table_id, string rule_id); + bool updateAclRule(shared_ptr updatedAclRule); bool updateAclRule(string table_id, string rule_id, string attr_name, void *data, bool oper); bool updateAclRule(string table_id, string rule_id, bool enableCounter); AclRule* getAclRule(string table_id, string rule_id); diff --git a/orchagent/pbh/pbhrule.cpp b/orchagent/pbh/pbhrule.cpp index 52812e35b647..d4e2218cf09d 100644 --- a/orchagent/pbh/pbhrule.cpp +++ b/orchagent/pbh/pbhrule.cpp @@ -11,15 +11,7 @@ bool AclRulePbh::validateAddPriority(const sai_uint32_t &value) { SWSS_LOG_ENTER(); - if ((value < m_minPriority) || (value > m_maxPriority)) - { - SWSS_LOG_ERROR("Failed to validate priority: invalid value %d", value); - return false; - } - - m_priority = value; - - return true; + return setPriority(value); } bool AclRulePbh::validateAddMatch(const sai_attribute_t &attr) @@ -52,9 +44,7 @@ bool AclRulePbh::validateAddMatch(const sai_attribute_t &attr) return false; } - m_matches[attrId] = attr.value; - - return true; + return setMatch(attrId, attr.value.aclfield); } bool AclRulePbh::validateAddAction(const sai_attribute_t &attr) @@ -83,15 +73,7 @@ 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; + return setAction(attrId, attr.value.aclaction); } bool AclRulePbh::validate() @@ -107,7 +89,12 @@ bool AclRulePbh::validate() return true; } -void AclRulePbh::update(SubjectType, void *) +void AclRulePbh::onUpdate(SubjectType, void *) { // Do nothing } + +bool AclRulePbh::validateAddAction(string attr_name, string attr_value) +{ + SWSS_LOG_THROW("This API should not be used on PbhRule"); +} diff --git a/orchagent/pbh/pbhrule.h b/orchagent/pbh/pbhrule.h index 3e753a0f0390..9e661761c406 100644 --- a/orchagent/pbh/pbhrule.h +++ b/orchagent/pbh/pbhrule.h @@ -11,5 +11,6 @@ class AclRulePbh: public AclRule bool validateAddMatch(const sai_attribute_t &attr); bool validateAddAction(const sai_attribute_t &attr); bool validate() override; - void update(SubjectType, void *) override; + void onUpdate(SubjectType, void *) override; + bool validateAddAction(string attr_name, string attr_value) override; }; diff --git a/orchagent/saiattr.cpp b/orchagent/saiattr.cpp new file mode 100644 index 000000000000..1c24489ed5a5 --- /dev/null +++ b/orchagent/saiattr.cpp @@ -0,0 +1,91 @@ +#include "saiattr.h" + +#include +#include + +SaiAttrWrapper::SaiAttrWrapper(sai_object_type_t objectType, const sai_attribute_t& attr) +{ + auto meta = sai_metadata_get_attr_metadata(objectType, attr.id); + if (!meta) + { + SWSS_LOG_THROW("Failed to get attribute %d metadata", attr.id); + } + + init(objectType, *meta, attr); +} + +SaiAttrWrapper::~SaiAttrWrapper() +{ + if (m_meta) + { + sai_deserialize_free_attribute_value(m_meta->attrvaluetype, m_attr); + } +} + +SaiAttrWrapper::SaiAttrWrapper(const SaiAttrWrapper& other) +{ + init(other.m_objectType, *other.m_meta, other.m_attr); +} + +SaiAttrWrapper& SaiAttrWrapper::operator=(const SaiAttrWrapper& other) +{ + init(other.m_objectType, *other.m_meta, other.m_attr); + return *this; +} + +SaiAttrWrapper::SaiAttrWrapper(SaiAttrWrapper&& other) +{ + swap(std::move(other)); +} + +SaiAttrWrapper& SaiAttrWrapper::operator=(SaiAttrWrapper&& other) +{ + swap(std::move(other)); + return *this; +} + +bool SaiAttrWrapper::operator<(const SaiAttrWrapper& other) const +{ + return m_serializedAttr < other.m_serializedAttr; +} + +const sai_attribute_t& SaiAttrWrapper::getSaiAttr() const +{ + return m_attr; +} + +std::string SaiAttrWrapper::toString() const +{ + return m_serializedAttr; +} + +sai_attr_id_t SaiAttrWrapper::getAttrId() const +{ + return m_attr.id; +} + +void SaiAttrWrapper::swap(SaiAttrWrapper&& other) +{ + m_objectType = other.m_objectType; + m_meta = other.m_meta; + m_attr = other.m_attr; + m_serializedAttr = other.m_serializedAttr; + other.m_attr = sai_attribute_t{}; + other.m_serializedAttr.clear(); +} + +void SaiAttrWrapper::init( + sai_object_type_t objectType, + const sai_attr_metadata_t& meta, + const sai_attribute_t& attr) +{ + m_objectType = objectType; + m_attr.id = attr.id; + m_meta = &meta; + + m_serializedAttr = sai_serialize_attr_value(*m_meta, attr); + + // deserialize to actually preform a deep copy of attr + // and attribute value's dynamically allocated lists. + sai_deserialize_attr_value(m_serializedAttr, *m_meta, m_attr); +} diff --git a/orchagent/saiattr.h b/orchagent/saiattr.h new file mode 100644 index 000000000000..c4fef0ba0ddb --- /dev/null +++ b/orchagent/saiattr.h @@ -0,0 +1,41 @@ +#pragma once + +extern "C" +{ +#include +#include +} + +#include + +class SaiAttrWrapper +{ +public: + SaiAttrWrapper() = default; + + SaiAttrWrapper(sai_object_type_t objectType, const sai_attribute_t& attr); + SaiAttrWrapper(const SaiAttrWrapper& other); + SaiAttrWrapper(SaiAttrWrapper&& other); + SaiAttrWrapper& operator=(const SaiAttrWrapper& other); + SaiAttrWrapper& operator=(SaiAttrWrapper&& other); + virtual ~SaiAttrWrapper(); + + bool operator<(const SaiAttrWrapper& other) const; + + const sai_attribute_t& getSaiAttr() const; + std::string toString() const; + sai_attr_id_t getAttrId() const; + +private: + + void init( + sai_object_type_t objectType, + const sai_attr_metadata_t& meta, + const sai_attribute_t& attr); + void swap(SaiAttrWrapper&& other); + + sai_object_type_t m_objectType {SAI_OBJECT_TYPE_NULL}; + const sai_attr_metadata_t* m_meta {nullptr}; + sai_attribute_t m_attr {}; + std::string m_serializedAttr; +}; diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 51bcf30547e8..7f31379658f5 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -59,6 +59,7 @@ tests_SOURCES = aclorch_ut.cpp \ $(top_srcdir)/orchagent/pbh/pbhrule.cpp \ $(top_srcdir)/orchagent/pbhorch.cpp \ $(top_srcdir)/orchagent/saihelper.cpp \ + $(top_srcdir)/orchagent/saiattr.cpp \ $(top_srcdir)/orchagent/switchorch.cpp \ $(top_srcdir)/orchagent/pfcwdorch.cpp \ $(top_srcdir)/orchagent/pfcactionhandler.cpp \ diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index d95ae2f87e06..4ffee617ad2f 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -827,21 +827,21 @@ namespace aclorch_test return false; } - if (it->second.aclaction.enable != true) + if (it->second.getSaiAttr().value.aclaction.enable != true) { return false; } if (attr_value == PACKET_ACTION_FORWARD) { - if (it->second.aclaction.parameter.s32 != SAI_PACKET_ACTION_FORWARD) + if (it->second.getSaiAttr().value.aclaction.parameter.s32 != SAI_PACKET_ACTION_FORWARD) { return false; } } else if (attr_value == PACKET_ACTION_DROP) { - if (it->second.aclaction.parameter.s32 != SAI_PACKET_ACTION_DROP) + if (it->second.getSaiAttr().value.aclaction.parameter.s32 != SAI_PACKET_ACTION_DROP) { return false; } @@ -874,14 +874,14 @@ namespace aclorch_test } char addr[20]; - sai_serialize_ip4(addr, it_field->second.aclfield.data.ip4); + sai_serialize_ip4(addr, it_field->second.getSaiAttr().value.aclfield.data.ip4); if (attr_value != addr) { return false; } char mask[20]; - sai_serialize_ip4(mask, it_field->second.aclfield.mask.ip4); + sai_serialize_ip4(mask, it_field->second.getSaiAttr().value.aclfield.mask.ip4); if (string(mask) != "255.255.255.255") { return false; @@ -896,14 +896,14 @@ namespace aclorch_test } char addr[46]; - sai_serialize_ip6(addr, it_field->second.aclfield.data.ip6); + sai_serialize_ip6(addr, it_field->second.getSaiAttr().value.aclfield.data.ip6); if (attr_value != addr) { return false; } char mask[46]; - sai_serialize_ip6(mask, it_field->second.aclfield.mask.ip6); + sai_serialize_ip6(mask, it_field->second.getSaiAttr().value.aclfield.mask.ip6); if (string(mask) != "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff") { return false; @@ -976,6 +976,25 @@ namespace aclorch_test return !aclEnable && aclOid == SAI_NULL_OBJECT_ID; } + + string getAclRuleSaiAttribute(const AclRule& rule, sai_acl_entry_attr_t attrId) + { + sai_attribute_t attr{}; + attr.id = attrId; + auto meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_ENTRY, attrId); + if (!meta) + { + SWSS_LOG_THROW("SAI BUG: Failed to get attribute metadata for SAI_OBJECT_TYPE_ACL_ENTRY attribute id %d", attrId); + } + + auto status = sai_acl_api->get_acl_entry_attribute(rule.m_ruleOid, 1, &attr); + EXPECT_TRUE(status == SAI_STATUS_SUCCESS); + + auto actualSaiValue = sai_serialize_attr_value(*meta, attr); + + return actualSaiValue; + } + }; map AclOrchTest::gProfileMap; @@ -1317,4 +1336,88 @@ namespace aclorch_test ASSERT_EQ(tableIt, orch->getAclTables().end()); } + TEST_F(AclOrchTest, AclRuleUpdate) + { + string acl_table_id = "acl_table_1"; + string acl_rule_id = "acl_rule_1"; + + auto orch = createAclOrch(); + + auto kvfAclTable = deque( + { { acl_table_id, + SET_COMMAND, + { { ACL_TABLE_DESCRIPTION, "TEST" }, + { ACL_TABLE_TYPE, TABLE_TYPE_L3 }, + { ACL_TABLE_STAGE, STAGE_INGRESS }, + { ACL_TABLE_PORTS, "1,2" } } } }); + + orch->doAclTableTask(kvfAclTable); + + // validate acl table ... + + auto acl_table_oid = orch->getTableById(acl_table_id); + ASSERT_NE(acl_table_oid, SAI_NULL_OBJECT_ID); + + const auto &acl_tables = orch->getAclTables(); + auto it_table = acl_tables.find(acl_table_oid); + ASSERT_NE(it_table, acl_tables.end()); + + class AclRuleTest : public AclRuleL3 + { + public: + AclRuleTest(AclOrch* orch, string rule, string table): + AclRuleL3(orch, rule, table, ACL_TABLE_L3, true) + {} + + void setCounterEnabled(bool enabled) + { + m_createCounter = enabled; + } + + void disableMatch(sai_acl_entry_attr_t attr) + { + m_matches.erase(attr); + } + }; + + auto rule = make_shared(orch->m_aclOrch, acl_rule_id, acl_table_id); + ASSERT_TRUE(rule->validateAddPriority(RULE_PRIORITY, "800")); + ASSERT_TRUE(rule->validateAddMatch(MATCH_SRC_IP, "1.1.1.1/32")); + ASSERT_TRUE(rule->validateAddAction(ACTION_PACKET_ACTION, PACKET_ACTION_FORWARD)); + + ASSERT_TRUE(orch->m_aclOrch->addAclRule(rule, acl_table_id)); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_PRIORITY), "800"); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP), "1.1.1.1&mask:255.255.255.255"); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION), "SAI_PACKET_ACTION_FORWARD"); + + auto updatedRule = make_shared(*rule); + ASSERT_TRUE(updatedRule->validateAddPriority(RULE_PRIORITY, "900")); + ASSERT_TRUE(updatedRule->validateAddMatch(MATCH_SRC_IP, "2.2.2.2/24")); + ASSERT_TRUE(updatedRule->validateAddMatch(MATCH_DST_IP, "3.3.3.3/24")); + ASSERT_TRUE(updatedRule->validateAddAction(ACTION_PACKET_ACTION, PACKET_ACTION_DROP)); + + ASSERT_TRUE(orch->m_aclOrch->updateAclRule(updatedRule)); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_PRIORITY), "900"); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP), "2.2.2.2&mask:255.255.255.0"); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_FIELD_DST_IP), "3.3.3.3&mask:255.255.255.0"); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION), "SAI_PACKET_ACTION_DROP"); + + auto updatedRule2 = make_shared(*updatedRule); + updatedRule2->setCounterEnabled(false); + updatedRule2->disableMatch(SAI_ACL_ENTRY_ATTR_FIELD_DST_IP); + ASSERT_TRUE(orch->m_aclOrch->updateAclRule(updatedRule2)); + ASSERT_TRUE(validateAclRuleCounter(*orch->m_aclOrch->getAclRule(acl_table_id, acl_rule_id), false)); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_PRIORITY), "900"); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP), "2.2.2.2&mask:255.255.255.0"); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_FIELD_DST_IP), "disabled"); + ASSERT_EQ(getAclRuleSaiAttribute(*rule, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION), "SAI_PACKET_ACTION_DROP"); + + auto updatedRule3 = make_shared(*updatedRule2); + updatedRule3->setCounterEnabled(true); + ASSERT_TRUE(orch->m_aclOrch->updateAclRule(updatedRule3)); + ASSERT_TRUE(validateAclRuleCounter(*orch->m_aclOrch->getAclRule(acl_table_id, acl_rule_id), true)); + + ASSERT_TRUE(orch->m_aclOrch->removeAclRule(rule->getTableId(), rule->getId())); + } + } // namespace nsAclOrchTest diff --git a/tests/mock_tests/portal.h b/tests/mock_tests/portal.h index 7dab40036ba7..c2438e8e1cfe 100644 --- a/tests/mock_tests/portal.h +++ b/tests/mock_tests/portal.h @@ -18,12 +18,12 @@ struct Portal return aclRule->m_ruleOid; } - static const map &getMatches(const AclRule *aclRule) + static const map &getMatches(const AclRule *aclRule) { return aclRule->m_matches; } - static const map &getActions(const AclRule *aclRule) + static const map &getActions(const AclRule *aclRule) { return aclRule->m_actions; }