route: ignore flags and weight of nexthops when doing a strict route comparison #384
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
thom311
reviewed
May 14, 2024
| @@ -136,6 +136,14 @@ int rtnl_route_nh_compare(struct rtnl_nexthop *a, struct rtnl_nexthop *b, | |||
| return diff; | |||
| } | |||
|
|
|||
| int rtnl_route_nh_identical(struct rtnl_nexthop *a, struct rtnl_nexthop *b) | |||
There was a problem hiding this comment.
could you please add a short doc-text for public API?
There was a problem hiding this comment.
Sure thing, will add one
There was a problem hiding this comment.
Added one (loosely based on nl_object_identical()'s).
Not all attributes of a nexthop are id attributes, e.g. the flags will contain state (LINKDOWN, DEAD) of the attached link about which the kernel will not send route updates. Likewise, the weight may not exist when processing an ECMP IPv6 route update which only contains a single nexthop. Since rtnl_nexthop isn't a first class cache object, we cannot use nl_object_identical(), so add a separate identical helper function which compares only fixed attributes. Signed-off-by: Jonas Gorski <[email protected]>
When a route is created while the interface has no link, we get a notification with the route and the nexthop having the flag LINKDOWN. If the interface later gets a link, we do not get a route notification about it, so the route and nexthop stay at LINKDOWN in the libnl cache. If the route then gets removed again, the to be removed route will not have the LINKDOWN flag anymore, which then can break comparison of the nexthop(s). So use the new nexthop identical helper to avoid this scenario. Signed-off-by: Jonas Gorski <[email protected]>
KanjiMonster
force-pushed
the
jogo_route_nh_cmp
branch
from
a65bc5d to
861fb80
Compare
|
merged. thank you!!! |
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.
When a route is created while the interface has no link, we get a
notification with the route and the nexthop having the flag
LINKDOWN.If the interface later gets a link, we do not get a route notification
about it, so the route and nexthop stay at
LINKDOWNin the libnl cache.If the route then gets removed again, the to be removed route will not
have the
LINKDOWNflag anymore, which then can break comparison of thenexthop(s).
So add a new helper for ignoring FLAGS and WEIGHT for nexthops, and use it
everywhere we want to tell if it's the same nexthop, but not necessarily
in the same state.
For loose comparisons, we still need to use the old function, as else e.g.
route lookup will require specifying exact nexthops, and just filtering for
all routes on a certain interfaces would break.