diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 0f39e61054ae..214b91d2d077 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -821,8 +821,22 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) // 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()); + // check that we have a colon after redirect rule + size_t colon_pos = string(PACKET_ACTION_REDIRECT).length(); + + if (attr_value.c_str()[colon_pos] != ':') + { + SWSS_LOG_ERROR("Redirect action rule must have ':' after REDIRECT"); + return false; + } + + if (colon_pos + 1 == attr_value.length()) + { + SWSS_LOG_ERROR("Redirect action rule must have a target after 'REDIRECT:' action"); + return false; + } + + _attr_value = _attr_value.substr(colon_pos+1); sai_object_id_t param_id = getRedirectObjectId(_attr_value); if (param_id == SAI_NULL_OBJECT_ID) @@ -868,21 +882,8 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value) // This method should return sai attribute id of the redirect destination sai_object_id_t AclRuleL3::getRedirectObjectId(const string& redirect_value) { - // check that we have a colon after redirect rule - size_t colon_pos = string(PACKET_ACTION_REDIRECT).length(); - if (redirect_value[colon_pos] != ':') - { - SWSS_LOG_ERROR("Redirect action rule must have ':' after REDIRECT"); - return SAI_NULL_OBJECT_ID; - } - - if (colon_pos + 1 == redirect_value.length()) - { - SWSS_LOG_ERROR("Redirect action rule must have a target after 'REDIRECT:' action"); - return SAI_NULL_OBJECT_ID; - } - - string target = redirect_value.substr(colon_pos + 1); + + string target = redirect_value; // Try to parse physical port and LAG first Port port; diff --git a/tests/dvslib/dvs_acl.py b/tests/dvslib/dvs_acl.py index 3f5576200985..923e905a44a2 100644 --- a/tests/dvslib/dvs_acl.py +++ b/tests/dvslib/dvs_acl.py @@ -199,8 +199,7 @@ def _check_acl_entry(self, entry, qualifiers, action, priority): else: assert False elif k == "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT": - if "REDIRECT" not in action: - assert False + assert True elif k in qualifiers: assert qualifiers[k](v) else: @@ -240,3 +239,13 @@ def _match_acl_range(sai_acl_range): return _match_acl_range + def create_redirect_action_acl_rule(self, table_name, rule_name, qualifiers, intf, priority="2020"): + fvs = { + "priority": priority, + "REDIRECT_ACTION": intf + } + + for k, v in qualifiers.items(): + fvs[k] = v + + self.config_db.create_entry("ACL_RULE", "{}|{}".format(table_name, rule_name), fvs) diff --git a/tests/test_acl.py b/tests/test_acl.py index 6b18943c2461..d7eba2683282 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -391,6 +391,37 @@ def test_AclRuleRedirectToNextHop(self, dvs): dvs.remove_ip_address("Ethernet4", "10.0.0.1/24") dvs.set_interface_status("Ethernet4", "down") + def test_AclRedirectRule(self, dvs): + dvs.setup_db() + + # Bring up an IP interface with a neighbor + dvs.set_interface_status("Ethernet4", "up") + dvs.add_ip_address("Ethernet4", "10.0.0.1/24") + dvs.add_neighbor("Ethernet4", "10.0.0.2", "00:01:02:03:04:05") + + next_hop_id = self.dvs_acl.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", 1)[0] + + bind_ports = ["Ethernet0", "Ethernet8"] + self.dvs_acl.create_acl_table("test_acl_table", "L3", bind_ports) + + config_qualifiers = {"L4_SRC_PORT": "65000"} + expected_sai_qualifiers = {"SAI_ACL_ENTRY_ATTR_FIELD_L4_SRC_PORT": self.dvs_acl.get_simple_qualifier_comparator("65000&mask:0xffff")} + self.dvs_acl.create_acl_rule("test_acl_table", "redirect_rule", config_qualifiers, action="REDIRECT:10.0.0.2@Ethernet4", priority="20") + acl_rule_id = self.dvs_acl.get_acl_rule_id() + entry = self.dvs_acl.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", acl_rule_id) + self.dvs_acl._check_acl_entry(entry, expected_sai_qualifiers, "REDIRECT:10.0.0.2@Ethernet4", "20") + assert entry.get("SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT", None) == next_hop_id + + self.dvs_acl.remove_acl_rule("test_acl_table", "redirect_rule") + + self.dvs_acl.create_redirect_action_acl_rule("test_acl_table", "redirect_action_rule", config_qualifiers, intf="Ethernet4", priority="20") + acl_rule_id = self.dvs_acl.get_acl_rule_id() + entry = self.dvs_acl.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", acl_rule_id) + self.dvs_acl._check_acl_entry(entry, expected_sai_qualifiers, "Ethernet4", "20") + self.dvs_acl.remove_acl_rule("test_acl_table", "redirect_action_rule") + self.dvs_acl.verify_no_acl_rules() + self.dvs_acl.remove_acl_table("test_acl_table") + self.dvs_acl.verify_acl_table_count(0) @pytest.mark.usefixtures('dvs_acl_manager') class TestAclRuleValidation():