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

[buffer orch] Bugfix: Don't query counter SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported #1857

Merged

Conversation

stephenxs
Copy link
Collaborator

What I did
Currently, SAI_BUFFER_POOL_STAT_WATERMARK_BYTES and SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES are queried for buffer pools.
However, the latter is not supported on all pools and all platforms, which will result in sairedis complaint like this:

Jun 17 23:31:34.949613 sonic ERR syncd#SDK: :- updateSupportedBufferPoolCounters: BUFFER_POOL_WATERMARK_STAT_COUNTER: counter SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES is not supported on buffer pool oid:0x1000000018, rv: SAI_STATUS_NOT_IMPLEMENTED
Jun 17 23:31:34.951915 sonic ERR syncd#SDK: :- updateSupportedBufferPoolCounters: BUFFER_POOL_WATERMARK_STAT_COUNTER: counter SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES is not supported on buffer pool oid:0x1200000018, rv: SAI_STATUS_NOT_IMPLEMENTED
Jun 17 23:31:34.954177 sonic ERR syncd#SDK: :- updateSupportedBufferPoolCounters: BUFFER_POOL_WATERMARK_STAT_COUNTER: counter SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES is not supported on buffer pool oid:0x400000018, rv: SAI_STATUS_NOT_IMPLEMENTED

To avoid that, we need to test whether it is supported before putting it to FLEX_COUNTER_DB

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

Why I did it

How I verified it
Run vstest and manually test.

Details if related

…f it is not supported by a pool

Currently, SAI_BUFFER_POOL_STAT_WATERMARK_BYTES and SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES are queried for buffer pools.
However, the latter is not supported on all pools and all platforms, which will results in sairedis complaint
To avoid that, we need to test whether it is supported before putting it to FLEX_COUNTER table

Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
The latter is not supported by all vendors

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@neethajohn kindly reminder to review this PR

@stephenxs
Copy link
Collaborator Author

/azpw

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Collaborator

prsunny commented Aug 23, 2021

@neethajohn , could you please the code? Also, I dont think we can backport to 202012 as this might break other platforms implementations. We already had an issue backporting this PR - #1789. Hence, removing label.

…rk polling

- If shared headroom pool watermark is not supported,
  only buffer pool watermark will be tested
- Otherwise, both buffer pool watermark and shared headroom pool watermark will be checked

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Copy link
Collaborator Author

@neethajohn fyi. both clear stat and watermarkstat work correctly with this PR merged.

orchagent/bufferorch.cpp Outdated Show resolved Hide resolved
orchagent/bufferorch.cpp Outdated Show resolved Hide resolved
orchagent/bufferorch.cpp Outdated Show resolved Hide resolved
Fix review comments.
neethajohn
neethajohn previously approved these changes Aug 28, 2021
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@neethajohn neethajohn merged commit 3d6b1f0 into sonic-net:master Aug 30, 2021
@stephenxs stephenxs deleted the fix-shp-watermark-not-supported branch August 30, 2021 22:49
judyjoseph pushed a commit that referenced this pull request Sep 2, 2021
…OOM_WATERMARK_BYTES on a pool where it is not supported (#1857)

* [Bugfix] Don't query SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES if it is not supported by a pool

Currently, SAI_BUFFER_POOL_STAT_WATERMARK_BYTES and SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES are queried for buffer pools.
However, the latter is not supported on all pools and all platforms, which will results in sairedis complaint
To avoid that, we need to test whether it is supported before putting it to FLEX_COUNTER table

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

How I verified it
Run vstest and manually test.
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…OOM_WATERMARK_BYTES on a pool where it is not supported (sonic-net#1857)

* [Bugfix] Don't query SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES if it is not supported by a pool

Currently, SAI_BUFFER_POOL_STAT_WATERMARK_BYTES and SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES are queried for buffer pools.
However, the latter is not supported on all pools and all platforms, which will results in sairedis complaint
To avoid that, we need to test whether it is supported before putting it to FLEX_COUNTER table

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

How I verified it
Run vstest and manually test.
vaibhavhd added a commit to vaibhavhd/sonic-swss that referenced this pull request Oct 6, 2021
…T_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported (sonic-net#1857)"

This reverts commit 3d6b1f0.
vaibhavhd added a commit that referenced this pull request Oct 6, 2021
…T_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported (#1857)" (#1945)

This reverts commit 3d6b1f0.

Fix sonic-net/sonic-buildimage#8893

What I did
This commit had earlier caused issue on master image warmboot - sonic-net/sonic-buildimage#8722

To fix this issue, this PR was created to retreat sonic-swss head on buildimage - sonic-net/sonic-buildimage#8732

Now, this commit was again pulled into sonic-buildimage as part of sonic-swss submodule advance:
sonic-net/sonic-buildimage#8839

And, warm-reboot again broke for the same reason.

This change is so that any other swss submodule update on buildimage will not fail warmboot again.
judyjoseph pushed a commit that referenced this pull request Oct 7, 2021
…T_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported (#1857)" (#1945)

This reverts commit 3d6b1f0.

Fix sonic-net/sonic-buildimage#8893

What I did
This commit had earlier caused issue on master image warmboot - sonic-net/sonic-buildimage#8722

To fix this issue, this PR was created to retreat sonic-swss head on buildimage - sonic-net/sonic-buildimage#8732

Now, this commit was again pulled into sonic-buildimage as part of sonic-swss submodule advance:
sonic-net/sonic-buildimage#8839

And, warm-reboot again broke for the same reason.

This change is so that any other swss submodule update on buildimage will not fail warmboot again.
stephenxs added a commit to stephenxs/sonic-swss that referenced this pull request Dec 14, 2021
…POOL_STAT_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported (sonic-net#1857)" (sonic-net#1945)"

This reverts commit 94d2d44.
stephenxs added a commit to stephenxs/sonic-swss that referenced this pull request Jan 7, 2022
…OOM_WATERMARK_BYTES on a pool where it is not supported (sonic-net#1857)

* [Bugfix] Don't query SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES if it is not supported by a pool

Currently, SAI_BUFFER_POOL_STAT_WATERMARK_BYTES and SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES are queried for buffer pools.
However, the latter is not supported on all pools and all platforms, which will results in sairedis complaint
To avoid that, we need to test whether it is supported before putting it to FLEX_COUNTER table

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

How I verified it
Run vstest and manually test.
liat-grozovik pushed a commit that referenced this pull request Jan 24, 2022
…OOM_WATERMARK_BYTES on a pool where it is not supported (#1857) (#2106)

- What I did
Currently, SAI_BUFFER_POOL_STAT_WATERMARK_BYTES and SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES are queried for buffer pools.
However, the latter is not supported on all pools and all platforms, which will results in sairedis complaint
To avoid that, we need to test whether it is supported before putting it to FLEX_COUNTER table

It depends on #1987 to be cherry-picked to 202012.

- Why I did it
[Bugfix] Don't query SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES if it is not supported by a pool

- How I verified it
Run vstest and manually test.
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
* Added portchannels support for static routes.
* Added check if nexthop is a portchannel and verify if this portchannel exists in config db.

Signed-off-by: d-dashkov <Dmytro_Dashkov@Jabil.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants