-
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
*: move common NHT update decoding bits into lib/ #14834
*: move common NHT update decoding bits into lib/ #14834
Conversation
All users of `ZEBRA_NEXTHOP_UPDATE` check the VRF and then call into `zapi_nexthop_update_decode` before further processing. Begin moving this into common code in lib/. Signed-off-by: David Lamparter <[email protected]>
Have the library decode the nexthop. Signed-off-by: David Lamparter <[email protected]>
Same as before, use shared nexthop decode function. Signed-off-by: David Lamparter <[email protected]>
Same as above. Signed-off-by: David Lamparter <[email protected]>
Same as before. Signed-off-by: David Lamparter <[email protected]>
Same as before. Signed-off-by: David Lamparter <[email protected]>
Same as before. Signed-off-by: David Lamparter <[email protected]>
Same as before. Signed-off-by: David Lamparter <[email protected]>
zapi_nexthop_update_decode() is no longer called externally. Also move the comment to zclient->nexthop_update where it now belongs. Signed-off-by: David Lamparter <[email protected]>
* covering route. This way the upper level protocol can make a | ||
* decision point about whether or not it wants to use the match or not. | ||
*/ | ||
void (*nexthop_update)(struct vrf *vrf, struct prefix *match, |
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 'match' and 'nhr' be 'const'?
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 tried that and it had a trail of fallout (from other functions taking non-const args) behind it, such that I didn't want to shove into this PR to keep it clean-ish 😞
* covering route. This way the upper level protocol can make a | ||
* decision point about whether or not it wants to use the match or not. | ||
*/ | ||
void (*nexthop_update)(struct vrf *vrf, struct prefix *match, |
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.
'nexthop_update' seems a bit ambiguous - now that we have nexthop-groups etc. could it be 'nht_update' or 'rnh_update' ?
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.
Happy to rename it, I just "inherited" the name from zapi_nexthop_update_decode
. Donald and I also have renaming the entirety of NHT on the TODO list (because the name is kinda misleading), maybe it doesn't matter at this point?
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 don't think rnh
is a good choice of name, this isn't about recursive resolution; nht
is off in general since this isn't a mechanism for specifically tracking nexthops, it tracks prefixes in general… cf. discussion in #development)
All users of
ZEBRA_NEXTHOP_UPDATE
check the VRF and then call intozapi_nexthop_update_decode
before further processing. Move this into common code in lib/.zapi_nexthop_update_decode
is private at the end of this chain of commits.This is prep work for further NHT refactoring & cleanup. There should be no visible/functional changes from this series of commits.