Skip to content

Adding support for storm control on L2 interfaces and copy description on PC - #855

Merged
mtarking merged 16 commits into
netascode:developfrom
juarocha:enhancement/description_storm-control
Aug 7, 2026
Merged

Adding support for storm control on L2 interfaces and copy description on PC#855
mtarking merged 16 commits into
netascode:developfrom
juarocha:enhancement/description_storm-control

Conversation

@juarocha

@juarocha juarocha commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Related Issue(s)

Fixes #850
Fixes #853
Fixes #854
Fixes #857

Related Collection Role

  • cisco.nac_dc_vxlan.validate
  • cisco.nac_dc_vxlan.dtc.create
  • cisco.nac_dc_vxlan.dtc.deploy
  • cisco.nac_dc_vxlan.dtc.remove
  • other

Related Data Model Element

  • vxlan.fabric
  • vxlan.global
  • vxlan.topology
  • vxlan.underlay
  • vxlan.overlay
  • vxlan.overlay_extensions
  • vxlan.policy
  • vxlan.multisite
  • defaults.vxlan
  • other

Proposed Changes

For storm control, adding new keys for under L2 interfaces:

  • enable_storm_control
  • storm_control_action
  • storm_control_broadcast_level_percent
  • storm_control_multicast_level_percent
  • storm_control_unicast_level_percent
  • storm_control_broadcast_level_pps
  • storm_control_multicast_level_pps
  • storm_control_unicast_level_pps

For copy description, adding new keys for under port-channel interfaces:

  • copy_description

Test Notes

Cisco Nexus Dashboard Version

4.1

Checklist

  • Latest commit is rebased from develop with merge conflicts resolved
  • New or updates to documentation has been made accordingly
  • Assigned the proper reviewers

@juburnet juburnet added the 0.9.0 Release 0.9.0 label Jul 24, 2026
@juarocha juarocha added the enhancement New feature or request label Jul 24, 2026
@juarocha
juarocha marked this pull request as ready for review July 24, 2026 18:56
@juarocha
juarocha requested a review from a team as a code owner July 24, 2026 18:56
Comment thread roles/dtc/common/templates/ndfc_interfaces/ndfc_interface_trunk_po.j2 Outdated
Comment thread roles/validate/files/defaults.yml
@juburnet
juburnet requested a review from mikewiebe July 29, 2026 15:48
@juarocha
juarocha requested a review from juburnet July 29, 2026 18:19
@juburnet juburnet changed the title Adding support for stom control on L2 interfaces and copy description on PC Adding support for storm control on L2 interfaces and copy description on PC Jul 29, 2026
Comment thread roles/dtc/common/templates/ndfc_interfaces/ndfc_interface_vpc.j2
@juarocha
juarocha requested a review from juburnet July 30, 2026 17:42

@juburnet juburnet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juarocha - Please check if this scenario would be an issue:

Rule 312 iterates switch.get('interfaces') but VPC peer configurations live under vxlan.topology.switches[].vpc_peers[].interfaces (or similar path).

The VPC template now renders storm control for VPC interfaces. If a user sets both _percent and _pps on a VPC peer interface, Rule 312 would not catch it.

Confirm whether VPC peer data is in a different path that needs a second traversal.

@juarocha

Copy link
Copy Markdown
Contributor Author

@juburnet just tested this, and it validates both sides of vPC. The rule is working for storm control is values but the we may be hitting #857

@juarocha
juarocha requested a review from juburnet July 30, 2026 21:51

@juburnet juburnet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

  1. Consistent implementation across all 5+ interface templates — storm control blocks are identical in ndfc_interface_access.j2, ndfc_interface_access_po.j2, ndfc_interface_dot1q.j2, ndfc_interface_trunk.j2, ndfc_interface_trunk_po.j2, and ndfc_interface_vpc.j2.
  2. Defaults properly seeded — defaults.yml updated for all affected interface types (dot1q, access, trunk, access_po, trunk_po). New copy_description: false default added wherever copy_description was introduced.
  3. Validation rule 312 added — New 312_topology_interface_storm_control.py enforces the mutual exclusivity of percent vs. PPS. Good defensive validation.
  4. vPC rule 305 bug fix is a real fix — The old logic (len(interfaces["interfaces"]) == 2) was incorrect in multi-fabric scenarios. The new per-pair iteration logic correctly scopes the consistency check to actual vPC peers.
  5. data_model_key_check is self-contained — Rule 312 copies the utility method locally rather than inheriting from a base class inconsistently, matching the pattern of other rules in the codebase.
  6. All CI passes — Clean across all Python/Ansible matrix combinations.

@juburnet juburnet added the ready for review PR Ready for Review label Jul 31, 2026
Comment thread roles/validate/files/rules/common/312_topology_interface_storm_control.py Outdated

