diff --git a/doc/swss-schema.md b/doc/swss-schema.md index 451f63d9401..6522bfa2c9b 100644 --- a/doc/swss-schema.md +++ b/doc/swss-schema.md @@ -650,6 +650,11 @@ Stores rules associated with a specific ACL table on the switch. 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 + packet_color_action = "green"/"yellow"/"red" + ; sets the packet color on matching packets; maps to + ; SAI_ACL_ENTRY_ATTR_ACTION_SET_PACKET_COLOR (case-insensitive). + ; Supported only where the platform SAI advertises the action for the + ; table's ACL stage (ingress and/or egress) ether_type = h16 ; Ethernet type field diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 51c7b3ac4eb..1745f38a20d 100755 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -112,7 +112,8 @@ static acl_rule_attr_lookup_t aclL3ActionLookup = { ACTION_PACKET_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION }, { ACTION_REDIRECT_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT }, { ACTION_DO_NOT_NAT_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_NO_NAT }, - { ACTION_DISABLE_TRIM, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_TRIM_DISABLE } + { ACTION_DISABLE_TRIM, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_TRIM_DISABLE }, + { ACTION_PACKET_COLOR_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_SET_PACKET_COLOR } }; static acl_rule_attr_lookup_t aclInnerActionLookup = @@ -148,6 +149,13 @@ static acl_packet_action_lookup_t aclPacketActionLookup = { PACKET_ACTION_COPY, SAI_PACKET_ACTION_COPY }, }; +static acl_packet_color_lookup_t aclPacketColorLookup = +{ + { PACKET_COLOR_GREEN, SAI_PACKET_COLOR_GREEN }, + { PACKET_COLOR_YELLOW, SAI_PACKET_COLOR_YELLOW }, + { PACKET_COLOR_RED, SAI_PACKET_COLOR_RED }, +}; + static acl_rule_attr_lookup_t aclMetadataDscpActionLookup = { { ACTION_META_DATA, SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA}, @@ -2217,6 +2225,17 @@ bool AclRulePacket::validateAddAction(string attr_name, string _attr_value) } actionData.parameter.oid = param_id; } + else if (attr_name == ACTION_PACKET_COLOR_ACTION) + { + const auto it = aclPacketColorLookup.find(attr_value); + if (it == aclPacketColorLookup.cend()) + { + SWSS_LOG_ERROR("Invalid packet color %s for action %s", + attr_value.c_str(), attr_name.c_str()); + return false; + } + actionData.parameter.s32 = it->second; + } else { return false; @@ -2322,7 +2341,32 @@ bool AclRulePacket::validate() { SWSS_LOG_ENTER(); - if ((m_rangeConfig.empty() && m_matches.empty()) || m_actions.size() != 1) + if (m_rangeConfig.empty() && m_matches.empty()) + { + return false; + } + + // A packet rule must carry at least one action. + if (m_actions.empty()) + { + return false; + } + + // SET_PACKET_COLOR is a composable QoS marking: it sets the drop-precedence + // color of a matched packet (consumed downstream by a color-aware policer or + // WRED) and may accompany a single forwarding/terminating action such as + // PACKET_ACTION, plus a counter. Only the non-color actions are capped at + // one; SAI applies SET_PACKET_COLOR as an independent CREATE_AND_SET action. + size_t nonColorActions = 0; + for (const auto &action : m_actions) + { + if (action.first != SAI_ACL_ENTRY_ATTR_ACTION_SET_PACKET_COLOR) + { + nonColorActions++; + } + } + + if (nonColorActions > 1) { return false; } @@ -4202,6 +4246,9 @@ void AclOrch::queryAclActionCapability() queryAclActionAttrEnumValues(ACTION_PACKET_ACTION, aclL3ActionLookup, aclPacketActionLookup); + queryAclActionAttrEnumValues(ACTION_PACKET_COLOR_ACTION, + aclL3ActionLookup, + aclPacketColorLookup); queryAclActionAttrEnumValues(ACTION_DTEL_FLOW_OP, aclDTelActionLookup, aclDTelFlowOpTypeLookup); diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 29dd7418e6f..458138de3bb 100755 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -84,6 +84,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_PACKET_COLOR_ACTION "PACKET_COLOR_ACTION" #define PACKET_ACTION_FORWARD "FORWARD" #define PACKET_ACTION_DROP "DROP" @@ -92,6 +93,10 @@ #define PACKET_ACTION_DO_NOT_NAT "DO_NOT_NAT" #define PACKET_ACTION_DISABLE_TRIM "DISABLE_TRIM" +#define PACKET_COLOR_GREEN "GREEN" +#define PACKET_COLOR_YELLOW "YELLOW" +#define PACKET_COLOR_RED "RED" + #define DTEL_FLOW_OP_NOP "NOP" #define DTEL_FLOW_OP_POSTCARD "POSTCARD" #define DTEL_FLOW_OP_INT "INT" @@ -154,6 +159,7 @@ typedef map acl_bind_point_type_lookup_t; typedef map acl_ip_type_lookup_t; typedef map acl_dtel_flow_op_type_lookup_t; typedef map acl_packet_action_lookup_t; +typedef map acl_packet_color_lookup_t; typedef tuple acl_range_properties_t; typedef map acl_capabilities_t; typedef map> acl_action_enum_values_capabilities_t; diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 3374cbccfda..cdfae5d0063 100755 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -2061,6 +2061,300 @@ namespace aclorch_test ASSERT_TRUE(validateAclRuleByConfOp(*it_rule->second, kfvFieldsValues(kvfAclRule.front()))); } + sai_switch_api_t *old_sai_switch_api_packet_color; + + // Override SAI get_switch_attribute so that the ASIC reports SET_PACKET_COLOR + // (and PACKET_ACTION) as supported ACL actions for both stages. + sai_status_t getSwitchAttributePacketColor(_In_ sai_object_id_t switch_id, _In_ uint32_t attr_count, + _Inout_ sai_attribute_t *attr_list) + { + if (attr_count == 1) + { + switch (attr_list[0].id) + { + case SAI_SWITCH_ATTR_MAX_ACL_ACTION_COUNT: + attr_list[0].value.u32 = 2; + return SAI_STATUS_SUCCESS; + case SAI_SWITCH_ATTR_ACL_STAGE_INGRESS: + case SAI_SWITCH_ATTR_ACL_STAGE_EGRESS: + attr_list[0].value.aclcapability.action_list.count = 2; + attr_list[0].value.aclcapability.action_list.list[0] = SAI_ACL_ACTION_TYPE_PACKET_ACTION; + attr_list[0].value.aclcapability.action_list.list[1] = SAI_ACL_ACTION_TYPE_SET_PACKET_COLOR; + attr_list[0].value.aclcapability.is_action_list_mandatory = false; + return SAI_STATUS_SUCCESS; + } + } + return old_sai_switch_api_packet_color->get_switch_attribute(switch_id, attr_count, attr_list); + } + + // Verify that the generic (CONFIG_DB) ACL path programs SAI_ACL_ENTRY_ATTR_ACTION_SET_PACKET_COLOR + // with the right sai_packet_color_t value for the PACKET_COLOR_ACTION rule action. + TEST_F(AclOrchTest, AclRuleSetPacketColorAction) + { + // Report SET_PACKET_COLOR as a supported ACL action. + old_sai_switch_api_packet_color = sai_switch_api; + sai_switch_api_t new_sai_switch_api = *sai_switch_api; + sai_switch_api = &new_sai_switch_api; + sai_switch_api->get_switch_attribute = getSwitchAttributePacketColor; + + // Restore the global SAI switch API on every exit path (including an + // early return from a failed ASSERT_* below) so TearDown() never + // dereferences the stack-allocated override above. + struct SaiSwitchApiRestore { + ~SaiSwitchApiRestore() { sai_switch_api = old_sai_switch_api_packet_color; } + } saiSwitchApiRestore; + + auto orch = createAclOrch(); + + const string aclTableTypeName = "PACKET_COLOR_TABLE_TYPE"; + const string aclTableName = "PACKET_COLOR_TABLE"; + + orch->doAclTableTypeTask( + deque( + { + { + aclTableTypeName, + SET_COMMAND, + { + { ACL_TABLE_TYPE_MATCHES, MATCH_SRC_IP }, + { ACL_TABLE_TYPE_ACTIONS, ACTION_PACKET_COLOR_ACTION } + } + } + }) + ); + + orch->doAclTableTask( + deque( + { + { + aclTableName, + SET_COMMAND, + { + { ACL_TABLE_TYPE, aclTableTypeName }, + { ACL_TABLE_STAGE, STAGE_INGRESS } + } + } + }) + ); + ASSERT_TRUE(orch->getAclTable(aclTableName)); + + const vector> colors = { + { PACKET_COLOR_GREEN, SAI_PACKET_COLOR_GREEN }, + { PACKET_COLOR_YELLOW, SAI_PACKET_COLOR_YELLOW }, + { PACKET_COLOR_RED, SAI_PACKET_COLOR_RED } + }; + + for (const auto &color : colors) + { + const string aclRuleName = "PACKET_COLOR_RULE_" + color.first; + orch->doAclRuleTask( + deque( + { + { + aclTableName + "|" + aclRuleName, + SET_COMMAND, + { + { RULE_PRIORITY, "999" }, + { MATCH_SRC_IP, "1.2.3.4/32" }, + { ACTION_PACKET_COLOR_ACTION, color.first } + } + } + }) + ); + + auto rule = orch->getAclRule(aclTableName, aclRuleName); + ASSERT_NE(rule, nullptr); + + const auto &actions = Portal::AclRuleInternal::getActions(rule); + auto it = actions.find(SAI_ACL_ENTRY_ATTR_ACTION_SET_PACKET_COLOR); + ASSERT_NE(it, actions.end()); + ASSERT_TRUE(it->second.getSaiAttr().value.aclaction.enable); + ASSERT_EQ(it->second.getSaiAttr().value.aclaction.parameter.s32, color.second); + } + + // An invalid packet color must be rejected (rule not created). + orch->doAclRuleTask( + deque( + { + { + aclTableName + "|PACKET_COLOR_RULE_INVALID", + SET_COMMAND, + { + { RULE_PRIORITY, "998" }, + { MATCH_SRC_IP, "1.2.3.5/32" }, + { ACTION_PACKET_COLOR_ACTION, "PURPLE" } + } + } + }) + ); + ASSERT_EQ(orch->getAclRule(aclTableName, "PACKET_COLOR_RULE_INVALID"), nullptr); + } + + // A single rule that combines PACKET_ACTION (FORWARD) with PACKET_COLOR_ACTION + // must be created -- validate() must not treat SET_PACKET_COLOR as an exclusive + // action -- and BOTH SAI actions must be programmed on the entry. This is the + // "classify, pre-color, then forward" pattern whose color feeds a downstream + // color-aware policer/WRED. + TEST_F(AclOrchTest, AclRuleSetPacketColorActionComposeForward) + { + // Report SET_PACKET_COLOR and PACKET_ACTION as supported ACL actions. + old_sai_switch_api_packet_color = sai_switch_api; + sai_switch_api_t new_sai_switch_api = *sai_switch_api; + sai_switch_api = &new_sai_switch_api; + sai_switch_api->get_switch_attribute = getSwitchAttributePacketColor; + + struct SaiSwitchApiRestore { + ~SaiSwitchApiRestore() { sai_switch_api = old_sai_switch_api_packet_color; } + } saiSwitchApiRestore; + + auto orch = createAclOrch(); + + const string aclTableTypeName = "PACKET_COLOR_FWD_TABLE_TYPE"; + const string aclTableName = "PACKET_COLOR_FWD_TABLE"; + const string aclRuleName = "PACKET_COLOR_FWD_RULE"; + const string comma = ","; + + orch->doAclTableTypeTask( + deque( + { + { + aclTableTypeName, + SET_COMMAND, + { + { ACL_TABLE_TYPE_MATCHES, MATCH_SRC_IP }, + { ACL_TABLE_TYPE_ACTIONS, string(ACTION_PACKET_ACTION) + comma + ACTION_PACKET_COLOR_ACTION } + } + } + }) + ); + + orch->doAclTableTask( + deque( + { + { + aclTableName, + SET_COMMAND, + { + { ACL_TABLE_TYPE, aclTableTypeName }, + { ACL_TABLE_STAGE, STAGE_INGRESS } + } + } + }) + ); + ASSERT_TRUE(orch->getAclTable(aclTableName)); + + orch->doAclRuleTask( + deque( + { + { + aclTableName + "|" + aclRuleName, + SET_COMMAND, + { + { RULE_PRIORITY, "999" }, + { MATCH_SRC_IP, "1.2.3.4/32" }, + { ACTION_PACKET_ACTION, PACKET_ACTION_FORWARD }, + { ACTION_PACKET_COLOR_ACTION, PACKET_COLOR_GREEN } + } + } + }) + ); + + // The composed rule must be created (validate() must not treat SET_PACKET_COLOR + // as exclusive). + auto rule = orch->getAclRule(aclTableName, aclRuleName); + ASSERT_NE(rule, nullptr); + + // Both actions are programmed: SET_PACKET_COLOR with the configured color ... + const auto &actions = Portal::AclRuleInternal::getActions(rule); + auto colorIt = actions.find(SAI_ACL_ENTRY_ATTR_ACTION_SET_PACKET_COLOR); + ASSERT_NE(colorIt, actions.end()); + ASSERT_TRUE(colorIt->second.getSaiAttr().value.aclaction.enable); + ASSERT_EQ(colorIt->second.getSaiAttr().value.aclaction.parameter.s32, SAI_PACKET_COLOR_GREEN); + + // ... and PACKET_ACTION set to FORWARD. + auto paIt = actions.find(SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION); + ASSERT_NE(paIt, actions.end()); + ASSERT_TRUE(paIt->second.getSaiAttr().value.aclaction.enable); + ASSERT_EQ(paIt->second.getSaiAttr().value.aclaction.parameter.s32, SAI_PACKET_ACTION_FORWARD); + } + + sai_switch_api_t *old_sai_switch_api_no_packet_color; + + // Override SAI get_switch_attribute so that the ASIC does NOT advertise + // SET_PACKET_COLOR (only PACKET_ACTION is supported). + sai_status_t getSwitchAttributeNoPacketColor(_In_ sai_object_id_t switch_id, _In_ uint32_t attr_count, + _Inout_ sai_attribute_t *attr_list) + { + if (attr_count == 1) + { + switch (attr_list[0].id) + { + case SAI_SWITCH_ATTR_MAX_ACL_ACTION_COUNT: + attr_list[0].value.u32 = 1; + return SAI_STATUS_SUCCESS; + case SAI_SWITCH_ATTR_ACL_STAGE_INGRESS: + case SAI_SWITCH_ATTR_ACL_STAGE_EGRESS: + attr_list[0].value.aclcapability.action_list.count = 1; + attr_list[0].value.aclcapability.action_list.list[0] = SAI_ACL_ACTION_TYPE_PACKET_ACTION; + attr_list[0].value.aclcapability.is_action_list_mandatory = false; + return SAI_STATUS_SUCCESS; + } + } + return old_sai_switch_api_no_packet_color->get_switch_attribute(switch_id, attr_count, attr_list); + } + + // When the ASIC does not advertise SET_PACKET_COLOR, an ACL table whose type + // lists PACKET_COLOR_ACTION must be rejected: AclOrch validates every table + // action against the queried switch capability on table creation. + TEST_F(AclOrchTest, AclTableSetPacketColorActionNotSupported) + { + old_sai_switch_api_no_packet_color = sai_switch_api; + sai_switch_api_t new_sai_switch_api = *sai_switch_api; + sai_switch_api = &new_sai_switch_api; + sai_switch_api->get_switch_attribute = getSwitchAttributeNoPacketColor; + + auto orch = createAclOrch(); + + const string aclTableTypeName = "PACKET_COLOR_UNSUP_TABLE_TYPE"; + const string aclTableName = "PACKET_COLOR_UNSUP_TABLE"; + + orch->doAclTableTypeTask( + deque( + { + { + aclTableTypeName, + SET_COMMAND, + { + { ACL_TABLE_TYPE_MATCHES, MATCH_SRC_IP }, + { ACL_TABLE_TYPE_ACTIONS, string(ACTION_PACKET_ACTION) + comma + ACTION_PACKET_COLOR_ACTION } + } + } + }) + ); + + orch->doAclTableTask( + deque( + { + { + aclTableName, + SET_COMMAND, + { + { ACL_TABLE_TYPE, aclTableTypeName }, + { ACL_TABLE_STAGE, STAGE_INGRESS } + } + } + }) + ); + + // Restore the SAI switch API before asserting so that a failed + // expectation does not leave TearDown's remove_switch dereferencing + // the temporary stack override. + sai_switch_api = old_sai_switch_api_no_packet_color; + + // The table is rejected because SET_PACKET_COLOR is not a supported action. + ASSERT_EQ(orch->getAclTable(aclTableName), nullptr); + } + TEST_F(AclOrchTest, AclInnerSourceMacRewriteTableValidation) { const string aclTableTypeName = "INNER_SRC_MAC_REWRITE_TABLE_TYPE";