From 707997b91287c012f809c8659b49a7bff78aad7f Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 20 Jul 2026 16:29:26 +0000 Subject: [PATCH 1/7] [neighorch]: Track remote next hops on inband RIF Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/neighorch.cpp | 17 +++++-- tests/mock_tests/neighorch_ut.cpp | 73 +++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 7ee60a95803..35335c6ca11 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -379,7 +379,7 @@ bool NeighOrch::addNextHop(NeighborContext& ctx) } assert(!hasNextHop(nexthop)); - sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(nh.alias); + sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(nexthop.alias); vector next_hop_attrs; @@ -457,7 +457,7 @@ bool NeighOrch::addNextHop(NeighborContext& ctx) next_hop_entry.nh_flags = 0; m_syncdNextHops[nexthop] = next_hop_entry; - m_intfsOrch->increaseRouterIntfsRefCount(nh.alias); + m_intfsOrch->increaseRouterIntfsRefCount(nexthop.alias); if (nexthop.isMplsNextHop()) { @@ -516,6 +516,15 @@ bool NeighOrch::processBulkAddNextHop(NeighborContext& ctx) } NextHopKey nexthop(nh); + if (m_intfsOrch->isRemoteSystemPortIntf(nh.alias)) + { + Port inbp; + gPortsOrch->getInbandPort(inbp); + assert(inbp.m_alias.length()); + + nexthop.alias = inbp.m_alias; + } + if (ctx.next_hop_id == SAI_NULL_OBJECT_ID) { sai_status_t bulker_status = gNextHopBulker.create_status(ctx.next_hop_id); @@ -549,7 +558,7 @@ bool NeighOrch::processBulkAddNextHop(NeighborContext& ctx) next_hop_entry.nh_flags = 0; m_syncdNextHops[nexthop] = next_hop_entry; - m_intfsOrch->increaseRouterIntfsRefCount(nh.alias); + m_intfsOrch->increaseRouterIntfsRefCount(nexthop.alias); if (nexthop.isMplsNextHop()) { @@ -784,7 +793,7 @@ bool NeighOrch::removeNextHop(const IpAddress &ipAddress, const string &alias) } m_syncdNextHops.erase(nexthop); - m_intfsOrch->decreaseRouterIntfsRefCount(alias); + m_intfsOrch->decreaseRouterIntfsRefCount(nexthop.alias); return true; } diff --git a/tests/mock_tests/neighorch_ut.cpp b/tests/mock_tests/neighorch_ut.cpp index dc08749c370..fceb38d8229 100644 --- a/tests/mock_tests/neighorch_ut.cpp +++ b/tests/mock_tests/neighorch_ut.cpp @@ -17,7 +17,9 @@ EXTERN_MOCK_FNS namespace neighorch_test { DEFINE_SAI_API_MOCK(neighbor); + DEFINE_SAI_GENERIC_API_MOCK(next_hop, next_hop); using namespace std; + using ::testing::Invoke; using namespace mock_orch_test; using ::testing::Return; using ::testing::Throw; @@ -49,6 +51,27 @@ namespace neighorch_test neigh_table.del(key); } + void ConfigureRemoteSystemPort(const string& remote_alias, const string& inband_alias) + { + Table intf_table = Table(m_app_db.get(), APP_INTF_TABLE_NAME); + intf_table.set(remote_alias, {{"NULL", "NULL"}}); + intf_table.set(inband_alias, {{"NULL", "NULL"}}); + gIntfsOrch->addExistingData(&intf_table); + static_cast(gIntfsOrch)->doTask(); + + Port remote_port; + ASSERT_TRUE(gPortsOrch->getPort(remote_alias, remote_port)); + remote_port.m_system_port_info.type = SAI_SYSTEM_PORT_TYPE_REMOTE; + remote_port.m_oper_status = SAI_PORT_OPER_STATUS_UP; + gPortsOrch->setPort(remote_alias, remote_port); + + Port inband_port; + ASSERT_TRUE(gPortsOrch->getPort(inband_alias, inband_port)); + inband_port.m_oper_status = SAI_PORT_OPER_STATUS_UP; + gPortsOrch->setPort(inband_alias, inband_port); + gPortsOrch->m_inbandPortName = inband_alias; + } + void ApplyInitialConfigs() { Table port_table = Table(m_app_db.get(), APP_PORT_TABLE_NAME); @@ -180,15 +203,65 @@ namespace neighorch_test void PostSetUp() override { INIT_SAI_API_MOCK(neighbor); + INIT_SAI_API_MOCK(next_hop); MockSaiApis(); } void PreTearDown() override { RestoreSaiApis(); + DEINIT_SAI_API_MOCK(next_hop); + DEINIT_SAI_API_MOCK(neighbor); } }; + TEST_F(NeighOrchTest, RemoteSystemPortNextHopUsesInbandRif) + { + ConfigureRemoteSystemPort(ETHERNET0, ETHERNET4); + + auto inband_rif = gIntfsOrch->getRouterIntfsId(ETHERNET4); + ASSERT_NE(inband_rif, SAI_NULL_OBJECT_ID); + auto remote_ref_count = gIntfsOrch->getSyncdIntfses().at(ETHERNET0).ref_count; + auto inband_ref_count = gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count; + NextHopKey inband_nexthop(TEST_IP, ETHERNET4); + + bool saw_inband_rif = false; + EXPECT_CALL(*mock_sai_next_hop_api, create_next_hop) + .WillOnce(Invoke([&](sai_object_id_t *next_hop_id, sai_object_id_t, uint32_t attr_count, + const sai_attribute_t *attr_list) { + *next_hop_id = 0x200000; + for (uint32_t i = 0; i < attr_count; ++i) + { + if (attr_list[i].id == SAI_NEXT_HOP_ATTR_ROUTER_INTERFACE_ID) + { + EXPECT_EQ(attr_list[i].value.oid, inband_rif); + saw_inband_rif = true; + } + } + return SAI_STATUS_SUCCESS; + })); + + NeighborContext ctx(NeighborEntry(TEST_IP, ETHERNET0)); + ASSERT_TRUE(gNeighOrch->addNextHop(ctx)); + ASSERT_TRUE(saw_inband_rif); + ASSERT_EQ(gNeighOrch->m_syncdNextHops.count(inband_nexthop), 1u); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET0).ref_count, remote_ref_count); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count + 1); + + ASSERT_TRUE(gNeighOrch->removeNextHop(IpAddress(TEST_IP), ETHERNET0)); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count); + + NeighborContext bulk_ctx(NeighborEntry(TEST_IP, ETHERNET0), true); + bulk_ctx.next_hop_id = 0x200001; + ASSERT_TRUE(gNeighOrch->processBulkAddNextHop(bulk_ctx)); + ASSERT_EQ(gNeighOrch->m_syncdNextHops.count(inband_nexthop), 1u); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET0).ref_count, remote_ref_count); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count + 1); + + ASSERT_TRUE(gNeighOrch->removeNextHop(IpAddress(TEST_IP), ETHERNET0)); + ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count); + } + TEST_F(NeighOrchTest, MultiVlanDuplicateNeighbor) { EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry); From 4fa09fc8310d0fc043a63f889f6d947a31774c07 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 20 Jul 2026 17:07:15 +0000 Subject: [PATCH 2/7] [neighorch]: Clarify remote next-hop port semantics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/neighorch.cpp | 4 ++-- tests/mock_tests/neighorch_ut.cpp | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 35335c6ca11..0294e9cfeda 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -787,8 +787,8 @@ bool NeighOrch::removeNextHop(const IpAddress &ipAddress, const string &alias) if (m_syncdNextHops[nexthop].ref_count > 0) { - SWSS_LOG_ERROR("Failed to remove still referenced next hop %s on %s", - ipAddress.to_string().c_str(), alias.c_str()); + SWSS_LOG_ERROR("Failed to remove still referenced next hop %s requested on %s, tracked on %s", + ipAddress.to_string().c_str(), alias.c_str(), nexthop.alias.c_str()); return false; } diff --git a/tests/mock_tests/neighorch_ut.cpp b/tests/mock_tests/neighorch_ut.cpp index fceb38d8229..a925b1d4640 100644 --- a/tests/mock_tests/neighorch_ut.cpp +++ b/tests/mock_tests/neighorch_ut.cpp @@ -219,6 +219,15 @@ namespace neighorch_test { ConfigureRemoteSystemPort(ETHERNET0, ETHERNET4); + Port remote_port; + ASSERT_TRUE(gPortsOrch->getPort(ETHERNET0, remote_port)); + remote_port.m_oper_status = SAI_PORT_OPER_STATUS_DOWN; + gPortsOrch->setPort(ETHERNET0, remote_port); + + Port inband_port; + ASSERT_TRUE(gPortsOrch->getPort(ETHERNET4, inband_port)); + ASSERT_EQ(inband_port.m_oper_status, SAI_PORT_OPER_STATUS_UP); + auto inband_rif = gIntfsOrch->getRouterIntfsId(ETHERNET4); ASSERT_NE(inband_rif, SAI_NULL_OBJECT_ID); auto remote_ref_count = gIntfsOrch->getSyncdIntfses().at(ETHERNET0).ref_count; @@ -245,6 +254,7 @@ namespace neighorch_test ASSERT_TRUE(gNeighOrch->addNextHop(ctx)); ASSERT_TRUE(saw_inband_rif); ASSERT_EQ(gNeighOrch->m_syncdNextHops.count(inband_nexthop), 1u); + ASSERT_TRUE(gNeighOrch->isNextHopFlagSet(inband_nexthop, NHFLAGS_IFDOWN)); ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET0).ref_count, remote_ref_count); ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count + 1); @@ -255,6 +265,7 @@ namespace neighorch_test bulk_ctx.next_hop_id = 0x200001; ASSERT_TRUE(gNeighOrch->processBulkAddNextHop(bulk_ctx)); ASSERT_EQ(gNeighOrch->m_syncdNextHops.count(inband_nexthop), 1u); + ASSERT_TRUE(gNeighOrch->isNextHopFlagSet(inband_nexthop, NHFLAGS_IFDOWN)); ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET0).ref_count, remote_ref_count); ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count + 1); From 6af04e5625c38095eebbf15353e07301b9b08079 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 20 Jul 2026 16:31:40 +0000 Subject: [PATCH 3/7] [neighorch]: Retry until inband port is available Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/neighorch.cpp | 21 +++++++++++++++------ tests/mock_tests/neighorch_ut.cpp | 13 +++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 0294e9cfeda..6dbdab8a5e4 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -372,8 +372,11 @@ bool NeighOrch::addNextHop(NeighborContext& ctx) { //For remote system ports kernel nexthops are always on inband. Change the key Port inbp; - gPortsOrch->getInbandPort(inbp); - assert(inbp.m_alias.length()); + if (!gPortsOrch->getInbandPort(inbp)) + { + SWSS_LOG_INFO("Inband port is not available for remote next hop %s", nh.to_string().c_str()); + return false; + } nexthop.alias = inbp.m_alias; } @@ -519,8 +522,11 @@ bool NeighOrch::processBulkAddNextHop(NeighborContext& ctx) if (m_intfsOrch->isRemoteSystemPortIntf(nh.alias)) { Port inbp; - gPortsOrch->getInbandPort(inbp); - assert(inbp.m_alias.length()); + if (!gPortsOrch->getInbandPort(inbp)) + { + SWSS_LOG_INFO("Inband port is not available for remote next hop %s", nh.to_string().c_str()); + return false; + } nexthop.alias = inbp.m_alias; } @@ -775,8 +781,11 @@ bool NeighOrch::removeNextHop(const IpAddress &ipAddress, const string &alias) { //For remote system ports kernel nexthops are always on inband. Change the key Port inbp; - gPortsOrch->getInbandPort(inbp); - assert(inbp.m_alias.length()); + if (!gPortsOrch->getInbandPort(inbp)) + { + SWSS_LOG_INFO("Inband port is not available for remote next hop %s", nexthop.to_string().c_str()); + return false; + } nexthop.alias = inbp.m_alias; } diff --git a/tests/mock_tests/neighorch_ut.cpp b/tests/mock_tests/neighorch_ut.cpp index a925b1d4640..dd0fee1d3da 100644 --- a/tests/mock_tests/neighorch_ut.cpp +++ b/tests/mock_tests/neighorch_ut.cpp @@ -273,6 +273,19 @@ namespace neighorch_test ASSERT_EQ(gIntfsOrch->getSyncdIntfses().at(ETHERNET4).ref_count, inband_ref_count); } + TEST_F(NeighOrchTest, RemoteSystemPortNextHopRetriesWithoutInbandPort) + { + ConfigureRemoteSystemPort(ETHERNET0, ETHERNET4); + gPortsOrch->m_inbandPortName.clear(); + + NeighborContext ctx(NeighborEntry(TEST_IP, ETHERNET0), true); + ctx.next_hop_id = 0x200001; + + EXPECT_FALSE(gNeighOrch->addNextHop(ctx)); + EXPECT_FALSE(gNeighOrch->processBulkAddNextHop(ctx)); + EXPECT_FALSE(gNeighOrch->removeNextHop(IpAddress(TEST_IP), ETHERNET0)); + } + TEST_F(NeighOrchTest, MultiVlanDuplicateNeighbor) { EXPECT_CALL(*mock_sai_neighbor_api, create_neighbor_entry); From 44c012a8f831f30cc93d49b462a3b3e0b82aad23 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 20 Jul 2026 17:23:53 +0000 Subject: [PATCH 4/7] [neighorch]: Propagate inband retry failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/neighorch.cpp | 47 +++++++++++++++++++++++-------- orchagent/routeorch.cpp | 10 +++++-- tests/mock_tests/neighorch_ut.cpp | 9 ++++++ 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 6dbdab8a5e4..cee688677d8 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -1501,9 +1501,12 @@ bool NeighOrch::addNeighbor(NeighborContext& ctx) if (bulk_op) { SWSS_LOG_INFO("Adding neighbor entry %s on %s to bulker.", ip_address.to_string().c_str(), alias.c_str()); + if (!addNextHop(ctx)) + { + return false; + } object_statuses.emplace_back(); gNeighBulker.create_entry(&object_statuses.back(), &neighbor_entry, (uint32_t)neighbor_attrs.size(), neighbor_attrs.data()); - addNextHop(ctx); return true; } @@ -1629,6 +1632,12 @@ bool NeighOrch::removeNeighbor(NeighborContext& ctx, bool disable) bool bulk_op = ctx.bulk_op; NextHopKey nexthop = { ip_address, alias }; + auto neighborIt = m_syncdNeighbors.find(neighborEntry); + if (neighborIt == m_syncdNeighbors.end()) + { + return true; + } + sai_object_id_t port_vrf_id; port_vrf_id = gVirtualRouterId; @@ -1636,8 +1645,11 @@ bool NeighOrch::removeNeighbor(NeighborContext& ctx, bool disable) { //For remote system ports kernel nexthops are always on inband. Change the key Port inbp; - gPortsOrch->getInbandPort(inbp); - assert(inbp.m_alias.length()); + if (!gPortsOrch->getInbandPort(inbp)) + { + SWSS_LOG_INFO("Inband port is not available for remote neighbor %s", nexthop.to_string().c_str()); + return false; + } nexthop.alias = inbp.m_alias; } @@ -1652,12 +1664,6 @@ bool NeighOrch::removeNeighbor(NeighborContext& ctx, bool disable) SWSS_LOG_ERROR("Port does not exist for %s!", alias.c_str()); } - auto neighborIt = m_syncdNeighbors.find(neighborEntry); - if (neighborIt == m_syncdNeighbors.end()) - { - return true; - } - SWSS_LOG_INFO("Try to remove neighbor %s on %s", ip_address.to_string().c_str(), alias.c_str()); @@ -1760,7 +1766,10 @@ bool NeighOrch::removeNeighbor(NeighborContext& ctx, bool disable) gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEIGHBOR); } - removeNextHop(ip_address, alias); + if (!removeNextHop(ip_address, nexthop.alias)) + { + return false; + } m_intfsOrch->decreaseRouterIntfsRefCount(alias); SWSS_LOG_NOTICE("Removed neighbor %s on %s", m_syncdNeighbors[neighborEntry].mac.to_string().c_str(), alias.c_str()); @@ -1909,12 +1918,25 @@ bool NeighOrch::processBulkDisableNeighbor(NeighborContext& ctx) const NeighborEntry neighborEntry = ctx.neighborEntry; string alias = neighborEntry.alias; IpAddress ip_address = neighborEntry.ip_address; + NextHopKey nexthop = { ip_address, alias }; if (m_syncdNeighbors.find(neighborEntry) == m_syncdNeighbors.end()) { return true; } + if (m_intfsOrch->isRemoteSystemPortIntf(alias)) + { + Port inbp; + if (!gPortsOrch->getInbandPort(inbp)) + { + SWSS_LOG_INFO("Inband port is not available for remote neighbor %s", nexthop.to_string().c_str()); + return false; + } + + nexthop.alias = inbp.m_alias; + } + SWSS_LOG_INFO("Checking neighbor remove entry status %s on %s.", ip_address.to_string().c_str(), m_syncdNeighbors[neighborEntry].mac.to_string().c_str()); if (isHwConfigured(neighborEntry)) @@ -1990,7 +2012,10 @@ bool NeighOrch::processBulkDisableNeighbor(NeighborContext& ctx) gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEIGHBOR); } - removeNextHop(ip_address, alias); + if (!removeNextHop(ip_address, nexthop.alias)) + { + return false; + } m_intfsOrch->decreaseRouterIntfsRefCount(alias); SWSS_LOG_NOTICE("Removed neighbor %s on %s", m_syncdNeighbors[neighborEntry].mac.to_string().c_str(), alias.c_str()); diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index 5ef3ba6f7ea..fec0fc87618 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 @@ -2118,7 +2121,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/neighorch_ut.cpp b/tests/mock_tests/neighorch_ut.cpp index dd0fee1d3da..1db64893f73 100644 --- a/tests/mock_tests/neighorch_ut.cpp +++ b/tests/mock_tests/neighorch_ut.cpp @@ -280,10 +280,19 @@ namespace neighorch_test NeighborContext ctx(NeighborEntry(TEST_IP, ETHERNET0), true); ctx.next_hop_id = 0x200001; + ctx.mac = MacAddress(MAC1); EXPECT_FALSE(gNeighOrch->addNextHop(ctx)); EXPECT_FALSE(gNeighOrch->processBulkAddNextHop(ctx)); EXPECT_FALSE(gNeighOrch->removeNextHop(IpAddress(TEST_IP), ETHERNET0)); + EXPECT_FALSE(gNeighOrch->addNeighbor(ctx)); + EXPECT_TRUE(ctx.object_statuses.empty()); + + gNeighOrch->m_syncdNeighbors[ctx.neighborEntry] = {ctx.mac, true}; + EXPECT_FALSE(gNeighOrch->removeNeighbor(ctx)); + EXPECT_EQ(gNeighOrch->m_syncdNeighbors.count(ctx.neighborEntry), 1u); + EXPECT_FALSE(gNeighOrch->processBulkDisableNeighbor(ctx)); + EXPECT_EQ(gNeighOrch->m_syncdNeighbors.count(ctx.neighborEntry), 1u); } TEST_F(NeighOrchTest, MultiVlanDuplicateNeighbor) From f8230187a27cd86e643d5e32ffc0db5071a5f08b Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 21 Jul 2026 11:19:17 +0000 Subject: [PATCH 5/7] [neighorch]: Retry remote MPLS next hop removal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/mplsrouteorch.cpp | 19 ++++++++++++++++++ orchagent/neighorch.cpp | 8 ++++++-- orchagent/nhgorch.cpp | 17 ++++++++++++++++ orchagent/routeorch.cpp | 18 +++++++++++++++++ tests/mock_tests/neighorch_ut.cpp | 8 ++++++++ tests/mock_tests/routeorch_ut.cpp | 32 +++++++++++++++++++++++++++++++ 6 files changed, 100 insertions(+), 2 deletions(-) diff --git a/orchagent/mplsrouteorch.cpp b/orchagent/mplsrouteorch.cpp index cce3fe5ca2c..4877d8b475a 100644 --- a/orchagent/mplsrouteorch.cpp +++ b/orchagent/mplsrouteorch.cpp @@ -16,6 +16,7 @@ extern sai_object_id_t gSwitchId; extern CrmOrch *gCrmOrch; extern NhgOrch *gNhgOrch; extern CbfNhgOrch *gCbfNhgOrch; +extern PortsOrch *gPortsOrch; void RouteOrch::doLabelTask(ConsumerBase& consumer) { @@ -876,6 +877,24 @@ bool RouteOrch::removeLabelRoute(LabelRouteBulkContext& ctx) return true; } + if (it_route->second.nhg_index.empty()) + { + for (const auto& nexthop : it_route->second.nhg_key.getNextHops()) + { + if (nexthop.isMplsNextHop() && + m_intfsOrch->isRemoteSystemPortIntf(nexthop.alias)) + { + Port inbp; + if (!gPortsOrch->getInbandPort(inbp)) + { + SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", + nexthop.to_string().c_str()); + return false; + } + } + } + } + auto& object_statuses = ctx.object_statuses; object_statuses.emplace_back(); diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index cee688677d8..a35738a327c 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -815,8 +815,12 @@ bool NeighOrch::removeMplsNextHop(const NextHopKey& nh) { //For remote system ports kernel nexthops are always on inband. Change the key Port inbp; - gPortsOrch->getInbandPort(inbp); - assert(inbp.m_alias.length()); + if (!gPortsOrch->getInbandPort(inbp)) + { + SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", + nexthop.to_string().c_str()); + return false; + } nexthop.alias = inbp.m_alias; } diff --git a/orchagent/nhgorch.cpp b/orchagent/nhgorch.cpp index 548072740cc..d51db5edcdf 100644 --- a/orchagent/nhgorch.cpp +++ b/orchagent/nhgorch.cpp @@ -878,6 +878,23 @@ bool NextHopGroup::remove() { return true; } + + for (const auto& member : m_members) + { + const auto& nh_key = member.first; + if (nh_key.isMplsNextHop() && + gIntfsOrch->isRemoteSystemPortIntf(nh_key.alias)) + { + Port inbp; + if (!gPortsOrch->getInbandPort(inbp)) + { + SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", + nh_key.to_string().c_str()); + return false; + } + } + } + // If the group is temporary or non-recursive, update the neigh or rif ref-count and reset the ID. if (m_is_temp || (!isRecursive() && m_members.size() == 1)) diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index fec0fc87618..ff663bac480 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -2798,6 +2798,24 @@ bool RouteOrch::removeRoute(RouteBulkContext& ctx) return true; } + if (it_route != it_route_table->second.end() && it_route->second.nhg_index.empty()) + { + for (const auto& nexthop : it_route->second.nhg_key.getNextHops()) + { + if (nexthop.isMplsNextHop() && + m_intfsOrch->isRemoteSystemPortIntf(nexthop.alias)) + { + Port inbp; + if (!gPortsOrch->getInbandPort(inbp)) + { + SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", + nexthop.to_string().c_str()); + return false; + } + } + } + } + auto& object_statuses = ctx.object_statuses; // set to blackhole for default route diff --git a/tests/mock_tests/neighorch_ut.cpp b/tests/mock_tests/neighorch_ut.cpp index 1db64893f73..20acdd7f02a 100644 --- a/tests/mock_tests/neighorch_ut.cpp +++ b/tests/mock_tests/neighorch_ut.cpp @@ -293,6 +293,14 @@ namespace neighorch_test EXPECT_EQ(gNeighOrch->m_syncdNeighbors.count(ctx.neighborEntry), 1u); EXPECT_FALSE(gNeighOrch->processBulkDisableNeighbor(ctx)); EXPECT_EQ(gNeighOrch->m_syncdNeighbors.count(ctx.neighborEntry), 1u); + + NextHopKey remote_mpls_nexthop("push100+" + TEST_IP + "@" + ETHERNET0); + NextHopKey inband_mpls_nexthop(remote_mpls_nexthop); + inband_mpls_nexthop.alias = ETHERNET4; + gNeighOrch->m_syncdNextHops[inband_mpls_nexthop] = {0x200002, 0, 0}; + + EXPECT_FALSE(gNeighOrch->removeMplsNextHop(remote_mpls_nexthop)); + EXPECT_EQ(gNeighOrch->m_syncdNextHops.count(inband_mpls_nexthop), 1u); } TEST_F(NeighOrchTest, MultiVlanDuplicateNeighbor) diff --git a/tests/mock_tests/routeorch_ut.cpp b/tests/mock_tests/routeorch_ut.cpp index 93b6d2f9cc7..b0fed7e8da4 100644 --- a/tests/mock_tests/routeorch_ut.cpp +++ b/tests/mock_tests/routeorch_ut.cpp @@ -1733,4 +1733,36 @@ namespace routeorch_test (void)gRouteOrch->removeRoutePrefix(IpPrefix("7.7.7.0/24")); ASSERT_TRUE(gRouteOrch->removeRoutePrefix(IpPrefix("7.7.7.0/24"))); } + + TEST_F(RouteOrchTest, RemoteMplsRouteRemovalRetriesWithoutInbandPort) + { + Port remote_port; + ASSERT_TRUE(gPortsOrch->getPort("Ethernet0", remote_port)); + remote_port.m_system_port_info.type = SAI_SYSTEM_PORT_TYPE_REMOTE; + gPortsOrch->setPort("Ethernet0", remote_port); + gPortsOrch->m_inbandPortName.clear(); + + NextHopGroupKey nhg( + "push100+10.0.0.2@Ethernet0,push200+10.0.0.3@Ethernet0"); + + IpPrefix prefix("8.8.8.0/24"); + gRouteOrch->m_syncdRoutes[gVirtualRouterId][prefix] = RouteNhg(nhg, ""); + + RouteBulkContext route_ctx(prefix.to_string(), false); + route_ctx.vrf_id = gVirtualRouterId; + route_ctx.ip_prefix = prefix; + + EXPECT_FALSE(gRouteOrch->removeRoute(route_ctx)); + EXPECT_TRUE(route_ctx.object_statuses.empty()); + + Label label = 1000; + gRouteOrch->m_syncdLabelRoutes[gVirtualRouterId][label] = RouteNhg(nhg, ""); + + LabelRouteBulkContext label_ctx; + label_ctx.vrf_id = gVirtualRouterId; + label_ctx.label = label; + + EXPECT_FALSE(gRouteOrch->removeLabelRoute(label_ctx)); + EXPECT_TRUE(label_ctx.object_statuses.empty()); + } } From b2f2a01bcde8984990686ad14f76694e4a8bd2b8 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Wed, 22 Jul 2026 03:13:20 +1000 Subject: [PATCH 6/7] [neighorch]: Hoist remote MPLS inband preflight Check the global inband dependency once per route or next-hop group before beginning removal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- orchagent/mplsrouteorch.cpp | 24 ++++++++++++++---------- orchagent/nhgorch.cpp | 26 +++++++++++++++----------- orchagent/routeorch.cpp | 24 ++++++++++++++---------- 3 files changed, 43 insertions(+), 31 deletions(-) diff --git a/orchagent/mplsrouteorch.cpp b/orchagent/mplsrouteorch.cpp index 4877d8b475a..161f14d3046 100644 --- a/orchagent/mplsrouteorch.cpp +++ b/orchagent/mplsrouteorch.cpp @@ -879,18 +879,22 @@ bool RouteOrch::removeLabelRoute(LabelRouteBulkContext& ctx) if (it_route->second.nhg_index.empty()) { - for (const auto& nexthop : it_route->second.nhg_key.getNextHops()) + const auto& nexthops = it_route->second.nhg_key.getNextHops(); + const auto remote_mpls_nexthop = find_if(nexthops.begin(), nexthops.end(), + [this](const auto& nexthop) + { + return nexthop.isMplsNextHop() && + m_intfsOrch->isRemoteSystemPortIntf(nexthop.alias); + }); + + if (remote_mpls_nexthop != nexthops.end()) { - if (nexthop.isMplsNextHop() && - m_intfsOrch->isRemoteSystemPortIntf(nexthop.alias)) + Port inbp; + if (!gPortsOrch->getInbandPort(inbp)) { - Port inbp; - if (!gPortsOrch->getInbandPort(inbp)) - { - SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", - nexthop.to_string().c_str()); - return false; - } + SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", + remote_mpls_nexthop->to_string().c_str()); + return false; } } } diff --git a/orchagent/nhgorch.cpp b/orchagent/nhgorch.cpp index d51db5edcdf..1c0678ba95a 100644 --- a/orchagent/nhgorch.cpp +++ b/orchagent/nhgorch.cpp @@ -1,3 +1,4 @@ +#include #include #include "nhgorch.h" #include "neighorch.h" @@ -879,19 +880,22 @@ bool NextHopGroup::remove() return true; } - for (const auto& member : m_members) + const auto remote_mpls_member = find_if(m_members.begin(), m_members.end(), + [](const auto& member) + { + const auto& nh_key = member.first; + return nh_key.isMplsNextHop() && + gIntfsOrch->isRemoteSystemPortIntf(nh_key.alias); + }); + + if (remote_mpls_member != m_members.end()) { - const auto& nh_key = member.first; - if (nh_key.isMplsNextHop() && - gIntfsOrch->isRemoteSystemPortIntf(nh_key.alias)) + Port inbp; + if (!gPortsOrch->getInbandPort(inbp)) { - Port inbp; - if (!gPortsOrch->getInbandPort(inbp)) - { - SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", - nh_key.to_string().c_str()); - return false; - } + SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", + remote_mpls_member->first.to_string().c_str()); + return false; } } diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index ff663bac480..94f0588182e 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -2800,18 +2800,22 @@ bool RouteOrch::removeRoute(RouteBulkContext& ctx) if (it_route != it_route_table->second.end() && it_route->second.nhg_index.empty()) { - for (const auto& nexthop : it_route->second.nhg_key.getNextHops()) + const auto& nexthops = it_route->second.nhg_key.getNextHops(); + const auto remote_mpls_nexthop = find_if(nexthops.begin(), nexthops.end(), + [this](const auto& nexthop) + { + return nexthop.isMplsNextHop() && + m_intfsOrch->isRemoteSystemPortIntf(nexthop.alias); + }); + + if (remote_mpls_nexthop != nexthops.end()) { - if (nexthop.isMplsNextHop() && - m_intfsOrch->isRemoteSystemPortIntf(nexthop.alias)) + Port inbp; + if (!gPortsOrch->getInbandPort(inbp)) { - Port inbp; - if (!gPortsOrch->getInbandPort(inbp)) - { - SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", - nexthop.to_string().c_str()); - return false; - } + SWSS_LOG_INFO("Inband port is not available for remote MPLS next hop %s", + remote_mpls_nexthop->to_string().c_str()); + return false; } } } From b7c61bb6e2a9ef0b7af97a003b850b061439fa94 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Wed, 22 Jul 2026 03:19:33 +1000 Subject: [PATCH 7/7] [neighorch]: Update remote next-hop RIF expectation Remote system-port next hops remain on the global inband RIF when their neighbor moves between remote ports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6b568a1-5d2a-4309-b1ac-ef21ca155079 Signed-off-by: Xichen96 --- tests/test_virtual_chassis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_virtual_chassis.py b/tests/test_virtual_chassis.py index 0a6f4bdbf8b..bb4fba0e108 100644 --- a/tests/test_virtual_chassis.py +++ b/tests/test_virtual_chassis.py @@ -1370,7 +1370,7 @@ def test_remote_neighbor_add(self, vct): nexthop_entry = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", nexthop_keys[0]) print("3:nexthop_entrty:",nexthop_entry) rif3 = nexthop_entry.get("SAI_NEXT_HOP_ATTR_ROUTER_INTERFACE_ID") - assert rif1 != rif3, "Neighbor is not replaced with new rif" + assert rif1 == rif3, "Remote neighbor next hop moved off the inband rif" #del the neighbor self.configure_neighbor(local_lc_dvs, "del", test_neigh_ip_1, test_neigh_mac_1, test_neigh_dev_2)