From 37c2370165c9343ceed36f78bac56a3bcf909bed Mon Sep 17 00:00:00 2001 From: Neetha John Date: Wed, 6 Jul 2022 10:27:05 -0700 Subject: [PATCH] Minigraph parser changes for storage backend acl (#11221) Signed-off-by: Neetha John nejo@microsoft.com Why I did it For storage backend, certain rules will be applied to the DATAACL table to allow only vlan tagged packets and drop untagged packets. How I did it Create DATAACL table if the device is a storage backend device To avoid ACL resource issues, remove EVERFLOW related tables if the device is a storage backend device How to verify it Added the following unit tests - verify that EVERFLOW acl tables is removed and DATAACL table is added for storage backend tor - verify that no DATAACL tables are created and EVERFLOW tables exist for storage backend leaf --- src/sonic-config-engine/minigraph.py | 23 +- .../tests/sample-graph-storage-backend.xml | 781 ++++++++++++++++++ .../tests/sample-graph-subintf.xml | 44 - src/sonic-config-engine/tests/test_cfggen.py | 41 +- 4 files changed, 838 insertions(+), 51 deletions(-) create mode 100644 src/sonic-config-engine/tests/sample-graph-storage-backend.xml diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index b79ffff039c6..bee0d89af989 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -1172,7 +1172,26 @@ def parse_spine_chassis_fe(results, vni, lo_intfs, phyport_intfs, pc_intfs, pc_m # ############################################################################### -def filter_acl_table_bindings(acls, neighbors, port_channels, sub_role): +def filter_acl_table_for_backend(acls, vlan_members): + filter_acls = {} + for acl_name, value in acls.items(): + if 'everflow' not in acl_name.lower(): + filter_acls[acl_name] = value + + ports = set() + for vlan, member in vlan_members: + ports.add(member) + filter_acls['DATAACL'] = { 'policy_desc': 'DATAACL', + 'stage': 'ingress', + 'type': 'L3', + 'ports': list(ports) + } + return filter_acls + +def filter_acl_table_bindings(acls, neighbors, port_channels, sub_role, device_type, is_storage_device, vlan_members): + if device_type == 'BackEndToRRouter' and is_storage_device: + return filter_acl_table_for_backend(acls, vlan_members) + filter_acls = {} # If the asic role is BackEnd no ACL Table (Ctrl/Data/Everflow) is binded. @@ -1741,7 +1760,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw results['DHCP_RELAY'] = dhcp_relay_table results['NTP_SERVER'] = dict((item, {}) for item in ntp_servers) results['TACPLUS_SERVER'] = dict((item, {'priority': '1', 'tcp_port': '49'}) for item in tacacs_servers) - results['ACL_TABLE'] = filter_acl_table_bindings(acls, neighbors, pcs, sub_role) + results['ACL_TABLE'] = filter_acl_table_bindings(acls, neighbors, pcs, sub_role, current_device['type'], is_storage_device, vlan_members) results['FEATURE'] = { 'telemetry': { 'state': 'enabled' diff --git a/src/sonic-config-engine/tests/sample-graph-storage-backend.xml b/src/sonic-config-engine/tests/sample-graph-storage-backend.xml new file mode 100644 index 000000000000..837400b2d5e4 --- /dev/null +++ b/src/sonic-config-engine/tests/sample-graph-storage-backend.xml @@ -0,0 +1,781 @@ + + + + + + switch-t0 + 10.1.0.32 + BGPMonitor + 10.20.30.40 + 30 + 10 + 3 + + + false + switch-t0 + 10.0.0.58 + ARISTA02T1 + 10.0.0.59 + 1 + 180 + 60 + + + switch-t0 + FC00::75 + ARISTA02T1 + FC00::76 + 1 + 180 + 60 + + + switch-t0 + FC00::75 + ARISTA02T1 + FC00::76 + 1 + 180 + 60 + + + false + switch-t0 + 10.2.0.20 + CHASSIS_PEER + 10.2.0.21 + 1 + 180 + 60 + voq + + + + + 1 + + BGPMonitor + + + BGPPeer +
10.1.0.32
+ + + + + + + + + 65100 + switch-t0 + + +
10.0.0.59
+ + + + + +
10.2.0.21
+ + + + + + + + + 64600 + ARISTA01T1 + + + + 64600 + ARISTA02T1 + + + + 64600 + ARISTA03T1 + + + + 64600 + ARISTA04T1 + + + + 65100 + CHASSIS_PEER + + + + + + + + + + HostIP + Loopback0 + + 10.1.0.32/32 + + 10.1.0.32/32 + + + HostIP1 + Loopback0 + + FC00:1::32/128 + + FC00:1::32/128 + + + + + HostIP + eth0 + + 10.0.0.100/24 + + 10.0.0.100/24 + + + + + + + switch-t0 + + + PortChannel1 + fortyGigE0/4 + + + + + + ab1 + fortyGigE0/8 + 192.0.0.1;192.0.0.2 + 1000 + 1000 + 192.168.0.0/27 + + + ab4 + fortyGigE0/8 + 192.0.0.1;192.0.0.2 + 1001 + 1001 + 192.168.0.32/27 + + + kk1 + fortyGigE0/12 + 192.0.0.1;192.0.0.2 + 2020 + 2020 + Tagged + 192.168.0.0/28 + + + ab2 + fortyGigE0/12 + 192.0.0.1;192.0.0.2 + 2000 + 2000 + Tagged + 192.168.0.240/27 + + + ab3 + fortyGigE0/12 + 192.0.0.1;192.0.0.2 + 2001 + 2001 + 192.168.0.240/27 + + + + + + PortChannel1 + 10.0.0.56/31 + + + + PortChannel1 + FC00::71/126 + + + + fortyGigE0/0 + 10.0.0.58/31 + + + + fortyGigE0/0 + FC00::75/126 + + + + ab1 + 192.168.0.1/27 + + + + + + DataAcl + + ERSPAN + everflow + Everflow + 0 + everflow.xml + + + DataAcl + + ERSPANv6 + everflowV6 + Everflow + 0 + everflow.xml + + + DataAcl + + Loopback0 + ipv6-mgmt-only + Management + 0 + + + + DataAcl + + Loopback0 + mgmt-only + Management + 0 + + + + DataAcl + + StaticERSPAN + everflowStatic + Everflow + 0 + everflow.xml + + + + + + + + + + DeviceInterfaceLink + 1000 + ARISTA01T1 + et1 + true + switch-t0 + fortyGigE0/8 + true + + + DeviceMgmtLink + 1000 + switch-t0 + fortyGigE0/16 + true + ChassisMTS1 + mgmt0 + true + + + + + switch-t0 + Arista-7050-QX-32S + AAA00PrdStr00 + + + ARISTA01T1 + Arista + + + ARISTA02T1 + Arista + + + ARISTA03T1 + Arista + + + ARISTA04T1 + Arista + + + + + + + + DeviceInterface + + true + 1 + fortyGigE0/0 + + false + 0 + 0 + 10000 + + + DeviceInterface + + true + 1 + Ethernet1 + + false + 0 + 0 + 10000 + + + DeviceInterface + + true + 1 + Ethernet2 + + false + 0 + 0 + 10000 + + + DeviceInterface + + true + 1 + fortyGigE0/4 + + false + 0 + 0 + 25000 + + + DeviceInterface + + true + 1 + fortyGigE0/8 + + false + 0 + 0 + 40000 + Interface description + + + DeviceInterface + + true + 1 + fortyGigE0/12 + + false + 0 + 0 + 100000 + Interface description + + + DeviceInterface + + true + 1 + fortyGigE0/16 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/20 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/24 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/28 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/32 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/36 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/40 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/44 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/48 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/52 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/56 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/60 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/64 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/68 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/72 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/76 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/80 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/84 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/88 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/92 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/96 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/100 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/104 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/108 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/112 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/116 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/120 + + false + 0 + 0 + 100000 + + + DeviceInterface + + true + 1 + fortyGigE0/124 + + false + 0 + 0 + 100000 + + + true + 0 + Arista-7050-QX-32S + + + DeviceInterface + + 1 + Management1 + false + mgmt1 + 1000 + + + + + + + + switch-t0 + + + DeploymentId + + 1 + + + ResourceType + + Storage + + + + + + + switch-t0 + Arista-7050-QX-32S + diff --git a/src/sonic-config-engine/tests/sample-graph-subintf.xml b/src/sonic-config-engine/tests/sample-graph-subintf.xml index 7fa35c44cbe4..6dab0ecd4e14 100644 --- a/src/sonic-config-engine/tests/sample-graph-subintf.xml +++ b/src/sonic-config-engine/tests/sample-graph-subintf.xml @@ -11,25 +11,6 @@ 10 3 - - false - switch-t0 - 10.0.0.56 - ARISTA01T1 - 10.0.0.57 - 1 - 180 - 60 - - - switch-t0 - FC00::71 - ARISTA01T1 - FC00::72 - 1 - 180 - 60 - false switch-t0 @@ -90,12 +71,6 @@ 65100 switch-t0 - -
10.0.0.57
- - - -
10.0.0.59
@@ -253,16 +228,6 @@ PortChannel1 FC00::71/126 - - - PortChannel1001 - 10.0.0.57/31 - - - - PortChannel1001 - FC00::72/126 - ab1 @@ -308,15 +273,6 @@ mgmt0 true - - DeviceMgmtLink - 1000 - switch-t0 - Management1 - switch-m0 - Management1 - true - diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 0c3519068e90..232fded47eb5 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -39,6 +39,7 @@ def setUp(self): self.packet_chassis_graph = os.path.join(self.test_dir, 'sample-chassis-packet-lc-graph.xml') self.packet_chassis_port_ini = os.path.join(self.test_dir, 'sample-chassis-packet-lc-port-config.ini') self.macsec_profile = os.path.join(self.test_dir, 'macsec_profile.json') + self.sample_backend_graph = os.path.join(self.test_dir, 'sample-graph-storage-backend.xml') # To ensure that mock config_db data is used for unit-test cases os.environ["CFGGEN_UNIT_TESTING"] = "2" @@ -710,14 +711,11 @@ def test_minigraph_bgp_voq_chassis_peer(self): output = self.run_script(argument) self.assertEqual(output.strip(), "") - def test_minigraph_sub_port_interfaces(self, check_stderr=True): - self.verify_sub_intf(check_stderr=check_stderr) - def test_minigraph_sub_port_intf_resource_type_non_backend_tor(self, check_stderr=True): self.verify_sub_intf_non_backend_tor(graph_file=self.sample_resource_graph, check_stderr=check_stderr) - def test_minigraph_sub_port_intf_resource_type(self, check_stderr=True): - self.verify_sub_intf(graph_file=self.sample_resource_graph, check_stderr=check_stderr) + def test_minigraph_sub_port_intf_hwsku(self, check_stderr=True): + self.verify_sub_intf(graph_file=self.sample_backend_graph, check_stderr=check_stderr) def test_minigraph_sub_port_intf_sub(self, check_stderr=True): self.verify_sub_intf(graph_file=self.sample_subintf_graph, check_stderr=check_stderr) @@ -725,6 +723,32 @@ def test_minigraph_sub_port_intf_sub(self, check_stderr=True): def test_minigraph_no_vlan_member(self, check_stderr=True): self.verify_no_vlan_member() + def test_minigraph_backend_acl_leaf(self, check_stderr=True): + try: + print('\n Change device type to %s' % (BACKEND_LEAF_ROUTER)) + if check_stderr: + output = subprocess.check_output("sed -i \'s/%s/%s/g\' %s" % (TOR_ROUTER, BACKEND_LEAF_ROUTER, self.sample_backend_graph), stderr=subprocess.STDOUT, shell=True) + else: + output = subprocess.check_output("sed -i \'s/%s/%s/g\' %s" % (TOR_ROUTER, BACKEND_LEAF_ROUTER, self.sample_backend_graph), shell=True) + + self.test_jinja_expression(self.sample_backend_graph, self.port_config, BACKEND_LEAF_ROUTER) + + # ACL_TABLE should contain EVERFLOW related entries + argument = '-m "' + self.sample_backend_graph + '" -p "' + self.port_config + '" -v "ACL_TABLE"' + output = self.run_script(argument) + sample_output = utils.to_dict(output.strip()).keys() + assert 'DATAACL' not in sample_output, sample_output + assert 'EVERFLOW' in sample_output, sample_output + + finally: + print('\n Change device type back to %s' % (TOR_ROUTER)) + if check_stderr: + output = subprocess.check_output("sed -i \'s/%s/%s/g\' %s" % (BACKEND_LEAF_ROUTER, TOR_ROUTER, self.sample_backend_graph), stderr=subprocess.STDOUT, shell=True) + else: + output = subprocess.check_output("sed -i \'s/%s/%s/g\' %s" % (BACKEND_LEAF_ROUTER, TOR_ROUTER, self.sample_backend_graph), shell=True) + + self.test_jinja_expression(self.sample_backend_graph, self.port_config, TOR_ROUTER) + def test_minigraph_sub_port_no_vlan_member(self, check_stderr=True): try: print('\n Change device type to %s' % (BACKEND_LEAF_ROUTER)) @@ -780,6 +804,13 @@ def verify_sub_intf(self, **kwargs): output = self.run_script(argument) self.assertEqual(output.strip(), "") + # ACL_TABLE should not contain EVERFLOW related entries + argument = '-m "' + graph_file + '" -p "' + self.port_config + '" -v "ACL_TABLE"' + output = self.run_script(argument) + sample_output = utils.to_dict(output.strip()).keys() + assert 'DATAACL' in sample_output, sample_output + assert 'EVERFLOW' not in sample_output, sample_output + # All the other tables stay unchanged self.test_minigraph_vlans(graph_file=graph_file) self.test_minigraph_vlan_interfaces(graph_file=graph_file)