Fix manual_underlay_allocation with TOR switches - #866
Conversation
ccoueffe
changed the title
juarocha
left a comment
juarocha
left a comment
There was a problem hiding this comment.
LGTM
Tested on 4.1 with VPC-to-Standalone ToR. before testing the data model shows an error: rule 208: Verify IP addresses when manual_underlay_allocation is true with tor switch. After testing with this branch, no error is shown; everything is configured correctly
| {%- endif %} | ||
| {%- endfor %} | ||
| {% if not ((switch.role == 'spine' or switch.role == 'super_spine') and loopback_id == vtep_lo_id) %} | ||
| {% if switch.role != 'tor' %} |
There was a problem hiding this comment.
Medium: Existing ToR pool allocations are omitted but never released
Issue
The changed template stops rendering routing-loopback, VTEP-loopback, and vPC VIP resource-manager entries for ToR switches, but neither the create nor remove pipeline releases entries which were allocated by an earlier run. This is a state-transition defect: the desired file becomes correct while NDFC retains the old allocation.
Evidence
- The new role gate omits ToR loopback allocations at
ndfc_underlay_ip_address.j2:28, and the new peer-role gate similarly omits the vPC VIP entry at line 47. underlay_ip_addressis structurally diffed byentity_name. A focused old-versus-new fixture containing one former ToR allocation producedupdated: 0,removed: 1,equal: 0.- Both active fabric registries invoke
cisco.dcnm.dcnm_resource_managerwithstate: merged(resources/create_resources.yml, VXLAN_EVPN lines 96-108 and eBGP_VXLAN lines 286-298). In a selective run,ResourceManager._resolve_create_data()consumes onlydiff.updated; it never consumesdiff.removed. In a full run, the remote audit compares only desired entries, so an omitted controller entry is not selected either. - There is no
underlay_ip_addressstep inresources/remove_resources.yml. At both the declared floorcisco.dcnm3.12.1 (6311b66ff02b8ed593cbbfed4abdd502607dd6d2) and current inspected ref939c75bb667498631e9047ff9f62f424f00b98c2, thedcnm_resource_managercontract saysmergedleaves unspecified resources untouched anddeletedreleases resources explicitly supplied inconfig.
Practical example
Before this change, assume manual allocation created these controller resources for ToR TOR1:
- entity_name: TOR-SERIAL-1~loopback0
pool_name: LOOPBACK0_IP_POOL
resource: 10.10.0.21
- entity_name: TOR-SERIAL-1~loopback1
pool_name: LOOPBACK1_IP_POOL
resource: 10.20.0.21After upgrading, the user removes those ToR loopbacks from the model to comply with the new validation. The desired file correctly omits them, and the structural result is effectively:
updated: []
removed:
- entity_name: TOR-SERIAL-1~loopback0
- entity_name: TOR-SERIAL-1~loopback1The selective create path has nothing in diff.updated, and no remove step consumes these two entries. A forced full run still uses state: merged, so omission does not release them. NDFC therefore continues to reserve 10.10.0.21 and 10.20.0.21 for TOR1; a later attempt to reuse either address can fail even though neither appears in the current NaC model.
Existing PR overlap
No matching existing PR comment found. The visible approval tested creation from a happy-path ToR topology; it did not test migration from a previously rendered ToR allocation or the last-item-removed transition.
Impact
An environment that previously ran manual allocation with ToR loopbacks or a ToR vPC VIP can continue to reserve those IPs after upgrading to this fix. The stale reservations can exhaust pools, block reuse, or make controller state disagree with the saved NaC artifact. Both diff_run=true and a full reconciliation leave them untouched.
Suggested fix
Add an underlay-resource removal step that sends underlay_ip_address.diff.removed to cisco.dcnm.dcnm_resource_manager with state: deleted, with appropriate opt-in/safety semantics if resource release is intentionally guarded. Test one removed ToR loopback, one removed ToR vPC VIP, mixed removed-and-updated entries, the last allocation removed, a full run, a selective run, and a no-change rerun.
| cls.validate_vpc_peers_and_vtep_vip(data_model) | ||
|
|
||
| # Check if TOR switches have IPv4 on fabric links | ||
| cls.validate_tor_no_fabric_link_ipv4(data_model) |
There was a problem hiding this comment.
Medium: ToR restrictions do not run for eBGP VXLAN fabrics
Issue
The new ToR semantic checks were added only to the iBGP Rule 208 path. The modified underlay resource template is active for both VXLAN_EVPN and eBGP_VXLAN, so an eBGP model can still render prohibited ToR fabric-link allocations.
Evidence
- The new fabric-link check is called from
208_manual_ipaddress_allocation.py:84, underroles/validate/files/rules/ibgp_vxlan/. VXLAN_EVPNloadsibgp_vxlan, whileeBGP_VXLANloads its separate rule directory, which currently contains no equivalent check.ndfc_underlay_ip_address.j2is active for both fabric types. Its new ToR conditions cover loopbacks and the vPC VIP, but the fabric-link section relies on semantic validation.- A full eBGP preparation/render fixture containing a leaf-to-ToR IPv4 link emitted allocations for both endpoints.
- The authoritative model documentation supports ToR pairing in both iBGP and eBGP fabrics.
Existing PR overlap
No matching existing PR comment was found. The approval reports VXLAN_EVPN testing only and does not cover eBGP routing.
Impact
Equivalent ToR models behave differently depending on fabric type. On eBGP_VXLAN, underlay allocations can reach NDFC for a ToR that this PR says should not participate in the VXLAN underlay.
Suggested fix
Move the fabric-independent manual-allocation and ToR checks into common_vxlan, or add and maintain an equivalent eBGP rule. Also guard fabric-link rendering against ToR endpoints as defense in depth. Add positive and negative eBGP tests for omitted ToR loopbacks/VIP and prohibited ToR-involving IPv4 fabric links.
| for link in fabric_links: | ||
| src = link.get("source_device", "") | ||
| dst = link.get("dest_device", "") | ||
| if (src == peer1 and dst == peer2) or (src == peer2 and dst == peer1): |
There was a problem hiding this comment.
Medium: The new peer-link validator rejects a valid later match
Issue
validate_vpc_peer_fabric_link() stops at the first fabric-link object whose endpoints match the vPC pair. If that first object has no complete IPv4 block, it reports an error even when a later link between the same devices contains the required subnet and endpoint addresses.
Evidence
- The new search assigns the first endpoint match and immediately
breaks at208_manual_ipaddress_allocation.py:210; only that one object is checked at lines 220-223. - The source schema models
fabric_linksas a list and imposes no one-link-per-device-pair uniqueness rule. Link identity includes interfaces, so parallel links between the same switches are representable. - A focused unnumbered fixture with
fabric_peering: false, two leaf1/leaf2 link objects, no IPv4 on the first, and a complete/31IPv4 block on the second produced:Fabric link between 'leaf1' and 'leaf2' is missing IPv4 configuration....
Practical example
A vPC pair can have two modeled links between the same switches:
vxlan:
topology:
vpc_peers:
- peer1: dc1-leaf1
peer2: dc1-leaf2
fabric_peering: false
fabric_links:
# Valid unnumbered physical link
- source_device: dc1-leaf1
source_interface: Ethernet1/49
dest_device: dc1-leaf2
dest_interface: Ethernet1/49
# Numbered backup link required for underlay peering
- source_device: dc1-leaf1
source_interface: Vlan3600
dest_device: dc1-leaf2
dest_interface: Vlan3600
ipv4:
subnet: 10.5.0.0/31
source_ipv4: 10.5.0.0
dest_ipv4: 10.5.0.1The loop examines only the device names and stops at the first match:
for link in fabric_links:
if link connects dc1-leaf1 and dc1-leaf2:
peer_link = link
breakIt therefore selects Ethernet1/49, sees no ipv4 block, and reports:
Fabric link between 'dc1-leaf1' and 'dc1-leaf2' is missing IPv4 configuration
The complete Vlan3600 entry is never examined. Simply placing that entry first makes the same topology pass, so validation depends on list order rather than intent:
| Order | Result |
|---|---|
| Unnumbered link first, numbered link second | Incorrect validation failure |
| Numbered link first, unnumbered link second | Validation passes |
Existing PR overlap
No matching existing PR comment found.
Impact
Validation becomes dependent on list order and can reject a topology that contains a complete configured vPC backup link. Reordering the same two links can change the result without changing intent.
Suggested fix
Collect all endpoint matches and succeed when any matching link has subnet, source_ipv4, and dest_ipv4; report missing-link only when there are no endpoint matches and missing-IPv4 only when matches exist but none is complete. Add zero-match, one complete, one incomplete, incomplete-then-complete, complete-then-incomplete, and reversed-endpoint tests.
| switch_name = switch.get("name") | ||
| switch_role = switch.get("role", "").lower() | ||
|
|
||
| if switch_role == "tor": |
There was a problem hiding this comment.
Low: The source documentation still instructs users to configure values the PR rejects
Issue
The implementation introduces a ToR exception to manual underlay allocation, but the authoritative model documentation still says every non-spine needs routing and VTEP loopback addresses and broadly describes vtep_vip as required when manual allocation is enabled. No companion documentation change explains the ToR contract.
Evidence
- Rule 208 now skips ToR underlay requirements and actively rejects ToR loopback IPv4 at
208_manual_ipaddress_allocation.py:53, while its vPC logic rejects a ToRvtep_vipat lines 156-164. - At current model ref
f997000b00b383725a9853c7f200b3a2c38cbb08,docs/templates/vxlan/underlay/underlay_general.md:89-91gives only a spine exception, anddocs/templates/vxlan/topology/topology_vpc_peer.md:74-96does not describe the ToR exception. - PR 866 changes only two implementation files; it adds no model-doc companion, example, changelog entry, or durable behavioral test.
Practical example
A user following the current manual-allocation documentation could reasonably configure this ToR vPC pair:
vxlan:
underlay:
general:
manual_underlay_allocation: true
underlay_routing_loopback_id: 0
underlay_vtep_loopback_id: 1
topology:
switches:
- name: dc1-tor1
role: tor
interfaces:
- name: Loopback0
mode: fabric_loopback
ipv4_address: 10.0.0.31
- name: Loopback1
mode: fabric_loopback
ipv4_address: 10.1.0.31
- name: dc1-tor2
role: tor
interfaces:
- name: Loopback0
mode: fabric_loopback
ipv4_address: 10.0.0.32
- name: Loopback1
mode: fabric_loopback
ipv4_address: 10.1.0.32
vpc_peers:
- peer1: dc1-tor1
peer2: dc1-tor2
fabric_peering: false
vtep_vip: 10.1.0.30This follows the documented interpretation: routing and VTEP loopback IPs are required for every role except spine, and the vPC guide says manual allocation requires vtep_vip; neither passage identifies tor as an exception. PR 866 now rejects the model with errors equivalent to:
TOR switch 'dc1-tor1': underlay loopback 'Loopback0' with IPv4
should not be defined (TOR switches do not participate in VXLAN underlay).
TOR switch 'dc1-tor1': underlay loopback 'Loopback1' with IPv4
should not be defined (TOR switches do not participate in VXLAN underlay).
TOR switch 'dc1-tor2': underlay loopback 'Loopback0' with IPv4
should not be defined (TOR switches do not participate in VXLAN underlay).
TOR switch 'dc1-tor2': underlay loopback 'Loopback1' with IPv4
should not be defined (TOR switches do not participate in VXLAN underlay).
vPC peer 'dc1-tor1-dc1-tor2': vtep_vip should not be defined for
TOR switches (TOR switches do not participate in VXLAN underlay).
To pass the new validation, the user must remove the ToR underlay loopback addresses and vtep_vip, but the current documentation never tells them to do that.
Existing PR overlap
No matching existing PR comment found.
Impact
Users following the current source documentation can add ToR loopbacks or a ToR VIP and receive the new Rule 208 errors. Conversely, users cannot discover from the manual-allocation guide which ToR values must be omitted.
Suggested fix
Update the model repository's manual-allocation and vPC/ToR documentation in the coordinated change, including iBGP/eBGP applicability, ToR loopback/VIP/fabric-link rules, and a minimal ToR example. Add a changelog/release note if that is the project's release convention.
- Move TOR underlay restriction checks to common_vxlan/406 (runs for iBGP and eBGP) - Remove TOR error methods from ibgp_vxlan/208 (keep skip logic only) - Fix validate_vpc_peer_fabric_link: check all matching links, not just first - Add super_spine exclusion for VTEP loopback requirement - Add TOR guard on fabric links section in ndfc_underlay_ip_address.j2
|
Hi Mike, thanks for the detailed review. Point 1 — Stale ToR resource allocations: Point 2 — TOR checks only in iBGP (fixed): Point 3 — Peer-link validator stops at first match (fixed): Point 4 — Documentation: |
3 participants
Related Issue(s)
Fixes #867
Fixes TOR switch support with
manual_underlay_allocation: true. Without this fix, the validate role and underlay IP template fail when TOR switches are present in the topology.Related Collection Role
Related Data Model Element
Proposed Changes
TOR switches do not participate in the VXLAN underlay — they have no loopback IPs, no VTEP, no
vtep_vip, and no underlay P2P or backup links. Whenmanual_underlay_allocation: true, the validate rule and underlay IP template must skip TOR switches entirely and warn if underlay configuration is incorrectly provided for TOR.Rule 208 (
208_manual_ipaddress_allocation.py)TOR exclusions (nothing mandatory for TOR):
Loopback0orLoopback1with IPv4vtep_vipvalidationvtep_vipvalidate_fabric_linksswitch listfabric_peering: falseno longer require a fabric link entryError if TOR has underlay config (catch misconfigurations):
"TOR switch 'X': underlay loopback 'Loopback0' with IPv4 should not be defined..."vtep_vipdefined"vPC peer 'X-Y': vtep_vip should not be defined for TOR switches...""Fabric link 'X' → 'Y': IPv4 underlay configuration should not be defined for TOR switch..."Fix
fabric_peering: false+unnumberedinteraction:validate_vpc_peer_fabric_link()fabric_interface_numbering: unnumbered, only validates the specific vPC peer-link between two peers (not all fabric links for all switches)Underlay IP template (
ndfc_underlay_ip_address.j2)LOOPBACK0_IP_POOLorLOOPBACK1_IP_POOLentriesvtep_vipresource manager entriesData Model Example (TOR topology)
Test Notes
Tested with a VXLAN_EVPN fabric containing:
manual_underlay_allocation: true+fabric_interface_numbering: unnumberedfabric_peering: false'dict object' has no attribute 'vtep_vip'errorCisco Nexus Dashboard Version
12.4.1
Checklist