Pr/routing control plane participation#561
Conversation
…rticipation create rr_design
…rticipation update rr_design
…rticipation update rr_design
… pr/routing-control-plane-participation
| self.__subnets = None if asn > 255 else list(IPv4Network(subnetTemplate.format(asn)).subnets(new_prefix = 24)) | ||
| self.__name_servers = [] | ||
| self.__clusters = {} | ||
| self.__ibgp_mode = None |
There was a problem hiding this comment.
Setting it to IBGP_MODE_FULL_MESH directly here improves readability for both users and developers. Using the getIbgpMode() function at a later position may make the logic harder to follow.
| IBGP_MODE_ROUTE_REFLECTOR = "route-reflector" | ||
| IBGP_MODE_DISABLED = "disabled" | ||
|
|
||
| IBGP_MODE_LEGACY_FULL_MESH = "legacy-full-mesh" |
There was a problem hiding this comment.
We already have IBGP_MODE_FULL_MESH, which is also set as our default mode. There is no need to introduce additional definitions for IBGP_MODE_LEGACY_FULL_MESH and IBGP_MODE_ALIASES. Furthermore, no functions within this file reference these variables, and users should not configure parameters to these values manually.
| self.__ibgp_mode = None | ||
| self.__bgp_scope = BGP_SCOPE_ALL_ROUTERS | ||
| self.__core_forwarding = CORE_FORWARDING_PLAIN_IP | ||
| self.__ospf_mode = None |
There was a problem hiding this comment.
We can also directly set it to OSPF_MODE_LEGACY here, which would be more intuitive for readers.
| ROUTER_BGP_ROLE_CORE = "core" | ||
| ROUTER_BGP_ROLES = {ROUTER_BGP_ROLE_EDGE, ROUTER_BGP_ROLE_CORE} | ||
|
|
||
| ROUTER_CONTROL_PLANE_ROLE_EDGE = ROUTER_BGP_ROLE_EDGE |
There was a problem hiding this comment.
We already have BGP role, so introducing another BGP_CONTROL_PLANE_ROLE seems unnecessary, as both represent the exact same concept.
| """ | ||
| return self.__masked | ||
|
|
||
| def setAsMode(self, asn: int, mode: str) -> Ibgp: |
There was a problem hiding this comment.
Didn’t we agree to remove this setAsMode function earlier? All relevant configurations are now uniformly managed at the AS level.
| """ | ||
| return set(self.__excluded.get(int(asn), set())) | ||
|
|
||
| def addSession(self, asn: int, localRouterName: str, peerRouterName: str) -> Ibgp: |
There was a problem hiding this comment.
It is unclear what purpose this direct error-reporting function serves, as no calls to it can be found in the surrounding code context.
| ) | ||
| } | ||
|
|
||
| def __get_legacy_router_list(self, asn: int, routers: List[Node], routers_map: Dict[str, Node]) -> List[Node]: |
There was a problem hiding this comment.
The purpose of this function is unclear, and it is not invoked by any other functions in the codebase.
|
|
||
| n += 1 | ||
|
|
||
| def configure(self, emulator: Emulator): |
There was a problem hiding this comment.
It seems there is no integration with setCoreForwarding("mpls") inside the AS module. This logic remains independent and fails to replace mpls.enableOn(asn).
| def getName(self) -> str: | ||
| return 'Ospf' | ||
|
|
||
| def setAsMode(self, asn: int, mode: str) -> Ospf: |
There was a problem hiding this comment.
This presents the same issue. This API should have been replaced by AutonomousSystem.setOspfMode().
…rticipation update
3 participants
No description provided.