Skip to content
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.

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

support for topology datasets #181

Open
wants to merge 13 commits into
base: development
Choose a base branch
from

Conversation

amdfxlucas
Copy link
Contributor

Intra domain topologies for ASes

Formats

  • ORBIS
  • INET
  • BRITE
  • python networkx Graphs

Three ASes mutually connected by IXPs

See examples/scion/S06-intra-domain-topo/three_core_as_with_topo.py

three_core_ases04

internal topology of each of the AS in the above image

endhosts in read, routers in green
super_simple

DefaultScionGenerator, CaidaDataProvider, ScionTopoJsonProvider

DefaultScionGenerator

Given a DataProvider it generates a SCION AS from it

CaidaDataProvider

It can read CAIDA internet datasets of ASes and their interconnections. See examples/scion/S06-intra-domain-topo/caida_topo.py

ScionTopoJsonProvider

A data source for the topology generator that reads topology.json files from real SCION deployments. Can be used to run virtual experiments with your configuration without jeopardising the physical deployment.
See examples/scion/S06-intra-domain-topo/scion_topo.py

@wonkr
Copy link
Member

wonkr commented Mar 5, 2024

@amdfxlucas ,
Are you a member of SCION Team?
As your code is changing the scion code as well, it seems that the review from SCION team is needed.

@amdfxlucas
Copy link
Contributor Author

no, but the changes are in coordination with @martenwallewein

@wonkr
Copy link
Member

wonkr commented Mar 5, 2024

@amdfxlucas ,
We are extremely cautious about making modifications to the core code from a maintenance perspective. Therefore, when proposing the addition of the SCION service, we suggested creating a new class called ScionAutonomousSystem instead of modifying the existing AutonomousSystem class.

While we are open to discussions about adding new code, if significant modifications to the existing codebase are necessary, it would require a more in-depth discussion.

The proposed addition of the brnode type doesn't seem to be essential for the current SEED internet emulator. If it's required for the SCION service, we would appreciate alternative suggestions that minimize changes to the existing core code. Thank you.

@amdfxlucas
Copy link
Contributor Author

I chose to distinguish the kind of routers into border-routers and intra-domain-routers ,
because I think it is simpler and more expressive ( i.e. its really just Registry::getByType('brnode') ),
than the alternative of having to get all rnodes and then iterate over all of the individual rnode's interfaces
and checking if the network reachable through that interface is either of type InternetExchange or CrossConnect
to determine if the rnode participates in inter domain routing. (i.e. as done by the MPLS layer)

three_core_ases04_annotated

@wonkr
Copy link
Member

wonkr commented Mar 6, 2024

@amdfxlucas ,
I agree with the idea of adding a new brnode type. However, I’d like to minimize the code changes resulting from adding this brnode type.

For instance, it seems that there were significant changes in the existing code when the NodeRole class type in enum.py was changed from Enum to IntFlag.

We have two suggestions on this issue. Those suggestions aim to minimize alterations to the existing codes.

  1. Adding a new field in the exisiting class (for example, a subtype for the router nodes). => this is more acceptable, because this does not affect the other code.
  2. Keep Enum type of NodeRole class and simply add new type.

Moreover, at present, it appears that the changes due to introducing the brnode type are limited to the SCION design. While considering introducing the brnode type into the overall design may be beneficial in the future, it seems difficult to review due to limited resources at the moment.

Could you also consider moving the AutonomousSystem::makeBorderRouter method to the ScionAutonomousSystem Class method?

Furthermore, though minor, it’s worth noting that the term brnode currently serves as an abbreviation for bridge node (

brNode = self.createRouter('br-{}'.format(net.getName()))
). Would it be possible to use a different abbreviation instead of “brnode”?

@amdfxlucas
Copy link
Contributor Author

@kevin-w-du Appologies for my oversight, how about 'brdrouter' for border-routers ?

With the change to IntFlag for NodeRole I wanted to correspond to the circumstance that one node might have more than one role.
i.e. all border-routers are also normal routers.
They are listed as both 'rnode' and 'brdnode' in the emulators registry.

However with normal 'Enum' the Node::getRole() would only report a single NodeRole ('Router') that it got assigned during its construction (in AutonomousSystem::createRouter() ),
and omit the 'BorderRouter' role it took on, the moment it joined an IXP network or cross-connected to another router.

This I found unintuitive and somewhat violating the law of least surprise.

Do you suggest to create a sub-class 'BorderRouter' of 'Router(Node)' which returns 'NodeRole.BorderRouter' from its getRole() method
and do something like ' rnode.class = BorderRouter' in the AutonomousSystem::makeBorderRouter() method to graft it into a BorderRouter ?

The AutonomousSystem::makeBorderRouter() you mentioned is not specific to SCION, I just haven't touched all the EBGP examples.
But they'd need the asXYZ.makeBorderRouter() calls as well.

Another option would be to do the graft of routers to border routers automatically in Node::configure() if they happen to join an IX network or cross connect.
Here the emulator is available as a function parameter and we could re-register the node as 'brdnode'.

Node::configure is called by AutonomousSystem::configure which in turn is called by the Base layer's configure,
which is rendered at very first by the Emulator, before any other i.e. routing-layer that could depend on a distinction of routers.

In this approach the makeBorderRouter() method is obsolete.

@wonkr
Copy link
Member

wonkr commented Mar 13, 2024

@amdfxlucas
Let me explain the suggestion in detail. We are suggesting to add the concept of border router as a attribute of Router instead of NodeRole. For example, we can add an attribute to Class Router(Node) to set its role.
i.e)

Class Router(Node):
…
…
_is_border_router:bool
 
def setBorderRouter(self, is_border_router=True):
Self._is_border_router = is_border_router
Return
 
def isBorderRouter(self):
Return self._is_border_router
...

And for the AutonomousSystem:makeBorderRouter(), We agree with your idea of configuring routers to border routers internally at the configuration phase.

Appreciated for your suggestions and contributions. Please let us know if you have any suggestions or doubts.

not all routers within an AS are border routers, and hence need not all the software installed
now that there is a distinction between brnodes and rnodes
@amdfxlucas
Copy link
Contributor Author

@wonkr @kevin-w-du done ! :) please check again.. Could you please also comment on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants