[vpp] Add MPLS data-plane support (INSEG + IP-route push)

[augusdn]

Description of PR

Summary:
Part of sonic-net/sonic-buildimage#25782

Adds MPLS data-plane support to the VPP SAI backend so the tests/mpls data-plane
tests can run on the sonic-vpp testbed. Today SAI_OBJECT_TYPE_INSEG_ENTRY is not
handled at all in vslib/vpp, so MPLS traffic is never programmed into VPP.

Note

Companion PR sonic-net/sonic-mgmt#26619 enables mpls/test_mpls.py on sonic-vpp and
depends on this change being in the image, so it merges after this one.

Type of change

• Bug fix
• New feature
• Refactor / cleanup
• Documentation update
• Test improvement

Approach

What is the motivation for this PR?

sonic-buildimage#25782 asks for MPLS support in sonic-vpp so that mpls/test_mpls.py
can be enabled on the VPP KVM testbed. The tests currently can't run because the VPP
SAI backend has no INSEG handling.

Work item tracking

• Microsoft ADO (number only):

How did you do it?

10 files, +737 / -5, all under vslib/vpp:

File	Change
vslib/vpp/SwitchVppMpls.cpp (new)	INSEG create/remove: pop, swap and push disposition into the VPP MPLS FIB.
vslib/vpp/SwitchVpp.{cpp,h}	Dispatch SAI_OBJECT_TYPE_INSEG_ENTRY from create()/remove(); declare the MPLS helpers and the MPLS-table state.
vslib/vpp/SwitchVppNexthop.{cpp,h}	Carry the out-label stack on next hops; n_labels is explicitly zeroed in fillNHGrpMember().
vslib/vpp/SwitchVppRoute.cpp	Impose the out-label stack on IP routes (ingress LER), gated on n_labels > 0.
vslib/vpp/SwitchVppRif.cpp	Honour SAI_ROUTER_INTERFACE_ATTR_ADMIN_MPLS_STATE (sw_interface_set_mpls_enable, ensure the MPLS table exists).
vslib/vpp/vppxlate/SaiVppXlate.{c,h}	mpls_table_add_del(), mpls_route_add_del(), and the label stack on fib paths for both MPLS and IP routes.
vslib/Makefile.am	Build the new source file.

Details worth a reviewer's attention:

• pop: inject an implicit-null out-label in UNIFORM LSP mode so the disposition
  derives the inner IP TTL from the popped MPLS TTL. Without it VPP defaults the
  disposition to PIPE and the inner TTL is only decremented once by the IP stage.
• swap/push: read the SAI next hop's out-label stack. SAI_NEXT_HOP_ATTR_LABELSTACK
  is a list attribute, so the output buffer has to be supplied before the get,
  otherwise the read fails and the labels are silently dropped - which turns a swap
  into a bare pop.
• Resolve the next hop's router interface to its VPP egress hwif so the path is
  programmed attached rather than recursive. VPP only inserts the MPLS disposition
  for an attached next hop, and a recursive labelled path fails to resolve and is
  dropped at the MPLS DROP DPO.
• The SAI INSEG key has no end-of-stack qualifier but the VPP MPLS FIB is keyed by
  {label, eos}, so eos=1 is always programmed and eos=0 as well when out-labels are
  present, so a non-bottom label in a stack is handled.
• mpls_route_add_del() validates every path (address family and label count) before
  allocating the API message, matching what ip_route_add_del_get_stats() already does,
  so the error path can't leak the message.

Status is propagated the same way the neighbouring addIpRoute()/removeIpRoute()
do it: addMplsRoute() rolls back the VPP programming if create_internal() fails,
and ensureMplsTable() only latches its "created" flag once the table really exists.

How did you verify/test it?

With this change in the image. A throwaway sonic-buildimage build carrying this PR
produced image SONiC.master-28652.1177040-4e2ccc26c. Run on a vms-kvm-vpp-t1-lag KVM
testbed together with the companion sonic-mgmt PR, ElasticTest plan
6a69da68f481df03c4e59c5e - SUCCESS, 22 tests, 17 passed / 5 skipped / 0 failed /
0 errors:

mpls/test_mpls.py::TestBasicMpls::test_pop_label        PASSED
mpls/test_mpls.py::TestBasicMpls::test_swap_label       PASSED
mpls/test_mpls.py::TestBasicMpls::test_push_label       SKIPPED
mpls/test_mpls.py::TestBasicMpls::test_swap_labelstack  PASSED

