[show priority-group drop counters] Add user info output when user want to check PG counters and polling are disabled #1678

ayurkiv-nvda · 2021-06-16T19:43:22Z

Signed-off-by: Andriy Yurkiv [email protected]

What I did

Added additional output info for user when trying to show priority group counters and pg drop counters are disabled

How I did it

modify pg-drop script, add additional print during executing "show priority-group drop counters" if PG polling disabled

How to verify it

counterpoll pg-drop disable
show priority-group drop counters

Expect:
Warning: PG counters are disabled. Current stats may be outdated. Use 'counterpoll pg-drop enable' to enable'

Previous command output (if the output of a command-line utility has changed)

admin@r-tigon-04:/usr/local/bin$ show priority-group drop counters
Ingress PG dropped packets:
       Port    PG0    PG1    PG2    PG3    PG4    PG5    PG6    PG7
-----------  -----  -----  -----  -----  -----  -----  -----  -----
  Ethernet0      0      0      0      0      0      0      0      0
  Ethernet2      0      0      0      0      0      0      0      0
  Ethernet8      0      0      0      0      0      0      0      0
 Ethernet10      0      0      0      0      0      0      0      0
 Ethernet16      0      0      0      0      0      0      0      0

New command output (if the output of a command-line utility has changed)

admin@r-tigon-04:/usr/local/bin$ show priority-group drop counters
Warning: PG counters are disabled. Use 'counterpoll pg-drop enable' to enable polling

…nt to check PG counters and polling are disabled Signed-off-by: Andriy Yurkiv <[email protected]>

liat-grozovik · 2021-06-18T07:53:04Z

@neethajohn would like to consult with you on the suggested change. it is a good and informative but should we consider not to show the table in that case and just the note instead? what is the purpose of showing some values when the counter is disabled? in many cases it will not be 0 if was enabled and then disabled so the main question why we need it.

liat-grozovik · 2021-06-23T17:56:26Z

@neethajohn kindly reminder, appreciate your feedback on the above question

liat-grozovik · 2021-06-25T11:24:43Z

/azp run

azure-pipelines · 2021-06-25T11:25:09Z

Azure Pipelines successfully started running 1 pipeline(s).

jleveque · 2021-06-25T16:59:39Z

/azp run

azure-pipelines · 2021-06-25T16:59:48Z

Azure Pipelines successfully started running 1 pipeline(s).

jleveque · 2021-06-25T17:48:25Z

/azp run

azure-pipelines · 2021-06-25T17:48:35Z

Azure Pipelines successfully started running 1 pipeline(s).

neethajohn · 2021-06-30T20:40:20Z

@neethajohn would like to consult with you on the suggested change. it is a good and informative but should we consider not to show the table in that case and just the note instead? what is the purpose of showing some values when the counter is disabled? in many cases it will not be 0 if was enabled and then disabled so the main question why we need it.

I agree with you that showing the table makes no sense when its disabled. Looks like in the current change, we still show the counters along with the warning message. I am fine either way - leaving it there or removing it as long as the warning is there.

liat-grozovik

i suggest to remove the table when it is disabled.

ayurkiv-nvda · 2021-07-12T12:29:04Z

i suggest to remove the table when it is disabled.

Accepted.
Fix is really easy but creating additional unit test still in progress - faced with some difficulties.
I will add fix and appropriate UT soon

…s disabled & add Unit test

ayurkiv-nvda · 2021-07-15T14:20:47Z

i suggest to remove the table when it is disabled.

Done
UT also added

liat-grozovik · 2021-08-10T12:49:59Z

@neethajohn kindly reminder to review and approve. this issue is relevant for active release branches.

neethajohn

Please update the 'output' in the PR description to reflect this new change of not displaying counters

ayurkiv-nvda · 2021-08-11T15:23:39Z

Please update the 'output' in the PR description to reflect this new change of not displaying counters

Done

liat-grozovik · 2021-08-11T15:31:46Z

@qiluo-msft and @abdosi please check if this fix is required for 202012 and 201911