@dacasti2mx dacasti2mx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule 312 assumes storm control is disabled whenever enable_storm_control is not explicitly repeated on the interface, but, this setting can also be inherited from defaults.vxlan

Since semantic validation runs before factory and custom defaults are merged, a valid interface that inherits enable_storm_control: true and only defines
its threshold is rejected.

Could Rule 312 validate the effective value using this precedence:

  • explicit interface value, matching custom default, then factory default?
  • It would also behelpful to add a test covering this inherited-default scenario.

For example, this should be valid because storm control is enabled through the access-interface defaults:

defaults:
vxlan:
  topology:
    switches:
      interfaces:
        topology_switch_access_interface:
          enable_storm_control: true
vxlan:
topology:
  switches:
    - name: leaf-1
      interfaces:
        - name: Ethernet1/1
          mode: access
          storm_control_broadcast_level_percent: 10

in this example, Rule 312 sees no enable_storm_control directly under Ethernet1/1, assumes false, and rejects the threshold even though the effective value inherited
from defaults is true.

@juarocha

juarocha commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Rule logic fixed. since defaults are merged after the validation we can't use them on the rule. The only validation is that percent-based and pps-based settings are not mixed on the same interface

@juarocha
juarocha requested a review from dacasti2mx August 5, 2026 16:13
@juburnet

juburnet commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@juarocha for Storm control, add ND 4.1 version requirement in templates as discussed.

@juarocha

juarocha commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@juarocha for Storm control, add ND 4.1 version requirement in templates as discussed.

done

@dacasti2mx dacasti2mx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

live validation

Env: ND 4.1.1g / NDFC 12.4.1.321 · MCFG child fabric (FAB1) · targets NX-OSv (N9K-C9300v, 10.5(5))
Heads: validated live at 318b9cd; re-checked at 21d215e (delta = templates-only version gate, 0 rule changes)
Result: LGTM — features validated end-to-end; the "no config when not applicable" behaviors are by-design (see
Notes).

Negative / validation

# Model Expected Result
N1 percent + pps on same intf Rule 312 FAIL ✅ 312 FAIL
N2 enable:false + threshold accepted; threshold not applied (storm-control off) ✅ nac-validate passes; render
omits it (by design)
N3 vPC peers differ Rule 305 FAIL ✅ 305 FAIL
N4 same vpc_id on 2 pairs PASS ✅ PASS (pairing fix)

Physical interface (subject Eth1/7, control Eth1/8 untouched)

Stage Change Result (NX-OS)
L0 access, no storm-control ✅ access, no storm-control
L1 enable + shutdown + bcast 5.00% storm-control broadcast level 5.00 + action shutdown
L2 rerun ✅ idempotent
L3 20.00% + trap + mcast 10.00% ✅ updated cleanly (no stale values)
L4 percent → pps 1000 storm-control broadcast level pps 1000
L5 disable ✅ storm-control fully removed
L6 trunk + 10.00% ✅ trunk + storm-control
L7 dot1q + 10.00% mode dot1q-tunnel + storm-control
L8 restore ✅ clean
inherited default enable=true, no explicit enable ✅ threshold renders (inheritance works)

Port-channels + copy_description

Stage Change Result
P1 access PO + 5% + copy_description:true ✅ PO storm-control; description copied to members
P2 verify ✅ member desc == PO desc
P3 update description ✅ propagated to members
P4 percent → pps ✅ clean transition
P5 copy_description:false ✅ stops copying; previously-copied member desc retained (by design)
P6 trunk PO + storm-control ✅ (see Notes: NX-OS box-wide %/pps constraint)
P7 routed PO + copy_description, no storm-control ✅ copy_description works; storm-control correctly absent
P8 removal ✅ requires role_remove (merged create is additive)
P9 rerun ✅ idempotent

vPC / ToR pair

# Case Result
D1–D3/D5 storm-control + copy_description on vPC PO, both peers ✅ propagated to both peers + members; Rule 305 PASS
D4 pps on vPC ⏭️ by equivalence (pps validated in L4/P4)
D6 update ✅ propagated to both
D7 removal ✅ reverted cleanly
D8 rerun ✅ idempotent
D9 peers differ ✅ Rule 305 FAIL before mutation
D10 percent + pps ✅ Rule 312 FAIL
D11 same vpc_id on 2 pairs ✅ Rule 305 PASS (pairing fix)

@mtarking
mtarking merged commit 435770a into netascode:develop Aug 7, 2026
23 checks passed
@juarocha
juarocha deleted the enhancement/description_storm-control branch August 7, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.9.0 Release 0.9.0 enhancement New feature or request ready for review PR Ready for Review

Projects

None yet

5 participants