From b0cfd2d1c7955906459cf5bb444f977cc8b5c07f Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 20 Jul 2026 16:05:58 +0000 Subject: [PATCH 1/2] [intfsorch]: Fence new RIF dependencies during removal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/intfsorch.cpp | 15 +++++++++++++-- orchagent/intfsorch.h | 1 + orchagent/neighorch.cpp | 15 +++++++++++++++ orchagent/routeorch.cpp | 17 +++++++++++++++-- tests/mock_tests/intfsorch_ut.cpp | 15 ++++++++++++++- tests/mock_tests/neighorch_ut.cpp | 14 ++++++++++++++ 6 files changed, 72 insertions(+), 5 deletions(-) diff --git a/orchagent/intfsorch.cpp b/orchagent/intfsorch.cpp index a2b3078b0cc..5a1d38655e5 100644 --- a/orchagent/intfsorch.cpp +++ b/orchagent/intfsorch.cpp @@ -117,6 +117,11 @@ sai_object_id_t IntfsOrch::getRouterIntfsId(const string &alias) return port.m_rif_id; } +bool IntfsOrch::isIntfRemovalPending(const string &alias) const +{ + return m_removingIntfses.find(alias) != m_removingIntfses.end(); +} + bool IntfsOrch::isPrefixSubnet(const IpPrefix &ip_prefix, const string &alias) { if (m_syncdIntfses.find(alias) == m_syncdIntfses.end()) @@ -1204,12 +1209,18 @@ void IntfsOrch::doTask(Consumer &consumer) { if (removeIntf(alias, port.m_vr_id, ip_prefix_in_key ? &ip_prefix : nullptr)) { - m_removingIntfses.erase(alias); + if (!ip_prefix_in_key) + { + m_removingIntfses.erase(alias); + } it = consumer.m_toSync.erase(it); } else { - m_removingIntfses.insert(alias); + if (!ip_prefix_in_key) + { + m_removingIntfses.insert(alias); + } it++; continue; } diff --git a/orchagent/intfsorch.h b/orchagent/intfsorch.h index 5de9a7fb016..240af331c28 100644 --- a/orchagent/intfsorch.h +++ b/orchagent/intfsorch.h @@ -39,6 +39,7 @@ class IntfsOrch : public Orch static const int intfsorch_pri; sai_object_id_t getRouterIntfsId(const string&); + bool isIntfRemovalPending(const string&) const; bool isPrefixSubnet(const IpPrefix&, const string&); bool isInbandIntfInMgmtVrf(const string& alias); string getRouterIntfsAlias(const IpAddress &ip, const string &vrf_name = ""); diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 7ee60a95803..d31ab6440c1 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -379,6 +379,13 @@ bool NeighOrch::addNextHop(NeighborContext& ctx) } assert(!hasNextHop(nexthop)); + if (!m_intfsOrch->isRemoteSystemPortIntf(nh.alias) && + m_intfsOrch->isIntfRemovalPending(nh.alias)) + { + SWSS_LOG_INFO("Interface %s is pending removal", nh.alias.c_str()); + return false; + } + sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(nh.alias); vector next_hop_attrs; @@ -1324,6 +1331,13 @@ bool NeighOrch::addNeighbor(NeighborContext& ctx) string alias = neighborEntry.alias; bool bulk_op = ctx.bulk_op; + if (!m_intfsOrch->isRemoteSystemPortIntf(alias) && + m_intfsOrch->isIntfRemovalPending(alias)) + { + SWSS_LOG_INFO("Interface %s is pending removal", alias.c_str()); + return false; + } + sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(alias); if (rif_id == SAI_NULL_OBJECT_ID) { @@ -2065,6 +2079,7 @@ bool NeighOrch::enableNeighbors(std::list& bulk_ctx_list) if(!addNeighbor(*ctx)) { SWSS_LOG_ERROR("Neighbor %s create entry failed.", neighborEntry.ip_address.to_string().c_str()); + ret = false; continue; } } diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index 5ef3ba6f7ea..af13bea73d6 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -1507,7 +1507,10 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops) m_neighOrch->hasNextHop(NextHopKey(it.ip_address, it.alias))) { NeighborContext ctx = NeighborContext(it); - m_neighOrch->addNextHop(ctx); + if (!m_neighOrch->addNextHop(ctx)) + { + return false; + } next_hop_id = m_neighOrch->getNextHopId(it); } else @@ -2083,6 +2086,13 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) return true; } + if (!m_intfsOrch->isRemoteSystemPortIntf(nexthop.alias) && + m_intfsOrch->isIntfRemovalPending(nexthop.alias)) + { + SWSS_LOG_INFO("Interface %s is pending removal", nexthop.alias.c_str()); + return false; + } + next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias); /* rif is not created yet */ if (next_hop_id == SAI_NULL_OBJECT_ID) @@ -2118,7 +2128,10 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) { /* since IP neighbor NH exists, neighbor is resolved, add MPLS NH */ NeighborContext ctx = NeighborContext(nexthop); - m_neighOrch->addNextHop(ctx); + if (!m_neighOrch->addNextHop(ctx)) + { + return false; + } next_hop_id = m_neighOrch->getNextHopId(nexthop); } /* IP neighbor is not yet resolved */ diff --git a/tests/mock_tests/intfsorch_ut.cpp b/tests/mock_tests/intfsorch_ut.cpp index 558a47266ff..a0acec95843 100644 --- a/tests/mock_tests/intfsorch_ut.cpp +++ b/tests/mock_tests/intfsorch_ut.cpp @@ -313,17 +313,30 @@ namespace intfsorch_test static_cast(gIntfsOrch)->doTask(); ASSERT_EQ(current_create_count + 1, create_rif_count); + // Add a prefix so the whole-interface DEL is processed before its dependency DEL. + entries.clear(); + entries.push_back({"Ethernet0:10.0.0.1/24", "SET", { + {"scope", "global"}, + {"family", "IPv4"} + }}); + consumer->addToSync(entries); + static_cast(gIntfsOrch)->doTask(); + // create dependency to the interface gIntfsOrch->increaseRouterIntfsRefCount("Ethernet0"); - // delete the interface, expect retry because dependency exists + // Delete the interface and prefix in lexical order. The successful prefix + // DEL must not clear the whole-interface removal fence. entries.clear(); entries.push_back({"Ethernet0", "DEL", { {} }}); + entries.push_back({"Ethernet0:10.0.0.1/24", "DEL", { {} }}); consumer = dynamic_cast(gIntfsOrch->getExecutor(APP_INTF_TABLE_NAME)); consumer->addToSync(entries); auto current_remove_count = remove_rif_count; static_cast(gIntfsOrch)->doTask(); ASSERT_EQ(current_remove_count, remove_rif_count); + ASSERT_EQ(consumer->m_toSync.size(), 1u); + ASSERT_TRUE(gIntfsOrch->isIntfRemovalPending("Ethernet0")); // create the interface again, expect retry because interface is in removing entries.clear(); diff --git a/tests/mock_tests/neighorch_ut.cpp b/tests/mock_tests/neighorch_ut.cpp index dc08749c370..499799b113d 100644 --- a/tests/mock_tests/neighorch_ut.cpp +++ b/tests/mock_tests/neighorch_ut.cpp @@ -189,6 +189,20 @@ namespace neighorch_test } }; + TEST_F(NeighOrchTest, LocalNeighborDependenciesRetryWhileInterfaceRemovalPending) + { + gIntfsOrch->m_removingIntfses.insert(VLAN_1000); + + NeighborContext neighbor_ctx(VLAN1000_NEIGH); + neighbor_ctx.mac = MacAddress(MAC1); + EXPECT_FALSE(gNeighOrch->addNeighbor(neighbor_ctx)); + + NeighborContext next_hop_ctx(VLAN1000_NEIGH); + EXPECT_FALSE(gNeighOrch->addNextHop(next_hop_ctx)); + + gIntfsOrch->m_removingIntfses.erase(VLAN_1000); + } + TEST_F(NeighOrchTest, MultiVlanDuplicateNeighbor) { EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry); From 4c38411af466f770e8eb7282423443eed43fe872 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 20 Jul 2026 17:05:49 +0000 Subject: [PATCH 2/2] [intfsorch]: Fence VNET interface removal retries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/intfsorch.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/orchagent/intfsorch.cpp b/orchagent/intfsorch.cpp index 5a1d38655e5..2c1bb8d5b8d 100644 --- a/orchagent/intfsorch.cpp +++ b/orchagent/intfsorch.cpp @@ -1196,11 +1196,19 @@ void IntfsOrch::doTask(Consumer &consumer) if (vnet_orch->delIntf(alias, vnet_name, ip_prefix_in_key ? &ip_prefix : nullptr)) { + if (!ip_prefix_in_key) + { + m_removingIntfses.erase(alias); + } m_vnetInfses.erase(alias); it = consumer.m_toSync.erase(it); } else { + if (!ip_prefix_in_key) + { + m_removingIntfses.insert(alias); + } it++; continue; }