diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index be9f0a10e9..5869d54902 100755 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -14,6 +14,7 @@ #include "sai_serialize.h" #include "directory.h" #include "saihelper.h" +#include "policerorch.h" using namespace std; using namespace swss; @@ -32,6 +33,7 @@ extern sai_object_id_t gSwitchId; extern PortsOrch* gPortsOrch; extern CrmOrch *gCrmOrch; extern SwitchOrch *gSwitchOrch; +extern PolicerOrch *gPolicerOrch; extern string gMySwitchType; extern Directory gDirectory; @@ -141,6 +143,11 @@ static acl_rule_attr_lookup_t aclOtherActionLookup = { ACTION_COUNTER, SAI_ACL_ENTRY_ATTR_ACTION_COUNTER} }; +static acl_rule_attr_lookup_t aclPolicerActionLookup = +{ + { ACTION_POLICER_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_SET_POLICER} +}; + static acl_packet_action_lookup_t aclPacketActionLookup = { { PACKET_ACTION_FORWARD, SAI_PACKET_ACTION_FORWARD }, @@ -842,6 +849,7 @@ bool AclTableTypeParser::parseAclTableTypeActions(const std::string& value, AclT auto otherAction = aclOtherActionLookup.find(action); auto metadataAction = aclMetadataDscpActionLookup.find(action); auto innerAction = aclInnerActionLookup.find(action); + auto policerAction = aclPolicerActionLookup.find(action); if (l3Action != aclL3ActionLookup.end()) { saiActionAttr = l3Action->second; @@ -866,6 +874,10 @@ bool AclTableTypeParser::parseAclTableTypeActions(const std::string& value, AclT { saiActionAttr = metadataAction->second; } + else if (policerAction != aclPolicerActionLookup.end()) + { + saiActionAttr = policerAction->second; + } else { SWSS_LOG_ERROR("Unknown action %s", action.c_str()); @@ -1813,6 +1825,10 @@ shared_ptr AclRule::makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOr { return make_shared(acl, rule, table); } + else if (aclPolicerActionLookup.find(action) != aclPolicerActionLookup.cend()) + { + return make_shared(acl, rule, table); + } else if (acl->isUsingEgrSetDscp(table) || table == EGR_SET_DSCP_TABLE_ID) { return make_shared(acl, rule, table, m_metadataMgr); @@ -2247,6 +2263,104 @@ AclRuleInnerSrcMacRewrite::AclRuleInnerSrcMacRewrite(AclOrch *aclOrch, string ru //do nothing } +AclRulePolicer::AclRulePolicer(AclOrch *aclOrch, string rule, string table, bool createCounter) : + AclRule(aclOrch, rule, table, createCounter) +{ +} + +bool AclRulePolicer::validateAddAction(string attr_name, string attr_value) +{ + SWSS_LOG_ENTER(); + + if (attr_name != ACTION_POLICER_ACTION) + { + return false; + } + + if (attr_value.empty()) + { + SWSS_LOG_ERROR("Empty policer name for action %s in rule %s", attr_name.c_str(), m_id.c_str()); + return false; + } + + sai_object_id_t policer_oid = SAI_NULL_OBJECT_ID; + if (!gPolicerOrch->getPolicerOid(attr_value, policer_oid)) + { + SWSS_LOG_ERROR("Failed to add policer action to rule %s: policer %s does not exist", + m_id.c_str(), attr_value.c_str()); + return false; + } + + sai_acl_action_data_t actionData = {}; + actionData.enable = true; + actionData.parameter.oid = policer_oid; + + m_policerName = attr_value; + + return setAction(aclPolicerActionLookup[attr_name], actionData); +} + +bool AclRulePolicer::validate() +{ + SWSS_LOG_ENTER(); + + if ((m_rangeConfig.empty() && m_matches.empty()) || m_actions.size() != 1) + { + return false; + } + + return true; +} + +void AclRulePolicer::onUpdate(SubjectType, void *) +{ + // Do nothing +} + +bool AclRulePolicer::createRule() +{ + SWSS_LOG_ENTER(); + + if (!AclRule::createRule()) + { + return false; + } + + // Hold a reference on the policer so it cannot be removed while this rule + // binds it. Bump only once per created rule to keep the count balanced + // across recreate cycles. + if (!m_policerRefHeld) + { + if (!gPolicerOrch->increaseRefCount(m_policerName)) + { + SWSS_LOG_ERROR("Failed to increase reference count for policer %s bound by rule %s", + m_policerName.c_str(), m_id.c_str()); + return false; + } + m_policerRefHeld = true; + } + + return true; +} + +bool AclRulePolicer::removeRule() +{ + SWSS_LOG_ENTER(); + + if (!AclRule::removeRule()) + { + return false; + } + + if (m_policerRefHeld) + { + gPolicerOrch->decreaseRefCount(m_policerName); + m_policerRefHeld = false; + } + + return true; +} + AclRuleMirror::AclRuleMirror(AclOrch *aclOrch, MirrorOrch *mirror, string rule, string table) : AclRule(aclOrch, rule, table), m_state(false), @@ -5573,6 +5687,29 @@ void AclOrch::doAclRuleTask(Consumer &consumer) type = table_id == m_mirrorTableId[stage] ? TABLE_TYPE_MIRROR : TABLE_TYPE_MIRRORV6; } + /* If the rule references a policer that has not been created yet, + * defer it and rely on the Consumer m_toSync retry (same pattern as + * "Wait for ACL table" above). This covers the case where the + * POLICER table is synced after the ACL_RULE table. */ + bool waitForPolicer = false; + for (const auto& itr : kfvFieldsValues(t)) + { + if (to_upper(fvField(itr)) == ACTION_POLICER_ACTION) + { + if (!fvValue(itr).empty() && !gPolicerOrch->policerExists(fvValue(itr))) + { + SWSS_LOG_INFO("Wait for policer %s to be created for ACL rule %s", + fvValue(itr).c_str(), key.c_str()); + waitForPolicer = true; + } + break; + } + } + if (waitForPolicer) + { + it++; + continue; + } try { diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 08aa0b7989..9ddb74762e 100755 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -80,6 +80,7 @@ #define ACTION_META_DATA "META_DATA_ACTION" #define ACTION_DSCP "DSCP_ACTION" #define ACTION_INNER_SRC_MAC_REWRITE_ACTION "INNER_SRC_MAC_REWRITE_ACTION" +#define ACTION_POLICER_ACTION "POLICER_ACTION" #define PACKET_ACTION_FORWARD "FORWARD" #define PACKET_ACTION_DROP "DROP" @@ -419,6 +420,23 @@ class AclRuleInnerSrcMacRewrite: public AclRule void onUpdate(SubjectType, void *) override; }; +class AclRulePolicer: public AclRule +{ +public: + AclRulePolicer(AclOrch *m_pAclOrch, string rule, string table, bool createCounter = true); + + bool validateAddAction(string attr_name, string attr_value); + bool validate(); + void onUpdate(SubjectType, void *) override; + +protected: + bool createRule() override; + bool removeRule() override; + + string m_policerName; + bool m_policerRefHeld {false}; +}; + class AclRuleMirror: public AclRule { public: diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 46df998f4e..f6ef4ee6d0 100755 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -15,10 +15,12 @@ extern Srv6Orch *gSrv6Orch; extern FdbOrch *gFdbOrch; extern MirrorOrch *gMirrorOrch; +extern PolicerOrch *gPolicerOrch; extern VRFOrch *gVrfOrch; extern sai_acl_api_t *sai_acl_api; extern sai_switch_api_t *sai_switch_api; +extern sai_policer_api_t *sai_policer_api; extern sai_hash_api_t *sai_hash_api; extern sai_port_api_t *sai_port_api; extern sai_vlan_api_t *sai_vlan_api; @@ -325,6 +327,7 @@ namespace aclorch_test sai_api_query(SAI_API_MPLS, (void **)&sai_mpls_api); sai_api_query(SAI_API_ACL, (void **)&sai_acl_api); sai_api_query(SAI_API_NEXT_HOP_GROUP, (void **)&sai_next_hop_group_api); + sai_api_query(SAI_API_POLICER, (void **)&sai_policer_api); sai_attribute_t attr; @@ -447,14 +450,15 @@ namespace aclorch_test TableConnector(m_config_db.get(), CFG_PORT_STORM_CONTROL_TABLE_NAME) }; TableConnector stateDbStorm(m_state_db.get(), "BUM_STORM_CAPABILITY"); - PolicerOrch *policer_orch = new PolicerOrch(policer_tables, gPortsOrch); + ASSERT_EQ(gPolicerOrch, nullptr); + gPolicerOrch = new PolicerOrch(policer_tables, gPortsOrch); TableConnector stateDbMirrorSession(m_state_db.get(), STATE_MIRROR_SESSION_TABLE_NAME); TableConnector confDbMirrorSession(m_config_db.get(), CFG_MIRROR_SESSION_TABLE_NAME); ASSERT_EQ(gMirrorOrch, nullptr); gMirrorOrch = new MirrorOrch(stateDbMirrorSession, confDbMirrorSession, - gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch, policer_orch, gSwitchOrch); + gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch, gPolicerOrch, gSwitchOrch); auto consumer = unique_ptr(new Consumer( new swss::ConsumerStateTable(m_app_db.get(), APP_PORT_TABLE_NAME, 1, 1), gPortsOrch, APP_PORT_TABLE_NAME)); @@ -471,6 +475,8 @@ namespace aclorch_test gSwitchOrch = nullptr; delete gMirrorOrch; gMirrorOrch = nullptr; + delete gPolicerOrch; + gPolicerOrch = nullptr; delete gRouteOrch; gRouteOrch = nullptr; delete gFlowCounterRouteOrch; @@ -1330,6 +1336,91 @@ namespace aclorch_test } } + // An ACL rule with POLICER_ACTION referencing a POLICER entry is realized as + // SAI_ACL_ENTRY_ATTR_ACTION_SET_POLICER. A rule referencing a policer that + // does not exist yet is deferred (relies on the m_toSync retry) and not created. + TEST_F(AclOrchTest, AclRule_Policer_Action) + { + const string aclTableTypeName = "TEST_POLICER_TYPE"; + const string aclTableName = "TEST_POLICER_TABLE"; + const string aclRuleName = "TEST_POLICER_RULE"; + const string policerName = "test_policer"; + + auto orch = createAclOrch(); + + // Create a policer so POLICER_ACTION can resolve its OID. + auto policerConsumer = unique_ptr(new Consumer( + new swss::ConsumerStateTable(m_config_db.get(), CFG_POLICER_TABLE_NAME, 1, 1), + gPolicerOrch, CFG_POLICER_TABLE_NAME)); + policerConsumer->addToSync(deque( + { { policerName, SET_COMMAND, + { { "METER_TYPE", "packets" }, { "MODE", "sr_tcm" }, + { "CIR", "100" }, { "CBS", "100" }, { "RED_PACKET_ACTION", "drop" } } } })); + static_cast(gPolicerOrch)->doTask(*policerConsumer); + ASSERT_TRUE(gPolicerOrch->policerExists(policerName)); + + // Custom table type advertising the SRC_IP match and the policer action. + orch->doAclTableTypeTask( + deque( + { { aclTableTypeName, SET_COMMAND, + { { ACL_TABLE_TYPE_MATCHES, MATCH_SRC_IP }, + { ACL_TABLE_TYPE_ACTIONS, ACTION_POLICER_ACTION }, + { ACL_TABLE_TYPE_BPOINT_TYPES, BIND_POINT_TYPE_PORT } } } })); + + orch->doAclTableTask( + deque( + { { aclTableName, SET_COMMAND, + { { ACL_TABLE_DESCRIPTION, "policer table" }, + { ACL_TABLE_TYPE, aclTableTypeName }, + { ACL_TABLE_STAGE, STAGE_INGRESS }, + { ACL_TABLE_PORTS, "1,2" } } } })); + ASSERT_TRUE(orch->getAclTable(aclTableName)); + + // Rule referencing the existing policer is created and carries SET_POLICER. + orch->doAclRuleTask( + deque( + { { aclTableName + "|" + aclRuleName, SET_COMMAND, + { { MATCH_SRC_IP, "1.1.1.1/32" }, + { ACTION_POLICER_ACTION, policerName } } } })); + + auto rule = orch->getAclRule(aclTableName, aclRuleName); + ASSERT_TRUE(rule); + const auto &actions = Portal::AclRuleInternal::getActions(rule); + ASSERT_NE(actions.find(SAI_ACL_ENTRY_ATTR_ACTION_SET_POLICER), actions.end()); + + // Deleting the rule releases the policer reference it held + // (exercises AclRulePolicer::removeRule()). + orch->doAclRuleTask( + deque( + { { aclTableName + "|" + aclRuleName, DEL_COMMAND, {} } })); + ASSERT_FALSE(orch->getAclRule(aclTableName, aclRuleName)); + + // Rule referencing a non-existent policer is deferred, not created. + orch->doAclRuleTask( + deque( + { { aclTableName + "|" + "MISSING_POLICER_RULE", SET_COMMAND, + { { MATCH_SRC_IP, "2.2.2.2/32" }, + { ACTION_POLICER_ACTION, "no_such_policer" } } } })); + ASSERT_FALSE(orch->getAclRule(aclTableName, "MISSING_POLICER_RULE")); + + // A POLICER_ACTION with an empty policer name is rejected by + // AclRulePolicer::validateAddAction() and the rule is not created. + orch->doAclRuleTask( + deque( + { { aclTableName + "|" + "EMPTY_POLICER_RULE", SET_COMMAND, + { { MATCH_SRC_IP, "3.3.3.3/32" }, + { ACTION_POLICER_ACTION, "" } } } })); + ASSERT_FALSE(orch->getAclRule(aclTableName, "EMPTY_POLICER_RULE")); + + // A POLICER_ACTION rule with no match/range fails AclRulePolicer:: + // validate() (needs at least one match and exactly one action). + orch->doAclRuleTask( + deque( + { { aclTableName + "|" + "NOMATCH_POLICER_RULE", SET_COMMAND, + { { ACTION_POLICER_ACTION, policerName } } } })); + ASSERT_FALSE(orch->getAclRule(aclTableName, "NOMATCH_POLICER_RULE")); + } + // When received ACL table/rule SET_COMMAND, orchagent can create corresponding ACL table/rule // When received ACL table/rule DEL_COMMAND, orchagent can delete corresponding ACL table/rule //