[QoS] Fix issue: the WRED profile can not be set if current min > new max or current max < new min #2379
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
…r current max < new min Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
stephenxs
changed the title
|
/azpw run |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azpw run |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@neethajohn , @prsunny could you please help to review? |
|
@neethajohn kindly reminder to help review this pr. this is a fix needed for 202205 |
|
/easycla |
neethajohn
approved these changes
Aug 8, 2022
yxieca
pushed a commit
that referenced
this pull request
Aug 8, 2022
… max or current max < new min (#2379) What I did Fix issue: Setting a WRED profile can fail in case the current min threshold is greater than the new max threshold or the current max threshold is less than the new min threshold for any color and at any time. Eg. Current min=1M, max=2M, new min=3M, new max=4M The min threshold is set first, so current max (2M) < new min (3M), which violates the condition This is because there can be only one attribute in each SAI SET operation, which means the vendor SAI does not understand the whole information of the new attributes and can only perform the sanity check against each SET operation. Signed-off-by: Stephen Sun stephens@nvidia.com Why I did it Fix the issue How I verified it Manual test and mock test. Details if related The fix The thresholds that have been applied to SAI will be stored in orchagent. The original logic is to handle each attribute to be set and append it to an attribute list. To resolve the issue, a deferred attribute list is introduced and will be appended to the original attribute list after all the attributes have been handled. In the new logic, each threshold to be set will be checked against the stored data. In case it violates the condition, the violating attribute will be deferred, done via putting it into the deferred attributes list. For any color, there can be only 1 threshold violating the condition. Otherwise, it means both new min > old max and new max > old min, which means either old max < old min or new max < new min, which means either old or new data is illegal. This can not happen because illegal data will not be applied and stored. By doing so, the other threshold will be applied first, which extends the threshold range and breaks the violating condition. A logic is also introduced to guarantee the min threshold is always less than the max threshold in the new profile to be set. For the above example, In the new logic, the min threshold will be deferred so the new max threshold will be applied first and then the new min is applied. In this flow, there is no violation at any time. min = 1M, max = 2M => min = 1M, max = 4M => min = 3M, max = 4M
7 tasks
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.
What I did
Fix issue:
Setting a WRED profile can fail in case
for any color and at any time.
Eg.
This is because there can be only one attribute in each SAI SET operation,
which means the vendor SAI does not understand the whole information of the new attributes and can only perform the sanity check against each SET operation.
Signed-off-by: Stephen Sun stephens@nvidia.com
Why I did it
Fix the issue
How I verified it
Manual test and mock test.
Details if related
The fix
The thresholds that have been applied to SAI will be stored in orchagent.
The original logic is to handle each attribute to be set and append it to an attribute list.
To resolve the issue, a deferred attribute list is introduced and will be appended to the original attribute list after all the attributes have been handled.
In the new logic, each threshold to be set will be checked against the stored data.
In case it violates the condition, the violating attribute will be deferred, done via putting it into the deferred attributes list.
For any color, there can be only 1 threshold violating the condition.
Otherwise, it means both new min > old max and new max > old min, which means either old max < old min or new max < new min, which means either old or new data is illegal.
This can not happen because illegal data will not be applied and stored.
By doing so, the other threshold will be applied first, which extends the threshold range and breaks the violating condition.
A logic is also introduced to guarantee the min threshold is always less than the max threshold in the new profile to be set.
For the above example,
In the new logic, the min threshold will be deferred so the new max threshold will be applied first and then the new min is applied. In this flow, there is no violation at any time.