[WIP] Rework route programming to solve various problems #8393
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
Previously, there were two problems: - The routing table was no updated to use the new parent. - Even after restarting felix, the routing table was unable to clean up the old routes. Fix is in two parts: - Add a default-enabled feature flag to force cleanup of conflicting routes. - Move creation of the no-encap routing table to the main loop and recreate it when the parent changes. Also noticed that KeepVXLANDeviceInSync could take a long time to respond to changes, add a kick channel. Move feature gates to the feature detector. Avoid needing to pass yet another object to 30 places.
Much cleaner, kernel does the conflict resolution.
…lict routes. Fix route deletion.
Covers case wehre RouteTable is created after start-of-day.
marvin-tigera
added
release-note-required
fasaxc
force-pushed
the
route-delta-tracker
branch
from
6fe2c60 to
b4dc142
Compare
fasaxc
added
docs-not-required
- Avoid using 'lo''s ifindex. - Make LinkNotFoundError valid.
fasaxc
force-pushed
the
route-delta-tracker
branch
2 times, most recently
from
6b2426c to
a018495
Compare
fasaxc
force-pushed
the
route-delta-tracker
branch
from
a018495 to
f267890
Compare
fasaxc
force-pushed
the
route-delta-tracker
branch
from
abf2e64 to
592ea68
Compare
|
Closing in favour of #8418; I abandoned the routing metric idea so I don't think it's a good stepping stone |
marvin-tigera
removed
release-note-required
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.
Description
Will likely break this into smaller PRs before it's ready to merge...
Refactor RouteTable:
Resolve conflicts between different routing table views by adding metric/priority to each view; from lowest metric (most preferred) to highest:
Routes with different metrics can co-exist in the kernel so the different views can't clobber each other, even transiently (and the behaviour during an overlay is sane).
Move VXLAN layer 2 handling to its own object, managed separately from the RouteTable. The L2 state is basically independent of Routes so there's no need to do it in the same object.
Move creation of cross-subnet routing table to main goroutine to avoid races. Fix handling of parent interface change.
Related issues/PRs
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.