[intfsorch]: Fence new RIF dependencies during interface removal

[Xichen96]

How to read this two-PR fix

The easiest place to start is the externally visible failure in PR #4764: an explicit VRF-changing interface SET could be consumed while old prefixes still existed, so the RIF remained in the old VRF.

Retaining that SET is necessary but not sufficient. While the move waits for old dependencies to drain, RouteOrch and NeighOrch must not attach new work to the old RIF. This PR fixes that prerequisite dependency-drain race.

The required implementation and merge order remains this PR first, then PR #4764, because the VRF-rehome fix builds on the explicit removal-state predicate introduced here.

What I did

Once deletion of an interface has started, new route and neighbor dependencies must not attach to the RIF being removed.

Root cause

Whole-interface deletion already had the correct retry model.

For these independent APP_DB keys:

Vlan1000                 DEL
Vlan1000:10.0.0.1/24     DEL

the bare interface key is normally visited first because the consumer stores pending work in a key-ordered multimap. That ordering was safe:

attempt Vlan1000 DEL
→ prefix still exists
→ return false
→ retain the interface DEL in m_toSync

process Vlan1000:10.0.0.1/24 DEL
→ remove the prefix and IP2Me route

retry Vlan1000 DEL
→ remove the RIF

This is why interface DEL arriving before prefix DEL had worked historically: deletion already checked its dependencies and retried.

The deletion fence was incomplete, however:

1. A failed whole-interface DEL inserted the alias into m_removingIntfses.
2. A later successful prefix DEL also erased that marker, even though the whole-interface DEL was still queued.
3. RouteOrch and NeighOrch obtained the old RIF through the unrestricted getRouterIntfsId() accessor.
4. A new route, neighbor, or neighbor next hop could therefore attach to the RIF while IntfsOrch was trying to remove it.
5. The new dependency incremented the RIF reference count, so the retained interface DEL remained blocked and could repeatedly lose the race to new work.

The problem was not that deletion lacked retry. The problem was that other orchagents could refill the dependency count during that retry.

Fix

This PR completes the existing deletion state machine:

OLD/STABLE
    new dependencies allowed
        |
        | whole-interface DEL starts
        v
REMOVING
    existing dependency DELs allowed
    new dependency SETs blocked and retained
        |
        | prefixes and references reach zero
        v
REMOVED

The implementation has three parts:

1. The whole-interface DEL owns the removal marker.

   Prefix deletion no longer clears m_removingIntfses. The marker is cleared only when the whole-interface deletion succeeds.

2. Addition paths explicitly check removal state.

   Standard local route, neighbor, and neighbor-next-hop creation calls isIntfRemovalPending() before obtaining the RIF. If removal is pending, the addition returns false, leaving the original APP_DB operation in that orchagent's m_toSync queue.

   The existing getRouterIntfsId() accessor is unchanged. Existing dependencies and deletion paths must still find the old RIF so they can remove themselves and release its reference count. Guarding the getter itself would block both additions and deletions and deadlock the drain.

3. RouteOrch honors deferred neighbor-next-hop creation.

   Callers no longer request a next-hop ID immediately after NeighOrch::addNextHop() reports that creation was deferred.

The resulting execution is:

attempt interface DEL → retained; removal fence established
prefix/dependency DELs → allowed to drain old state
new route/neighbor SETs → retained, not attached to old RIF
retry interface DEL → succeeds
retained additions → retry only after an interface is available again

Why I did it

Why not enforce one global APP_DB order?

APP_DB StateTable is primarily a latest desired-state transport, not a durable transactional command log.

Even if one producer emits operations in the desired order, that does not create a global execution order because:

• IntfsOrch keeps interface keys in a key-sorted multimap.
• Interface, route, neighbor, and next-hop work use different APP_DB tables and different consumers.
• A blocked operation remains queued while other work continues.
• Multiple producers operate independently.
• After restart, orchagent reconstructs work from current database state, not the original event history.

Replacing one multimap with a FIFO or priority queue would therefore order only one table. It would not order IntfsOrch against RouteOrch and NeighOrch, and a single blocked entry could introduce head-of-line blocking for unrelated interfaces.

The established SWSS design is instead:

recognize an unmet dependency
→ return false
→ retain the operation
→ let prerequisite deletions proceed
→ retry later

This PR follows that design and adds a local per-interface admission fence. It does not claim to provide a transaction or generation protocol across all orchagents.

Scope

The guarded creation paths are the standard local interface route, neighbor, and neighbor-next-hop paths involved in the traced failure. Remote-system-port behavior is explicitly unchanged; its pre-existing inband RIF identity, bookkeeping, and boot-order issues are being handled independently.

How I verified it

• Extended IntfsOrchDeleteCreateRetry with the real lexical sequence: whole-interface DEL, then prefix DEL.
• The regression verifies that:
  • the whole-interface DEL remains queued;
  • prefix deletion does not clear the removal fence;
  • old references can drain;
  • the retained whole-interface deletion then succeeds.
