Skip to content

Commit 9aac9ca

Browse files
ipspaceipspace
authored
Change the management MAC calculation, fix other large-node-id quirks (ipspace#2610)
* The management MAC address is calculated using the old algorithm for node IDs below 255 (to keep Cumulus NVUE happy) and the "base + id" approach for node IDs above 255. * The _dataplane.get_next_id function accepts a hint that is used by node ID allocation code to point the user to the system constant that needs to be changed. * Validation code for integers allows min_value and max_value to be specified in defaults.const dictionary. That functionality is used to validate node ID against the topology defaults. Finally, added a transformation test with a large node ID. We will add more features to it as we discover what else can go wrong. Fixes ipspace#2606
1 parent 06becc0 commit 9aac9ca

File tree

9 files changed

+181
-15
lines changed

9 files changed

+181
-15
lines changed

‎netsim/augment/components.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from .. import data
1515
from . import nodes,links
1616
from ..data.types import must_be_dict,must_be_list,must_be_string,must_be_id
17+
from ..data import global_vars
1718

1819
'''
1920
Validate topology components:
@@ -87,6 +88,8 @@ def validate_include(n_name: str, n_data: Box, topology: Box) -> bool:
8788
Also: check that the total number of nodes does not exceed the maximum allowed
8889
'''
8990
def include_nodes(n_name: str, c_data: Box, topology: Box) -> None:
91+
MAX_NODE_ID = global_vars.get_const('MAX_NODE_ID',150)
92+
9093
for inc_name,inc_data in c_data.nodes.items():
9194
node_name = f'{n_name}_{inc_name}'
9295
must_be_id(
@@ -108,7 +111,7 @@ def include_nodes(n_name: str, c_data: Box, topology: Box) -> None:
108111
else:
109112
topology.nodes[node_name] = data.get_box(inc_data) # Regular included node, just copy it
110113
topology.nodes[node_name].name = node_name # Fix the name of the newly-generated node
111-
if len(topology.nodes) > nodes.MAX_NODE_ID:
114+
if len(topology.nodes) > MAX_NODE_ID:
112115
log.fatal(
113116
'Exceeded maximum node limit while adding node {node_name}',
114117
module='components',

‎netsim/augment/nodes.py‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
from ..data import global_vars,is_true_int
2121
from ..modules._dataplane import extend_id_set,is_id_used,set_id_counter,get_next_id
2222

23-
MAX_NODE_ID: int
24-
2523
"""
2624
Reserve a node ID, for example for gateway ID, return True if successful, False if duplicate
2725
"""
@@ -38,9 +36,6 @@ def reserve_id(n_id: int) -> bool:
3836
"""
3937

4038
def create_node_dict(nodes: Box) -> Box:
41-
global MAX_NODE_ID # Max node ID has to be initialized early in the transformation
42-
MAX_NODE_ID = global_vars.get_const('MAX_NODE_ID',250)
43-
4439
if isinstance(nodes,dict):
4540
node_dict = nodes
4641
else:
@@ -118,6 +113,14 @@ def augment_mgmt_if(node: Box, defaults: Box, addrs: typing.Optional[Box]) -> No
118113

119114
node.mgmt.ifname = mgmt_if
120115

116+
# The node management MAC address can be specified as a static attribute (in which case
117+
# it's parsed and accepted as-is), or generated from the addressing.mgmt.mac prefix
118+
#
119+
# The MAC prefix for node IDs lower than 256 is generated by inserting the node ID
120+
# into the 4th octet of the MAC address to retain the backward compatibility triggered
121+
# by Cumulus NVUE peculiarity. For node IDs above 255, the node ID is added to the
122+
# base MAC address.
123+
#
121124
if 'mac' in node.mgmt: # Check static management MAC address
122125
try:
123126
node.mgmt.mac = netaddr.EUI(node.mgmt.mac).format(netaddr.mac_unix_expanded)
@@ -128,8 +131,8 @@ def augment_mgmt_if(node: Box, defaults: Box, addrs: typing.Optional[Box]) -> No
128131
category=log.IncorrectValue,
129132
module='nodes')
130133
elif addrs and addrs.mac_eui: # ... or assign one from the pool
131-
addrs.mac_eui[3] = node.id # ... using a difference in the 4th octet, not the last one #1954
132-
node.mgmt.mac = addrs.mac_eui.format(netaddr.mac_unix_expanded)
134+
mgmt_mac = netaddr.EUI(int(addrs.mac_eui) + node.id * (65536 if node.id < 256 else 1))
135+
node.mgmt.mac = mgmt_mac.format(netaddr.mac_unix_expanded)
133136

134137
# If the mgmt ipaddress is statically set (IPv4 or IPv6 address) and there are no
135138
# other parameters, skip the address assignment part
@@ -472,7 +475,8 @@ def augment_node_device_data(n: Box, topology: Box) -> None:
472475
* set management IP and MAC addresses
473476
'''
474477
def transform(topology: Box, defaults: Box, pools: Box) -> None:
475-
global MAX_NODE_ID
478+
MAX_NODE_ID = global_vars.get_const('MAX_NODE_ID',150)
479+
476480
for name,n in topology.nodes.items():
477481
if not must_be_int(n,'id',f'nodes.{name}',module='nodes',min_value=1,max_value=MAX_NODE_ID):
478482
continue
@@ -487,7 +491,7 @@ def transform(topology: Box, defaults: Box, pools: Box) -> None:
487491
set_id_counter('node_id',1,MAX_NODE_ID)
488492
for name,n in topology.nodes.items():
489493
if not 'id' in n:
490-
n.id = get_next_id('node_id')
494+
n.id = get_next_id('node_id',hint='Change defaults.const.MAX_NODE_ID parameter to increase the maximum node ID')
491495

492496
if not n.name: # pragma: no cover (name should have been checked way before)
493497
log.fatal(f"Internal error: node does not have a name {n}",'nodes')

‎netsim/data/types.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,11 @@ def check_int_type(
502502
if isinstance(value,bool): # but not a bool
503503
return { '_value': 'a true integer (not a bool)' }
504504

505+
if isinstance(max_value,str):
506+
max_value = resolve_const_value(max_value,None)
507+
if isinstance(min_value,str):
508+
min_value = resolve_const_value(min_value,None)
509+
505510
if isinstance(min_value,int) and isinstance(max_value,int):
506511
if value < min_value or value > max_value:
507512
return { '_value': f'an integer between {min_value} and {max_value}' }

‎netsim/defaults/attributes.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ node:
9191
_subtype: id
9292
device: device
9393
box: str
94-
id: { type: int, min_value: 1, max_value: 150 }
94+
id: { type: int, min_value: 1, max_value: MAX_NODE_ID }
9595
config:
9696
type: dict
9797
_subtype:

‎netsim/defaults/const.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ multi_provider: [ libvirt, clab ]
77
ifname:
88
neighbors: 5
99
maxlength: 255
10+
MAX_NODE_ID: 150

‎netsim/modules/_dataplane.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ def set_id_counter(name: str, start: int, max_value: int = 4096) -> int:
8383

8484
return start
8585

86-
def get_next_id(name: str) -> int:
86+
def get_next_id(name: str, hint: typing.Optional[str] = None) -> int:
8787
idvar = global_vars.get(f'{name}_id')
8888
if not 'next' in idvar:
8989
log.fatal(f'Initial {name} value is not set, get_next_id failed')
@@ -94,10 +94,12 @@ def get_next_id(name: str) -> int:
9494

9595
idvar.next = idvar.next + 1
9696
if idvar.next > idvar.max:
97-
log.fatal(
97+
log.error(
9898
f'Ran out of {name} values, next value would be greater than {idvar.max}',
9999
module='dataplane',
100-
header=True)
100+
category=log.FatalError,
101+
more_hints=hint)
102+
log.exit_on_error()
101103

102104
"""
103105
validate_object_reference_list

‎tests/errors/node-max-id.log‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
1-
Fatal error in dataplane: Ran out of node_id values, next value would be greater than 4
1+
FatalError in dataplane: Ran out of node_id values, next value would be greater than 4
2+
... Change defaults.const.MAX_NODE_ID parameter to increase the maximum node ID
3+
Fatal error in netlab: Cannot proceed beyond this point due to errors, exiting

‎tests/topology/expected/node-large-id.yml‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
input:
2+
- topology/input/node-large-id.yml
3+
- package:topology-defaults.yml
4+
links:
5+
- _linkname: links[1]
6+
bridge: input_1
7+
interfaces:
8+
- ifindex: 1
9+
ifname: eth1
10+
ipv4: 172.16.0.1/24
11+
node: r1
12+
- ifindex: 1
13+
ifname: eth1
14+
ipv4: 172.16.0.2/24
15+
node: r2
16+
- ifindex: 1
17+
ifname: eth1
18+
ipv4: 172.16.0.3/24
19+
node: r3
20+
linkindex: 1
21+
name: Standard LAN link (happens to be v6only)
22+
node_count: 3
23+
prefix:
24+
ipv4: 172.16.0.0/24
25+
type: lan
26+
name: input
27+
nodes:
28+
r1:
29+
af:
30+
ipv4: true
31+
box: none
32+
device: none
33+
id: 7
34+
interfaces:
35+
- bridge: input_1
36+
ifindex: 1
37+
ifname: eth1
38+
ipv4: 172.16.0.1/24
39+
linkindex: 1
40+
name: Standard LAN link (happens to be v6only)
41+
neighbors:
42+
- ifname: eth1
43+
ipv4: 172.16.0.2/24
44+
node: r2
45+
- ifname: eth1
46+
ipv4: 172.16.0.3/24
47+
node: r3
48+
type: lan
49+
loopback:
50+
ifindex: 0
51+
ifname: Loopback0
52+
ipv4: 10.0.0.7/32
53+
neighbors: []
54+
type: loopback
55+
virtual_interface: true
56+
mgmt:
57+
ifname: eth0
58+
ipv4: 192.168.64.107
59+
mac: 08:4f:a9:07:00:00
60+
name: r1
61+
r2:
62+
af:
63+
ipv4: true
64+
box: none
65+
device: none
66+
id: 300
67+
interfaces:
68+
- bridge: input_1
69+
ifindex: 1
70+
ifname: eth1
71+
ipv4: 172.16.0.2/24
72+
linkindex: 1
73+
name: Standard LAN link (happens to be v6only)
74+
neighbors:
75+
- ifname: eth1
76+
ipv4: 172.16.0.1/24
77+
node: r1
78+
- ifname: eth1
79+
ipv4: 172.16.0.3/24
80+
node: r3
81+
type: lan
82+
loopback:
83+
ifindex: 0
84+
ifname: Loopback0
85+
ipv4: 10.0.1.44/32
86+
neighbors: []
87+
type: loopback
88+
virtual_interface: true
89+
mgmt:
90+
ifname: eth0
91+
ipv4: 192.168.65.144
92+
mac: 08:4f:a9:00:01:2c
93+
name: r2
94+
r3:
95+
af:
96+
ipv4: true
97+
box: none
98+
device: none
99+
id: 1
100+
interfaces:
101+
- bridge: input_1
102+
ifindex: 1
103+
ifname: eth1
104+
ipv4: 172.16.0.3/24
105+
linkindex: 1
106+
name: Standard LAN link (happens to be v6only)
107+
neighbors:
108+
- ifname: eth1
109+
ipv4: 172.16.0.1/24
110+
node: r1
111+
- ifname: eth1
112+
ipv4: 172.16.0.2/24
113+
node: r2
114+
type: lan
115+
loopback:
116+
ifindex: 0
117+
ifname: Loopback0
118+
ipv4: 10.0.0.1/32
119+
neighbors: []
120+
type: loopback
121+
virtual_interface: true
122+
mgmt:
123+
ifname: eth0
124+
ipv4: 192.168.64.101
125+
mac: 08:4f:a9:01:00:00
126+
name: r3
127+
provider: libvirt

‎tests/topology/input/node-large-id.yml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#
2+
# Adjust standard address pools to allow large node IDs
3+
#
4+
addressing:
5+
loopback.ipv4: 10.0.0.0/22
6+
mgmt.ipv4: 192.168.64.0/22
7+
8+
defaults.device: none
9+
defaults.const.MAX_NODE_ID: 1024
10+
11+
nodes:
12+
r1:
13+
id: 7
14+
r2:
15+
id: 300
16+
r3:
17+
18+
links:
19+
- name: Standard LAN link (happens to be v6only)
20+
r1:
21+
r2:
22+
r3:

0 commit comments

Comments
 (0)