…nt to check PG counters and polling are disabled (#1678) Signed-off-by: Andriy Yurkiv <[email protected]> What I did Added additional output info for user when trying to show priority group counters and pg drop counters are disabled How I did it modify pg-drop script, add additional print during executing "show priority-group drop counters" if PG polling disabled How to verify it counterpoll pg-drop disable show priority-group drop counters Expect: Warning: PG counters are disabled. Current stats may be outdated. Use 'counterpoll pg-drop enable' to enable' Previous command output (if the output of a command-line utility has changed) admin@r-tigon-04:/usr/local/bin$ show priority-group drop counters Ingress PG dropped packets: Port PG0 PG1 PG2 PG3 PG4 PG5 PG6 PG7 ----------- ----- ----- ----- ----- ----- ----- ----- ----- Ethernet0 0 0 0 0 0 0 0 0 Ethernet2 0 0 0 0 0 0 0 0 Ethernet8 0 0 0 0 0 0 0 0 Ethernet10 0 0 0 0 0 0 0 0 Ethernet16 0 0 0 0 0 0 0 0 New command output (if the output of a command-line utility has changed) admin@r-tigon-04:/usr/local/bin$ show priority-group drop counters Warning: PG counters are disabled. Use 'counterpoll pg-drop enable' to enable polling

Update sonic-utilities submodule to latest in 202012 branch: [show priority-group drop counters] Add user info output when user want to check PG counters and polling are disabled sonic-net/sonic-utilities#1678 [route_check] Filter out VNET routes sonic-net/sonic-utilities#1612 [Show] Update the subcommands of Kdump. sonic-net/sonic-utilities#1682 Add mock support for swsscommon classes sonic-net/sonic-utilities#1780 [acl_loader]: add iptype match to the rules for dataplane acl sonic-net/sonic-utilities@205aff8 [202012][fast-reboot] Remove FLEX_COUNTER_TABLE from config_db.json before fast-reboot sonic-net/sonic-utilities#1774

[show priority-group drop counters] Add user info output when user wa…

1c2f21d

…nt to check PG counters and polling are disabled Signed-off-by: Andriy Yurkiv <[email protected]>

liat-grozovik requested a review from neethajohn June 18, 2021 07:53

liat-grozovik marked this pull request as ready for review June 25, 2021 11:32

neethajohn previously approved these changes Jul 1, 2021

View reviewed changes

liat-grozovik requested changes Jul 7, 2021

View reviewed changes

[show priority-group drop counters] Do not show table when PG counter…

3b772ae

…s disabled & add Unit test

ayurkiv-nvda dismissed neethajohn’s stale review via 3b772ae July 15, 2021 06:23

liat-grozovik approved these changes Jul 18, 2021

View reviewed changes

neethajohn approved these changes Aug 10, 2021

View reviewed changes

liat-grozovik added Enhancement Request for 202106 Branch labels Aug 11, 2021

neethajohn added the Request for 202012 Branch label Aug 11, 2021

neethajohn merged commit 4175cb9 into sonic-net:master Aug 11, 2021

qiluo-msft added the Included in 202012 Branch label Aug 12, 2021

judyjoseph added the Included in 202106 Branch label Aug 20, 2021

This was referenced Aug 24, 2021

Fix ConfigDBConnector import in pg-drop script #1779

Closed

[advance submodule] Update sonic-utilities 202012 head to latest sonic-net/sonic-buildimage#8590

Merged

ayurkiv-nvda mentioned this pull request May 25, 2022

[201911] [Flex Counters] add CLI for PG drop packets counters (counterpoll, show/clear counters) #2155

Merged

ayurkiv-nvda deleted the pg_drop_disabled_azure branch May 30, 2023 21:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[show priority-group drop counters] Add user info output when user want to check PG counters and polling are disabled #1678

[show priority-group drop counters] Add user info output when user want to check PG counters and polling are disabled #1678

ayurkiv-nvda commented Jun 16, 2021 •

edited

Loading

liat-grozovik commented Jun 18, 2021

liat-grozovik commented Jun 23, 2021

liat-grozovik commented Jun 25, 2021

azure-pipelines bot commented Jun 25, 2021

jleveque commented Jun 25, 2021

azure-pipelines bot commented Jun 25, 2021

jleveque commented Jun 25, 2021

azure-pipelines bot commented Jun 25, 2021

neethajohn commented Jun 30, 2021

liat-grozovik left a comment

ayurkiv-nvda commented Jul 12, 2021

ayurkiv-nvda commented Jul 15, 2021 •

edited

Loading

liat-grozovik commented Aug 10, 2021

neethajohn left a comment

ayurkiv-nvda commented Aug 11, 2021

liat-grozovik commented Aug 11, 2021

[show priority-group drop counters] Add user info output when user want to check PG counters and polling are disabled #1678

[show priority-group drop counters] Add user info output when user want to check PG counters and polling are disabled #1678

Conversation

ayurkiv-nvda commented Jun 16, 2021 • edited Loading

What I did

How I did it

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

liat-grozovik commented Jun 18, 2021

liat-grozovik commented Jun 23, 2021

liat-grozovik commented Jun 25, 2021

azure-pipelines bot commented Jun 25, 2021

jleveque commented Jun 25, 2021

azure-pipelines bot commented Jun 25, 2021

jleveque commented Jun 25, 2021

azure-pipelines bot commented Jun 25, 2021

neethajohn commented Jun 30, 2021

liat-grozovik left a comment

Choose a reason for hiding this comment

ayurkiv-nvda commented Jul 12, 2021

ayurkiv-nvda commented Jul 15, 2021 • edited Loading

liat-grozovik commented Aug 10, 2021

neethajohn left a comment

Choose a reason for hiding this comment

ayurkiv-nvda commented Aug 11, 2021

liat-grozovik commented Aug 11, 2021

ayurkiv-nvda commented Jun 16, 2021 •

edited

Loading

ayurkiv-nvda commented Jul 15, 2021 •

edited

Loading