Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[portstat pfcstat] Unify the packet number format in the output of portstat and pfcstat in all cases #1755

Merged
merged 1 commit into from
Sep 2, 2021

Conversation

keboliu
Copy link
Collaborator

@keboliu keboliu commented Aug 12, 2021

What I did

Unify the packet number format in the output of "portstat" and "pfcstat" for all cases.
In some cases, the packet numbers in the output of these two commands are formatted with a comma, in some cases not.
This is because only when the numbers are treated by function ns_diff, it will format the numbers with commas: return '{:,}'.format(max(0, new - old)) , but ns_diff is not called in all cases.

for example, packet numbers in the output are NOT formatted with commas:

pfcstat

root@r-qa-sw-eth-2133:/home/admin# pfcstat
    Port Rx    PFC0    PFC1    PFC2    PFC3    PFC4    PFC5    PFC6    PFC7
-----------  ------  ------  ------  ------  ------  ------  ------  ------
Ethernet120       0       0       0       0       0       0       0       0
Ethernet124  137407       0   45659   45660       0   45662       0       0 <-----FORMAT OF DATA BEFORE clear counters

portstat
portstat
      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         0         0       N/A        N/A         0         0         0
  Ethernet4        U  12804864       N/A        N/A         0         0         0         9       N/A        N/A         0         0         0
  Ethernet8        X         0       N/A        N/A         0         0         0         0       N/A        N/A         0         0         0
Ethernet120        U         1       N/A        N/A         0         0         0        11       N/A        N/A         0         0         0
Ethernet124        U         1       N/A        N/A         0         0         0  12363470       N/A        N/A         0    429517         0

packet numbers in the output are formatted with commas:

pfcstat
Last cached time was 2021-08-02 10:35:32.725158
    Port Rx    PFC0    PFC1    PFC2    PFC3    PFC4    PFC5    PFC6    PFC7
-----------  ------  ------  ------  ------  ------  ------  ------  ------
Ethernet120       0       0       0       0       0       0       0       0
Ethernet124  25,007       0  25,006  25,006       0  25,007       0       0 <-----DIFFERENT FORMAT AFTER COUNTER CLEAR COMMAND

portstat
Last cached time was 2021-08-02 10:35:47.829677
      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         0          0     0.00 B/s      0.00%         0         0         0
  Ethernet4        U  1,570,775   862.60 MB/s     69.01%         0         0         0          0   28.29 KB/s      0.00%         0         0         0
Ethernet120        U          0      0.00 B/s      0.00%         0         0         0          0     0.00 B/s      0.00%         0         0         0
Ethernet124        U          0  1406.45 KB/s      0.11%         0         0         0  1,488,765  817.56 MB/s     65.40%         0    83,040         0 <----DIFFERENT PRESENTATION FORMAT OF NUMBERS

How I did it

Add a new function format_number_with_comma to format the packet numbers with comma, this function will be called in case ns_diff is not applicable.

Update the unitest to cover this new change.

How to verify it

execute portstat and pfcstat to check the output whether the number format is expected.

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

pfcstat

root@r-qa-sw-eth-2133:/home/admin# pfcstat
    Port Rx    PFC0    PFC1    PFC2    PFC3    PFC4    PFC5    PFC6    PFC7
-----------  ------  ------  ------  ------  ------  ------  ------  ------
Ethernet116       0       0       0       0       0       0       0       0
Ethernet120       0       0       0       0       0       0       0       0
Ethernet124  137407       0   45659   45660       0   45662       0       0 

portstat
      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         0         0       N/A        N/A         0         0         0
  Ethernet4        U    804864       N/A        N/A         0         0         0         9       N/A        N/A         0         0         0
Ethernet120        U         1       N/A        N/A         0         0         0        11       N/A        N/A         0         0         0
Ethernet124        U         1       N/A        N/A         0         0         0    363470       N/A        N/A         0    429517         0

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

pfcstat
root@r-qa-sw-eth-2133:/home/admin# pfcstat
    Port Rx    PFC0    PFC1    PFC2    PFC3    PFC4    PFC5    PFC6    PFC7
-----------  ------  ------  ------  ------  ------  ------  ------  ------
Ethernet116       0       0       0       0       0       0       0       0
Ethernet120       0       0       0       0       0       0       0       0
Ethernet124 137,407       0  45,659  45,660       0  45,662       0       0 

portstat
Last cached time was 2021-08-02 10:35:47.829677
      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         0         0       N/A        N/A         0         0         0
  Ethernet4       U   804,864       N/A        N/A         0         0         0         9       N/A        N/A         0         0         0
Ethernet120       U         1       N/A        N/A         0         0         0        11       N/A        N/A         0         0         0
Ethernet124       U         1       N/A        N/A         0         0         0   363,470       N/A        N/A         0   429,517         0

@keboliu
Copy link
Collaborator Author

keboliu commented Aug 16, 2021

Hi @neethajohn not sure whether you are the right person to review this PR, if not, would you please suggest?

@neethajohn neethajohn requested review from abdosi and smaheshm August 20, 2021 16:22
@liat-grozovik
Copy link
Collaborator

@abdosi and @smaheshm could you please help to review?

Copy link
Contributor

@smaheshm smaheshm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks clean and good to me!!

@keboliu
Copy link
Collaborator Author

keboliu commented Aug 30, 2021

@neethajohn @abdosi are we good to go?

