-
Notifications
You must be signed in to change notification settings - Fork 615
EVPN MultiHoming Fast ReRoute for L3VNI-routed traffic #2332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
manamand2020
wants to merge
2
commits into
opencomputeproject:master
Choose a base branch
from
manamand2020:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This docblock is where the pre-existing
SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_SET_SWITCHOVERfirst acquires hardware-mode meaning, and two interactions look undefined to me.Manual request vs. autonomous selection in
MODE_HARDWARE. If the NOS writesSET_SWITCHOVER = truewhile the primary is still healthy, revertive hardware is documented to "switch back to the bridge port once it recovers" — but it never left. Does hardware immediately revert, making the request a no-op or a transient blip? Is a manual request sticky until explicitly cleared, or always subordinate to hardware's own selection? Section 5 says the resulting path is readable immediately fromPROTECTION_STATE, which only holds if the answer is deterministic.Re-arming in
MODE_HARDWARE_NON_REVERTIVE. The walkthrough says traffic stays on the protection path "until the NOS explicitly reverts it (e.g. viaSET_SWITCHOVER)". Two gaps: if the NOS writesfalsewhile the primary is still down, is that rejected, or committed and then immediately failed over again? And after a successful revert, is the latch re-armed so the next failure switches over autonomously? The "e.g." also leaves it ambiguous whetherSET_SWITCHOVERis the defined mechanism or merely one option.Header/proposal divergence. The proposal document is considerably more precise than the header here — that
falsereverts to primary, that the outcome is the return status ofset_bridge_port_attribute(), and thatPROTECTION_STATEis readable immediately are all stated in section 5 but not in the docblock. Since vendors implement from headers, it would help to pull those sentences into the Doxygen comment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_ADMIN_MODEandSAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_REVERTIVEto address these concerns. I have also deprecated theSAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_SET_SWITCHOVERattribute.