3 passed, 1 skipped

Without this change in the image (negative control). The dependency was verified
rather than assumed. The identical test code, run by the t1-lag-vpp PR checker on a
stock sonic-vpp image that does not carry this PR, fails:

mpls/test_mpls.py|||2 failed for RUN_TEST_CASE_FAILED

(ElasticTest plan 6a70603deb2c1b23503d2e2f, build 1182699; same signature on the
earlier build 1178163.) mpls/test_mpls.py is the only failure in that run - the job
executes the whole t1-lag-vpp list. That is exactly the gap this PR closes.

Regression check. The same image also ran the full t1-lag-vpp suite
(1762 tests, 1222 passed) to check this doesn't regress anything else on VPP.

test_push_label is skipped for a reason outside this change: it 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.

Any platform specific information?

sonic-vpp only - everything is under vslib/vpp. Non-MPLS traffic is unaffected: the
INSEG path only runs for INSEG objects, the next-hop label handling is gated on
SAI_NEXT_HOP_TYPE_MPLS, and label imposition is gated on n_labels > 0.

Known limitations, called out deliberately rather than half-implemented - happy to
follow up on any of these if reviewers would prefer them in scope:

• IPv6 payloads: eos_proto_af is IPv4.
• ECMP / next-hop-group MPLS next hops are not programmed.
• INSEG set() does not reprogram VPP (create/remove only).
• NUM_OF_POP > 1, and non-default POP TTL/QoS modes, are not implemented.

Documentation

No user-facing documentation change.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Adds MPLS data-plane programming to the VPP SAI backend (vslib/vpp) so SONiC MPLS dataplane tests can run on the sonic-vpp testbed. This introduces INSEG (local label) handling into VPP’s MPLS FIB, plus label imposition on IP routes via fib-path label stacks, and enables MPLS on router interfaces.

Changes:

• Implement INSEG (SAI_OBJECT_TYPE_INSEG_ENTRY) create/remove handling by translating to VPP MPLS route programming (including eos handling and pop vs swap/push behavior).
• Extend VPP translation/plumbing to support MPLS table/route APIs and fib-path label stacks for both MPLS routes and IP routes.
• Honor SAI_ROUTER_INTERFACE_ATTR_ADMIN_MPLS_STATE to enable MPLS on the underlying VPP interface and ensure the MPLS table exists.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
vslib/vpp/vppxlate/SaiVppXlate.h	Adds MPLS label-stack limits and new MPLS route/nexthop structs + API declarations.
vslib/vpp/vppxlate/SaiVppXlate.c	Adds VPP MPLS API integration and implements MPLS table/route programming + label stacks on IP fib paths.
vslib/vpp/SwitchVppRoute.cpp	Propagates per-next-hop label stacks into VPP IP route programming.
vslib/vpp/SwitchVppRif.cpp	Enables/disables MPLS per RIF using VPP API and ensures MPLS table exists.
vslib/vpp/SwitchVppNexthop.h	Extends next-hop group member structure with MPLS label stack fields.
vslib/vpp/SwitchVppNexthop.cpp	Reads MPLS next-hop label stacks and resolves attached egress sw_if_index for labeled forwarding.
vslib/vpp/SwitchVppMpls.cpp	New INSEG→VPP MPLS FIB programming implementation (pop + swap/push).
vslib/vpp/SwitchVpp.h	Adds MPLS route helpers and tracks MPLS table creation state.
vslib/vpp/SwitchVpp.cpp	Dispatches INSEG create/remove to the new MPLS implementation.
vslib/Makefile.am	Adds the new SwitchVppMpls.cpp to the build.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[augusdn]

Thanks — both review comments were valid, and I've pushed fixes in 6c02b43.

1. SAI_STATUS_BUFFER_OVERFLOW on the label stack

Confirmed by tracing the propagation path rather than assuming:

• meta/SaiSerialize.cpp:151 — transfer_list returns SAI_STATUS_BUFFER_OVERFLOW when the caller's buffer is too small, overwrites count with the required size, and copies nothing.
• vslib/SwitchStateBase.cpp:768 — propagates it as final_status.
• vslib/vpp/SaiObjectDB.cpp:386 / :398 / :420 — all three get_attr implementations pass it through.

