-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[fastboot] Preserve CoPP table HLD to improve fastboot flow #1107
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
5831b79
Preserve CoPP table HLD
arfeigin 4e75e54
Update HLD
arfeigin 4a83b92
Update HLD - fix diagram link
arfeigin 4567cec
Fix spelling
arfeigin ff6960e
Merge branch 'master' into copp_preserve
prsunny 0a14b7e
Merge branch 'master' into copp_preserve
liat-grozovik b45752a
Merge branch 'master' into copp_preserve
liat-grozovik 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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| # Introduction | ||
|
|
||
| The scope of this document is to provide the requirements and a high-level design proposal for preserving the contents of the CoPP (Sonic Control Plane Policing) tables during reboot for faster LAG creation in order to improve fast-reboot's dataplane downtime. | ||
|
|
||
| # Requirements | ||
|
|
||
| The following are the high level requirements for preserving CoPP tables contents during reboot. | ||
|
|
||
| 1. CoPP Tables shouldn't be cleared from APP DB by DB migrator. | ||
| 2. coppmgr mergeConfig logic should be enhanced to: | ||
| 1. Ignore setting existing entries. | ||
| 2. Overwrite entry when value differs, use value from default init file merged with user configuration. | ||
| 3. Delete entries with keys that are not supported in the copp default initialization (backward compatibility). | ||
| 4. In case an entry exists in user's configuration file or in the default json initialization file while missing from the preserved CoPP table it will be add to the table as a new entry. | ||
|
|
||
| # Design Proposal | ||
|
|
||
| ## Current behavior | ||
|
|
||
| In the current implementation DB migrator clears CoPP tables contents and it is being initialized with default values at startup. This process takes some time and leads to that LACP are being missed shortly after reboot since LACP trap is not set yet, thus delaying LAG creation and extending dataplane downtime during fast-reboot. | ||
|
|
||
| ## Proposed behavior | ||
|
|
||
| With the new proposal, the CoPP tables contents will be preserved during reboot, i.e. they won't be cleared by DB migrator. Then, initializing CoPP tables in startup phase for any key-value entry it will be checked if such entry exists, in case it does, the entry will be ignored. In case there is an entry with the same key but with different value preserved from prior reboot the existing entry will be deleted and a new entry will be added to the CoPP tables with the key and the new value. | ||
| In addition, for backwards compatibility, in case CoPP tables preserve an entry with a key that is not supported (i.e. such key is not present in the json default initialization file) it will be deleted from the CoPP tables during merge. | ||
arfeigin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| The solution of deleting old entry and creating a new one instead is proposed since there is no SAI implementation to check for overwrites and this might lead to trying to re-create create-only entries which will cause orchagent crash. | ||
|
|
||
| # Flows | ||
|
|
||
| ## DB migrator copp tables handling logic | ||
|
|
||
| The following flow captures the DB migrator proposed functionality. | ||
arfeigin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|  | ||
|
|
||
| ## coppmgr mergeConfig logic | ||
|
|
||
| The following flow captures CoPP manager configuration merge proposed functionality. | ||
|
|
||
arfeigin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|  | ||
|
|
||
| # Manual tests | ||
| 1. Perform fast-reboot with value set according to user's configuration (whether it is the same as default values or not) for one of the CoPP tables contents, in the specific case - LACP. Examine that it is being preserved through reboot process including DB migration and preserves the value after coppmgr merge logic without additional set operation. | ||
| 2. Perform fast-reboot with value that is different from user's configuration (whether user's configuration is same as default value or not) for one of the CoPP tables contents, in the specific case - LACP. Examine that it is being deleted and a new entry is being added to the table instead of it during the coppmgr merge logic. | ||
| 3. Perform reboot to previous SONiC version that doesn't support one of CoPP entries that were found in the table prior to the reboot. Check that the entry is being preserved through DB migration and being cleared in the coppmgr merge logic. | ||
| 4. Remove one of CoPP table entries before rebooting and examine that it is missing on startup and being added as a new entry during coppmgr merge. | ||
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.
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.
Uh oh!
There was an error while loading. Please reload this page.