sonic-utilities : CLICK support to add/delete rules in openconfig format

acl_loader/main.py

@@ -766,6 +766,49 @@ def incremental_update(self):
                 for namespace_configdb in self.per_npu_configdb.values():
                     namespace_configdb.set_entry(self.ACL_RULE, key, self.rules_info[key])
 
+    def add_rule(self):
+        """
+        Perform update/addition of new rules only. Get existing rules from
+        Config DB. Compare with rules specified in file and perform corresponding
+        modifications. Do not delete the existing rules. Handles only dataplane rules
+        :return:
+        """
+
+        # TODO: Alternate command for adding rules in dataplane ACLs instead of removing
+        # all the rules and populating like in full/incremental.
+        new_rules = set(self.rules_info.keys())
+        new_dataplane_rules = set()
+        current_rules = set(self.rules_db_info.keys())
+        current_dataplane_rules = set()
+
+        for key in new_rules:
+            table_name = key[0]
+            if self.tables_db_info[table_name]['type'].upper() != self.ACL_TABLE_TYPE_CTRLPLANE:
+                new_dataplane_rules.add(key)
+
+        for key in current_rules:
+            table_name = key[0]
+            if self.tables_db_info[table_name]['type'].upper() != self.ACL_TABLE_TYPE_CTRLPLANE:
+                current_dataplane_rules.add(key)
+
+        added_dataplane_rules = new_dataplane_rules.difference(current_dataplane_rules)
+        existing_dataplane_rules = new_rules.intersection(current_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 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, self.rules_info[key])
+
+        for key in existing_dataplane_rules:
+            if not operator.eq(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
+                for namespace_configdb in self.per_npu_configdb.values():
+                    namespace_configdb.set_entry(self.ACL_RULE, key, self.rules_info[key])
+
     def delete(self, table=None, rule=None):
         """
         :param table:
@@ -1068,6 +1111,17 @@ def incremental(ctx, filename, session_name, mirror_stage, max_priority):
     acl_loader.load_rules_from_file(filename)
     acl_loader.incremental_update()
 
+@cli.command()
+@click.argument('filename', type=click.Path(exists=True))
+@click.pass_context
+def add(ctx, filename):
+    """
+    Add ACL rules.
+    """
+    acl_loader = ctx.obj["acl_loader"]
+
+    acl_loader.load_rules_from_file(filename)
+    acl_loader.add_rule()
 
 @cli.command()
 @click.argument('table', required=False)

config/main.py

@@ -5579,6 +5579,20 @@ def table(ctx, table_name, table_type, description, ports, stage):
 
     config_db.set_entry("ACL_TABLE", table_name, table_info)
 
+#
+# 'add' subcommand
+#
+
+@add.command()
+@click.argument('file_name', required=True)
+def rule(file_name):
+    """
+    Add ACL rule
+    """
+    log.log_info("'config acl add rule {}' executing...".format(file_name))
+    command = "acl-loader add {}".format(file_name)
+    clicommon.run_command(command)
+
 #
 # 'remove' subgroup ('config acl remove ...')
 #
@@ -5600,10 +5614,28 @@ def table(table_name):
     """
     Remove ACL table
     """
+    log.log_info("'config acl remove table {}' executing...".format(table_name))
+    command = "acl-loader delete {}".format(table_name)
+    clicommon.run_command(command)
+
     config_db = ConfigDBConnector()
     config_db.connect()
     config_db.set_entry("ACL_TABLE", table_name, None)
 
+#
+# 'delete' subcommand
+#
+
+@remove.command()
+@click.argument("table_name", required=True, metavar="<table_name>")
+@click.argument("rule_name", required=True, metavar="<rule_name>")
+def rule(table_name, rule_name):
+    """
+    Remove ACL rule
+    """
+    log.log_info("'config acl remove rule {} {}' executing...".format(table_name, rule_name))
+    command = "acl-loader delete {} {}".format(table_name, rule_name)
+    clicommon.run_command(command)
 
 #
 # 'acl update' group

tests/acl_config_test.py

@@ -1,6 +1,8 @@
 import pytest
 
 import config.main as config
+from unittest import mock
+
 
 from click.testing import CliRunner
 from config.main import expand_vlan_ports, parse_acl_table_info
@@ -79,3 +81,35 @@ def test_acl_add_table_empty_vlan(self):
 
         assert result.exit_code != 0
         assert "Cannot bind empty VLAN Vlan3000" in result.output
+
+    def test_acl_add_del_rule(self):
+        runner = CliRunner()
+
+        result = runner.invoke(
+            config.config.commands["acl"].commands["add"].commands["table"],
+            ["DATAACL_ADD_DEL", "L3", "-p", "Ethernet0"])
+
+        assert result.exit_code == 0
+
+        with mock.patch('utilities_common.cli.run_command') as mock_run_command:
+            result = runner.invoke(
+                config.config.commands["acl"].commands["add"].commands["rule"],
+                ["tests/acl_input/acl_add_1.json"])
+
+            assert result.exit_code == 0
+
+        with mock.patch('utilities_common.cli.run_command') as mock_run_command:
+            result = runner.invoke(
+                config.config.commands["acl"].commands["remove"].commands["rule"],
+                ["DATAACL_ADD_DEL", "RULE_1"])
+
+            assert result.exit_code == 0
+
+        with mock.patch('utilities_common.cli.run_command') as mock_run_command:
+            result = runner.invoke(
+                config.config.commands["acl"].commands["remove"].commands["table"],
+                ["DATAACL_ADD_DEL"])
+
+            assert result.exit_code == 0
+
+

tests/acl_input/acl_add_1.json

@@ -0,0 +1,47 @@
+{
+	"acl": {
+		"acl-sets": {
+			"acl-set": {
+				"DATAACL_ADD_DEL": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"source-ip-address": "20.0.0.2/32",
+										"destination-ip-address": "30.0.0.3/32"
+									}
+								}
+							},
+							"2": {
+								"config": {
+									"sequence-id": 2
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"source-ip-address": "21.0.0.2/32",
+										"destination-ip-address": "31.0.0.3/32"
+									}
+								}
+							}
+
+						}
+					}
+				}
+			}
+		}
+	}
+}

tests/acl_input/acl_add_2.json

@@ -0,0 +1,46 @@
+{
+	"acl": {
+		"acl-sets": {
+			"acl-set": {
+				"DATAACL_ADD_DEL": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"source-ip-address": "30.0.0.2/32",
+										"destination-ip-address": "40.0.0.3/32"
+									}
+								}
+							},
+							"3": {
+								"config": {
+									"sequence-id": 3
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"source-ip-address": "31.0.0.2/32",
+										"destination-ip-address": "41.0.0.3/32"
+									}
+								}
+							}
+						}
+					}
+				}
+			}
+		}
+	}
+}

tests/acl_loader_test.py

@@ -2,6 +2,8 @@
 import os
 import pytest
 from unittest import mock
+from sonic_py_common import multi_asic
+from swsssdk import ConfigDBConnector
 
 test_path = os.path.dirname(os.path.abspath(__file__))
 modules_path = os.path.dirname(test_path)
@@ -217,3 +219,63 @@ def test_incremental_update(self, acl_loader):
         acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/incremental_2.json'))
         acl_loader.incremental_update()
         assert acl_loader.rules_info[(('NTP_ACL', 'RULE_1'))]["PACKET_ACTION"] == "DROP"
+
+    def test_add_rule(self, acl_loader):
+        acl_loader.rules_info = {}
+        acl_loader.tables_db_info['DATAACL_ADD_DEL'] = {
+            "policy_desc": "DATAACL_ADD_DEL",
+            "ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023",
+            "stage": "INGRESS",
+            "type": "L3"
+        }
+        acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl_add_1.json'))
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_1')]
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_1')] == {
+            'SRC_IP': '20.0.0.2/32',
+            'DST_IP': '30.0.0.3/32',
+            'ETHER_TYPE': '2048',
+            'PACKET_ACTION': 'FORWARD',
+            'PRIORITY': '9999'
+        }
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_2')]
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_2')] == {
+            'SRC_IP': '21.0.0.2/32',
+            'DST_IP': '31.0.0.3/32',
+            'ETHER_TYPE': '2048',
+            'PACKET_ACTION': 'FORWARD',
+            'PRIORITY': '9998'
+        }
+
+        acl_loader.per_npu_configdb = {}
+        namespaces = multi_asic.get_front_end_namespaces()
+        for namespace in namespaces:
+            asic_id = multi_asic.get_asic_index_from_namespace(namespace)
+            # replace these with correct macros
+            acl_loader.per_npu_configdb[asic_id] = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace)
+            acl_loader.per_npu_configdb[asic_id].connect()
+
+        acl_loader.rules_db_info = acl_loader.rules_info
+
+        acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl_add_2.json'))
+        acl_loader.add_rule()
+        print(acl_loader.rules_info)
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_1')]
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_2')]
+        assert acl_loader.rules_info[("DATAACL_ADD_DEL", "RULE_1")] == {
+            "SRC_IP": "30.0.0.2/32",
+            "DST_IP": "40.0.0.3/32",
+            "ETHER_TYPE": "2048",
+            "PACKET_ACTION": "FORWARD",
+            "PRIORITY": "9999"
+        }
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_1')]
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_2')]
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_3')]
+        assert acl_loader.rules_info[('DATAACL_ADD_DEL', 'RULE_3')] == {
+            'SRC_IP': '31.0.0.2/32',
+            'DST_IP': '41.0.0.3/32',
+            'ETHER_TYPE': '2048',
+            'PACKET_ACTION': 'FORWARD',
+            'PRIORITY': '9997'
+        }
+