Skip to content

Add FEC support for physical interfaces (NDFC 12.4.1+) - #856

Open
ccoueffe wants to merge 6 commits into
netascode:developfrom
ccoueffe:feat/Forwarding_Error_Correction
Open

Add FEC support for physical interfaces (NDFC 12.4.1+)#856
ccoueffe wants to merge 6 commits into
netascode:developfrom
ccoueffe:feat/Forwarding_Error_Correction

Conversation

@ccoueffe

@ccoueffe ccoueffe commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Related Issue(s)

Depends on CiscoDevNet/ansible-dcnm#661 — adds fec parameter to dcnm_interface module.

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

Adds Forward Error Correction (FEC) support for physical ethernet interfaces, gated behind NDFC >= 12.4.1.

Templates updated

FEC attribute added (with version gate version_compare('12.4.1', '>=')):

Template Interface type
ndfc_interface_trunk.j2 Trunk
ndfc_interface_access.j2 Access
ndfc_interface_routed.j2 Routed
ndfc_interface_dot1q.j2 Dot1Q tunnel

Defaults added

Default fec: auto added for all four interface types in defaults.yml:

  • topology_switch_access_interface.fec
  • topology_switch_trunk_interface.fec
  • topology_switch_dot1q_interface.fec
  • topology_switch_routed_interface.fec

Data Model Example

vxlan:
  topology:
    switches:
      - name: DC1-LEAF1
        interfaces:
          - name: Ethernet1/12
            mode: trunk
            speed: 25Gb
            fec: rs-fec
            description: "25G link with RS-FEC"

Supported FEC values

auto, fc-fec, off, rs-cons16, rs-fec, rs-ieee

Test Notes

Tested with NDFC 12.4.1:

  • Trunk interface with fec: rs-fec → FEC applied, idempotent on second run
  • Access interface with fec: fc-fec → FEC applied
  • Routed interface with fec: rs-fec → FEC applied
  • Templates correctly omit FEC on NDFC < 12.4.1

Cisco Nexus Dashboard Version

12.4.1+ (ND 4.1.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

…ed with fec. Adding default values for these interface type.

Signed-off-by: ccoueffe <ccoueffe@cisco.com>
@ccoueffe ccoueffe changed the title update templates for physical interface access, trunk, dot1q and rout… Add FEC support for physical interfaces (NDFC 12.4.1+) Jul 27, 2026
@ccoueffe
ccoueffe marked this pull request as ready for review July 27, 2026 13:00
@ccoueffe
ccoueffe requested a review from a team as a code owner July 27, 2026 13:00
@ccoueffe ccoueffe self-assigned this Jul 27, 2026
@ccoueffe ccoueffe added ready for review PR Ready for Review 0.9.0 Release 0.9.0 labels Jul 27, 2026
@juburnet
juburnet requested a review from dacasti2mx July 30, 2026 15:00

@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.

fec: off

The companion schema in nac-vxlan PR #654 supports off as a valid FEC value

  fec: {{ interface['fec'] | default(...) }}

Therefore, this valid data-model input:

  fec: "off"

is rendered in the generated resource file as:

  fec: off

When that file is subsequently loaded with yaml.safe_load(), PyYAML interprets off as the boolean False, rather than the string "off" expected by
dcnm_interface.

I confirmed this conversion in a local serialization/parsing test. The exact downstream module failure will also be captured during the end-to-end validation.

Please ensure FEC is serialized as a quoted string in all four new templates:

  • ndfc_interface_access.j2
  • ndfc_interface_trunk.j2
  • ndfc_interface_routed.j2
  • ndfc_interface_dot1q.j2

@ccoueffe
ccoueffe requested a review from dacasti2mx July 31, 2026 18:20
@dacasti2mx

Copy link
Copy Markdown
Contributor

fec: off issue solved, it works.
After testing finishes I will provide another review.

@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.

Currently:

{% if ndfc_version | cisco.nac_dc_vxlan.version_compare('12.4.1', '>=') %}
      fec: "{{ interface['fec'] | default(defaults...trunk_interface.fec) }}"
{% endif %}

This part of the code tells us that we render fec with its value only if the version is 12.4.1 or higher. That means a lower version — even when FEC is explicitly set — does not render that value, and since module dcnm_interface ( 661 dcnm)) never receives it, its fec requires NDFC >= 12.4.1 check never fires.

@juburnet
juburnet requested a review from dacasti2mx August 10, 2026 15:50

@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.

LGTM from my side.
The FEC functionality was validated across the supported physical interface types, including off serialization, defaults, idempotency, and end-to-end application.

We agreed to keep the current NDFC 12.4.1+ version gate and handle the unsupported-version behavior through clear user documentation.

@juburnet
juburnet requested a review from mikewiebe August 11, 2026 15:13
@juburnet juburnet added the enhancement New feature or request label Aug 11, 2026
@mikewiebe

Copy link
Copy Markdown
Collaborator

Medium: Make the companion FEC documentation schema-valid and version-accurate

Issue

The authoritative model documentation added for this feature uses speed: 25000 in every FEC example even though the schema accepts 25gb, not numeric 25000. The access example also uses unquoted fec: off, which PyYAML interprets as Boolean false and the FEC enum rejects. The same pages map NDFC 12.4.1 to ND 4.1, while this PR and current DCNM identify it as ND 4.1.1+.

Evidence

  • At model commit f997000b00b383725a9853c7f200b3a2c38cbb08, the interface schema accepts speeds matching auto|100mb|1gb|10gb|25gb|... (schema line 471). The FEC example uses speed: 25000 and fec: off. Numeric 25000 is also present in the trunk examples at lines 116/124, routed examples at lines 64/70, and dot1q example at line 100.
  • Focused Yamale validation against that exact schema failed with speed: '25000' is not a regex match; changing it to speed: 25gb passed. A separate speed: 25gb, fec: off input failed with fec: 'False' not in (...); quoting it as fec: "off" passed.
  • Each companion page says NDFC 12.4.1 (ND 4.1), for example access line 10, whereas the PR body and current DCNM version guard describe NDFC 12.4.1 as ND 4.1.1+.
  • scripts/verify_schema.py only extracts the first fenced YAML block from each Markdown file, so these later “Example 2” FEC blocks are not covered. The source-wide run was additionally blocked before validation by an unrelated existing malformed YAML block (spanning_tree_portfast).

Existing PR overlap

No matching existing PR comment found. The earlier fec: off review fixed quoting in the rendered NaC-to-DCNM payload, not the unquoted user-facing model example.

Impact

Users who copy the documented FEC examples fail model validation before PR #856's templates run, including the documented way to disable FEC. The ND version text can also lead users to expect the feature on the wrong platform release.

Suggested fix

Change every FEC example to speed: 25gb, quote the disabled value as fec: "off", and make the ND release text consistent with the verified NDFC-to-ND mapping (ND 4.1.1+ if that mapping remains authoritative). Extend documentation validation to check every fenced YAML example, then add the corrected FEC examples to that test.

@ccoueffe

Copy link
Copy Markdown
Collaborator Author

documentation updated

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

Development

Successfully merging this pull request may close these issues.

4 participants