-
Notifications
You must be signed in to change notification settings - Fork 162
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
add support to generate seed simulation file to scion topology tool #4508
base: master
Are you sure you want to change the base?
Conversation
… isd code generation, start as code generation)
…aml file instead of using the given data structure
cool ! we thought about something very similar at OvGu magdeburg as well for seed pr181 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 7 files at r1, 2 of 3 files at r2, all commit messages.
Reviewable status: 6 of 7 files reviewed, 21 unresolved discussions (waiting on @Bruol)
tools/topogen.py
line 52 at r2 (raw file):
help='Generate a SIG per AS (only available with -d, the SIG image needs\ to be built manually e.g. when running acceptance tests)') parser.add_argument('--features', help='Feature flags to enable, a comma separated list\
Keep --features
flag as last flag.
tools/topology/seed.py
line 71 at r2 (raw file):
_skipIPv6Check: bool = False _parentNetwork: str = "10.3.0.0/16" _brProperties: Dict[str, Dict[str, Dict]] # dict containing maping from ISD_AS to list of border router properties
Suggestion:
mapping
tools/topology/seed.py
line 268 at r2 (raw file):
return isd_num, as_num, br_if def _parse_links(self) -> Dict:
Check the type annotations.
tools/topology/seed.py
line 360 at r2 (raw file):
a_isd = link['a'][0] b_as = link['b'][1] b_isd = link['b'][0]
Suggestion:
a_isd, a_as, a_br = link['a']
b_isd, b_as, b_br = link['b']
tools/topology/seed.py
line 380 at r2 (raw file):
for i in range(0, len(self._links)): link = self._links[i] a_br = link['a'][2]
Same as above
tools/topology/seed.py
line 405 at r2 (raw file):
# replace border router interface names with border router names in brProperties new_brProperties = {}
Don't mix CamelCase into snake_case.
Suggestion:
new_br_properties
tools/topology/seed.py
line 447 at r2 (raw file):
(as_num, isd_num, is_core, cert_issuer, as_int_bw, as_int_lat, as_int_drop, as_int_mtu, as_note) = self._parse_AS_properties(As) code += f"\n# AS-{as_num}\n"
This might be more readable if you have a base template string with placeholders which you then (conditionally) format.
topology/README.md
line 10 at r2 (raw file):
- [Links Section](#links-section) - [borderRouterProperties Section](#border-router-properties-section) - [Examples](#examples-section)
Reference does not match target
Suggestion:
#examples
topology/README.md
line 25 at r2 (raw file):
- "issuing" -- boolean, whether the AS is an issuing AS - "underlay" -- default is UDP/IPv4, can be set to UDP/IPv6, seed does not support IPv6 underlay - "cert_issuer" -- string, the issuer TRC this attribute is necessary if AS is not core
This is wrong.
Suggestion:
the IA of the CA. This
topology/README.md
line 26 at r2 (raw file):
- "underlay" -- default is UDP/IPv4, can be set to UDP/IPv6, seed does not support IPv6 underlay - "cert_issuer" -- string, the issuer TRC this attribute is necessary if AS is not core - "MTU" -- integer, the internal MTU of the AS used by seed emulator
This is not specific to the SEED-Emulator topology generation. Mark fields that are used by Seed for link properties and mention it at the end.
Suggestion:
. *
topology/README.md
line 31 at r2 (raw file):
- "drop" -- float, the internal drop rate (% in range(0.0,1.0)) of the AS used by seed emulator - "note" -- string, a note for the AS seed emulator will include this in the beacons
Fields marked with * are used by the SEED-Emulator for setting link properties.
topology/README.md
line 60 at r2 (raw file):
**Supported attributes:** - "a" -- string, necessary, see above
Suggestion:
mandatory
topology/README.md
line 61 at r2 (raw file):
- "a" -- string, necessary, see above - "b" -- string, necessary, see above
Suggestion:
mandatory
topology/README.md
line 62 at r2 (raw file):
- "a" -- string, necessary, see above - "b" -- string, necessary, see above - "linkAtoB" -- string, necessary, the type of link, can be CORE, PEER, CHILD
Suggestion:
mandatory
topology/README.md
line 65 at r2 (raw file):
- "mtu" -- integer, the MTU of the link - "underlay" -- default is UDP/IPv4, can be set to UDP/IPv6, seed doesn't support IPv6 - "bw" -- integer, the bandwidth in bit/s of the link used by seed emulator
Same as above
topology/README.md
line 71 at r2 (raw file):
## Border Router Properties Section The **opitonal** 'borderRouterProperties' section describes properties of BRs such as Geolocation.
typo
Suggestion:
**optional**
topology/README.md
line 74 at r2 (raw file):
Entries in the 'borderRouterProperties' section are optional. This means not every BR defined in the links section must appear in the 'borderRouterProperties' section. To specify a border router one can use the same string identifiers used in the link section as a key.
Suggestion:
The same string identifiers as in the link section specify the key for a border router.
topology/README.md
line 75 at r2 (raw file):
To specify a border router one can use the same string identifiers used in the link section as a key. Though watch out as one border router can have several scion interfaces but there can only be one property section for each border router.
Suggestion:
SCION
topology/README.md
line 88 at r2 (raw file):
Notice though how the 6 scion interfaces are connected to only 3 BorderRouters. Now in the 'borderRotuerProperties' section we can specify properties for each one of the three BorderRouters like this:
typo
Suggestion:
'borderRouterProperties'
topology/tiny4_link_properties.topo
line 1 at r2 (raw file):
--- # Tiny Topology, IPv4 Only
Suggestion:
Tiny Topology with link properties
topology/tiny_borderRouterProperties.topo
line 1 at r2 (raw file):
--- # Tiny Topology
Suggestion:
Tiny Topology with border router properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 6 of 7 files reviewed, 19 unresolved discussions (waiting on @FR4NK-W)
tools/topogen.py
line 52 at r2 (raw file):
Previously, FR4NK-W wrote…
Keep
--features
flag as last flag.
Done.
tools/topology/seed.py
line 71 at r2 (raw file):
_skipIPv6Check: bool = False _parentNetwork: str = "10.3.0.0/16" _brProperties: Dict[str, Dict[str, Dict]] # dict containing maping from ISD_AS to list of border router properties
Done.
tools/topology/seed.py
line 268 at r2 (raw file):
Previously, FR4NK-W wrote…
Check the type annotations.
Done.
tools/topology/seed.py
line 360 at r2 (raw file):
a_isd = link['a'][0] b_as = link['b'][1] b_isd = link['b'][0]
Done.
tools/topology/seed.py
line 380 at r2 (raw file):
Previously, FR4NK-W wrote…
Same as above
Done.
tools/topology/seed.py
line 405 at r2 (raw file):
Previously, FR4NK-W wrote…
Don't mix CamelCase into snake_case.
Done.
tools/topology/seed.py
line 447 at r2 (raw file):
Previously, FR4NK-W wrote…
This might be more readable if you have a base template string with placeholders which you then (conditionally) format.
I agree that this would be more readable, though I dont quite understand how exactly I would do this as a lot of the lines, like the the border router creation, are generated dynamically, as the number of border routers is not known in advance.
topology/README.md
line 10 at r2 (raw file):
Previously, FR4NK-W wrote…
Reference does not match target
Done.
topology/README.md
line 25 at r2 (raw file):
Previously, FR4NK-W wrote…
This is wrong.
Done.
topology/README.md
line 26 at r2 (raw file):
Previously, FR4NK-W wrote…
This is not specific to the SEED-Emulator topology generation. Mark fields that are used by Seed for link properties and mention it at the end.
Done.
topology/README.md
line 31 at r2 (raw file):
Previously, FR4NK-W wrote…
Fields marked with * are used by the SEED-Emulator for setting link properties.
Done.
topology/README.md
line 60 at r2 (raw file):
**Supported attributes:** - "a" -- string, necessary, see above
Done.
topology/README.md
line 61 at r2 (raw file):
- "a" -- string, necessary, see above - "b" -- string, necessary, see above
Done.
topology/README.md
line 62 at r2 (raw file):
- "a" -- string, necessary, see above - "b" -- string, necessary, see above - "linkAtoB" -- string, necessary, the type of link, can be CORE, PEER, CHILD
Done.
topology/README.md
line 65 at r2 (raw file):
Previously, FR4NK-W wrote…
Same as above
Done.
topology/README.md
line 74 at r2 (raw file):
Entries in the 'borderRouterProperties' section are optional. This means not every BR defined in the links section must appear in the 'borderRouterProperties' section. To specify a border router one can use the same string identifiers used in the link section as a key.
Done.
topology/README.md
line 75 at r2 (raw file):
To specify a border router one can use the same string identifiers used in the link section as a key. Though watch out as one border router can have several scion interfaces but there can only be one property section for each border router.
Done.
topology/tiny4_link_properties.topo
line 1 at r2 (raw file):
--- # Tiny Topology, IPv4 Only
Done.
topology/tiny_borderRouterProperties.topo
line 1 at r2 (raw file):
--- # Tiny Topology
Done.
2) added OSPF to seed emulation to make specific topologies work 3) added dump feature to emulation to dump emulation before rendering
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 7 files reviewed, 19 unresolved discussions (waiting on @FR4NK-W)
tools/topology/seed.py
line 447 at r2 (raw file):
Previously, Bruol (Lorin Urbantat) wrote…
I agree that this would be more readable, though I dont quite understand how exactly I would do this as a lot of the lines, like the the border router creation, are generated dynamically, as the number of border routers is not known in advance.
I've tried to implement this now. Though I've only managed to make it marginally better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 7 files at r3, all commit messages.
Reviewable status: all files reviewed, 19 unresolved discussions (waiting on @FR4NK-W)
… changing link properties dynamicially
…ternal network prefixes
Overview:
As part of my Bachelor Thesis on creating a SCION Traffic Simulator with Beacon Meta-data, I made some changes and additions to the scion topology tool in order to make it possible to generate a seed emulation of a topology given in
.topo
format.To generate a seed emulation I added a
-s
argument to the topology tool. When this argument is given, agen/scion-seed.py
file will be created that specifies the emulation. On top of that agen/seed-compiled
folder will be created that contains the compiled seed emulation in the form of a docker-compose file.On top of this I added support for an optional
borderRouterProperties
Section in the.topo
files. Here one can specify border router properties such as geolocation and Note. Check out thetopology/tiny_borderRouterProperties.topo
, to see an example of this.Limitations:
In order to generate a seed emulation for the
tiny4.topo
example the following command works:./scion.sh topology -c topology/tiny4.topo -s
Important
In order to generate a seed-emulation a modified version of seed is needed. At the moment I am waiting for these changes to be merged into the main seed-emulator repository. For this reason I also have not yet updated the dependencies in this repo as the main seed-emulator repository does not contain the neccessary changes yet. You can check on the progress of the Pull-Request here seed-labs/seed-emulator#212
I tried to adhere to the general style of the topology tool when writing my code. But I am happy about any suggestions that would improve the code.
Disclaimer:
Parts of this Code were created with Github Copilot autocompletion enabled
Also Thanks to my two Supervisors François Wirz and Jordi Subirà Nieto who have given me guidance in this project