sync route cache on link state changes #475
Draft
Conversation
This file contains hidden or 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
d92e7fc to
7ef5c29
Compare
If we get an update on a route, we first "add" the route again, then "delete" the old version in update route mode which leaves the flow, but updates everything else. This works fine for routes with ip nexthops, but for link local rotes, we would first mark all l3 neighbors as routable, then on handling the delete, mark them all unroutable, breaking any routes via nexthops on that interface. So ignore updates for link local routes, since we cannot do anything meaningful with the update anyway. Signed-off-by: Jonas Gorski <jonas.gorski@bisdn.de>
Work around missing route removal notifications from kernel on link state changes with routes targeting nexthops on that link by forcing a cache resync on link state change events to catch removed/readded routes. When a link goes down, the kernel will delete all nexthops on that link without sending out netlink notifications. When a nexthop gets deleted, the kernel will delete all IPv4 routes with the nexthop as destination without sending out netlink notifications. These two combined means that when using nexthop objects, like FRR does by default, deletion of routes on link loss are never seen by libnl, and consequently never seen by baseboxd. This causes flow table entries for routes to persist even though they were deleted in the kernel. For IPv6 it seems the routes are only hidden, and gets readded on link up. But this re-add does not trigger a kernel notification either, so in case we disabled the route on link down, we will miss the readd on link up. To work around this, use nl_cache_resync_v2() to trigger a resync of all routes when a link goes up or down. Signed-off-by: Jonas Gorski <jonas.gorski@bisdn.de>
7ef5c29 to
e09db4e
Compare
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.
This is retry of #469 with added fixes.
Work around missing route removal notifications from kernel on link
state changes with routes targeting nexthops on that link by forcing a
cache resync on link state change events to catch removed/readded
routes.
When a link goes down, the kernel wil 8000 l delete all nexthops on that link
without sending out netlink notifications.
When a nexthop gets deleted, the kernel will delete all IPv4 routes with
the nexthop as destination without sending out netlink notifications.
These two combined means that when using nexthop objects, like FRR does
by default, deletion of routes on link loss are never seen by libnl, and
consequently never seen by baseboxd.
This causes flow table entries for routes to persist even though they
were deleted in the kernel.
For IPv6 it seems the routes are only hidden, and gets readded on link
up. But this re-add does not trigger a kernel notification either, so
in case we disabled the route on link down, we will miss the readd on
link up.
To work around this, use nl_cache_resync_v2() to trigger a resync of all
routes when a link goes up or down.
Changes from #469: