[DNM] Ovs cpu part2 saga revert set6 #2563
Open
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
tssurya
changed the title
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tssurya The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
@tssurya: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
openshift-merge-robot
added
the
needs-rebase
In ovn-cluster-manager, we have some code that depends on the gateway mode. Signed-off-by: Peng Liu <[email protected]> (cherry picked from commit d5aa5d5)
Signed-off-by: Peng Liu <[email protected]> (cherry picked from commit b0f6d9f)
For a number of reasons: 1. Performance With BGP the number of routes that can be installed on the system scales up. Tests with > 1500 routes causes trouble. The current sync implementation has exponential loops. On each iteration, RouteListFiltered is called which dumps all the routes on the system and then filters client side. netlink API is by value, and this causes high GC crunch. Changed sync implementation to have sequential loops and a single list call to netlink. Changed to avoid listing routes in any other case. sinc is still a heavy operation that keeps a lock and migh require further optimizations. This is a first take. 2. Consistency The used RoutePartiallyEqual had a different criteria than what was used to actually delete a route from netlink, leaving the door open for inconsistencies. This could be thought of as a compromise due to the fact that routes can assume different defaults when installed than those provided. Proposal is to do a bit better and ignore those defaults when comparing against kernel routes as a unified criteria. Should fulfill our use cases but might need further tweaking in the future. Potential problem is that kernel switches bad an incorrect value to default instead of throwing us an error which can cause a route to constantly sync. Much of this is undocumented as far as I could tell but anyway has not been observed with the specific routes that we install. Also, the current implementation was validating and indexing by link index which is actually not a required field of a route. Instead, key routes by priority, prefix and table. Route manager assumes ownership of all the routes with any of the provided keys and will delete unrecognized routes on sync. This is inline to what our previous netlink delete was doing. 3. Other * Reset ticker on each sync iteration to avoid backlogging. * Cleaned up excessive logging Signed-off-by: Jaime Caamaño Ruiz <[email protected]> (cherry picked from commit 78a94d4)
This reverts commit 0ee80bf. Conflict in gateway_shared_intf.go because of not having https://github.com/ovn-kubernetes/ovn-kubernetes/pull/5153/files#diff-d3aa58d9b58a0a09264f072df46ab01d0501eb508c4656411ae2dc1ac68fb3c4 Signed-off-by: Surya Seetharaman <[email protected]> (cherry picked from commit ebb7339) (cherry picked from commit b3760a1)
This reverts commit b1525c3. Signed-off-by: Surya Seetharaman <[email protected]> (cherry picked from commit 936e621) (cherry picked from commit def2909)
tssurya
force-pushed
the
OVS-CPU-part2-saga-revert-set6
branch
from
35bbb5c to
df92245
Compare
openshift-merge-robot
removed
the
needs-rebase
|
/payload 4.20 nightly blocking |
|
@tssurya: trigger 11 job(s) of type blocking for the nightly release of OCP 4.20
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/97c63030-2f07-11f0-9e4d-434818c28e50-0 |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
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.
MISC fixes for BGP