Fixed the issues with sonic-clear queuecounter for egress queue and voq #3671
Conversation
|
@rlhui @vmittal-msft @arista-nwolfe @abdosi for viz |
saksarav-nokia
force-pushed
the
saksarav-nokia-queue-counter
branch
from
dc373b0 to
a80e9e6
Compare
|
@saksarav-nokia Can you elaborate on the usecase of namespace/asic specific clear commands? I recall when I was working on adding multi-asic support to queuestat we decided to go forward with the goal that a clear would "clear all namespaces". |
|
|
||
| cnstat_fqn_file_name = cnstat_fqn_file + port | ||
| cache_ns = '' | ||
| if self.voq and self.namespace is not None: |
There was a problem hiding this comment.
We should also handle cases when namespace arg is provided but the voq flag is absent
There was a problem hiding this comment.
We should also handle cases when namespace arg is provided but the voq flag is absent
@arista-hpandya, the asic-namespace is required to be added to the cache file only for the --voq case and not required when --voq is absent. Please check my previous comment #3671 (comment)
| cntr['totalpacket'], cntr['totalbytes'], | ||
| cntr['droppacket'], cntr['dropbytes'])) | ||
|
|
||
| table.append((port, cntr['queuetype'] + str(cntr['queueindex']), |
There was a problem hiding this comment.
Could you please add a unit test to verify the logic change here?
There was a problem hiding this comment.
Could you please add a unit test to verify the logic change here?
There are tests already in https://github.com/sonic-net/sonic-utilities/blob/master/tests/queue_counter_test.py which covers these changes.
@arista-hpandya , queuestat is called for the following 2 scenarios for sonic-clear queuecounter.
|
Signed-off-by: saksarav <[email protected]>
saksarav-nokia
force-pushed
the
saksarav-nokia-queue-counter
branch
from
a80e9e6 to
8e224c5
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bingwang-ms
added
Approved for 202405 Branch
and removed
Approved for 202405 Branch
labels
|
Readding label to trigger auto-cherry-pick |
…oq (sonic-net#3671) Fixed the sonic-clear queuecounter issues reported in sonic-net/sonic-buildimage#20913 For the egress queue counters, after the queue counter is cleared and show is issued with --nonzero option, the code takes the elif path at line 323 (old code) even if the diff between cached counter and current counter is 0, prints the current counter values from counter_db . This issue was for both egress queue counter and voq counter. When the sonic-clear queuecounter is issued , the queuestat is called first without --voq option and this gets the port names and queue ids for each port in each asic , reads the egress queue counters and cache the values in /tmp/cache/queuestat/. Then queuestat is called with --voq option and this gets all the system ports names and voq id's for each asic , reads the voq counters and cache the values in /tmp/cache/queuestat. Since each asic has the all the system ports and all the voqs, and since the cache file name is queuestat+system_port_name with out asic namespace, caching asic1's voq counters overwrites the asic0's voq counters . How I did it 1)Corrected the logic mistake for issue 1. Since cnstat_diff_print is called only if cache file is present and this should print only if non-zero & valid diff or (not non-zero ) 2) Added asic namespace with the cache file name for vow counters. Signed-off-by: saksarav <[email protected]>
|
Cherry-pick PR to 202405: #3697 |
…oq (#3671) Fixed the sonic-clear queuecounter issues reported in sonic-net/sonic-buildimage#20913 For the egress queue counters, after the queue counter is cleared and show is issued with --nonzero option, the code takes the elif path at line 323 (old code) even if the diff between cached counter and current counter is 0, prints the current counter values from counter_db . This issue was for both egress queue counter and voq counter. When the sonic-clear queuecounter is issued , the queuestat is called first without --voq option and this gets the port names and queue ids for each port in each asic , reads the egress queue counters and cache the values in /tmp/cache/queuestat/. Then queuestat is called with --voq option and this gets all the system ports names and voq id's for each asic , reads the voq counters and cache the values in /tmp/cache/queuestat. Since each asic has the all the system ports and all the voqs, and since the cache file name is queuestat+system_port_name with out asic namespace, caching asic1's voq counters overwrites the asic0's voq counters . How I did it 1)Corrected the logic mistake for issue 1. Since cnstat_diff_print is called only if cache file is present and this should print only if non-zero & valid diff or (not non-zero ) 2) Added asic namespace with the cache file name for vow counters. Signed-off-by: saksarav <[email protected]>
mssonicbld
added
Included in 202405 Branch
and removed
Created PR to 202405 Branch
labels
What I did
Fixed the sonic-clear queuecounter issues reported in sonic-net/sonic-buildimage#20913
Fixed the following 2 issues
How I did it
1)Corrected the logic mistake for issue 1. Since cnstat_diff_print is called only if cache file is present and this should print only if non-zero & valid diff or (not non-zero )
2) Added asic namespace with the cache file name for vow counters.
How to verify it
Previous command output (if the output of a command-line utility has changed)
show command output is not changed.
New command output (if the output of a command-line utility has changed)
show command output is not changed.