Skip to content

Add hierarchical-fec bool leaf to static nexthop-group#1497

Open
romeyod wants to merge 1 commit into
openconfig:masterfrom
romeyod:static-nhg-hfec
Open

Add hierarchical-fec bool leaf to static nexthop-group#1497
romeyod wants to merge 1 commit into
openconfig:masterfrom
romeyod:static-nhg-hfec

Conversation

@romeyod

@romeyod romeyod commented May 20, 2026

Copy link
Copy Markdown
Contributor

Change Scope

  • Add hierarchical-fec bool leaf to Static nexthop-group to control hierarchical forwarding equivalence class (FEC) behavior on next-hop groups:
    /network-instances/network-instance/static/next-hop-groups/next-hop-group/config/hierarchical-fec
    /network-instances/network-instance/static/next-hop-groups/next-hop-group/state/hierarchical-fec
    When True: The nexthop-group preserves its hierarchical structure, explicitly pointing to the resolving NHG for each next-hop.
    When False: The nexthop-group contents are flattened, or a single random next-hop is selected.
  • This change is backwards compatible

Context:
Add hierarchical-fec bool leaf to Static NHG

Tree View

module: openconfig-network-instance
  +--rw network-instances
     +--rw network-instance* [name]
           +--rw static
           |  +--rw next-hop-groups
           |  |  +--rw next-hop-group* [name]
           |  |     +--rw name         -> ../config/name
           |  |     +--rw config
           |  |     |  +--rw name?   string
  +        |  |     |  +--rw hierarchical-fec?   boolean
           |  |     +--ro state
           |  |     |  +--ro name?   string
  +        |  |     |  +--ro hierarchical-fec?   boolean
           |  |     +--rw next-hops
           |  |        +--rw next-hop* [index]
           |  |           +--rw index     -> ../config/index
           |  |           +--rw config
           |  |           |  +--rw index?   -> ../../../../../../next-hops/next-hop/index
           |  |           +--ro state
           |  |              +--ro index?   -> ../../../../../../next-hops/next-hop/index

@romeyod romeyod requested a review from a team as a code owner May 20, 2026 19:11
@romeyod

romeyod commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

@nupkanoi for visibility

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the openconfig-network-instance-static YANG module to version 0.3.0 and introduces a new hierarchical-fec boolean leaf to the static next-hop-group configuration. Feedback indicates a logical inconsistency in the leaf's description, which references next-hop-group pointers that are currently missing from the schema. It was also suggested to define a default value of false for the new leaf to maintain backward compatibility and ensure deterministic behavior.

Comment on lines +161 to +168
description
"When set to true, the next-hop-group preserves its hierarchical
structure, with each next-hop entry explicitly pointing to the
resolving next-hop-group. This enables load-balancing of traffic
forwarded through each next-hop-group entry based on the
bandwidth of the resolving links. When set to false, the
next-hop-group contents are flattened, or a single next-hop is
selected from each referenced group.";

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.

high

The description for hierarchical-fec refers to next-hop entries "explicitly pointing to the resolving next-hop-group" and mentions a "referenced group". However, the current static next-hop configuration (defined in static-nhg-next-hop and oc-loc-rt:local-static-nexthop-config) does not provide a mechanism for a next-hop to reference another next-hop-group.

To enable the hierarchical behavior described, a leaf (e.g., next-hop-group as a leafref) should be added to the next-hop configuration. Additionally, there is a discrepancy between the PR description ("single random next-hop") and the leaf description ("single next-hop... from each referenced group").

}

leaf hierarchical-fec {
type boolean;

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.

medium

It is recommended to provide a default value for boolean leaves in OpenConfig to ensure deterministic behavior when the leaf is not explicitly configured. Based on the description, false (representing flattened behavior) appears to be the appropriate default for backward compatibility.

      type boolean;
      default false;

@robshakir

Copy link
Copy Markdown
Member

@dplore @navaneethyv @ElodinLaarz -- can we please make sure that we review this carefully with other vendors since this sounds like it has the risk of being very platform specific.

@romeyod

romeyod commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@rgwilton @earies for other vendor inputs

@rgwilton

Copy link
Copy Markdown
Contributor

We have discussed this internally and we believe that this behaviour should be managed by the system without requiring explicit user configuration. I.e., this feels too implementation (and perhaps hardware) specific. E.g., certainly the implication that if you don't set this flag then that would mean that you wouldn't automatically be able to make these hierarchical would be unhelpful.

So, in conclusion we are not supportive of this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants