acl-loader update command enhancement

acl_loader/main.py

@@ -592,18 +592,16 @@ def full_update(self):
         for namespace_configdb in self.per_npu_configdb.values():
             namespace_configdb.mod_config({self.ACL_RULE: self.rules_info})
 
-    def incremental_update(self):
+    def incremental_update(self, dont_del=False):
         """
         Perform incremental ACL rules configuration update. Get existing rules from
         Config DB. Compare with rules specified in file and perform corresponding
         modifications.
         :return:
         """
 
-        # TODO: Until we test ASIC behavior, we cannot assume that we can insert
-        # dataplane ACLs and shift existing ACLs. Therefore, we perform a full
-        # update on dataplane ACLs, and only perform an incremental update on
-        # control plane ACLs.
+        # Broadcom ASIC supports incremental update for dataplane rules (shuffling and insertion)
+        # ASIC which doesnt support incremental behaviour can use full or load (new) command
 
         new_rules = set(self.rules_info.keys())
         new_dataplane_rules = set()
@@ -614,32 +612,46 @@ def incremental_update(self):
 
         for key in new_rules:
             table_name = key[0]
+            if self.current_table is not None and self.current_table != table_name:
+                continue
             if self.tables_db_info[table_name]['type'].upper() == self.ACL_TABLE_TYPE_CTRLPLANE:
                 new_controlplane_rules.add(key)
             else:
                 new_dataplane_rules.add(key)
 
         for key in current_rules:
             table_name = key[0]
+            if self.current_table is not None and self.current_table != table_name:
+                continue
             if self.tables_db_info[table_name]['type'].upper() == self.ACL_TABLE_TYPE_CTRLPLANE:
                 current_controlplane_rules.add(key)
             else:
                 current_dataplane_rules.add(key)
 
-        # Remove all existing dataplane rules
-        for key in current_dataplane_rules:
-            self.configdb.mod_entry(self.ACL_RULE, key, None)
-            # Program for per-asic namespace also if present
-            for namespace_configdb in self.per_npu_configdb.values():
-                namespace_configdb.mod_entry(self.ACL_RULE, key, None)
-
+        # Do Incremental update of data path rules
+        added_dataplane_rules = new_dataplane_rules.difference(current_dataplane_rules)
+        removed_dataplane_rules = current_dataplane_rules.difference(new_dataplane_rules)
+        existing_dataplane_rules = new_rules.intersection(current_dataplane_rules)
 
-        # Add all new dataplane rules
-        for key in new_dataplane_rules:
+        for key in added_dataplane_rules:
             self.configdb.mod_entry(self.ACL_RULE, key, self.rules_info[key])
-            # Program for per-asic namespace corresponding to front asic also if present. 
+
             for namespace_configdb in self.per_npu_configdb.values():
-                namespace_configdb.mod_entry(self.ACL_RULE, key, self.rules_info[key])
+                namespace_configdb.mod_entry(self.ACL_RULE, key,  self.rules_info[key]) 
+
+        if not dont_del:
+            for key in removed_dataplane_rules:
+                self.configdb.mod_entry(self.ACL_RULE, key, None)
+            # Program for per-asic namespace also if present
+                for namespace_configdb in self.per_npu_configdb.values():
+                    namespace_configdb.mod_entry(self.ACL_RULE, key, None)
+
+        for key in existing_dataplane_rules:
+            if (self.rules_info[key] != self.rules_db_info[key]):
+                self.configdb.set_entry(self.ACL_RULE, key, self.rules_info[key])
+
+                for namespace_configdb in self.per_npu_configdb.values():
+                    namespace_configdb.set_entry(self.ACL_RULE, key, self.rules_info[key])
 
         added_controlplane_rules = new_controlplane_rules.difference(current_controlplane_rules)
         removed_controlplane_rules = current_controlplane_rules.difference(new_controlplane_rules)
@@ -652,15 +664,16 @@ def incremental_update(self):
             for namespace_configdb in self.per_npu_configdb.values():
                 namespace_configdb.mod_entry(self.ACL_RULE, key, self.rules_info[key])
 
