Skip to content

[portstat] check TX/RX utilization calculation correctness#1840

Merged
qiluo-msft merged 1 commit intosonic-net:masterfrom
ayurkiv-nvda:porstat_fix_azure
Dec 17, 2021
Merged

[portstat] check TX/RX utilization calculation correctness#1840
qiluo-msft merged 1 commit intosonic-net:masterfrom
ayurkiv-nvda:porstat_fix_azure

Conversation

@ayurkiv-nvda
Copy link
Copy Markdown
Contributor

@ayurkiv-nvda ayurkiv-nvda commented Sep 27, 2021

What I did

Update test for checking TX/RX utilization

How I did it

Add real values for TX/RX to mock table instead of zeroes

How to verify it

run portstat_test.py via python3 setup.py test in sonic-utilities tests

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
liat-grozovik previously approved these changes Sep 29, 2021
@liat-grozovik
Copy link
Copy Markdown
Collaborator

@ayurkiv-nvda could you please specify on which branches this fix should be taken?
@qiluo-msft or @lguohan could you please recommend a reviewer?

@qiluo-msft qiluo-msft requested a review from ganglyu September 29, 2021 10:53
@ayurkiv-nvda
Copy link
Copy Markdown
Contributor Author

@ayurkiv-nvda could you please specify on which branches this fix should be taken? @qiluo-msft or @lguohan could you please recommend a reviewer?

Problem related to this fix appeared after merging 1750
Currently it is related only to 202106 branch but maybe I will double check it

@volodymyrsamotiy
Copy link
Copy Markdown
Collaborator

@ganglyu, did you have a chance to take a look at these changes?

@ganglyu
Copy link
Copy Markdown
Contributor

ganglyu commented Oct 8, 2021

@ganglyu, did you have a chance to take a look at these changes?

Will take a look soon.

ganglyu
ganglyu previously approved these changes Oct 9, 2021
@ayurkiv-nvda ayurkiv-nvda marked this pull request as draft October 11, 2021 20:37
@ayurkiv-nvda
Copy link
Copy Markdown
Contributor Author

ayurkiv-nvda commented Oct 13, 2021

Converting this PR to draft because additional investigation showed that "brate" already in octets, and "port_rate" in bits so calculation
brate/(float(port_rate)*1000*1000/8.0)*100 is correct

@ayurkiv-nvda ayurkiv-nvda dismissed stale reviews from ganglyu and liat-grozovik via c411bba October 20, 2021 11:41
@ayurkiv-nvda ayurkiv-nvda force-pushed the porstat_fix_azure branch 2 times, most recently from c411bba to 25db0d2 Compare October 20, 2021 11:45
@ayurkiv-nvda ayurkiv-nvda changed the title [portstat] Fix wrong utilization calculation [portstat] Add test for checking TX/RX utilization for portstat Oct 20, 2021
@ayurkiv-nvda ayurkiv-nvda changed the title [portstat] Add test for checking TX/RX utilization for portstat [portstat] check TX/RX utilization calculation correctness Oct 20, 2021
Signed-off-by: Andriy Yurkiv <ayurkiv@nvidia.com>
@ayurkiv-nvda ayurkiv-nvda marked this pull request as ready for review November 24, 2021 19:46
@liat-grozovik
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Copy Markdown
Collaborator

@qiluo-msft any further comments on this PR that should be followed?

@liat-grozovik
Copy link
Copy Markdown
Collaborator

@qiluo-msft kindly reminder. Can you please review?

@qiluo-msft qiluo-msft merged commit 74d2a09 into sonic-net:master Dec 17, 2021
judyjoseph pushed a commit that referenced this pull request Dec 22, 2021
#### What I did
Update test for checking TX/RX utilization

#### How I did it
Add real values for TX/RX to mock table instead of zeroes 

#### How to verify it
run **portstat_test.py** via  `python3 setup.py test`  in  sonic-utilities tests
@ayurkiv-nvda ayurkiv-nvda deleted the porstat_fix_azure branch February 5, 2025 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants