Skip to content

Use list instead of dict for breakdown #304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
congwang09 merged 4 commits into from
May 7, 2026
Merged

Use list instead of dict for breakdown #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de3502c
Use list instead of dict for breakdown
congwang09 Apr 22, 2026
724fa74
Fix test
congwang09 Apr 22, 2026
7ce91b2
Fix test
congwang09 Apr 22, 2026
7086ecb
Merge branch 'main' into use-list-breakdown
congwang09 Apr 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 38 additions & 21 deletions src/sdx_pce/topology/temanager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,13 +785,17 @@ def generate_connection_breakdown(
self._logger.warning(f"Can't find a TE solution for {connection_request}")
raise TEError(f"Can't find a TE solution for: {connection_request}", 410)

breakdown = {}
# Keep path order and allow the same domain to appear more than once.
# A dict collapses paths such as tenet -> sax -> tenet into one
# tenet segment, which corrupts alternate-path breakdown generation.
breakdown = []
paths = solution.connection_map # p2p for now

for domain, links in paths.items():
self._logger.info(f"domain: {domain}, links: {links}")

current_link_set = []
current_domain = None

for count, link in enumerate(links):
self._logger.info(f"count: {count}, link: {link}")
Expand Down Expand Up @@ -827,13 +831,13 @@ def generate_connection_breakdown(
if src_domain == dst_domain:
# current_domain = domain_1
if count == len(links) - 1:
breakdown[current_domain] = current_link_set.copy()
breakdown.append((current_domain, current_link_set.copy()))
else:
breakdown[current_domain] = current_link_set.copy()
breakdown.append((current_domain, current_link_set.copy()))
if count == len(links) - 1:
current_link_set = []
current_link_set.append(link)
breakdown[dst_domain] = current_link_set.copy()
breakdown.append((dst_domain, current_link_set.copy()))
current_domain = None
current_link_set = []

Expand All @@ -842,12 +846,7 @@ def generate_connection_breakdown(
# now starting with the ingress_port
first = True
i = 0
domain_breakdown = {}

# TODO: using dict to represent a breakdown is dubious, and
# may lead to incorrect results. Dicts are lexically ordered,
# and that may break some assumptions about the order in which
# we form and traverse the breakdown.
domain_breakdown = []

# Note: Extra flag to indicate if the connection request is in
# the format of TrafficMatrix or not.
Expand Down Expand Up @@ -891,7 +890,7 @@ def generate_connection_breakdown(
ingress_user_port = connection_request.get("ingress_port")
egress_user_port = connection_request.get("egress_port")

for domain, links in breakdown.items():
for domain, links in breakdown:
self._logger.debug(
f"Creating domain_breakdown: domain: {domain}, links: {links}"
)
Expand Down Expand Up @@ -922,6 +921,7 @@ def generate_connection_breakdown(
if (
not request_format_is_tm
and same_domain_port_flag
and len(breakdown) == 1
and connection_request["egress_port"]["id"]
not in self.topology_manager.get_port_link_map()
):
Expand All @@ -933,7 +933,7 @@ def generate_connection_breakdown(
_, next_ingress_port = self._get_ports_by_link(links[-1])
else:
egress_port, next_ingress_port = self._get_ports_by_link(links[-1])
if same_domain_port_flag:
if same_domain_port_flag and len(breakdown) == 1:
egress_port = next_ingress_port
self._logger.debug(
f"ingress_port:{ingress_port}, egress_port:{egress_port}, next_ingress_port:{next_ingress_port}"
Expand Down Expand Up @@ -968,13 +968,13 @@ def generate_connection_breakdown(

self._logger.info(f"segment for {domain}: {segment}")

domain_breakdown[domain] = segment.copy()
domain_breakdown.append((domain, segment.copy()))
i = i + 1

if len(domain_breakdown.keys()) > max_number_oxps:
if len(domain_breakdown) > max_number_oxps:
self._logger.warning(
"Breakdown has more domains than max number of OXPs required in the request:"
f" {len(domain_breakdown.keys())=} {max_number_oxps=}"
f" {len(domain_breakdown)=} {max_number_oxps=}"
)
raise TEError(
"Can't fulfill QoS requiments: max number of OXPs exceeded", 410
Expand Down Expand Up @@ -1070,7 +1070,7 @@ def _get_ports_by_link(self, link: ConnectionPath):

def _reserve_vlan_breakdown(
self,
domain_breakdown: dict,
domain_breakdown: list,
connection_request: dict,
ingress_user_port=None,
egress_user_port=None,
Expand Down Expand Up @@ -1121,7 +1121,7 @@ def _reserve_vlan_breakdown(
f"reserve_vlan_breakdown: domain_breakdown: {domain_breakdown}"
)

domain_breakdown_list = list(domain_breakdown.items())
domain_breakdown_list = domain_breakdown
domain_breakdown_list_len = len(domain_breakdown_list)
common_vlan_on_link = {} # {domain1: upstream_egress_vlan}
for i in range(domain_breakdown_list_len - 1):
Expand Down Expand Up @@ -1149,13 +1149,13 @@ def _reserve_vlan_breakdown(
None,
f"Failed: No common VLAN found on the link:{upstream_egress['id']} -> {downstream_ingress['id']}",
)
common_vlan_on_link[domain] = upstream_egress_vlan
common_vlan_on_link[i] = upstream_egress_vlan

breakdowns = {}
i = 0
upstream_egress_vlan = None
downstream_ingress_vlan = None
for domain, segment in domain_breakdown.items():
for domain, segment in domain_breakdown:
# These are topology ports
ingress_port = segment.get("ingress_port")
egress_port = segment.get("egress_port")
Expand Down Expand Up @@ -1184,11 +1184,11 @@ def _reserve_vlan_breakdown(

if i == 0: # first domain
upstream_egress_vlan = None
downstream_ingress_vlan = common_vlan_on_link.get(domain)
downstream_ingress_vlan = common_vlan_on_link.get(i)
elif i == domain_breakdown_list_len - 1: # last domain
downstream_ingress_vlan = None
else: # middle domain
downstream_ingress_vlan = common_vlan_on_link.get(domain)
downstream_ingress_vlan = common_vlan_on_link.get(i)

i += 1

Expand Down Expand Up @@ -1351,6 +1351,9 @@ def _find_common_vlan_on_link(
# accordingly. This code could probably be simplified if we
# use a "proper" data structure to represent the original
# connection request internally.
ingress_vlans_str = None
egress_vlans_str = None

if connection_request and isinstance(connection_request, dict):
ingress_vlans_str = connection_request.get("ingress_port").get("vlan_range")
egress_vlans_str = connection_request.get("egress_port").get("vlan_range")
Expand Down Expand Up @@ -1394,6 +1397,20 @@ def _find_common_vlan_on_link(
return ingress_vlans_str

for vlan in common_vlans:
if (
ingress_vlans_str
and ingress_vlans_str == egress_vlans_str
and str(ingress_vlans_str).isdigit()
):
requested_vlan = int(ingress_vlans_str)
if (
requested_vlan in common_vlans
and upstream_vlan_table[requested_vlan] is UNUSED_VLAN
and downstream_vlan_table[requested_vlan] is UNUSED_VLAN
):
return requested_vlan

for vlan in sorted(common_vlans):
if (
upstream_vlan_table[vlan] is UNUSED_VLAN
and downstream_vlan_table[vlan] is UNUSED_VLAN
Expand Down
8 changes: 4 additions & 4 deletions tests/test_te_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2137,7 +2137,7 @@ def test_update_available_vlans_across_two_domains(self):
reserved_vlans_amlight["urn:sdx:port:ampath.net:Ampath1:50"][150]
)
self.assertIn("urn:sdx:port:sax.net:Sax01:41", reserved_vlans_sax)
self.assertIsNotNone(reserved_vlans_sax["urn:sdx:port:sax.net:Sax01:41"][1])
self.assertIsNotNone(reserved_vlans_sax["urn:sdx:port:sax.net:Sax01:41"][150])

# Update available VLANs
temanager.update_available_vlans(temanager._vlan_tags_table)
Expand All @@ -2152,7 +2152,7 @@ def test_update_available_vlans_across_two_domains(self):
reserved_vlans_amlight["urn:sdx:port:ampath.net:Ampath1:50"][150]
)
self.assertIn("urn:sdx:port:sax.net:Sax01:40", reserved_vlans_sax)
self.assertIsNotNone(reserved_vlans_sax["urn:sdx:port:sax.net:Sax01:40"][1])
self.assertIsNotNone(reserved_vlans_sax["urn:sdx:port:sax.net:Sax01:40"][150])

# Verify the 'vlan_range' property of the 'service' property in the corresponding port
amlight_port = temanager.topology_manager.get_port_obj_by_id(
Expand Down Expand Up @@ -2187,13 +2187,13 @@ def test_update_available_vlans_across_two_domains(self):
reserved_vlans_amlight["urn:sdx:port:ampath.net:Ampath1:50"][150]
)
self.assertIn("urn:sdx:port:sax.net:Sax01:40", reserved_vlans_sax)
self.assertIsNone(reserved_vlans_sax["urn:sdx:port:sax.net:Sax01:40"][1])
self.assertIsNone(reserved_vlans_sax["urn:sdx:port:sax.net:Sax01:40"][150])

low, high = amlight_port.services.l2vpn_ptp["vlan_range"][0].split("-")
self.assertTrue(int(low) <= 150 <= int(high))

low, high = sax_port.services.l2vpn_ptp["vlan_range"][0].split("-")
self.assertTrue(int(low) <= 1 <= int(high))
self.assertTrue(int(low) <= 150 <= int(high))

def test_update_available_vlans_basic_checks(self):
"""
Expand Down
Loading