Skip to content
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

Merged

Conversation

eqvinox
Copy link
Contributor

@eqvinox eqvinox commented Nov 20, 2023

All users of ZEBRA_NEXTHOP_UPDATE check the VRF and then call into zapi_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.

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,
Copy link
Contributor

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'?

Copy link
Contributor Author

@eqvinox eqvinox Nov 21, 2023

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,
Copy link
Contributor

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' ?

Copy link
Contributor Author

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?

Copy link
Contributor Author

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)

@donaldsharp donaldsharp merged commit 01af05f into FRRouting:master Nov 21, 2023
87 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants