Skip to content

[Draft Only - Do Not Merge/Review] SAI VPP UT Harness - Phase 3 - #1992

Open
nicholasching wants to merge 97 commits into
sonic-net:masterfrom
nicholasching:sai_vpp_ut_phase3
Open

[Draft Only - Do Not Merge/Review] SAI VPP UT Harness - Phase 3#1992
nicholasching wants to merge 97 commits into
sonic-net:masterfrom
nicholasching:sai_vpp_ut_phase3

Conversation

@nicholasching

Copy link
Copy Markdown
Contributor

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Approach

What is the motivation for this PR?

Work item tracking
  • Microsoft ADO (number only):

How did you do it?

How did you verify/test it?

Any platform specific information?

Documentation

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Backend portion of the L3-over-LAG forwarding fix (BVI/bond FDB handling in
SwitchVppFdb.cpp). The UT-harness portion (run_test.sh LAG/SVI interface-IP
assignment) and the related devdocs are in the harness PR.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
programNeighborHostRoute() left the host-route nexthop sw_if_index at 0, so
ip_route_add_del() pinned the neighbor /32//128 host route to VPP's local0
and dropped traffic even though the correct adjacency over the real egress
interface existed. Set sw_if_index = ~0 to resolve the egress via hwif_name.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
…test

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
…dog, --relax

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
…ature and restarting backend per group

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
…ugging

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
UT-harness portion of the L3-over-LAG forwarding fix: run_test.sh assigns the
DUT-side connected IPs to each LAG (be<N>) and SVI (bvi<vlan>) so routed-to-LAG
/SVI traffic forwards in the standalone PTF environment. The backend portion
(SwitchVppFdb.cpp) is in the VPP SAI backend PR.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
…SER)

- gen_compatibility_matrix.py: prefer defusedxml for parsing PTF JUnit XML
  (XXE-hardened), with a stdlib fallback; input is our own local test output.
- Dockerfile: annotate the root entrypoint with a nosemgrep justification — the
  --privileged harness must run as root to start VPP/Redis/saiserver and manage
  veth/AF_PACKET; it is a disposable test container, never a deployed service.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Satisfy tests/swsslogentercheck.sh for static helpers introduced in
the neighbor host-route refactor.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Satisfy tests/checkwhitespace.sh (Azure Build amd64 gate).

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
member_hwif_str was loop-scoped while create_vpp_nexthop_entry stored its
c_str() and ip_route_add_del_get_stats ran after the loop, leaving dangling
pointers in ip_route->nexthop[].

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Deduplicate RIF-to-hwif resolution used when programming multipath routes
and single-path route updates.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Add getTapNameFromPortOrLagId for be<N> LCP taps, teach tap_to_hwif_name to
map bond taps to BondEthernet<N>, and route vpp_get_hwif_name and
vpp_add_del_intf_ip_addr through the shared helpers.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Merge programNeighborHostRoute into addRemoveIpNbr with adjacency and
host-route phase flags, and read NO_HOST_ROUTE via SaiCachedObject or
SaiDBObject get_attr instead of manual attribute scans.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (4)

vslib/vpp/SwitchVppHostif.cpp:579

  • vs_remove_hostif_tap_interface() will attempt VPP teardown even when tap_to_hwif_name() returns the sentinel "Unknown" (which is non-null). That can trigger spurious VPP API calls against a non-existent interface during hostif removal.
    vslib/vpp/vppxlate/SaiVppXlate.c:394
  • vpp_normalize_ret() contains a duplicated VALUE_EXIST-on-add normalization block. Because the first branch already handles the add case, the second if (!is_del && ret == VNET_API_ERROR_VALUE_EXIST) is redundant and can cause duplicate log lines and confusion (and risks diverging behavior if one branch is later edited).
    vslib/vpp/SwitchVppNbr.cpp:24
  • SwitchVppNbr.cpp adds forward declarations for create_route_prefix_entry() and create_vpp_nexthop_entry() that are implemented in SwitchVppRoute.cpp. Keeping cross-TU helper declarations in a .cpp makes it easy for signatures to drift silently and requires non-static global linkage. Consider moving these helpers (or their declarations) into a shared header/utility (or converting to SwitchVpp private helpers).
    .azure-pipelines/build-template.yml:66
  • build-template.yml sets VPP_RUN_ID using dependencies.ResolveVpp.outputs[...], but the template parameter depends_on is optional. If a caller passes a non-empty vpp_run_id without also adding a ResolveVpp dependency, this runtime expression will fail evaluation. Prefer using the provided parameter value directly.
    ${{ if ne(parameters.vpp_run_id, '') }}:
      VPP_RUN_ID: $[ dependencies.ResolveVpp.outputs['resolveVppRun.VPP_RUN_ID'] ]

# Conflicts:
#	vslib/vpp/SwitchVpp.h
#	vslib/vpp/SwitchVppRif.cpp

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Copilot AI review requested due to automatic review settings August 5, 2026 17:59
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

vslib/vpp/SwitchVppHostif.cpp:404

  • The admin-state reapply block is duplicated back-to-back, which will run the same get()+interface_set_state() twice and makes future edits error-prone. Remove the second duplicated comment+block (keep only one copy).
    .azure-pipelines/docker-sai-test-vpp/swss_log_stdout_preload.cpp:3
  • The file header says logs are routed to stdout, but the implementation routes SWSS logs to STDERR (and the later comment explicitly explains why). This is misleading when debugging harness output; update the header comment to match the actual behavior.
// Harness-only LD_PRELOAD shim: route SAI VS library SWSS_LOG_* to stdout so
// run_test.sh capture in /var/log/sai-server.log works without pulling SONiC
// logger setup back into the SAI server binary.

Comment thread azure-pipelines.yml
Comment on lines +128 to +133
set -euo pipefail
vpp_run_id="$(downloadVpp.BuildNumber)"
test -n "$vpp_run_id"
echo "Resolved sonic platform-vpp run ID: $vpp_run_id"
echo "##vso[task.setvariable variable=VPP_RUN_ID;isOutput=true]$vpp_run_id"
name: resolveVppRun
Comment on lines +99 to +102
if [[ "$(downloadVpp.BuildNumber)" != "${{ parameters.vpp_run_id }}" ]]; then
echo "Resolved VPP run ${{ parameters.vpp_run_id }}, downloaded $(downloadVpp.BuildNumber)" >&2
exit 1
fi
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants