[portstat, intfstat] added rates and utilization #1750
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@dgsudharsan could you please review? |
|
This pull request introduces 1 alert when merging b784d50 into 30907c4 - view on LGTM.com new alerts:
|
vadymhlushko-mlnx
force-pushed
the
master_rates_pr_fix
branch
from
e70e851 to
6358537
Compare
dgsudharsan
suggested changes
Aug 12, 2021
| return "{:.2f}".format(float(rate))+'/s' | ||
|
|
||
| def format_util(brate, port_rate=PORT_RATE): | ||
| """ |
There was a problem hiding this comment.
Can we avoid assuming PORTE_RATE=40 here? I believe there are multitude of speeds and If speed in unavailable in config_db.json I am ok with displaying rate as N/A rather than using 40.
There was a problem hiding this comment.
already fixed
utilities_common/netstat.py
Outdated
| if brate == STATUS_NA: | ||
| return STATUS_NA | ||
| else: | ||
| util = brate/(float(port_rate)*1024*1024*1024/8.0)*100 |
There was a problem hiding this comment.
Should we assume that speed is always in Gbps? I believe here we convert it from Mbps to Gpbs and pass it on here. But in future if we have a system which has deployment less than 1Gpbs this logic will fail. Instead I propose to use the default measurement unit used in the port table (Mbps) and modify the calculation accordingly here.
There was a problem hiding this comment.
already fixed
vadymhlushko-mlnx
force-pushed
the
master_rates_pr_fix
branch
from
057ff86 to
6358537
Compare
dgsudharsan
previously approved these changes
Aug 16, 2021
vadymhlushko-mlnx
force-pushed
the
master_rates_pr_fix
branch
from
d289f07 to
0f60ccc
Compare
dgsudharsan
previously approved these changes
Aug 16, 2021
|
@lguohan could you please merge? |
ganglyu
requested changes
Aug 31, 2021
|
@ganglyu, @dgsudharsan could you please approve? |
dgsudharsan
previously approved these changes
Sep 1, 2021
ganglyu
previously approved these changes
Sep 2, 2021
vadymhlushko-mlnx
dismissed stale reviews from ganglyu and dgsudharsan
via
f32bd91
vadymhlushko-mlnx
force-pushed
the
master_rates_pr_fix
branch
from
02c7265 to
f32bd91
Compare
Signed-off-by: Vadym Hlushko <[email protected]>
vadymhlushko-mlnx
force-pushed
the
master_rates_pr_fix
branch
from
f32bd91 to
44976ea
Compare
|
@ganglyu, @dgsudharsan could you please review and approve again, because there were some merge conflicts? |
ganglyu
approved these changes
Sep 7, 2021
dgsudharsan
approved these changes
Sep 7, 2021
|
@liat-grozovik could you please help to merge? |
|
This commit could not be cleanly cherry-pick to 202012. Please submit another PR. |
qiluo-msft
pushed a commit
that referenced
this pull request
Sep 13, 2021
#### What I did Backport the [pull/1750](#1750) to `202012` branch Depends on [sonic-swss-common/pull/330](sonic-net/sonic-swss-common#330) According to [HLD](https://github.com/Azure/SONiC/blob/master/doc/rates-and-utilization/Rates_and_utilization_HLD.md) added calculation of rates and utilization columns to the `portstat` and `intfstat` scripts output #### How I did it Modified the `portstat` and `intfstat` scripts #### How to verify it Added UT
judyjoseph
pushed a commit
that referenced
this pull request
Sep 14, 2021
#### What I did Depends on [sonic-swss-common/pull/330](sonic-net/sonic-swss-common#330) According to [HLD](https://github.com/Azure/SONiC/blob/master/doc/rates-and-utilization/Rates_and_utilization_HLD.md) added calculation of rates and utilization columns to the `portstat` and `intfstat` scripts output #### How I did it Modified the `portstat` and `intfstat` scripts #### How to verify it Added UT #### Previous command output (if the output of a command-line utility has changed) ``` admin@sonic:~$ show int count IFACE STATE RX_OK RX_BPS RX_UTIL RX_ERR RX_DRP RX_OVR TX_OK TX_BPS TX_UTIL TX_ERR TX_DRP TX_OVR ----------- ------- ------- -------- --------- -------- -------- -------- ------- -------- --------- -------- -------- -------- Ethernet0 X 0 N/A N/A 0 0 N/A 0 N/A N/A 0 0 N/A Ethernet2 U 287 N/A N/A 0 0 N/A 818 N/A N/A 0 0 N/A Ethernet4 U 380 N/A N/A 0 0 N/A 858 N/A N/A 0 0 N/A Ethernet6 U 286 N/A N/A 0 0 N/A 850 N/A N/A 0 0 N/A ``` #### New command output (if the output of a command-line utility has changed) ``` admin@sonic:~$ show int count IFACE STATE RX_OK RX_BPS RX_UTIL RX_ERR RX_DRP RX_OVR TX_OK TX_BPS TX_UTIL TX_ERR TX_DRP TX_OVR ----------- ------- ------- ---------- --------- -------- -------- -------- ------- ---------- --------- -------- -------- -------- Ethernet0 X 0 0.00 B/s 0.00% 0 0 N/A 0 0.00 B/s 0.00% 0 0 N/A Ethernet2 U 0 0.00 B/s 0.00% 0 0 N/A 0 0.00 B/s 0.00% 0 0 N/A Ethernet4 U 0 0.00 B/s 0.00% 0 0 N/A 0 0.00 B/s 0.00% 0 0 N/A Ethernet6 U 0 0.00 B/s 0.00% 0 0 N/A 0 0.00 B/s 0.00% 0 0 N/A ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What I did
Depends on sonic-swss-common/pull/330
According to HLD added calculation of rates and utilization columns to the
portstatandintfstatscripts outputHow I did it
Modified the
portstatandintfstatscriptsHow to verify it
Added UT
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)