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

VM Networking improvements #237

Open
1 of 5 tasks
hoh opened this issue Oct 4, 2022 · 4 comments
Open
1 of 5 tasks

VM Networking improvements #237

hoh opened this issue Oct 4, 2022 · 4 comments
Labels
wip Work in progress...

Comments

@hoh
Copy link
Member

hoh commented Oct 4, 2022

With contributions from @tomribbens

Notes

  • Using the same IPv4 range inside VMs on all tap networks might be possible.
  • Using the same Tap interface for all VMs of a user’s cluster would be ideal.

Include (and improve) the cleanup of network configuration on VM stop and on supervisor crash/start.

@hoh hoh added the wip Work in progress... label Oct 4, 2022
@hoh
Copy link
Member Author

hoh commented Oct 5, 2022

Solving this issue as well would be handy: #18

@tomribbens
Copy link
Contributor

I've been looking at point 1, on making the ip addresses RFC1918 compliant. The easy way obviously would be to limit the vm_id generated to be limited to 4096 (assigning /24 networks out of 172.16.0.0/12 gives 12 bits to work with) and then modifying the guest_ip and host_ip functions in firecracker/microvm.py to f"172.{16 + self.vm_id // 256}.{self.vm_id % 256}.1".

There are 2 issues with that:

  • This puts a hard limit on amount of VMs at 4096, which I understand is bordering on potentially not enough
  • This is not flexible at all if in the future a change to another address pool is wanted

I would propose as a solution to make two new settings, VM_ADDRESS_POOL and VM_NETWORK_SIZE. This would make everything more flexible on a host per host basis.

However, this would mean passing those settings to the MicroVM class, which doesn't seem to be cleanest either. Therefor I would propose to move the calculation of the ip addresses out of firecracker/microvm.py and into functions called by the create_a_vm() function. This seems cleaner as the VM logic itself has no business determining the IP addresses, but should be a function in the supervisor anyway.

Additionally, this would make it easier in the future when looking at using a single tap interface per user, as at that point more knowledge about other VMs is required to determine which IP address to hand out, which is clearly a supervisor task.

@hoh
Copy link
Member Author

hoh commented Oct 6, 2022

That reads like the right way to go ! 👍

tomribbens added a commit to tomribbens/aleph-vm that referenced this issue Oct 18, 2022
Since iptables is deprecated, a move to the replacement nftables was implemented in this change. This is in line with various network improvements that are ongoing as discussed in Issue aleph-im#237. The use of nftables will facilitate some future enhancements, especially related to IPv6 support.

At the same time, this commit refactors all the networking and firewall code into its own package "network", as multiple packages were depending on making network changes. The new package is now an interface for all networking related tasks.
tomribbens added a commit to tomribbens/aleph-vm that referenced this issue Oct 18, 2022
Since iptables is deprecated, a move to the replacement nftables was implemented in this change. This is in line with various network improvements that are ongoing as discussed in Issue aleph-im#237. The use of nftables will facilitate some future enhancements, especially related to IPv6 support.

At the same time, this commit refactors all the networking and firewall code into its own package "network", as multiple packages were depending on making network changes. The new package is now an interface for all networking related tasks.
tomribbens added a commit to tomribbens/aleph-vm that referenced this issue Oct 18, 2022
Since iptables is deprecated, a move to the replacement nftables was implemented in this change. This is in line with various network improvements that are ongoing as discussed in Issue aleph-im#237. The use of nftables will facilitate some future enhancements, especially related to IPv6 support.

At the same time, this commit refactors all the networking and firewall code into its own package "network", as multiple packages were depending on making network changes. The new package is now an interface for all networking related tasks.
@tomribbens
Copy link
Contributor

Did some research into sharing the tap interface between multiple hosts. As I understand it currently, this would require the tap interface to be set with the option multi_queue, but this currently is not supported by Firecracker:

firecracker-microvm/firecracker#750

hoh added a commit that referenced this issue Nov 21, 2022
Refactor: Move from iptables to nft as iptables is deprecated

Since iptables is deprecated, a move to the replacement nftables was implemented in this change. This is in line with various network improvements that are ongoing as discussed in Issue #237. The use of nftables will facilitate some future enhancements, especially related to IPv6 support.

At the same time, this commit refactors all the networking and firewall code into its own package "network", as multiple packages were depending on making network changes. The new package is now an interface for all networking related tasks.

Co-authored-by: Hugo Herter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wip Work in progress...
Projects
None yet
Development

No branches or pull requests

2 participants