Analysis: changes needed to cope with #830

[ipspace]

830 changed the way global VRF data is merged with node VRF data. The node VRF data structures contain just parts of global VRF data during most of the transformation process, which means that some other module using "node or global VRF" logic might use incorrect values.

We need to find all references to node.vrfs executed before VRF post_link_transform hook and fix them to use get_node_vrf_data abstraction function.

[ipspace]

Modules that are transformed after VRF module

ack --yaml 'transform_after.*vrf' netsim/modules -l
netsim/modules/gateway.yml
netsim/modules/evpn.yml
netsim/modules/ospf.yml
netsim/modules/eigrp.yml
netsim/modules/vxlan.yml
netsim/modules/isis.yml

BGP is transformed before VRF

[ipspace]

Files referencing 'vrfs' in one way or another:

netsim/augment/groups.py
netsim/extra/ebgp.utils/plugin.py
netsim/extra/proxy-arp/plugin.py
netsim/modules/vrf.py
netsim/modules/evpn.py
netsim/modules/bgp.py
netsim/modules/_dataplane.py
netsim/modules/mpls.py
netsim/modules/__init__.py
netsim/modules/_routing.py
netsim/data/validate.py
netsim/devices/junos.py
netsim/devices/arubacx.py

• The devices files are OK, those are called late in the transformation process.
• Plugins are OK, both of them use post_transform hook, and plugin post-transform hook executes after module post-transform hooks
• Groups does group node_data manipulation and copies that into nodes, it's safe.
• Validate routines don't care what they're dealing with.
• VRF module (hopefully) knows what it's doing ;)
• _dataplane.py references VRFs only in a comment and deals with ID sets and references. Its reference routines just check the presence of objects not their contents, so it's safe

That leaves us with EVPN, BGP, MPLS modules, and __init__ and _routing routines.

[ipspace]

EVPN:

• Mostly uses global VRFs
• Checks whether node VRFs contain transit_vni or bundle (correct, should not use merged data)
• Changes node VRF EVPN values in set_local_evpn_rd. Called in node_post_transform
• Uses node VRF EVPN values in check_node_vrf_irb and check_node_vrf_bundle. Called in node_post_transform

EVPN post-transform is executed after VRF post-transform. The module is OK as-is.

[ipspace]

MPLS:

• Uses node VRFs only in prune_mplsvpn_af called from node_adjust_mplsvpn called from post_transform hook.

MPLS post-transform is executed after VRF post-transform. The module is OK as-is.

[ipspace]

☣️ modules __init__ uses node VRF data in copy_node_data_into_interfaces which is called before module post-transform hooks, so it's potentially dangerous. Have to create a test case (example: OSPF area for VRF interfaces) to prove it.

[ipspace]

_routing library uses node VRF data primarily in build_vrf_interface_list. It also refers to node VRF data in remove_vrf_routing_blocks and remove_unused_igp.

build_vrf_interface_list is only used by OSPF (the only VRF-aware IGP) in its post-transform hook. remove_unused_igp is used by several modules (MPLS, OSPF, IS-IS) in post-transform hooks. remove_vrf_routing_blocks is used by OSPF and BGP.

☣️ The only questionable reference is remove_vrf_routing_blocks in BGP. Let's try to build a test case ;)

[ipspace]

☣️ BGP module uses node VRF data to build per-VRF neighbor list. The only race condition could arise if the global VRF has bgp set to False.

[ipspace]

Update: the VRF module copies global- into VRF data in the post_link_transform hook. It's OK to use VRF data in post-transform hook.