From b809d854adfbb682529c1a6bb9397fc9fc8680aa Mon Sep 17 00:00:00 2001 From: Augustine Lee Date: Tue, 30 Jun 2026 07:30:53 -0700 Subject: [PATCH 1/3] [mpls][sonic-vpp] Enable MPLS data-plane tests on the VPP t1-lag testbed Enable tests/mpls on the sonic-vpp KVM testbed (t1-lag-vpp): - conftest: support t1-lag by gating on the topology *type* ('t1') and resolving PortChannel spine/tor interfaces to their PTF member port ids (a physical interface resolves to a single-element list). The old check was `name not in ('t1')`, which compares against a string rather than a tuple, so anything other than a plain 't1' topology was skipped. - test_mpls: pass the dst PTF port-id list directly to verify_packet_any_port (ports=dst_pid), and drop a duplicate mask.Mask() wrap in the pop expected-packet helper. - conditional_mark: replace the blanket mpls/test_mpls.py vpp skip with a skip for test_push_label only. pop/swap/swap_labelstack run and pass on VPP; push is blocked by an orchagent ROUTE_TABLE MPLS-push install gap (tracked separately), not the VPP SAI backend. - golden_config: enable the 'mpls' feature on sonic-vpp testbeds so the tests are not skipped by the "'mpls' not in feature_status" condition. The feature is absent from both init_cfg.json and the minigraph-derived config, so it has to be turned on somewhere for the tests to run. - pre/posttest: skip 'mpls' when walking enabled features to toggle the rsyslog rate limit. Those loops assume every enabled feature has a container of the same name and run 'docker exec -i ', but mpls is a config flag rather than a containerized service, so the exec fails with "No such container: mpls" and aborts the run. This mirrors the existing frr_bmp skip, which is there for the same reason. - pr_test_scripts: add mpls/test_mpls.py to the t1-lag-vpp set so the vpp PR checker actually exercises it. Note that mpls/test_mpls.py is already listed for t1-lag, where the topology check above silently skipped it. With that check fixed the tests run wherever the mpls feature is enabled; on images without the feature they stay skipped by the existing feature_status condition. Verified on vms-kvm-vpp-t1-lag: 3 passed, 1 skipped (push). Signed-off-by: Augustine Lee Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .azure-pipelines/pr_test_scripts.yaml | 1 + ansible/library/generate_golden_config_db.py | 7 +++++ .../tests_mark_conditions_sonic_vpp.yaml | 6 ++-- tests/mpls/conftest.py | 28 +++++++++++++++---- tests/mpls/test_mpls.py | 9 +++--- tests/test_posttest.py | 3 ++ tests/test_pretest.py | 4 +++ 7 files changed, 45 insertions(+), 13 deletions(-) diff --git a/.azure-pipelines/pr_test_scripts.yaml b/.azure-pipelines/pr_test_scripts.yaml index 527190a2124..be5463f544d 100644 --- a/.azure-pipelines/pr_test_scripts.yaml +++ b/.azure-pipelines/pr_test_scripts.yaml @@ -694,6 +694,7 @@ t1-lag-vpp: - lldp/test_lldp.py - lldp/test_lldp_syncd.py - log_fidelity/test_bgp_shutdown.py + - mpls/test_mpls.py - pc/test_po_voq.py - pc/test_lag_member.py - pc/test_po_update.py diff --git a/ansible/library/generate_golden_config_db.py b/ansible/library/generate_golden_config_db.py index 2fe6b6e284b..d6595dff41a 100644 --- a/ansible/library/generate_golden_config_db.py +++ b/ansible/library/generate_golden_config_db.py @@ -1338,6 +1338,13 @@ def generate(self): if self.has_otel_image(): config = self.overwrite_feature_golden_config_db_singleasic(config, "otel", "enabled", "enabled") + # Enable the mpls feature on the sonic-vpp testbeds so the MPLS data-plane + # tests (tests/mpls) are not skipped by the "'mpls' not in feature_status" + # condition in tests_mark_conditions.yaml. The VPP SAI implementation + # supports MPLS (INSEG disposition/imposition). + if "vpp" in self.topo_name: + config = self.overwrite_feature_golden_config_db_singleasic(config, "mpls") + # Disable dash-ha feature for all multi-asic platforms if multi_asic.is_multi_asic(): config = self.overwrite_feature_golden_config_db_multiasic(config, "dash-ha", feature_data={ diff --git a/tests/common/plugins/conditional_mark/tests_mark_conditions_sonic_vpp.yaml b/tests/common/plugins/conditional_mark/tests_mark_conditions_sonic_vpp.yaml index 6751c280243..5c6938bfc11 100644 --- a/tests/common/plugins/conditional_mark/tests_mark_conditions_sonic_vpp.yaml +++ b/tests/common/plugins/conditional_mark/tests_mark_conditions_sonic_vpp.yaml @@ -401,10 +401,12 @@ memory_checker/test_memory_checker.py::test_monit_reset_counter_failure: ####################################### ##### MPLS ##### ####################################### -mpls/test_mpls.py: +mpls/test_mpls.py::TestBasicMpls::test_push_label: skip: reason: > - Unsupported + MPLS push is not testable here: the test injects the push route + directly into ROUTE_TABLE and orchagent does not install that route + into ASIC_DB, so it never reaches the SAI backend. conditions_logical_operator: or conditions: - "asic_type in ['vpp']" diff --git a/tests/mpls/conftest.py b/tests/mpls/conftest.py index ce32578262b..84338a0a232 100644 --- a/tests/mpls/conftest.py +++ b/tests/mpls/conftest.py @@ -14,6 +14,22 @@ LABEL_DEL_ROUTES = 'label_del_routes' +def _resolve_ptf_port_ids(dut_port, mg_facts): + """Resolve a DUT L3 interface to its PTF port indices. + + On t1-lag the spine/tor facing interfaces can be PortChannels, which are not + present in minigraph_port_indices. Resolve such a PortChannel to the PTF port + indices of its physical member ports. A physical interface resolves to a + single-element list. + """ + portchannels = mg_facts.get('minigraph_portchannels', {}) + if dut_port in portchannels: + members = portchannels[dut_port]['members'] + else: + members = [dut_port] + return [mg_facts['minigraph_port_indices'][member] for member in members] + + @pytest.fixture(scope='module') def setup(duthost, tbinfo, ptfadapter): """ @@ -22,7 +38,7 @@ def setup(duthost, tbinfo, ptfadapter): :param tbinfo: fixture provides information about testbed :return: dictionary with all test required information """ - if tbinfo['topo']['name'] not in ('t1'): + if tbinfo['topo']['type'] != 't1': pytest.skip('Unsupported topology') # gather ansible facts @@ -61,14 +77,12 @@ def setup(duthost, tbinfo, ptfadapter): logger.info('tor_addr: {}'.format(tor_addr)) for dut_port in tor_ports: - port_id = mg_facts['minigraph_port_indices'][dut_port] - tor_ports_ids[dut_port] = port_id + tor_ports_ids[dut_port] = _resolve_ptf_port_ids(dut_port, mg_facts) ansible_port = 'ansible_'+dut_port tor_mac[dut_port] = host_facts[ansible_port]['macaddress'] for dut_port in spine_ports: - port_id = mg_facts['minigraph_port_indices'][dut_port] - spine_ports_ids[dut_port] = port_id + spine_ports_ids[dut_port] = _resolve_ptf_port_ids(dut_port, mg_facts) ansible_port = 'ansible_'+dut_port spine_mac[dut_port] = host_facts[ansible_port]['macaddress'] @@ -78,8 +92,10 @@ def setup(duthost, tbinfo, ptfadapter): src_port = random.choice(spine_ports) dst_port = random.choice(tor_ports) + # dst_pid is the list of egress PortChannel member PTF ports (verify on any member). + # src_pid is a single ingress member PTF port used to inject the test packet. dst_pid = tor_ports_ids[dst_port] - src_pid = spine_ports_ids[src_port] + src_pid = spine_ports_ids[src_port][0] dst_mac = tor_mac[dst_port] src_mac = spine_mac[src_port] diff --git a/tests/mpls/test_mpls.py b/tests/mpls/test_mpls.py index fb0a5aa1b5f..e67972356c4 100644 --- a/tests/mpls/test_mpls.py +++ b/tests/mpls/test_mpls.py @@ -106,7 +106,6 @@ def expected_mask_ip_packet(self, pkt): exp_pkt['Ethernet'].remove_payload() exp_pkt /= pkt1 exp_pkt = mask.Mask(exp_pkt) - exp_pkt = mask.Mask(exp_pkt) exp_pkt.set_do_not_care_scapy(packet.Ether, 'dst') exp_pkt.set_do_not_care_scapy(packet.Ether, 'src') exp_pkt.set_do_not_care_scapy(packet.IP, 'chksum') @@ -198,7 +197,7 @@ def test_pop_label(self, setup, ptfadapter): ptfadapter.dataplane.flush() testutils.send(ptfadapter, src_pid, pkt) try: - res = testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=[dst_pid]) + res = testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=dst_pid) logger.info(res) except Exception as e: self.teardown_labels(setup) @@ -222,7 +221,7 @@ def test_swap_label(self, setup, ptfadapter): ptfadapter.dataplane.flush() testutils.send(ptfadapter, src_pid, pkt) try: - res = testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=[dst_pid]) + res = testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=dst_pid) logger.info(res) except Exception as e: self.teardown_labels(setup) @@ -255,7 +254,7 @@ def test_push_label(self, setup, ptfadapter): testutils.send(ptfadapter, src_pid, pkt) try: - res = testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=[dst_pid]) + res = testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=dst_pid) logger.info(res) except Exception as e: self.teardown_labels(setup) @@ -280,7 +279,7 @@ def test_swap_labelstack(self, setup, ptfadapter): testutils.send(ptfadapter, src_pid, pkt) try: - res = testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=[dst_pid]) + res = testutils.verify_packet_any_port(ptfadapter, exp_pkt, ports=dst_pid) logger.info(res) except Exception as e: self.teardown_labels(setup) diff --git a/tests/test_posttest.py b/tests/test_posttest.py index e712316f596..4465d79d1de 100644 --- a/tests/test_posttest.py +++ b/tests/test_posttest.py @@ -44,6 +44,9 @@ def test_recover_rsyslog_rate_limit(duthosts, enum_dut_hostname): if feature_name == "frr_bmp": # Skip frr_bmp since it's not container just bmp option used by bgpd continue + if feature_name == "mpls": + # Skip mpls since it's not a container, just a config flag + continue duthost.modify_syslog_rate_limit(feature_name, rl_option='enable') diff --git a/tests/test_pretest.py b/tests/test_pretest.py index fafcadb7ae0..654597f06a2 100644 --- a/tests/test_pretest.py +++ b/tests/test_pretest.py @@ -280,6 +280,10 @@ def disable_rsyslog_rate_limit(dut): continue if feature_name == "frr_bmp": continue + if feature_name == "mpls": + # mpls is a config flag rather than a containerized service, so + # there is no "mpls" docker to configure rsyslog in. + continue if feature_name == "telemetry": # Skip telemetry if there's no docker image output = dut.shell("docker images", module_ignore_errors=True)['stdout'] From 9e8a384388699d57b1b3ff1decf0d4d95beb7283 Mon Sep 17 00:00:00 2001 From: Augustine Lee Date: Sat, 1 Aug 2026 23:53:00 -0700 Subject: [PATCH 2/3] [mpls][sonic-vpp] Register mpls feature as disabled and harden the topology guard Register the mpls FEATURE with state "disabled" instead of "enabled", and drop the shared-file workarounds that only existed because it was enabled. mpls is a config flag, not a containerized service, so there is no "mpls" docker. With state "enabled" the image's own monit container_checker reports "Expected containers not running: mpls", which fails the pre-test sanity check for every test on this platform. The frr_bmp exemption for the same class of containerless feature lives in sonic-buildimage's /usr/bin/container_checker, so it cannot be worked around from sonic-mgmt. The tests are gated by "'mpls' not in feature_status", which is a key-membership test, so "disabled" is enough to un-skip them. Verified on a live DUT: the feature is still listed by "show feature status", container_checker exits 0 and monit reports container_checker OK. Because the feature is no longer enabled, the guards added to test_pretest.py and test_posttest.py are dead code - both loops already skip features whose state is not enabled - so they are reverted. That removes this change's footprint on shared, global test files entirely. Also skip the module when the topology has no T2-facing or no T0-facing interface. Gating on topology type instead of name correctly admits t1-lag, but it also admits t1 variants that have no T2 peer at all (t1-backend, whose neighbors are all BT0, plus t1-isolated-d128/d32 and the v6 variant). Those would have failed with an IndexError from random.choice([]). Ignore the kernel-MPLS error intfmgrd logs on sonic-vpp. Configuring MPLS on an interface makes intfmgrd run "sysctl -w net.mpls.conf..input=1", which needs the mpls_router kernel module. No SONiC image loads that module, so the command fails and is logged as an ERR. On sonic-vpp it is benign because MPLS forwarding is done by VPP in userspace via the SAI INSEG entries, not by the Linux kernel data path. Signed-off-by: Augustine Lee Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 251d571f-a53b-47f8-9eb4-293dbc6ff7a4 --- ansible/library/generate_golden_config_db.py | 12 ++++++++-- tests/mpls/conftest.py | 24 ++++++++++++++++++++ tests/test_posttest.py | 3 --- tests/test_pretest.py | 4 ---- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/ansible/library/generate_golden_config_db.py b/ansible/library/generate_golden_config_db.py index d6595dff41a..ac4da87a1a2 100644 --- a/ansible/library/generate_golden_config_db.py +++ b/ansible/library/generate_golden_config_db.py @@ -1338,12 +1338,20 @@ def generate(self): if self.has_otel_image(): config = self.overwrite_feature_golden_config_db_singleasic(config, "otel", "enabled", "enabled") - # Enable the mpls feature on the sonic-vpp testbeds so the MPLS data-plane + # Register the mpls feature on the sonic-vpp testbeds so the MPLS data-plane # tests (tests/mpls) are not skipped by the "'mpls' not in feature_status" # condition in tests_mark_conditions.yaml. The VPP SAI implementation # supports MPLS (INSEG disposition/imposition). + # + # The state is deliberately "disabled": mpls is a config flag, not a + # containerized service, so there is no "mpls" docker. Registering it as + # "enabled" would make the image's monit container_checker report + # "Expected containers not running: mpls" and fail the sanity check for + # every test on this platform. The condition above only tests for the + # presence of the key, so "disabled" is enough to un-skip the tests. if "vpp" in self.topo_name: - config = self.overwrite_feature_golden_config_db_singleasic(config, "mpls") + config = self.overwrite_feature_golden_config_db_singleasic( + config, "mpls", auto_restart="disabled", state="disabled") # Disable dash-ha feature for all multi-asic platforms if multi_asic.is_multi_asic(): diff --git a/tests/mpls/conftest.py b/tests/mpls/conftest.py index 84338a0a232..e82d3fa75cb 100644 --- a/tests/mpls/conftest.py +++ b/tests/mpls/conftest.py @@ -30,6 +30,23 @@ def _resolve_ptf_port_ids(dut_port, mg_facts): return [mg_facts['minigraph_port_indices'][member] for member in members] +@pytest.fixture(autouse=True) +def ignore_expected_loganalyzer_exceptions(duthost, loganalyzer): + """Ignore the kernel-MPLS errors intfmgrd emits on platforms without mpls_router. + + Configuring MPLS on an interface makes intfmgrd run + "sysctl -w net.mpls.conf..input=1". That requires the mpls_router kernel + module, which no SONiC image loads by default, so the command fails and is + logged as an ERR. On sonic-vpp this is harmless: MPLS forwarding is done by + VPP in userspace via the SAI INSEG entries, not by the Linux kernel data path. + """ + if loganalyzer and duthost.facts.get('asic_type') == 'vpp': + loganalyzer[duthost.hostname].ignore_regex.extend([ + r".*ERR swss#intfmgrd.*setIntfMpls: Command 'sysctl -w net\.mpls\.conf\..* failed with rc 1.*", + ]) + yield + + @pytest.fixture(scope='module') def setup(duthost, tbinfo, ptfadapter): """ @@ -76,6 +93,13 @@ def setup(duthost, tbinfo, ptfadapter): logger.info('spine_ports: {}'.format(spine_ports)) logger.info('tor_addr: {}'.format(tor_addr)) + # The test needs both a T2-facing ingress and a T0-facing egress interface. + # Some t1 variants (e.g. t1-backend, whose neighbors are all BT0) have no T2 + # peer at all, so bail out cleanly instead of failing later in random.choice(). + if not spine_ports or not tor_ports: + pytest.skip('Topology has no T2-facing ({}) or T0-facing ({}) interface' + .format(len(spine_ports), len(tor_ports))) + for dut_port in tor_ports: tor_ports_ids[dut_port] = _resolve_ptf_port_ids(dut_port, mg_facts) ansible_port = 'ansible_'+dut_port diff --git a/tests/test_posttest.py b/tests/test_posttest.py index 4465d79d1de..e712316f596 100644 --- a/tests/test_posttest.py +++ b/tests/test_posttest.py @@ -44,9 +44,6 @@ def test_recover_rsyslog_rate_limit(duthosts, enum_dut_hostname): if feature_name == "frr_bmp": # Skip frr_bmp since it's not container just bmp option used by bgpd continue - if feature_name == "mpls": - # Skip mpls since it's not a container, just a config flag - continue duthost.modify_syslog_rate_limit(feature_name, rl_option='enable') diff --git a/tests/test_pretest.py b/tests/test_pretest.py index 654597f06a2..fafcadb7ae0 100644 --- a/tests/test_pretest.py +++ b/tests/test_pretest.py @@ -280,10 +280,6 @@ def disable_rsyslog_rate_limit(dut): continue if feature_name == "frr_bmp": continue - if feature_name == "mpls": - # mpls is a config flag rather than a containerized service, so - # there is no "mpls" docker to configure rsyslog in. - continue if feature_name == "telemetry": # Skip telemetry if there's no docker image output = dut.shell("docker images", module_ignore_errors=True)['stdout'] From 5e2f795e127a0dd73b0c48e41830fd75ab457de5 Mon Sep 17 00:00:00 2001 From: Augustine Lee Date: Mon, 3 Aug 2026 01:28:53 -0700 Subject: [PATCH 3/3] [mpls][sonic-vpp] Load mpls_router instead of ignoring the intfmgrd error Enabling MPLS on an interface makes intfmgrd run "sysctl -w net.mpls.conf..input=1", which needs the mpls_router kernel module. The module ships in the image but nothing loads it, so the sysctl fails and is logged as an ERR. The previous revision suppressed that error with a LogAnalyzer ignore. Load the module in the setup fixture instead, which removes the cause rather than the symptom: the sysctl then succeeds and SONiC's own "config interface mpls add" completes as designed. sonic-swss's own MPLS test (tests/test_mpls.py setup_mpls) loads it the same way, and sonic-mgmt already modprobes in tests/console/conftest.py and tests/test_pktgen.py. Verified from a clean baseline (module rmmod'ed first): 3 passed, 1 skipped, zero setIntfMpls errors, with no LogAnalyzer ignore in place. The module is deliberately left loaded: modprobe is idempotent, the module ships in the image, and unloading could disrupt anything else using MPLS. It touches no CONFIG_DB state. Signed-off-by: Augustine Lee Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 251d571f-a53b-47f8-9eb4-293dbc6ff7a4 --- tests/mpls/conftest.py | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/tests/mpls/conftest.py b/tests/mpls/conftest.py index e82d3fa75cb..0c1d6c83615 100644 --- a/tests/mpls/conftest.py +++ b/tests/mpls/conftest.py @@ -30,23 +30,6 @@ def _resolve_ptf_port_ids(dut_port, mg_facts): return [mg_facts['minigraph_port_indices'][member] for member in members] -@pytest.fixture(autouse=True) -def ignore_expected_loganalyzer_exceptions(duthost, loganalyzer): - """Ignore the kernel-MPLS errors intfmgrd emits on platforms without mpls_router. - - Configuring MPLS on an interface makes intfmgrd run - "sysctl -w net.mpls.conf..input=1". That requires the mpls_router kernel - module, which no SONiC image loads by default, so the command fails and is - logged as an ERR. On sonic-vpp this is harmless: MPLS forwarding is done by - VPP in userspace via the SAI INSEG entries, not by the Linux kernel data path. - """ - if loganalyzer and duthost.facts.get('asic_type') == 'vpp': - loganalyzer[duthost.hostname].ignore_regex.extend([ - r".*ERR swss#intfmgrd.*setIntfMpls: Command 'sysctl -w net\.mpls\.conf\..* failed with rc 1.*", - ]) - yield - - @pytest.fixture(scope='module') def setup(duthost, tbinfo, ptfadapter): """ @@ -58,6 +41,21 @@ def setup(duthost, tbinfo, ptfadapter): if tbinfo['topo']['type'] != 't1': pytest.skip('Unsupported topology') + # Enabling MPLS on an interface makes intfmgrd run + # "sysctl -w net.mpls.conf..input=1", which needs the mpls_router kernel + # module. The module ships in the image but nothing loads it, so without this + # the sysctl fails and is logged as an ERR. sonic-swss's own MPLS test loads it + # the same way. Left loaded on teardown: modprobe is idempotent and unloading + # could disrupt anything else using MPLS. + if duthost.facts['asic_type'] == 'vpp': + result = duthost.shell('modprobe mpls_router', module_ignore_errors=True) + if result['rc'] != 0: + # Not fatal here: let the test itself fail on the resulting syslog + # error rather than hiding a genuine loss of kernel MPLS support + # behind a setup failure. + logger.warning('Failed to load mpls_router: %s. Enabling MPLS on an ' + 'interface will log a setIntfMpls error.', result['stderr']) + # gather ansible facts mg_facts = duthost.minigraph_facts(host=duthost.hostname)['ansible_facts'] host_facts = duthost.setup()['ansible_facts']