From 5a313e691287ea44d816eabd8d8811e693fcd4c5 Mon Sep 17 00:00:00 2001 From: bingwang-ms <66248323+bingwang-ms@users.noreply.github.com> Date: Tue, 26 Jul 2022 02:00:00 +0800 Subject: [PATCH] Automatically enable tunnel_qos_remap on T1 and T0 in DualToR deployment (#11508) Why I did it This PR is to backport PR #11056 and PR #11045 into master branch. This PR is to enable tunnel_qos_remap on T1 and T0 in DualToR deployment. On T1, we check the property DownstreamRedundancyTypes. On T0, we check the property RedundancyType. tunnel_qos_remap is set to enabled if gemini is in DownstreamRedundancyTypes (on T1) or RedundancyType (on T0). How I did it The change is implemented in minigraph.py. How to verify it Verified by test_minigraph_case.py and 'test_j2files.py`. --- src/sonic-config-engine/minigraph.py | 66 +- ...ample-arista-7050cx3-dualtor-minigraph.xml | 13 +- .../sample-arista-7260-dualtor-minigraph.xml | 13 +- .../tests/sample-arista-7260-t1-minigraph.xml | 13 +- .../sample-mellanox-4600c-t1-minigraph.xml | 13 +- ...imple-sample-graph-case-remap-disabled.xml | 8 - ...ase-remap-enabled-no-tunnel-attributes.xml | 891 ++++++++++++++++++ ...simple-sample-graph-case-remap-enabled.xml | 13 +- .../tests/test_minigraph_case.py | 10 +- 9 files changed, 959 insertions(+), 81 deletions(-) create mode 100644 src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index e804201908..b8bb02d828 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -894,6 +894,7 @@ def parse_meta(meta, hname): kube_data = {} macsec_profile = {} redundancy_type = None + downstream_redundancy_types = None qos_profile = None device_metas = meta.find(str(QName(ns, "Devices"))) @@ -940,33 +941,13 @@ def parse_meta(meta, hname): macsec_profile = parse_macsec_profile(value) elif name == "RedundancyType": redundancy_type = value + elif name == "DownstreamRedundancyTypes": + downstream_redundancy_types = value elif name == "SonicQosProfile": qos_profile = value - return syslog_servers, dhcp_servers, dhcpv6_servers, ntp_servers, tacacs_servers, mgmt_routes, erspan_dst, deployment_id, region, cloudtype, resource_type, downstream_subrole, switch_id, switch_type, max_cores, kube_data, macsec_profile, redundancy_type, qos_profile + return syslog_servers, dhcp_servers, dhcpv6_servers, ntp_servers, tacacs_servers, mgmt_routes, erspan_dst, deployment_id, region, cloudtype, resource_type, downstream_subrole, switch_id, switch_type, max_cores, kube_data, macsec_profile, downstream_redundancy_types, redundancy_type, qos_profile -def parse_system_defaults(meta): - system_default_values = {} - - system_defaults = meta.find(str(QName(ns1, "SystemDefaults"))) - - if system_defaults is None: - return system_default_values - - for system_default in system_defaults.findall(str(QName(ns1, "SystemDefault"))): - name = system_default.find(str(QName(ns1, "Name"))).text - value = system_default.find(str(QName(ns1, "Value"))).text - - # Tunnel Qos remapping - if name == "TunnelQosRemapEnabled": - if value.lower() == "true": - status = "enabled" - else: - status = "disabled" - system_default_values["tunnel_qos_remap"] = {"status": status} - - return system_default_values - def parse_linkmeta(meta, hname): link = meta.find(str(QName(ns, "Link"))) linkmetas = {} @@ -1368,6 +1349,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw static_routes = {} system_defaults = {} macsec_profile = {} + downstream_redundancy_types = None redundancy_type = None qos_profile = None @@ -1400,13 +1382,11 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw elif child.tag == str(QName(ns, "UngDec")): (u_neighbors, u_devices, _, _, _, _, _, _) = parse_png(child, hostname, None) elif child.tag == str(QName(ns, "MetadataDeclaration")): - (syslog_servers, dhcp_servers, dhcpv6_servers, ntp_servers, tacacs_servers, mgmt_routes, erspan_dst, deployment_id, region, cloudtype, resource_type, downstream_subrole, switch_id, switch_type, max_cores, kube_data, macsec_profile, redundancy_type, qos_profile) = parse_meta(child, hostname) + (syslog_servers, dhcp_servers, dhcpv6_servers, ntp_servers, tacacs_servers, mgmt_routes, erspan_dst, deployment_id, region, cloudtype, resource_type, downstream_subrole, switch_id, switch_type, max_cores, kube_data, macsec_profile, downstream_redundancy_types, redundancy_type, qos_profile) = parse_meta(child, hostname) elif child.tag == str(QName(ns, "LinkMetadataDeclaration")): linkmetas = parse_linkmeta(child, hostname) elif child.tag == str(QName(ns, "DeviceInfos")): (port_speeds_default, port_descriptions, sys_ports) = parse_deviceinfo(child, hwsku) - elif child.tag == str(QName(ns, "SystemDefaultsDeclaration")): - system_defaults = parse_system_defaults(child) else: if child.tag == str(QName(ns, "DpgDec")): (intfs, lo_intfs, mvrf, mgmt_intf, voq_inband_intfs, vlans, vlan_members, dhcp_relay_table, pcs, pc_members, acls, vni, tunnel_intfs, dpg_ecmp_content, static_routes, tunnel_intfs_qos_remap_config) = parse_dpg(child, asic_name) @@ -1421,8 +1401,6 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw linkmetas = parse_linkmeta(child, hostname) elif child.tag == str(QName(ns, "DeviceInfos")): (port_speeds_default, port_descriptions, sys_ports) = parse_deviceinfo(child, hwsku) - elif child.tag == str(QName(ns, "SystemDefaultsDeclaration")): - system_defaults = parse_system_defaults(child) select_mmu_profiles(qos_profile, platform, hwsku) # set the host device type in asic metadata also @@ -1459,10 +1437,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw 'ip': kube_data.get('ip', '') } } - - if len(system_defaults) > 0: - results['SYSTEM_DEFAULTS'] = system_defaults - + results['PEER_SWITCH'], mux_tunnel_name, peer_switch_ip = get_peer_switch_info(linkmetas, devices) if bool(results['PEER_SWITCH']): @@ -1471,7 +1446,20 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw print("Warning: more than one peer switch was found. Only the first will be parsed: {}".format(results['PEER_SWITCH'].keys()[0])) results['DEVICE_METADATA']['localhost']['peer_switch'] = list(results['PEER_SWITCH'].keys())[0] + + # Enable tunnel_qos_remap if downstream_redundancy_types(T1) or redundancy_type(T0) = Gemini/Libra + enable_tunnel_qos_map = False + if results['DEVICE_METADATA']['localhost']['type'].lower() == 'leafrouter' and ('gemini' in str(downstream_redundancy_types).lower() or 'libra' in str(downstream_redundancy_types).lower()): + enable_tunnel_qos_map = True + elif results['DEVICE_METADATA']['localhost']['type'].lower() == 'torrouter' and ('gemini' in str(redundancy_type).lower() or 'libra' in str(redundancy_type).lower()): + enable_tunnel_qos_map = True + if enable_tunnel_qos_map: + system_defaults['tunnel_qos_remap'] = {"status": "enabled"} + + if len(system_defaults) > 0: + results['SYSTEM_DEFAULTS'] = system_defaults + # for this hostname, if sub_role is defined, add sub_role in # device_metadata if sub_role is not None: @@ -1872,14 +1860,28 @@ def get_tunnel_entries(tunnel_intfs, tunnel_intfs_qos_remap_config, lo_intfs, tu break tunnels = {} + + default_qos_map_for_mux_tunnel = { + "decap_dscp_to_tc_map": "AZURE_TUNNEL", + "decap_tc_to_pg_map": "AZURE_TUNNEL", + "encap_tc_to_dscp_map": "AZURE_TUNNEL", + "encap_tc_to_queue_map": "AZURE_TUNNEL" + } + for type, tunnel_dict in tunnel_intfs.items(): for tunnel_key, tunnel_attr in tunnel_dict.items(): tunnel_attr['dst_ip'] = lo_addr if (tunnel_qos_remap.get('status') == 'enabled') and (mux_tunnel_name == tunnel_key) and (peer_switch_ip is not None): tunnel_attr['src_ip'] = peer_switch_ip + # The DSCP mode must be pipe if remap is enabled + tunnel_attr['dscp_mode'] = "pipe" if tunnel_key in tunnel_intfs_qos_remap_config[type]: tunnel_attr.update(tunnel_intfs_qos_remap_config[type][tunnel_key].items()) + # Use default value if qos remap attribute is missing + for k, v in default_qos_map_for_mux_tunnel.items(): + if k not in tunnel_attr: + tunnel_attr[k] = v tunnels[tunnel_key] = tunnel_attr diff --git a/src/sonic-config-engine/tests/sample-arista-7050cx3-dualtor-minigraph.xml b/src/sonic-config-engine/tests/sample-arista-7050cx3-dualtor-minigraph.xml index 6ffea8427d..521b5c5ae0 100644 --- a/src/sonic-config-engine/tests/sample-arista-7050cx3-dualtor-minigraph.xml +++ b/src/sonic-config-engine/tests/sample-arista-7050cx3-dualtor-minigraph.xml @@ -2334,19 +2334,16 @@ 10.20.6.16 + + RedundancyType + + Gemini + - - - - TunnelQosRemapEnabled - True - - - diff --git a/src/sonic-config-engine/tests/sample-arista-7260-dualtor-minigraph.xml b/src/sonic-config-engine/tests/sample-arista-7260-dualtor-minigraph.xml index e1a7d35805..37e1a87288 100644 --- a/src/sonic-config-engine/tests/sample-arista-7260-dualtor-minigraph.xml +++ b/src/sonic-config-engine/tests/sample-arista-7260-dualtor-minigraph.xml @@ -4600,19 +4600,16 @@ 10.20.6.16 + + RedundancyType + + Gemini + - - - - TunnelQosRemapEnabled - True - - - diff --git a/src/sonic-config-engine/tests/sample-arista-7260-t1-minigraph.xml b/src/sonic-config-engine/tests/sample-arista-7260-t1-minigraph.xml index f4f4a349e9..47dcdafcd4 100644 --- a/src/sonic-config-engine/tests/sample-arista-7260-t1-minigraph.xml +++ b/src/sonic-config-engine/tests/sample-arista-7260-t1-minigraph.xml @@ -2481,19 +2481,16 @@ 10.20.6.16 + + DownstreamRedundancyTypes + + Gemini + - - - - TunnelQosRemapEnabled - True - - - str-7260cx3-acs-7 Arista-7260CX3-C64 diff --git a/src/sonic-config-engine/tests/sample-mellanox-4600c-t1-minigraph.xml b/src/sonic-config-engine/tests/sample-mellanox-4600c-t1-minigraph.xml index e7f581178b..bdb8acc631 100644 --- a/src/sonic-config-engine/tests/sample-mellanox-4600c-t1-minigraph.xml +++ b/src/sonic-config-engine/tests/sample-mellanox-4600c-t1-minigraph.xml @@ -2450,19 +2450,16 @@ 10.0.0.7 + + DownstreamRedundancyTypes + + Gemini + - - - - TunnelQosRemapEnabled - True - - - r-tigon-11 Mellanox-SN4600C-C64 diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml index 7d0be06e4e..91c0aa46c1 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml @@ -474,14 +474,6 @@ - - - - TunnelQosRemapEnabled - False - - - diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml new file mode 100644 index 0000000000..a88bc4e2ad --- /dev/null +++ b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml @@ -0,0 +1,891 @@ + + + + + + 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 + DataAcl + DataPlane + + + SNMP + SNMP_ACL + SNMP + + + ERSPAN_DSCP + Everflow_dscp + Everflow_dscp + + + + + + + + + + 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 + mux-cable + L + true + + + LogicalLink + 10000 + false + switch-t0 + fortyGigE0/8 + true + mux-cable + U + 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 + + + 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 + + Gemini + + + + + + + + + + + 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-remap-enabled.xml b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml index d38f77774e..406c613a9c 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml @@ -469,19 +469,16 @@ Storage + + RedundancyType + + Gemini + - - - - TunnelQosRemapEnabled - True - - - diff --git a/src/sonic-config-engine/tests/test_minigraph_case.py b/src/sonic-config-engine/tests/test_minigraph_case.py index bfee76c754..2aa7894416 100644 --- a/src/sonic-config-engine/tests/test_minigraph_case.py +++ b/src/sonic-config-engine/tests/test_minigraph_case.py @@ -382,7 +382,7 @@ class TestCfgGenCaseInsensitive(TestCase): "tunnel_type": "IPINIP", "src_ip": "25.1.1.10", "dst_ip": "10.1.0.32", - "dscp_mode": "uniform", + "dscp_mode": "pipe", "encap_ecn_mode": "standard", "ecn_mode": "copy_from_outer", "ttl_mode": "pipe", @@ -399,6 +399,14 @@ class TestCfgGenCaseInsensitive(TestCase): expected_tunnel ) + # Validate extra config for mux tunnel is generated automatically when tunnel_qos_remap = enabled + sample_graph_enabled_remap = os.path.join(self.test_dir, 'simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml') + argument = '-m "' + sample_graph_enabled_remap + '" -p "' + self.port_config + '" -v "TUNNEL"' + output = self.run_script(argument) + self.assertEqual( + utils.to_dict(output.strip()), + expected_tunnel + ) def test_minigraph_mux_cable_table(self): argument = '-m "' + self.sample_graph + '" -p "' + self.port_config + '" -v "MUX_CABLE"'