So with VPP_MPLS_MAX_LABELS = 16, a deeper stack failed the == SAI_STATUS_SUCCESS test, left n_labels at 0, and programmed a bare pop instead of swap/push — silent misforwarding, exactly as described. Both sites now detect BUFFER_OVERFLOW explicitly, log the required depth, and return SAI_STATUS_NOT_SUPPORTED. Any other failure still falls through to "no label stack", which is the legitimate pop/disposition path for a SAI_NEXT_HOP_TYPE_IP next hop.

One thing the review didn't mention: the cnt > VPP_MPLS_MAX_LABELS clamps in both functions were dead code. On SAI_STATUS_SUCCESS the returned count can never exceed the supplied 16-entry buffer, so those branches were unreachable and implied a bound check that never actually ran. Removed them rather than leave a misleading safety net.

2. Partial two-EOS rollback (found in a separate review pass)

MplsRouteAddRemove programs two VPP FIB entries for a swap/push — eos=0 then eos=1. If eos=0 succeeded and eos=1 failed, the orphaned eos=0 entry stayed in the MPLS FIB with no SAI object referring to it, because addMplsRoute returns through CHECK_STATUS(...) before setting route_programmed = true, so the existing create-time rollback never ran.

The loop now unwinds whatever it managed to program. Removing an entry that was never added is safe — vpp_normalize_ret (vslib/vpp/vppxlate/SaiVppXlate.c:454) maps NO_SUCH_ENTRY to success on delete.

Verification

Rebuilt against VPP 2606 to match the sonic-vpp image (the tree's older helper linked 2510, which the DUT can't load) — clean compile, zero warnings under -Werror -Wall -Wextra -Wconversion -Wshadow -Wcast-align=strict -Wformat=2. The repo's own gates also pass locally: aspellcheck.pl, checkwhitespace.sh, swsslogentercheck.sh.

Functionally re-verified on a vms-kvm-vpp-t1-lag KVM testbed with this exact binary installed in syncd:

mpls/test_mpls.py::TestBasicMpls::test_pop_label        PASSED
mpls/test_mpls.py::TestBasicMpls::test_swap_label       PASSED
mpls/test_mpls.py::TestBasicMpls::test_push_label       SKIPPED
mpls/test_mpls.py::TestBasicMpls::test_swap_labelstack  PASSED

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

vslib/vpp/SwitchVppMpls.cpp:254

