Clear fabric counters queue/port #2892

kenneth-arista · 2023-07-05T06:08:19Z

What I did

This PR is a clone of #2789.

Added two clear commands for fabric counters queue and fabric counters port.

sonic-clear fabriccountersport
sonic-clear fabriccountersqueue

Fabric counters are cleared and saved with these two commands. For example,

# show fabric counters port
  ASIC    PORT    STATE    IN_CELL    IN_OCTET    OUT_CELL    OUT_OCTET    CRC    FEC_CORRECTABLE    FEC_UNCORRECTABLE    SYMBOL_ERR
------  ------  -------  ---------  ----------  ----------  -----------  -----  -----------------  -------------------  ----------
...
     0      49       up          1         244           0            0       0                  2        2,372,752,496               0
     0      50       up          2         315           1          135       0                  4        2,522,457,120               4
...

# sonic-clear fabriccountersport
Clear and update saved counters port

# show fabric counters port
  ASIC    PORT    STATE    IN_CELL    IN_OCTET    OUT_CELL    OUT_OCTET    CRC    FEC_CORRECTABLE    FEC_UNCORRECTABLE    SYMBOL_ERR
------  ------  -------  ---------  ----------  ----------  -----------  -----  -----------------  -------------------  ------------
...
    0      49       up          0           0           0            0      0                  0                    0             0
    0      50       up          0           0           0            0      0                  0                    0             0

kenneth-arista · 2023-07-05T06:08:55Z

@jfeng-arista

kenneth-arista · 2023-07-10T15:58:53Z

@arlakshm

kenneth-arista · 2023-07-10T15:59:46Z

@gechiang

This PR is a clone of sonic-net#2789. Added two clear commands for fabric counters queue and fabric counters port. sonic-clear fabriccountersport sonic-clear fabriccountersqueue

kenneth-arista · 2023-07-18T19:06:01Z

/azpw run Azure.sonic-utilities

mssonicbld · 2023-07-18T19:06:04Z

/AzurePipelines run Azure.sonic-utilities

azure-pipelines · 2023-07-18T19:06:13Z

Azure Pipelines successfully started running 1 pipeline(s).

kenneth-arista · 2023-07-19T06:40:19Z

The test_clear_tag failure doesn't appear to be related to this change. The error is as follows:

E       assert '          IFACE    RX_OK       RX_BPS    RX_PPS    RX_ERR    TX_OK      TX_BPS    TX_PPS    TX_ERR\n---------------  ...0.00/s         0\n       Vlan1000        0     0.00 B/s    0.00/s         0        0    0.00 B/s    0.00/s         0\n' in "\nFile '/tmp/cache/intfstat/1001-test/intfstat' does not exist\nDid you run 'intfstat -c -t test' to record the counters via tag test?\n\n"

jfeng-arista · 2023-07-19T16:27:26Z

/azpw run Azure.sonic-utilities

jfeng-arista · 2023-07-19T16:28:31Z

/AzurePipelines run Azure.sonic-utilities

azure-pipelines · 2023-07-19T16:28:36Z

Commenter does not have sufficient privileges for PR 2892 in repo sonic-net/sonic-utilities

gechiang · 2023-07-19T16:34:08Z

Restarted failed job

gechiang · 2023-07-19T20:36:11Z

@qiluo-msft , many PRs are all failing on this test:

2023-07-19T16:42:39.2354382Z tests/intfstat_test.py::TestIntfstat::test_clear_tag FAILED [ 40%]

Is this something broken due to some recent change that went in? How do we unblock this PR where this failure is not related to the PR changes?

Thanks!

gechiang · 2023-07-19T21:26:42Z

/easyCLA

gechiang · 2023-07-19T21:27:25Z

@kenneth-arista , can you update your branch to fix the outdated branch issue?
Thanks!

kenneth-arista · 2023-07-19T22:46:11Z

Looks like @qiluo-msft already merged master into the branch. Thanks!

kenneth-arista · 2023-07-21T05:37:51Z

@StormLiangMS this PR supersedes #2789 and should be backported to 202305

arlakshm · 2023-08-09T00:15:46Z

/Azp run Azure.sonic-utilities

azure-pipelines · 2023-08-09T00:15:55Z

Azure Pipelines successfully started running 1 pipeline(s).

gechiang

LGTM.

gechiang · 2023-08-09T00:43:02Z

@qiluo-msft , can you help review this PR?
Thanks!

qiluo-msft · 2023-08-15T00:17:54Z

scripts/fabricstat

+            cnstat_fqn_file_port_name = cnstat_fqn_file_port + asic_name
+            json.dump(cnstat_dict, open(cnstat_fqn_file_port_name, 'w'), default=json_serial)
+        except IOError as e:
+            print(e.errno, e)


print

print to stderr. #WontFix

I can change it, just that I see other commands print the same IOError like this, so I was trying to get the same style as those. Please let me know if we still want to print to stderr or like this. thank you

qiluo-msft · 2023-08-15T00:18:52Z

scripts/fabricstat

+            try:
+                cnstat_cached_dict = json.load(open(cnstat_fqn_file_port_name, 'r'))
+            except IOError as e:
+                print(e.errno, e)


print

print to stderr. #WontFix

I can change it, just that I see other commands print the same IOError like this, so I was trying to get the same style as those. Please let me know if we still want to print to stderr or like this. thank you

qiluo-msft · 2023-08-15T00:19:49Z

scripts/fabricstat

+        if os.path.isfile(cnstat_fqn_file_port_name):
+            try:
+                cnstat_cached_dict = json.load(open(cnstat_fqn_file_port_name, 'r'))
+            except IOError as e:


except

When exception is caught, do you want to exit earlier? #WontFix

If the file not exist, I think we can still print out the counters ( with no cleaned ) , instead of print out nothing .
I read the clean code for other counters ( not exactly the same code piece as what I wrote ), but when they don't have the cached file, it still print out the uncleared counters. Please suggest if this behavior is ok or you want to print out nothing in this case. thank you

qiluo-msft · 2023-08-15T00:20:42Z

scripts/fabricstat

+    def save_fresh_stats(self):
+        # Get stat for each port and save
+        counter_port_name_map = self.db.get_all(self.db.COUNTERS_DB, COUNTERS_FABRIC_PORT_NAME_MAP)
+        if counter_port_name_map is None:


counter_port_name_map

not used. is it needed? if yes, do you want to print some info before return? #Closed

printed out a message before the return

qiluo-msft · 2023-08-15T00:21:16Z

scripts/fabricstat

+        if os.path.isfile(cnstat_fqn_file_queue_name):
+            try:
+                cnstat_cached_dict = json.load(open(cnstat_fqn_file_queue_name, 'r'))
+            except IOError as e:


except

do you want to return earlier if exception caught? #WontFix

If the file not exist, I think we can still print out the counters ( with no cleaned ) , instead of print out nothing .
I read the clean code for other counters ( not exactly the same code piece as what I wrote ), but when they don't have the cached file, it still print out the uncleared counters. Please suggest if this behavior is ok or you want to print out nothing in this case. thank you

qiluo-msft

LGTM. Please also check with other active reviewers.

jfeng-arista · 2023-08-18T23:52:51Z

The change looks good to me

rlhui · 2023-08-23T05:17:14Z

@arlakshm please help create a msft ado if it helps to speed up cherry-pick, thx

gechiang · 2023-08-27T21:26:19Z

@kenneth-arista , This PR when cherry-picked to the msft repo it is causing build issue as described by @mlok-nokia
this is causing build issues where PR is failing on
tests/fabricstat_test.py::TestFarbicStat::test_single_clear_fabric_counter
tests/fabricstat_test.py::TestMultiAsicFabricStat::test_multi_show_fabric_counters_queue_clear

I have reverted this PR from the msft repo and removing the "included in chassis for 202205 branch".
@kenneth-arista , please help investigate why this is an issue... you can try re-include this PR on the "https://github.com/Azure/sonic-buildimage-msft/tree/202205" repo and see the build hitting PR test failure issue.
Once this PR is reverted, then it works fine.

gechiang · 2023-08-30T01:10:45Z

@arlakshm , please help follow up with Arista team (@kenneth-arista) on the issue that this PR is causing on the azure/sonic-buildimage-msft 202205 branch. I have reverted this Change from the sonic-utilities.msft 202205 repo so the most current snapshot on "azure/sonic-buildimage-msft 202205 branch" no longer has this problem PR. BUt we need this be investigated and fix so we can bring back this fix to "azure/spmoc-utilities.msft 202205" and "azure/sonic-buildimage-msft 202205" repos.

jfeng-arista · 2023-08-30T01:21:49Z

I can work on this and create a PR of clear fabric counters in 202205 branch. Please let me know which branches should I work on . thank you

gechiang · 2023-08-30T01:34:37Z

I can work on this and create a PR of clear fabric counters in 202205 branch. Please let me know which branches should I work on . thank you

Can you raise a PR in this repo:
https://github.com/Azure/sonic-utilities.msft/tree/202205

And make sure you test it (build it) under this repo:
https://github.com/Azure/sonic-buildimage-msft/tree/202205

If it addresses the build PR test failure issues, then it is ready.
Appreciate your extra effort @jfeng-arista
Thanks!

jfeng-arista · 2023-08-30T01:36:16Z

got it . thank you !

jfeng-arista · 2023-09-01T06:04:35Z

Azure/sonic-utilities.msft#26

gechiang · 2023-09-30T21:48:00Z

Adding back the label to indicate it got merged in MSFT repo.

rlhui requested a review from arlakshm July 12, 2023 17:06

