-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Zebra rework factor #15473
Zebra rework factor #15473
Conversation
Reduce the code size by reusing the same code to display nexthops and backup nexthops. Signed-off-by: Philippe Guibert <[email protected]>
…nction Move the rib_evpn_route_add() and rib_evpn_route_delete() functions in a separate place. Signed-off-by: Philippe Guibert <[email protected]>
Factorise the nexthop display handling in a separate function. Signed-off-by: Philippe Guibert <[email protected]>
Create a separate function to handle separately the nexthop structure. On the next commit, when a nexthop group is used, the nexthop of each nexthop group dependency is used. Signed-off-by: Philippe Guibert <[email protected]>
The show_route_nexthop_group_helper() and the show_nexthop_group_json_helper() helper functions are introduced to separate nexthop handling from the remaining 'show ip route' command. Signed-off-by: Philippe Guibert <[email protected]>
Create a second function zsend_redistribute_route_nhg() to handle nexthop group. Signed-off-by: Philippe Guibert <[email protected]>
Create a mpls_ftn_uninstall_nhg() function that handles the nexthop parsing for each route entry. This parsing is common to primary and backup nexthops. Signed-off-by: Philippe Guibert <[email protected]>
The zebra_nexthop_vty_helper() and zebra_nexthop_json_helper() functions could be very helpful to display nexthop information from whatever daemon. Move the core function in the nexthop_vty_helper() and the nexthop_json_helper() function. The zebra API call remains unchanged. Signed-off-by: Philippe Guibert <[email protected]>
@@ -1158,3 +1158,331 @@ bool nexthop_is_ifindex_type(const struct nexthop *nh) | |||
return true; | |||
return false; | |||
} | |||
|
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.
Part of the reason I've never moved to having a vty function and a json function is that you end up with code that is in two functions and an addition to the data structure being dumped is easy to forget the json aspect of it ( or vice versa ). It always sucked but I felt the in your face aspect was warranted. What advantage are you looking for with this change? How do we ensure that a new developer is going to remember to hit both sides of the equation?
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 had to factor some changes for the BGP NHG changes. but it seems I no longer need to support it.
So I try to upstream it.
I admit most of the code change is not justified, today.
What advantage are you looking for with this change?
- I could see that nexthop display is not up to date everywhere.
A little more work could help in having a single function.
How do we ensure that a new developer is going to remember to hit both sides of the equation?
A single function could help, to handle both json and vty.
closing for now |
This is a list of change in zebra that factorize the code.
There is no change in the behavior.