• Added LocalNeighborDependenciesRetryWhileInterfaceRemovalPending to verify that local neighbor and next-hop additions return retry while the fence is active.
• The earlier combined branches containing this fence completed the amd64, ARM, ASAN, Docker, and Trixie compile stages in Azure builds 1168368 (master) and 1168367 (202605). Their vstest jobs stopped before executing tests because those workers lacked pip3.
• An earlier combined candidate, hardware build 1167872, passed traced and production 1x/5x runs and all ten production 10x test bodies. This PR is the smaller local deletion-fence subset; exact split-commit validation is provided by this PR's CI.

Current exact-head master PR build

• Head: 4c38411af466f770e8eb7282423443eed43fe872
• Fresh Azure PR build: 1171673 — in progress.
• Prior exact-head build 1171069 passed amd64, arm64, armhf, ASAN, Docker, Docker-ASAN, Trixie, and TestAsan vstest.
• Its regular Test vstest lane was not green after unrelated VNET, P4RT, and ACL full-suite flakes across retries. No failure in the focused interface-removal coverage was observed.
• CodeQL, Semgrep, DCO, EasyCLA, and the exact-head Copilot review are clean.
• No local build was used; validation is PR-build only.

202605 PR-build validation

• Validation-only PR: #4747
• The validation branch is being rebuilt from current 202605, which already contains the shared PR-CI dependency fix.
• The complete dependency-ordered stack contains merged PR [intfsorch]Fix race condition between VRF bind and ip address removal #4758 followed by user PRs [intfsorch]: Fence new RIF dependencies during interface removal #4746, [intfsorch]: Retry loopback action update failures #4766, [neighorch]: Track remote next hops on inband RIF #4769, [neighorch]: Avoid inserting missing bulk neighbors #4772, [intfsorch]: Retry explicit router interface VRF rehome #4764, and [neighorch]: Retry until inband port is available #4770.
• Exact validation head: d4b0502ecdac76d56fdf063a633045c0fb0de977
• Azure PR build: 1171672 — in progress.
• The Tested for 202605 branch label must remain absent until that exact full-stack build is complete.

Details if related

• Follow-up VRF rehome retry: PR #4764
• Microsoft ADO: 38514061
• Merge batch 1. PR [intfsorch]: Retry explicit router interface VRF rehome #4764 must follow this PR.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR fixes a race in the interface/VRF event flow that can cause an interface IP’s IP2Me (trap-to-CPU) /32 route to be programmed into the wrong virtual router when the interface is (re)bound to a non-default VRF. This is in the core SWSS control-plane path (cfgmgr → APP_DB INTF_TABLE → orchagent IntfsOrch → SAI route programming), and directly impacts correctness of return traffic (e.g., DHCP relay giaddr flows) in non-default VRFs.

Changes:

• Propagate the interface’s VRF binding into the IP-address INTF_TABLE entry (vrf_name) so orchagent has an explicit target VRF for IP adds.
• In IntfsOrch::setIntf, defer installing the IP2Me route until the interface RIF’s current VR matches the configured VRF to avoid programming the route in a stale/previous VRF.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
orchagent/intfsorch.cpp	Defers interface-IP IP2Me route creation until the RIF has moved to the configured VRF, preventing wrong-VRF route installs during VRF bind races.
cfgmgr/intfmgr.cpp	Adds vrf_name to IP-key INTF_TABLE entries by reading the interface VRF binding from STATE_DB, enabling correct VRF selection in orchagent.

Comments suppressed due to low confidence (1)

orchagent/intfsorch.cpp:613

• The new VRF-mismatch deferral is a behavior change that should be covered by a regression test: when an interface IP add arrives while the RIF is still in the previous VRF, setIntf() should return false (stay in toSync), and then succeed once the RIF is recreated in the target VRF (installing the IP2Me route in that VRF). Adding a focused mock test would prevent future refactors from reintroducing the race.

    addIp2MeRoute(port.m_vr_id, *ip_prefix);

    if(gMySwitchType == "voq")
    {
        if(gPortsOrch->isInbandPort(alias))
        {
            //Need to sync the inband intf neighbor for other asics

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[mssonicbld]

/azp run

[azure-pipelines]

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

[mssonicbld]

/azp run

[azure-pipelines]

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

[mssonicbld]

/azp run

[azure-pipelines]

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

[mssonicbld]

/azp run

[azure-pipelines]

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.

[mssonicbld]

/azp run

[azure-pipelines]

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

[mssonicbld]

/azp run

[azure-pipelines]

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

[mssonicbld]

/azp run

[azure-pipelines]

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

[Xichen96]

Refreshing the unchanged exact head for a full PR build; the previous regular vstest lane ended on unrelated full-suite flakes.

[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).

[mssonicbld]

This PR has backport request label(s) for branch(es): 202605, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202605: <test result>) in the Test result section as well in your PR description.

_{---Powered by SONiC BuildBot}

[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).

[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).

[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).

[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).