Skip to content

Commit

Permalink
Add FEC correctable and uncorrectable port stats (sonic-net#2027)
Browse files Browse the repository at this point in the history 
* Add FEC correctable and uncorrectable port stats

Signed-off-by: Prince George <[email protected]>

* fix pytest failures

Signed-off-by: Prince George <[email protected]>

* fix pytest failure

* Added separate command for fec stats

* Fix test failure

* Fix LGTM warning

* Improve code coveraged
  • Loading branch information
@prgeor @lukasstockner
prgeor authored and lukasstockner committed Mar 2, 2023
1 parent b992f09 commit a70ac02
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 20 deletions.
47 changes: 38 additions & 9 deletions scripts/portstat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@ NStats = namedtuple("NStats", "rx_ok, rx_err, rx_drop, rx_ovr, tx_ok,\
rx_uca, rx_mca, rx_bca, rx_all,\
tx_64, tx_65_127, tx_128_255, tx_256_511, tx_512_1023, tx_1024_1518, tx_1519_2047, tx_2048_4095, tx_4096_9216, tx_9217_16383,\
tx_uca, tx_mca, tx_bca, tx_all,\
rx_jbr, rx_frag, rx_usize, rx_ovrrun")
rx_jbr, rx_frag, rx_usize, rx_ovrrun,\
fec_corr, fec_uncorr, fec_symbol_err")
header_all = ['IFACE', 'STATE', 'RX_OK', 'RX_BPS', 'RX_PPS', 'RX_UTIL', 'RX_ERR', 'RX_DRP', 'RX_OVR',
'TX_OK', 'TX_BPS', 'TX_PPS', 'TX_UTIL', 'TX_ERR', 'TX_DRP', 'TX_OVR']
header_std = ['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']
header_errors_only = ['IFACE', 'STATE', 'RX_ERR', 'RX_DRP', 'RX_OVR', 'TX_ERR', 'TX_DRP', 'TX_OVR']
header_fec_only = ['IFACE', 'STATE', 'FEC_CORR', 'FEC_UNCORR', 'FEC_SYMBOL_ERR']
header_rates_only = ['IFACE', 'STATE', 'RX_OK', 'RX_BPS', 'RX_PPS', 'RX_UTIL', 'TX_OK', 'TX_BPS', 'TX_PPS', 'TX_UTIL']

rates_key_list = [ 'RX_BPS', 'RX_PPS', 'RX_UTIL', 'TX_BPS', 'TX_PPS', 'TX_UTIL' ]
Expand All @@ -64,7 +66,7 @@ RateStats = namedtuple("RateStats", ratestat_fields)
The order and count of statistics mentioned below needs to be in sync with the values in portstat script
So, any fields added/deleted in here should be reflected in portstat script also
"""
BUCKET_NUM = 42
BUCKET_NUM = 45
counter_bucket_dict = {
0:['SAI_PORT_STAT_IF_IN_UCAST_PKTS', 'SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS'],
1:['SAI_PORT_STAT_IF_IN_ERRORS'],
Expand Down Expand Up @@ -107,7 +109,10 @@ counter_bucket_dict = {
38:['SAI_PORT_STAT_ETHER_STATS_JABBERS'],
39:['SAI_PORT_STAT_ETHER_STATS_FRAGMENTS'],
40:['SAI_PORT_STAT_ETHER_STATS_UNDERSIZE_PKTS'],
41:['SAI_PORT_STAT_IP_IN_RECEIVES']
41:['SAI_PORT_STAT_IP_IN_RECEIVES'],
42:['SAI_PORT_STAT_IF_IN_FEC_CORRECTABLE_FRAMES'],
43:['SAI_PORT_STAT_IF_IN_FEC_NOT_CORRECTABLE_FRAMES'],
44:['SAI_PORT_STAT_IF_IN_FEC_SYMBOL_ERRORS']
}

STATUS_NA = 'N/A'
Expand Down Expand Up @@ -246,7 +251,7 @@ class Portstat(object):
return STATUS_NA


def cnstat_print(self, cnstat_dict, ratestat_dict, intf_list, use_json, print_all, errors_only, rates_only, detail=False):
def cnstat_print(self, cnstat_dict, ratestat_dict, intf_list, use_json, print_all, errors_only, fec_stats_only, rates_only, detail=False):
"""
Print the cnstat.
"""
Expand Down Expand Up @@ -291,6 +296,12 @@ class Portstat(object):
format_number_with_comma(data.tx_err),
format_number_with_comma(data.tx_drop),
format_number_with_comma(data.tx_ovr)))
elif fec_stats_only:
header = header_fec_only
table.append((key, self.get_port_state(key),
format_number_with_comma(data.fec_corr),
format_number_with_comma(data.fec_uncorr),
format_number_with_comma(data.fec_symbol_err)))
elif rates_only:
header = header_rates_only
table.append((key, self.get_port_state(key),
Expand Down Expand Up @@ -384,7 +395,10 @@ class Portstat(object):
print("Time Since Counters Last Cleared............... " + str(cnstat_old_dict.get('time')))


def cnstat_diff_print(self, cnstat_new_dict, cnstat_old_dict, ratestat_dict, intf_list, use_json, print_all, errors_only, rates_only, detail=False):
def cnstat_diff_print(self, cnstat_new_dict, cnstat_old_dict,
ratestat_dict, intf_list, use_json,
print_all, errors_only, fec_stats_only,
rates_only, detail=False):
"""
Print the difference between two cnstat results.
"""
Expand Down Expand Up @@ -461,6 +475,19 @@ class Portstat(object):
format_number_with_comma(cntr.tx_err),
format_number_with_comma(cntr.tx_drop),
format_number_with_comma(cntr.tx_ovr)))
elif fec_stats_only:
header = header_fec_only
if old_cntr is not None:
table.append((key, self.get_port_state(key),
ns_diff(cntr.fec_corr, old_cntr.fec_corr),
ns_diff(cntr.fec_uncorr, old_cntr.fec_uncorr),
ns_diff(cntr.fec_symbol_err, old_cntr.fec_symbol_err)))
else:
table.append((key, self.get_port_state(key),
format_number_with_comma(cntr.fec_corr),
format_number_with_comma(cntr.fec_uncorr),
format_number_with_comma(cntr.fec_symbol_err)))

elif rates_only:
header = header_rates_only
if old_cntr is not None:
Expand Down Expand Up @@ -547,6 +574,7 @@ Examples:
parser.add_argument('-d', '--delete', action='store_true', help='Delete saved stats, either the uid or the specified tag')
parser.add_argument('-D', '--delete-all', action='store_true', help='Delete all saved stats')
parser.add_argument('-e', '--errors', action='store_true', help='Display interface errors')
parser.add_argument('-f', '--fec-stats', action='store_true', help='Display FEC related statistics')
parser.add_argument('-j', '--json', action='store_true', help='Display in JSON format')
parser.add_argument('-r', '--raw', action='store_true', help='Raw stats (unmodified output of netstat)')
parser.add_argument('-R', '--rate', action='store_true', help='Display interface rates')
Expand All @@ -563,6 +591,7 @@ Examples:
delete_saved_stats = args.delete
delete_all_stats = args.delete_all
errors_only = args.errors
fec_stats_only = args.fec_stats
rates_only = args.rate
use_json = args.json
raw_stats = args.raw
Expand Down Expand Up @@ -619,7 +648,7 @@ Examples:

# Now decide what information to display
if raw_stats:
portstat.cnstat_print(cnstat_dict, ratestat_dict, intf_list, use_json, print_all, errors_only, rates_only)
portstat.cnstat_print(cnstat_dict, ratestat_dict, intf_list, use_json, print_all, errors_only, fec_stats_only, rates_only)
sys.exit(0)

# At this point, either we'll create a file or open an existing one.
Expand Down Expand Up @@ -647,21 +676,21 @@ Examples:
cnstat_cached_dict = pickle.load(open(cnstat_fqn_file, 'rb'))
if not detail:
print("Last cached time was " + str(cnstat_cached_dict.get('time')))
portstat.cnstat_diff_print(cnstat_dict, cnstat_cached_dict, ratestat_dict, intf_list, use_json, print_all, errors_only, rates_only, detail)
portstat.cnstat_diff_print(cnstat_dict, cnstat_cached_dict, ratestat_dict, intf_list, use_json, print_all, errors_only, fec_stats_only, rates_only, detail)
except IOError as e:
print(e.errno, e)
else:
if tag_name:
print("\nFile '%s' does not exist" % cnstat_fqn_file)
print("Did you run 'portstat -c -t %s' to record the counters via tag %s?\n" % (tag_name, tag_name))
else:
portstat.cnstat_print(cnstat_dict, ratestat_dict, intf_list, use_json, print_all, errors_only, rates_only, detail)
portstat.cnstat_print(cnstat_dict, ratestat_dict, intf_list, use_json, print_all, errors_only, fec_stats_only, rates_only, detail)
else:
#wait for the specified time and then gather the new stats and output the difference.
time.sleep(wait_time_in_seconds)
print("The rates are calculated within %s seconds period" % wait_time_in_seconds)
cnstat_new_dict, ratestat_new_dict = portstat.get_cnstat_dict()
portstat.cnstat_diff_print(cnstat_new_dict, cnstat_dict, ratestat_new_dict, intf_list, use_json, print_all, errors_only, rates_only, detail)
portstat.cnstat_diff_print(cnstat_new_dict, cnstat_dict, ratestat_new_dict, intf_list, use_json, print_all, errors_only, fec_stats_only, rates_only, detail)

if __name__ == "__main__":
main()
33 changes: 25 additions & 8 deletions show/interfaces/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,15 +338,15 @@ def expected(db, interfacename):
@click.pass_context
def mpls(ctx, interfacename, namespace, display):
"""Show Interface MPLS status"""

#Edge case: Force show frontend interfaces on single asic
if not (multi_asic.is_multi_asic()):
if (display == 'frontend' or display == 'all' or display is None):
display = None
else:
print("Error: Invalid display option command for single asic")
return

display = "all" if interfacename else display
masic = multi_asic_util.MultiAsic(display_option=display, namespace_option=namespace)
ns_list = masic.get_ns_list_based_on_options()
Expand All @@ -372,13 +372,13 @@ def mpls(ctx, interfacename, namespace, display):
if (interfacename is not None):
if (interfacename != ifname):
continue

intf_found = True

if (display != "all"):
if ("Loopback" in ifname):
continue

if ifname.startswith("Ethernet") and multi_asic.is_port_internal(ifname, ns):
continue

Expand All @@ -391,11 +391,11 @@ def mpls(ctx, interfacename, namespace, display):
if 'mpls' not in mpls_intf or mpls_intf['mpls'] == 'disable':
intfs_data.update({ifname: 'disable'})
else:
intfs_data.update({ifname: mpls_intf['mpls']})
intfs_data.update({ifname: mpls_intf['mpls']})

# Check if interface is valid
if (interfacename is not None and not intf_found):
ctx.fail('interface {} doesn`t exist'.format(interfacename))
ctx.fail('interface {} doesn`t exist'.format(interfacename))

header = ['Interface', 'MPLS State']
body = []
Expand Down Expand Up @@ -558,6 +558,23 @@ def errors(verbose, period, namespace, display):

clicommon.run_command(cmd, display_cmd=verbose)

# 'fec-stats' subcommand ("show interfaces counters errors")
@counters.command('fec-stats')
@click.option('-p', '--period')
@multi_asic_util.multi_asic_click_options
@click.option('--verbose', is_flag=True, help="Enable verbose output")
def fec_stats(verbose, period, namespace, display):
"""Show interface counters fec-stats"""
cmd = "portstat -f"
if period is not None:
cmd += " -p {}".format(period)

cmd += " -s {}".format(display)
if namespace is not None:
cmd += " -n {}".format(namespace)

clicommon.run_command(cmd, display_cmd=verbose)

# 'rates' subcommand ("show interfaces counters rates")
@counters.command()
@click.option('-p', '--period')
Expand Down
15 changes: 12 additions & 3 deletions tests/mock_tables/counters_db.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,10 @@
"SAI_PORT_STAT_ETHER_OUT_PKTS_9217_TO_16383_OCTETS": "0",
"SAI_PORT_STAT_ETHER_STATS_FRAGMENTS": "0",
"SAI_PORT_STAT_ETHER_STATS_UNDERSIZE_PKTS": "0",
"SAI_PORT_STAT_ETHER_STATS_JABBERS": "0"
"SAI_PORT_STAT_ETHER_STATS_JABBERS": "0",
"SAI_PORT_STAT_IF_IN_FEC_CORRECTABLE_FRAMES": "130402",
"SAI_PORT_STAT_IF_IN_FEC_NOT_CORRECTABLE_FRAMES": "3",
"SAI_PORT_STAT_IF_IN_FEC_SYMBOL_ERRORS": "4"
},
"COUNTERS:oid:0x1000000000013": {
"SAI_PORT_STAT_IF_IN_UCAST_PKTS": "4",
Expand Down Expand Up @@ -768,7 +771,10 @@
"SAI_PORT_STAT_ETHER_OUT_PKTS_9217_TO_16383_OCTETS": "0",
"SAI_PORT_STAT_ETHER_STATS_FRAGMENTS": "0",
"SAI_PORT_STAT_ETHER_STATS_UNDERSIZE_PKTS": "0",
"SAI_PORT_STAT_ETHER_STATS_JABBERS": "0"
"SAI_PORT_STAT_ETHER_STATS_JABBERS": "0",
"SAI_PORT_STAT_IF_IN_FEC_CORRECTABLE_FRAMES": "110412",
"SAI_PORT_STAT_IF_IN_FEC_NOT_CORRECTABLE_FRAMES": "1",
"SAI_PORT_STAT_IF_IN_FEC_SYMBOL_ERRORS": "0"
},
"COUNTERS:oid:0x1000000000014": {
"SAI_PORT_STAT_IF_IN_UCAST_PKTS": "6",
Expand Down Expand Up @@ -825,7 +831,10 @@
"SAI_PORT_STAT_ETHER_OUT_PKTS_9217_TO_16383_OCTETS": "0",
"SAI_PORT_STAT_ETHER_STATS_FRAGMENTS": "0",
"SAI_PORT_STAT_ETHER_STATS_UNDERSIZE_PKTS": "0",
"SAI_PORT_STAT_ETHER_STATS_JABBERS": "0"
"SAI_PORT_STAT_ETHER_STATS_JABBERS": "0",
"SAI_PORT_STAT_IF_IN_FEC_CORRECTABLE_FRAMES": "100317",
"SAI_PORT_STAT_IF_IN_FEC_NOT_CORRECTABLE_FRAMES": "0",
"SAI_PORT_STAT_IF_IN_FEC_SYMBOL_ERRORS": "0"
},
"COUNTERS:oid:0x21000000000000": {
"SAI_SWITCH_STAT_OUT_DROP_REASON_RANGE_BASE": "1000",
Expand Down
50 changes: 50 additions & 0 deletions tests/portstat_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,23 @@
Ethernet8 N/A 6 1350.00 KB/s 9000.00/s N/A 100 10 N/A 60 13.37 MB/s 9000.00/s N/A N/A N/A N/A
"""

intf_fec_counters = """\
IFACE STATE FEC_CORR FEC_UNCORR FEC_SYMBOL_ERR
--------- ------- ---------- ------------ ----------------
Ethernet0 D 130,402 3 4
Ethernet4 N/A 110,412 1 0
Ethernet8 N/A 100,317 0 0
"""

intf_fec_counters_period = """\
The rates are calculated within 3 seconds period
IFACE STATE FEC_CORR FEC_UNCORR FEC_SYMBOL_ERR
--------- ------- ---------- ------------ ----------------
Ethernet0 D 0 0 0
Ethernet4 N/A 0 0 0
Ethernet8 N/A 0 0 0
"""

intf_counters_period = """\
The rates are calculated within 3 seconds period
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
Expand Down Expand Up @@ -258,6 +275,39 @@ def test_show_intf_counters_all(self):
assert return_code == 0
assert result == intf_counters_all

def test_show_intf_fec_counters(self):
runner = CliRunner()
result = runner.invoke(
show.cli.commands["interfaces"].commands["counters"].commands["fec-stats"], [])
print(result.exit_code)
print(result.output)
assert result.exit_code == 0
assert result.output == intf_fec_counters

return_code, result = get_result_and_return_code('portstat -f')
print("return_code: {}".format(return_code))
print("result = {}".format(result))
assert return_code == 0
assert result == intf_fec_counters

def test_show_intf_fec_counters_period(self):
runner = CliRunner()
result = runner.invoke(show.cli.commands["interfaces"].commands["counters"].commands["fec-stats"],
["-p {}".format(TEST_PERIOD)])
print(result.exit_code)
print(result.output)
assert result.exit_code == 0
assert result.output == intf_fec_counters_period

return_code, result = get_result_and_return_code(
'portstat -f -p {}'.format(TEST_PERIOD))
print("return_code: {}".format(return_code))
print("result = {}".format(result))
assert return_code == 0
assert result == intf_fec_counters_period



def test_show_intf_counters_period(self):
runner = CliRunner()
result = runner.invoke(show.cli.commands["interfaces"].commands["counters"], [
Expand Down

0 comments on commit a70ac02

Please sign in to comment.