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 multiple network interfaces #132

Open
8 tasks
PetrichorIT opened this issue Jul 25, 2023 · 8 comments
Open
8 tasks

Support multiple network interfaces #132

PetrichorIT opened this issue Jul 25, 2023 · 8 comments
Assignees

Comments

@PetrichorIT
Copy link
Contributor

PetrichorIT commented Jul 25, 2023

This refactor aims to introduce the ability of nodes to have multiple addresses
in distinct subnets.

Immediate Goals

  • Each node should be bound to an unique Ipv4Addr AND an unqiue Ipv6Addr
  • All bound addresses should be in a predefined subnet (like 192.168.0.0/16)
  • The available subnets should be configurable in the Builder
  • Addresses can be either automatically or manually assigned

Challenges

I have tried to implement this, and come to the conlsuion that some major changes
internally AND externally would be nessecary. Notably:

  • Nodes, and thus Rt/Host can no longer by identified by a single IpAddr.
    The best possible solution would be to identify them by something like a MAC addr,
    but that would warrent major internal changes
  • lookup would need to return more than one possible address, thus the public API
    of ToIpAddr / lookup / lookup_many would need to change. This could be a good
    moment to introduce API compliance with either std::net or tokio::net
  • using ToIpAddr for module creation creates problems when statically assigning addresses.
    Even without this refactoring, nodes with explicitly assigned address cannot have human readable
    names, since their place is traded for the address assignment. Mixed IP subnets only enlarge this
    problem. The current api of Sim::client / Sim::host provides no way to explicitly assign both
    an Ipv4 and an Ipv6 address. In short, the current API cannot support a node with explicitly assigned
    addresses, let alone a human-readable name, so changes would be nessecary.

In my opinion, this amount of changes would exceed the scope of one PR, so it might be benifical
to make step by step changes to the public API. However this warrants discussion.

Some related thoughts

  • While not in the scope of this refactoring, binding sockets to specific addresses may be beneficial
  • in std/tokio Ipv6 sockets bound to [::] can receive incoming Ipv4 packets (addresses are being mapped to Ipv6),
    however the reverse is not possible. This seems like an rare edge case, so i do not know whether we
    should ever support this behaviour
  • I might prove useful in the future, to refrain from hardcoding only two possible addresses in two possible subnets per node.
    Supporting a set of bound addresses+subnets might be beneficial to a) remodel localhost, to use top.rslinks or b) add support for multiple interface, thus multiple subnets, should that ever be a goal

As a reference, my current test implementation can be found here.
I have closed the corresponding PR #125, since it is already out of date.

Progress

  • types representing subnets
  • dns lookups that may return multiple IpAddrs
  • decoupling dns lookup and dns registration
  • uuid as Host/Rt identifers
  • updated node creation API
  • subnet configuration in Builder
  • multiple network interfaces per node (according to subnet configuration)
  • socket support for binding to specific addresses
@mcches
Copy link
Contributor

mcches commented Jul 25, 2023

Linking #79 for context, and closing it to track in one place.

@mcches
Copy link
Contributor

mcches commented Jul 25, 2023

I think we should set the goal as: Support multiple network interfaces. And then a piece of that is supporting subnets.

@mcches
Copy link
Contributor

mcches commented Jul 26, 2023

Nodes, and thus Rt/Host can no longer by identified by a single IpAddr. The best possible solution would be to identify them by something like a MAC addr, but that would warrant major internal changes.

I think we should use a unique identifier for each host, which could be a uuid. This would be an internal concept and our search capabilities would take string or IpAddr to resolve.

It'd be good to decouple the DNS "lookup" function from the network manipulation ones. So lookup should always return one IpAddr or panic, but the others could resolve ids and and manipulate the network.

And then lastly, we need to change how hosts are added to the simulation to allow for different addressing schemes. Perhaps a builder pattern works best here:

sim.host()
    .with_ipv4(...)
    ...
    .build(<software>)

In terms of iterating on this we could create a new remote so that main is available for bug fixes.

@PetrichorIT
Copy link
Contributor Author

It'd be good to decouple the DNS "lookup" function from the network manipulation ones. So lookup should always return one IpAddr or panic, but the others could resolve ids and and manipulate the network.

Since each node has two possible addresses, lookup should return a set of addresses.

I think we should use a unique identifier for each host, which could be a uuid. This would be an internal concept and our search capabilities would take string or IpAddr to resolve.

And then lastly, we need to change how hosts are added to the simulation to allow for different addressing schemes. Perhaps a builder pattern works best here:

As an idea, if create a builder pattern like this,

sim.host(<name>)
    ...
    .build(<software>)

we could ensure that each node has a human-readable name. Since these names should always be unique, we could them as a uuid. This would more expressive than a simple numeric uuid.

Additionally, we could change the API of the network manipulation functions like partition to take in a set of nodenames instead of impl ToIpAddrs, removing the need for that trait, and making the API overall simpler. This would also reduce the complexity of ToIpAddr to be only used in simple dns lookups or APIs that internally use dns lookups like TcpStream::connect.

Of course, reverse dns lookups would need to be publicly accessible, to indirectly allow network manipulation with IpAddrs, but that should not be a problem.

@mcches
Copy link
Contributor

mcches commented Jul 26, 2023

Since each node has two possible addresses, lookup should return a set of addresses.

Makes sense.

removing the need for that trait, and making the API overall simpler

We might still need a trait to support the regex use cases.

@PetrichorIT
Copy link
Contributor Author

We might still need a trait to support the regex use cases.

Yeah, but we can completly detach this trait from dns resolutions

@PetrichorIT
Copy link
Contributor Author

Basically using ToIpAddr for dns resultions, and something like ToNodeNames for network manipulation

@mcches mcches changed the title Refactoring: Adding mixed IP subnets Support multiple network interfaces Jul 26, 2023
@PetrichorIT
Copy link
Contributor Author

I think a good first step would be the integration of uuids and the internal support of more complex dns queries.
See PR #134.

On a related note, will we ever support async dns queries (see #69)? I don't really see the point, since all queries in turmoil are sync either way. Additionally async lookups would prevent use of lookup in non-async contexts, like fn main...

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

No branches or pull requests

4 participants