diff --git a/tests/macsec/macsec_config_helper.py b/tests/macsec/macsec_config_helper.py index 87e74afbb76..92e3fe06cec 100644 --- a/tests/macsec/macsec_config_helper.py +++ b/tests/macsec/macsec_config_helper.py @@ -47,13 +47,14 @@ def set_macsec_profile(host, port, profile_name, priority, cipher_suite, "primary_cak": primary_cak, "primary_ckn": primary_ckn, "policy": policy, - "send_sci": send_sci, + "send_sci" if send_sci else "no_send_sci": "", "rekey_period": rekey_period, } - cmd = "sonic-db-cli {} CONFIG_DB HMSET 'MACSEC_PROFILE|{}' ".format( - getns_prefix(host, port), profile_name) + opts = "" for k, v in list(macsec_profile.items()): - cmd += " '{}' '{}' ".format(k, v) + opts += " --{} {}".format(k, v) + cmd = "config macsec {} profile add {} {}".format( + getns_prefix(host, port), profile_name, opts) host.command(cmd) if send_sci == "false": # The MAC address of SONiC host is locally administrated @@ -77,10 +78,10 @@ def delete_macsec_profile(host, port, profile_name): if host.is_multi_asic and port is None: for ns in host.get_asic_namespace_list(): CMD_PREFIX = "-n {}".format(ns) if ns is not None else " " - cmd = "sonic-db-cli {} CONFIG_DB DEL 'MACSEC_PROFILE|{}'".format(CMD_PREFIX, profile_name) + cmd = "config macsec {} profile del {}".format(CMD_PREFIX, profile_name) host.command(cmd) else: - cmd = ("sonic-db-cli {} CONFIG_DB DEL 'MACSEC_PROFILE|{}'" + cmd = ("config macsec {} profile del {}" .format(getns_prefix(host, port), profile_name)) host.command(cmd) @@ -100,7 +101,7 @@ def enable_macsec_port(host, port, profile_name): host.command("sudo config portchannel {} member del {} {}".format(getns_prefix(host, port), pc["name"], port)) time.sleep(2) - cmd = "sonic-db-cli {} CONFIG_DB HSET 'PORT|{}' 'macsec' '{}'".format(getns_prefix(host, port), port, profile_name) + cmd = "config macsec {} port add {} {}".format(getns_prefix(host, port), port, profile_name) host.command(cmd) if dnx_platform and pc: @@ -125,7 +126,7 @@ def disable_macsec_port(host, port): host.command("sudo config portchannel {} member del {} {}".format(getns_prefix(host, port), pc["name"], port)) time.sleep(2) - cmd = "sonic-db-cli {} CONFIG_DB HDEL 'PORT|{}' 'macsec'".format(getns_prefix(host, port), port) + cmd = "config macsec {} port del {}".format(getns_prefix(host, port), port) host.command(cmd) if dnx_platform and pc: diff --git a/tests/macsec/macsec_helper.py b/tests/macsec/macsec_helper.py index da13721f417..f5fa9da9ec1 100644 --- a/tests/macsec/macsec_helper.py +++ b/tests/macsec/macsec_helper.py @@ -1,9 +1,9 @@ -import ast import binascii import json import logging import struct import time +import re from collections import defaultdict, deque from multiprocessing import Process @@ -472,18 +472,104 @@ def macsec_dp_poll(test, device_number=0, port_number=None, timeout=None, exp_pk return test.dataplane.PollFailure(exp_pkt, recent_packets, packet_count) -def get_macsec_counters(sonic_asic, namespace, name): - lines = [ - 'from swsscommon.swsscommon import DBConnector, CounterTable, MacsecCounter,SonicDBConfig', - 'from sonic_py_common import multi_asic', - 'SonicDBConfig.initializeGlobalConfig() if multi_asic.is_multi_asic() else {}', - 'counterTable = CounterTable(DBConnector("COUNTERS_DB", 0, False, "{}"))'.format(namespace), - '_, values = counterTable.get(MacsecCounter(), "{}")'.format(name), - 'print(dict(values))' - ] - cmd = "python -c '{}'".format(';'.join(lines)) - output = sonic_asic.sonichost.command(cmd)["stdout_lines"][0] - return {k: int(v) for k, v in list(ast.literal_eval(output).items())} +def _parse_show_macsec_counters(text): + ''' + This function takes the output of a show macsec command, and returns a dict + of the counters. + Returns following dict format: + { + 'egress': {}, + 'ingress': {} + } + TODO: enhance show macsec command to output in json directly + + Here is an example of `show macsec Ethernet216` + MACsec port(Ethernet216) + --------------------- --------------- + cipher_suite GCM-AES-XPN-256 + enable true + enable_encrypt true + enable_protect true + enable_replay_protect false + profile MACSEC_PROFILE + replay_window 0 + send_sci true + --------------------- --------------- + MACsec Egress SC (XXX) + ----------- - + encoding_an 1 + ----------- - + MACsec Egress SA (1) + ------------------------------------- ---------------------------------------------------------------- + auth_key XXX + next_pn 1 + sak XXX + salt XXX + ssci 2 + SAI_MACSEC_SA_ATTR_CURRENT_XPN 8 + SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED 28532 + SAI_MACSEC_SA_STAT_OCTETS_PROTECTED 0 + SAI_MACSEC_SA_STAT_OUT_PKTS_ENCRYPTED 7 + SAI_MACSEC_SA_STAT_OUT_PKTS_PROTECTED 0 + ------------------------------------- ---------------------------------------------------------------- + MACsec Ingress SC (XXX) + + MACsec Ingress SA (1) + --------------------------------------- ---------------------------------------------------------------- + active true + auth_key XXX + lowest_acceptable_pn 1 + sak XXX + salt XXX + ssci 1 + SAI_MACSEC_SA_ATTR_CURRENT_XPN 6661 + SAI_MACSEC_SA_STAT_IN_PKTS_DELAYED 0 + SAI_MACSEC_SA_STAT_IN_PKTS_INVALID 0 + SAI_MACSEC_SA_STAT_IN_PKTS_LATE 0 + SAI_MACSEC_SA_STAT_IN_PKTS_NOT_USING_SA 1 + SAI_MACSEC_SA_STAT_IN_PKTS_NOT_VALID 0 + SAI_MACSEC_SA_STAT_IN_PKTS_OK 8 + SAI_MACSEC_SA_STAT_IN_PKTS_UNCHECKED 0 + SAI_MACSEC_SA_STAT_IN_PKTS_UNUSED_SA 0 + SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED 523517 + SAI_MACSEC_SA_STAT_OCTETS_PROTECTED 0 + --------------------------------------- ---------------------------------------------------------------- + ''' + out = {'egress': {}, 'ingress': {}} + stats = None + reg = re.compile(r'(SAI_MACSEC.*?) *(\d+)') + for line in text.splitlines(): + line = line.strip() + + # Found the egress header, following stats will be for egress + if line.startswith("MACsec Egress SA"): + stats = 'egress' + continue + # Found the ingress header, following stats will be for ingress + elif line.startswith("MACsec Ingress SA"): + stats = 'ingress' + continue + # No header yet, so no stats coming + if not stats: + continue + + found = reg.match(line) + if found: + out[stats].update({found.group(1): int(found.group(2))}) + return out + + +def get_macsec_counters(duthost, port): + cmd = f"show macsec {port}" + output = duthost.command(cmd)["stdout"] + + out_dict = _parse_show_macsec_counters(output) + + return (out_dict['egress'], out_dict['ingress']) + + +def clear_macsec_counters(duthost): + assert duthost.command("sonic-clear macsec")["failed"] is False __origin_dp_poll = testutils.dp_poll diff --git a/tests/macsec/test_dataplane.py b/tests/macsec/test_dataplane.py index 98d4fbbeafa..e9e8244f2cd 100644 --- a/tests/macsec/test_dataplane.py +++ b/tests/macsec/test_dataplane.py @@ -8,7 +8,7 @@ from tests.common.devices.eos import EosHost from .macsec_helper import create_pkt, create_exp_pkt, check_macsec_pkt,\ - get_ipnetns_prefix, get_macsec_sa_name, get_macsec_counters + get_ipnetns_prefix, get_macsec_counters, clear_macsec_counters from .macsec_platform_helper import get_portchannel, find_portchannel_from_member logger = logging.getLogger(__name__) @@ -120,18 +120,53 @@ def test_neighbor_to_neighbor(self, duthost, ctrl_links, upstream_links, wait_mk requester["peer_ipv4_addr"]), module_ignore_errors=True) def test_counters(self, duthost, ctrl_links, upstream_links, rekey_period, wait_mka_establish): - if rekey_period: - pytest.skip("Counter increase is not guaranteed in case rekey is happening") - EGRESS_SA_COUNTERS = ( + + def get_counters(duthost, up_ports): + egress_counters = Counter() + ingress_counters = Counter() + for up_port in up_ports: + + egress_dict, ingress_dict = get_macsec_counters(duthost, up_port) + + egress_counters += Counter(egress_dict) + ingress_counters += Counter(ingress_dict) + + return (egress_counters, ingress_counters) + + # multiple of rekey period to wait, to ensure a rekey has happened + REKEY_PERIOD_WAIT_SCALE = 1.5 + PKT_NUM = 5 if not rekey_period else int(rekey_period * REKEY_PERIOD_WAIT_SCALE) + PKT_OCTET = 1024 + + # Counters which only go up + MONOTONIC_COUNTERS = { + "ingress": [ 'SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED', - 'SAI_MACSEC_SA_STAT_OUT_PKTS_ENCRYPTED', - ) - INGRESS_SA_COUNTERS = ( + ], + "egress": [ 'SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED', - 'SAI_MACSEC_SA_STAT_IN_PKTS_OK', - ) - PKT_NUM = 5 - PKT_OCTET = 1024 + ], + } + + # Counters which can reset during a rekey + RESET_OVER_REKEY_COUNTERS = { + "ingress": [ + 'SAI_MACSEC_SA_ATTR_CURRENT_XPN', + 'SAI_MACSEC_SA_STAT_IN_PKTS_OK' + ], + "egress": [ + 'SAI_MACSEC_SA_ATTR_CURRENT_XPN', + 'SAI_MACSEC_SA_STAT_OUT_PKTS_ENCRYPTED' + ], + } + + ALL_COUNTERS = {key: MONOTONIC_COUNTERS[key] + RESET_OVER_REKEY_COUNTERS[key] for key in MONOTONIC_COUNTERS} + + if rekey_period: + # can only check monotonic counters if rekeying + COUNTERS = MONOTONIC_COUNTERS + else: + COUNTERS = ALL_COUNTERS # Select some one macsec link port_name = list(ctrl_links)[0] @@ -143,58 +178,62 @@ def test_counters(self, duthost, ctrl_links, upstream_links, rekey_period, wait_ else: up_ports = [port_name] - # Sum up start counter - egress_start_counters = Counter() - ingress_start_counters = Counter() for up_port in up_ports: assert up_port in ctrl_links - asic = duthost.get_port_asic_instance(up_port) - ns = duthost.get_namespace_from_asic_id(asic.asic_index) if duthost.is_multi_asic else '' - egress_sa_name = get_macsec_sa_name(asic, up_port, True) - ingress_sa_name = get_macsec_sa_name(asic, up_port, False) - if not egress_sa_name or not ingress_sa_name: - continue - - egress_start_counters += Counter(get_macsec_counters(asic, ns, egress_sa_name)) - ingress_start_counters += Counter(get_macsec_counters(asic, ns, ingress_sa_name)) + # Sum up start counter + egress_start_counters, ingress_start_counters = get_counters(duthost, up_ports) - # Launch traffic + # Launch traffic at 1 sec intervals + logging.info(f"Sending {PKT_NUM} packets") ret = duthost.command( - "{} ping -c {} -s {} {}".format(get_ipnetns_prefix(duthost, port_name), PKT_NUM, PKT_OCTET, nbr_ip_addr)) + "{} ping -c {} -s {} -i 1 {}".format(get_ipnetns_prefix(duthost, port_name), + PKT_NUM, PKT_OCTET, nbr_ip_addr)) assert not ret['failed'] sleep(10) # wait 10s for polling counters # Sum up end counter - egress_end_counters = Counter() - ingress_end_counters = Counter() - for up_port in up_ports: - asic = duthost.get_port_asic_instance(up_port) - ns = duthost.get_namespace_from_asic_id(asic.asic_index) if duthost.is_multi_asic else '' - egress_sa_name = get_macsec_sa_name(asic, up_port, True) - ingress_sa_name = get_macsec_sa_name(asic, up_port, False) - if not egress_sa_name or not ingress_sa_name: - continue + egress_end_counters, ingress_end_counters = get_counters(duthost, up_ports) - egress_end_counters += Counter(get_macsec_counters(asic, ns, egress_sa_name)) - ingress_end_counters += Counter(get_macsec_counters(asic, ns, ingress_sa_name)) - - i = 'SAI_MACSEC_SA_ATTR_CURRENT_XPN' - assert egress_end_counters[i] - egress_start_counters[i] >= PKT_NUM - assert ingress_end_counters[i] - ingress_start_counters[i] >= PKT_NUM - - if duthost.facts["asic_type"] == "vs": + if duthost.facts["asic_type"] == "vs" and not rekey_period: # vsonic only has xpn counter + i = 'SAI_MACSEC_SA_ATTR_CURRENT_XPN' + assert egress_end_counters[i] - egress_start_counters[i] >= PKT_NUM + assert ingress_end_counters[i] - ingress_start_counters[i] >= PKT_NUM return - for i in EGRESS_SA_COUNTERS: - if 'OCTETS' in i: - assert egress_end_counters[i] - egress_start_counters[i] >= PKT_NUM * PKT_OCTET + for counter in COUNTERS['egress']: + if 'OCTETS' in counter: + assert egress_end_counters[counter] - egress_start_counters[counter] >= PKT_NUM * PKT_OCTET else: - assert egress_end_counters[i] - egress_start_counters[i] >= PKT_NUM + assert egress_end_counters[counter] - egress_start_counters[counter] >= PKT_NUM - for i in INGRESS_SA_COUNTERS: - if 'OCTETS' in i: - assert ingress_end_counters[i] - ingress_start_counters[i] >= PKT_NUM * PKT_OCTET + for counter in COUNTERS['ingress']: + if 'OCTETS' in counter: + assert ingress_end_counters[counter] - ingress_start_counters[counter] >= PKT_NUM * PKT_OCTET else: - assert ingress_end_counters[i] - ingress_start_counters[i] >= PKT_NUM + assert ingress_end_counters[counter] - ingress_start_counters[counter] >= PKT_NUM + + # check that the counters get cleared + clear_macsec_counters(duthost) + + egress_cleared_counters, ingress_cleared_counters = get_counters(duthost, up_ports) + + for counter in ALL_COUNTERS['egress']: + assert egress_end_counters[counter] > egress_cleared_counters[counter] + + for counter in ALL_COUNTERS['ingress']: + assert ingress_end_counters[counter] > ingress_cleared_counters[counter] + + # Wait a rekey period, and ensure the counters are still sane (ie. clear) + if rekey_period: + sleep_sec = rekey_period * REKEY_PERIOD_WAIT_SCALE + logger.info(f"Waiting {sleep_sec} sec to allow for a rekey (rekey_period: {rekey_period})") + sleep(sleep_sec) + egress_cleared_counters, ingress_cleared_counters = get_counters(duthost, up_ports) + + for counter in ALL_COUNTERS['egress']: + assert egress_end_counters[counter] > egress_cleared_counters[counter] + + for counter in ALL_COUNTERS['ingress']: + assert ingress_end_counters[counter] > ingress_cleared_counters[counter]