-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Extend show ip route nexthop-group commands (Summary view and ECMP filtering) #19833
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
base: master
Are you sure you want to change the base?
Extend show ip route nexthop-group commands (Summary view and ECMP filtering) #19833
Conversation
35d3c35 to
732331e
Compare
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.
Could you add a topotest showing that it works?
will do |
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.
can you say some more about the motivation for this? is there information here that couldn't be extracted from existing json output? what's the use-case you anticipate: it seems like any "summary" that potentially includes a large number of prefixes is not going to be very useful at scale?
Given that we are going to support higher ECMP, what I see is
Majorly, JSON output is not huge with this fix in this case, as compared to the existing. (so memory/BGP MAIN CPU would benefit). But again, this is my opinion. Let me know if you have some inputs as well, so that we scrap this idea/make this better. |
732331e to
d0321f8
Compare
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.
Could you use a consistent nexthop description for the JSON fields?
Now we have:
internalNextHopNum
installedNexthopGroupId
nexthopGroupId
nexthop keyword has a mixed variation...
zebra/zebra_vty.c
Outdated
| if (re->nhe) { | ||
| json_object_int_add(json_route, "nexthopGroupFlags", | ||
| re->nhe->flags); | ||
| json_object_string_add(json_route, "nexthopGroupValid", |
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.
Use json_object_boolean_add().
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.
Done
| json_object_boolean_true_add(json_route, "offloaded"); | ||
|
|
||
| if (CHECK_FLAG(re->flags, ZEBRA_FLAG_OFFLOAD_FAILED)) | ||
| json_object_boolean_false_add(json_route, "offloaded"); |
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.
Does it just overwrite the previous one?
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.
yes, same used to happen before as well.
if (CHECK_FLAG(re->flags, ZEBRA_FLAG_OFFLOADED))
json_object_boolean_true_add(json_route, "offloaded");
if (CHECK_FLAG(re->flags, ZEBRA_FLAG_OFFLOAD_FAILED))
json_object_boolean_false_add(json_route, "offloaded");
i think idea is to set offloaded correctly.
|
some comments:
but ... while it's true that 128K lines is more than 1K lines, 1K lines is still probably not very useful for a human to read? in other words, it doesn't take much scale to make vanilla ascii output ... not very useful. And for any system-level orchestration or controller, the output really has got to be json, to make the output machine-parseable. and if the output is parsed into python objects (for example), then it can be searched, sorted, filtered, etc in many ways, not just one way? I don't know - I just think we have a lot of knob-turning in the vty output code, and I worry a bit about adding more and more, for maintainability. |
Previously, the json and nexthop-group options were mutually exclusive in the show ip/ipv6 route commands. As of now, sh ip route json or sh ip route nexthop-group json shows same output. This commit is to be used by future new summary command. Signed-off-by: Rajasekar Raja <[email protected]>
Add a utility function to format nexthop group flags into a comma-separated string for display. This allows consistent flag formatting across different commands. Signed-off-by: Rajasekar Raja <[email protected]>
Use dump_nhg_flags for text output in show nexthop-group command. Signed-off-by: Rajasekar Raja <[email protected]>
@ton31337 |
3a1da2e to
1e1ad65
Compare
Add support for 'show ip route nexthop-group summary' command to display
a concise view of routes with ECMP counts instead of listing all nexthops.
The existing 'show ip route nexthop-group' command displays all nexthop
details for each route, which can be verbose and difficult to review when
dealing with many ECMP paths.
Example:
sh ip route vrf vrf-101 nexthop-group
B>* 10.0.101.1/32 [200/0] (39) via 192.168.1.1, bridge-101 onlink, weight 1, 03:11:12
via 192.168.1.1, bridge-101 onlink (dup), weight 1, 03:11:12
sh ip route vrf vrf-101 bgp nexthop-group
B>* 10.0.101.1/32 [200/0] (39) via 192.168.1.1, bridge-101 onlink, weight 1, 00:03:15
via 192.168.1.1, bridge-101 onlink (dup), weight 1, 00:03:15
The new 'summary' option provides a single-line view per prefix showing:
- Received/Installed NHG IDs
- ECMP count and FIB installed count
- Route entry status
- Nexthop group flags
Example:
sh ip route vrf vrf-101 nexthop-group summary
B>* 10.0.101.1/32 [200/0] Rcv/Ins NHG: 39/39 ECMP/FIB count: 2/1 Status: installed Flags: Valid, Installed
sh ip route vrf vrf-101 bgp nexthop-group summary
B>* 10.0.101.1/32 [200/0] Rcv/Ins NHG ID: 39/39 ECMP/FIB count: 2/1 Status: Installed Flags: Valid, Installed
This makes it easier to quickly assess ECMP path counts across multiple routes.
Signed-off-by: Rajasekar Raja <[email protected]>
Add support for filtering routes by ECMP count in the nexthop-group summary
command. The new syntax allows filtering routes based on ECMP count:
- show ip route nexthop-group summary ecmp-count gt N (greater than)
- show ip route nexthop-group summary ecmp-count lt N (less than)
- show ip route nexthop-group summary ecmp-count eq N (equal to)
and their corresponding json
Example:
r1# sh ip route sharp nexthop-group summary ecmp-count eq 2
D> 2.2.2.1/32 [150/0] Rcv/Ins NHG ID: 90/90 ECMP/FIB count: 2/2 Status: Installed Flags: Valid, Installed
D>* 2.2.2.2/32 [150/0] Rcv/Ins NHG ID: 109/109 ECMP/FIB count: 2/2 Status: Installed Flags: Valid, Installed
D> 3.3.3.2/32 [150/0] Rcv/Ins NHG ID: 118/118 ECMP/FIB count: 2/1 Status: Installed Flags: Valid, Installed
D> 5.5.5.1/32 [150/0] Rcv/Ins NHG ID: 152/152 ECMP/FIB count: 2/8 Status: Installed Flags: Valid, Installed
r1# sh ip route sharp nexthop-group summary ecmp-count lt 2
D> 3.3.3.1/32 [150/0] Rcv/Ins NHG ID: 111/112 ECMP/FIB count: 1/2 Status: Installed Flags: Valid, Recursive
r1# sh ip route sharp nexthop-group summary ecmp-count eq 1 json
{"3.3.3.1/32":[{"prefix":"3.3.3.1/32","prefixLen":32,"protocol":"sharp",
"vrfId":0,"vrfName":"default","selected":true,"destSelected":true,
"distance":150,"metric":0,"installed":true,"table":254,
"nexthopGroupId":113,"ecmpCount":1,"fibInstalledCount":2,
"installedNexthopGroupId":114,"receivedNexthopGroupId":113,
"nexthopGroupFlags":9,"nexthopGroupValid":"true"}]
}
Signed-off-by: Rajasekar Raja <[email protected]>
Documentation for nexthop-group summary and ECMP filtering in show ip route nexthop-group cmd Signed-off-by: Rajasekar Raja <[email protected]>
Add test coverage for 'show ip route nexthop-group summary' command with various filters (ecmp-count, protocol, vrf) Signed-off-by: Rajasekar Raja <[email protected]>
1e1ad65 to
ef3e77b
Compare
|
I think he meant the word "nexthop" specifically, there are three different forms in there
|
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.
I'm ok with this
|
ci:rerun |
Extend show ip route nexthop-group commands