diff --git a/ansible/library/minigraph_facts.py b/ansible/library/minigraph_facts.py index 83d75818fe0..2d69f750c95 100644 --- a/ansible/library/minigraph_facts.py +++ b/ansible/library/minigraph_facts.py @@ -626,29 +626,6 @@ def port_alias_to_name_map_50G(all_ports, s100G_ports): return port_alias_to_name_map -def parse_linkmeta(meta, hname): - link = meta.find(str(QName(ns, "Link"))) - macsec_neighbors = [] - macsec_enabled_ports = [] - for linkmeta in link.findall(str(QName(ns1, "LinkMetadata"))): - local_port = None - # Sample: ARISTA05T1:Ethernet1/33;switch-t0:fortyGigE0/4 - key = linkmeta.find(str(QName(ns1, "Key"))).text - endpoints = key.split(';') - local_endpoint = endpoints[1] - remote_endpoint = endpoints[0] - t = local_endpoint.split(':') - if len(t) == 2 and t[0].lower() == hname.lower(): - local_port = t[1] - macsec_enabled_ports.append(local_port) - neighbor_host = remote_endpoint.split(':')[0] - macsec_neighbors.append(neighbor_host) - else: - # Cannot find a matching hname, something went wrong - continue - return macsec_enabled_ports, macsec_neighbors - - def parse_xml(filename, hostname, asic_name=None): mini_graph_path, root = reconcile_mini_graph_locations(filename, hostname) @@ -677,8 +654,6 @@ def parse_xml(filename, hostname, asic_name=None): bgp_peers_with_range = [] deployment_id = None is_storage_device = None - macsec_enabled_ports = [] - macsec_neighbors = [] if asic_name is not None: asic_id = asic_name[len('asic'):] @@ -724,9 +699,8 @@ def parse_xml(filename, hostname, asic_name=None): elif child.tag == str(QName(ns, "UngDec")): (u_neighbors, u_devices, _, _, _, _) = parse_png(child, hostname) elif child.tag == str(QName(ns, "MetadataDeclaration")): - (syslog_servers, ntp_servers, mgmt_routes, deployment_id, resource_type) = parse_meta(child, hostname) - elif child.tag == str(QName(ns, "LinkMetadataDeclaration")): - macsec_enabled_ports, macsec_neighbors = parse_linkmeta(child, hostname) + (syslog_servers, ntp_servers, mgmt_routes, deployment_id, + resource_type) = parse_meta(child, hostname) else: if child.tag == str(QName(ns, "DpgDec")): (intfs, lo_intfs, mgmt_intf, vlans, pcs, acls, @@ -736,9 +710,8 @@ def parse_xml(filename, hostname, asic_name=None): (bgp_sessions, bgp_asn, bgp_peers_with_range) = parse_cpg( child, asic_name) elif child.tag == str(QName(ns, "PngDec")): - (neighbors, devices, _) = parse_asic_png(child, asic_name, hostname) - elif child.tag == str(QName(ns, "LinkMetadataDeclaration")): - macsec_enabled_ports, macsec_neighbors = parse_linkmeta(child, hostname) + (neighbors, devices, _) = parse_asic_png( + child, asic_name, hostname) current_device = [devices[key] for key in devices if key.lower() == hostname.lower()][0] @@ -872,10 +845,6 @@ def Tree(): return defaultdict(Tree) if is_storage_device: results['minigraph_device_metadata']['storage_device'] = "true" - if macsec_enabled_ports: - results['macsec_enabled_ports'] = macsec_enabled_ports - if macsec_neighbors: - results['macsec_neighbors'] = macsec_neighbors return results diff --git a/ansible/templates/minigraph_link_meta.j2 b/ansible/templates/minigraph_link_meta.j2 index fff5e393cd5..2bcf14880bf 100644 --- a/ansible/templates/minigraph_link_meta.j2 +++ b/ansible/templates/minigraph_link_meta.j2 @@ -28,31 +28,6 @@ {% endif %} -{% if macsec_card is defined and macsec_card == True and 't2' in topo %} - - -{% for index in range(vms_number) %} -{% set vm_intfs=vm_topo_config['vm'][vms[index]]['intfs'][dut_index|int]|sort %} -{% set dut_intfs=vm_topo_config['vm'][vms[index]]['interface_indexes'][dut_index|int]|sort %} -{% for if_index in range(vm_intfs | length) %} -{% if 'IB' not in port_alias[dut_intfs[if_index]] %} - - - - - MacSecEnabled - True - - - {{ vms[index] }}:{{ vm_intfs[if_index] }};{{ inventory_hostname }}:{{ port_alias[dut_intfs[if_index]] }} - -{% endif %} -{% endfor %} -{% endfor %} - - -{% endif %} - {% if msft_an_enabled is defined and vm_topo_config.get('autoneg_interfaces') is not none %} diff --git a/ansible/templates/minigraph_meta.j2 b/ansible/templates/minigraph_meta.j2 index e57765273c4..2d62182ac42 100644 --- a/ansible/templates/minigraph_meta.j2 +++ b/ansible/templates/minigraph_meta.j2 @@ -212,12 +212,6 @@ {{ switch_type }} -{% endif %} -{% if macsec_card is defined and macsec_card == True and 't2' in topo %} - - MacSecProfile - PrimaryKey="MACSEC_PROFILE" FallbackKey="macsec-profile2" MacsecPolicy="" - {% endif %} diff --git a/tests/common/devices/multi_asic.py b/tests/common/devices/multi_asic.py index d879e468481..f367eb1c220 100644 --- a/tests/common/devices/multi_asic.py +++ b/tests/common/devices/multi_asic.py @@ -169,26 +169,6 @@ def get_dut_iface_mac(self, iface_name): logger.error('Failed to get MAC address for interface "{}", exception: {}'.format(iface_name, repr(e))) return None - def iface_macsec_ok(self, interface_name): - """ - Check if macsec is functional on specified interface. - - Returns: True or False - """ - try: - if self.sonichost.facts['num_asic'] == 1: - cmd_prefix = " " - else: - asic = self.get_port_asic_instance(interface_name) - cmd_prefix = "-n {}".format(asic.namespace) - - cmd = 'sonic-db-cli {} STATE_DB HGET \"MACSEC_PORT_TABLE|{}\" state'.format(cmd_prefix, interface_name) - state = self.shell(cmd)['stdout'].strip() - return state == 'ok' - except Exception as e: - logger.error('Failed to get macsec status for interface {} exception: {}'.format(interface_name, repr(e))) - return False - def get_frontend_asic_ids(self): if self.sonichost.facts['num_asic'] == 1: return [DEFAULT_ASIC_ID] diff --git a/tests/common/devices/sonic.py b/tests/common/devices/sonic.py index c4947d736c8..7c63bc58f2a 100644 --- a/tests/common/devices/sonic.py +++ b/tests/common/devices/sonic.py @@ -16,7 +16,7 @@ from tests.common.devices.base import AnsibleHostBase from tests.common.devices.constants import ACL_COUNTERS_UPDATE_INTERVAL_IN_SEC -from tests.common.helpers.dut_utils import is_supervisor_node, is_macsec_capable_node +from tests.common.helpers.dut_utils import is_supervisor_node from tests.common.str_utils import str2bool from tests.common.utilities import get_host_visible_vars from tests.common.cache import cached @@ -420,11 +420,6 @@ def is_frontend_node(self): """ return not self.is_supervisor_node() - def is_macsec_capable_node(self): - im = self.host.options['inventory_manager'] - inv_files = im._sources - return is_macsec_capable_node(inv_files, self.hostname) - def is_service_fully_started(self, service): """ @summary: Check whether a SONiC specific service is fully started. diff --git a/tests/common/helpers/dut_utils.py b/tests/common/helpers/dut_utils.py index e7307c7c4dc..3198f842044 100644 --- a/tests/common/helpers/dut_utils.py +++ b/tests/common/helpers/dut_utils.py @@ -46,13 +46,6 @@ def is_frontend_node(inv_files, hostname): return not is_supervisor_node(inv_files, hostname) -def is_macsec_capable_node(inv_files, hostname): - dut_vars = get_host_visible_vars(inv_files, hostname) - if dut_vars and 'macsec_card' in dut_vars and dut_vars['macsec_card']: - return True - return False - - def is_container_running(duthost, container_name): """Decides whether the container is running or not @param duthost: Host DUT. diff --git a/tests/common/macsec/__init__.py b/tests/common/macsec/__init__.py index 234c61c8485..08f1be37de6 100644 --- a/tests/common/macsec/__init__.py +++ b/tests/common/macsec/__init__.py @@ -51,25 +51,16 @@ def pytest_generate_tests(self, metafunc): ids=profiles, scope="module") - def get_ctrl_nbr_names(self, macsec_duthost, nbrhosts, tbinfo): - return NotImplementedError() - - def downstream_neighbor(self,tbinfo, neighbor): - return NotImplementedError() - - def upstream_neighbor(self,tbinfo, neighbor): - return NotImplementedError() - @pytest.fixture(scope="module") - def start_macsec_service(self, macsec_duthost, macsec_nbrhosts): + def start_macsec_service(self, duthost, macsec_nbrhosts): def __start_macsec_service(): - enable_macsec_feature(macsec_duthost, macsec_nbrhosts) + enable_macsec_feature(duthost, macsec_nbrhosts) return __start_macsec_service @pytest.fixture(scope="module") - def stop_macsec_service(self, macsec_duthost, macsec_nbrhosts): + def stop_macsec_service(self, duthost, macsec_nbrhosts): def __stop_macsec_service(): - disable_macsec_feature(macsec_duthost, macsec_nbrhosts) + disable_macsec_feature(duthost, macsec_nbrhosts) return __stop_macsec_service @pytest.fixture(scope="module") @@ -79,20 +70,19 @@ def macsec_feature(self, start_macsec_service, stop_macsec_service): stop_macsec_service() @pytest.fixture(scope="module") - def startup_macsec(self, request, macsec_duthost, ctrl_links, macsec_profile, tbinfo): - topo_name = tbinfo['topo']['name'] + def startup_macsec(self, request, duthost, ctrl_links, macsec_profile): def __startup_macsec(): profile = macsec_profile if request.config.getoption("neighbor_type") == "eos": - if macsec_duthost.facts["asic_type"] == "vs" and profile['send_sci'] == "false": + if duthost.facts["asic_type"] == "vs" and profile['send_sci'] == "false": # On EOS, portchannel mac is not same as the member port mac (being as SCI), # then src mac is not equal to SCI in its sending packet. The receiver of vSONIC # will drop it for macsec kernel module does not correctly handle it. pytest.skip( "macsec on dut vsonic, neighbor eos, send_sci false") - if 't2' not in topo_name: - cleanup_macsec_configuration(macsec_duthost, ctrl_links, profile['name']) - setup_macsec_configuration(macsec_duthost, ctrl_links, + + cleanup_macsec_configuration(duthost, ctrl_links, profile['name']) + setup_macsec_configuration(duthost, ctrl_links, profile['name'], profile['priority'], profile['cipher_suite'], profile['primary_cak'], profile['primary_ckn'], profile['policy'], profile['send_sci'], profile['rekey_period']) @@ -101,10 +91,10 @@ def __startup_macsec(): return __startup_macsec @pytest.fixture(scope="module") - def shutdown_macsec(self, macsec_duthost, ctrl_links, macsec_profile): + def shutdown_macsec(self, duthost, ctrl_links, macsec_profile): def __shutdown_macsec(): profile = macsec_profile - cleanup_macsec_configuration(macsec_duthost, ctrl_links, profile['name']) + cleanup_macsec_configuration(duthost, ctrl_links, profile['name']) return __shutdown_macsec @pytest.fixture(scope="module", autouse=True) @@ -121,34 +111,32 @@ def macsec_nbrhosts(self, ctrl_links): return {nbr["name"]: nbr for nbr in list(ctrl_links.values())} @pytest.fixture(scope="module") - def ctrl_links(self, macsec_duthost, tbinfo, nbrhosts): - + def ctrl_links(self, duthost, tbinfo, nbrhosts): if not nbrhosts: topo_name = tbinfo['topo']['name'] pytest.skip("None of neighbors on topology {}".format(topo_name)) - - ctrl_nbr_names = self.get_ctrl_nbr_names(macsec_duthost, nbrhosts, tbinfo) + ctrl_nbr_names = natsort.natsorted(list(nbrhosts.keys()))[:2] logger.info("Controlled links {}".format(ctrl_nbr_names)) nbrhosts = {name: nbrhosts[name] for name in ctrl_nbr_names} - return self.find_links_from_nbr(macsec_duthost, tbinfo, nbrhosts) + return self.find_links_from_nbr(duthost, tbinfo, nbrhosts) @pytest.fixture(scope="module") - def unctrl_links(self, macsec_duthost, tbinfo, nbrhosts, ctrl_links): + def unctrl_links(self, duthost, tbinfo, nbrhosts, ctrl_links): unctrl_nbr_names = set(nbrhosts.keys()) - for _, nbr in ctrl_links.items(): + for _, nbr in list(ctrl_links.items()): if nbr["name"] in unctrl_nbr_names: unctrl_nbr_names.remove(nbr["name"]) - logger.info("Uncontrolled links {}".format(unctrl_nbr_names)) nbrhosts = {name: nbrhosts[name] for name in unctrl_nbr_names} - return self.find_links_from_nbr(macsec_duthost, tbinfo, nbrhosts) + return self.find_links_from_nbr(duthost, tbinfo, nbrhosts) @pytest.fixture(scope="module") - def downstream_links(self, macsec_duthost, tbinfo, nbrhosts): + def downstream_links(self, duthost, tbinfo, nbrhosts): links = collections.defaultdict(dict) def filter(interface, neighbor, mg_facts, tbinfo): - if self.downstream_neighbor(tbinfo, neighbor): + if ((tbinfo["topo"]["type"] == "t0" and "Server" in neighbor["name"]) + or (tbinfo["topo"]["type"] == "t2" and "T1" in neighbor["name"])): port = mg_facts["minigraph_neighbors"][interface]["port"] if interface not in mg_facts["minigraph_ptf_indices"]: logger.info("Interface {} not in minigraph_ptf_indices".format(interface)) @@ -158,15 +146,16 @@ def filter(interface, neighbor, mg_facts, tbinfo): "ptf_port_id": mg_facts["minigraph_ptf_indices"][interface], "port": port } - self.find_links(macsec_duthost, tbinfo, filter) + self.find_links(duthost, tbinfo, filter) return links @pytest.fixture(scope="module") - def upstream_links(self, macsec_duthost, tbinfo, nbrhosts): + def upstream_links(self, duthost, tbinfo, nbrhosts): links = collections.defaultdict(dict) def filter(interface, neighbor, mg_facts, tbinfo): - if self.upstream_neighbor(tbinfo, neighbor): + if ((tbinfo["topo"]["type"] == "t0" and "T1" in neighbor["name"]) + or (tbinfo["topo"]["type"] == "t2" and "T3" in neighbor["name"])): for item in mg_facts["minigraph_bgp"]: if item["name"] == neighbor["name"]: if isinstance(ip_address(item["addr"]), IPv4Address): @@ -187,13 +176,12 @@ def filter(interface, neighbor, mg_facts, tbinfo): "port": port, "host": nbrhosts[neighbor["name"]]["host"] } - self.find_links(macsec_duthost, tbinfo, filter) + self.find_links(duthost, tbinfo, filter) return links def find_links(self, duthost, tbinfo, filter): - mg_facts = duthost.get_extended_minigraph_facts(tbinfo) - for interface, neighbor in mg_facts["minigraph_neighbors"].items(): + for interface, neighbor in list(mg_facts["minigraph_neighbors"].items()): filter(interface, neighbor, mg_facts, tbinfo) def is_interface_portchannel_member(self, pc, interface): @@ -204,6 +192,7 @@ def is_interface_portchannel_member(self, pc, interface): def find_links_from_nbr(self, duthost, tbinfo, nbrhosts): links = collections.defaultdict(dict) + def filter(interface, neighbor, mg_facts, tbinfo): if neighbor["name"] not in list(nbrhosts.keys()): return @@ -217,50 +206,3 @@ def filter(interface, neighbor, mg_facts, tbinfo): } self.find_links(duthost, tbinfo, filter) return links - -class MacsecPluginT0(MacsecPlugin): - """ - Pytest macsec plugin - """ - - - def __init__(self): - super(MacsecPluginT0, self).__init__() - - def get_ctrl_nbr_names(self, macsec_duthost, nbrhosts, tbinfo): - ctrl_nbr_names = natsort.natsorted(nbrhosts.keys())[:2] - return ctrl_nbr_names - - def downstream_neighbor(self,tbinfo, neighbor): - if (tbinfo["topo"]["type"] == "t0" and "Server" in neighbor["name"]): - return True - return False - - def upstream_neighbor(self,tbinfo, neighbor): - if (tbinfo["topo"]["type"] == "t0" and "T1" in neighbor["name"]): - return True - return False - -class MacsecPluginT2(MacsecPlugin): - """ - Pytest macsec plugin - """ - - - def __init__(self): - super(MacsecPluginT2, self).__init__() - - def get_ctrl_nbr_names(self, macsec_duthost, nbrhosts, tbinfo): - mg_facts = macsec_duthost.get_extended_minigraph_facts(tbinfo) - ctrl_nbr_names = mg_facts['macsec_neighbors'] - return ctrl_nbr_names - - def downstream_neighbor(self,tbinfo, neighbor): - if ("t2" in tbinfo["topo"]["type"] and "T1" in neighbor["name"]): - return True - return False - - def upstream_neighbor(self,tbinfo, neighbor): - if ("t2" in tbinfo["topo"]["type"] and "T3" in neighbor["name"]): - return True - return False diff --git a/tests/common/macsec/macsec_config_helper.py b/tests/common/macsec/macsec_config_helper.py index ffec635677a..eee88b18f4e 100644 --- a/tests/common/macsec/macsec_config_helper.py +++ b/tests/common/macsec/macsec_config_helper.py @@ -197,10 +197,10 @@ def setup_macsec_configuration(duthost, ctrl_links, profile_name, default_priori logger.info("Setup macsec configuration step1: set macsec profile") # 1. Set macsec profile i = 0 - for dut_port, nbr in ctrl_links.items(): - submit_async_task(set_macsec_profile, (duthost, dut_port, profile_name, default_priority, - cipher_suite, primary_cak, primary_ckn, policy, - send_sci, rekey_period)) + for dut_port, nbr in list(ctrl_links.items()): + submit_async_task(set_macsec_profile, + (duthost, dut_port, profile_name, default_priority, + cipher_suite, primary_cak, primary_ckn, policy, send_sci, rekey_period)) if i % 2 == 0: priority = default_priority - 1 else: diff --git a/tests/conftest.py b/tests/conftest.py index b8b4e0c15c0..b2da8184d27 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -69,7 +69,7 @@ from tests.common.plugins.ptfadapter.dummy_testutils import DummyTestUtils try: - from tests.common.macsec import MacsecPluginT2, MacsecPluginT0 + from tests.common.macsec import MacsecPlugin except ImportError as e: logging.error(e) @@ -235,11 +235,7 @@ def pytest_addoption(parser): def pytest_configure(config): if config.getoption("enable_macsec"): - topo = config.getoption("topology") - if topo is not None and "t2" in topo: - config.pluginmanager.register(MacsecPluginT2()) - else: - config.pluginmanager.register(MacsecPluginT0()) + config.pluginmanager.register(MacsecPlugin()) @pytest.fixture(scope="session", autouse=True) @@ -458,22 +454,6 @@ def mg_facts(duthost): return duthost.minigraph_facts(host=duthost.hostname)['ansible_facts'] -@pytest.fixture(scope="session") -def macsec_duthost(duthosts, tbinfo): - # get the first macsec capable node - macsec_dut = None - if 't2' in tbinfo['topo']['name']: - # currently in the T2 topo only the uplink linecard will have - # macsec enabled - for duthost in duthosts: - if duthost.is_macsec_capable_node(): - macsec_dut = duthost - break - else: - return duthosts[0] - return macsec_dut - - # Make sure in same test module, always use same random DUT rand_one_dut_hostname_var = None