From 63665382938d4a474c3a61c5f38a81ba14b15c72 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Fri, 8 Oct 2021 03:52:58 +0000 Subject: [PATCH 1/4] [minigraph] Fix tagged VlanInterface if attached to multiple vlan as untagged member Signed-off-by: Qi Luo --- src/sonic-config-engine/minigraph.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index fd0012fc2f6d..1f96cbc52da9 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -528,13 +528,18 @@ def parse_dpg(dpg, hname): vlan_members = {} dhcp_relay_table = {} vlantype_name = "" - intf_vlan_mbr = defaultdict(list) + # Dict: vlan member (port/PortChannel) -> set of VlanID, in which the member if an untagged vlan member + untagged_vlan_mbr = defaultdict(set) for vintf in vlanintfs.findall(str(QName(ns, "VlanInterface"))): vlanid = vintf.find(str(QName(ns, "VlanID"))).text + vlantype = vintf.find(str(QName(ns, "Type"))) + if vlantype != None: + vlantype_name = vintf.find(str(QName(ns, "Type"))).text vintfmbr = vintf.find(str(QName(ns, "AttachTo"))).text vmbr_list = vintfmbr.split(';') - for i, member in enumerate(vmbr_list): - intf_vlan_mbr[member].append(vlanid) + if vlantype_name != "Tagged": + for member in vmbr_list: + untagged_vlan_mbr[member].add(vlanid) for vintf in vlanintfs.findall(str(QName(ns, "VlanInterface"))): vintfname = vintf.find(str(QName(ns, "Name"))).text vlanid = vintf.find(str(QName(ns, "VlanID"))).text @@ -548,7 +553,7 @@ def parse_dpg(dpg, hname): sonic_vlan_member_name = "Vlan%s" % (vlanid) if vlantype_name == "Tagged": vlan_members[(sonic_vlan_member_name, vmbr_list[i])] = {'tagging_mode': 'tagged'} - elif len(intf_vlan_mbr[member]) > 1: + elif len(untagged_vlan_mbr[member]) > 1: vlan_members[(sonic_vlan_member_name, vmbr_list[i])] = {'tagging_mode': 'tagged'} else: vlan_members[(sonic_vlan_member_name, vmbr_list[i])] = {'tagging_mode': 'untagged'} From 4deb792af978377a9b43cc9275627e36573d652f Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Fri, 8 Oct 2021 05:34:56 +0000 Subject: [PATCH 2/4] Fix bug: uninitialized vlantype_name --- src/sonic-config-engine/minigraph.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 1f96cbc52da9..e6445fcfed4a 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -527,14 +527,15 @@ def parse_dpg(dpg, hname): vlans = {} vlan_members = {} dhcp_relay_table = {} - vlantype_name = "" # Dict: vlan member (port/PortChannel) -> set of VlanID, in which the member if an untagged vlan member untagged_vlan_mbr = defaultdict(set) for vintf in vlanintfs.findall(str(QName(ns, "VlanInterface"))): vlanid = vintf.find(str(QName(ns, "VlanID"))).text vlantype = vintf.find(str(QName(ns, "Type"))) - if vlantype != None: - vlantype_name = vintf.find(str(QName(ns, "Type"))).text + if vlantype is None: + vlantype_name = "" + else: + vlantype_name = vlantype.text vintfmbr = vintf.find(str(QName(ns, "AttachTo"))).text vmbr_list = vintfmbr.split(';') if vlantype_name != "Tagged": @@ -545,8 +546,10 @@ def parse_dpg(dpg, hname): vlanid = vintf.find(str(QName(ns, "VlanID"))).text vintfmbr = vintf.find(str(QName(ns, "AttachTo"))).text vlantype = vintf.find(str(QName(ns, "Type"))) - if vlantype != None: - vlantype_name = vintf.find(str(QName(ns, "Type"))).text + if vlantype is None: + vlantype_name = "" + else: + vlantype_name = vlantype.text vmbr_list = vintfmbr.split(';') for i, member in enumerate(vmbr_list): vmbr_list[i] = port_alias_map.get(member, member) From 211776c330ef4a2ef31e29c53d356ffd774bf654 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Sun, 10 Apr 2022 20:22:16 +0000 Subject: [PATCH 3/4] Add test case of vlan member tagged attribute for non-backend TOR Signed-off-by: Qi Luo --- .../tests/sample-graph-resource-type.xml | 9 +++++ .../tests/sample-graph-subintf.xml | 8 +++++ .../tests/simple-sample-graph.xml | 9 +++++ src/sonic-config-engine/tests/test_cfggen.py | 34 ++++++++++++++++--- 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/sonic-config-engine/tests/sample-graph-resource-type.xml b/src/sonic-config-engine/tests/sample-graph-resource-type.xml index 9ba4f1e70267..62c2dfd64f3b 100644 --- a/src/sonic-config-engine/tests/sample-graph-resource-type.xml +++ b/src/sonic-config-engine/tests/sample-graph-resource-type.xml @@ -190,6 +190,14 @@ 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 @@ -205,6 +213,7 @@ 192.0.0.1;192.0.0.2 2000 2000 + Tagged 192.168.0.240/27 diff --git a/src/sonic-config-engine/tests/sample-graph-subintf.xml b/src/sonic-config-engine/tests/sample-graph-subintf.xml index d5fd2d461c8a..a23b668c2c2f 100644 --- a/src/sonic-config-engine/tests/sample-graph-subintf.xml +++ b/src/sonic-config-engine/tests/sample-graph-subintf.xml @@ -208,6 +208,14 @@ 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 diff --git a/src/sonic-config-engine/tests/simple-sample-graph.xml b/src/sonic-config-engine/tests/simple-sample-graph.xml index a8bd8b0b4685..8d7800686c7a 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph.xml @@ -190,6 +190,14 @@ 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 @@ -205,6 +213,7 @@ 192.0.0.1;192.0.0.2 2000 2000 + Tagged 192.168.0.240/27 diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index e1f6844dd5f2..a3b6e784d075 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -151,6 +151,7 @@ def test_var_json_data(self, **kwargs): utils.to_dict(output.strip()), utils.to_dict( '{\n "Vlan1000|Ethernet8": {\n "tagging_mode": "tagged"\n },' + ' \n "Vlan1001|Ethernet8": {\n "tagging_mode": "tagged"\n },' ' \n "Vlan2000|Ethernet12": {\n "tagging_mode": "tagged"\n },' ' \n "Vlan2001|Ethernet12": {\n "tagging_mode": "tagged"\n },' ' \n "Vlan2020|Ethernet12": {\n "tagging_mode": "tagged"\n }\n}' @@ -160,9 +161,10 @@ def test_var_json_data(self, **kwargs): self.assertEqual( utils.to_dict(output.strip()), utils.to_dict( - '{\n "Vlan1000|Ethernet8": {\n "tagging_mode": "untagged"\n },' + '{\n "Vlan1000|Ethernet8": {\n "tagging_mode": "tagged"\n },' + ' \n "Vlan1001|Ethernet8": {\n "tagging_mode": "tagged"\n },' ' \n "Vlan2000|Ethernet12": {\n "tagging_mode": "tagged"\n },' - ' \n "Vlan2001|Ethernet12": {\n "tagging_mode": "tagged"\n },' + ' \n "Vlan2001|Ethernet12": {\n "tagging_mode": "untagged"\n },' ' \n "Vlan2020|Ethernet12": {\n "tagging_mode": "tagged"\n }\n}' ) ) @@ -249,6 +251,7 @@ def test_minigraph_vlans(self, **kwargs): utils.to_dict(output.strip()), utils.to_dict( "{'Vlan1000': {'alias': 'ab1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1000'}, " + "'Vlan1001': {'alias': 'ab4', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1001'}," "'Vlan2001': {'alias': 'ab3', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2001'}," "'Vlan2000': {'alias': 'ab2', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2000'}," "'Vlan2020': {'alias': 'kk1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2020'}}" @@ -265,6 +268,7 @@ def test_minigraph_vlan_members(self, **kwargs): utils.to_dict(output.strip()), utils.to_dict( "{('Vlan2000', 'Ethernet12'): {'tagging_mode': 'tagged'}, " + "('Vlan1001', 'Ethernet8'): {'tagging_mode': 'tagged'}, " "('Vlan1000', 'Ethernet8'): {'tagging_mode': 'tagged'}, " "('Vlan2020', 'Ethernet12'): {'tagging_mode': 'tagged'}, " "('Vlan2001', 'Ethernet12'): {'tagging_mode': 'tagged'}}" @@ -275,9 +279,10 @@ def test_minigraph_vlan_members(self, **kwargs): utils.to_dict(output.strip()), utils.to_dict( "{('Vlan2000', 'Ethernet12'): {'tagging_mode': 'tagged'}, " - "('Vlan1000', 'Ethernet8'): {'tagging_mode': 'untagged'}, " + "('Vlan1000', 'Ethernet8'): {'tagging_mode': 'tagged'}, " + "('Vlan1001', 'Ethernet8'): {'tagging_mode': 'tagged'}, " "('Vlan2020', 'Ethernet12'): {'tagging_mode': 'tagged'}, " - "('Vlan2001', 'Ethernet12'): {'tagging_mode': 'tagged'}}" + "('Vlan2001', 'Ethernet12'): {'tagging_mode': 'untagged'}}" ) ) @@ -704,6 +709,9 @@ def test_minigraph_bgp_voq_chassis_peer(self): 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_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) @@ -737,6 +745,24 @@ def verify_no_vlan_member(self): output = self.run_script(argument) self.assertEqual(output.strip(), "{}") + def verify_sub_intf_non_backend_tor(self, **kwargs): + graph_file = kwargs.get('graph_file', self.sample_graph_simple) + + # INTERFACE table does not exist + # argument = '-m "' + graph_file + '" -p "' + self.port_config + '" -v "INTERFACE"' + # output = self.run_script(argument) + # self.assertEqual(output.strip(), "") + + # PORTCHANNEL_INTERFACE table does not exist + # argument = '-m "' + graph_file + '" -p "' + self.port_config + '" -v "PORTCHANNEL_INTERFACE"' + # output = self.run_script(argument) + # self.assertEqual(output.strip(), "") + + # All the other tables stay unchanged + self.test_var_json_data(graph_file=graph_file) + self.test_minigraph_vlans(graph_file=graph_file) + self.test_minigraph_vlan_members(graph_file=graph_file) + def verify_sub_intf(self, **kwargs): graph_file = kwargs.get('graph_file', self.sample_graph_simple) check_stderr = kwargs.get('check_stderr', True) From 072ef98590e610c08b023635f077e363430e7c40 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Mon, 18 Apr 2022 23:06:00 +0000 Subject: [PATCH 4/4] Remove commented code, change test case name --- src/sonic-config-engine/tests/test_cfggen.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index a3b6e784d075..29773dffc628 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -709,7 +709,7 @@ def test_minigraph_bgp_voq_chassis_peer(self): 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_non_backend_tor(self, check_stderr=True): + 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): @@ -748,16 +748,6 @@ def verify_no_vlan_member(self): def verify_sub_intf_non_backend_tor(self, **kwargs): graph_file = kwargs.get('graph_file', self.sample_graph_simple) - # INTERFACE table does not exist - # argument = '-m "' + graph_file + '" -p "' + self.port_config + '" -v "INTERFACE"' - # output = self.run_script(argument) - # self.assertEqual(output.strip(), "") - - # PORTCHANNEL_INTERFACE table does not exist - # argument = '-m "' + graph_file + '" -p "' + self.port_config + '" -v "PORTCHANNEL_INTERFACE"' - # output = self.run_script(argument) - # self.assertEqual(output.strip(), "") - # All the other tables stay unchanged self.test_var_json_data(graph_file=graph_file) self.test_minigraph_vlans(graph_file=graph_file)