Skip to content
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

[202205] [QoS] Enforce drop probability only for colors whose WRED are enabled (#2422) #2457

Closed
wants to merge 1 commit into from

Conversation

stephenxs
Copy link
Collaborator

@stephenxs stephenxs commented Sep 19, 2022

What I did

This is to cherry-pick #2422 to 202205 branch

Why I did it

Currently, there is a logic to enforce the drop probability if it is not explicitly designated for a color. However, the drop probability is not a mandatory attribute. It can incur vendor SAI complaints to set it when the color is disabled.
The logic was introduced from the very beginning (by PR #571) because no drop probability was defined in the QoS template at the time, which is no longer true.
So we will enforce drop probability only if it is not configured and the color is enabled.

How I verified it

Unit test and manual test

Details if related

…sonic-net#2422)

What I did
Do not enforce drop probability for a color whose WRED is disabled.

Signed-off-by: Stephen Sun stephens@nvidia.com

Why I did it
Currently, there is a logic to enforce the drop probability if it is not explicitly designated for a color. However, the drop probability is not a mandatory attribute. It can incur vendor SAI complaints to set it when the color is disabled.
The logic was introduced from the very beginning (by PR sonic-net#571) because no drop probability was defined in the QoS template at the time, which is no longer true.
So we will enforce drop probability only if it is not configured and the color is enabled.

How I verified it
Unit test and manual test
@liat-grozovik liat-grozovik marked this pull request as ready for review September 19, 2022 12:14
@liat-grozovik liat-grozovik changed the title [QoS] Enforce drop probability only for colors whose WRED are enabled (#2422) [202205] [QoS] Enforce drop probability only for colors whose WRED are enabled (#2422) Sep 19, 2022
@stephenxs
Copy link
Collaborator Author

stephenxs commented Sep 20, 2022

failed due to an irrelevant test case (warm reboot). retrying.

@stephenxs
Copy link
Collaborator Author

all test cases passed but the pipeline failed due to

ApplicationInsightsTelemetrySender correlated 2 events with X-TFS-Session 4720fc4d-dd26-4bb0-a785-f8eda1852bd1
##[error]Artifact sonic-gcov already exists for build 150304.
Finishing: Publish gcov output

@liushilongbuaa
Copy link
Contributor

all test cases passed but the pipeline failed due to

ApplicationInsightsTelemetrySender correlated 2 events with X-TFS-Session 4720fc4d-dd26-4bb0-a785-f8eda1852bd1
##[error]Artifact sonic-gcov already exists for build 150304.
Finishing: Publish gcov output

Test is not finished.
If only Gcov failed, go ahead.

@stephenxs
Copy link
Collaborator Author

all test cases passed but the pipeline failed due to

ApplicationInsightsTelemetrySender correlated 2 events with X-TFS-Session 4720fc4d-dd26-4bb0-a785-f8eda1852bd1
##[error]Artifact sonic-gcov already exists for build 150304.
Finishing: Publish gcov output

Test is not finished. If only Gcov failed, go ahead.

Thanks.
The test had been finished with the only error like the above. I retriggered it later on...

@stephenxs
Copy link
Collaborator Author

Retried and the same. only Gcov failed. all test cases passed
image

@liushilongbuaa
Copy link
Contributor

Retried and the same. only Gcov failed. all test cases passed image

I see. Rerun will not help. It is pipeline issue.
I will fix it.

@stephenxs
Copy link
Collaborator Author

Closing this one as the original PR has been cherry picked to 202205.

@stephenxs stephenxs closed this Sep 21, 2022
@stephenxs stephenxs deleted the cherry-pick-2422 branch September 21, 2022 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants