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

add support to generate seed simulation file to scion topology tool #4508

Draft
wants to merge 39 commits into
base: master
Choose a base branch
from

Conversation

Bruol
Copy link

@Bruol Bruol commented Apr 18, 2024

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, a gen/scion-seed.py file will be created that specifies the emulation. On top of that a gen/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 the topology/tiny_borderRouterProperties.topo, to see an example of this.

Limitations:

  • seed does not support IPv6. because of this the seed file generation only works for Topologies that have no UDP/IPv6 underlay networks.
    • The IPv6 check can be skipped by setting the --features SeedSkipIPv6Check argument. If this is done the topology tool will treat IPv6 underlay networks as if they were IPv4 networks

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

@matzf
Copy link
Member

matzf commented Apr 18, 2024

This change is Reviewable

@FR4NK-W FR4NK-W marked this pull request as draft April 18, 2024 12:37
@FR4NK-W FR4NK-W self-requested a review April 18, 2024 12:37
@amdfxlucas
Copy link

cool ! we thought about something very similar at OvGu magdeburg as well for seed pr181

Copy link
Contributor

@FR4NK-W FR4NK-W left a 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

Copy link
Author

@Bruol Bruol left a 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.

Copy link
Author

@Bruol Bruol left a 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

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.

None yet

4 participants