From 7dc40ac3211f85e3ba2cf3e00708697d6b5e7fd1 Mon Sep 17 00:00:00 2001 From: saksarav-nokia Date: Thu, 12 Dec 2024 15:09:07 -0500 Subject: [PATCH] Fixed the issues with sonic-clear queuecounter for egress queue and voq (#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 --- scripts/queuestat | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/scripts/queuestat b/scripts/queuestat index 3774ede6d9..befe8acc30 100755 --- a/scripts/queuestat +++ b/scripts/queuestat @@ -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 @@ -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')) @@ -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): @@ -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)