rlhui assigned kenneth-arista Jul 12, 2023

kenneth-arista added 2 commits July 17, 2023 13:13

Clear fabric counters queue/port

621b275

This PR is a clone of sonic-net#2789. Added two clear commands for fabric counters queue and fabric counters port. sonic-clear fabriccountersport sonic-clear fabriccountersqueue

Fixed params to get_result_and_return_code

fa00dd6

kenneth-arista force-pushed the master-clear-fabric-counters branch from 60468bc to fa00dd6 Compare July 17, 2023 20:13

Correct command definition

98dd3d6

Merge branch 'master' into master-clear-fabric-counters

72aaca6

kenneth-arista mentioned this pull request Jul 21, 2023

Clear fabric counters queue/port #2789

Merged

arlakshm previously approved these changes Aug 9, 2023

View reviewed changes

gechiang previously approved these changes Aug 9, 2023

View reviewed changes

gechiang requested a review from qiluo-msft August 9, 2023 00:42

qiluo-msft reviewed Aug 15, 2023

View reviewed changes

Add a message before early return when issuing the clear counter command

95ad6e3

jfeng-arista dismissed stale reviews from gechiang and arlakshm via 95ad6e3 August 15, 2023 21:56

rlhui added the p0 label Aug 18, 2023

qiluo-msft approved these changes Aug 18, 2023

View reviewed changes

gechiang requested review from arlakshm and jfeng-arista August 18, 2023 23:45

gechiang approved these changes Aug 18, 2023

View reviewed changes

jfeng-arista approved these changes Aug 18, 2023

View reviewed changes

arlakshm approved these changes Aug 19, 2023

View reviewed changes

arlakshm merged commit 7f8779d into sonic-net:master Aug 19, 2023

kenneth-arista deleted the master-clear-fabric-counters branch August 22, 2023 21:33

gechiang added the included in chassis for 202205 branch indicate that this PR got merged into the "chassis for 202205 branch" label Aug 23, 2023

gechiang removed the included in chassis for 202205 branch indicate that this PR got merged into the "chassis for 202205 branch" label Aug 27, 2023

jfeng-arista mentioned this pull request Sep 1, 2023

Clear fabric counters queue/port Azure/sonic-utilities.msft#26

Merged

gechiang added the included in chassis for 202205 branch indicate that this PR got merged into the "chassis for 202205 branch" label Sep 30, 2023

Clear fabric counters queue/port #2892

Clear fabric counters queue/port #2892

Conversation

kenneth-arista commented Jul 5, 2023 • edited Loading

What I did

kenneth-arista commented Jul 5, 2023

kenneth-arista commented Jul 10, 2023

kenneth-arista commented Jul 10, 2023

kenneth-arista commented Jul 18, 2023

mssonicbld commented Jul 18, 2023

azure-pipelines bot commented Jul 18, 2023

kenneth-arista commented Jul 19, 2023

jfeng-arista commented Jul 19, 2023

jfeng-arista commented Jul 19, 2023

azure-pipelines bot commented Jul 19, 2023

gechiang commented Jul 19, 2023

gechiang commented Jul 19, 2023

gechiang commented Jul 19, 2023

gechiang commented Jul 19, 2023

kenneth-arista commented Jul 19, 2023

kenneth-arista commented Jul 21, 2023

arlakshm commented Aug 9, 2023

azure-pipelines bot commented Aug 9, 2023

gechiang left a comment

Choose a reason for hiding this comment

gechiang commented Aug 9, 2023

qiluo-msft Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

jfeng-arista Aug 15, 2023

Choose a reason for hiding this comment

qiluo-msft Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

jfeng-arista Aug 15, 2023

Choose a reason for hiding this comment

qiluo-msft Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

jfeng-arista Aug 15, 2023

Choose a reason for hiding this comment

qiluo-msft Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

jfeng-arista Aug 15, 2023

Choose a reason for hiding this comment

qiluo-msft Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

jfeng-arista Aug 15, 2023

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

jfeng-arista commented Aug 18, 2023

rlhui commented Aug 23, 2023

gechiang commented Aug 27, 2023 • edited Loading

gechiang commented Aug 30, 2023

jfeng-arista commented Aug 30, 2023

gechiang commented Aug 30, 2023

jfeng-arista commented Aug 30, 2023

jfeng-arista commented Sep 1, 2023

gechiang commented Sep 30, 2023

kenneth-arista commented Jul 5, 2023 •

edited

Loading

qiluo-msft Aug 15, 2023 •

edited

Loading

qiluo-msft Aug 15, 2023 •

edited

Loading

qiluo-msft Aug 15, 2023 •

edited

Loading

qiluo-msft Aug 15, 2023 •

edited

Loading

qiluo-msft Aug 15, 2023 •

edited

Loading

gechiang commented Aug 27, 2023 •

edited

Loading