-
Notifications
You must be signed in to change notification settings - Fork 526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fgnhgorch] Fg Nhg link handling #1537
Conversation
This pull request introduces 3 alerts when merging 44065d1 into b7e4410 - view on LGTM.com new alerts:
|
retest this please |
This pull request introduces 3 alerts when merging 44065d1 into b7e4410 - view on LGTM.com new alerts:
|
This pull request introduces 3 alerts when merging 9a586c6 into 1301d0d - view on LGTM.com new alerts:
|
|
||
/* add next-hop into SAI group */ | ||
if (!validNextHopInNextHopGroup(nhk)) | ||
else if (link_oper) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the flow here handled for the case where port doesn't exists (get_port failed)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If get_port fails then link_oper will have the default value of 1 per: https://github.com/Azure/sonic-swss/blob/9a586c6cdbc95e2b75e429964f19047d68c9b751/orchagent/fgnhgorch.cpp#L1498
So it will end up going into the else if and call into validNextHopInNextHopGroup(assuming hasNextHop is true).
…eanup, and fix LGTM warnings on test_fgnhg
9a586c6
to
2600f75
Compare
retest this please |
retest this please |
2 similar comments
retest this please |
retest this please |
What I did
Added fine grained ecmp link oper state handling changes
Why I did it
To adjust ECMP appropriately on link oper change
How I verified it
Verified using VS swss test cases
Details if related
HLD update associated with change: sonic-net/SONiC#693