Skip to content

NHG with Unequal Cost Members#2302

Open
JaiOCP wants to merge 2 commits into
opencomputeproject:masterfrom
JaiOCP:wecmp
Open

NHG with Unequal Cost Members#2302
JaiOCP wants to merge 2 commits into
opencomputeproject:masterfrom
JaiOCP:wecmp

Conversation

@JaiOCP

@JaiOCP JaiOCP commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

This PR brings in support for high performant fabric where NHG is created with prior information about the members with weights.

NOS can use sai_query_attribute_capability() to find out if the platform supports such behavior or not. Legacy implementation will always return the status as not supported attribute.

Comment thread inc/sainexthopgroup.h Outdated
* @flags CREATE_AND_SET
* @default false
*/
SAI_NEXT_HOP_GROUP_ATTR_MEMBERS_WITH_UNEQUAL_WEIGHTS,

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.

@JaiOCP, what is the additional benefit of querying this capability?

If NOS is configured to program unequal paths, NOS can directly add those members to the NHG with the required weights, right? I believe we don't need any additional hardware capability/support to handle unequal-cost paths in this case.

Please let me know if I am missing any specific use case where this capability is required.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For the legacy implementation, yes.
For newer implementation where there is a dedicated resource for weight NHG vs unweighted NHG, we need to differentiate the NHG during NHG create.
Now you can make an argument that why not a new NHG type instead of a bool. Problem with that approach is that NHG may be of any type with members as wieghted or unweighted. Hence the introduction of a new bool attribute.

@JaiOCP

JaiOCP commented Jun 23, 2026 via email

Copy link
Copy Markdown
Contributor Author

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

NHG with UECMP

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>
@tjchadaga

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment thread inc/sainexthopgroup.h
Signed-off-by: JaiOCP <jai.kumar@broadcom.com>
@JaiOCP

JaiOCP commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@eddyk-nvidia @rck-innovium @tjchadaga Changes made as per the review.
Please take a look and let me know if anything else is needed

@JaiOCP
JaiOCP requested a review from rck-innovium June 26, 2026 18:15
Comment thread inc/sainexthopgroup.h
* NOS must query this attribute for the support.
* False: All members are with equal cost
* True: Members are configured with unequal cost
* @brief This attribute indicates if all the members are with weights or are unweighted

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.

  1. Please add some more text to bring out that this attribute is an hint. For e.g., a NOS can set the type as SAI_NEXT_HOP_GROUP_MEMBERS_WEIGHT_WEIGHTED and still not give any weights to the members. On the other hand, if the NOS sets the type as SAI_NEXT_HOP_GROUP_MEMBERS_WEIGHT_UNWEIGHTED, the NOS MUST NOT give any weights to the members.

    "This attribute is for the NOS to provide a hint that members of this group will be created with weights. When this is set as SAI_NEXT_HOP_GROUP_MEMBERS_WEIGHT_UNWEIGHTED, it allows the SAI implementation to optimize hardware resources for ECMP."

  2. We need to describe the relationship with SAI_NEXT_HOP_GROUP_ATTR_TYPE.
    Example:
    @validonly SAI_NEXT_HOP_GROUP_ATTR_TYPE == SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_UNORDERED_ECMP or SAI_NEXT_HOP_GROUP_ATTR_TYPE == SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_ORDERED_ECMP

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi Ravi,

  1. Your first sentence is what we have today. Instead of adding detail for each type in comment (and go out of space) I have added comments in the sai_next_hop_group_members_weight_t stating what each enum means. We can extend that if you think it needs more details.

  2. I though about it and this attribute will be valid for almost all the NHG types. For example PROTECTION NHG can be weighted or unweighted as well. Today we do not distinguish for any NHG type that if it can be weighted or unweighted. So I felt that adding a valid would break the backward compatibility.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants