[buffer orch] Bugfix: Don't query counter SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported by stephenxs · Pull Request #1857 · sonic-net/sonic-swss

stephenxs · 2021-08-06T10:33:32Z

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 [email protected]

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 <[email protected]>

Signed-off-by: Stephen Sun <[email protected]>

The latter is not supported by all vendors Signed-off-by: Stephen Sun <[email protected]>

liat-grozovik · 2021-08-11T13:41:55Z

/azp run

azure-pipelines · 2021-08-11T13:42:07Z

Azure Pipelines successfully started running 1 pipeline(s).

liat-grozovik · 2021-08-11T13:42:31Z

@neethajohn kindly reminder to review this PR

stephenxs · 2021-08-13T03:35:39Z

/azpw

liat-grozovik · 2021-08-17T14:38:45Z

/azp run

azure-pipelines · 2021-08-17T14:38:55Z

Azure Pipelines successfully started running 1 pipeline(s).

prsunny · 2021-08-23T16:39:08Z

@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 <[email protected]>

stephenxs · 2021-08-26T23:07:55Z

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

orchagent/bufferorch.cpp

Fix review comments.

orchagent/bufferorch.cpp

Signed-off-by: Stephen Sun <[email protected]>

qiluo-msft

LGTM

…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 <[email protected]> How I verified it Run vstest and manually test.

…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 <[email protected]> How I verified it Run vstest and manually test.

…T_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported (sonic-net#1857)" This reverts commit 3d6b1f0.

…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.

…POOL_STAT_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported (sonic-net#1857)" (sonic-net#1945)" This reverts commit 94d2d44.

…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 <[email protected]> How I verified it Run vstest and manually test.

…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.

* 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 <[email protected]>

…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 <[email protected]> How I verified it Run vstest and manually test.

…T_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported (sonic-net#1857)" (sonic-net#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 2 commits August 6, 2021 11:25

Fix comments

d58455b

Signed-off-by: Stephen Sun <[email protected]>

stephenxs requested a review from prsunny as a code owner August 6, 2021 10:33

stephenxs requested review from neethajohn and removed request for prsunny August 6, 2021 10:33

stephenxs added Bug 🐛 Request for 202012 Branch Request for 202106 Branch labels Aug 6, 2021

Use get_buffer_pool_stats instead of get_buffer_pool_stats_ext

87cf819

The latter is not supported by all vendors Signed-off-by: Stephen Sun <[email protected]>

kcudnik requested review from prsunny and qiluo-msft August 23, 2021 15:59

prsunny removed the Request for 202012 Branch label Aug 23, 2021

neethajohn reviewed Aug 27, 2021

View reviewed changes

orchagent/bufferorch.cpp Outdated Show resolved Hide resolved

orchagent/bufferorch.cpp Outdated Show resolved Hide resolved

orchagent/bufferorch.cpp Outdated Show resolved Hide resolved

Update bufferorch.cpp

628949e

Fix review comments.

neethajohn previously approved these changes Aug 28, 2021

View reviewed changes

qiluo-msft reviewed Aug 30, 2021

View reviewed changes

orchagent/bufferorch.cpp Outdated Show resolved Hide resolved

Replace all "Shp" to "SharedHeadroomPool" for better understanding

c7835cc

Signed-off-by: Stephen Sun <[email protected]>

stephenxs dismissed neethajohn’s stale review via c7835cc August 30, 2021 08:11

stephenxs requested review from neethajohn and qiluo-msft August 30, 2021 10:03

qiluo-msft approved these changes Aug 30, 2021

View reviewed changes

neethajohn approved these changes Aug 30, 2021

View reviewed changes

neethajohn merged commit 3d6b1f0 into sonic-net:master Aug 30, 2021

stephenxs deleted the fix-shp-watermark-not-supported branch August 30, 2021 22:49

judyjoseph added the Included in 202106 Branch label Sep 2, 2021

yxieca mentioned this pull request Sep 14, 2021

[warmboot] Recovery path failed when syncd hits "Runtime error: :- translateVidToRid" sonic-net/sonic-buildimage#8722

Closed

vaibhavhd added a commit to vaibhavhd/sonic-swss that referenced this pull request Oct 6, 2021

Revert "[buffer orch] Bugfix: Don't query counter SAI_BUFFER_POOL_STA…

710fa0f

…T_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported (sonic-net#1857)" This reverts commit 3d6b1f0.

vaibhavhd mentioned this pull request Oct 6, 2021

Revert "[buffer orch] Bugfix: Don't query counter SAI_BUFFER_POOL_STA… #1945

Merged

stephenxs mentioned this pull request Dec 14, 2021

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

[buffer orch] Bugfix: Don't query counter SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES on a pool where it is not supported#1857
neethajohn merged 6 commits intosonic-net:masterfrom
stephenxs:fix-shp-watermark-not-supported

stephenxs commented Aug 6, 2021

Uh oh!

liat-grozovik commented Aug 11, 2021

Uh oh!

azure-pipelines bot commented Aug 11, 2021

Uh oh!

liat-grozovik commented Aug 11, 2021

Uh oh!

stephenxs commented Aug 13, 2021

Uh oh!

liat-grozovik commented Aug 17, 2021

Uh oh!

azure-pipelines bot commented Aug 17, 2021

Uh oh!

prsunny commented Aug 23, 2021

Uh oh!

stephenxs commented Aug 26, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qiluo-msft left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

stephenxs commented Aug 6, 2021

Uh oh!

liat-grozovik commented Aug 11, 2021

Uh oh!

azure-pipelines bot commented Aug 11, 2021

Uh oh!

liat-grozovik commented Aug 11, 2021

Uh oh!

stephenxs commented Aug 13, 2021

Uh oh!

liat-grozovik commented Aug 17, 2021

Uh oh!

azure-pipelines bot commented Aug 17, 2021

Uh oh!

prsunny commented Aug 23, 2021

Uh oh!

stephenxs commented Aug 26, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants