-
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
Fix PIMD RPF lookup mode and nexthop tracking #17252
Conversation
17aaccf
to
3bcc070
Compare
34f41ee
to
74f6819
Compare
New mrib tests pass when built with changes from PR #17254. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
01f34c8
to
953d829
Compare
Hm. I think this is a little problematic UX-wise because it's a "silent" change users can trip over without noticing. Can we just add a |
Since the rpf lookup mode was moved out of zebra, the command cannot work as it did before these changes. So should this command |
I think that's the only reasonable option, and/or replace it with a stub that prints "this command doesn't exist anymore because it's in pimd now" Now that I think about it… the command can be implemented in |
can you please fix this documentation issue, that is now showing up:
|
More issues:
|
5a00aac
to
a6119f9
Compare
I rebased and fixed the errors from a bad merge with embedded RP. Docs link issue also fixed. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Modified ZEBRA_NEXTHOP_LOOKUP_MRIB to include the SAFI from which to do the lookup. This generalizes the API away from MRIB specifically and allows the user to decide how it should do lookups. Rename ZEBRA_NEXTHOP_LOOKUP_MRIB to ZEBRA_NEXTHOP_LOOKUP now that it is more generalized. This change is in preperation to remove multicast lookup mode completely from zebra. Signed-off-by: Nathan Bahr <[email protected]>
Multicast mode belongs in PIM, so removing it completely from zebra. Modified `show (ip|ipv6) rpf ADDRESS` to always lookup from SAFI_MULTICAST. This means this command is now specific to the multicast table and does not necessarily reflect the PIM RPF lookup, but that should be implemented in PIM instead. Signed-off-by: Nathan Bahr <[email protected]>
Add `mrib` flag to existing "show ip route" commands which then use the multicast safi rather than the unicast safi. Updated the vty output to include the AFI and SAFI string when printing the table. Deprecate `show ip rpf` command, aliased to `show ip route mrib`. Removed `show ip rpf A.B.C.D`. Signed-off-by: Nathan Bahr <[email protected]>
Remove use of `ip multicast rpf-lookup-mode` from unrelated tests. Looks like this test was just unlucky enough to pick that command as an example for use here. Just changed it to something less likely to be removed in the future. Update route table output to include AFI SAFI output. Signed-off-by: Nathan Bahr <[email protected]>
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.
Overall PR looks good, I've made just a few observations.
Add rpf-lookup-mode MODE vty command under router pim block. Including NB piping and config write. Using the mode still pending. Signed-off-by: Nathan Bahr <[email protected]>
Add prefix length in nexthop response. Apply lookup mode to the sychronous lookups, where we may lookup the MRIB, URIB, or both and make a decision based on the nexthop. Signed-off-by: Nathan Bahr <[email protected]>
Refactor the next hop tracking in PIM to fully support the configured RPF lookup mode. Moved many NHT related functions to pim_nht.h/c NHT now tracks both MRIB and URIB tables and makes nexthop decisions based on the configured lookup mode. Signed-off-by: Nathan Bahr <[email protected]>
Link up the RPF lookup mode changing to a force update to RP's and upstreams registered for nexthop lookup cache updates. Signed-off-by: Nathan Bahr <[email protected]>
Moved `show ip rpf A.B.C.D` command here from zebra, deprecated and aliased to `show ip pim nexthop-lookup`. Allow group to be optional in the lookup command. Only validate group if source is ANY. Documented setting source via RP if not provided. Added new output if ANY source + group lookup is performed and no RP is found for the group. Updated output to include souce and group for lookup. Signed-off-by: Nathan Bahr <[email protected]>
Test mrib overrides and rpf lookup mode changes. Signed-off-by: Nathan Bahr <[email protected]>
Moved it all to PIM section and updated docs for recent changes. Signed-off-by: Nathan Bahr <[email protected]>
Generalized the zapi
ZEBRA_NEXTHOP_LOOKUP_MRIB
to justZEBRA_NEXTHOP_LOOKUP
and added the safi into the request message.Moved the RPF lookup mode configuration from zebra into pimd.
Now the command
show ip rpf x.x.x.x
command will just do a lookup against the MRIB, not according to the lookup mode.To do a nexthop lookup according to the lookup mode, use the command
show ip pim nexthop-lookup x.x.x.x y.y.y.y
wherex.x.x.x
is the source address, andy.y.y.y
is the group address, which may affect the lookup.Refactored the PIM zlookup (the synchronous nexthop lookup using
ZEBRA_NEXTHOP_LOOKUP
) to take the RPF lookup mode into account, which means we may lookup via the URIB, MRIB, or both.Refactored the PIM next hop tracking and isolated the nexthop lookup and tracking code into pim_nht.h/c.
Now we track in both the MRIB and URIB tables so that decisions about the nexthop based on the RPF lookup mode can be taken into account, and changes to the lookup mode can be reflected immediately without performing additional lookups.
Updated show commands, documentation, and tests. Including a new PIM mrib test to exercise the RPF lookup mode command and verify that NHT is working correctly.