[aclorch.cpp] Handle all the ACL redirect requests in AclRuleL3::vali…

…dateAddAction() (sonic-net#1278) * changed method AclRuleL3::validateAddAction() and AclRuleL3::getRedirectObjectId() to handle all ACL redirect cases * Handled string handling of attr_value in AclRuleL3::validateAddAction() method * added VS testcases for ACL redirect and priority ACL tests Conflicts: tests/test_acl.py * added VS testcases for ACL redirect and priority ACL tests Conflicts: tests/test_acl.py Conflicts: tests/test_acl.py * removed test_AclpriorityRule() test case, as it does not belong here * removed test_AclprioriyRule() test case as it does not belong here * redone the wrongly done resolved conflict of test_acl.py file * address vs test failure * addressed vs test failure * fixed self.asic_db.wait_for_n_keys to self.dvs_acl.asic_db.wait_for_n_keys in the relevant test case * fixed all the required function calls relevant to dvs_acl in my testcase Co-authored-by: Madhan Babu <madhan@arc-build-server.mtr.labs.mlnx> Co-authored-by: root <root@arc-host16-009.mtr.labs.mlnx>
lguohan · Jul 15, 2020 · b16368c · b16368c
1 parent 310e0aa
commit b16368c
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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():