@qiluo-msft qiluo-msft merged commit 1bc24ca into sonic-net:master Sep 2, 2021
@qiluo-msft
Copy link
Contributor

This commit could not be cleanly cherry-pick to 202012. Please submit another PR.

judyjoseph pushed a commit that referenced this pull request Sep 2, 2021
…rtstat and pfcstat in all cases (#1755)

#### What I did

Unify the packet number format in the output of "portstat" and "pfcstat" for all cases.  
In some cases, the packet numbers in the output of these two commands are formatted with a comma, in some cases not. 
This is because only when the numbers are treated by function `ns_diff`, it will format the numbers with commas:  `return '{:,}'.format(max(0, new - old))` , but `ns_diff` is not called in all cases.

**for example, packet numbers in the output are NOT formatted with commas:**

```
pfcstat

root@r-qa-sw-eth-2133:/home/admin# pfcstat
    Port Rx    PFC0    PFC1    PFC2    PFC3    PFC4    PFC5    PFC6    PFC7
-----------  ------  ------  ------  ------  ------  ------  ------  ------
Ethernet120       0       0       0       0       0       0       0       0
Ethernet124  137407       0   45659   45660       0   45662       0       0 <-----FORMAT OF DATA BEFORE clear counters

portstat
portstat
      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         0         0       N/A        N/A         0         0         0
  Ethernet4        U  12804864       N/A        N/A         0         0         0         9       N/A        N/A         0         0         0
  Ethernet8        X         0       N/A        N/A         0         0         0         0       N/A        N/A         0         0         0
Ethernet120        U         1       N/A        N/A         0         0         0        11       N/A        N/A         0         0         0
Ethernet124        U         1       N/A        N/A         0         0         0  12363470       N/A        N/A         0    429517         0

```
**packet numbers in the output are formatted with commas:**

```
pfcstat
Last cached time was 2021-08-02 10:35:32.725158
    Port Rx    PFC0    PFC1    PFC2    PFC3    PFC4    PFC5    PFC6    PFC7
-----------  ------  ------  ------  ------  ------  ------  ------  ------
Ethernet120       0       0       0       0       0       0       0       0
Ethernet124  25,007       0  25,006  25,006       0  25,007       0       0 <-----DIFFERENT FORMAT AFTER COUNTER CLEAR COMMAND

portstat
Last cached time was 2021-08-02 10:35:47.829677
      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         0          0     0.00 B/s      0.00%         0         0         0
  Ethernet4        U  1,570,775   862.60 MB/s     69.01%         0         0         0          0   28.29 KB/s      0.00%         0         0         0
Ethernet120        U          0      0.00 B/s      0.00%         0         0         0          0     0.00 B/s      0.00%         0         0         0
Ethernet124        U          0  1406.45 KB/s      0.11%         0         0         0  1,488,765  817.56 MB/s     65.40%         0    83,040         0 <----DIFFERENT PRESENTATION FORMAT OF NUMBERS
```

#### How I did it

Add a new function `format_number_with_comma` to format the packet numbers with comma, this function will be called in case `ns_diff` is not applicable.

Update the unitest to cover this new change.

#### How to verify it

execute portstat and pfcstat to check the output whether the number format is expected. 

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

```
pfcstat

root@r-qa-sw-eth-2133:/home/admin# pfcstat
    Port Rx    PFC0    PFC1    PFC2    PFC3    PFC4    PFC5    PFC6    PFC7
-----------  ------  ------  ------  ------  ------  ------  ------  ------
Ethernet116       0       0       0       0       0       0       0       0
Ethernet120       0       0       0       0       0       0       0       0
Ethernet124  137407       0   45659   45660       0   45662       0       0 

portstat
      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         0         0       N/A        N/A         0         0         0
  Ethernet4        U    804864       N/A        N/A         0         0         0         9       N/A        N/A         0         0         0
Ethernet120        U         1       N/A        N/A         0         0         0        11       N/A        N/A         0         0         0
Ethernet124        U         1       N/A        N/A         0         0         0    363470       N/A        N/A         0    429517         0

```

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

```
pfcstat
root@r-qa-sw-eth-2133:/home/admin# pfcstat
    Port Rx    PFC0    PFC1    PFC2    PFC3    PFC4    PFC5    PFC6    PFC7
-----------  ------  ------  ------  ------  ------  ------  ------  ------
Ethernet116       0       0       0       0       0       0       0       0
Ethernet120       0       0       0       0       0       0       0       0
Ethernet124 137,407       0  45,659  45,660       0  45,662       0       0 

portstat
Last cached time was 2021-08-02 10:35:47.829677
      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         0         0       N/A        N/A         0         0         0
  Ethernet4       U   804,864       N/A        N/A         0         0         0         9       N/A        N/A         0         0         0
Ethernet120       U         1       N/A        N/A         0         0         0        11       N/A        N/A         0         0         0
Ethernet124       U         1       N/A        N/A         0         0         0   363,470       N/A        N/A         0   429,517         0
```
qiluo-msft pushed a commit that referenced this pull request Sep 3, 2021
…cases (#1795)

#### What I did

backport #1755  to 202012 branch

#### How I did it

Add a new function format_number_with_comma to format the packet numbers with comma, this function will be called in case ns_diff is not applicable.

Update the unitest to cover this new change.

#### How to verify it

execute portstat and pfcstat to check the output whether the number format is expected.
@keboliu keboliu deleted the couter_number_format branch October 28, 2023 03:25
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.

5 participants