Skip to content

[202605][intfsorch]: Retry explicit router interface VRF rehome#4765

Closed
Xichen96 wants to merge 2 commits into
sonic-net:202605from
Xichen96:dev/xichenlin/intfsorch-vrf-rehome-202605
Closed

[202605][intfsorch]: Retry explicit router interface VRF rehome#4765
Xichen96 wants to merge 2 commits into
sonic-net:202605from
Xichen96:dev/xichenlin/intfsorch-vrf-rehome-202605

Conversation

@Xichen96

@Xichen96 Xichen96 commented Jul 19, 2026

Copy link
Copy Markdown

What I did

Backported the explicit physical-interface VRF rehome retry to 202605.

  • Keep the interface SET in m_toSync while prefixes or RIF references still exist.
  • Mark the rehome as pending so new prefix SETs and the guarded route/neighbor dependencies wait for the replacement RIF.
  • Allow dependency DELs to continue using the old RIF so it can drain.
  • Remove the old RIF and create its replacement in the requested VRF only after the interface is locally quiescent.
  • Preserve the full Port state used to create the RIF, including loopback action, MPLS state, NAT zone, and custom RIF MAC.
  • Apply simultaneous subinterface MTU/admin updates and avoid treating later partial updates without vrf_name as another move.
  • If replacement creation throws, recreate the old RIF and leave the SET queued for retry; logical VRF/refcount state changes only after replacement succeeds.

Why I did it

Whole-interface deletion already handled cross-key ordering correctly: if it arrived before the final prefix DEL, it returned false, stayed queued, and retried after dependencies drained.

An explicit VRF-changing SET did not. When a physical interface already had a RIF, the old create path returned success without moving it. If the consumer visited:

Vlan1000                 SET vrf_name=VrfNew
Vlan1000:10.0.0.1/24     DEL
Vlan1000:10.0.0.1/24     SET

the first SET could be consumed while the old prefix still existed. The later DEL removed the dependency, but there was no VRF request left to revisit, so the RIF and IP2Me route remained in the old virtual router.

The fix does not require APP_DB producers to coordinate a global barrier or deliver keys in lifecycle order. IntfsOrch retains the unresolved VRF request, lets old dependencies drain, performs the replacement, and then allows queued additions to resolve against the new RIF. This is local quiescence and retry, not a universal priority queue.

If ProducerStateTable coalesces multiple operations on the same key before the consumer receives them, the consumer cannot reconstruct those lost intermediate events; this PR does not claim to solve that separate StateTable lifecycle-coalescing case.

How I verified it

Added focused mock coverage for:

  • VRF SET retained across prefix/reference drain and prefix re-add on the new RIF.
  • Loopback action, MPLS state, custom MAC, and other creation attributes preserved across replacement.
  • Simultaneous subinterface VRF/MTU/admin update.
  • A later partial SET without vrf_name.
  • Replacement-RIF creation failure and rollback to the old RIF.

The earlier combined 202605 branch containing this implementation completed the amd64, ARM, ASAN, Docker, and Trixie compile stages in Azure build 1168367. Its vstest jobs stopped before executing tests because the workers lacked pip3.

An earlier combined 202605 candidate, hardware build 1167872, passed traced and production 1x/5x runs and all ten production 10x test bodies. This smaller stacked commit adds state-preservation and rollback hardening after that run, so exact-commit validation is provided by this PR's new CI.

Details if related

Depends on the 202605 deletion-fence PR: #4747

Master PR: #4764

Xichen96 added 2 commits July 19, 2026 21:26
…ce removal

Keep the existing whole-interface DEL retry fenced after prefix deletion. Standard route, neighbor, and neighbor-next-hop creation now retries instead of attaching to the RIF being removed, while teardown continues to use the raw RIF lookup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079
Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
Retain an explicit vrf_name SET until prefixes and RIF references drain, then replace the RIF in the requested VRF. Preserve interface attributes, block replacement prefix additions, and roll back to the old RIF if replacement creation fails.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079
Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to 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

Backport to the 202605 branch of the IntfsOrch “explicit VRF rehome retry” behavior, ensuring interface VRF changes are retained and retried until prefixes and dependent objects drain, and that newly created dependencies are fenced while the replacement RIF is pending.

Changes:

  • Add m_pendingVrfUpdates fencing and VRF rehome replacement/rollback logic in IntfsOrch, including preservation of key RIF creation attributes (MAC, loopback action, MPLS/NAT state via Port).
  • Update RouteOrch/NeighOrch to honor the new dependency fence via getRouterIntfsIdForNewDependency() and to propagate addNextHop() failure.
  • Extend mock-test coverage for VRF rehome drain/retry, attribute preservation, partial updates, and rollback-on-create-failure.

Reviewed changes

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

Show a summary per file
File Description
tests/mock_tests/intfsorch_ut.cpp Adds focused unit tests for VRF rehome retry/drain behavior, attribute preservation, and rollback.
orchagent/routeorch.cpp Switches some interface NH lookups to the fenced API and propagates neighbor next-hop creation failure.
orchagent/neighorch.cpp Uses fenced RIF lookup for new dependency creation and returns retry when RIF is unavailable.
orchagent/intfsorch.h Extends IntfsEntry with preserved attributes and introduces getRouterIntfsIdForNewDependency() plus pending-update tracking.
orchagent/intfsorch.cpp Implements pending VRF update fencing, VRF rehome replacement/rollback, explicit-field handling, and attribute preservation updates.
Comments suppressed due to low confidence (1)

orchagent/routeorch.cpp:2103

  • addRoute() now correctly uses getRouterIntfsIdForNewDependency() for interface next-hops, but other route-programming paths still appear to bypass the dependency fence. In particular, addRoutePost() still calls m_intfsOrch->getRouterIntfsId(nexthop.alias) (around routeorch.cpp:2450) for interface NH validation, which can allow a route update to proceed and take a new ref on a RIF while an interface is in m_pendingVrfUpdates/m_removingIntfses. That undermines the intended “new dependencies wait for replacement RIF” behavior during VRF rehome/removal.

Recommend switching the interface NH lookup in addRoutePost() (and any similar validation paths) to getRouterIntfsIdForNewDependency() so all route additions consistently honor the fence.

            next_hop_id = m_intfsOrch->getRouterIntfsIdForNewDependency(nexthop.alias);
            /* rif is not created yet */
            if (next_hop_id == SAI_NULL_OBJECT_ID)
            {
                SWSS_LOG_INFO("Failed to get next hop %s for %s",
                        nextHops.to_string().c_str(), ipPrefix.to_string().c_str());
                return false;

@Xichen96

Copy link
Copy Markdown
Author

Closing this dedicated 202605 backport because the release backport will be generated automatically after the ordered master PRs #4746 and #4764 merge.

@Xichen96 Xichen96 closed this Jul 19, 2026
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