-        for key in removed_controlplane_rules:
-            self.configdb.mod_entry(self.ACL_RULE, key, None)
-            # Program for per-asic namespace corresponding to front asic also if present. 
-            # For control plane ACL it's not needed but to keep all db in sync program everywhere
-            for namespace_configdb in self.per_npu_configdb.values():
-                namespace_configdb.mod_entry(self.ACL_RULE, key, None)
+        if not dont_del:
+            for key in removed_controlplane_rules:
+                self.configdb.mod_entry(self.ACL_RULE, key, None)
+                # Program for per-asic namespace corresponding to front asic also if present. 
+                # For control plane ACL it's not needed but to keep all db in sync program everywhere
+                for namespace_configdb in self.per_npu_configdb.values():
+                    namespace_configdb.mod_entry(self.ACL_RULE, key, None)
 
         for key in existing_controlplane_rules:
-            if cmp(self.rules_info[key], self.rules_db_info[key]) != 0:
+            if (self.rules_info[key] != self.rules_db_info[key]):
                 self.configdb.set_entry(self.ACL_RULE, key, self.rules_info[key])
                 # Program for per-asic namespace corresponding to front asic also if present. 
                 # For control plane ACL it's not needed but to keep all db in sync program everywhere
@@ -946,16 +959,21 @@ def full(ctx, filename, table_name, session_name, mirror_stage, max_priority):
 
 @update.command()
 @click.argument('filename', type=click.Path(exists=True))
+@click.option('--table_name', type=click.STRING, required=False)
 @click.option('--session_name', type=click.STRING, required=False)
 @click.option('--mirror_stage', type=click.Choice(["ingress", "egress"]), default="ingress")
 @click.option('--max_priority', type=click.INT, required=False)
 @click.pass_context
-def incremental(ctx, filename, session_name, mirror_stage, max_priority):
+def incremental(ctx, filename, table_name, session_name, mirror_stage, max_priority):
     """
     Incremental update of ACL rule configuration.
+    If a table_name is provided, the operation will be restricted in the specified table.
     """
     acl_loader = ctx.obj["acl_loader"]
 
+    if table_name:
+        acl_loader.set_table_name(table_name)
+
     if session_name:
         acl_loader.set_session_name(session_name)
 
@@ -967,6 +985,34 @@ def incremental(ctx, filename, session_name, mirror_stage, max_priority):
     acl_loader.load_rules_from_file(filename)
     acl_loader.incremental_update()
 
+@update.command()
+@click.argument('filename', type=click.Path(exists=True))
+@click.option('--table_name', type=click.STRING, required=False)
+@click.option('--session_name', type=click.STRING, required=False)
+@click.option('--mirror_stage', type=click.Choice(["ingress", "egress"]), default="ingress")
+@click.option('--max_priority', type=click.INT, required=False)
+@click.pass_context
+def load(ctx, filename, table_name, session_name, mirror_stage, max_priority):
+    """
+    Add or Overwrite ACL rules configuration.
+    This operation will update the existing acl rules with new rules from the 
+    configuration without removing the common rules.
+    """
+    acl_loader = ctx.obj["acl_loader"]
+
+    if table_name:
+        acl_loader.set_table_name(table_name)
+
+    if session_name:
+        acl_loader.set_session_name(session_name)
+
+    acl_loader.set_mirror_stage(mirror_stage)
+
+    if max_priority:
+        acl_loader.set_max_priority(max_priority)
+
+    acl_loader.load_rules_from_file(filename)
+    acl_loader.incremental_update(True)
 
 @cli.command()
 @click.argument('table', required=False)

config/main.py

@@ -2946,6 +2946,17 @@ def incremental(file_name):
     command = "acl-loader update incremental {}".format(file_name)
     clicommon.run_command(command)
 
+#
+# 'load' subcommand
+#
+
+@update.command()
+@click.argument('file_name', required=True)
+def load(file_name):
+    """Load ACL rules to configuration (doesnt delete the rules)"""
+    log.log_info("'acl update load {}' executing...".format(file_name))
+    command = "acl-loader update load {}".format(file_name)
+    clicommon.run_command(command)
 
 #
 # 'dropcounters' group ('config dropcounters ...')

doc/Command-Reference.md

@@ -1315,8 +1315,7 @@ When the optional argument "max_priority"  is specified, each rule’s priority
 
 This command is used to perform incremental update of ACL rule table. This command gets existing rules from Config DB and compares with rules specified in input file and performs corresponding modifications.
 
-With respect to DATA ACLs, the command does not assume that new dataplane ACLs can be inserted in betweeen by shifting existing ACLs in all ASICs. Therefore, this command performs a full update on dataplane ACLs.
-With respect to control plane ACLs, this command performs an incremental update.
+This command performs an incremental update. The DATA ACL behaviour will be same as control plane ACL which is explained below. The ASICs which doesnt support shuffling (insert in between by shifting) use full or load commands.
 If we assume that "file1.json" is the already loaded ACL rules file and if "file2.json" is the input file that is passed as parameter for this command, the following requirements are valid for the input file.
 1) First copy the file1.json to file2.json.
 2) Remove the unwanted ACL rules from file2.json
@@ -1359,6 +1358,48 @@ When the optional argument "max_priority"  is specified, each rule’s priority
   File "acl_incremental_snmp_1_3_ssh_4.json" has got SNMP Rule1, SNMP Rule3 and SSH Rule4.
   This file is created by copying the file "acl_full_snmp_1_2_ssh_4.json" to "acl_incremental_snmp_1_3_ssh_4.json" and then removing SNMP Rule2 and adding SNMP Rule3.
 
+**config acl update load**
+
+This command is used to perform incremental update of ACL rule table. The difference with above 'update incremental' command is that this doesnt remove any rules from the table. This command gets existing rules from Config DB and compares with rules specified in input file and performs corresponding modifications.
+
+If we assume that "file1.json" is the already loaded ACL rules file and if "file2.json" is the input file that is passed as parameter for this command, the following requirements are valid for the input file.
+1) First copy the file1.json to file2.json.
+2) Add the newly required ACL rules into file2.json.
+3) Modify the existing ACL rules (that require changes) in file2.json.
+
+When "--session_name" optional argument is specified, command sets the session_name for the ACL table with this mirror session name. It fails if the specified mirror session name does not exist.
+
+When "--mirror_stage" optional argument is specified, command sets the mirror action to ingress/egress based on this parameter. By default command sets ingress mirror action in case argument is not specified.
+
+When the optional argument "max_priority"  is specified, each rule’s priority is calculated by subtracting its “sequence_id” value from the “max_priority”. If this value is not passed, the default “max_priority” 10000 is used.
+
+- Usage:
+  ```
+  config acl update load [--session_name <session_name>] [--mirror_stage (ingress | egress)] [--max_priority <priority_value>] <acl_json_file_name>
+  ```
+
+  - Parameters:
+    - table_name: Specifiy the name of the ACL table to load. Example: config acl update full "--table_name DT_ACL_T1  /etc/sonic/acl_table_1.json"
+    - session_name: Specifiy the name of the ACL session to load. Example: config acl update full "--session_name mirror_ses1 /etc/sonic/acl_table_1.json"
+    - priority_value: Specify the maximum priority to use when loading ACL rules. Example: config acl update full "--max-priority 100  /etc/sonic/acl_table_1.json"
+
+    *NOTE 1: All these optional parameters should be inside double quotes. If none of the options are provided, double quotes are not required for specifying filename alone.*
+    *NOTE 2: Any number of optional parameters can be configured in the same command.*
+
+- Examples:
+  ```
+  admin@sonic:~$ sudo config acl update load /etc/sonic/acl_incremental_snmp_1_3_ssh_4.json
+  ```
+  ```
+  admin@sonic:~$ sudo config acl update load "--session_name everflow0 /etc/sonic/acl_incremental_snmp_1_3_ssh_4.json"
+  ```
+
+  Refer the example file [acl_incremental_snmp_1_3_ssh_4.json](#) that adds two rules for SNMP (Rule1 and Rule3) and one rule for SSH (Rule4)
+  When this "load" command is executed after "full" command, it has NOT removed SNMP Rule2 and added SNMP Rule3 in the example.
+  File "acl_full_snmp_1_2_ssh_4.json" has got SNMP Rule1, SNMP Rule2 and SSH Rule4.
+  File "acl_incremental_snmp_1_3_ssh_4.json" has got SNMP Rule1, SNMP Rule3 and SSH Rule4.
+  This file is created by copying the file "acl_full_snmp_1_2_ssh_4.json" to "acl_incremental_snmp_1_3_ssh_4.json" and then removing SNMP Rule2 and adding SNMP Rule3.
+
 Go Back To [Beginning of the document](#) or [Beginning of this section](#acl)