diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index fd8cc06ca181..9d8dd6e3c00f 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -605,6 +605,40 @@ def parse_dpg(dpg, hname): is_mirror = False is_mirror_v6 = False is_mirror_dscp = False + use_port_alias = True + + # Walk through all interface names/alias to determine whether the input string is + # port name or alias.We need this logic because there can be duplicaitons in port alias + # and port names + # The input port name/alias can be either port_name or port_alias. A mix of name and alias is not accepted + port_name_count = 0 + port_alias_count = 0 + total_count = 0 + for member in aclattach: + member = member.strip() + if member in pcs or \ + member in vlans or \ + member.lower().startswith('erspan') or \ + member.lower().startswith('egress_erspan') or \ + member.lower().startswith('erspan_dscp'): + continue + total_count += 1 + if member in port_alias_map: + port_alias_count += 1 + if member in port_names_map: + port_name_count += 1 + # All inputs are port alias + if port_alias_count == total_count: + use_port_alias = True + # All inputs are port name + elif port_name_count == total_count: + use_port_alias = False + # There are both port alias and port name, then port alias is preferred to keep the behavior not changed + else: + use_port_alias = True + # For CTRLPLANE ACL, both counters are 0 + if (port_alias_count != 0) and (port_name_count != 0): + print("Warning: The given port name for ACL " + aclname + " is inconsistent. It must be either port name or alias ", file=sys.stderr) # TODO: Ensure that acl_intfs will only ever contain front-panel interfaces (e.g., # maybe we should explicity ignore management and loopback interfaces?) because we @@ -622,11 +656,15 @@ def parse_dpg(dpg, hname): acl_intfs.extend(vlans[member]['members']) else: acl_intfs.append(member) - elif member in port_alias_map: + elif use_port_alias and (member in port_alias_map): acl_intfs.append(port_alias_map[member]) # Give a warning if trying to attach ACL to a LAG member interface, correct way is to attach ACL to the LAG interface if port_alias_map[member] in intfs_inpc: print("Warning: ACL " + aclname + " is attached to a LAG member interface " + port_alias_map[member] + ", instead of LAG interface", file=sys.stderr) + elif (not use_port_alias) and (member in port_names_map): + acl_intfs.append(member) + if member in intfs_inpc: + print("Warning: ACL " + aclname + " is attached to a LAG member interface " + member + ", instead of LAG interface", file=sys.stderr) elif member.lower().startswith('erspan') or member.lower().startswith('egress_erspan') or member.lower().startswith('erspan_dscp'): if 'dscp' in member.lower(): is_mirror_dscp = True @@ -1254,6 +1292,8 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw docker_routing_config_mode = child.text (ports, alias_map, alias_asic_map) = get_port_config(hwsku=hwsku, platform=platform, port_config_file=port_config_file, asic_name=asic_name, hwsku_config_file=hwsku_config_file) + + port_names_map.update(ports) port_alias_map.update(alias_map) port_alias_asic_map.update(alias_asic_map) @@ -1803,6 +1843,7 @@ def parse_asic_meta_get_devices(root): return local_devices +port_names_map = {} port_alias_map = {} port_alias_asic_map = {} diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case-acl-test.xml b/src/sonic-config-engine/tests/simple-sample-graph-case-acl-test.xml new file mode 100644 index 000000000000..13dbbd17a234 --- /dev/null +++ b/src/sonic-config-engine/tests/simple-sample-graph-case-acl-test.xml @@ -0,0 +1,974 @@ + + + + + + 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 + + + + + 65100 + switch-t0 + + +
10.0.0.57
+ + + + + +
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 + fc02:2000::1;fc02:2000::2 + 1000 + 1000 + 192.168.0.0/27 + 00:aa:bb:cc:dd:ee + + + ab2 + fortyGigE0/4 + 192.0.0.1 + fc02:2000::3;fc02:2000::4 + 2000 + 2000 + + + + + + + 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 + + + + + + PortChannel01;Ethernet20;Ethernet24 + DataAcl_port_name + DataPlane + + + PortChannel01;Ethernet6/1;Ethernet7/1 + DataAcl_port_alias + DataPlane + + + PortChannel01;Ethernet0;Ethernet1;Ethernet2;Ethernet3 + DataAcl_mixed_name_alias_1 + DataPlane + + + PortChannel01;Ethernet1;Ethernet2;Ethernet6/1;Ethernet7/1 + DataAcl_mixed_name_alias_2 + DataPlane + + + Ethernet1 + DataAcl_mixed_name_alias_3 + DataPlane + + + SNMP + SNMP_ACL + SNMP + + + ERSPAN_DSCP + Everflow_dscp + Everflow_dscp + + + + + + + + + + LoopbackInterface + HostIP + Loopback0 + + 10.10.10.2/32 + + 10.10.10.2/32 + + + LoopbackInterface + HostIP1 + Loopback0 + + fe80::0002/128 + + fe80::0002/128 + + + LoopbackInterface + SoCHostIP0 + server2SOC + + 10.10.10.3/32 + + 10.10.10.3/32 + + + LoopbackInterface + SoCHostIP1 + server2SOC + + fe80::0003/128 + + fe80::0003/128 + + + + + + + + server2 + + + + + + + + + + + + + + + DeviceSerialLink + 9600 + switch-t0 + console + true + switch-t1 + 1 + + + DeviceSerialLink + 9600 + switch-t0 + 1 + true + managed_device + console + + + DeviceInterfaceLink + 10000 + switch-t0 + fortyGigE0/0 + switch-01t1 + port1 + + + DeviceInterfaceLink + 10000 + switch-t0 + fortyGigE0/12 + switch-02t1 + port1 + + + DeviceInterfaceLink + 25000 + switch-t0 + fortyGigE0/4 + server1 + port1 + + + DeviceInterfaceLink + 40000 + switch-t0 + fortyGigE0/8 + server2 + port1 + + + LogicalLink + 10000 + false + switch-t0 + fortyGigE0/4 + true + server1-SC + L + true + + + LogicalLink + 0 + false + switch-t0 + MuxTunnel0 + false + switch2-t0 + MuxTunnel0 + true + + + + + ToRRouter +
+ 26.1.1.10/32 +
+ switch-t0 + Force10-S6000 + AAA00PrdStr00 + + +
+ 25.1.1.10/32 +
+ + 10.7.0.196/26 + + switch2-t0 + Force10-S6000 + + + switch-01t1 +
+ 10.1.0.186/32 +
+ 2 + + + 10.7.0.196/26 + + Force10-S6000 + + + SmartCable +
+ 0.0.0.0/0 +
+ + ::/0 + + + 0.0.0.0/0 + + + ::/0 + + + server1-SC + smartcable-sku + + + Server +
+ 10.10.10.1/32 +
+ + fe80::0001/80 + + + 10.0.0.1/32 + + server1 + server-sku + + + Server +
+ 10.10.10.2/32 +
+ + fe80::0002/128 + + + 10.0.0.2/32 + + server2 + server-sku + + + + + + + + + + GeminiPeeringLink + + True + + + UpperTOR + + switch-t0 + + + LowerTOR + + switch2-t0 + + + switch2-t0:MuxTunnel0;switch-t0:MuxTunnel0 + + + + + + AutoNegotiation + + True + + + switch-01t1:port1;switch-t0:fortyGigE0/0 + + + + + + AutoNegotiation + + True + + + switch-02t1:port1;switch-t0:fortyGigE0/12 + + + + + + AutoNegotiation + + True + + + server1:port1;switch-t0:fortyGigE0/4 + + + + + + AutoNegotiation + + True + + + server2:port1;switch-t0:fortyGigE0/8 + + + + + + + switch-t0 + + + DeploymentId + + 1 + + + ErspanDestinationIpv4 + + 10.0.100.1 + + + NtpResources + + 10.0.10.1;10.0.10.2 + + + + SnmpResources + + 10.0.10.3;10.0.10.4 + + + + SyslogResources + + 10.0.10.5;10.0.10.6 + + + + TacacsServer + + 10.0.10.7;10.0.10.8 + + + KubernetesEnabled + + 0 + + + KubernetesServerIp + + 10.10.10.10 + + + ResourceType + + Storage + + + RedundancyType + + Mixed + + + + + + + + + + + DeviceInterface + + true + 1 + fortyGigE0/0 + + 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 + Force10-S6000 + + + DeviceInterface + + true + 1 + eth0 + false + eth0 + 1000 + + + + + switch-t0 + Force10-S6000 + diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case.xml b/src/sonic-config-engine/tests/simple-sample-graph-case.xml index acaab2c4c7e3..9e15e1a0c486 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph-case.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph-case.xml @@ -180,7 +180,7 @@ - PortChannel01 + PortChannel01;Ethernet0;Ethernet12 DataAcl DataPlane diff --git a/src/sonic-config-engine/tests/t0-sample-port-config-duplicated-name-alias.ini b/src/sonic-config-engine/tests/t0-sample-port-config-duplicated-name-alias.ini new file mode 100644 index 000000000000..dafdb570e9cc --- /dev/null +++ b/src/sonic-config-engine/tests/t0-sample-port-config-duplicated-name-alias.ini @@ -0,0 +1,36 @@ +# name lanes alias index +Ethernet0 9 Ethernet1 1 +Ethernet1 10 Ethernet2 2 +Ethernet2 11 Ethernet3 3 +Ethernet3 12 Ethernet4 4 +Ethernet4 13,14,15,16 Ethernet6/1 6 +Ethernet8 17,18,19,20 Ethernet7/1 7 +Ethernet12 21,22,23,24 Ethernet8/1 8 +Ethernet16 29,30,31,32 Ethernet9/1 9 +Ethernet20 25,26,27,28 Ethernet10/1 10 +Ethernet24 33,34,35,36 Ethernet11/1 11 +Ethernet28 37,38,39,40 Ethernet12/1 12 +Ethernet32 45,46,47,48 Ethernet13/1 13 +Ethernet36 41,42,43,44 Ethernet14/1 14 +Ethernet40 49,50,51,52 Ethernet15/1 15 +Ethernet44 53,54,55,56 Ethernet16/1 16 +Ethernet48 69,70,71,72 Ethernet17/1 17 +Ethernet52 65,66,67,68 Ethernet18/1 18 +Ethernet56 73,74,75,76 Ethernet19/1 19 +Ethernet60 77,78,79,80 Ethernet20/1 20 +Ethernet64 93,94,95,96 Ethernet21/1 21 +Ethernet68 89,90,91,92 Ethernet22/1 22 +Ethernet72 97,98,99,100 Ethernet23/1 23 +Ethernet76 101,102,103,104 Ethernet24/1 24 +Ethernet80 109,110,111,112 Ethernet25/1 25 +Ethernet84 105,106,107,108 Ethernet26/1 26 +Ethernet88 121,122,123,124 Ethernet27/1 27 +Ethernet92 125,126,127,128 Ethernet28/1 28 +Ethernet96 61,62,63,64 Ethernet29 29 +Ethernet100 57,58,59,60 Ethernet30 30 +Ethernet104 81,82,83,84 Ethernet31 31 +Ethernet108 85,86,87,88 Ethernet32 32 +Ethernet112 117,118,119,120 Ethernet33 33 +Ethernet116 113,114,115,116 Ethernet34 34 +Ethernet120 1,2,3,4 Ethernet35 35 +Ethernet124 5,6,7,8 Ethernet36 36 diff --git a/src/sonic-config-engine/tests/test_minigraph_case.py b/src/sonic-config-engine/tests/test_minigraph_case.py index 2f7a74bf3ed0..bd2151459fad 100644 --- a/src/sonic-config-engine/tests/test_minigraph_case.py +++ b/src/sonic-config-engine/tests/test_minigraph_case.py @@ -456,6 +456,29 @@ def test_minigraph_mirror_dscp(self): expected_ports.sort() ) + def test_minigraph_acl_attach_to_ports(self): + """ + The test case is to verify ACL table can be bound to both port names and alias + """ + sample_graph = os.path.join(self.test_dir,'simple-sample-graph-case-acl-test.xml') + port_config_duplicated_name_alias = os.path.join(self.test_dir, 't0-sample-port-config-duplicated-name-alias.ini') + result = minigraph.parse_xml(sample_graph, port_config_file=port_config_duplicated_name_alias) + # TC1: All ports are portchannels or port names + expected_dataacl_ports = ['PortChannel01','Ethernet20','Ethernet24'] + self.assertEqual(sorted(result['ACL_TABLE']['DATAACL_PORT_NAME']['ports']), sorted(expected_dataacl_ports)) + # TC2: All ports are portchanels or port alias + expected_dataacl_ports = ['PortChannel01','Ethernet4','Ethernet8'] + self.assertEqual(sorted(result['ACL_TABLE']['DATAACL_PORT_ALIAS']['ports']), sorted(expected_dataacl_ports)) + # TC3: Duplicated values in port names and alias, but all fall in port names + expected_dataacl_ports = ['PortChannel01','Ethernet0','Ethernet1','Ethernet2','Ethernet3'] + self.assertEqual(sorted(result['ACL_TABLE']['DATAACL_MIXED_NAME_ALIAS_1']['ports']), sorted(expected_dataacl_ports)) + # TC4: Duplicated values in port names and alias, but all fall in port alias + expected_dataacl_ports = ['PortChannel01','Ethernet0','Ethernet1','Ethernet4','Ethernet8'] + self.assertEqual(sorted(result['ACL_TABLE']['DATAACL_MIXED_NAME_ALIAS_2']['ports']), sorted(expected_dataacl_ports)) + # TC5: Same count in port names and alias, port alias is preferred + expected_dataacl_ports = ['Ethernet0'] + self.assertEqual(sorted(result['ACL_TABLE']['DATAACL_MIXED_NAME_ALIAS_3']['ports']), sorted(expected_dataacl_ports)) + def test_parse_device_desc_xml_mgmt_interface(self): # Regular device_desc.xml with both IPv4 and IPv6 mgmt address result = minigraph.parse_device_desc_xml(self.sample_simple_device_desc)