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

Don't query buffer profile attributes before APPLY_VIEW #2231

Open
kcudnik opened this issue Apr 16, 2022 · 7 comments
Open

Don't query buffer profile attributes before APPLY_VIEW #2231

kcudnik opened this issue Apr 16, 2022 · 7 comments
Assignees

Comments

@kcudnik
Copy link
Contributor

kcudnik commented Apr 16, 2022

Change required to not query SAI_QUEUE_ATTR_BUFFER_PROFILE_ID and SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE before APPLY_VIEW, because in warm boot scenario is bringing previous buffer profile into OA scope, and this is causing extra buffer profile to be created in syncd.

Issue in depth is described here: sonic-net/sonic-sairedis#1030 (comment)

@svsivm
Copy link

svsivm commented Jun 9, 2022

Hello, do we have any update on this one? Thanks!

@kcudnik
Copy link
Contributor Author

kcudnik commented Jun 9, 2022

@prsunny ping

@prsunny
Copy link
Collaborator

prsunny commented Jun 9, 2022

I see the original issue refers to zero-buffer pools. A related change is here - #2304. @neethajohn , @stephenxs , can you check if its related?

@stephenxs
Copy link
Collaborator

I see the original issue refers to zero-buffer pools. A related change is here - #2304. @neethajohn , @stephenxs , can you check if its related?

I’ll check it. Originally the purpose was to share the zero buffer profile by PFCWD with the existing one. Now that we will not apply zero buffer profile on ingress side, this change is not required

@svsivm
Copy link

svsivm commented Jun 10, 2022

#2304

Hi, I doubt if this problem has anything to do with #2304. #2231 is seen specifically when warmboot is done while PFC storm is ongoing. After warmboot we see 4 zero buffer profiles instead of the expected and that throws things into disarray.

@svsivm
Copy link

svsivm commented Jun 10, 2022

I see the original issue refers to zero-buffer pools. A related change is here - #2304. @neethajohn , @stephenxs , can you check if its related?

I’ll check it. Originally the purpose was to share the zero buffer profile by PFCWD with the existing one. Now that we will not apply zero buffer profile on ingress side, this change is not required

Hi Stephen, can you please look at #2319. Why were the zero buffer profile attribute values changed as part of #2185? Thanks.

@stephenxs
Copy link
Collaborator

I see the original issue refers to zero-buffer pools. A related change is here - #2304. @neethajohn , @stephenxs , can you check if its related?

I’ll check it. Originally the purpose was to share the zero buffer profile by PFCWD with the existing one. Now that we will not apply zero buffer profile on ingress side, this change is not required

Hi Stephen, can you please look at #2319. Why were the zero buffer profile attribute values changed as part of #2185? Thanks.

Hi I'm working on analyzing the issue. Will let you know once I have an answer

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

No branches or pull requests

4 participants