From 6fe583ed1c0fbdb74106142c88d15ff87af7ee20 Mon Sep 17 00:00:00 2001 From: Neetha John Date: Fri, 8 Jul 2022 08:47:25 -0700 Subject: [PATCH] [202012] Minigraph parser changes for storage backend acl (#11267) Signed-off-by: Neetha John Backport #11221 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 | 440 ++++++++++++++++++ .../tests/sample-graph-subintf.xml | 69 --- src/sonic-config-engine/tests/test_cfggen.py | 41 +- 4 files changed, 497 insertions(+), 76 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 4b564d83f830..035fb0bcb558 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -1052,7 +1052,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. @@ -1570,7 +1589,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': { 'status': '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..5333c073c8be --- /dev/null +++ b/src/sonic-config-engine/tests/sample-graph-storage-backend.xml @@ -0,0 +1,440 @@ + + + + + + switch-t0 + 10.1.0.32 + BGPMonitor + 10.20.30.40 + 30 + 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 + 10.0.0.58 + ARISTA02T1 + 10.0.0.59 + 1 + 180 + 60 + + + switch-t0 + FC00::75 + ARISTA02T1 + FC00::76 + 1 + 180 + 60 + + + + + 0 + + BGPMonitor + + + BGPPeer +
10.1.0.32
+ + + + + + + + + 65100 + switch-t0 + + +
10.0.0.59
+ + + + + + + + + 64600 + ARISTA01T1 + + + + 64600 + ARISTA02T1 + + + + 64600 + ARISTA03T1 + + + + 64600 + ARISTA04T1 + + + + + + + + + + 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 + + + PortChannel01 + 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 + + + + + + PortChannel01 + 10.0.0.56/31 + + + + PortChannel01 + 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 + + + true + 0 + Arista-7050-QX-32S + + + DeviceInterface + + 1 + Management1 + false + mgmt1 + 1000 + + + + + + + + switch-t0 + + + 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 8940297e7488..a5db5f55aac8 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 @@ -70,12 +51,6 @@ 65100 switch-t0 - -
10.0.0.57
- - - -
10.0.0.59
@@ -149,11 +124,6 @@ fortyGigE0/4 - - PortChannel1001 - fortyGigE0/1;fortyGigE0/2 - - @@ -227,16 +197,6 @@ PortChannel01 FC00::71/126 - - - PortChannel1001 - 10.0.0.57/31 - - - - PortChannel1001 - FC00::72/126 - ab1 @@ -272,26 +232,6 @@ fortyGigE0/8 true - - DeviceInterfaceLink - 10000 - switch-t0 - fortyGigE0/1 - true - ARISTA05T1 - Ethernet1/32 - true - - - DeviceInterfaceLink - 10000 - switch-t0 - fortyGigE0/2 - true - ARISTA06T1 - Ethernet1/33 - true - DeviceMgmtLink 1000 @@ -302,15 +242,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 5702841ceea4..87189bef27d2 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -32,6 +32,7 @@ def setUp(self): self.ecmp_graph = os.path.join(self.test_dir, 'fg-ecmp-sample-minigraph.xml') self.sample_resource_graph = os.path.join(self.test_dir, 'sample-graph-resource-type.xml') self.sample_subintf_graph = os.path.join(self.test_dir, 'sample-graph-subintf.xml') + 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" @@ -674,14 +675,11 @@ def test_minigraph_bgp_mon(self): utils.to_dict("{'10.20.30.40': {'rrclient': 0, 'name': 'BGPMonitor', 'local_addr': '10.1.0.32', 'nhopself': 0, 'holdtime': '10', 'asn': '0', 'keepalive': '3'}}") ) - 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) @@ -689,6 +687,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, 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, 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)) @@ -743,6 +767,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)