ipspace / netlab

Making virtual networking labs suck less
https://netlab.tools
Other
458 stars 69 forks source link

Adjust link addressing for subnets with small prefix sizes #455

Closed jbemmel closed 2 years ago

jbemmel commented 2 years ago

For LAN links, the absolute node.id is used as an index into the prefix: https://github.com/ipspace/netlab/blob/dev/netsim/augment/links.py#L320

This can lead to issues e.g. in case of the following topology: (edited)

defaults.device: srlinux

provider: clab

module: [ vlan ]

addressing:
  lan: # Default pool used for vlans, together with 'vlan'
    ipv4: 192.0.0.0/24  # Carve out /31 point-to-point prefixes from this range
    prefix: 31

vlans:
  routed-with-p2p-addressing:
    mode: route

  some-other: # to avoid 'thou shalt not use routed access vlans'
    mode: bridge

nodes:
  r1:
  r2:
  r3:

links:
- r1:
  r2:
  vlan.trunk: [ routed-with-p2p-addressing, some-other ]

- r2:
  r3:
  vlan.trunk: [ routed-with-p2p-addressing, some-other ]

This attempts to assign /31 IPs to each pair of IPs on routed vlan links, but results in an error:

jeroen@jvm:~/srlinux/netlab/tests$ netlab create -o json topology/input/todo/vlan-routed-p2p-addressing.yml 
IncorrectValue in links: Cannot assign ipv4 address from prefix 192.0.0.6/31 to node r3 with ID 3
... index out range for address range size!
... link data: {'linkindex': 5, 'parentindex': 2, 'vlan': {'access':
      'routed-with-p2p-addressing', 'mode': 'route'}, 'vlan_name':
      'routed-with-p2p-addressing', 'type': 'vlan_member',
      'interfaces': [{'node': 'r2', '_selfloop_ifindex': 0, 'vlan':
      {'access': 'routed-with-p2p-addressing', 'mode': 'route'},
      'ipv4': '192.0.0.7/31', 'ifindex': 5}, {'node': 'r3',
      '_selfloop_ifindex': 1, 'vlan': {'access': 'routed-
      with-p2p-addressing', 'mode': 'route'}}], 'node_count': 2,
      'bridge': 'todo_5', 'prefix': {'ipv4': '192.0.0.6/31'}}
Fatal error in netlab: Cannot proceed beyond this point due to errors, exiting
ipspace commented 2 years ago

I never thought I'd have to write this, but here we go (and maybe I'll have to use it in the future for the "this is how you open an issue" guideline). There are two reasons I can see why someone would open an issue on GitHub:

(A) To make a note of what he needs to do in the future (B) To prompt someone else to take an action (for example, fix what might be a bug, or add a new feature).

I don't care what goes into issues of the (A) type, but if someone expects a bug to be fix, the very minimum I'd expect is a minimum working topology that makes it feasible to reproduce the bug without second-guessing what the reporter was doing. Please note the "minimum" and "working" adjectives.

What you submitted is not a working topology. It's a definition of addressing pool and a VLAN. Please reduce your topology to bare essentials that reproduce the problem and submit that.

jbemmel commented 2 years ago

I did create some code to fix this, but it became a complete rewrite of the ip allocation routines for LAN links: https://github.com/ipspace/netlab/compare/dev...jbemmel:netlab:refactor-lan-ip-allocation. I wrote that also to catch cases like https://github.com/ipspace/netlab/issues/444 - someone might statically allocate IPs to a subset of nodes, which could overlap with automatic node.id based allocation.

One could argue that people shouldn't do that to begin with - but once we get to larger, modular designs where - for example - a pod design uses anycast gateway IPs of .1, a more flexible approach to automatic IP allocation could be useful

A smaller change to achieve the same, is to use a relative index into a list of nodes on that link (instead of node.id)

ipspace commented 2 years ago

Well, I like having consistent IP addresses (derived from node ID), but of course if someone insists on smaller prefixes, something is going to break. We could use an allocation method where we use node ID if the prefix is large enough, and position in the interface list if the prefix is too small.

Then we have the special cases:

Finally, as I don't think anyone uses the extra bits that P2P links get, this change might bring us to the point where we could merge the LAN and P2P code.

However, all the changes have to be thoroughly documented in links and addressing, plus additional examples in the addressing tutorial. If you feel like doing all of that, please go for it.

ipspace commented 2 years ago

Working on a complete refactoring of link augmentation code, WIP in 'addr' branch.

ipspace commented 2 years ago

Notes:

ipspace commented 2 years ago

Fixed in 2556439c6a05f94fb8d152da2409c6fdd0533a1f