zebra: Kernel routes are not updated properly in zebra RIB / nhg nexthop check is wrong #13561 #17045
+1
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: DennyAgussy <gkruparaju@msystechnologies.com>
|
put the discussion in the actual commit message then we can look at this. I'm pretty dubious about making this chagne as that I believe it would break some onlink routes |
| @@ -2648,7 +2648,7 @@ static unsigned nexthop_active_check(struct route_node *rn, | |||
|
|
|||
| ifp = if_lookup_by_index(nexthop->ifindex, nexthop->vrf_id); | |||
|
|
|||
| if (ifp && ifp->vrf->vrf_id == vrf_id && if_is_up(ifp)) { | |||
| if (ifp && ifp->vrf->vrf_id == vrf_id && if_is_up(ifp) && if_connected_count(ifp->connected)) { | |||
| SET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); | |||
There was a problem hiding this comment.
I wonder if it is enough. what happens if, for a given ipv4 route, you remove all ipv4 addresses, knowing that you have 1 ipv6 address left? or the opposite for a given ipv6 route.
should not we involve the 'v6-with-v4-nexthop' flag into this control too ?
There was a problem hiding this comment.
Why? The kernel is telling us that it has a valid route. What does it matter that there are any addresses on the interface? I can have a route without any addresses on an interface especially with a onlink attribute
DennyAgussy
changed the title
### I have tested whether ONLINK routes will get break with the fix and master branch with two scenarios mentioned below and also let's know if we have include any additional cases :
Tested on Master-branch(56495a8): Config:
Is this a bug scenario? Logs after flush: scenario 2 will of course works as expected. On the branch we did fix:We have tested the same two scenarios, the behavior is same as the Master branch(Not breaking any route in scenario 2). Also behaves the same in scenario 1. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Kernel routes are not updated properly in zebra RIB / nhg nexthop check is wrong: #13561
Introduction:
Zebra is not restoring the routes properly as mentioned in the bug-13561. When we do flush on an interface having the default route, zebra is still advertising the default route which it shouldn't. As we flush the interface the default route should unadvertised in vtysh -c "show ip route".
Implementation details:
To overcome this issue we have added a check. Which helps in checking whether the interface has at least one connected route. If the interface has a connected route then it will skip the intermediate checks and go directly to "skip_check: ". With this check in place everything is working - where default route is being unadvertised in vtysh -c "show ip route".
Necessary unit testing and topotests have been done.
Thanks
Denny Agussy
Nandini D