• mpls_route_add_del() is invoked for multiple EOS keys, but rollback only happens on add. If a multi-EOS delete fails after removing one entry, the SAI object remains (because remove_internal is not reached) while some VPP FIB entries have already been removed, leaving dataplane state inconsistent with the in-memory object. Consider rolling back partial deletes (re-add any successfully removed EOS entries) the same way partial adds are rolled back.

    if (ret != 0 && is_add) {
        for (int e = 0; e < programmed; e++) {
            route->eos = eos_list[e];
            mpls_route_add_del(route, false);
        }

vslib/vpp/vppxlate/SaiVppXlate.c:2820

• ip_route_add_del_get_stats() now forwards nexthop->n_labels directly into the VPP API message. If n_labels is ever > VPP_MPLS_MAX_LABELS, VPP will be told there are more labels than the message actually populates (only up to the max), which can lead to invalid API input. Clamp (or validate+fail) n_labels before assigning it to fib_path->n_labels.

        fib_path->n_labels = nexthop->n_labels;
        for (uint8_t l = 0; l < nexthop->n_labels && l < VPP_MPLS_MAX_LABELS; l++) {
            fib_path->label_stack[l].label = htonl(nexthop->label_stack[l]);
            fib_path->label_stack[l].ttl = 64;
            fib_path->label_stack[l].exp = 0;

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (3)

vslib/vpp/vppxlate/SaiVppXlate.c:2621

• mpls_route_add_del() stores route->nexthop_cnt in path_count but later writes mr->mr_n_paths = (u8)path_count without validating the range. If nexthop_cnt > 255, the value will truncate and the allocated message size (sizeof(vl_api_fib_path_t) * path_count) won’t match what gets encoded, leading to incorrect programming or a potential overrun.

    path_count = route->nexthop_cnt;

vslib/vpp/vppxlate/SaiVppXlate.c:2666

• When nexthop->hwif_name can’t be resolved to a sw_if_index, the code currently falls back to ~0 and continues. For MPLS routes this can silently turn an intended attached path into a recursive one, which (per the PR’s own constraints) can result in traffic being dropped while the API still reports success. It’s safer to treat an unknown hwif_name as an error and fail the operation.

        } else if (nexthop->hwif_name) {
            idx = get_swif_idx(vam, nexthop->hwif_name);
            fib_path->sw_if_index = htonl(idx != (u32) -1 ? idx : (uint32_t)~0);
        } else {
            fib_path->sw_if_index = htonl((uint32_t)~0);
        }

vslib/vpp/SwitchVppMpls.cpp:256

• The log message reports out_labels using route->nexthop[0].n_labels, but in the pop/disposition case the code injects an implicit-null label and sets n_labels to 1. That makes the log misleading (it will show 1 out-label for a pop). Logging based on has_outlabels preserves the intended meaning.

    SWSS_LOG_NOTICE("%s inseg label %u out_labels %u status %d",
                    (is_add ? "Add" : "Remove"), inseg_entry.label,
                    route->nexthop[0].n_labels, ret);

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[augusdn]

Pushed 14401c7, which addresses the remaining automated-review feedback. Going through all five suppressed comments from the last two review passes explicitly, since two of them were worth acting on and three I think should be declined.

Fixed

1. ip_route_add_del_get_stats did not clamp n_labels (SaiVppXlate.c)

Valid, and the more important of the two. The code assigned nexthop->n_labels straight into fib_path->n_labels while the loop only ever populated up to VPP_MPLS_MAX_LABELS entries, so a deeper stack would have told VPP the message carried more labels than it actually did.

It is currently unreachable — fillNHGrpMember now fails on BUFFER_OVERFLOW, so n_labels can never exceed 16 — but the asymmetry was real: mpls_route_add_del validates this and returns -EINVAL, and the IP path had no equivalent. Now clamped before the assignment, so the count and the populated entries always agree.

2. Misleading out_labels in the MplsRouteAddRemove log (SwitchVppMpls.cpp)

Also valid. The pop case injects an implicit-null label and sets n_labels to 1, so a pop was logged as having one out-label. Now reports the SAI-visible count, which is what a reader of that log is actually after.

Declined, with reasoning

3. Roll back partial deletes the way partial adds are rolled back

I do not think this one should be actioned. Rolling back a failed delete means re-installing MPLS FIB entries during a teardown that is already failing — re-adding forwarding state for a next hop that is on its way out is worse than the transient inconsistency it would fix.

It is also self-healing: vpp_normalize_ret maps NO_SUCH_ENTRY to success on delete, so when orchagent retries the remove, the already-deleted EOS entry is a no-op and the remaining one is retried. The add case is genuinely different — there the caller aborts before recording the route, so nothing in SAI refers to the orphaned entry, which is why that one was worth unwinding.

4. mr_n_paths could truncate if nexthop_cnt > 255

True in the abstract, but this matches the existing convention exactly: the neighbouring ip_route_add_del_get_stats does the identical unvalidated ip_route->n_paths = (u8)path_count. Adding a check to only the MPLS path would make the two inconsistent without fixing the general case. In practice INSEG always passes nexthop_cnt = 1 (hardcoded in MplsRouteAddRemove). If this is worth tightening it should be done for both paths, which is beyond the scope of this PR.

5. Treat an unresolvable hwif_name as an error rather than falling back to ~0

The fallback is deliberate and documented in MplsRouteAddRemove. It is also effectively unreachable: hwif_name is only set after vpp_get_hwif_name() has already succeeded for that port, so get_swif_idx failing afterwards would mean the interface disappeared mid-call. Turning that into a hard failure trades a rare degraded-forwarding case for a hard error on a path that today works, which I do not think is a good trade without evidence it happens.

Happy to revisit any of these three if a reviewer disagrees.

Verification

Rebuilt against VPP 2606 to match the sonic-vpp image — clean, zero warnings under -Werror -Wall -Wextra -Wconversion -Wshadow -Wcast-align=strict -Wformat=2. The repo's own gates pass locally (aspellcheck.pl, checkwhitespace.sh, swsslogentercheck.sh), as does make check for meta, lib, vslib and unittest/vslib, and the full tests suite (all 11).

Both changes here are on paths the functional tests already cover: the label-count clamp sits in the IP-route imposition path exercised by test_swap_labelstack, and the log change is observational only.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

[augusdn]

/azp run Azure.sonic-sairedis

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[augusdn]

Rebased onto current master (11984240) and pushed de99c7a. Flagging why, since the CI failure that prompted it was not a flake and the cause is worth knowing for anyone with an older VPP branch in flight.

What broke

Two consecutive builds failed at Compile sonic sairedis with coverage enabled:

vpp/vppxlate/SaiVppXlate.c:5494:5: error: redefinition of 'get_sw_if_idx'
vpp/vppxlate/SaiVppXlate.c:2673:5: note: previous definition of 'get_sw_if_idx' with type 'int(const char *)'
make[4]: *** [Makefile:1029: vpp/vppxlate/libSaiVS_a-SaiVppXlate.o] Error 1

This is a semantic merge conflict, which is why it wasn't visible earlier:

• The merge base (24c799da) has zero definitions of get_sw_if_idx.
• This PR adds one.
• 51fe70f "[vslib] Add support for SPAN mirror sessions" ([vslib] Add support for SPAN mirror sessions #1920) landed after this branch was cut and adds another.

Git merges both cleanly — the PR reported MERGEABLE throughout — but the merged result doesn't compile. Local builds passed because the branch base predated #1920.

Fix

Dropped ours and kept the upstream definition, which is strictly better: it takes VPP_LOCK() around get_swif_idx(), which ours did not. Behaviour is otherwise identical — ours mapped (u32)-1 to -1 explicitly, and upstream's return (int)idx; yields the same value for that input.

No header change was needed: the extern declaration is identical on both sides, so this PR no longer touches SaiVppXlate.h for that symbol. Net effect of the new commit is -11 lines.

Verification

Full CI replication in sonic-slave-trixie against the rebased tree:

dpkg_buildpackage_rc=0        <- the step that was failing
9 debs produced
vslib/vpp/vppxlate/libSaiVS_a-SaiVppXlate.o   built clean
0 compile errors, 0 redefinition errors

make check reports one failure, unittest/lib → ServerSai.stats_st_capability_query, aborting on Select::add_fd:epoll_ctl: error=(1):Operation not permitted. That one is environmental and not related to this change:

• ServerSai mocks getFd() to return 0 (stdin), and epoll_ctl returns EPERM for a stdin that isn't pollable — which is what you get running the suite under docker run with stdin redirected.
• This PR touches zero files under lib/ or unittest/.
• unittest/lib/tests doesn't even link vslib: tests_LDADD = ... lib/libSaiRedis.a -lhiredis -lswsscommon -lpthread -L meta/.libs -lsaimetadata -lsaimeta -lzmq. Nothing from this PR is in that binary.

The four functional commits are unchanged by the rebase; only the new fifth commit differs.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

vslib/vpp/vppxlate/SaiVppXlate.c:2953

• sw_interface_set_mpls_enable() normalizes the VPP return code as if the operation is always an "add" (vpp_normalize_ret(..., false)). For enable/disable toggles, disable behaves like a delete in terms of idempotency (similar to sw_interface_ip6_enable_disable), so repeated disables can legitimately return NO_SUCH_ENTRY and should be treated as success. Using !enable here avoids failing router-interface create/update when MPLS is already disabled/enabled.

    ret = vpp_normalize_ret(ret, false, __func__);

[augusdn]

CI is green on the rebased head de99c7a: 19 checks pass, 0 fail (ADO build 1184588).

That includes the two jobs that were failing before the rebase - Build amd64 and BuildAsan amd64 - plus Test vstest (1h26m) and TestAsan vstest (1h38m), and the ARM/Trixie/Docker/Swss builds.

For anyone reviewing the timeline: the earlier red builds were the get_sw_if_idx redefinition described in the previous comment, caused by #1920 landing after this branch was cut. That is fixed; no functional commit changed.

[yue-fred-gao]

can you derive eos_proto_af from nexthop family, instead of hardcode?

[yue-fred-gao]

can we set label stack is_uniform and ttl value based on sai attribute SAI_NEXT_HOP_ATTR_OUTSEG_TTL_MODE and SAI_NEXT_HOP_ATTR_OUTSEG_TTL_VALUE?

[yue-fred-gao]

minor: you can use get_linked_object to get rif object from nexthop

[yue-fred-gao]

Can we support ttl/qos mode and ttl/exp value from SAI attribute instead of hardcode here? It seems not requiring a lot of code

[yue-fred-gao]

camel case

[yue-fred-gao]

should we set is_uniform based on SAI_INSEG_ENTRY_ATTR_POP_TTL_MODE?