sonic-utilities icon indicating copy to clipboard operation
sonic-utilities copied to clipboard

suppport multi asic for show queue counter

Open zhixzhu opened this issue 2 years ago • 8 comments

What I did

Support multi asic for "show queue counter".

How I did it

Added option -n for both "show queue counter" and "queuestat", using multi_asic module in queuestat to query database of specified namespace. Removed function get_queue_port() to decrease the times of connecting database.

How to verify it

Execute command "queuestat -p <port_name> -n " in multi-asic platform.

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

cisco@sonic:~$ show queue counter Ethernet14 Port doesn't exist! Ethernet14

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

cisco@sonic:~$ queuestat -p Ethernet14 -n asic1 Port TxQ Counter/pkts Counter/bytes Drop/pkts Drop/bytes


Ethernet14 UC0 1118473981 1145317326171 0 0 Ethernet14 UC1 0 0 0 0 Ethernet14 UC2 0 0 0 0 Ethernet14 UC3 0 0 0 0 Ethernet14 UC4 17432290 17850664960 2253261 2307339264 Ethernet14 UC5 0 0 0 0 Ethernet14 UC6 0 0 0 0 Ethernet14 UC7 0 0 0 0 Ethernet14 MC8 0 0 0 0 Ethernet14 MC9 0 0 0 0 Ethernet14 MC10 0 0 0 0 Ethernet14 MC11 0 0 0 0 Ethernet14 MC12 0 0 0 0 Ethernet14 MC13 0 0 0 0 Ethernet14 MC14 0 0 0 0 Ethernet14 MC15 0 0 0 0

zhixzhu avatar Oct 13 '22 17:10 zhixzhu

This pull request introduces 1 alert and fixes 1 when merging 3d81ed610c189bc52db2e6d7526acaf5be9db40a into abd5eba492bdfa35c931738070ef60398a4460dc - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Oct 13 '22 17:10 lgtm-com[bot]

This pull request fixes 1 alert when merging 207b31b52d2eb1c42229b453c9d3ae98426f5034 into abd5eba492bdfa35c931738070ef60398a4460dc - view on LGTM.com

fixed alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Oct 13 '22 22:10 lgtm-com[bot]

@abdosi Can you do the needful on review?

Can we discuss what CLI for multi-asic, what needs to be added?

alpeshspatel avatar Oct 14 '22 20:10 alpeshspatel

Need in 202205 also

alpeshspatel avatar Oct 14 '22 20:10 alpeshspatel

@abdosi How to make UT printing the log? "python3 setup.py test" doesn't print out the log, I need the detail log to see which point the case failed at.

Or how to run single UT script directly? "python3 tests/queue_counter_test.py" gave out an import error. Traceback (most recent call last): File "/sonic/src/sonic-utilities/tests/queue_counter_test.py", line 10, in from .mock_tables import dbconnector ImportError: attempted relative import with no known parent package

zhixzhu avatar Oct 18 '22 23:10 zhixzhu

This pull request fixes 1 alert when merging bb39296df64685d3e77dbe17cba540a3e7586dfd into 7e7d05c0442d0eda05434bc503d2518eba08f00e - view on LGTM.com

fixed alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Oct 26 '22 21:10 lgtm-com[bot]

@abdosi can you please help

alpeshspatel avatar Nov 09 '22 01:11 alpeshspatel

@vmittal-msft can you please help here.

cc @rlhui

abdosi avatar Nov 09 '22 19:11 abdosi

Hi @abdosi gentle reminder.

alpeshspatel avatar Nov 17 '22 00:11 alpeshspatel

@vmittal-msft can you please help review.

abdosi avatar Nov 18 '22 00:11 abdosi

@abdosi I need some guidance on running UT.

How to make UT printing the log that the source code generates? "python3 setup.py test" doesn't print out those log, I need the detail log to see which point the case failed at.

And how to run single UT script directly? "python3 tests/queue_counter_test.py" gave out an import error. Traceback (most recent call last): File "/sonic/src/sonic-utilities/tests/queue_counter_test.py", line 10, in from .mock_tables import dbconnector ImportError: attempted relative import with no known parent package

zhixzhu avatar Nov 18 '22 06:11 zhixzhu

  1. Did we try running these changes on non chassis (pizza box) to make sure it doesn't cause any problems ?

  2. As per my understanding, these tests are run as part of sonic-buildimage build ? Can you please try to do it locally ? In the meantime, i will find how you can run them individually.

@vmittal-msft, I tried 220205 branch image on pizza-box, replaced the script /usr/local/bin/queuestat, queuestat cmd worked well on it. Both "queuestat -p Ethernet0" and "queuestat -p Ethernet0 -n asic0" works on pizza-box.

Building sonic-buildimage with the change, same error observed as the UT failure.

zhixzhu avatar Nov 22 '22 06:11 zhixzhu

@abdosi @vmittal-msft I have added UT cases for multi-asic, and all UT cases passed. Could you help review? Also need an approval to run workflows, thanks!

zhixzhu avatar Jan 18 '23 22:01 zhixzhu

@abdosi @vmittal-msft There is 1 workflow awaiting approval, could you help approve running workflows?

zhixzhu avatar Jan 19 '23 18:01 zhixzhu

Please add unit-test to validate it.

@abdosi UT cases have been added and passed, could you help review it again?

zhixzhu avatar Jan 20 '23 06:01 zhixzhu

@abdosi @vmittal-msft UT cases have been added and all checks have passed, could you help review it again?

zhixzhu avatar Jan 30 '23 17:01 zhixzhu

@zhixzhu this change cannot be cherry-picked to 202205 branch cleanly. Please raise separate PR.

@abdosi why is this change not needed for 202211 branch?

yxieca avatar Feb 02 '23 21:02 yxieca

@zhixzhu this change cannot be cherry-picked to 202205 branch cleanly. Please raise separate PR.

@abdosi why is this change not needed for 202211 branch?

@yxieca I created this PR for 202205 branch: https://github.com/sonic-net/sonic-utilities/pull/2647

zhixzhu avatar Feb 02 '23 22:02 zhixzhu