Skip to content

Commit

Permalink
Fixed the issues with sonic-clear queuecounter for egress queue and v…
Browse files Browse the repository at this point in the history 
…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]>
  • Loading branch information
@saksarav-nokia
saksarav-nokia authored Dec 12, 2024
1 parent 72ee4fc commit 7dc40ac
Showing 1 changed file with 20 additions and 23 deletions.
43 changes: 20 additions & 23 deletions scripts/queuestat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,38 +294,27 @@ class Queuestat(object):
old_cntr = cnstat_old_dict.get(key)
if old_cntr is not None:
if self.voq:
if not non_zero or ns_diff(cntr['totalpacket'], old_cntr['totalpacket']) != '0' or \
if not non_zero or ns_diff(cntr['totalpacket'], old_cntr['totalpacket']) != '0' or \
ns_diff(cntr['totalbytes'], old_cntr['totalbytes']) != '0' or \
ns_diff(cntr['droppacket'], old_cntr['droppacket']) != '0' or \
ns_diff(cntr['dropbytes'], old_cntr['dropbytes']) != '0' or \
ns_diff(cntr['creditWDpkts'], old_cntr['creditWDpkts']) != '0':
table.append((port, cntr['queuetype'] + str(cntr['queueindex']),
table.append((port, cntr['queuetype'] + str(cntr['queueindex']),
ns_diff(cntr['totalpacket'], old_cntr['totalpacket']),
ns_diff(cntr['totalbytes'], old_cntr['totalbytes']),
ns_diff(cntr['droppacket'], old_cntr['droppacket']),
ns_diff(cntr['dropbytes'], old_cntr['dropbytes']),
ns_diff(cntr['creditWDpkts'], old_cntr['creditWDpkts'])))
elif not non_zero or cntr['totalpacket'] != '0' or cntr['totalbytes'] != '0' or \
cntr['droppacket'] != '0' or cntr['dropbytes'] != '0' or cntr['creditWDpkts'] != '0':
table.append((port, cntr['queuetype'] + str(cntr['queueindex']),
cntr['totalpacket'], cntr['totalbytes'],
cntr['droppacket'], cntr['dropbytes'], cntr['creditWDpkts']))
else:
if not non_zero or ns_diff(cntr['totalpacket'], old_cntr['totalpacket']) != '0' or \
if not non_zero or ns_diff(cntr['totalpacket'], old_cntr['totalpacket']) != '0' or \
ns_diff(cntr['totalbytes'], old_cntr['totalbytes']) != '0' or \
ns_diff(cntr['droppacket'], old_cntr['droppacket']) != '0' or \
ns_diff(cntr['dropbytes'], old_cntr['dropbytes']) != '0':
table.append((port, cntr['queuetype'] + str(cntr['queueindex']),
ns_diff(cntr['totalpacket'], old_cntr['totalpacket']),
ns_diff(cntr['totalbytes'], old_cntr['totalbytes']),
ns_diff(cntr['droppacket'], old_cntr['droppacket']),
ns_diff(cntr['dropbytes'], old_cntr['dropbytes'])))
elif not non_zero or cntr['totalpacket'] != '0' or cntr['totalbytes'] != '0' or \
cntr['droppacket'] != '0' or cntr['dropbytes'] != '0':
table.append((port, cntr['queuetype'] + str(cntr['queueindex']),
cntr['totalpacket'], cntr['totalbytes'],
cntr['droppacket'], cntr['dropbytes']))

table.append((port, cntr['queuetype'] + str(cntr['queueindex']),
ns_diff(cntr['totalpacket'], old_cntr['totalpacket']),
ns_diff(cntr['totalbytes'], old_cntr['totalbytes']),
ns_diff(cntr['droppacket'], old_cntr['droppacket']),
ns_diff(cntr['dropbytes'], old_cntr['dropbytes'])))
if json_opt:
json_output[port].update(build_json(port, table, self.voq))
return json_output
Expand All @@ -346,8 +335,10 @@ class Queuestat(object):
for port in natsorted(self.counter_port_name_map):
json_output[port] = {}
cnstat_dict = self.get_cnstat(self.port_queues_map[port])

cnstat_fqn_file_name = cnstat_fqn_file + port
cache_ns = ''
if self.voq and self.namespace is not None:
cache_ns = '-' + self.namespace + '-'
cnstat_fqn_file_name = cnstat_fqn_file + cache_ns + port
if os.path.isfile(cnstat_fqn_file_name):
try:
cnstat_cached_dict = json.load(open(cnstat_fqn_file_name, 'r'))
Expand Down Expand Up @@ -378,7 +369,10 @@ class Queuestat(object):

# Get stat for the port queried
cnstat_dict = self.get_cnstat(self.port_queues_map[port])
cnstat_fqn_file_name = cnstat_fqn_file + port
cache_ns = ''
if self.voq and self.namespace is not None:
cache_ns = '-' + self.namespace + '-'
cnstat_fqn_file_name = cnstat_fqn_file + cache_ns + port
json_output = {}
json_output[port] = {}
if os.path.isfile(cnstat_fqn_file_name):
Expand All @@ -403,10 +397,13 @@ class Queuestat(object):

def save_fresh_stats(self):
# Get stat for each port and save
cache_ns = ''
if self.voq and self.namespace is not None:
cache_ns = '-' + self.namespace + '-'
for port in natsorted(self.counter_port_name_map):
cnstat_dict = self.get_cnstat(self.port_queues_map[port])
try:
json.dump(cnstat_dict, open(cnstat_fqn_file + port, 'w'), default=json_serial)
json.dump(cnstat_dict, open(cnstat_fqn_file + cache_ns + port, 'w'), default=json_serial)
except IOError as e:
print(e.errno, e)
sys.exit(e.errno)
Expand Down

0 comments on commit 7dc40ac

Please sign in to comment.