[vpp] SAI RIF loopback packet action and pre-existing VPP vslib defect fixes - #1983
Draft
aaronber0614 wants to merge 8 commits into
Draft
aaronber0614 wants to merge 8 commits into
aaronber0614 wants to merge 8 commits into
Conversation
Translate SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION on router interface create and set into a call to the iface_loopback VPP plugin's binary API. SwitchVppRif: add vpp_set_interface_loopback_action(), wired into both vpp_create_router_interface and vpp_update_router_interface. The RIF is resolved to its hwif name via the existing vpp_get_hwif_name() helper (handles port, LAG/BondEthernet, and sub-ports); SAI_PACKET_ACTION_DROP maps to 1, everything else to 0 (forward). SaiVppXlate: add the iface_loopback plugin API include block, msg_id_base registration, the set_action reply handler, and a vpp_iface_loopback_set_action() send helper that resolves the hwif name to a sw_if_index via the existing get_swif_idx() helper. Depends on the iface_loopback plugin in sonic-platform-vpp. Issue: sonic-net/sonic-buildimage#25788 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
RIF counters (show interfaces counters rif) read 0 for every router interface on VPP because only PORT stats were wired to the VPP stats segment (setPortStats); ROUTER_INTERFACE fell through to the base virtual-switch handler which returns 0. Add setRifStats(), mirroring setPortStats(): resolve the RIF to its VPP hardware interface (port/LAG, including sub-ports via the outer VLAN id) and map the VPP interface counters to SAI_ROUTER_INTERFACE_STAT_*, including tx-error -> SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_PACKETS. Wire it into getStatsExt for SAI_OBJECT_TYPE_ROUTER_INTERFACE. This makes RIF counters reflect dataplane activity and, in particular, surfaces the loopback (hairpin) drop tx-error increment in the RIF tx_err counter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
VLAN router interfaces are backed by the bridge BVI (bvi<vlan-id>) rather than a port/LAG, so vpp_update_router_interface returned early for the VLAN type and never applied SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION, and setRifStats could not resolve their counters. Add a unified vpp_get_rif_hwif_name() resolver that maps any RIF (port, LAG, sub-port, or VLAN) to its backing VPP interface, handle the loopback action for VLAN RIFs on the BVI, and use the resolver in setRifStats so VLAN RIF counters are populated too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
Fixes several pre-existing defects surfaced while enabling iface_loopback_action on the VPP KVM testbed, plus review hardening: - Guard every std::stoi on SONiC interface names with a shared vpp_safe_stoi() (SwitchVppUtils): an uncaught std::invalid_argument dropped syncd into shutdown-wait mode and stalled the whole pipeline. - Program VLAN RIF neighbors: resolve the backing VPP interface via vpp_get_rif_hwif_name() (bvi<id> for a VLAN RIF) in addRemoveIpNbr. - Add the PortChannel sub-port RIF IP on BondEthernet<id>.<vlan>, including SONiC's short Po<id>.<vlan> form, so the connected route resolves. - Skip PortChannel sub-interface entries in find_new_bond_id() so a bogus bond id 0 is not assigned to a real LAG. - Apply the loopback packet action symmetrically: on VLAN RIF create (vpp_create_bvi_interface) and clear it to FORWARD on RIF remove so a recycled interface does not inherit a stale DROP. - De-duplicate the loopback setters into vpp_apply_loopback_action and fix log/format hygiene (%u for uint32_t, descriptive parse tags, explicit <string>/<stdexcept> includes). Signed-off-by: Aaron Bernardino <aaronber@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The RIF loopback packet action node moved from the standalone iface_loopback VPP plugin into the consolidated sonic_ext plugin (sonic-platform-vpp). Point the saivpp binding at sonic_ext's binary API: include the sonic_ext .api headers, look up the sonic_ext plugin msg-id base, and rename the api-version symbol. The API message name (iface_loopback_set_action) is unchanged, so the generated VL_API_* / vl_api_* symbols and the vpp_iface_loopback_set_action() send helper are unchanged. Signed-off-by: Aaron Bernardino <aaronber@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This was referenced Jul 9, 2026
[vpp] Add sonic_ext RIF loopback packet action nodes and binary API
sonic-net/sonic-platform-vpp#259
Open
Signed-off-by: Aaron Bernardino <aaronber@microsoft.com> # Conflicts: # vslib/vpp/vppxlate/SaiVppXlate.c # vslib/vpp/vppxlate/SaiVppXlate.h
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds control-plane support in the VPP virtual switch layer for SAI RIF loopback packet action, wiring SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION through the sonic_ext VPP plugin, and enhances VPP-backed stats/translation utilities to unblock t1-lag-vpp testing across additional RIF shapes.
Changes:
- Integrate
sonic_extplugin API into the VPP xlate layer and add an RPC (vpp_iface_loopback_set_action) to apply loopback action per interface. - Add a unified RIF→VPP hwif resolver (port/LAG/sub-port/VLAN BVI) and use it for VLAN-RIF neighbor programming and loopback action application (create/set/remove paths).
- Improve robustness by adding
vpp_safe_stoi, fixing PortChannel sub-interface name handling, tightening bond-id discovery filtering, and exposing VPPtx_errorvia RIF stats.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vslib/vpp/vppxlate/SaiVppXlate.h | Declares the new VPP API shim for loopback action. |
| vslib/vpp/vppxlate/SaiVppXlate.c | Registers sonic_ext plugin message base + reply handler and implements vpp_iface_loopback_set_action. |
| vslib/vpp/SwitchVppUtils.h | Declares vpp_safe_stoi helper for guarded integer parsing. |
| vslib/vpp/SwitchVppUtils.cpp | Implements vpp_safe_stoi with exception handling + logging. |
| vslib/vpp/SwitchVppRif.cpp | Adds RIF hwif resolver and applies loopback action on RIF create/set/remove; replaces unsafe stoi call sites; improves PortChannel sub-port mapping. |
| vslib/vpp/SwitchVppNbr.cpp | Uses the unified RIF hwif resolver so VLAN RIF neighbors are programmed into VPP. |
| vslib/vpp/SwitchVppFdb.cpp | Applies loopback action on VLAN BVI create path; filters non-base PortChannel entries when discovering new bond IDs. |
| vslib/vpp/SwitchVpp.h | Declares new loopback helpers, hwif resolver, and setRifStats. |
| vslib/vpp/SwitchVpp.cpp | Implements setRifStats and hooks it into stats retrieval to surface VPP tx/rx error counters on RIFs. |
Comment on lines
+637
to
+645
| const char *hwif_name = ifname.c_str(); | ||
| int action = (packet_action == SAI_PACKET_ACTION_DROP) ? 1 : 0; | ||
|
|
||
| int ret = vpp_iface_loopback_set_action(hwif_name, action); | ||
| SWSS_LOG_NOTICE("Setting router interface loopback action %s to %s (ret %d)", | ||
| hwif_name, action ? "drop" : "forward", ret); | ||
|
|
||
| return (ret != 0) ? SAI_STATUS_FAILURE : SAI_STATUS_SUCCESS; | ||
| } |
Comment on lines
38
to
40
|
|
||
| namespace saivs | ||
| { |
Re-integrate the SAI RIF loopback packet action after upstream landed a large VPP vslib refactor (MPLS, mirror, hostif, port-speed, and a rework of the neighbor and BVI-update paths). Conflict resolutions: - SwitchVpp.h: keep both the loopback method decls and upstream vpp_set_port_speed. - SaiVppXlate.c: keep both the iface_loopback reply handler and upstream MPLS reply handlers in the msg-id table. - SwitchVppRif.cpp: keep all four loopback helpers alongside upstream vpp_set_port_speed; add the loopback attribute handler next to upstream's new MPLS handler in create/update RIF; on the VLAN branch, apply the loopback action then delegate to upstream's new vpp_update_bvi_interface (MAC/MTU). - SwitchVppNbr.cpp: take upstream's addRemoveIpNbr. Upstream independently reimplemented the VLAN-RIF neighbor programming (host-route + static adjacency, inline bvi<vlan-id> resolution), which supersedes this branch's earlier vpp_get_rif_hwif_name-based neighbor fix. Defect fixes vpp_safe_stoi, find_new_bond_id non-base skip, and the PortChannel Po<id>.<vlan> short-form handling are retained (auto-merged). The VLAN-RIF neighbor fix is dropped as redundant with upstream. Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Master advanced (sflow interface APIs, sonic_ext ip2me, PortChannel RIF fix). Conflict resolutions: - SaiVppXlate.c/.h: keep both the loopback and upstream sflow externs; converge the sonic_ext plugin msg-id registration onto upstream's canonical sonic_ext_msg_id_base / SONIC_EXT_MSG_ID (the iface_loopback message is a sonic_ext plugin message), dropping this branch's redundant iface_loopback_msg_id_base duplicate. - SwitchVppRif.cpp: upstream independently fixed the long-form PortChannel<id>.<vlan> RIF-IP resolution (defect sonic-net#4); take upstream's version and keep only this branch's additional Po<id>.<vlan> short-form branch that upstream lacks. Also address two Copilot review suggestions: - vpp_apply_loopback_action now rejects any sai_packet_action_t other than FORWARD/DROP (SAI_STATUS_NOT_SUPPORTED) instead of silently mapping TRAP/LOG/COPY to forward. - SwitchVppUtils.h includes <string> directly instead of relying on a transitive include. Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why I did it
Control-plane half of the RIF loopback packet action feature. Translate SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION into the sonic_ext plugin call iface_loopback_set_action, and surface the RIF tx_err counter from VPP. Reaching a green t1-lag-vpp run also required fixing pre-existing, feature-independent defects in the VS VPP layer that blocked the test at successive RIF shapes.
Feature tracking issue: sonic-net/sonic-buildimage#25788
How I did it
Feature:
Pre-existing defect fixes (kept as separate commits, and can be split into their own PR if reviewers prefer):
Note: an earlier version of this PR also carried a VLAN-RIF neighbor programming fix (dynamic hwif resolution via vpp_get_rif_hwif_name). That defect has since been fixed independently on master, which reworked addRemoveIpNbr with inline BVI (bvi id) resolution plus host-route and static-adjacency handling. On the latest merge from master this branch drops its own version in favor of the upstream implementation, so SwitchVppNbr.cpp carries no net change here.
Note: a mutex fix that was also found during this work is intentionally omitted here; it is already fixed upstream by #1971 and #1973.
How to verify it
Build libsaivs, deploy it into the syncd container, and run the iface_loopback_action sonic-mgmt module on vms-kvm-vpp-t1-lag. Expected result: 3 passed (basic, port_flap, reload).
This PR depends on the sonic_ext plugin PR in the series below; it includes the plugin API headers at build time.
Which release branch to backport (provide reason below if selected)
None.
Description for the changelog
Add SAI support for RIF loopback packet action on the VPP virtual switch, plus fixes for pre-existing VPP vslib defects.
Part of sonic-net/sonic-buildimage#25788.
PR series (landing order)
Tracking issue: sonic-net/sonic-buildimage#25788